Skip to content
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

Fix mapping limitation in vendor #179

Merged
merged 3 commits into from
Aug 21, 2024
Merged

Fix mapping limitation in vendor #179

merged 3 commits into from
Aug 21, 2024

Conversation

phsauter
Copy link
Contributor

vendor did not handle it well when multiple files/directories were copied together into one (as is done in croc).

Fixing it involved prioritizing more specific mappings (eg file to file) over less specific ones (dir to dir) and excluding already diffed files when handling their parent directories.

Also fixes an annoying warning caused by a default configuration.

Tested bender vendor init and one random patch with bender vendor patch in opentitan_peripherals, register_interface, redundancy_cells and croc. The resulting directories were identical to upstream so it is unlikely to break compatibility with existing configurations.
It should just make it possible to use more elaborate setups without issue.

@micprog micprog requested review from micprog and niwis August 20, 2024 17:18
Copy link
Contributor

@niwis niwis left a comment

Choose a reason for hiding this comment

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

Thanks a lot, that's a great improvement for mappings. LGTM

File mappings are more specific and
should be preferred over directory mappings.

As an example we have two mappings:
a/ -> c/
b/file -> c/file

Any change in c/ should be then result in a patch.
If the change is in c/file we must make sure it is linked back
to b/file, not erroneously to a/, the sorting does this.

Similarly when initializing we first need to copy the a/ dir to c/
and only then copy b/file to c/file.
@micprog micprog merged commit 7fa835c into master Aug 21, 2024
6 checks passed
@micprog micprog deleted the phsauter/vendor branch August 21, 2024 15:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants