Bug 39623 - "make install" re-runs "make" process unnecessarily
Summary: "make install" re-runs "make" process unnecessarily
Status: Pushed to main
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All All
: P5 - low minor
Assignee: David Cook
QA Contact: Martin Renvoize (ashimema)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2025-04-14 07:06 UTC by David Cook
Modified: 2025-05-09 07:14 UTC (History)
7 users (show)

See Also:
GIT URL:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
25.05.00
Circulation function:


Attachments
Bug 39623: Remove "install" target's dependency on "all" target (2.30 KB, patch)
2025-04-14 07:13 UTC, David Cook
Details | Diff | Splinter Review
Bug 39623: Remove "install" target's dependency on "all" target (2.36 KB, patch)
2025-04-14 07:27 UTC, Paul Derscheid
Details | Diff | Splinter Review
Bug 39623: Remove "install" target's dependency on "all" target (2.42 KB, patch)
2025-05-08 12:12 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description David Cook 2025-04-14 07:06:41 UTC
Have you ever noticed that "make install" seems to do all the same things as "make" plus install?

This became even more obvious after bug 32609.

And it got me thinking... surely there has to be a better way and it looks like there is.
Comment 1 David Cook 2025-04-14 07:13:10 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
Comment 2 David Cook 2025-04-14 07:16:44 UTC
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.
Comment 3 Paul Derscheid 2025-04-14 07:27:09 UTC
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>
Comment 4 Paul Derscheid 2025-04-14 07:28:39 UTC
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.
Comment 5 David Cook 2025-04-14 23:29:25 UTC
(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.
Comment 6 David Cook 2025-04-15 06:35:21 UTC
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).
Comment 7 David Cook 2025-04-15 06:41:20 UTC
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.
Comment 8 David Cook 2025-04-15 06:51:13 UTC
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".
Comment 9 Martin Renvoize (ashimema) 2025-05-02 10:47:53 UTC
This seems very sensible to me.. minor concern is making sure our packaging pipeline is unaffected by the changes.
Comment 10 David Cook 2025-05-05 00:39:37 UTC
(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.
Comment 11 David Cook 2025-05-07 00:17:22 UTC
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...
Comment 12 David Cook 2025-05-07 06:46:23 UTC
Going to just move this to In Discussion while we work on bug 39849
Comment 13 David Cook 2025-05-08 03:36:37 UTC
This should still work over top of bug 39849. I'm just going to test that...
Comment 14 David Cook 2025-05-08 05:56:07 UTC
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.
Comment 15 David Cook 2025-05-08 05:57:09 UTC
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
Comment 16 Martin Renvoize (ashimema) 2025-05-08 12:12:16 UTC
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>
Comment 17 Katrin Fischer 2025-05-09 07:14:15 UTC
Pushed for 25.05!

Well done everyone, thank you!