Skip to content

Define a set of named features and logic to manage their dependencies.#480

Draft
albu-diku wants to merge 6 commits intonextfrom
add/requirements-per-feature
Draft

Define a set of named features and logic to manage their dependencies.#480
albu-diku wants to merge 6 commits intonextfrom
add/requirements-per-feature

Conversation

@albu-diku
Copy link
Copy Markdown
Contributor

@albu-diku albu-diku commented Mar 9, 2026

Summary:

This work povides a way to make use of per-feature requirements files while maintaining the ability to have certain requirements overridden during build and other be met by system packages. This would make it suitable for immediate use in docker-migrid where it would replace the series of hand rolled pip install lines gated by feature checks and the package installation lines those checks contain.

Detail:

The codebase has a number of features that, when selected, require certain dependencies. Thus far the handling for this has been split between this repository and the docker-migrid, which encodes a large amount of version information for the various features and in addition allows overriding these at build time.

There is a desire to see these become per-feature requirements files expressed in this repository - that would mean the dependencies are captured alongside the code that depends upon them. However, the ability to override the versions being used and, more critically, to make use of packages provided by the distribution must remain possible.

This PR introduces work that will allow that. The features are explicitly defined by name in a static file that describes them. The added features tool reads this file and, after consulting a source for what is enabled (this can be the environment but also a .env file, as used within the docker builds, directly) will output the lines needed to install the packages.

This simple conceptual design allows us to handle package overrides and distribution packages. We maintain a list of environmental keys to be consulted for certain package versions and if present expand the packages we must install from the requirements file and make use of the an applicable override instead. In the situation of distribution packages, we perform the same requirements expansion but check whether the dependency is already met in the python environment and skip its installation if so.

Review

To anyone taking a look: not all the features are fully defined here as a way to first share the approach. The tests coves all installation cases, but when run directly the repository level notion of defined features are used. Two named features re currently defined, CLOUD and MIGUX (which likely gives you an idea of what I was working on). I would recommend comparing the output from the following commands:

./envhelp/lpython ./mig/install/features.py install --env --check
ENABLE_CLOUD=true ENABLE_WORKFLOWS=true ./envhelp/lpython ./mig/install/features.py install --env --check
ENABLE_CLOUD=true OPENSTACKSDK_VERSION_OVERRIDE=1.2.3 ./envhelp/lpython ./mig/install/features.py install --env --check

@albu-diku albu-diku force-pushed the add/requirements-per-feature branch 3 times, most recently from 6b810ce to 6597a2a Compare March 14, 2026 19:17
@jonasbardino
Copy link
Copy Markdown
Contributor

I think this is an important and also quite big task we want to pursue so thanks for getting it rolling 👍
How would you feel about adding a new milestone on it and e.g. moving the high-level description and motivation there with this PR then assigned to it?

@jonasbardino jonasbardino added the enhancement New feature or request label Mar 19, 2026
Copy link
Copy Markdown
Contributor

@jonasbardino jonasbardino left a comment

Choose a reason for hiding this comment

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

Being a draft I don't want to go into pedantic mode but just say that it needs an overhaul regarding the usual style fixes including copyright header and docstrings before consideration for merging.
I'll comment on a few other conceptual and overall bits to discuss separately.

@jonasbardino
Copy link
Copy Markdown
Contributor

Noticing the function to strip versions and considering how we may also want to detect or handle e.g. different features requesting different versions of the same package (say Cryptography) we may want to look into dedicated helpers to reuse. The packaging package has several version and requirements parsing helpers
https://packaging.pypa.io/en/latest/version.html
https://packaging.pypa.io/en/stable/requirements.html
and it is already packaged in RHEL and Debian-based distros. So something to consider to avoid reinventing solid utilities.

@jonasbardino
Copy link
Copy Markdown
Contributor

I noticed quite a heavy use of f-strings in this PR. We have historically stayed with %-expansion e.g. over f-strings for various reasons, and I think we want to discuss it in the dev team first if that is to change.

@jonasbardino
Copy link
Copy Markdown
Contributor

I've raised a couple of issues and made a few suggestions, but after looking it over and trying out the command examples you suggested it generally looks good and a step in the right direction.

@albu-diku
Copy link
Copy Markdown
Contributor Author

I think this is an important and also quite big task we want to pursue so thanks for getting it rolling 👍 How would you feel about adding a new milestone on it and e.g. moving the high-level description and motivation there with this PR then assigned to it?

That is a great idea and have done just that!

@albu-diku
Copy link
Copy Markdown
Contributor Author

Being a draft I don't want to go into pedantic mode but just say that it needs an overhaul regarding the usual style fixes including copyright header and docstrings before consideration for merging. I'll comment on a few other conceptual and overall bits to discuss separately.

Just want to confirm, this was very well known and the review you have given it, at the level of approach, is exactly what was needed to confirm it worth pursuing.

Nonetheless making significant inroads on those notes will be pressing as part of adding the other suggested feature example.

@albu-diku
Copy link
Copy Markdown
Contributor Author

I noticed quite a heavy use of f-strings in this PR. We have historically stayed with %-expansion e.g. over f-strings for various reasons, and I think we want to discuss it in the dev team first if that is to change.

Ah yah, actually that switch wasn't entirely conscious beyond realising their use was no longer a worry given our as of recent minimum Python version and then writing this as I would my personal python code.

My own take is f-strings are extremely pleasant for clarity of intent, but I'm certainly not married to this. Not an issue for me to switch it back right away but otherwise perhaps it can serve as a useful case in point for discussion by the team for now.

@albu-diku
Copy link
Copy Markdown
Contributor Author

Noticing the function to strip versions and considering how we may also want to detect or handle e.g. different features requesting different versions of the same package (say Cryptography) we may want to look into dedicated helpers to reuse. The packaging package has several version and requirements parsing helpers https://packaging.pypa.io/en/latest/version.html https://packaging.pypa.io/en/stable/requirements.html and it is already packaged in RHEL and Debian-based distros. So something to consider to avoid reinventing solid utilities.

Something perhaps worth a bit of vocal discussion but this was actually rather front of mind for me, hence trying to re-use as much of pip as possible. When I wrote this my thinking was to minimise external dependencies given this is itself a bootstrap tool - but agree it is a very fine line and I think the biggest argument for bringing something in would be one of maintenance i.e. it may well be far better to defer to a formal packaging library (thus requiring it) if it isolates us from the packaging tools themselves moving forward. Definite food for thought.

The codebase has a number of features that, when selected, require
certain dependencies. Thus far the handling for this hsa been split
between this repository and the docker-migrid, which encodes a large
amount of version information for the various features and in addition
allows overriding these at build time.

There is a desire to see these become per-feature requirements files
expresed in this repository, but then to also retain the ability to
override the package versions at the time of build.

This commit introduces work that will allow that. The features are
explicitly defined by name in a ini file that describes them. The
added features tool reads this file and, after consulting a source
for what is enabled (this can be the environment but also a .env
file, as used within the docker builds, directly) will output the
lines needed to install the packages.

Any complexity here stems from the requirement to be able to override
certain packages. Since a requirements file can easily contain more
than one package, we must be able to override one of the set of
specified dependencies.
This mode can be enabled by a command line flag which, when set, causes
the dependencies for each feature to be checked for presence within the
active python environment and their installation skipped if present.

In practice this means the requirements files can list dependencies
exhaustively but still allow the host environment arranged by a build
process to provision packages in the host environment and have these
respected during installation. Further, if such a package is only one
of many requirements then it alone will be skipped while all other
dependencies will be correctly installed.
@albu-diku albu-diku force-pushed the add/requirements-per-feature branch from d8c1158 to 2871099 Compare March 27, 2026 14:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants