#2965 PR merged: Add proper python support / fix GALAXY11-related issues

Labels: fixed / solved / done, sponsored, Dedicated Priority Support

schlomo opened issue at 2023-04-02 21:11:

(this work is for a client who uses GALAXY11 and Python, hence multiple topics in one PR - sorry)

New

  • Include Python interpreter (minimal or full) via new variables PYTHON_INTERPRETER and PYTHON_MINIMAL (both false by default)
  • mkboot workflow for bare boot image without rescue info (for testing stuff not related to backup/restore)
  • checklayout workflow now runs full prep stage, first step towards resolving #2951 and required for GALAXY11

Improved for GALAXY11

  • reworked point-in-time recovery
  • inject credentials and configuration via environment variables for remote-controlled recovery automation
  • reduce false positives in library dependency check for Galaxy11 binaries
  • checklayout will also notice a change in GALAXY11 installation to allow updating the rescue image after a GALAXY11 update

Tested

Manually on OL8u7

jsmeix commented at 2023-04-04 12:21:

@schlomo
I had only a very quick look.
Currently I am busy with other things
so I cannot do a review.

schlomo commented at 2023-04-04 13:36:

That is OK @jsmeix, thanks for taking a quick look. For newer systems (Python 3.4ff) this PR could simplify adding Python for YUM and DUPLICITY, but somebody who is actually using those will need to test and adapt the old code.

schlomo commented at 2023-04-14 12:29:

Dear @rear/contributors I'd like to merge this really soon, latest on Monday.

This PR adds generic Python support and also enables the full prep stage for the checklayout workflow to enable using dynamic infos in checklayout.

I believe that the biggest risk is that during checklayout ReaR will create some files in TMP_DIR or in ROOTFS_DIR which are not required for checklayout, and which can be safely ignored.

We can then still slowly work on moving those scripts out of the prep stage into the rescue stage.

pcahyna commented at 2023-04-17 12:11:

give me a moment to review this please - I'll do it today hopefully.

pcahyna commented at 2023-04-17 15:16:

Hi @schlomo , it seems that the PR does at least 4 different things:

  • Python support
  • GALAXY11 changes (is GALAXY written in Python, or what is the relation?)
  • Adding a new "mkboot" workflow (how is that related?)
  • Using the "prep" stage in checklayout (why? and how is this related?)

the last two are not even described in the PR title, nor in the description.

I believe unrelated changes (if they are unrelated) should be preferably split into separate PRs (if too difficult, then at least into separate commits), and in any case properly described to avoid surprises like: https://github.com/rear/rear/issues/2967

Since
https://github.com/rear/rear/commit/b83059a72d2ea8735719cf415c6cafd5f43312f0
there is - by the way - a new
init/default/998_dump_variables.sh
which is unrelated to what the commit message describes.

schlomo commented at 2023-04-17 15:37:

@pcahyna this PR contains all what i need for this customer who uses Galaxy 11 and Python.

I hear your feedback on PR discipline and agree with you. I'll be happy to be better next time and would appreciate is to go forward with this.

The variable dump script is something I'll be happy to remove if it helps.

Everything else i needed to make it work or to test the code.

mkboot allows me to create a generic boot media without caring about a backup method or rescue stuff, super useful for working on a feature like Python that doesn't depend on anything.

schlomo commented at 2023-04-17 15:39:

Prep stage in Checklayout is for #2951 and using variables defined in prep during layout check.

pcahyna commented at 2023-04-17 15:46:

@schlomo could you please do an interactive rebase to correct the issues in commits? While we at it, the typo, fix stage ordering, fix stage ordering commits can be squashed into the commits that they fix. And Include Pyhon and dependencies should be easily split to a Python part commit and the mkboot workflow part commit, with proper descriptions.

pcahyna commented at 2023-04-17 15:47:

Also, while rebasing, please use shorter title lines for commits, see https://cbea.ms/git-commit/#limit-50 (also see https://cbea.ms/git-commit/#separate )

pcahyna commented at 2023-04-17 15:50:

Concerning PR discipline, if you don't want to file separate PRs, please edit this PR description to mention all the significant changes, and not just some of them.

pcahyna commented at 2023-04-17 15:53:

Prep stage in Checklayout is for #2951 and using variables defined in prep during layout check.

Ok, can you please describe the reason in the commit message of 9e7e52163afaa7acca56ed2cc7a4b49aa369186d ? (I now see what you meant by "Add Galaxy Home Dir to checklayout verification, this is an example i…

…mplementation for https://github.com/rear/rear/issues/2951", but the description itself was too terse.)

pcahyna commented at 2023-04-17 15:55:

The variable dump script is something I'll be happy to remove if it helps.

This was just an example to show how confusing it is to submit changes without describing them, the dump script itself should be discussed separately.

schlomo commented at 2023-04-18 10:53:

@pcahyna I updated everything and hope that you can now review/approve this PR.

pcahyna commented at 2023-04-18 12:57:

@schlomo thanks, looks better - please just split 155d619 into "Include Pyhon and dependencies" and "new mkboot workflow to create boot image without rescue information" with correcting the typo in title (see https://git-scm.com/docs/git-rebase#_splitting_commits on how to do it), and improve the description of b4fdb61 - see https://github.com/rear/rear/pull/2965#issuecomment-1511639361 .

I do not have any major comments on the change content.

schlomo commented at 2023-04-19 14:25:

Thanks @pcahyna for your detailed feedback!

pcahyna commented at 2023-04-19 14:36:

Thank you, it looks much better now.

jsmeix commented at 2023-04-20 09:06:

@schlomo
thank you for those major improvements in ReaR
and please forward my sincere thanks to those
who sponsor (i.e. pay for) your efforts!

[Export of Github issue for rear/rear.]