Bug 277269 - [nanoBSD] setting NANO_PMAKE in defaults.sh renders setting NANO_NCPU in config useless
Summary: [nanoBSD] setting NANO_PMAKE in defaults.sh renders setting NANO_NCPU in conf...
Status: In Progress
Alias: None
Product: Base System
Classification: Unclassified
Component: misc (show other bugs)
Version: 14.0-RELEASE
Hardware: Any Any
: --- Affects Only Me
Assignee: Jose Luis Duran
URL: https://reviews.freebsd.org/D47476
Keywords:
Depends on:
Blocks:
 
Reported: 2024-02-23 21:12 UTC by embhd
Modified: 2024-11-16 07:24 UTC (History)
1 user (show)

See Also:
jlduran: mfc-stable14?
jlduran: mfc-stable13-


Attachments
Patch (2.69 KB, patch)
2024-02-23 21:12 UTC, embhd
no flags Details | Diff
nanobsd: Add missing options to usage() (1.45 KB, patch)
2024-11-07 14:53 UTC, Jose Luis Duran
no flags Details | Diff
nanobsd: Fix parallel make (1.29 KB, patch)
2024-11-07 14:54 UTC, Jose Luis Duran
no flags Details | Diff
nanobsd: Fix parallel make (1.05 KB, patch)
2024-11-07 17:42 UTC, embhd
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description embhd 2024-02-23 21:12:53 UTC
Created attachment 248702 [details]
Patch

As of now, the default value of the variable NANO_PMAKE ("Parallel MAKE") as set in the script `default.sh` contains the default value of NANO_NCPU ("Number of CPUs used").
Thus, if NANO_NCPU is set to a different value in the respective config file, this won't change anything for the actual build, since the value of NANO_PMAKE is not automatically updated to the configured number of cores - rendering the variable more or less useless.

The proposed solution (c.f. the patch attached) is to eliminate the variable NANO_PMAKE and use the "original" variables NANO_MAKE and NANO_NCPU directly, since it only affects two places in `defaults.sh´.

In addition, the patch adds the missing available options to the output of `nanobsd.sh -h`.
Comment 1 Jose Luis Duran freebsd_committer freebsd_triage 2024-11-07 14:52:29 UTC
Good catch!
Let's divide this PR in two parts:

1. Fix the usage string.
2. Fix parallel make.

Attached are the two proposed patches.

The reason I want to avoid your approach for No. 2, if possible, is that NANO_PMAKE is older than NANO_NCPU.
Nobody knows what NANO_PMAKE string other users might have, and your proposed change could "break" their build system.

If you don't mind testing, I would highly appreciate it.

Thank you for reporting it!
Comment 2 Jose Luis Duran freebsd_committer freebsd_triage 2024-11-07 14:53:27 UTC
Created attachment 255003 [details]
nanobsd: Add missing options to usage()

-B suppress installs (both kernel and world)
-I build disk image from existing build/install
-W suppress installworld
Comment 3 Jose Luis Duran freebsd_committer freebsd_triage 2024-11-07 14:54:18 UTC
Created attachment 255004 [details]
nanobsd: Fix parallel make

The NANO_NCPU (number of CPUs) value gets ignored by the build script
when setting the NANO_PMAKE (parallel make) value.

Fix it by setting the NANO_PMAKE later in the process.
Comment 4 embhd 2024-11-07 17:42:41 UTC
Created attachment 255010 [details]
nanobsd: Fix parallel make
Comment 5 embhd 2024-11-07 17:44:33 UTC
(In reply to Jose Luis Duran from comment #3)

Looks good to me, I would just like to propose to add the line setting NANO_PMAKE in the same way NANO_LOG, etc., is set. For overview's sake, if you will.
Comment 6 embhd 2024-11-07 17:48:33 UTC
(In reply to Jose Luis Duran from comment #2)

Nothing to add here, looks good!
Comment 8 commit-hook freebsd_committer freebsd_triage 2024-11-16 07:24:47 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=3c5d19a40de7273bb478163639dd8532af425595

commit 3c5d19a40de7273bb478163639dd8532af425595
Author:     Jose Luis Duran <jlduran@FreeBSD.org>
AuthorDate: 2024-11-07 14:32:01 +0000
Commit:     Jose Luis Duran <jlduran@FreeBSD.org>
CommitDate: 2024-11-16 06:55:25 +0000

    nanobsd: Fix parallel make

    The NANO_NCPU (number of CPUs) value gets ignored by the build script
    when setting the NANO_PMAKE (parallel make) value.

    Fix it by setting the NANO_PMAKE later in the process.

    PR:             277269
    Reviewed by:    imp, emaste
    Approved by:    emaste (mentor)
    MFC after:      1 month
    Differential Revision:  https://reviews.freebsd.org/D47476

 tools/tools/nanobsd/defaults.sh | 5 ++++-
 tools/tools/nanobsd/nanobsd.sh  | 4 ----
 2 files changed, 4 insertions(+), 5 deletions(-)
Comment 9 commit-hook freebsd_committer freebsd_triage 2024-11-16 07:24:48 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=999f288a0eeb230d3655da94c1186ca03c0cc404

commit 999f288a0eeb230d3655da94c1186ca03c0cc404
Author:     Jose Luis Duran <jlduran@FreeBSD.org>
AuthorDate: 2024-11-07 14:16:50 +0000
Commit:     Jose Luis Duran <jlduran@FreeBSD.org>
CommitDate: 2024-11-16 06:54:58 +0000

    nanobsd: Add missing options to usage()

    -B suppress installs (both kernel and world)
    -I build disk image from existing build/install
    -W suppress installworld

    PR:             277269
    Reviewed by:    imp, emaste
    Approved by:    emaste (mentor)
    MFC after:      1 month
    Differential Revision:  https://reviews.freebsd.org/D47475

 tools/tools/nanobsd/defaults.sh | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)