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(core): versioning fix for remote sources #5735

Merged
merged 2 commits into from
Feb 15, 2024

Conversation

thsig
Copy link
Collaborator

@thsig thsig commented Feb 15, 2024

What this PR does / why we need it:

Before this fix, we were including the relative path to a file from project root in the string to be hashed when calculating versions.

This was problematic when linking remote sources, since the chosen name and path of the locally linked repo would be factored into the version (resulting in unwanted cache misses).

This was fixed by using the relative path from the directory containing the file's parent config. For example, an action's directory should be able to be moved around or renamed without it affecting the version of that action.

Which issue(s) this PR fixes:

Fixes #5501.

Before this fix, we were including the relative path to a file from
project root in the string to be hashed when calculating versions.

This was problematic when linking remote sources, since the chosen name
and path of the locally linked repo would be factored into the version
(resulting in unwanted cache misses).

This was fixed by using the relative path from the directory containing
the file's parent config. For example, an action's directory should be
able to be moved around or renamed without it affecting the version of
that action.
vvagaytsev
vvagaytsev previously approved these changes Feb 15, 2024
Copy link
Collaborator

@vvagaytsev vvagaytsev left a comment

Choose a reason for hiding this comment

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

Looks good! 💯 Thanks for converting a test project to the action-based config! We should definitely do it incrementally and often.

@vvagaytsev
Copy link
Collaborator

There are some test failures. Likely just some trivial assertion failures because of the changed hash calculation.

Changed the `include-exclude` test project to use actions instead of
modules (we should keep incrementally doing this to prepare for
deprecating modules).

Also wrote a test case for the fix introduced in the previous commit.
@vvagaytsev vvagaytsev added this pull request to the merge queue Feb 15, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Feb 15, 2024
@vvagaytsev vvagaytsev added this pull request to the merge queue Feb 15, 2024
Merged via the queue into main with commit 91bfd48 Feb 15, 2024
43 checks passed
@vvagaytsev vvagaytsev deleted the repo-name-version-change-fix branch February 15, 2024 14:21
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.

Version hash is dependent on repository source folder name
2 participants