Move most of min_requirements.py to the framework #86

Closed
opened 2024-12-02 03:45:34 -06:00 by mpg · 5 comments
mpg commented 2024-12-02 03:45:34 -06:00 (Migrated from github.com)

We want to keep scripts/min_requirements.py in each branch (3.6 and development), as that's the entry point that the CI is calling. But we want to move most of its code to the framework, as a module that can be imported. So, the end result would probably have scripts/min_requirements.py in each branch looking like

import framework_scripts_path # pylint: disable=unused-import
from mbedtls_framework import min_requirements

min_requirements.main()

and then the current body of scripts/min_requirements.py would be moved to the framework repo, in scripts/mbedtls_framework (as a non-executable file).

In terms of history, we probably want to first completely move min_requirements.py to the framework (so that its full history is present there) and then add back the new stub in 3.6 and development.

We want to keep `scripts/min_requirements.py` in each branch (3.6 and development), as that's the entry point that the CI is calling. But we want to move most of its code to the framework, as a module that can be imported. So, the end result would probably have `scripts/min_requirements.py` in each branch looking like ```py import framework_scripts_path # pylint: disable=unused-import from mbedtls_framework import min_requirements min_requirements.main() ``` and then the current body of `scripts/min_requirements.py` would be moved to the framework repo, in `scripts/mbedtls_framework` (as a non-executable file). In terms of history, we probably want to first completely move `min_requirements.py` to the framework (so that its full history is present there) and then add back the new stub in 3.6 and development.
valeriosetti commented 2024-12-19 04:22:57 -06:00 (Migrated from github.com)

Question: min_requirements.py uses scripts/ci.requirements.txt and scripts/driver.requirements.txt which are on the same scripts folder and which are identical between mbedtls-3.6 and development. Can I move those files as well to the framework? Or is better to keep a copy for each branch so that they are allowed to diverge in the future if needed?

Question: `min_requirements.py` uses `scripts/ci.requirements.txt` and `scripts/driver.requirements.txt` which are on the same `scripts` folder and which are identical between `mbedtls-3.6` and `development`. Can I move those files as well to the framework? Or is better to keep a copy for each branch so that they are allowed to diverge in the future if needed?
mpg commented 2024-12-19 04:37:25 -06:00 (Migrated from github.com)

We're not entirely decided about the long-term future of the .requirements.txt files but for now I think they'll remain in each branch until we've made a decision.

We're not entirely decided about the long-term future of the `.requirements.txt` files but for now I think they'll remain in each branch until we've made a decision.
gilles-peskine-arm commented 2024-12-19 05:33:21 -06:00 (Migrated from github.com)

The most technically correct approach would be to have each requirements.txt line in the same repository that contains the import line that provides this requirement. I'm not sure if we have to adhere to this strictly, but I suspect that in the medium term, we'll want to do it. Otherwise it's going to be a pain to manage requirements changes.

I don't know if we want to keep the same separate *.requirements.txt files going forward. I had initially separated them so that different types of users could stick to a minimal Python environment for their needs. But in practice there only seem to be two kinds of users:

  • Users who are just building the library and would like to use plain Python, but are currently bothered because they need jsonschema and jinja2.
  • Maintainers (and contributors) who just install everything.
The most technically correct approach would be to have each `requirements.txt` line in the same repository that contains the `import` line that provides this requirement. I'm not sure if we have to adhere to this strictly, but I suspect that in the medium term, we'll want to do it. Otherwise it's going to be a pain to manage requirements changes. I don't know if we want to keep the same separate `*.requirements.txt` files going forward. I had initially separated them so that different types of users could stick to a minimal Python environment for their needs. But in practice there only seem to be two kinds of users: * Users who are just building the library and would like to use plain Python, but are currently bothered because they need `jsonschema` and `jinja2`. * Maintainers (and contributors) who just install everything.
gilles-peskine-arm commented 2024-12-19 05:36:23 -06:00 (Migrated from github.com)

We want to keep scripts/min_requirements.py in each branch (3.6 and development), as that's the entry point that the CI is calling.

We don't necessarily have to do it that way. The Groovy code already calls min_requirements.py conditionally, because it (still) supports older branches without it. We could change it to also look in framework/scripts before scripts. (And change info.has_min_requirements to a nullable string containing the path, I suppose.)

> We want to keep scripts/min_requirements.py in each branch (3.6 and development), as that's the entry point that the CI is calling. We don't necessarily have to do it that way. The Groovy code already calls `min_requirements.py` conditionally, because it (still) supports older branches without it. We could change it to also look in `framework/scripts` before `scripts`. (And change `info.has_min_requirements` to a nullable string containing the path, I suppose.)
mpg commented 2024-12-20 05:04:54 -06:00 (Migrated from github.com)

Yeah, but I was hoping to simplify the Groovy code by removing the part where it still support older branches without it. (Even 2.28 has this script, and it's EOL very soon.)

More generally speaking, if we have to choose between making some python code a bit more complex or some groovy code a bit more complex, I'm inclined to put the complexity on the python side, because python is more familiar than groovy to most team members.

That said, I agree the way suggested here is not the only one, and we can absolutely revisit that decision in the future, but I wanted to pick something that allows us to move forward with little friction.

Yeah, but I was hoping to simplify the Groovy code by removing the part where it still support older branches without it. (Even 2.28 has this script, and it's EOL very soon.) More generally speaking, if we have to choose between making some python code a bit more complex or some groovy code a bit more complex, I'm inclined to put the complexity on the python side, because python is more familiar than groovy to most team members. That said, I agree the way suggested here is not the only one, and we can absolutely revisit that decision in the future, but I wanted to pick something that allows us to move forward with little friction.
Sign in to join this conversation.
No milestone
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
gh-Mbed-TLS/mbedtls-framework#86
No description provided.