dracutdevs / dracut

dracut the event driven initramfs infrastructure
https://github.com/dracutdevs/dracut/wiki
GNU General Public License v2.0
604 stars 400 forks source link

feat(assure_command_shell): assure proper installation of a shell #2368

Open FGrose opened 1 year ago

FGrose commented 1 year ago

Introduce a new meta module, 99~sh, that will check that a command shell has been specified after module checks have been performed and before executable dependencies are checked. This avoids implicit installations of a command shell that skip the dracut module checks that the shell might need.

This PR now includes 5 commits to implement the feature:

  1. feat(module-postprocess): provide a postprocessing loop for modules
  2. feat(99~sh): assure explicit installation of a command shell
  3. feat(99~no~sh): allow a build without any command shell
  4. fix(00bash): fail if Bash below version 4.4 is linked to /bin/sh
  5. refactor(99squash): gather code into the squash module

2 (indirectly) and 3, 4, & 5 above use the postprocessing feature directly to achieve the intention.

Completes fix to #1530 augmenting #1534.

An alternative command shell, for example, zsh can be installed by providing an appropriate module-setup.sh at ../modules.d/00zsh/ and running dracut with ln -sf zsh /bin/sh.

Checklist

LaszloGombos commented 1 year ago

busybox is also supported as a shell

FGrose commented 1 year ago

@LaszloGombos > busybox is also supported as a shell Updated to include busybox in supported list.

LaszloGombos commented 1 year ago

Updated to include busybox in supported list.

Thanks.

1./ In my mental model, dracut.sh is kind of a package/dependency manager for dracut modules. When we add/remove new dracut modules we should try to avoid keep changing the "package manager" itself.

In order to do this, you can try to introduce a new dracut "meta package" module for shell selection. Perhaps you can call it "sh" module. I hope you can model it after the dbus or network meta module. Please take a close look at https://github.com/dracutdevs/dracut/pull/2181 for a good way to do this.

Ideally this would result in moving the assure_command_shell logic to the sh meta package. The base dracut module should be made dependent of the new "sh" meta dracut package.

2./ The current PR seems to be breaking test (and likely introducing regression). Please check test next time you upload the new revision of the PR.

Thank you so much for taking this on.

LaszloGombos commented 1 year ago

By chance I just run into the following (non-fatal) error running the following fro ma git checkout

dracut.sh --list-modules -l

/home/usr/canary/dracut-prlist/dracut-init.sh: line 255: -D: command not found
dracut[E]: FAILED:  -D /var/tmp/dracut.U7jRKD/initramfs /bin/sh
dracut[I]: Executing: /home/usr/canary/dracut-prlist/dracut.sh --list-modules -l

Just wondering would this PR also resolve this (as it moves some code into dracut module itself) ?

FGrose commented 1 year ago

@LaszloGombos re: /bin/sh not found See PR #2394.

LaszloGombos commented 1 year ago

This is what I would recommend for depends (with not much error handling). Not sure how much error handling is actually needed and how likely that /bin/sh is broken..

Let's try to keep this simple first and just introduce the concept of a new module without adding a bunch of additional use-cases

depends() {                                                                                                                                                                                                  
    shells='dash bash mksh busybox'                                                                                                                                                                          
    for shell in $shells; do                                                                                                                                                                                 
        if dracut_module_included "$shell"; then                                                                                                                                                             
            echo "$shell"                                                                                                                                                                                    
            return 0                                                                                                                                                                                         
        fi                                                                                                                                                                                                   
    done                                                                                                                                                                                                     

    shell=$(realpath -e /bin/sh)                                                                                                                                                                             
    shell=${shell##*/}                                                                                                                                                                                       

    echo "$shell"                                                                                                                                                                                            
    return 0                                                                                                                                                                                                 
} 
stale[bot] commented 1 year ago

This issue is being marked as stale because it has not had any recent activity. It will be closed if no further activity occurs. If this is still an issue in the latest release of Dracut and you would like to keep it open please comment on this issue within the next 7 days. Thank you for your contributions.

Henrik66 commented 1 year ago

@FGrose Do you plan on working on this PR more based on feedback from @LaszloGombos ? I am asking because I am also available try to help with this PR. Thank you @FGrose .

FGrose commented 1 year ago

@FGrose Do you plan on working on this PR more based on feedback from @LaszloGombos ? I am asking because I am also available try to help with this PR. Thank you @FGrose .

@Henrik66 - Yes, I have a refactoring that I am testing now.

LaszloGombos commented 1 year ago

I am very sad to see a completly new complex rewrite instead of iterating over the previous version.

In order for this to ever land, we would need two independent approvals for:

feat(module-postprocess): provide a postprocessing loop for modules

Can you please elaborate why it is not possible to solve this problem without introducing a new module interface ? What would be a use case that would fail without this complex modification in the core logic of dracut.

Do you see any use-case of postprocessing for any other dracut modules ?

@Henrik66 If you still interested I think you should explore https://github.com/dracutdevs/dracut/pull/2368#issuecomment-1588285469. That should at least make it easier to compare and contrast the two solutions.

LaszloGombos commented 1 year ago

@FGrose Perhaps can you make an independent PR just for

refactor(99squash): gather code into the squash module

This PR is simply too big and impacts too many parts and people for it to ever land with our current processes.