#1609 Add an option to allow skipping branding and letting the distributor set their own branding package
Merged 2 years ago by lsedlar. Opened 2 years ago by lzhuang.
Unknown source master  into  master

Add skip_branding to ostree_installer.
Lingyan Zhuang • 2 years ago  
file modified
+2
@@ -1786,6 +1786,8 @@

      with the optional key:

  

      * ``extra_runroot_pkgs`` -- (*[str]*)

+     * ``skip_branding`` -- (*bool*) Stops lorax to install packages with branding.

+       Defaults to ``False``.

  

  **ostree_installer_overwrite** = False

      (*bool*) -- by default if a variant including OSTree installer also creates

file modified
+1
@@ -1078,6 +1078,7 @@

                          "template_repo": {"type": "string"},

                          "template_branch": {"type": "string"},

                          "extra_runroot_pkgs": {"$ref": "#/definitions/list_of_strings"},

+                         "skip_branding": {"type": "boolean", "default": False},

                      },

                      "additionalProperties": False,

                  }

@@ -272,6 +272,7 @@

                  rootfs_size=config.get("rootfs_size"),

                  is_final=compose.supported,

                  log_dir=self.logdir,

+                 skip_branding=config.get("skip_branding"),

              )

              cmd = "rm -rf %s && %s" % (

                  shlex_quote(output_dir),

Add parameters --skip-branding and brand to ostree/installer

Add skip_branding to lorax_cmd in phases/ostree_installer

Signed-off-by: Lingyan Zhuang lzhuang@redhat.com

Please add Fixes: #1594 into the commit message.

On second reading of the code, it seems that the pungi/ostree/installer.py is not actually used, and only the change in phases/ostree_installer is really needed. It's quite messy actually. As far as I can tell, the phase has two ways of submitting the task. It can use either runroot or pungi_buildinstall koji plugins. The new option is now only supported in the runroot path (which is fine, as the other plugin would need to implement a corresponding change too).

Then there is a separate pungi-make-ostree installer executable. As far as I can tell, that is not used by the rest of the Pungi code anywhere. If that's correct, then the code should be dropped rather than fixed.

The new option should be added to the schema for config: https://pagure.io/pungi/blob/master/f/pungi/checks.py#_1063 and also mentioned in documentation.

I'm not sure about purpose of this option.

The default lorax behavior is that it installs some package with branding. It has an option --skip-branding which stops that, and instead the caller is required to use --installpkgs custom-branding-package to provide their own package. The ostree_installer phase should already support the installpkgs option (allowing user to customize what gets installed).

black would like to see a , at the end of this line.

1 new commit added

  • Revert "Add an option to allow skipping branding and letting the distributor set their own branding package"
2 years ago

rebased onto 2e160bc

2 years ago

Please add Fixes: #1594 into the commit message.

On second reading of the code, it seems that the pungi/ostree/installer.py is not actually used, and only the change in phases/ostree_installer is really needed. It's quite messy actually. As far as I can tell, the phase has two ways of submitting the task. It can use either runroot or pungi_buildinstall koji plugins. The new option is now only supported in the runroot path (which is fine, as the other plugin would need to implement a corresponding change too).

Then there is a separate pungi-make-ostree installer executable. As far as I can tell, that is not used by the rest of the Pungi code anywhere. If that's correct, then the code should be dropped rather than fixed.

The new option should be added to the schema for config: https://pagure.io/pungi/blob/master/f/pungi/checks.py#_1063 and also mentioned in documentation.

I've removed change to pungi/ostree/installer.py and wrappers/lorax.py.

And added skip_branding in checks.py and related doc.

Looks good to me. The default value in schema is not really needed. If removed, it will default to None which is effectively identical, and the tests will be passing then. I'll update that and merge.

Commit c4aa45b fixes this pull-request

Pull-Request has been merged by lsedlar

2 years ago