Description
David Cook
2025-04-14 07:06:41 UTC
Created attachment 180896 [details] [review] Bug 39623: Remove "install" target's dependency on "all" target This change removes the install target's dependency on "all". There is no clear reason why "install" would need to depend on all other build targets. After you remove this dependency, running "make" and "make install" work the way you'd expect them to work. "make" does the build and "make install" does the install. Test plan: 0. DO NOT APPLY THE PATCH YET! 1. Setup intall target sudo mkdir /opt/koha sudo chown kohadev-koha /opt/koha 2. Clean build env rm -rf blib Makefile /opt/koha/* 3. perl Makefile.PL Installation mode (dev, single, standard) [standard] single Please specify the directory in which to install Koha [/var/lib/koha/kohadev/koha] /opt/koha NOTE: Otherwise accept all defaults 4. make NOTE: build-resources.PL is run and the blib directory is populated 5. make install NOTE: build-resources.PL is run and the blib directory is populated and the built files are installed to /opt/koha 6. Apply the patch now 7. mv /opt/koha /opt/koha.bak 8. Re-setup install target sudo mkdir /opt/koha sudo chown kohadev-koha /opt/koha 9. Clean build env rm -rf blib Makefile /opt/koha/* 10. perl Makefile.PL Installation mode (dev, single, standard) [standard] single Please specify the directory in which to install Koha [/var/lib/koha/kohadev/koha] /opt/koha NOTE: Otherwise accept all defaults 11. make NOTE: build-resources.PL is run and the blib directory is populated 12. make install NOTE: Only the install step is run 13. diff -r /opt/koha /opt/koha.bak 14. Note that there are no differences between the two directories Temporarily marking as "In Discussion", as I think we will want to test the git "dev" install, the "standard" install, "make test" and "make upgrade". -- Of course, as I say that, I've run "make upgrade" and it looks good. Created attachment 180897 [details] [review] Bug 39623: Remove "install" target's dependency on "all" target This change removes the install target's dependency on "all". There is no clear reason why "install" would need to depend on all other build targets. After you remove this dependency, running "make" and "make install" work the way you'd expect them to work. "make" does the build and "make install" does the install. Test plan: 0. DO NOT APPLY THE PATCH YET! 1. Setup intall target sudo mkdir /opt/koha sudo chown kohadev-koha /opt/koha 2. Clean build env rm -rf blib Makefile /opt/koha/* 3. perl Makefile.PL Installation mode (dev, single, standard) [standard] single Please specify the directory in which to install Koha [/var/lib/koha/kohadev/koha] /opt/koha NOTE: Otherwise accept all defaults 4. make NOTE: build-resources.PL is run and the blib directory is populated 5. make install NOTE: build-resources.PL is run and the blib directory is populated and the built files are installed to /opt/koha 6. Apply the patch now 7. mv /opt/koha /opt/koha.bak 8. Re-setup install target sudo mkdir /opt/koha sudo chown kohadev-koha /opt/koha 9. Clean build env rm -rf blib Makefile /opt/koha/* 10. perl Makefile.PL Installation mode (dev, single, standard) [standard] single Please specify the directory in which to install Koha [/var/lib/koha/kohadev/koha] /opt/koha NOTE: Otherwise accept all defaults 11. make NOTE: build-resources.PL is run and the blib directory is populated 12. make install NOTE: Only the install step is run 13. diff -r /opt/koha /opt/koha.bak 14. Note that there are no differences between the two directories Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de> We can keep this 'In Discussion', but I signed off anyway. This would be better if there's no specific reason it was setup as previously. (In reply to Paul Derscheid from comment #4) > We can keep this 'In Discussion', but I signed off anyway. > This would be better if there's no specific reason it was setup as > previously. I ran out of time yesterday, but looking today.... it looks like commit 5befdd2cd37 added the "install" target with the dependency on "all". It looks like Galen wrote the commit but it was in 2007 so I would doubt he'd remember now why he added the "all" dependency, and it was pre-Bugzilla, so there's no bug report for it. I do think the old bug register was imported into Bugzilla, but I can't find a bug report to link to that commit. I've actually adopted this into my production workflow, and it certainly looks like it works for the "standard" install used by the Debian package build process (or at least the Debian package build process I use which isn't identical to what the community uses). Test plan for dev install: 0. Apply the patch 1. Setup install target sudo mkdir /opt/koha-dev sudo chown kohadev-koha /opt/koha-dev 2. Clean build env rm -rf koha-tmpl/intranet-tmpl/prog/js/vue/dist/* blib Makefile 3. perl Makefile.PL Installation mode (dev, single, standard) [standard] dev Configuration directory: [/var/lib/koha/kohadev/koha-dev] /opt/koha-dev NOTE: Otherwise accept all defaults 4. make 5. make install It all looks good to me. The output of "make test" is a bit scary but it seems to be the same for "main" and this patch. (Not that I'm letting the whole thing run.) I think that we can move this to "Signed Off". This seems very sensible to me.. minor concern is making sure our packaging pipeline is unaffected by the changes. (In reply to Martin Renvoize (ashimema) from comment #9) > This seems very sensible to me.. minor concern is making sure our packaging > pipeline is unaffected by the changes. In theory, it should just automagically work, but I suppose that depends on if the packaging pipeline does anything unexpected/abnormal along the way. Something that needs double-checking: the population of blib/API_CGI_DIR/v1/swagger/swagger_bundle.json from api/v1/swagger/swagger_bundle.json Philippe has raised an issue about "main" that I'm looking into right now... Going to just move this to In Discussion while we work on bug 39849 This should still work over top of bug 39849. I'm just going to test that... Moving this back to signed off, as I tested it over top of bug 39849 and it works perfectly. I am going to copy over the test plan from 39849 though, since it's better than the one here. To test: 1a. Run 'git archive --format=tar HEAD > "koha.orig.tar"' 1b. sudo mkdir /opt/koha /opt/build_koha 1c. sudo chown kohadev-koha /opt/koha /opt/build_koha 1d. mv koha.orig.tar /opt/build_koha/. 1e. cd /opt/build_koha 1f. tar xvf koha.orig.tar 2. Run "perl Makefile.PL" Choose "single" Choose "/opt/koha" for target directory Choose the defaults for the rest 3. Run "make" 4. Note the first step is the CSS/JS build 5. Note the second step is the copying of files to "blib" 6. Run "find -name 'erm.js.map'" and note it appears in the regular build location as well as in "blib" 7. Run "find -name 'swagger_bundle.json'" and note it appears in the regular build location as well as in "blib" 8a. Run "make install" 8b. Note that the output only shows Installation. It doesn't rebuild Koha like it does before applying the patch. 9. Run the above "find" commands using "/opt/koha" and note that the files appear in the installed directory Created attachment 182072 [details] [review] Bug 39623: Remove "install" target's dependency on "all" target This change removes the install target's dependency on "all". There is no clear reason why "install" would need to depend on all other build targets. After you remove this dependency, running "make" and "make install" work the way you'd expect them to work. "make" does the build and "make install" does the install. Test plan: 0. DO NOT APPLY THE PATCH YET! 1. Setup intall target sudo mkdir /opt/koha sudo chown kohadev-koha /opt/koha 2. Clean build env rm -rf blib Makefile /opt/koha/* 3. perl Makefile.PL Installation mode (dev, single, standard) [standard] single Please specify the directory in which to install Koha [/var/lib/koha/kohadev/koha] /opt/koha NOTE: Otherwise accept all defaults 4. make NOTE: build-resources.PL is run and the blib directory is populated 5. make install NOTE: build-resources.PL is run and the blib directory is populated and the built files are installed to /opt/koha 6. Apply the patch now 7. mv /opt/koha /opt/koha.bak 8. Re-setup install target sudo mkdir /opt/koha sudo chown kohadev-koha /opt/koha 9. Clean build env rm -rf blib Makefile /opt/koha/* 10. perl Makefile.PL Installation mode (dev, single, standard) [standard] single Please specify the directory in which to install Koha [/var/lib/koha/kohadev/koha] /opt/koha NOTE: Otherwise accept all defaults 11. make NOTE: build-resources.PL is run and the blib directory is populated 12. make install NOTE: Only the install step is run 13. diff -r /opt/koha /opt/koha.bak 14. Note that there are no differences between the two directories Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de> Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> Pushed for 25.05! Well done everyone, thank you! |