fmf icon indicating copy to clipboard operation
fmf copied to clipboard

PEP621 and `hatch` scripts

Open LecrisUT opened this issue 2 years ago • 28 comments

Here are various modernizaitons to the pyproject

  • Switched to PEP621
  • ~~Switched to src-layout structure.~~ Dropped as people are opposed to it
  • Changed the build backend to hatchling
  • Added some hatch environments and scripts to align with tmt a bit
  • Switched the bin/fmf file to a script entry-point
  • Functional change: Switched to dynamic version
  • Simplified the spec file
  • ~~Removed .travis.yaml~~ #237
  • python-coveralls dependency was removed. Not sure where that one was used
  • Switch PyPI publishing to trusted-publishing. This eliminates the use of the secret token. Closes #224

LecrisUT avatar Aug 09 '23 09:08 LecrisUT

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 ;)

lukaszachy avatar Aug 09 '23 15:08 lukaszachy

Makefile contains useful stuff as 'make (s)rpm', 'make clean'. IMO we need leave it (it can call hatch commands though).

lukaszachy avatar Aug 09 '23 15:08 lukaszachy

Switched to src-layout structure.

We moved AWAY from it in the past. Easier to use libraries locally when python path works out of the box.

lukaszachy avatar Aug 09 '23 15:08 lukaszachy

We need to be able to build on epel-9 which fails now (very likely contains too old macros/utils).

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.

Makefile contains useful stuff as 'make (s)rpm', 'make clean'. IMO we need leave it (it can call hatch commands though).

Sure thing. I would still prefer the CI, packit and spec to use the direct commands though. Fine with that?

We moved AWAY from it in the past. Easier to use libraries locally when python path works out of the box.

Roger. It is a dev preference in the end

LecrisUT avatar Aug 09 '23 15:08 LecrisUT

I am not sure why Rhel9 build is failing. I can't seem to debug it to check if the tmp folder includes .git_archival.txt. Any ideas on how to debug that?

LecrisUT avatar Aug 09 '23 18:08 LecrisUT

Seems like the error message in the JsonSchemaValidator is different on the other distros:

[31mFailed to load step data for DiscoverFmfStepData: Field 'DiscoverFmfStepData:ref' must be a string, 'int' found.[0m
:: [ 18:35:04 ] :: [   FAIL   ] :: File '/var/tmp/rlRun_LOG.VEO4lNoT' should contain 'Failed to load step data for DiscoverFmfStepData: Field 'DiscoverFmfStepData:ref' must be unset or a string, 'int' found.' 

(note the lack of must be unset or...

And similar with tmt errors reported.

LecrisUT avatar Aug 09 '23 19:08 LecrisUT

@psss What about epel-8 though, can we stop providing updates as we did with tmt? epel-8 has even older macros ;)

Yes, I'd say we should keep fmf and tmt aligned, let's stop providing updates for el-8.

psss avatar Aug 10 '23 07:08 psss

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.

LecrisUT avatar Aug 10 '23 08:08 LecrisUT

@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 tmt not being in the stable repo. Should be good now.

psss avatar Aug 10 '23 12:08 psss

/packit test

psss avatar Aug 10 '23 12:08 psss

@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?

LecrisUT avatar Aug 11 '23 09:08 LecrisUT

/packit test

LecrisUT avatar Aug 11 '23 11:08 LecrisUT

@lukaszachy, @psss, @happz Review for this one please?

LecrisUT avatar Aug 25 '23 08:08 LecrisUT

@psss What about epel-8 though, can we stop providing updates as we did with tmt? epel-8 has even older macros ;)

Yes, I'd say we should keep fmf and tmt aligned, let's stop providing updates for el-8.

Filed #212 to drop support for el8.

psss avatar Sep 19 '23 07:09 psss

@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.

psss avatar Sep 19 '23 07:09 psss

Let's finish this as one of the first things for the 1.4 release.

psss avatar Sep 29 '23 18:09 psss

Oh, I need to rebase (after #215), need anything else than that?

LecrisUT avatar Sep 29 '23 19:09 LecrisUT

Yes, please rebase. No other changes needed right now (at least from the quick look through the changes).

psss avatar Oct 02 '23 16:10 psss

One main comment, afaik python-build is not a build backend, just an interface for pep517

I wrote frontend, not backend :)

martinhoyer avatar Oct 04 '23 08:10 martinhoyer

One main comment, afaik python-build is not a build backend, just an interface for pep517

I wrote frontend, not backend :)

Oh midnight brain didn't quite parse that :D. So the main reason for using hatch frontend instead of build is because it limits the dependencies? At least one frontend dependency would still have to be added and I think we can maneuver to use either one. I would prefer to use something backend agnostic like build unless there is heavy use of the hatch specific options like the venv

I will deffer the decision to the maintainers. @psss, @lukaszachy what are your preferences?

LecrisUT avatar Oct 04 '23 09:10 LecrisUT

Since it seems to have converged to using hatch I have converted the scripts to hatch. @martinhoyer, can you review again to see which ones need to be converted? Other than that, @psss can we get this merged soon?

LecrisUT avatar Dec 01 '23 13:12 LecrisUT

@LecrisUT, thanks much for preparing this! We definitely want to go this way to make the approach as close to tmt as possible. However, we would like to not block the 1.4 release on this. Let's try to finish the review early after fmf-1.4 is out. Thanks for the patience.

psss avatar Apr 26 '24 13:04 psss

Of course. It might be better to split this one a bit into:

  • Quick fixes like: aea90a12494c8822725ff8a10ff39395678a0dce
  • Maintenance: like drop epel8
  • PEP621

Other CI tasks, I will probably remove them from here. Any idea on which one you would want early for fmf-1.4?

LecrisUT avatar Apr 26 '24 13:04 LecrisUT

Let's try this again, I've minimized this PR to just PEP621 + hatch related changes, @martinhoyer I will resolve all conversations, but re-open them or create a new one if I missed something important

LecrisUT avatar May 21 '24 08:05 LecrisUT

@psss @lukaszachy @martinhoyer @happz What do you think of getting this in for 1.4, now that there has been quite some simplification to this PR. This PR is decoupled from most changes that require further discussion (e.g. #241), see the top-level comment for all changes in this PR. Right now it should not have any functional changes, other than the dynamic versioning, but @martinhoyer can you double-check it?

The current status is:

  • 1 unresolved discussion about tests optional dependency. @martinhoyer can we move this for another PR issue?
  • setting up trusted publishing

Addressing #224 (40023ee833a34c1a68340150f0c2c567ad0958ad) would be very useful before pushing and update, maybe I can cherry-pick that one out if needed.

LecrisUT avatar Jun 05 '24 12:06 LecrisUT

@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)

martinhoyer avatar Jun 05 '24 13:06 martinhoyer

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 tmt experience and the packaging changes) I'd expect there will be a couple of bumps on the road. So let's not block 1.4 any more to get it out finally and let's try to finish this pull request among the first ones in 1.5.

psss avatar Jun 05 '24 13:06 psss

any luck with this PR?

chenrui333 avatar Oct 02 '24 19:10 chenrui333

/packit build

psss avatar Nov 28 '24 07:11 psss

@martinhoyer, could you please review as well?

psss avatar Nov 28 '24 07:11 psss