Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

document contribution policy for adding software to EESSI #108

Merged
merged 11 commits into from
Nov 9, 2023
Merged

document contribution policy for adding software to EESSI #108

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
544d928
document contribution policy for adding software to EESSI
boegel Jul 5, 2023
3e25c08
tweak initial contribution policy based on provided feedback
boegel Oct 24, 2023
d559a58
Merge branch 'main' into contrib_policy
boegel Oct 24, 2023
00bb6df
link to test-suite section in contribution policy
boegel Oct 24, 2023
86f99c3
rework v0.1.0 of contribution policy based on provided feedback
boegel Nov 9, 2023
4d00501
"It informs contributors"
boegel Nov 9, 2023
3d2600a
Merge branch 'main' into contrib_policy
boegel Nov 9, 2023
991f1e6
Merge branch 'contrib_policy' of github.com:boegel/docs into contrib_…
boegel Nov 9, 2023
5b4240f
fix links to other docs pages
boegel Nov 9, 2023
c580bfd
fix more links to other docs pages, consistently use .md in links to …
boegel Nov 9, 2023
cef6f85
fix typo
boegel Nov 9, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions docs/software_layer/adding_software.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@ To add software to EESSI, you should go through the semi-automatic software inst
* 3) Instructing the [bot :robot:](../bot.md) to deploy the built software for ingestion into the EESSI repository;
* 4) Merging the pull request once CI indicates that the software has been ingested. :white_check_mark:

!!! warning

Make sure you are also aware of our [contribution policy](contribution_policy.md) when adding software to EESSI.

### Preparation

Before you can make a pull request to the [software-layer](https://github.com/EESSI/software-layer),
Expand Down
80 changes: 80 additions & 0 deletions docs/software_layer/contribution_policy.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
# Contribution policy
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we add a version to the policy?


When [openining a pull request to add software to EESSI](adding_software.md), the following requirements must
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd start with a brief description of what this policy is for. For example,

The purpose of the contribution policy is to provide guidelines for adding software to the shared EESSI repository. It informs about what requirements a software to be added must meet. ...

Small typo openining $\longrightarrow$ opening. I'd rephrase this, however, to leave out the technical aspect (opening a pull request). E.g.,

Any software to be added to the EESSI repository must meet the following requirements:

1. Open Source Software only (see section X for details)
2. ...
3. ...

be taken into account.

!!! note

These requirements are subject to change, please check back regularly.

## Open source software

Only **open source software** can be added to the EESSI repository.

Make sure that you are aware of software license, and that redistribution is allowed.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"aware of relevant software licenses" ??


For more information about a specific license, see the [SPDX license list](https://spdx.org/licenses/).

!!! note

We intend to automatically verify that this requirement is met,
by requiring that the [SPDX license identifier](https://spdx.dev/ids/) is provided for all software.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Specifying the SPDX license identifier is probably not enough. Verifying it (that the identifier reflects the license) automatically is likely difficult.

I'd suggest to be rather restrictive at the start:

  1. Every software to be added must provide license information covering the full sources of the software package.
  2. For all dependencies of the software, license information covering the full sources must be provided too.
  3. License information must be given as SPDX license identifier.
  4. At the start (policy version 0.1) only the following license identifiers are accepted: list SPDX license identifiers

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that we want to start doing this, but this shouldn't be required in the first version of the policy.

We need to set up a mechanism for this first, for example a licenses.yaml file that maps software names to SPDX identifiers, and ideally also a way to detect that one or more entries are missing...

So, for now, I would keep it like it is now, and then work towards requiring that SPDX license identifiers can be provided somehow, and then update the policy accordingly once that is in place.


## Built by the bot

All software included in the EESSI repository *must* be built autonomously by [our bot :robot:](../bot.md),
see also the [semi-automatic software installation procedure](adding_software.md).

## Supported by EasyBuild

Currently, we require that all software being added to EESSI is supported by the *EasyBuild release* being used
to perform the installation.

That is, the easyconfig files used for the installation *must be included in the EasyBuild release*.

We do allow the use of [`--from-pr`](https://docs.easybuild.io/integration-with-github/#github_from_pr) and
[`--include-easyblocks-from-pr`](https://docs.easybuild.io/integration-with-github/#github_include_easyblocks_from_pr)
to pull in changes required to make the installation work correctly in the EESSI build environment,
but only if that is strictly required.

!!! note

This restriction may be relaxed later to also allow adding software that is not supported yet in the latest
EasyBuild release, or to allow for installing software with other tools.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd remove this. Raises expectations we may not (want to) meet.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added this mostly to counter feedback like "why are you only using EasyBuild", but perhaps the contribution policy is not the place for that, so will remove.


## Supported compiler toolchains

A [compiler toolchain](https://docs.easybuild.io/terminology/#toolchains) that is still supported by the latest
EasyBuild must be used for building the software.

More information on deprecated toolchains in EasyBuild is available
[here](https://docs.easybuild.io/deprecated-easyconfigs/#deprecated_easyconfigs_toolchains).
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we be more restrictive here? E.g.,

Only toolchains already available in the EESSI repository may be used.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd say that any toolchain still supported in EasyBuild is eligible to be included in EESSI, at least in the initial version of the policy.

Going back to older toolchains is likely going to be significantly more painful, if only to get even the installation of GCC to work, so this is sort of self-regulating...

Also, we can't really use a statement like "Only toolchains already available in the EESSI repository may be used.", because then adding toolchains (however recent) would be against the contribution policy? :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it should be up to the EESSI maintainers what toolchains are and are not supported by a particular EESSI release, this can be handled via our hook, see for example https://github.com/easybuilders/JSC/blob/2024/Custom_Hooks/eb_hooks.py#L570-L640

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Allowing someone to use a new toolchain is a big step, as obviously this is extremely likely to trigger a massive number of new software packages (and maintainer effort).


## Supported CPU targets

The software *should* work on all [CPU targets supported by EESSI](cpu_targets.md).

Exceptions to this requirement are allowed if technical problems that can not be resolved with reasonable effort
prevent the installation of the software for specific CPU targets.

## Software versions & toolchains

Recent software versions and toolchains *should* be preferred,
although the installation of older versions of use of older toolchains is allowed if sufficiently motivated.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"versions of use of older toolchains" this is rather unclear, what is really meant by this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reworded, should be clearer now

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know we've only started to discuss this, and the machinery to help enable this doesn't yet exist, but we should maintain that once a toolchain is connected to a particular compat layer we don't include that toolchain in future compat layers. If we allow people to create PRs for older toolchains to newer compat layers we will bring a lot of baggage.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, that's perhaps a bit too restrictive, there can be very valid reasons to have a recent toolchain like foss/2023a both in the current EESSI version, but also have it in a future version?

Copy link
Member

@ocaisa ocaisa Oct 30, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What type of reason? I can't think of a need for this. You can have the later toolchain, just with a different compat layer. If we select compat layer via a module, rather than sourcing a script (which is entirely possible) they can happily live side by side using a (reduced) hierarchical view.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Especially when we already know there are cases where it can't work, for example the whole OpenSSH thing we ran into with 2023.04


## Testing

We should be able to test the software installations via the [EESSI test suite](https://github.com/EESSI/test-suite)
being developed.

Ideally one or more tests are available that verify that the software is functionally correct,
and performs well.

It should be possible to run a minimal *smoke test*, for example using EasyBuild's `--sanity-check-only` feature.

!!! note

The [EESSI test suite](https://github.com/EESSI/test-suite) is still in active development,
and currently only has a minimal set of tests available.

When the test suite is more mature, this requirement will be enforced more strictly.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reads very vague, more like a goal/aim. I'd rather require that a software must be tested for functional correctness, single-core performance and multi-core scalability. Since the machinery (and tests) do not exist yet, the request to add a software should detail how the software can be tested.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's way more restrictive than what we're doing currently, for many installations (dependencies) we don't really test at all...

So we need to keep this relatively loose for now.

Eventually we can hopefully require that for example --sanity-check-only works on all installations, but that needs more work, and that the latest release of the EESSI test suite passes with certain tags (like CI).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the wording is OK for now.

One of the plans in the EESSI test suite is to add a "smoke test" that basically just runs eb --sanity-check-only on all installations.

Once that is in place, the minimal requirement for this part could be that this test must pass for all software installations being added, but then we should first fix some known problems in EasyBuild like easybuilders/easybuild-easyblocks#2986

1 change: 1 addition & 0 deletions mkdocs.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ nav:
- software_layer/cpu_targets.md
- software_layer/build_nodes.md
- software_layer/adding_software.md
- software_layer/contribution_policy.md
- Build-test-deploy bot: bot.md
- Pilot repository: pilot.md
- Getting access to EESSI:
Expand Down