-
Notifications
You must be signed in to change notification settings - Fork 53
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.
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
Prepare ODK 1.5.3 #1113
Merged
Merged
Prepare ODK 1.5.3 #1113
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Users may want to bind their local OAK cache into the /home/odkuser/.data/oaklib directory within the container, so that any OAK process started from within the container can benefit from the user's cache. When running in user mode, we must then make sure that the /home/odkuser/.data/oaklib directory (which would be automatically created by Docker when setting up the binding) belongs to the odkuser, instead of the root user.
Setting Git's configuration option safe.directory to /work is enough to allow working on a pre-seeded repository (when the repository root is bound to /work in the container), but does not work when seeding a new repository, because then the root of the newly created repository is not the /work directory itself, but is in fact located two directories below that (under /work/target/<name>). The safe.directory directive has thus no effect when seeding. We must set safe.directory to "*" instead, to completely disable the permission check regardless of where the repository is. Security-wise, this does not change anything -- that check does not bring any security benefit in the context of the ODK, and we were already disabling it when working on pre-seeded repositories. closes #1105
Calling `odk.py seed` without any argument results in a crash because the `repo` variable (supposed to hold the name of the repository) is set to None, which is not expected by the Jinja template (it expects a string). When no positional arguments are specified, we set the `repo` variable to `noname` instead, thereby allowing the creation of a full repository with a dummy name instead of causing an error.
When seeding, and unless the --skipgit option has been used, we need a username and email address to pass to Git. If we don't have those, the seeding process will crash at the very end, after all files have been generated. Since the Git username and email are necessary, we should detect whether we have them at the very beginning, and exit cleanly (with a message telling the user what they need to to) as soon as possible, without even initiating a process that is bound to fail.
The seed command expects only one positional arguments. If we got more than one, we should exit with a clean error message, instead of crashing with an uncaught exception and the associated stacktrace.
When we run under a non-privileged user account (which is the default situation), we need to make the SSH authentication socket, if it exists, belongs to the odkuser. Under some conditions, Docker may create that socket as belonging to root by default.
Bump the SSSOM-Java (ROBOT plugin and command line tool) to the latest 0.9.0 version.
We update the KGCL plugin for ROBOT to the latest released version 0.5.0. That version brings only one meaningful (user-visible) change compared to the version currently in the ODK, but it is a rather important one: the ability to refer to nodes by their label rather than their ID. That is, it makes it possible to write obsolete 'my class' rather than obsolete EX:1234
The NodeJs package manager (NPM) is no longer automatically installed when we install NodeJS, so we ask for it explicitly. We need it to install Obographviz. Ideally it should be possible to install both NPM and Obographviz in the builder image and then to transfer only Obographviz to the final ODKFull image, thereby reducing the clutter in ODKFull, but this will need further investigation.
This is a runtime dependency of Konclude (even the statically compiled one that we use on x86_64). It was probably automatically pulled by another package on Ubuntu 22.04, but it is not on 22.04, so we need to ask for it explicitly.
Pip is now by default refusing to install anything in the system-wide Python library path, which is considered the "private garden" of the underlying operating system (e.g., on Ubuntu, only APT tools should add Python packages to the system-wide path). This may be fine in general, in a user-facing scenario, but not for the ODK which is in effect a "read-only" system overall. The OBO Dashboard is part of the tools/libraries we provide with the ODK and there is no reason for us to package it separately inside a virtual environment (the recommended way of installing non-system Python packages) -- the entire ODK is already a "virtual environment". Ultimately the right thing to do here would be for the OBO Dashboard upstream to make proper releases, which we could then install at the same time as any other Python packages.
The odk.py script has some issues when we run it under Python 3.12, we fix them here.
PIP emits a warning when we attempt to install executable scripts in a location that is not in the system PATH. In this instance the warning is not warranted. The scripts are installed in a staging location ON PURPOSE -- they are later copied over to the final ODK images, where the scripts will end up in the system PATH. So we shut that warning down.
Where to begin? It's just another Python screw-up. Developers of setuptools have announced, back in 2017 (!) their intention to remove the `test` command (found in setuptools.command.test) [1]. Today, they thought that 7 years of advance deprecation warnings were enough, and they published setuptools v72.0.0 which no longer provides that command. [2] But the Python ecosystem being what it is (a joke that has lasted long enough), it turns out that 7 years was not actually enough, and there are still many Python packages out there that didn't get the memo and that are still dependent on setuptools.command.test (including some packages that are used in the ODK). There are several ways of fixing this: a) Ditch Python altogether. This would be by far the best solution, but one that is, alas, unlikely to happen any time soon. b) Fix _all_ the packages used by the ODK (and their dependencies) so that they do what they should have done in the past 7 years. This is the correct solution, but one that will take time (7 years were already not enough, so who knows how long it is going to take?). c) Torture the setuptools developers until they agree to revert the removal of the `test` command. I believe this may be illegal in many jurisdictions. d) Force the ODK build process to use a version of `setuptools` older than 72.0.0. This is what we are doing here. Yes, this is a local workaround and we should favour "upstream fixing" instead (solution b), but if we want to be be able to build the ODK before 2031 (optimistic estimation of the time it will take to get all the Python packages fixed), we don't have much choice. [1] pypa/setuptools#931 [2] https://setuptools.pypa.io/en/stable/history.html
The j2cli project (which provides the j2 tool) is no longer maintained and is broken under Python 3.12, so we replace it with Jinjanator (providing the similar command jinjanate).
Installing virtualenv in the builder image has nafarious consequences when we try to later install Python packages. That's because the Ubuntu package for virtualenv automatically installs platformdirs version 2.5.1, which then prevents us from installing the platformdirs 4.x that we need as a dependency for some of our packages. The only reason we had virtualenv in the builder image was that we use it to run the update-constraints workflow (in which we try to install all our Python packages in a virtualenv). So here, we 1) remove virtualenv from the builder image; 2) amend the update-constraints.sh script to make it install virtualenv itself.
Bump version number and update the changelog.
matentzn
approved these changes
Oct 28, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, shall I make a release?
(or can you, I forgot) |
My humble desktop machine would rather have you do it. :D FYI, I’ve tested a locally built 1.5.3 this afternoon on both Uberon and FBbt without issue. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR updates the
BRANCH-1.5-MAINTENANCE
branch to make it ready for a 1.5.3 point release.It back ports several updates to the ODK tooling, including the update of the base image to Ubuntu 24.04 (which, as discussed here, is considered a minor change, on the ground that it does not affect the standard ODK-generated workflows -- it does mean that we have to backport the J2cli -> Jinjanator replacement though, which in turns means that any custom workflow that uses
j2
will not work with the new image until the workflow is amended to usejinjanate
instead).It also backports a few fixes that only concern the image and not the workflows (#1096, #1097, #1105).