Page MenuHomeFreeBSD

Mk/bsd.gcc.mk: Add jit argument
ClosedPublic

Authored by jrm on Jul 3 2021, 6:49 PM.

Details

Summary

This is useful for ports that require a LIB_DEPENDS on GCC for libgccjit
and also want to make use of the USE_GCC logic to choose an appropriate
version of GCC to depend on.

For the NATIVECOMP option in editors/emacs-devel, we would rather keep the
default compiler for building emacs regardless what port options a user chooses,
but still add the LIB_DEPENDS on GCC when the user chooses NATIVECOMP. We are
now just doing LIB_DEPENDS=libgccjit.so:lang/gcc11 when the user chooses the
NATIVECOMP option, but we don't get the helpful logic for choosing a GCC version
with something like USE_GCC=11+:jit.

editors/emacs-devel: Switch to USE_GCC for NATIVECOMP

Diff Detail

Repository
R11 FreeBSD ports repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

jrm requested review of this revision.Jul 3 2021, 6:49 PM

Mk/bsd.gcc.mk: Update comments and IGNORE value

gerald requested changes to this revision.Sep 5 2021, 11:54 AM

I support this patch and while I'm offering a couple of minor suggestions, there's only one "contested" aspect (which looks like it may just be a naming issue (or me missing an aspect)).

Mk/bsd.gcc.mk
17–18 ↗(On Diff #91704)

Would you mind using a full stop here. :-)

("Yes" = not adding it is an okay answer.)

19 ↗(On Diff #91704)

This loses the "only" from the original description, which is an important nuance: Yes, "build" adds a BUILD_DEPENDS. The key point is that it *avoids* the run-time dependency (that RUN_DEPENDS or LIB_DEPENDS would bring).

I'm not a native speaker and would be happy for you to improve upon the following idea:

build: adds GCC as a build dependency only (no run-time dependency)

39 ↗(On Diff #91704)

"GCC 11 or later" (is what 11+ indicates)

72 ↗(On Diff #91704)

Can we lose the "only" here, which is shorter.

(I acknowledge I just wanted to get "only" just two comments above. ;-) )

119–120 ↗(On Diff #91704)

I'm not sure I understand. Why this? Do we explicitly want to prevent this version of GCC being used to build? That would be a good explicit note when describing this option above. Intuitively I would not have expected it. If we can do without this, I would prefer so.

(Thought experiment: How would we go about the case where GCC should be used to build something *and* this dependency be added? Or should this new option be called differently? I think I may understand what you are after, just _USE_GCC_LIB_DEPENDS confuses me.)

126 ↗(On Diff #91704)

Same as above, and this is really the only part of this patch that I'm struggling with: _USE_GCC_LIB_DEPENDS confuses me as the condition used; can we use something more descriptive / clear?

145 ↗(On Diff #91704)

Could/should this be ${_GCC_RUNTIME} instead? Should be the same, but avoids duplicating the logic.

This revision now requires changes to proceed.Sep 5 2021, 11:54 AM
jrm marked 4 inline comments as done.Sep 5 2021, 6:21 PM

I'll update the diff momentarily with your comment suggestions, then update again to address your logical concerns.

Mk/bsd.gcc.mk
17–18 ↗(On Diff #91704)

I'm guessing you mean a : rather than a . here. I made that change, but if it's not what you are suggesting, please let me know.

119–120 ↗(On Diff #91704)

At least for the Emacs NATIVECOMP case, we only want to add the LIB_DEPENDS on the chosen GCC and not necessary build with GCC.

I imagined the answer to your thought experiment would be to simply USE_GCC=yes, however I can now appreciate the problem. Namely, adding a LIB_DEPENDS on the appropriate version of GCC shouldn't change the build compiler logic.

jrm marked an inline comment as done.

Mk/bsd.gcc.mk: Address comment issues

Maybe instead of the jit argument, it would be better to do something similar to what's suggested in PR 256893.

.if ${PORT_OPTIONS:MNATIVECOMP} && !defined(_CHECKING_GCC_PORT)
_GCC_PORT!=	${MAKE} -V_GCC_PORT USE_GCC=11+ _CHECKING_GCC_PORT=yes
.endif

The downside is that it spawns a shell.

Thoughts?

This revision was not accepted when it landed; it landed in state Needs Review.Sep 7 2021, 2:46 AM
This revision was automatically updated to reflect the committed changes.

Ended up committing something based on PR 256893, which is probably less confusing and intrusive. Maybe we can revisit this idea if we can come up with a better solution and there is more demand for GCC jit.