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

Use tito for builds and share the actions #2956

Merged
merged 2 commits into from
Oct 30, 2023

Conversation

lachmanfrantisek
Copy link
Contributor

No description provided.

@lachmanfrantisek
Copy link
Contributor Author

/packit build

@lachmanfrantisek
Copy link
Contributor Author

Looks like it's failing on the explicit setup.py version check in specfiles:

RPM build errors:
+ STATUS=0
+ '[' 0 -ne 0 ']'
+ cd python-copr-common-git-31.739a509
+ rm -rf /builddir/build/BUILD/python-copr-common-git-31.739a509-SPECPARTS
+ /usr/bin/mkdir -p /builddir/build/BUILD/python-copr-common-git-31.739a509-SPECPARTS
+ /usr/bin/chmod -Rf a+rX,u+w,g-w,o-w .
+ grep '"0.20"' setup.py

@praiskup
Copy link
Member

praiskup commented Oct 16, 2023

Looks like it's failing on the explicit setup.py version check in specfiles:

Yes, that's what tito does. If you do tito build --test --srpm, both the Version in spec file file is bumped and the version in setup.py.

@lachmanfrantisek
Copy link
Contributor Author

@praiskup Good to know. How should I incorporate this with the existing tito build --tgz --test -o . action?

@praiskup
Copy link
Member

@praiskup Good to know. How should I incorporate this with the existing tito build --tgz --test -o . action?

Good question, but dunno :-) when I tested such a generated tarball, it seemed correct to me (the correct version was dumped into the archived setup.py file).

@praiskup
Copy link
Member

It rather feels like the spec file generator and tarball generator are too disconnected; tarball is correct, but the Version tag in spec file is not.

@lachmanfrantisek lachmanfrantisek force-pushed the packit-use-tito branch 2 times, most recently from d4ede76 to 4a17dbe Compare October 20, 2023 11:17
@praiskup
Copy link
Member

Well, if this fixes the problem ... but I'd be much more happy if we well-integrated Tito into packit, and we could use the tito versioning scheme. It is mostly cosmetics, but if someone is used to work with Tito, having completely different versions feels weird.

@@ -16,7 +16,7 @@
%endif

Name: python-copr-common
Version: 0.20.1.dev1
Version: 1.130
Copy link
Member

Choose a reason for hiding this comment

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

Weird ... :-) this seems like cross-package version leak

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ups, I didn't want to commit this...;)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, that was tito who committed that... I should be more careful about playing with that..;)

@lachmanfrantisek
Copy link
Contributor Author

Well, if this fixes the problem ... but I'd be much more happy if we well-integrated Tito into packit, and we could use the tito versioning scheme. It is mostly cosmetics, but if someone is used to work with Tito, having completely different versions feels weird.

Yes, it would be better. I thought that git describe --match $PACKIT_CONFIG_PACKAGE_NAME-[0-9]* --abbrev=0 HEAD | egrep -o [0-9]+\.[0-9]+ should provide the same version as tito.

Also, I didn't find a way how to update setup.py through tito. I see it only in tito tag which we probably don't want to use here...

@lachmanfrantisek
Copy link
Contributor Author

OK, I've changed the approach to reading the version from the specfile so I don't need to change anything. Packit will just edit the release field. I hope this is fine.

@praiskup
Copy link
Member

I thought that git describe --match ... should provide the same version as tito.

I think we should create a dedicated command in Tito for this, if there's no such command, yet.

Also, I didn't find a way how to update setup.py through tito.

Well, Tito never touches the files in the current git repo, unless you do a new release (in which case it is not a temporary change, but a committed change). Do you want just a temporary local change? (not sure Tito knows this, @FrostyX?). Tito normally modifies the file "on the fly" while generating the distribution tarball, in a temporary directory.

@praiskup
Copy link
Member

OK, I've changed the approach to reading the version from the specfile so I don't need to change anything. Packit will just edit the release field. I hope this is fine.

Not sure. The tarball generated by tito will have a bumped version in setup.py, but specfile is not generated by Tito.

@praiskup
Copy link
Member

We have tito build --test --tgz (and --srpm), I suppose we could also implement tito build --test --spec?

Signed-off-by: Frantisek Lachman <flachman@redhat.com>
Packit updates just the release field.

Signed-off-by: Frantisek Lachman <flachman@redhat.com>
@lachmanfrantisek
Copy link
Contributor Author

/packit build

@praiskup
Copy link
Member

Ok, after the Monday meeting, I think this is much better than before. We basically checked that both setup.py and the spec file are not touched by our process (the version is used from git as-is). There's a leftover rpm-software-management/tito#460 that blocks the original issue, but that can be solved separately.

Can't this be merged?

@praiskup
Copy link
Member

@packit-as-a-service rpm-build:fedora-39-x86_64:copr-messaging "In progress"

Still in progress after several days. Is this a new bug? I thought this should have been fixed already.

@lachmanfrantisek lachmanfrantisek marked this pull request as ready for review October 25, 2023 10:48
@lachmanfrantisek
Copy link
Contributor Author

Still in progress after several days. Is this a new bug? I thought this should have been fixed already.

I need to take a look but it looks to me like we've hit a project limit for GitHub interactions since the build itself went well.

@lachmanfrantisek
Copy link
Contributor Author

/packit build

@lachmanfrantisek
Copy link
Contributor Author

Looks like it was really a flake.

@praiskup
Copy link
Member

Nice! Thank you.

@praiskup praiskup merged commit e8d96a2 into fedora-copr:main Oct 30, 2023
@lachmanfrantisek lachmanfrantisek deleted the packit-use-tito branch October 30, 2023 07:45
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.

2 participants