CASMTRIAGE-7457: Fix bug causing some Python script symlinks from not being created #624
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
jacobsalmela
approved these changes
Nov 5, 2024
There was a problem hiding this comment.
I didn't extensively test #617, just some quick manual smoke tests 🍳
I ran a fair amount of tests, but I guess I missed running the one test which used the one script whose symlink wasn't being created. :) |
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.
An error in the regular expressions used in the
create-python-script-symlinks.shscript resulted in no symlinks being created for some Python scripts in the virtual environment. Specifically, any scripts whose names contained numbers were omitted.This PR refactors the build shell scripts to remove the need for that kind of regular expression. Now the script that creates the symlinks does not need to parse and transform the output from the script which updates pyproject.yaml. Instead, that latter script now adds two different types of lines to pyproject.yaml. One is the same as before, to be consumed by Python setuptools when building the package. The second is a comment line which provides exactly the data needed as input for the script that creates the Python symlinks.
This hopefully reduces the chances of a bug like the one we saw, where the script was registered with the Python module, but no symlink was created.
I have tested this on mug and verified that the missing symlinks are now created.
This PR also adds me and @jacobsalmela to the codeowners file, since we do a lot of work in the repo, and probably should get tagged as PR reviewers.