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(librustzcash): bump librustzcash lib to fix CI cache issue #2299

Open
wants to merge 6 commits into
base: dev
Choose a base branch
from

Conversation

borngraced
Copy link
Member

#2292
debugging if this will actually fix CI cache issue regarding librustzcash lib

@borngraced borngraced changed the title fix(librustzcash): bump librustzcash lib fix(librustzcash): bump librustzcash lib to fix CI cache issue Dec 21, 2024
@dimxy
Copy link
Collaborator

dimxy commented Dec 23, 2024

Why the librustzcash revision is not tagged as 'tag' as usual, but as 'branch'

@laruh
Copy link
Member

laruh commented Dec 23, 2024

@borngraced is this still in progress?

@borngraced
Copy link
Member Author

Why the librustzcash revision is not tagged as 'tag' as usual, but as 'branch'

I will update until after the PR is merged in librustzcash

@borngraced
Copy link
Member Author

@borngraced is this still in progress?

#2299 (comment) same reason why it's in progress. I will mark ready for review when the main PR is merged in librustzcash

@onur-ozkan
Copy link
Member

This didn't happen this often before. Something should regress this problem.. Instead of putting big patch on already heavily modified fork, can we try some hacks on this cache plugin? Did you try to play with caching key? Or at least, you mentioned that these could work too:

image

I think they are better than the current approach as it's just a single line addition for CI rather than making big changes on the actual code base.

@borngraced
Copy link
Member Author

borngraced commented Dec 23, 2024

This didn't happen this often before. Something should regress this problem.. Instead of putting big patch on already heavily modified fork, can we try some hacks on this cache plugin? Did you try to play with caching key? Or at least, you mentioned that these could work too:

image

I think they are better than the current approach as it's just a single line addition for CI rather than making big changes on the actual code base.

maybe not really, and yes, while those other options could work, I actually think it makes more sense to commit the file since the original updated repo(librustzcash) now commit the file too so I guess it fine if we just delete .gitignore and commit compact_format.rs instead of "hacking"

I can try the other option for sure if you're against this.

@onur-ozkan
Copy link
Member

I think it's still unnecessary addition. Making such changes just to bypass cache invalidation doesn't make sense IMO. Just made #2303 PR to use a rust-specific caching mechanism. Let's see if it works. If not, we can revisit this approach.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants