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

git tag was not created for 4.3.0 #2753

Open
MartinThoma opened this issue Jul 14, 2024 · 21 comments
Open

git tag was not created for 4.3.0 #2753

MartinThoma opened this issue Jul 14, 2024 · 21 comments
Labels
nf-ci Non-functional change: Continuous Integration

Comments

@MartinThoma
Copy link
Member

This is mostly a reminder for me to check why the git tag was not created for the commit REL: 4.3.0.
Something does not work as expected with https://github.com/py-pdf/pypdf/blob/main/.github/workflows/release.yaml

image

@MartinThoma MartinThoma added the nf-ci Non-functional change: Continuous Integration label Jul 14, 2024
@MasterOdin
Copy link
Member

I wonder if it's a limitation of using secrets.GITHUB_TOKEN for trying to create a protected tag, where the user associated doesn't have admin/maintainer rights on the repo? Might need to use a PAT, though that feels like it would bypass the entire reason to have protected tags.

@stefan6419846
Copy link
Collaborator

As far as I remember, we already had discussions about PATs, but tried to avoid it as they have a limited lifetime as well. According to the docs, our approach should work, although it seems like push protections prevent this from actually running correctly.

@MartinThoma
Copy link
Member Author

Yes, my guess was as well that this is a security mechanism. I'm uncertain if there is a reasonable way around it.

In the end, I also would trust you @stefan6419846 / @pubpub-zz enough to handle releases for pypdf. So I'm also thinking about simply giving you maintainer permissions.

@kitterma
Copy link
Contributor

It looks to me like there's a tag there (at least now), but it's not signed, which is a departure from previous releases.

@stefan6419846
Copy link
Collaborator

Yes, the tag has been created in the meantime, but it should have been created automatically - this is what this issue seeks to further analyze. In this case, it seems like there is some setting which show an issue on the GitHub UI:

ksnip_20240719-090637

Do you rely on all tags being signed?

@kitterma
Copy link
Contributor

kitterma commented Jul 19, 2024 via email

@MartinThoma
Copy link
Member Author

I could force-push a signed tag. Could that cause issues?

The workflow will attempt to create a new release on pypi, but that will fail (which is ok)

@kitterma
Copy link
Contributor

In Debian our tooling is set up to pull from a signed tag, so for us, that's what's most important.

@MartinThoma
Copy link
Member Author

@pubpub-zz @stefan6419846 You have done a great job as core contributors to pypdf for an extended period of time. I trust your intentions and abilities, hence I gave both of you the "Maintainer" permissions on the pypdf repository. That should mean you can trigger the release by merging a "REL: XYZ" commit in future.

@MartinThoma
Copy link
Member Author

MartinThoma commented Jul 21, 2024

@kitterma pypdf releases relatively often. Is it fine when we just release the next release with a signed git tag? We could release 4.3.1 today :-)

edit: PR is prepared #2764

@kitterma
Copy link
Contributor

kitterma commented Jul 21, 2024 via email

@stefan6419846
Copy link
Collaborator

Release 4.3.1 failed in CI again.

As far as I understand, the executing user (probably GitHub Actions in this case) needs push permissions for protected branches - even if just creating a tag: https://github.blog/changelog/2021-11-19-allow-bypassing-required-pull-requests/

@MartinThoma
Copy link
Member Author

I've created the tag manually via git tag -s 4.3.1, pasting the part of the changelog, git push --tags.

I'll look more into that next week 👀 🥲

@kitterma
Copy link
Contributor

That worked. Thanks.

@pubpub-zz
Copy link
Collaborator

@MartinThoma
Have you been able to progress in your analysis? Just for information we have some PRs in the pipe / already merged that should be released in 5.0.0 although we are not ready yet to tag it

@pubpub-zz
Copy link
Collaborator

@MartinThoma
+1 ?

@pubpub-zz
Copy link
Collaborator

@kitterma
Can you tell me if release 5.0.0 is good for you ?

@pubpub-zz
Copy link
Collaborator

@MartinThoma
Will you have time to fix this issue or should we consider that we have to use the workaround using git tag if so can you update the documentation ?

@kitterma
Copy link
Contributor

@kitterma Can you tell me if release 5.0.0 is good for you ?

No. I can see the tag, but it's not signed. The commit is signed, but the tag is not. If you look in the GitHub U/I both 4.2.0 and 5.0.0 both show as verified, but if you click on the little verified tag for the tag it shows a signed tag in 4.2.0 (and 4.3.1) and a signed commit in 5.0.0. I guess this is progress, but it's the tag we need to verify.

@stefan6419846
Copy link
Collaborator

stefan6419846 commented Sep 18, 2024

Could you please elaborate why each tag needs to be signed and verifiable in this case? While I appreciate a good relationship and factual discussion between downstream and upstream, I do not really understand why you as the downstream packager for a distro have to strictly rely on each of our upstream tags to actually be signed? (The goal is to further automate creating tags once we manage to fix the remaining CI issues and AFAIK these tags will not be signed as well unless we explicitly implement it.)

@pubpub-zz
Copy link
Collaborator

at next release (most probably by end of the month) I will test if the tag can be signed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
nf-ci Non-functional change: Continuous Integration
Projects
None yet
Development

No branches or pull requests

5 participants