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

signature from "Alexey Pavlov (Alexpux) <alexpux@gmail.com>" is invalid #99

Closed
eyal0 opened this issue Dec 30, 2020 · 26 comments
Closed

Comments

@eyal0
Copy link

eyal0 commented Dec 30, 2020

I'm getting many errors about invalid signatures in my CI:

2020-12-30T07:53:10.1381061Z error: mingw-w64-x86_64-icu: signature from "Alexey Pavlov (Alexpux) <alexpux@gmail.com>" is invalid
2020-12-30T07:53:10.1382702Z :: File /var/cache/pacman/pkg/mingw-w64-x86_64-icu-68.2-1-any.pkg.tar.zst is corrupted (invalid or corrupted package (PGP signature)).
2020-12-30T07:53:10.1395297Z Do you want to delete it? [Y/n] error: mingw-w64-x86_64-boost: signature from "Alexey Pavlov (Alexpux) <alexpux@gmail.com>" is invalid
2020-12-30T07:53:10.1396779Z 
2020-12-30T07:53:10.1398296Z :: File /var/cache/pacman/pkg/mingw-w64-x86_64-boost-1.75.0-1-any.pkg.tar.zst is corrupted (invalid or corrupted package (PGP signature)).
2020-12-30T07:53:10.1416780Z Do you want to delete it? [Y/n] error: mingw-w64-x86_64-harfbuzz: signature from "Alexey Pavlov (Alexpux) <alexpux@gmail.com>" is invalid
2020-12-30T07:53:10.1418414Z 
2020-12-30T07:53:10.1419562Z :: File /var/cache/pacman/pkg/mingw-w64-x86_64-harfbuzz-2.7.3-1-any.pkg.tar.zst is corrupted (invalid or corrupted package (PGP signature)).
2020-12-30T07:53:10.1420770Z Do you want to delete it? [Y/n] error: failed to commit transaction (invalid or corrupted package)
2020-12-30T07:53:10.1731366Z 
2020-12-30T07:53:10.1734005Z Errors occurred, no packages were upgraded.
2020-12-30T07:53:10.1797630Z ##[error]The process 'C:\windows\system32\cmd.exe' failed with exit code 1

Similar to these: Alexpux/MSYS2-pacman#58

Is it a problem only in this repo or in all of msys2 or all of pacman?

@umarcor
Copy link
Contributor

umarcor commented Dec 30, 2020

It sounds stupid but... did you try restarting the CI workflow?

@lazka
Copy link
Member

lazka commented Dec 30, 2020

some packages got incorrectly replaced and the package caching in setup-msys2/pacman doesn't like that. should be fixed in the next upload

stupid workaround: reorder the packages in the "install: " part of the setup-msys2 action, that will trigger a cache miss.

@eyal0
Copy link
Author

eyal0 commented Dec 30, 2020

That seems to have worked.

So why is setup-msys2 caching a broken result? Is it possible that this project is caching results before first confirming that they are successful?

@lazka
Copy link
Member

lazka commented Dec 30, 2020

We restore /var/cache/pacman/pkg/mingw-w64-x86_64-harfbuzz-2.7.3-1-any.pkg.tar.zst, pacman tries to download harfbuzz, sees it's already there and tries to use it, checks its signatures (in the DB), which fails because the signature was created for a different file.

Ideally pacman should verify the checksum of the file, see that it doesn't match and re-download the file from the mirror.

Might be worth filing a pacman bug report about that.

But ideally we should just handle this in the repo update step, so files don't get replaced in the future.

marysaka pushed a commit to WerWolv/ImHex that referenced this issue Dec 30, 2020
@eyal0
Copy link
Author

eyal0 commented Dec 30, 2020

Sure, let's file that bug! I'm new to msys2. Can you help me with that?

@lazka
Copy link
Member

lazka commented Dec 30, 2020

thanks, but pacman is developed by Arch Linux developers and we have forked it for msys2, so it's better if we file a bug ourselves.

@selsta
Copy link

selsta commented Dec 30, 2020

Still seems to fail for me after reordering: https://github.com/monero-project/monero-gui/pull/3286/checks?check_run_id=1627423478

@eyal0
Copy link
Author

eyal0 commented Dec 30, 2020

Re-ordering worked for me once but now it's failing again and re-ordering isn't working for me now.

@eyal0
Copy link
Author

eyal0 commented Dec 31, 2020

Can there be a test in CI that will cover this situation?

@lazka
Copy link
Member

lazka commented Dec 31, 2020

I guess then the DB and files are out of sync. hopefully fixed in the next upload

@eyal0
Copy link
Author

eyal0 commented Dec 31, 2020

The workaround is easy enough: Take everything out of the install: line and put it in a new step like this:

    - name: pacman
      if: matrix.os == 'windows'
      run: pacman --noconfirm -S --needed mingw-w64-x86_64-gtkmm git base-devel bash mingw-w64-x86_64-gcc mingw-w64-x86_64-boost mingw-w64-x86_64-cairo

@umarcor
Copy link
Contributor

umarcor commented Dec 31, 2020

@eyal0 that works because you are changing the cache key, not because you removed the install field. That is, if you had added any dummy package to the field, it would have worked too. It does not compare against the latest key only, but against all the different keys that were used in the repo. See https://github.com/msys2/setup-msys2/blob/master/main.js#L96-L105.

@eyal0
Copy link
Author

eyal0 commented Dec 31, 2020

@umarcor No. I tried to just change the install field and it didn't not work for me. Here are two consecutive commits that I made:

pcb2gcode/pcb2gcode@ba92e2f

pcb2gcode/pcb2gcode@19a114f

Here are the corresponding CI runs:

https://github.com/pcb2gcode/pcb2gcode/runs/1628278503

https://github.com/pcb2gcode/pcb2gcode/runs/1628833615

You can see that they are both failing even though I have made a brand-new list of installs.

If I look in the log, they have this:

Cache restore for msys2-pkgs-upd:true-conf:69592945, got msys2-pkgs-upd:true-conf:67967524-files:a42aa44f3eae3a469733e4af145c33868de65e0f
Cache restore for msys2-pkgs-upd:true-conf:6f832eb4, got msys2-pkgs-upd:true-conf:67967524-files:a42aa44f3eae3a469733e4af145c33868de65e0f

Notice that they are requesting different restores. One is 69592945 and the other is 6f832eb4. But they are both getting the same one, even though the request was not the same. They are both getting 67967524. That's weird.

I think that the caching action that GitHub provides uses a prefix, where if a cached item is not found, it looks for something by prefix. Because the prefixes match, maybe it's taking the wrong one? I'll will read about it.

@eyal0
Copy link
Author

eyal0 commented Dec 31, 2020

Oops, I have updated my comment. Please re-read.

@eyal0
Copy link
Author

eyal0 commented Dec 31, 2020

Yes, the partial match feature is explained here:

https://docs.github.com/en/free-pro-team@latest/actions/guides/caching-dependencies-to-speed-up-workflows#matching-a-cache-key

I think that partial matches should be ignored. If they are ignored, that will solve this problem. I will create a fork of this repo that ignores partial matches and let you know how it works.

@johnny-bit
Copy link

Side question - how long can this cache problem persist?

@eine
Copy link
Collaborator

eine commented Jan 3, 2021

@lazka, can be we think about a mechanism for flushing the cache? That is, do not load it but have it saved. I'm not sure whether it should be an option in the Action (which users would need to commit in and commit out); or parsing the commit message looking for something such as [skip cache]; or both options.

My main concern is that a simple "clean" run might not suffice. Are cache hits guaranteed to find the latest match?

@lazka
Copy link
Member

lazka commented Jan 3, 2021

if we want to handle such cases in the action we can just have a counter, add it to the hash and bump it +release to invalidate all caches

@eyal0
Copy link
Author

eyal0 commented Jan 3, 2021

@johnny-bit If the cache is using https://github.com/actions/cache then it ought to be 7 days, but only if it is unused for 7 days.

https://github.com/actions/cache#cache-limits

@johnny-bit
Copy link

Well... darktable CI runs certainly use caches ;) IMO cache busting is important since we can run into "15min issue" that in case of cache can last days/week(s) :/

@eine
Copy link
Collaborator

eine commented Jan 3, 2021

if we want to handle such cases in the action we can just have a counter, add it to the hash and bump it +release to invalidate all caches

Do you mean to handle it by us and force all users at the same time, instead of relying on each user reacting to "problems"? That sounds sensible.

@lazka
Copy link
Member

lazka commented Jan 4, 2021

yes

UncombedCoconut added a commit to UncombedCoconut/naev that referenced this issue Jan 5, 2021
UncombedCoconut added a commit to UncombedCoconut/naev that referenced this issue Jan 5, 2021
UncombedCoconut added a commit to UncombedCoconut/naev that referenced this issue Jan 5, 2021
@lazka
Copy link
Member

lazka commented Jan 5, 2021

should be fixed now

@eine
Copy link
Collaborator

eine commented Jan 5, 2021

Shall I still add the counter in this Action?

@lazka
Copy link
Member

lazka commented Jan 5, 2021

sure.

(I'll look into building a new installer today)

UncombedCoconut added a commit to UncombedCoconut/naev that referenced this issue Jan 5, 2021
UncombedCoconut added a commit to UncombedCoconut/naev that referenced this issue Jan 5, 2021
eine added a commit that referenced this issue Jan 6, 2021
@eine
Copy link
Collaborator

eine commented Jan 6, 2021

The base distribution was updated (thanks @lazka!), a constant named CACHE_FLUSH_COUNTER was added to the computation of the cache key (the value is 0 at the moment), a new release was published (v2.1.3) and tag v2 was updated accordingly.

Therefore, this issue should be fixed. I'm closing it, but don't hesitate to comment if you find further issues.

@eine eine closed this as completed Jan 6, 2021
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

No branches or pull requests

6 participants