I'll remove the |= bit of this patch and related commentary in the commit message. I won't change the kern_exec.c part of the patch, but I do want to think about whether it would be useful for imgp->interpreted to reflect all of the interpreters involved in the chain -- it could be that it's useful to set IMGACT_INTERP_ELF in imgp->interpreted back where __elfN(load_interp) is first called in the elf activator. I'm not sure I see how at the moment, though:

In D58063#1331520, @kevans wrote:

In D58063#1331519, @sjg wrote:

In D58063#1331516, @kevans wrote:

Hmm, the last bit seems to be wrong because elf's imgact will set interpreted = 0 anyways. Actually, that seems to happen *after* we set it in load_interp_file -- was there something missing from the original patch?

I don't think so. The original problem was that when exec'ing ld-elf.so the 'interpreted' field was not set so if we set veriexec's indirect flag on ld-elf.so, mac_veriexec thinks it is a direct exec and blocks it - which is sub-optimal. Simply having imgp->interpreted != 0 is sufficient to address that.

Right, but as far as I can tell it won't work right in vanilla FreeBSD. We set imgp->interpreted = 0 here:

https://cgit.freebsd.org/src/tree/sys/kern/imgact_elf.c#n1461

After you would have set it non-zero up in load_interp -- I think clearing it needs to move above the interp != NULL branch, or the |= here changes back to = and we just clear it in the else block (interp == NULL).

In D58063#1331516, @kevans wrote:

Hmm, the last bit seems to be wrong because elf's imgact will set interpreted = 0 anyways. Actually, that seems to happen *after* we set it in load_interp_file -- was there something missing from the original patch?

In D57683#1322417, @stevek wrote:

Minor things in the python found by pep8, but the rest looks good.

ok

In D54542#1303186, @emaste wrote:

make(1) documents - as

-       in compatibility mode causes any non-zero exit status of
        the command line to be ignored.

and is followed by a fuller explanation of behaviour in non-compatibility mode in a paragraph. @sjg to me this implies that - does not ignore non-zero exit status in non-compatibility mode, and ought to be clarified. What about something like - causes any non-zero exit status of the command line to be ignored. The behaviour is slightly different when make is run in compatibility vs jobs mode; see below?

Check for error != 0

delete lines containing backtics or $()

Fix comment to match the expected result.

Add quotes around result if we see _ followed by space.

Add $() example

fair enough

Fix arg for LOADER_VERIEXEC_TESTING printf

In D56287#1287814, @sjg wrote:

In D56287#1287811, @imp wrote:

I like this, and also @kevans 's suggestion.

So rename it to secure_setenv ?

In D56287#1287811, @imp wrote:

I like this, and also @kevans 's suggestion.

In D56287#1287801, @kevans wrote:

I think the concept is fine, my main nit is: do we want to call this secure_setenv for symmetry with the secure_getenv name we use in libc, for use in security-sensitive getenv() calls?

In D56087#1283209, @ivy wrote:

about dirdeps: i'm hoping this will allow us to eventually support a dirdeps build for packages, but that's a long-term plan and not something i'm working on right now. but if there's anything we can change here to make that easier in the future (especially if it changes the developer-facing interface) i can definitely look at doing that now.

In D56087#1283196, @ivy wrote:

In D56087#1283195, @sjg wrote:

what is the difference b/w packages/ and release/packages/ ?

originally we only had release/packages, but i've moved the new bits to packages/ because packages aren't really a release thing, they're part of the src build. the remaining files in release/packages will go away when i redo the kernel/dtb and src packages.

what is the difference b/w packages/ and release/packages/ ?

In D56087#1282930, @brooks wrote:

I like the overall change. I've at most skimmed the makefile bits, but this seems like a move in the right direction.

Tagging @sjg in case he has any thoughts on this. IIRC he has previously said that driving package creation from individual directories was required to support dirdeps packages so hopefully this is a move in the right direction.

makes sense

FWIW this simple .export OBJTOP works ok for buildworld but not for targets like universe where multiple arches try to use the same OBJTOP which quickly ends in tears.
The more elaborate patch in https://reviews.freebsd.org/D54819 blows up in buildworld due to being incompatible with assumptions made by all the *compat* bits, but once that compat stuff is fixed, probably has a hope of working for universe too.

In D54819#1253097, @jrtc27 wrote:

Do we need to worry about $s in OBJROOT?

Remove all but the last := for OBJTOP

Please see https://reviews.freebsd.org/D54819

The patch below, builds buildworld ok, avoids these warnings (due to OBJTOP being empty when evaluatinng MAKEOBJDIR which is set to ${.CURDIR:S,${SRCTOP},${OBJTOP},}

diff --git a/share/mk/src.sys.obj.mk b/share/mk/src.sys.obj.mk
index e4fe3fa9a2aa..67af0b833faa 100644
--- a/share/mk/src.sys.obj.mk
+++ b/share/mk/src.sys.obj.mk
@@ -88,18 +88,22 @@ SB_OBJROOT:= ${OBJROOT}
 # in the source tree.
 .if ${MK_UNIFIED_OBJDIR} == "yes" && ${SRCTOP} != ${OBJROOT:tA}
 .if defined(TARGET) && defined(TARGET_ARCH)
-OBJTOP:=       ${OBJROOT}${TARGET}.${TARGET_ARCH}
+OBJTOP:=       ${OBJROOT}$${TARGET}.$${TARGET_ARCH}
 .elif defined(TARGET) && ${.CURDIR} == ${SRCTOP}
 # Not enough information, just use basic OBJDIR.  This can happen with some
 # 'make universe' targets or if TARGET is not being used as expected.
 OBJTOP:=       ${OBJROOT:H}
 .else
-OBJTOP:=       ${OBJROOT}${MACHINE}.${MACHINE_ARCH}
+OBJTOP:=       ${OBJROOT}$${MACHINE}.$${MACHINE_ARCH}
 .endif
 .else
 # TARGET.TARGET_ARCH handled in OBJROOT already.
 OBJTOP:=       ${OBJROOT:H}
 .endif # ${MK_UNIFIED_OBJDIR} == "yes"
+# export but do not track
+.export-env OBJTOP
+# resolve if needed
+OBJTOP:= ${OBJTOP}
 .endif # empty(OBJTOP)

Fix CVS Ids

Looks fine

In D53429#1221417, @jhb wrote:

Eh, I'm not sure this is actually the style we want? Lots of places in the tree fix the diff problem by just using multiple += statements, e.g.:

FOO= foo
FOO+= bar
FOO+= baz