From: Marc Espie <marc.espie.openbsd@gmail.com>
Subject: Re: Add BULK_COOKIES_DIR to proot(1)
To: Denis Bodor <dbodor@lefinnois.net>
Cc: ports <ports@openbsd.org>
Date: Sat, 3 Aug 2024 12:41:45 +0200

On Sat, Aug 03, 2024 at 11:53:28AM +0200, Denis Bodor wrote:
> Hi,
> 
> I ran into a permission problem when using proot(1). The ${PORTSDIR}/bulk
> directory cannot be created when you decide to place the other directories
> outside the ports tree.
> 
> Here's my configuration:
> 
> $ cat proot.cfg
> chroot=/prootports2
> PORT_USER=denis
> extra=/etc/doas.conf
>       /etc/installurl
> WRKOBJDIR=/bportbuild/pobj
> PLIST_REPOSITORY=/bportbuild/plist
> DISTDIR=/bportbuild/distfiles
> PACKAGE_REPOSITORY=/bportbuild/packages
> LOGDIR=/bportbuild/logs
> actions=unpopulate_light
>         copy_ports
> mkconf_tail=proot_mkadd.cfg
> 
> $ cat proot_mkadd.cfg
> PARALLEL_MAKE_JOBS=yes
> MAKE_JOBS=10
> #BULK=No
> 
> If the BULK line is commented out as here, "BULK=Auto" is used and the
> permission problem is encountered:
> 
> $ doas /usr/ports/infrastructure/bin/proot -c proot.cfg
> $ doas chroot -u denis /prootports2
> 
> $ cd /usr/port/sysutils/flashrom
> $ make fake
> [...]
> ===>  Building package for dwz-0.15
> Create /bportbuild/packages/amd64/all/dwz-0.15.tgz
> Creating package dwz-0.15
> Creating package debug-dwz-0.15
> Link to /bportbuild/packages/amd64/ftp/dwz-0.15.tgz
> Link to /bportbuild/packages/amd64/ftp/debug-dwz-0.15.tgz
> mkdir: /usr/ports/bulk: Permission denied
> 
> Of course, with "BULK=No", everything works straight away.
> 
> I've modified proot(1) to include BULK_COOKIES_DIR in the configuration
> file in the same way as DISTDIR, WRKOBJDIR, PLIST_REPOSITORY, and so on.
> 
> A patch for proot(1) is attached here. Not sure this is the right approach.
> 
> -- 
> Denis

> Index: infrastructure/bin/proot
> ===================================================================
> RCS file: /cvs/ports/infrastructure/bin/proot,v
> diff -u -p -u -r1.74 proot
> --- infrastructure/bin/proot	6 May 2023 05:21:15 -0000	1.74
> +++ infrastructure/bin/proot	3 Aug 2024 09:05:53 -0000
> @@ -151,12 +151,16 @@ sub do_parm($state, $k, $v)
>  		    $state->{PACKAGE_REPOSITORY} = File::Spec->canonpath($v);
>  	    }, PLIST_REPOSITORY => sub() {
>  		    $state->{PLIST_REPOSITORY} = File::Spec->canonpath($v);
> +	    }, BULK_COOKIES_DIR => sub() {
> +		    $state->{BULK_COOKIES_DIR} = File::Spec->canonpath($v);
>  	    }, NFSDIR => sub() {
>  		    $state->{DISTDIR} = File::Spec->canonpath("$v/distfiles");
>  		    $state->{PACKAGE_REPOSITORY} = 
>  			File::Spec->canonpath("$v/packages");
>  		    $state->{PLIST_REPOSITORY} =
>  		    	File::Spec->canonpath("$v/plist");
> +		    $state->{BULK_COOKIES_DIR} =
> +			File::Spec->canonpath("$v/bulk");
>  	    }, LOCALDIR => sub() {
>  		    $state->{WRKOBJDIR} = File::Spec->canonpath("$v/pobj");
>  		    $state->{LOCKDIR} = File::Spec->canonpath("$v/locks");
> @@ -325,7 +329,7 @@ sub handle_options($state)
>  	if ($< != 0) {
>  		$state->fatal("Must be root");
>  	}
> -	for my $i (qw(PORTSDIR DISTDIR WRKOBJDIR PACKAGE_REPOSITORY PLIST_REPOSITORY LOCKDIR LOGDIR FETCH_USER BUILD_USER)) {
> +	for my $i (qw(PORTSDIR DISTDIR WRKOBJDIR PACKAGE_REPOSITORY PLIST_REPOSITORY BULK_COOKIES_DIR LOCKDIR LOGDIR FETCH_USER BUILD_USER)) {
>  		if (defined $state->{$i}) {
>  			$state->{write}{$i} = 1;
>  		}
> @@ -340,8 +344,9 @@ sub handle_options($state)
>  	$state->{loguser} //= $state->{builduser};
>  	$state->{PACKAGE_REPOSITORY} //= join('/', $state->{PORTSDIR}, 'packages');
>  	$state->{PLIST_REPOSITORY} //= join('/', $state->{PORTSDIR}, 'plist');
> +	$state->{BULK_COOKIES_DIR} //= join('/', $state->{PORTSDIR}, 'bulk');
>  	$state->{sysdir} //= '/usr/src/sys';
> -	for my $dir (qw(DISTDIR WRKOBJDIR LOGDIR PACKAGE_REPOSITORY PLIST_REPOSITORY LOCKDIR)) {
> +	for my $dir (qw(DISTDIR WRKOBJDIR LOGDIR PACKAGE_REPOSITORY PLIST_REPOSITORY BULK_COOKIES_DIR LOCKDIR)) {
>  		$state->{$dir} = $state->canonical_dir($state->{$dir});
>  		$state->add_preserved($state->{$dir});
>  	}
> @@ -357,7 +362,7 @@ sub handle_options($state)
>  	}
>  	for my $i (qw(chroot srcroot 
>  	    PORTSDIR DISTDIR WRKOBJDIR LOCKDIR LOGDIR 
> -	    PACKAGE_REPOSITORY PLIST_REPOSITORY)) {
> +	    PACKAGE_REPOSITORY PLIST_REPOSITORY BULK_COOKIES_DIR)) {
>  		if (defined $state->{$i}) {
>  			$state->say("#1=#2", $i, $state->{$i});
>  		}
> @@ -689,6 +694,9 @@ sub best_user($state, $path)
>  	if (m/^\Q$state->{PLIST_REPOSITORY}\E/) {
>  		return $state->{builduser};
>  	}
> +	if (m/^\Q$state->{BULK_COOKIES_DIR}\E/) {
> +		return $state->{builduser};
> +	}
>  	if (m/^\Q$state->{PACKAGE_REPOSITORY}\E/) {
>  		return $state->{builduser};
>  	}
> @@ -1062,6 +1070,7 @@ sub make_ports_subdirs($state)
>  		$state->build_dir(WHINE|MKPATH , "builduser", "WRKOBJDIR");
>  		$state->build_dir(0, "builduser", "PACKAGE_REPOSITORY");
>  		$state->build_dir(0, "builduser", "PLIST_REPOSITORY");
> +		$state->build_dir(0, "builduser", "BULK_COOKIES_DIR");
>  		$state->build_dir(WHINE, "builduser", "LOCKDIR");
>  	    });
>  }

Yes, this looks sane, and should definitely go in.

I overlooked BULK_COOKIES_DIR in proot because I mostly use proot with dpb
(so no bulk cookies) but it's just an oversight.

(note that "fix-permissions" properly includes BULK_COOKIES_DIR)