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

dev-cmd/vendor-gems: run bundle clean #17351

Merged
merged 1 commit into from
May 23, 2024
Merged

dev-cmd/vendor-gems: run bundle clean #17351

merged 1 commit into from
May 23, 2024

Conversation

Bo98
Copy link
Member

@Bo98 Bo98 commented May 23, 2024

Fixes install inconsistencies between standalone and all other bundle operations.

Should work as is but will test properly in a bit

Fixes install inconsistencies between standalone and all other bundle operations
@MikeMcQuaid
Copy link
Member

Fixes install inconsistencies between standalone and all other bundle operations.

This resolves inconsistencies but in the wrong direction for base64 if we care about handling that.

Removing

BUNDLE_CLEAN: "true"
(or making it false) and making bundle clean always require an explicit call feels like it might be nicer?

@Bo98
Copy link
Member Author

Bo98 commented May 23, 2024

I wouldn't necessarily say wrong. Standalone mode is the mode that's much less used than regular mode. So regular mode using the base64 that ships with Ruby (when the versions match) is fine as that's what happens to everyone else.

What breaks things is when it flip flops between the two, but this should fix that as it undos the unique standalone mode behaviour.

@Bo98
Copy link
Member Author

Bo98 commented May 23, 2024

#16893 is proof of this working

Copy link
Member

@MikeMcQuaid MikeMcQuaid left a comment

Choose a reason for hiding this comment

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

I don't understand why this works when I tried this approach before and it didn't seem to work before but: it's working and that's great, nice work @Bo98!

@MikeMcQuaid MikeMcQuaid merged commit 5a850a3 into master May 23, 2024
27 checks passed
@MikeMcQuaid MikeMcQuaid deleted the vendor-gems-clean branch May 23, 2024 13:51
@Bo98
Copy link
Member Author

Bo98 commented May 23, 2024

I don't understand why this works when I tried this approach before and it didn't seem to work before but: it's working and that's great, nice work @Bo98!

If you were trying locally, it's possible the Bootsnap cache already got corrupt and would have required manually deleting. But in CI that's fine because we don't carry that cache over and for other end users it's also OK as we never shipped the bad state.

@MikeMcQuaid
Copy link
Member

If you were trying locally, it's possible the Bootsnap cache already got corrupt and would have required manually deleting. But in CI that's fine because we don't carry that cache over and for other end users it's also OK as we never shipped the bad state.

Yeh, Bootsnap was definitely messing some stuff up here. Thanks for explaining @Bo98!

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.

None yet

2 participants