Skip to content

Remove sort logic from inputs#942

Merged
vsvipul merged 1 commit intomainfrom
vsvipul/fix-sort
Oct 3, 2022
Merged

Remove sort logic from inputs#942
vsvipul merged 1 commit intomainfrom
vsvipul/fix-sort

Conversation

@vsvipul
Copy link
Contributor

@vsvipul vsvipul commented Oct 3, 2022

Fixes #941.

This bug was introduced with this commit https://github.com/actions/cache/pull/903/files and also sorts restore-keys which we don't want. Initial intention was to sort paths only.

This PR removes sorting completely. We can discuss later if we want to sort paths separately in save.ts / restore.ts .

After merging this, we'll need to release a new cache version immediately to fix this bug.

@vsvipul vsvipul requested a review from a team as a code owner October 3, 2022 06:40
@github-actions github-actions bot requested a review from pallavx October 3, 2022 06:41
@vsvipul vsvipul requested a review from Phantsure October 3, 2022 06:44
@Phantsure
Copy link
Contributor

I thought the previous PR was created to sort the files to be cached. Why remove that sort?

@vsvipul
Copy link
Contributor Author

vsvipul commented Oct 3, 2022

@Phantsure That can be done in the save.ts / restore.ts file separately only for the path array, but not in the getInput fxn. Should I add that change as well ?

@Phantsure
Copy link
Contributor

If that already didn't exist then we have to think if it would break anything

@vsvipul
Copy link
Contributor Author

vsvipul commented Oct 3, 2022

@Phantsure Then it's good to just remove sort here and merge this today as a fix.

Copy link
Contributor

@Phantsure Phantsure left a comment

Choose a reason for hiding this comment

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

makes sense

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.0.9 release is sorting restore-keys alphabetically which dramatically changes behaviour

2 participants