#86 Add support for multiple releases and f29, rawhide
Closed 10 months ago by nphilipp. Opened 11 months ago by otaylor.
modularity/ otaylor/fedmod multiple-releases  into  master

@@ -114,7 +114,7 @@ 

          modular_repo="https://dl.fedoraproject.org/pub/fedora/linux/development/rawhide/Modular/",

      )

  }

- _DEFAULT_RELEASE_NAME = "f28"

+ _DEFAULT_RELEASE_NAME = "f29"

  

  _ARCHES = set(["aarch64", "armv7hl", "i686", "ppc64", "ppc64le", "s390x", "x86_64"])

  _DEFAULT_ARCH = "x86_64"

file modified
+4 -2

@@ -23,16 +23,18 @@ 

          _load_dataset()

      return _ACTIVE_DATASET

  

+ def dataset_release_name():

+     return parse_dataset_name(dataset_name)[0]

+ 

  def set_dataset_name(name):

      """

      Set the dataset name that will be used for further operations

  

      Raises ValueError if validation fails.

      """

+     global dataset_name

      # validates as a side effect

      parse_dataset_name(name)

- 

-     global dataset_name

      dataset_name = name

  

  def list_modules():

@@ -9,18 +9,22 @@ 

  import gi

  from gi.repository import Modulemd

  

+ from ._repodata import dataset_release_name

  from .get_module_builds import get_module_builds

  from .module_generator import ModuleGenerator

  from .util import rpm_name_only

  from . import _depchase

  

- 

- FLATPAK_RUNTIME_STREAM = 'f28'

- DEFAULT_RPM_BRANCH = 'f28'

+ def dataset_release_branch():

+     release_name = dataset_release_name()

+     if release_name == 'rawhide':

+         return 'master'

+     else:

+         return release_name

  

  

  def _get_runtime_packages():

-     builds = get_module_builds('flatpak-runtime', FLATPAK_RUNTIME_STREAM)

+     builds = get_module_builds('flatpak-runtime', dataset_release_branch())

      # Each flatpak-runtime stream should be built against a single context

      assert len(builds) == 1

      build = builds[0]

@@ -34,7 +38,7 @@ 

  class FlatpakGenerator(ModuleGenerator):

      def __init__(self, pkg):

          super().__init__([pkg])

-         self.rpm_branch = DEFAULT_RPM_BRANCH

+         self.rpm_branch = dataset_release_branch()

  

      def _calculate_dependencies(self):

          pkg = self.pkgs[0]

@@ -50,7 +54,7 @@ 

          run_srpms = {rpm_name_only(_depchase.get_srpm_for_rpm(pool, dep)) for dep in pkgs}

          self.run_srpms = run_srpms - self.api_srpms

  

-         self.module_run_deps = {'flatpak-runtime': [FLATPAK_RUNTIME_STREAM]}

+         self.module_run_deps = {'flatpak-runtime': [dataset_release_branch()]}

  

      def _update_module_md(self):

          super()._update_module_md()

@@ -76,8 +76,8 @@ 

  

          buildrequires = dependencies[0].props.buildrequires

          assert set(buildrequires) == {'flatpak-runtime',}

-         assert buildrequires['flatpak-runtime'].get() == ['f28']

+         assert buildrequires['flatpak-runtime'].get() == ['f29']

  

          requires = dependencies[0].props.requires

          assert set(requires) == {'flatpak-runtime',}

-         assert requires['flatpak-runtime'].get() == ['f28']

+         assert requires['flatpak-runtime'].get() == ['f29']

There was some idea in the code that the release and architecture
should be configurable. Make this a reality by adding a:

 fedmod --dataset=RELEASE[-ARCH]

argument, and encapsulating things that were previously in global
variables in data objects. RELEASE can be f28 or rawhide, and
ARCH can be any supported Fedora arch.

(In general, differences between architectures should be very small,
but since the code already looked like it wanted to support choosing the
architecture, I decided to go ahead and implement it. You could use this
to check if you ran into a problem and suspected that the dependency
graphs were different on different architectures.)

3 new commits added

  • Default to the f29 dataset
  • Flatpak: Base runtime and package branches off of the current dataset
  • Add a f29 release version
11 months ago

Add F29 support, and made things default to F29.

@nphilipp - ping? this would be useful to merge.

@otaylor NIls should be back next week. Is this urgent?

@psabata - I want to send a "call for Flatpak packagers" out soon, and it
would be helpful to have a release of fedmod with these changes for that,
but I probably need the next couple of days to write docs and tie up some
loose ends, and Monday is a holiday in the US, so I think it can wait until
Nils is back. Hopefully he'll have a chance to look at it then!

Thanks! I'll make sure to remind him :)

Metadata Update from @nphilipp:
- Request assigned

10 months ago

I think it could be worthwhile to let people use the respective testing repos—updates-testing, updates-testing-modular—too, what do you think?

Not blocking this, but ultimately this should be configurable somewhere outside the source code.

Besides that, there shouldn't be spaces around = here because they're keyword parameter assignments.

It looks like if I commented all PEP8 issues it would clutter up the review too much. I'll just fix them afterwards. :wink:

Not sure it makes a real difference (since the ClickException is caught) but as we don't care for Python 2 compatibility this could be raise click.ClickException(e) from e.

It looks like if I commented all PEP8 issues it would clutter up the review too much. I'll just fix them afterwards. 😉

I think the only reasonably way to make sure that the code is PEP8 compliant is to have a policy of having zero errors from flake8. (Ideally set up CI - but at least contributors running flake8 manually.) Trying to spot commits that break PEP8 compliance by eye is no way to go.

Are you thinking of a config file in /etc, or just of a non-source file installed with Python code? The main advantage I see of a config file is that if you needed some custom dataset, you could create it. But some cleanups would be needed before we stabilized a config file format - e.g., the code:

   if "updates" in remote_prefix:
        remote_arch_path = arch + "/"
    else:
        remote_arch_path = os.path.join(arch, "os/")

Is not OK - you'd need yum style $basearch.

The main use case I can think of is:

  • You run rpm2flatpak <apprpm>
  • You see a whole bunch of extra deps due to a packaging error (e.g., the app dependended on gvfs rather than gvfs -client)
  • You fix the error in packaging and fire off a build
  • You want to run rpm2flatpak and get the correct result

updates-testing makes that a ~6 hour wait instead of a week. It would be even nicer to be able to do --with-koji-build=<nnnn> or something - but a lot more work in fedmod. In any case, I think I'll hold off on this for now.

(What I have patched in locally here is a f29koji dataset that uses https://kojipkgs.fedoraproject.org/repos/f29-build/latest/ - in the current f29 freeze period the latest compose has diverged from the buildroot in a way that causes problems for my flatpak-runtime generation scripts. But not convinced yet this is a general need. Maybe a case for a configuration file!)

7 new commits added

  • Default to the f29 dataset
  • Flatpak: Base runtime and package branches off of the current dataset
  • Add a rawhide release version
  • Allow for multiple release versions, and add a --dataset argument
  • Add a f29 release version
  • Add F28 modular updates repo
  • Fix usage of attrs
10 months ago

Pushed an update that fixes a few PEP8 issues and uses 'raise ... from'.

7 new commits added

  • Default to the f29 dataset
  • Flatpak: Base runtime and package branches off of the current dataset
  • Add a rawhide release version
  • Allow for multiple release versions, and add a --dataset argument
  • Add a f29 release version
  • Add F28 modular updates repo
  • Fix usage of attrs
10 months ago

I think the only reasonably way to make sure that the code is PEP8 compliant is to have a policy of having zero errors from flake8. (Ideally set up CI - but at least contributors running flake8 manually.) Trying to spot commits that break PEP8 compliance by eye is no way to go.

Absolutely.

Are you thinking of a config file in /etc, or just of a non-source file installed with Python code? The main advantage I see of a config file is that if you needed some custom dataset, you could create it. But some cleanups would be needed before we stabilized a config file format - e.g., the code: if "updates" in remote_prefix: remote_arch_path = arch + "/" else: remote_arch_path = os.path.join(arch, "os/") Is not OK - you'd need yum style $basearch.

I'd like a configuration file (thinking of /etc + somewhere in the user's home), but you're right that this needs some fleshing out and is out of scope of this PR.

The main use case I can think of is:

  • You run rpm2flatpak <apprpm>
  • You see a whole bunch of extra deps due to a packaging error (e.g., the app dependended on gvfs rather than gvfs -client)
  • You fix the error in packaging and fire off a build
  • You want to run rpm2flatpak and get the correct result updates-testing makes that a ~6 hour wait instead of a week.

It would be even nicer to be able to do --with-koji-build=<nnnn> or something - but a lot more work in fedmod. In any case, I think I'll hold off on this for now. (What I have patched in locally here is a f29koji dataset that uses https://kojipkgs.fedoraproject.org/repos/f29-build/latest/ - in the current f29 freeze period the latest compose has diverged from the buildroot in a way that causes problems for my flatpak-runtime generation scripts. But not convinced yet this is a general need. Maybe a case for a configuration file!)

Yes, being able to use updates-testing or a specific koji build id (or maybe even local/remote git hashes/spec files though getting deps reliably out of that sounds like a headache) would be useful to minimize turnaround times.

I've found a typo in one of the commit logs which I'll fix before applying:

commit 713244c61d0a856eeb324dc001116076b236a54e
Author:     Owen W. Taylor <otaylor@fishsoup.net>
AuthorDate: Tue Aug 21 17:44:14 2018 -0400
Commit:     Owen W. Taylor <otaylor@fishsoup.net>
CommitDate: Wed Sep 5 13:55:51 2018 -0400

    Add a f29 release version

    Add a release definition for --dataset=rawhide.

This should rather be ... --dataset=f29.

rebased onto 2cae1b3

10 months ago

Pull-Request has been closed by nphilipp

10 months ago