-
Notifications
You must be signed in to change notification settings - Fork 28
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
PEP621 and hatch
scripts
#202
base: main
Are you sure you want to change the base?
Conversation
We need to be able to build on epel-9 which fails now (very likely contains too old macros/utils). @psss WHat about epel-8 though, can we stop providing updates as we did with tmt? epel-8 has even older macros ;) |
Makefile contains useful stuff as 'make (s)rpm', 'make clean'. IMO we need leave it (it can call hatch commands though). |
We moved AWAY from it in the past. Easier to use libraries locally when python path works out of the box. |
Indeed, I was just about to look into that. I was debugging locally and I hit an issue with it just now about dynamic version. It's weird that the same setup is used in packit. That needs a bit of debugging.
Sure thing. I would still prefer the CI, packit and spec to use the direct commands though. Fine with that?
Roger. It is a dev preference in the end |
059f6d9
to
4d79f9c
Compare
I am not sure why Rhel9 build is failing. I can't seem to debug it to check if the tmp folder includes |
Seems like the error message in the JsonSchemaValidator is different on the other distros:
(note the lack of And similar with tmt errors reported. |
Yes, I'd say we should keep |
Ok, this should be ok for this PR. @psss can someone look into the testing-farm failures? They are in the integration test and it should just need a more relaxed check of the fail message. |
The testing farm failures were most probably caused by the fresh |
/packit test |
8ccf9b6
to
8ca3f85
Compare
@lukaszachy , @psss Ok, with this Epel 9 is fixed. Thanks to Nikola from packit for debugging the issue. I've also expanded the github tests, can someone trigger them? I've also enabled codecov, but on my project it did not work without a secret token. Maybe that needs to be setup here as well? |
/packit test |
@lukaszachy, @psss, @happz Review for this one please? |
Sorry, had a bunch of stuff on the plate so I did no get to this. Will have a look, hopefully this week. Sorry for the delay. |
Let's finish this as one of the first things for the |
Oh, I need to rebase (after #215), need anything else than that? |
Yes, please rebase. No other changes needed right now (at least from the quick look through the changes). |
@psss @lukaszachy @martinhoyer @happz What do you think of getting this in for The current status is:
Addressing #224 (40023ee) would be very useful before pushing and update, maybe I can cherry-pick that one out if needed. |
@LecrisUT Apologies, I'm afk for the rest of the week. Fwiw, I think it's in great state, but it might be a good idea to not rush it into 1.4 and instead maybe make 1.5 a dedicated release just for this? (thinking aloud) |
Thanks for putting this all together and for the nice summary! Had a quick look, sounds good, but it's not that little change and (also from the |
Signed-off-by: Cristian Le <cristian.le@mpsd.mpg.de>
Signed-off-by: Cristian Le <cristian.le@mpsd.mpg.de>
Signed-off-by: Cristian Le <cristian.le@mpsd.mpg.de>
Signed-off-by: Cristian Le <cristian.le@mpsd.mpg.de>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a few comments. One more thing I wonder about is how to generate manpage - if it make sense to have it same as https://github.com/teemtee/tmt/blob/54dfd29ede1e3a20bad98bb6bf8c79dc0da25e54/pyproject.toml#L183
.packit.yaml
Outdated
@@ -5,15 +5,18 @@ synced_files: | |||
upstream_package_name: fmf | |||
downstream_package_name: fmf | |||
|
|||
# Epel9 fails to build with dynamic version. Need to create archive with PKG-INFO | |||
# F37 works with setuptools_scm 7.0 | |||
actions: | |||
create-archive: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We might need to add - hatch run docs:man
(or however we'll generate man) before build, same as in tmt.
Would be cool to drop man completely, but it didn't fly in tmt, so assuming it won't fly here either.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is included in the spec
file
cp docs/header.txt man.rst
tail -n+7 README.rst >> man.rst
rst2man man.rst > fmf.1
For me I think sdist
should be as vanilla as close as possible to the git archive. But regarding the man generation, we should see about if we want to change it in teemtee/tmt#3088.
We still have quite a few discrepancies as tmt
has evolved, so how about handling these one step at a time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's tricky, because then you have discrepancies between rpm source and pypi source. It's a "hack" either way. No strong opinion from me.
fmf.spec
Outdated
BuildRequires: python3-devel | ||
BuildRequires: python3dist(docutils) | ||
BuildRequires: git-core | ||
Requires: python3-fmf == %{version}-%{release} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would suggest to do the same as in https://github.com/teemtee/tmt/blob/main/tmt.spec, i.e. %py_provides python3-fmf
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was discussed in other, now resolved thread.
@psss Would you agree that we should build fmf
package, which would also provide python3-fmf
, in the same fashion as tmt?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you remember why there wasn't a Obsoletes: python3-tmt
?
Signed-off-by: Cristian Le <cristian.le@mpsd.mpg.de>
Signed-off-by: Cristian Le <cristian.le@mpsd.mpg.de>
5f38cb8
to
b3bed5f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks ready to me, except for the 'test' subpackage.
Signed-off-by: Cristian Le <cristian.le@mpsd.mpg.de>
Here are various modernizaitons to the pyproject
Switched toDropped as people are opposed to itsrc-layout
structure.bin/fmf
file to a script entry-pointRemovedSmall cleanups #237.travis.yaml
python-coveralls
dependency was removed. Not sure where that one was usedsdist
in PyPI #224