foreman-packaging icon indicating copy to clipboard operation
foreman-packaging copied to clipboard

Fixes #18539 - Own the public/{assets,webpack} directories

Open ekohl opened this issue 2 years ago • 7 comments

By owning the directories we can also ensure they are properly cleaned up on upgrade.

This is now a draft to see if this works. If it does, we should also verify if this is a problem with other plugins and apply the same. Probably with macros.

ekohl avatar Aug 24 '23 14:08 ekohl

Confirmed this works:

Before with 3.11 (because katello nightly doesn't work):

# dnf install https://yum.theforeman.org/releases/3.11/el9/x86_64/foreman-release.rpm https://yum.theforeman.org/katello/4.13/katello/el9/x86_64/katello-repos-latest.rpm
# dnf install rubygem-katello
# dnf remove rubygem-katello
# ls /usr/share/gems/gems/katello*
public

After with nightly and the built RPM:

# dnf install https://yum.theforeman.org/katello/nightly/katello/el9/x86_64/katello-repos-latest.rpm https://yum.theforeman.org/releases/nightly/el9/x86_64/foreman-release.rpm
# dnf install https://download.copr.fedorainfracloud.org/results/@theforeman/katello-nightly-staging-scratch-0fbf0e16-ef37-5063-9658-c3bc6540da26/rhel-9-x86_64/07953152-rubygem-katello/rubygem-katello-4.15.0-0.2.pre.master.el9.noarch.rpm
# dnf remove rubygem-katello
# ls /usr/share/gems/gems/katello*
ls: cannot access '/usr/share/gems/gems/katello*': No such file or directory

ekohl avatar Aug 29 '24 15:08 ekohl

@evgeni before I copy all the plugins, mind taking a look? I don't like that this copies some logic from %foreman_assets_plugin and %foreman_webpack_plugin but other than introducing a new macro I don't know a better way.

ekohl avatar Aug 29 '24 15:08 ekohl

How about extending those macros?

diff --git a/packages/foreman/foreman/foreman.spec b/packages/foreman/foreman/foreman.spec
index ffd425623..7cc62953e 100644
--- a/packages/foreman/foreman/foreman.spec
+++ b/packages/foreman/foreman/foreman.spec
@@ -686,10 +686,15 @@ GEMFILE
 %%%{name}_bundlerd_plugin %%{%{name}_bundlerd_dir}/%%{gem_name}.rb
 %%%{name}_pluginconf_dir %{_sysconfdir}/%{name}/plugins
 # Common assets locations
-%%%{name}_assets_plugin %%{gem_instdir}/public/assets/%%{gem_name}
+%%%{name}_assets_plugin \\
+%%dir %%{gem_instdir}/public \\
+%%dir %%{gem_instdir}/public/assets \\
+%%{gem_instdir}/public/assets/%%{gem_name}
 %%%{name}_assets_foreman %%{foreman_dir}/public/assets/%%{gem_name}
 # Common webpack locations
-%%%{name}_webpack_plugin %%{gem_instdir}/public/webpack/%%{gem_name}
+%%%{name}_webpack_plugin \\
+%%dir %%{gem_instdir}/public/webpack \\
+%%{gem_instdir}/public/webpack/%%{gem_name}
 %%%{name}_webpack_foreman %%{foreman_dir}/public/webpack/%%{gem_name}
 # Common apipie locations
 %%%{name}_apipie_cache_plugin %%{gem_instdir}/public/apipie-cache/plugin/%%{gem_name}

evgeni avatar Aug 30 '24 09:08 evgeni

How about extending those macros?

I also debated introducing a new macro like foreman_plugin_files that takes arguments like foreman_precompile_plugin with which files to include.

Now that I read it, perhaps foreman_precompile_plugin could write out a file that you can pass to %files to include.

ekohl avatar Aug 30 '24 09:08 ekohl

It could, but it also sounds more complicated than extending the existing macros?

evgeni avatar Aug 30 '24 09:08 evgeni

I took a stab at it in https://github.com/theforeman/foreman-packaging/pull/11190 but it really should be easier to build RPMs locally.

ekohl avatar Aug 30 '24 10:08 ekohl

https://github.com/theforeman/foreman-packaging/pull/11233

ekohl avatar Sep 11 '24 17:09 ekohl