-
Notifications
You must be signed in to change notification settings - Fork 13
CFE-4561: Implemented support for converting modified files into patch files in cfbs-convert #289
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
Conversation
…e helper function This improved how the program messages read (especially when in interactive mode, where there will be a prompt about editing the commit message) and reduced code verbosity. Signed-off-by: jakub-nt <175944085+jakub-nt@users.noreply.github.com>
|
Thank you for submitting a PR! Maybe @larsewi can review this? |
larsewi
left a comment
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.
Great 🚀
|
|
||
|
|
||
| def _apply_masterfiles_patch(patch_path): | ||
| if not cli_tool_present("patch"): |
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.
This function could take a list of commands. You should check for gpatch first, to ensure GNU implementation. Some UNIX systems have their own implementation of patch which can behave differently.
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.
We only bundle cfbs in hub packages as far as I know. And they are all Linux. So should be fine to not do this.
| # make the patches local module on first use | ||
| if not patches_module_present: | ||
| print("Adding patches local module...") | ||
| mkdir(patches_dir) |
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.
Should probably not error in case of FileExistsError
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.
That directory can't exist due to
Lines 1138 to 1141 in 3f602d9
| if not (len(dir_content) == 1 and dir_content[0].startswith("masterfiles-")): | |
| raise CFBSUserError( | |
| "cfbs convert must be run in a directory containing only one item, a subdirectory named masterfiles-<some-name>" | |
| ) |
…the system Signed-off-by: jakub-nt <175944085+jakub-nt@users.noreply.github.com>
… a unified diff Signed-off-by: jakub-nt <175944085+jakub-nt@users.noreply.github.com>
Signed-off-by: jakub-nt <175944085+jakub-nt@users.noreply.github.com>
… cfbs-convert Signed-off-by: jakub-nt <175944085+jakub-nt@users.noreply.github.com>
Signed-off-by: jakub-nt <175944085+jakub-nt@users.noreply.github.com>
…st file converted to a patch file Signed-off-by: jakub-nt <175944085+jakub-nt@users.noreply.github.com>
Signed-off-by: jakub-nt <175944085+jakub-nt@users.noreply.github.com>
…e conversion together This makes sense since a module with no build steps is not considered valid. Signed-off-by: jakub-nt <175944085+jakub-nt@users.noreply.github.com>
larsewi
left a comment
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.
Thanks 🚀
No description provided.