Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
fiftydinar opened this issue May 17, 2024 · 6 comments · Fixed by #263
Labels
priority: medium Needs to be done soon type: fix Iterations on existing features or infrastructure.

Comments

@fiftydinar
Copy link
Collaborator

fiftydinar commented May 17, 2024

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.

@fiftydinar fiftydinar added type: fix Iterations on existing features or infrastructure. priority: medium Needs to be done soon labels May 17, 2024
@fiftydinar fiftydinar changed the title fix(default-flatpaks): Flatpak IDs input in the last recipe overwrittes the 1st one fix(default-flatpaks): Flatpak IDs input in the last recipe overwrite the 1st one May 17, 2024
@fiftydinar fiftydinar changed the title fix(default-flatpaks): Flatpak IDs input in the last recipe overwrite the 1st one fix(default-flatpaks): Flatpak IDs input in the last module definition overwrite the 1st one May 17, 2024
@fiftydinar fiftydinar changed the title fix(default-flatpaks): Flatpak IDs input in the last module definition overwrite the 1st one fix(default-flatpaks): Flatpak IDs input in the last module definition overwrites the 1st one May 17, 2024
@xynydev
Copy link
Member

xynydev commented May 17, 2024

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
Copy link
Contributor

lorduskordus commented Jun 9, 2024

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
Copy link
Member

xynydev commented Jun 19, 2024

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
Copy link
Contributor

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

  • the system/install list created at build time is correct (when using the module multiple times, new stuff is added to the list)
  • overriding the configuration while already booted by creating system/install & system/remove list in /etc/bluebuild/default-flatpaks and restarting the system-flatpak-setup.service manually works too.

@xynydev
Copy link
Member

xynydev commented Jun 19, 2024

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

@fiftydinar
Copy link
Collaborator Author

fiftydinar commented Jun 19, 2024

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:
#263

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: medium Needs to be done soon type: fix Iterations on existing features or infrastructure.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants