configure: improve cross-compilation detection by mobin-2008 · Pull Request #524 · davmac314/dinit

mobin-2008 · 2025-12-31T15:54:24Z

Assume the build as native if both $CXX and $CXX_FOR_BUILD have the same value. If the compiler for the host and the build machine are the same, it's a native build. There is a chance that the user may set both $CXX and $CXX_FOR_BUILD to the same compiler. For example, cbuild (Chimera Linux package builder) sets both variables regardless of native or cross build and because we use $CXX_FOR_BUILD for detecting cross build, we should unset that variable in this case.

Also, GNU autotools detects this case and assumes the build as native.

mobin-2008 · 2026-01-01T23:04:14Z

I just realised that the whole commit message should be written imperatively, not just the commit title or first sentence of commit comment.

davmac314 · 2026-01-02T01:09:27Z

I just realised that the whole commit message should be written imperatively, not just the commit title or first sentence of commit comment.

No, background information may follow. Your message here really isn't too bad and it's an improvement on previous commit messages. However, it could be improved:

Assume the build as native if both $CXX and $CXX_FOR_BUILD have the same value. If the compiler for the host and the build machine are the same, it's a native build

Assume the build is native.
These two sentences basically say the same thing; the 2nd one is just rephrasing the first. So when writing you make that clear, eg:

Assume the build is native if both $CXX and $CXX_FOR_BUILD have the same value (i.e. if the compiler for the host and the build machine are the same, it's a native build).

(This is nothing to do with commit messages per se - it's just better writing).

There is a chance that the user may set both $CXX and $CXX_FOR_BUILD to the same compiler

This is basically saying the same thing, a third time - I don't think it's necessary.

For example, cbuild (Chimera Linux package builder) sets both variables regardless of native or cross build and because we use $CXX_FOR_BUILD for detecting cross build, we should unset that variable in this case.

This is background info and in general the style used in commit messages is to start a new paragraph for the background information. You should read through the existing commit messages to get a sense for this. Here are some recent examples:

write_nx: use correct format specifier for unsigned types

In write_nx, use the correct 'printf' format specifier ('%u', '%lu') for
unsigned types.

As written beforehand, the signed and unsigned versions use the same
(signed value) format specifier, so large unsigned values would be
output incorrectly.

And

dinitctl: replace hardcoded name in error messages

Use a macro, DINITCTL_APPNAME, for the application name ("dinitctl") for
use in error messages etc.

This will make it easier to change the name or (with extra work) to use
the name used for invocation (i.e. argv[0]) at a later point.

See the general structure?

we should unset that variable in this case.

This is explaining what the change does at the code level. It doesn't make sense at the abstract level of the commit message (why should we unset the variable? we could also just flag "not a cross-build" internally in some other way).

Also, GNU autotools detects this case and assumes the build as native.

If the point is that the new behaviour is consistent with autotools, then it's better to say that directly rather than force the reader to make that connection themselves (even if it's fairly simple, there's no reason not to state your point explicitly).

mobin-2008 · 2026-01-06T20:45:11Z

Sorry for delay, I got sick. I'm better now.

I just realised that the whole commit message should be written imperatively, not just the commit title or first sentence of commit comment.

No, background information may follow. Your message here really isn't too bad and it's an improvement on previous commit messages. However, it could be improved:

Assume the build as native if both $CXX and $CXX_FOR_BUILD have the same value. If the compiler for the host and the build machine are the same, it's a native build
1. Assume the build _is_ native.

2. These two sentences basically say the same thing; the 2nd one is just rephrasing the first. So when writing you make that clear, eg:
Assume the build is native if both $CXX and $CXX_FOR_BUILD have the same value (i.e. if the compiler for the host and the build machine are the same, it's a native build).

(This is nothing to do with commit messages per se - it's just better writing).

I added the first sentence after my comment. It wasn't there and I added it without thinking twice because I wanted to rewrite it after I set the PR to draft. Thanks for the suggestion. Yes, It's better.

There is a chance that the user may set both $CXX and $CXX_FOR_BUILD to the same compiler

This is basically saying the same thing, a third time - I don't think it's necessary.

For example, cbuild (Chimera Linux package builder) sets both variables regardless of native or cross build and because we use $CXX_FOR_BUILD for detecting cross build, we should unset that variable in this case.

This is background info and in general the style used in commit messages is to start a new paragraph for the background information. You should read through the existing commit messages to get a sense for this. Here are some recent examples:
write_nx: use correct format specifier for unsigned types

In write_nx, use the correct 'printf' format specifier ('%u', '%lu') for
unsigned types.

As written beforehand, the signed and unsigned versions use the same
(signed value) format specifier, so large unsigned values would be
output incorrectly.
And
dinitctl: replace hardcoded name in error messages

Use a macro, DINITCTL_APPNAME, for the application name ("dinitctl") for
use in error messages etc.

This will make it easier to change the name or (with extra work) to use
the name used for invocation (i.e. argv[0]) at a later point.
See the general structure?

Yes. I need to get better in breaking down text to sections to understand them better.

we should unset that variable in this case.

This is explaining what the change does at the code level. It doesn't make sense at the abstract level of the commit message (why should we unset the variable? we could also just flag "not a cross-build" internally in some other way).

unset is mentioned in the commit message because I'm explaining why we need to unset and how that fixes the issue.
Currently the main purpose of $CXX_FOR_BUILD in the code is to use another compiler for the build machine which is required for cross-compiling. It can be read like this:
$CXX_FOR_BUILD is set: cross build.
$CXX_FOR_BUILD is not set: native build.
I think it's the correct thing to unset that variable at that point when it's a native build. Is there any reason to have special handling for this case?

Also, GNU autotools detects this case and assumes the build as native.

If the point is that the new behaviour is consistent with autotools, then it's better to say that directly rather than force the reader to make that connection themselves (even if it's fairly simple, there's no reason not to state your point explicitly).

Yes. That's better to explicitly state that point.

davmac314 · 2026-01-07T00:43:21Z

unset is mentioned in the commit message because I'm explaining why we need to unset and how that fixes the issue.

Currently the main purpose of $CXX_FOR_BUILD in the code is to use another compiler for the build machine which is required for cross-compiling. It can be read like this:
$CXX_FOR_BUILD is set: cross build.
$CXX_FOR_BUILD is not set: native build.
I think it's the correct thing to unset that variable at that point when it's a native build. Is there any reason to have special handling for this case?

I think you're misinterpreting what I said. I'm not saying the solution is wrong.

I think it's the correct thing to unset that variable at that point when it's a native build.

That's ok as a solution, but, I think the comment doesn't explain that very well. It doesn't say where the change is, it doesn't really explain how the solution works; you have to look at the code to make sense of it.

davmac314 · 2026-01-08T04:52:39Z

To explain more:

I'm explaining why we need to unset and how that fixes the issue

We don't need to unset the variable. That's the solution you have chosen.

And, it didn't explain how that fixes the issue. What was written was:

because we use $CXX_FOR_BUILD for detecting cross build, we should unset that variable in this case

it doesn't say how we "use" $CXX_FOR_BUILD for detecting a cross build, and it doesn't say how/why unsetting the variable alters the result of that detection. For example it doesn't explain the variable is unset just before the check for whether the $CXX_FOR_BUILD is set or not (which is used to detect a cross build). That's so easy to say and makes it so much clearer.

mobin-2008 · 2026-01-22T04:32:15Z

I'm still alive and I will fix the mentioned things once the situation gets better.

davmac314 · 2026-01-22T04:43:29Z

I'm still alive and I will fix the mentioned things once the situation gets better.

No worries, Mobin. I'm aware of the situation. Stay safe.

mobin-2008 · 2026-02-25T16:57:02Z

I cleaned up both commit message and code comments explaining the change, to be more clear and to respect the context of each other. The commit message doesn't mention variables anymore because it doesn't make sense to talk about it in the context of commit message. In the other hand, code comments now focus on variables which make sense in the context of the code.

Also the commit message is separated into three sections to be consistent with style of other commits.

davmac314 · 2026-02-26T09:20:41Z

Hey Mobin, welcome back!

The commit message is better, thanks. I do still have a few minor concerns about this PR:

The commit message still refers to "broken cross-compilation detection", that's unnecessarily negative and not really correct; the current cross-compilation detection isn't broken - it works exactly as it was designed to! You could phrase it in terms of improvement or nature of the change.
Relying on $CXX_FOR_BUILD == $CXX as an indicator of native build seems problematic, what if the same compiler is used for both host and target builds? For example clang can target a different platform via --target= which could be specified in CXXFLAGS_FOR_BUILD (and LDFLAGS_FOR_BUILD), with CXX and CXX_FOR_BUILD both set to clang++. If this was done, the current behaviour would be to assume a cross-build (with unknown target platform) and issue a warning. The new behaviour (introduced by this PR) would be to silently, and incorrectly, assume a native build. ~~How is the new behavior better?~~ The new behaviour seems worse to me, in this case at least.
The comment in the configure code reads as if it was a commit message. It's not really worth talking about cbuild or autotools; just explain what the code is doing, and/or why at the level of the code itself, not at the level of a broad overview. It's also confusing:

Unset $CXX_FOR_BUILD now to be consistent with GNU autotools and what people expect.

But this is only done if $CXX and $CXX_FOR_BUILD have the same value. This sentence in the comment makes it seem like it's done unconditionally; the sentence is also part of a paragraph which is giving background info, which means it's in the wrong place (should be a new paragraph, it's a new subject).

mobin-2008 · 2026-02-26T21:09:24Z

Hey Mobin, welcome back!

Thanks! I missed the project so much.

The commit message is better, thanks. I do still have a few minor concerns about this PR:

The commit message still refers to "broken cross-compilation detection", that's unnecessarily negative and not really correct; the current cross-compilation detection isn't broken - it works exactly as it was designed to! You could phrase it in terms of improvement or nature of the change.

OK.

Relying on $CXX_FOR_BUILD == $CXX as an indicator of native build seems problematic, what if the same compiler is used for both host and target builds? For example clang can target a different platform via --target= which could be specified in CXXFLAGS_FOR_BUILD (and LDFLAGS_FOR_BUILD), with CXX and CXX_FOR_BUILD both set to clang++. If this was done, the current behaviour would be to assume a cross-build (with unknown target platform) and issue a warning. The new behaviour (introduced by this PR) would be to silently, and incorrectly, assume a native build. ~~How is the new behavior better?~~ The new behaviour seems worse to me, in this case at least.

I didn't account for that. I think the problem here is that it should also check for *_FOR_BUILD variables because they affect the build environment and make a difference between host and build machine. I think this addresses your concern about the new behaviour. I try to think about all scenarios that make this check fail to correctly identify a native-build if there are any except the case you mentioned.

The comment in the configure code reads as if it was a commit message. It's not really worth talking about cbuild or autotools; just explain what the code is doing, and/or why at the level of the code itself, not at the level of a broad overview. It's also confusing:

Unset $CXX_FOR_BUILD now to be consistent with GNU autotools and what people expect.

But this is only done if $CXX and $CXX_FOR_BUILD have the same value. This sentence in the comment makes it seem like it's done unconditionally; the sentence is also part of a paragraph which is giving background info, which means it's in the wrong place (should be a new paragraph, it's a new subject).

I will rewrite it to improve it.

davmac314 · 2026-06-02T08:22:16Z

Hi @mobin-2008 , I'd really like you to finish this before opening any other pull requests please. I've already invested review time into it, I don't want that wasted, and this has taken too long for a small change (I know you've had a lot going on, but still, this should be finished before moving on to other things).

The grammar in comments doesn't have to be perfect but you should make sure comments are accurate, clear, and contain appropriate details. You should also switch to comparing each of the *_FOR_BUILD variables as you suggested in your last comment. That's not much to do, there's no reason not to just finish this off. Thanks.

mobin-2008 · 2026-06-02T11:14:35Z

Hi @mobin-2008 , I'd really like you to finish this before opening any other pull requests please. I've already invested review time into it, I don't want that wasted, and this has taken too long for a small change (I know you've had a lot going on, but still, this should be finished before moving on to other things).

The grammar in comments doesn't have to be perfect but you should make sure comments are accurate, clear, and contain appropriate details. You should also switch to comparing each of the *_FOR_BUILD variables as you suggested in your last comment. That's not much to do, there's no reason not to just finish this off. Thanks.

Ok. I'm on it.

mobin-2008 · 2026-06-02T13:09:12Z

I tested it with cports building and it correctly assumes the build is native because of equal environment for both specified compilers.

Also I think configure should always write *FLAGS_FOR_BUILD variables to mconfig even when they are empty to avoid issues further and I will open a PR to fix that after this getting merged.

davmac314

Other than the specific comments, I think this also needs documentation to be updated.

davmac314 · 2026-06-03T01:47:11Z

+## Assume the build is native if both $CXX and $CXX_FOR_BUILD and their respective flags have the
+## same value and unset CXX_FOR_BUILD if that is the case


The comment isn't very clear; it doesn't explain the connection between assuming the build is native and unsetting CXX_FOR_BUILD. I already made a very similar comment about this in the previous iteration:

it doesn't say how we "use" $CXX_FOR_BUILD for detecting a cross build, and it doesn't say how/why unsetting the variable alters the result of that detection. For example it doesn't explain the variable is unset just before the check for whether the $CXX_FOR_BUILD is set or not (which is used to detect a cross build). That's so easy to say and makes it so much clearer.

The text "their respective flags" is also vague (who is the "they" corresponding to "their"?), it would be better to be more specific.

davmac314 · 2026-06-03T03:00:49Z

+        FULL_CXXFLAGS="${CXXFLAGS:-}"
+        FULL_LDFLAGS="${LDFLAGS:-}"
+        if [ -n "${CXXFLAGS_EXTRA:-}" ]; then
+            if [ -n "${CXXFLAGS:-}" ]; then
+                FULL_CXXFLAGS="$CXXFLAGS $CXXFLAGS_EXTRA"
+            else
+                FULL_CXXFLAGS="$CXXFLAGS_EXTRA"
+            fi
+        fi
+        if [ -n "${LDFLAGS_EXTRA:-}" ]; then
+            if [ -n "${LDFLAGS:-}" ]; then
+                FULL_LDFLAGS="$LDFLAGS $LDFLAGS_EXTRA"
+            else
+                FULL_LDFLAGS="$LDFLAGS_EXTRA"
+            fi
+        fi
+        if [ "$FULL_CXXFLAGS" = "${CXXFLAGS_FOR_BUILD:-}" ] && \
+                [ "$FULL_LDFLAGS" = "${LDFLAGS_FOR_BUILD:-}" ]; then
+            unset CXX_FOR_BUILD
+        fi
+        unset FULL_CXXFLAGS
+        unset FULL_LDFLAGS


I have a few thoughts about the approach here. I guess you try to derive the full set of flags that will be used for regular compilation (i.e. both from CXXCFLAGS and CXXFLAGS_EXTRA) and compare with the flags for build (CXXFLAGS_FOR_BUILD ) to check whether the compiler flags that will ultimately be used for building application code and host code are the same (a brief comment to explain this would have been good, but that might be moot, as per following).

Mainly, I think this is too complex. We are anyway taking a heuristic approach (if flags differ we don't know for sure if it is or isn't a cross-build) and having overcomplicated heuristics isn't worth it. Some ideas for simplifying it:

First, the point of setting CXXFLAGS_EXTRA is to use the automatically-derived (default) flags for building application code (and add some additional, i.e. EXTRA, flags). Someone using this functionality is unlikely to go to the trouble of then specifying the exact same set of flags in CXXFLAGS_FOR_BUILD. So, it doesn't make a lot of sense to me to even consider CXXFLAGS_EXTRA in this equation, or if you do it should be more along the lines of "if CXXFLAGS_EXTRA is set, assume it might be setting compilation target and so it may be a cross-build".

Second, building for a different target can't be done by changing only the link stage. So I don't think there's much point doing the LDFLAGS comparison.

So, it doesn't make a lot of sense to me to even consider CXXFLAGS_EXTRA

In a similar vein, it doesn't make much sense to compare a configure-determined CXXFLAGS value with the CXXFLAGS_FOR_BUILD value. If the user didn't set CXXFLAGS and didn't set CXXFLAGS_FOR_BUILD, should they really be considered different?

First, the point of setting CXXFLAGS_EXTRA is to use the automatically-derived (default) flags for building application code (and add some additional, i.e. EXTRA, flags). Someone using this functionality is unlikely to go to the trouble of then specifying the exact same set of flags in CXXFLAGS_FOR_BUILD.

And I'm not assuming that. My case for such logic is this:

./configure CXX="clang++" CXX_FOR_BUILD="clang++" CXXFLAGS_EXTRA="--target=aarch64-linux-gnu"

This logic is there to detect any difference between host compiler and build compiler and CXXFLAGS_EXTRA would make a difference.

So, it doesn't make a lot of sense to me to even consider CXXFLAGS_EXTRA in this equation, or if you do it should be more along the lines of "if CXXFLAGS_EXTRA is set, assume it might be setting compilation target and so it may be a cross-build".

Now I'm thinking, yeah, that make sense to change it to "if CXXFLAGS_EXTRA is set, assume it might be setting compilation target and so it may be a cross-build".

In a similar vein, it doesn't make much sense to compare a configure-determined CXXFLAGS value with the CXXFLAGS_FOR_BUILD value. If the user didn't set CXXFLAGS and didn't set CXXFLAGS_FOR_BUILD, should they really be considered different?

No. They are not different and current logic just assume the build is native in that case because the CXXFLAGS is not utilized by configure at that point and the script never sets any *_FOR_BUILD variable so the check would be [ "" = "" ] && unset CXX_FOR_BUILD.

Second, building for a different target can't be done by changing only the link stage. So I don't think there's much point doing the LDFLAGS comparison.

You're right, checking for compiler flags is enough.

No. They are not different and current logic just assume the build is native in that case because the CXXFLAGS is not utilized by configure at that point

You are right, I thought this new logic was added after setting CXXFLAGS with default flags, but it is not.

davmac314 · 2026-06-03T04:10:37Z

I tested it with cports building and it correctly assumes the build is native because of equal environment for both specified compilers.

Ok, but what other configurations did you test? Seems to me like:

./configure CXX=gcc CXX_FOR_BUILD=gcc

... is going to be detected as a cross-build.

In general it feels to me like you are now rushing this PR. Please don't; take the time to get it right.

Also I think configure should always write *FLAGS_FOR_BUILD variables to mconfig even when they are empty to avoid issues further and I will open a PR to fix that after this getting merged.

I don't know what you mean. What issues will this avoid?

mobin-2008 · 2026-06-03T21:32:11Z

I tested it with cports building and it correctly assumes the build is native because of equal environment for both specified compilers.

Ok, but what other configurations did you test? Seems to me like:
./configure CXX=gcc CXX_FOR_BUILD=gcc
... is going to be detected as a cross-build.

No.

./configure CXX=gcc CXX_FOR_BUILD=gcc
Checking whether "gcc" is a working C++ compiler...
  ... Yes.
Checking whether "-std=c++11" is accepted by the compiler...
  ... Yes.
Checking whether "-Wall" is accepted by the compiler...
  ... Yes.
Checking whether "-Os" is accepted by the compiler...
  ... Yes.
Checking whether "-fno-plt" is accepted by the compiler...
  ... Yes.
Checking whether "-fno-rtti" is accepted by the compiler...
  ... Yes.
Checking whether "-fno-rtti" breaks exceptions...
  ... Yes. Disabling -fno-rtti
Checking whether "-flto" is accepted as an option for both compiling and linking...
  ... Yes.
Looking for "libcap" via pkg-config...
  ... Found.
Checking whether header "linux/ioprio.h" is available...
  ... Yes.
Checking whether "-fsanitize=address,undefined" is accepted as an option for both compiling and linking...
  ... Yes.
Creating mconfig...
  ... done.
Done!

Because the check

        if [ "$FULL_CXXFLAGS" = "${CXXFLAGS_FOR_BUILD:-}" ]; then
            unset CXX_FOR_BUILD
        fi

in this case will be

        if [ "" = "" ]; then
            unset CXX_FOR_BUILD
        fi

which is true.

In general it feels to me like you are now rushing this PR. Please don't; take the time to get it right.

I'm trying to balance the correctness with time to avoid taking too much time for a simple PR.

Also I think configure should always write *FLAGS_FOR_BUILD variables to mconfig even when they are empty to avoid issues further and I will open a PR to fix that after this getting merged.

I don't know what you mean. What issues will this avoid?

The problem is that if I generate mconfig with:

./configure CXX=clang++ CXX_FOR_BUILD=clang++ CXXFLAGS="-std=c++11 --target=aarch64-linux-gnu"

and then try to build the app, it will use clang++ -std=c++11 --target=aarch64-linux-gnu -o mconfig-gen mconfig-gen.cc for building mconfig-gen.cc which is wrong and will result in build failure. The Makefile uses $CXXFLAGS for compiler flags in this case, causing this issue.

mobin-2008 · 2026-06-03T22:05:46Z

Other than the specific comments, I think this also needs documentation to be updated.

Where? configure help message?

davmac314 · 2026-06-03T22:08:09Z

[...] Seems to me like:
./configure CXX=gcc CXX_FOR_BUILD=gcc
... is going to be detected as a cross-build.
No.

Ok.

In general it feels to me like you are now rushing this PR. Please don't; take the time to get it right.

I'm trying to balance the correctness with time to avoid taking too much time for a simple PR.

In this project correctness should be considered paramount.

Not considering things properly is what leads to taking too much time for the PR. You also ignored one of my earlier review comments (which I pointed out again this review); this is not making things faster, it is making it slower, and is using up my time as well as yours.

Also I think configure should always write *FLAGS_FOR_BUILD variables to mconfig even when they are empty to avoid issues further and I will open a PR to fix that after this getting merged.

The problem is that if I generate mconfig with:
./configure CXX=clang++ CXX_FOR_BUILD=clang++ CXXFLAGS="-std=c++11 --target=aarch64-linux-gnu"
and then try to build the app, it will use clang++ -std=c++11 --target=aarch64-linux-gnu -o mconfig-gen mconfig-gen.cc for building mconfig-gen.cc which is wrong and will result in build failure. The Makefile uses $CXXFLAGS for compiler flags in this case, causing this issue.

I think they should be written to mconfig if they are set (even if set to empty string), not always.

davmac314 · 2026-06-03T22:13:33Z

I'm happy for you to go ahead with changes to address the review. I think you may need to rebase onto main in order to pull the documentation (BUILD) changes so that you can updated them accordingly - please also go ahead and do that.

davmac314 · 2026-06-03T22:14:58Z

Other than the specific comments, I think this also needs documentation to be updated.

Where? configure help message?

BUILD.

If cross-compiling (i.e. specifying the CXX_FOR_BUILD variable), there are some additional
considerations regarding use of the "configure" script.

mobin-2008 · 2026-06-03T22:29:37Z

Ok. I will apply changes tomorrow (It's 2 AM in here) and also make sure to don't rush the process to avoid missing any review points and ensuring correctness.

davmac314 · 2026-06-03T22:36:22Z

Ok. I will apply changes tomorrow (It's 2 AM in here) and also make sure to don't rush the process to avoid missing any review points and ensuring correctness.

Thanks. It doesn't have to be done by tomorrow - get it right rather than do it quickly, please.

mobin-2008 · 2026-06-04T18:08:32Z

I tested it with cports and other cases (./configure CXX=gcc CXX_FOR_BUILD=gcc, CXX="clang++" CXX_FOR_BUILD="clang++" CXXFLAGS_EXTRA="--target=aarch64-linux-gnu" and ./configure CXX=clang++ CXX_FOR_BUILD=clang++ CXXFLAGS="-std=c++11 --target=aarch64-linux-gnu") and it works.

BUILD updated to reflect the how configure reacts to the CXX_FOR_BUILD and other compiler flags for cross-compile detection. Comment improved to properly explain connection between cross-build and $CXX_FOR_BUILD and also used your own comment for giving context in CXXFLAGS_EXTRA check.

davmac314 · 2026-06-04T21:48:15Z

+## Assume the build is native if both $CXX and $CXX_FOR_BUILD and their respective compiler flags
+## ($CXXFLAGS and $CXXFLAGS_FOR_BUILD) have the same value.
+# The $CXX_FOR_BUILD is a variable set by the user to the compiler for the build machine. The
+# configure script assume the build is cross when the $CXX_FOR_BUILD is set. The following check
+# ensures that the host compiler and the build compiler are actually different and if they're not,
+# unsets the $CXX_FOR_BUILD before all the checks for cross-compile along the script.


The textual content is now a lot better.

However:

The following check ensures that the host compiler and the build compiler are actually different and if they're not, unsets the $CXX_FOR_BUILD before all the checks for cross-compile along the script

This is not accurate. The check ensures that either the host compiler and the build compiler are different or that CXX_FOR_BUILD is unset.

Also, why is this mixing single and double hashes (# vs ##) at the beginning of the comment lines?

Also, why is this mixing single and double hashes (# vs ##) at the beginning of the comment lines?

The idea I had for ## and # were that ## was the subject and # was the additional description.

I see in other places ## seems to be used for a kind of brief heading describing what the following section of code does at a high level, but never with this much detail and never with # lines immediately underneath.

If you want to have a ## line similar to the others that's fine but it needs to be concise (like the others) and probably more of a general description than specifics about variable names and so on.

Here's an example of another ##-style heading already in the code:

## Verify PLATFORM value

It's true that this does mention a specific variable, but in a very general way. It doesn't go into detail about what "verify" means or how it is done; compare that to what is written (with ##) above.

Then, at least have a blank line or something between that heading and further description (with single '#'s), so that everything isn't jammed together. Paragraphs are supposed to be separated from any previous paragraph or heading (in all writing). Put the additional detail here in this part since that seems to be what it's intended for.

davmac314 · 2026-06-04T23:21:35Z


-If cross-compiling (i.e. specifying the CXX_FOR_BUILD variable), there are some additional
-considerations regarding use of the "configure" script.
+By specifying CXX_FOR_BUILD variable, the "configure" script will assume the build is cross, unless


"By specifying CXX_FOR_BUILD variable" -> "If the CXX_FOR_BUILD variable is specified"

"build is cross" -> "build is a cross-build".

davmac314 · 2026-06-04T23:23:01Z

+If cross-compiling, there are some additional considerations regarding use of the "configure"
+script.


This paragraph can probably just go. It was introductory, but it's been moved past the point where it can serve as introduction.

davmac314 · 2026-06-04T23:23:55Z

+If all of these conditions are met, the "configure" script will instead assume that the build is
+native in this case.


"in this case" is redundant, you have already specified the case for which this applicable (via "If all of these conditions are met").

davmac314 · 2026-06-05T01:45:01Z

Please go ahead and make changes to address the recent review comments (unless you need more discussion).

davmac314

Please make the changes requested.

davmac314 · 2026-06-05T23:04:03Z

+If the CXX_FOR_BUILD variable is specified, the "configure" script will assume the build is
+cross-build, unless all of the following conditions are met:


"build is cross-build" -> "build is a cross-build"

You left out "a".

davmac314 · 2026-06-05T23:18:35Z

    esac
 done

+## Verify the build is cross-build when CXX_FOR_BUILD is specificed


davmac314 · 2026-06-05T23:23:56Z

+## Verify the build is cross-build when CXX_FOR_BUILD is specificed
+if [ -n "${CXX_FOR_BUILD:-}" ]; then
+    # The $CXX_FOR_BUILD is a variable set by the user to the compiler for the build machine.
+    # The configure script assume the build is cross-build when the $CXX_FOR_BUILD is set. The


You can't say "the configure script assume the build is cross-build when the $CXX_FOR_BUILD is set." and then specifically say how the following check (which is within the configure script!) doesn't assume that at all. This comment contradicts itself. Qualify the first sentence so that it's accurate and is not being contradicted: "Later parts of this script assume that ...".

It's also clearer if you don't talk about "the configure script" in comments within the configure script, as if it is referring to something else. You can say "this script" instead of "the configure script".

davmac314

I'll approve this shortly, but please see the notes I've added and make those changes before merging.

davmac314 · 2026-06-09T03:56:30Z

+        # The point of setting $CXXFLAGS_EXTRA is to use the automatically-derived (default) flags
+        # for building application code (and add some additional, i.e. EXTRA, flags). Someone using
+        # this functionality is unlikely to go to the trouble of then specifying the exact same set
+        # of flags in $CXXFLAGS_FOR_BUILD, therefor when $CXXFLAGS_EXTRA is set, assume it's always
+        # different from $CXXFLAGS_FOR_BUILD.


Could you remove this, actually? It was never meant to be a code comment when I wrote it (as a review comment), the tone is all wrong and it's not really all that clear without the rest of the review context.

davmac314 · 2026-06-09T03:57:07Z

-If cross-compiling (i.e. specifying the CXX_FOR_BUILD variable), there are some additional
-considerations regarding use of the "configure" script.
+If the CXX_FOR_BUILD variable is specified, the "configure" script will assume the build is
+a cross-build, unless all of the following conditions are met:


Lines should extend as close to 100 columns as possible so "a" should be on the previous line.

davmac314

Please make fixes as per comments before merging.

mobin-2008 · 2026-06-09T15:20:11Z

Please make fixes as per comments before merging.

The issue here I think is that, the moment I push these new changes into my branch, GitHub will invalidate your approval on this PR.

Assume the build is native if the compiler for the host and the build machine are the same and both use the same flags. It is fairly common to set both variables to the same compiler with same flags in a native build. For example, cbuild (Chimera Linux package builder) defines both the host and the build compiler and their respective compiler flags, regardless of native or cross build. Also, that assumption in this case is consistent with GNU autotools behaviour.

mobin-2008 added Bug/Bugfix A-Importance: Normal Build-issue Something that affects build process of Dinit labels Dec 31, 2025

mobin-2008 marked this pull request as draft January 1, 2026 23:00

mobin-2008 force-pushed the fix_cross_compile branch from bfa8560 to 27cfb8d Compare February 25, 2026 16:49

mobin-2008 force-pushed the fix_cross_compile branch from 27cfb8d to 65340b0 Compare February 25, 2026 17:32

mobin-2008 marked this pull request as ready for review February 25, 2026 18:41

mobin-2008 mentioned this pull request May 9, 2026

configure: more robust compiler check and cross-build detection refactor #546

Closed

2 tasks

mobin-2008 closed this May 9, 2026

davmac314 mentioned this pull request May 31, 2026

Utilize features from C++17 #549

Closed

davmac314 changed the title ~~configure: fix broken cross-compilation detection~~ configure: improve cross-compilation detection Jun 2, 2026

davmac314 reopened this Jun 2, 2026

mobin-2008 force-pushed the fix_cross_compile branch from 65340b0 to f8fbd82 Compare June 2, 2026 13:06

davmac314 requested changes Jun 3, 2026

View reviewed changes

mobin-2008 force-pushed the fix_cross_compile branch 2 times, most recently from b3e0f3c to a091a33 Compare June 4, 2026 18:08

davmac314 requested changes Jun 4, 2026

View reviewed changes

mobin-2008 force-pushed the fix_cross_compile branch from a091a33 to c758ecd Compare June 5, 2026 12:10

davmac314 requested changes Jun 5, 2026

View reviewed changes

mobin-2008 force-pushed the fix_cross_compile branch from c758ecd to 0d7728e Compare June 6, 2026 17:42

davmac314 reviewed Jun 9, 2026

View reviewed changes

davmac314 previously approved these changes Jun 9, 2026

View reviewed changes

mobin-2008 dismissed davmac314’s stale review via d2c9a7a June 9, 2026 15:21

mobin-2008 force-pushed the fix_cross_compile branch from 0d7728e to d2c9a7a Compare June 9, 2026 15:21

davmac314 approved these changes Jun 9, 2026

View reviewed changes

mobin-2008 merged commit 54b9990 into davmac314:master Jun 10, 2026
4 checks passed

		## Assume the build is native if both $CXX and $CXX_FOR_BUILD and their respective flags have the
		## same value and unset CXX_FOR_BUILD if that is the case

		If cross-compiling, there are some additional considerations regarding use of the "configure"
		script.

		If all of these conditions are met, the "configure" script will instead assume that the build is
		native in this case.

		If the CXX_FOR_BUILD variable is specified, the "configure" script will assume the build is
		cross-build, unless all of the following conditions are met:

Uh oh!

Conversation

mobin-2008 commented Dec 31, 2025

Uh oh!

mobin-2008 commented Jan 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

davmac314 commented Jan 2, 2026

Uh oh!

mobin-2008 commented Jan 6, 2026

Uh oh!

davmac314 commented Jan 7, 2026

Uh oh!

davmac314 commented Jan 8, 2026

Uh oh!

mobin-2008 commented Jan 22, 2026

Uh oh!

davmac314 commented Jan 22, 2026

Uh oh!

mobin-2008 commented Feb 25, 2026

Uh oh!

davmac314 commented Feb 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mobin-2008 commented Feb 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

davmac314 commented Jun 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mobin-2008 commented Jun 2, 2026

Uh oh!

mobin-2008 commented Jun 2, 2026

Uh oh!

davmac314 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

davmac314 commented Jun 3, 2026

Uh oh!

mobin-2008 commented Jun 3, 2026

Uh oh!

mobin-2008 commented Jun 3, 2026

Uh oh!

davmac314 commented Jun 3, 2026

Uh oh!

davmac314 commented Jun 3, 2026

Uh oh!

davmac314 commented Jun 3, 2026

Uh oh!

mobin-2008 commented Jun 3, 2026

Uh oh!

davmac314 commented Jun 3, 2026

Uh oh!

mobin-2008 commented Jun 4, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

davmac314 commented Jun 5, 2026

mobin-2008 commented Jan 1, 2026 •

edited

Loading

davmac314 commented Feb 26, 2026 •

edited

Loading

mobin-2008 commented Feb 26, 2026 •

edited

Loading

davmac314 commented Jun 2, 2026 •

edited

Loading

mobin-2008 commented Jun 9, 2026 •

edited

Loading