Bug 269884

Summary: [nanoBSD] cust_pkgng() fetches ports-mgmt/pkg from remote regardless of required package
Product: Base System Reporter: embhd
Component: miscAssignee: Jose Luis Duran <jlduran>
Status: In Progress ---    
Severity: Affects Only Me CC: jlduran
Priority: ---    
Version: 14.0-RELEASE   
Hardware: Any   
OS: Any   
Attachments:
Description Flags
Patch
none
Patch
none
let host's ports-mgmt/pkg do all the work none

Description embhd 2023-02-28 20:44:07 UTC
Created attachment 240489 [details]
Patch

Somewhat related to https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=260467: Since /usr/sbin/pkg cannot add ports-mgmt/pkg via directory path as required by cust_pkgng() in /usr/src/tools/tools/nanobsd/defaults.sh, it defaults to bootstrapping from remote.

The proposed solution is to use the build system's ports-mgmt/pkg, which chroots to ${NANO_WORLDDIR}.
Comment 1 embhd 2023-09-05 09:57:11 UTC
Created attachment 244655 [details]
Patch
Comment 2 embhd 2023-09-05 10:01:04 UTC
Comment on attachment 244655 [details]
Patch

Also, allow the NANO_PACKAGE_DIR to be linked ("find -L") in /usr/src/tools/tools/nanobsd.
Comment 3 embhd 2024-02-24 07:37:38 UTC
(In reply to embhd from comment #0)
Since the linked bug seems to have been fixed: I would still propose the attached patch, since - depending on the target vs the build platform - the bootstrapped and then installed ports-mgmt/pkg does not necessarily run on the target system after all.
Comment 4 Jose Luis Duran freebsd_committer freebsd_triage 2024-11-12 20:09:07 UTC
Thank you for the bug reports! Please, keep them coming.

If I understand correctly, this can be divided in three issues:

1. cust_pkgng() always fetches ports-mgmt/pkg.
2. NANO_PACKAGEDIR cannot be a symbolic link.
3. If TARGET is not the same machine/architecture as the host, because of item (1), it will fail.

Let's start with the easy one, number (2):

The way I see it, after 9af130ae8c03, cust_pkgng() uses a nullfs mount to share the contents of NANO_PACKAGEDIR inside the chroot (NANO_WORLDDIR). It is more robust, indeed, but it has the drawback that if NANO_PACKAGEDIR is a symlink, it will fail.
My proposed fix is simply to use the realpath of NANO_PACKAGEDIR.

I will also update fill_pkg.sh accordingly.

Number (1) and (3) are closely related. From what I can read in the guide "Introduction to NanoBSD", you are supposed to already take care of the differences in architectures, etc. by providing a suitable Pkg directory, but maybe it is not explicit.
I do have however experimenting cross-building, and may have a small addition to the scripts, but this is outside the scope of this PR.

The code reviews take place on GitHub or Phabricator, so I will post my proposed fixes there. If you do not have a Phabricator account, please post your suggestions here, and I will forward them accordingly. The project is aware of this annoyance, and I apologize for the inconvenience.
Comment 6 embhd 2024-11-13 18:12:06 UTC
Created attachment 255143 [details]
let host's ports-mgmt/pkg do all the work

(In reply to Jose Luis Duran from comment #4)

(1): I would need to check, but since bug #260467 seems to have been fixed with commit c96b4d87ba3336e6d4bea04c49d6abde158b7aae, I don't think that's the case anymore - maybe I should have changed the title.. I'm sorry if that has led to some confusion.

(2): I'm not sure, if it is a problem with ${NANO_PACKAGE_DIR} being a symbolic link, since I just had to add "-L" for `find`ing the package for pkg(8)... I just recreated my NanoBSD-image and it worked - although my currently configured ${NANO_PACKAGE_DIR} is a subdirectory beneath a linked directory, so this may have something to do with it.

(3): Yes and no: As far as I understand it, during image creation, both CR() and CR0() chroot into ${NANO_WORLDDIR} and try to run the previously installed version of pkg(8) (which should have been compiled for the target architecture) to install any additional packages (see variable ${PKGCMD}) in the soon-to-be image. This happens on the host system - and should fail, if the host is not able to run code compiled for the target. So I suppose, the problem should not be limited to fetching ports-mgmt/pkg, but be a general one, shouldn't it?

I just rebuild my image - using host's ports-mgmt/pkg to chroot into ${NANO_WORLDDIR} and install everything even allowed me to reverse the "-L" at the aforementiond `find`, you'll find an updated patch attached.
Please note, that I also deleted the functions CR() and CR0(), since there was just a single call left - there might be reasons to keep them, that I wasn't aware of.

I don't have a Phabricator account, so I'd be glad, if you would be willing to forward my suggestion.
Comment 7 Jose Luis Duran freebsd_committer freebsd_triage 2024-11-13 21:51:13 UTC
If we pretend we don't care about having unused functions around, the change can be summarized as:

From:

    chroot "${NANO_WORLDDIR}" /bin/sh -exc /usr/sbin/pkg ...

To:

    /usr/local/sbin/pkg -c "${NANO_WORLDDIR}" ...

That is, instead of chroot'ing and executing pkg(7) from a shell inside the chroot, to executing pkg(8) straight from the host into the chroot?

I'll test it. Thank you!
Comment 8 Jose Luis Duran freebsd_committer freebsd_triage 2024-11-14 00:07:10 UTC
Applying the patches mentioned in comment #5, I was able to create an image without modifying the actual script.

Would you mind sharing the failing bits of your _.cust.cust_pkgng log file?

Also, if you don't mind, could you confirm that the pkg-* file is present and it is not a symbolic link:

    # ls -alh /usr/src/tools/tools/nanobsd/Pkg/pkg-*
    -rw-r--r--  1 root wheel   11M Nov 12 02:54 /usr/src/tools/tools/nanobsd/Pkg/pkg-1.21.3.pkg
Comment 9 embhd 2024-11-14 17:28:34 UTC
(In reply to Jose Luis Duran from comment #7)

Basically, yes. It's mainly about changing ${PKGCMD} once and removing the then unnecessary call to CR() at each instance. Plus a typo in line 501 ("matalog").
Comment 10 embhd 2024-11-14 17:35:30 UTC
(In reply to Jose Luis Duran from comment #8)

Failing bits in which regard? The host vs target architecture? I came to that conclusion, after I figured that the failing of the overall image creation process without an active internet connection was due to the bootstrapping of pkg(7).

Sure (my NANO_PACKAGE_DIR is defined as "${PWD}/${NANO_NAME}/PACKAGES/All"):
# ls -alh /usr/src/tools/tools/nanobsd/donald/PACKAGES/All/pkg-1.21.3.pkg
-rw-r--r--  1 root wheel  6.0M Jun  9 13:47 /usr/src/tools/tools/nanobsd/donald/PACKAGES/All/pkg-1.21.3.pkg
Comment 11 Jose Luis Duran freebsd_committer freebsd_triage 2024-11-14 19:02:48 UTC
(In reply to embhd from comment #10)
Thank you for the information.

I have tested building a NanoBSD image on a fully air-gapped system, and it worked well with the current CR() and CR0() subroutines.
I do not want to remove them, because they are working fine, and we might use them in the future, extending them for cross-building.

I'll commit the fixes, with some of the fixes proposed here, but leave this PR open, we can always revisit later, once we all are testing with the same code base.

I appreciate that you have taken the time to test and report it!
Comment 12 embhd 2024-11-15 19:24:33 UTC
(In reply to Jose Luis Duran from comment #11)

Sure, glad to help.
Comment 13 commit-hook freebsd_committer freebsd_triage 2024-11-16 07:24:44 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=4223c668e4b9ba71e2c6cfabbd66795729b7ff8b

commit 4223c668e4b9ba71e2c6cfabbd66795729b7ff8b
Author:     Jose Luis Duran <jlduran@FreeBSD.org>
AuthorDate: 2024-11-12 20:17:14 +0000
Commit:     Jose Luis Duran <jlduran@FreeBSD.org>
CommitDate: 2024-11-16 07:06:14 +0000

    nanobsd: Use the real path for NANO_PACKAGE_DIR

    As users may have the Pkg directory as a symbolic link to the NanoBSD
    "package dump directory".  In commit 9af130ae8c03, cust_pkgng() was
    greatly improved, however as a side effect of using a nullfs mount, the
    directories and files must not be symlinks.

    Fix it by using NANO_PACKAGE_DIR realpath().

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

 tools/tools/nanobsd/defaults.sh | 1 +
 1 file changed, 1 insertion(+)
Comment 14 commit-hook freebsd_committer freebsd_triage 2024-11-16 07:24:45 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=4db04f5e3a9bd927ba1173bf8d3b6a70178eab5f

commit 4db04f5e3a9bd927ba1173bf8d3b6a70178eab5f
Author:     Jose Luis Duran <jlduran@FreeBSD.org>
AuthorDate: 2024-11-12 07:33:12 +0000
Commit:     Jose Luis Duran <jlduran@FreeBSD.org>
CommitDate: 2024-11-16 07:01:27 +0000

    nanobsd: Update fill_pkg.sh

    fill_pkg.sh is a script that links a package and its dependencies from a
    "package dump" directory (like /usr/ports/packages/All) to a specified
    directory (NANO_PACKAGE_DIR), for cust_pkgng()[*].

    Update the script by:

    - Using `make package-name` instead of `make -V pkgname`
    - Looking for package files with *.pkg instead of *.txz
    - Adding a -c option that copies the files instead of linking them[*]

    [*] After 9af130ae8c03 cust_pkgng() cannot be used with a directory
    populated by fill_pkg.sh, because it uses a nullfs mount, which doesn't
    follow symlinks, therefore the links inside NANO_PACKAGE_DIR will not
    work.

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

 tools/tools/nanobsd/fill_pkg.sh | 32 +++++++++++++++++---------------
 1 file changed, 17 insertions(+), 15 deletions(-)
Comment 15 commit-hook freebsd_committer freebsd_triage 2024-11-16 07:24:46 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=12cbad923dbcccb3d6f71a77ad3241508f186048

commit 12cbad923dbcccb3d6f71a77ad3241508f186048
Author:     Jose Luis Duran <jlduran@FreeBSD.org>
AuthorDate: 2024-11-15 04:54:09 +0000
Commit:     Jose Luis Duran <jlduran@FreeBSD.org>
CommitDate: 2024-11-16 07:11:16 +0000

    nanobsd: Fix typos

    Fix a number of typos in the code or comments.
    Files under dhcpd, embedded, pcengines, and rescue were intentionally
    omitted. These directories will be reviewed at a later date.

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

 tools/tools/nanobsd/Files/root/save_cfg |  2 +-
 tools/tools/nanobsd/defaults.sh         | 10 +++++-----
 tools/tools/nanobsd/fill_pkg.sh         |  2 +-
 tools/tools/nanobsd/mtree-dedup.awk     |  2 +-
 4 files changed, 8 insertions(+), 8 deletions(-)