blue-build / modules

BlueBuild standard modules used for building your Atomic Images
Apache License 2.0
21 stars 27 forks source link

fix(default-flatpaks): Flatpak IDs input in the last module definition overwrites the 1st one #231

Closed fiftydinar closed 1 week ago

fiftydinar commented 1 month ago

Some users faced the issue when trying to include shared flatpaks for all of their images in multi-image setup.

The issue is that the last module definition in the recipe overwrites the 1st one, so only input from the last module definition is taking the place.

That should not happen. Every flatpak ID input from other module definitions should be added to the list, instead of overwriting it.

Here's the example:

Multi image example:

recipe.yml:

modules:
  - type: rpm-ostree
    install:
      - gnome-console
    remove:
      - gnome-classic-session
      - gnome-classic-session-xsession
      - gnome-terminal
      - gnome-terminal-nautilus
      - gnome-tweaks
      - qadwaitadecorations-qt5

  - type: default-flatpaks
    system:
      install:
        - org.gnome.baobab
        - org.gnome.Calculator
        - org.gnome.Calendar
        - org.gnome.Characters
        - org.gnome.clocks
        - org.gnome.Connections
        - org.gnome.Contacts
        - org.gnome.Evince
        - org.gnome.font-viewer
        - org.gnome.Logs
        - org.gnome.Loupe
        - org.gnome.Maps
        - org.gnome.NautilusPreviewer
        - org.gnome.SimpleScan
        - org.gnome.Snapshot
        - org.gnome.TextEditor
        - org.gnome.Weather

   - type: akmods
     install:
       - vl42loopback

shared.yml:

   - type: default-flatpaks
     system:
       install:
         - org.mozilla.firefox
         - id.example.application

Depending on the recipe order, either of these 2 scenarios happen:

  1. recipe.yml flatpaks are installing, but shared.yml flatpaks are not
  2. shared.yml flatpaks are installing, but recipe.yml flatpaks are not

single image recipe.yml example:

modules:
   - type: default-flatpaks
     system:
       install:
         - org.mozilla.firefox          # those 2 flatpaks basically become overwritten
         - id.example.application       # by the last module definition

  - type: rpm-ostree
    install:
      - gnome-console
    remove:
      - gnome-classic-session
      - gnome-classic-session-xsession
      - gnome-terminal
      - gnome-terminal-nautilus
      - gnome-tweaks
      - qadwaitadecorations-qt5

  - type: default-flatpaks
    system:
      install:
        - org.gnome.baobab
        - org.gnome.Calculator
        - org.gnome.Calendar
        - org.gnome.Characters
        - org.gnome.clocks
        - org.gnome.Connections
        - org.gnome.Contacts
        - org.gnome.Evince
        - org.gnome.font-viewer
        - org.gnome.Logs
        - org.gnome.Loupe
        - org.gnome.Maps
        - org.gnome.NautilusPreviewer
        - org.gnome.SimpleScan
        - org.gnome.Snapshot
        - org.gnome.TextEditor
        - org.gnome.Weather

This module needs to be refactored, so this issue should be taken in mind when doing that.

xynydev commented 1 month ago

I'd file this under #146. I'll post a couple notes there and then close this issue. IMO the refactoring wouldn't be that big of a job so it doesn't make sense to fix the current iteration with duct tape.

lorduskordus commented 3 weeks ago

I think the problem is in default-flatpaks.sh at lines 112-113 and 120-121.

cp -r overwrites the destination file, so with every run of the module, it deletes previous configurations.

Using something like this instead should work:

if [ ! -f "/usr/share/bluebuild/default-flatpaks/system/install" ]; then
    cp -r "$MODULE_DIRECTORY"/default-flatpaks/config/system/install /usr/share/bluebuild/default-flatpaks/system/install
fi

Edit: It does work on my image. So if you reconsider pushing a duct tape solution before refactoring the whole module, then this might be it.

xynydev commented 1 week ago

I believe this change would make the first configuration always take precedence over the later ones, right @lorduskordus ?

This would not improve the situation, as for example overriding the default-flatpaks configuration of the base image would become impossible.

lorduskordus commented 1 week ago

I'm not sure if I understood, but from my testing:

xynydev commented 1 week ago

Ah, maybe I misunderstood this. I'll ping @fiftydinar for good measure.

fiftydinar commented 1 week ago

Ah, maybe I misunderstood this. I'll ping @fiftydinar for good measure.

He's right. Good catch.

I saw his reply earlier but forgot about it.

I made a PR: https://github.com/blue-build/modules/pull/263