Skip to content

fix(librustzcash): bump librustzcash lib to fix CI cache issue#2299

Closed
borngraced wants to merge 6 commits intodevfrom
zcash-cache-fix
Closed

fix(librustzcash): bump librustzcash lib to fix CI cache issue#2299
borngraced wants to merge 6 commits intodevfrom
zcash-cache-fix

Conversation

@borngraced
Copy link
Copy Markdown

#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
Copy Markdown
Collaborator

dimxy commented Dec 23, 2024

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

@laruh
Copy link
Copy Markdown

laruh commented Dec 23, 2024

@borngraced is this still in progress?

@borngraced
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown

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
Copy Markdown
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
Copy Markdown

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.

@borngraced
Copy link
Copy Markdown
Author

PR #2303 seems to have fixed CI cache issue cc @onur-ozkan

closing.

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