I've been thinking how the "rm -rf koha-tmpl/intranet-tmpl/prog/js/vue/dist/*" step discussed in "Bug 37911 - Prototype vue islands within static pages" for a build from a git working directory isn't intuitive or easy to know about. I've also heard of stories where people building from git repos are still getting files included even though they've run "rm -rf koha-tmpl/intranet-tmpl/prog/js/vue/dist/*". This should be impossible, so there's probably something else going on there, but I think I have a solution which removes the necessity for this step anyway. (Note that for Debian package installs and regular tarball installs, the step isn't needed at all, because the tarballs are generating using "git archive" which doesn't include "vue/dist" files.) Anyway, the solution is to tell Makefile.PL specifically not to include "koha-tmpl/intranet-tmpl/prog/js/vue/dist/*" in the "$file_map" data structure. That way, it should be impossible for these files to wind up in the "blib" except where we copy them in via the "move_compiled_js" target from bug 39849
Created attachment 186271 [details] [review] Bug 40773: Do not map "vue/dist" files in Makefile This change ensures that "vue/dist" files are not mapped into the pm_to_blib target of the Makefile, so that they are only handled by the explicit compile steps in the top targets of the Makefile. Test plan: 0. Apply the patch 1. perl build-resources.PL 2. ls ./koha-tmpl/intranet-tmpl/prog/js/vue/dist 3. Note that files are present 4. perl Makefile.PL 5. Take all the defaults 6. vi Makefile 7. Search for vue\/dist 8. Note that the only hit is for "cp -r koha-tmpl/intranet-tmpl/prog/js/vue/dist blib/INTRANET_TMPL_DIR/prog/js/vue/"
Note that anything in "koha-tmpl/intranet-tmpl/prog/js/vue/dist/" will be copied across into the "blib". I wonder if rspack.config.js or something else should have a step that ensures that "koha-tmpl/intranet-tmpl/prog/js/vue/dist/" is empty before doing a build? I suppose build-resources.PL could also do that. Anyway, I'd say that's a bit of a side issue really.
Created attachment 186279 [details] [review] Bug 40773: Do not map "vue/dist" files in Makefile This change ensures that "vue/dist" files are not mapped into the pm_to_blib target of the Makefile, so that they are only handled by the explicit compile steps in the top targets of the Makefile. Test plan: 0. Apply the patch 1. perl build-resources.PL 2. ls ./koha-tmpl/intranet-tmpl/prog/js/vue/dist 3. Note that files are present 4. perl Makefile.PL 5. Take all the defaults 6. vi Makefile 7. Search for vue\/dist 8. Note that the only hit is for "cp -r koha-tmpl/intranet-tmpl/prog/js/vue/dist blib/INTRANET_TMPL_DIR/prog/js/vue/" Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Hi David, great change. Thanks.
Btw, there's a webpack/rspack option: output: { ... clean: true, } that does exactly that. But we should probably work on the structure beforehand. This should be configured for prod, not for dev where you might want to keep old assets. Meaning should be a follow-up, out of scope for this bug.
For reference: https://rspack.rs/config/output#outputclean
Created attachment 186281 [details] [review] Bug 40773: Do not map "vue/dist" files in Makefile This change ensures that "vue/dist" files are not mapped into the pm_to_blib target of the Makefile, so that they are only handled by the explicit compile steps in the top targets of the Makefile. Test plan: 0. Apply the patch 1. perl build-resources.PL 2. ls ./koha-tmpl/intranet-tmpl/prog/js/vue/dist 3. Note that files are present 4. perl Makefile.PL 5. Take all the defaults 6. vi Makefile 7. Search for vue\/dist 8. Note that the only hit is for "cp -r koha-tmpl/intranet-tmpl/prog/js/vue/dist blib/INTRANET_TMPL_DIR/prog/js/vue/" Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
(In reply to Paul Derscheid from comment #5) > Btw, there's a webpack/rspack option: > output: { > ... > clean: true, > } > that does exactly that. > > But we should probably work on the structure beforehand. > This should be configured for prod, not for dev where you might want to keep > old assets. Meaning should be a follow-up, out of scope for this bug. I had a feeling that you'd know what to do!