configure: fix broken cross-compilation detection

[mobin-2008]

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]

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

[davmac314]

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).

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]

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).

1. unset is mentioned in the commit message because I'm explaining why we need to unset and how that fixes the issue.
2. 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]

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

2. 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]

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.