Re: GNU make troubleshooting

[Torbjorn SVENSSON]

On 2023-07-16 15:58, Alejandro Colomar wrote:

Wow!  That's a great help for debugging the Linux man-pages's makefiles,
which don't need to be remade.  It was very painful to me to ignore so
much make -d output.  I applied the following patch, and it works like
a charm.




Why are you wasting time doing grep when you can have find return just 
those files that ends with .mk? Also, why need to sort the list when 
it's only for make to define a rule?


I would have done something along these lines if I were in your shoes:

MK := $(srcdir)/Makefile
MK += $(shell $(FIND) $(MAKEFILEDIR) -type f -name '*.mk')


Or, a more cleaner solution would have been to define the MK-array in 
the $(srcdir)/Makefile as you have it hard coded for the include 
statements now.


MK :=   \
$(MAKEFILEDIR)/check/_.mk   \
$(MAKEFILEDIR)/check/catman.mk  \
$(MAKEFILEDIR)/build/_.mk   \
$(MAKEFILEDIR)/build/catman.mk  \
$(MAKEFILEDIR)/build/html.mk\
$(MAKEFILEDIR)/build/pdf.mk \
$(MAKEFILEDIR)/build/pre.mk \
$(MAKEFILEDIR)/build/ps.mk  \
$(MAKEFILEDIR)/build/src.mk \
$(MAKEFILEDIR)/dist.mk  \
$(MAKEFILEDIR)/install/_.mk \
$(MAKEFILEDIR)/install/html.mk  \
$(MAKEFILEDIR)/install/man.mk   \
$(MAKEFILEDIR)/lint/_.mk\
$(MAKEFILEDIR)/lint/c.mk\
$(MAKEFILEDIR)/lint/man/_.mk\
$(MAKEFILEDIR)/lint/man/man.mk  \
$(MAKEFILEDIR)/lint/man/mdoc.mk \
$(MAKEFILEDIR)/make.mk  \
$(MAKEFILEDIR)/verbose.mk

include $(MK)
$(srcdir)/Makefile $(MK):: ;


Kind regards,
Torbjörn

Re: [PING] [PATCH] Escape space in path to $SHELL

[Torbjorn SVENSSON]

Thanks for the reply Paul.

On 2023-07-12 17:19, Paul Smith wrote:

On Wed, 2023-07-12 at 17:11 +0200, Torbjorn SVENSSON wrote:

* src/job.c (construct_command_argv_internal): Escape space in
$SHELL


Unfortunately this is so simple.  Currently the spaces in the SHELL
variable are not escaped, and things like this work:

 SHELL := /bin/sh -x

and do what you expect.  Or, perhaps you use a different shell than
/bin/sh and you add options that way.


I see.


I fully agree with you that it is not great that we have this, plus
.SHELLFLAGS, plus poor support for whitespace in pathnames (including
in the path of the SHELL program) but it is what it is.

My suspicion is that this non-backward-compatible change will break
some number of makefiles.

I would prefer to avoid doing this until we can make a comprehensive
and complete set of changes around show shells are managed and invoked,
so that we don't need to make backward-incompatible changes across
multiple releases (if they are needed).

In short, the fact that GNU Make doesn't work well with pathnames
containing whitespace is not limited to targets and prerequisites: it's
also true for programs that make invokes including the value of SHELL.


From what I can tell, this is possible to work around by quoting the 
path in the makefile (so that the quoted string is parsed by the shell 
and not make).



I have a lot of other things going on right now, but I will return to 
this subject in the future and try to figure out a way that would be 
compatible with the use-case you mentioned above, while still support 
space in the path part.

[PING] [PATCH] Escape space in path to $SHELL

[Torbjorn SVENSSON]

Gentle ping...

Kind regards,
Torbjörn

On 2023-07-05 11:09, Torbjörn SVENSSON wrote:

One way to trigger the error is to set SHELL to a path that contain
one space character.

To test the behavior, the following can be used:
TESTDIR="`mktemp -d '/tmp/make test.XX'`" && \
mkdir -p "$TESTDIR" && \
cd "$TESTDIR" && \
ln -s /bin/sh sh && \
echo -e 'all:\n\t@echo "SHELL is $(SHELL)"' > Makefile && \
make SHELL="$TESTDIR/sh"

Without the fix:
make: /tmp/make: Command not found
make: *** [Makefile:2: all] Error 127

With the fix:
SHELL is /tmp/make test.o8gS9Mba4b/sh

* src/job.c (construct_command_argv_internal): Escape space in $SHELL
---
  src/job.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/job.c b/src/job.c
index 1c66090c..0bd3f65c 100644
--- a/src/job.c
+++ b/src/job.c
@@ -3390,7 +3390,7 @@ construct_command_argv_internal (char *line, char 
**restp, const char *shell,
 whichever happens first.  */
  for (cp = shell; *cp != '\0'; ++cp)
{
-if (strchr (sh_chars, *cp) != 0)
+if (strchr (sh_chars, *cp) != 0 || ISSPACE(*cp))
*(ap++) = '\\';
  *(ap++) = *cp;
}

[PING^2] [PATCH 0/6] Fix compilation errors in maintainer mode

[Torbjorn SVENSSON]

Gentle 2nd ping.

Could anyone with write access review these patches and apply them for 
me if there is no comments on them?


Kind regards,
Torbjörn

On 2023-06-19 09:03, Torbjorn SVENSSON wrote:

Ping!

Kind regards,
Torbjörn

On 2023-06-09 16:09, Torbjörn SVENSSON wrote:

Hi,

This patch series addresses all the errors that GCC 7.3 emits when cross
building for Windows in maintainer mode.

Kind regards,
Torbjörn

Re: [PATCH 0/6] Fix compilation errors in maintainer mode

[Torbjorn SVENSSON]

Ping!

Kind regards,
Torbjörn

On 2023-06-09 16:09, Torbjörn SVENSSON wrote:

Hi,

This patch series addresses all the errors that GCC 7.3 emits when cross
building for Windows in maintainer mode.

Kind regards,
Torbjörn

Re: [PATCH 1/6] Fix GCC compile warning for "declaration-after-statement"

[Torbjorn SVENSSON]

On 2023-06-09 16:53, Martin Dorey wrote:

Primed by our earlier discussion, I wondered "How does that fix the 
bad-function-cast warning?", before belatedly noticing the Subject.


This particular patch does not address the bad cast, it's addressed in patch 6 
of the series.



--
*From:* bug-make-bounces+martin.dorey=hds@gnu.org 
 on behalf of Torbjörn SVENSSON 

*Sent:* Friday, June 9, 2023 07:39
*To:* bug-make@gnu.org 
*Cc:* Torbjörn SVENSSON 
*Subject:* [PATCH 1/6] Fix GCC compile warning for "declaration-after-statement"
* EXTERNAL EMAIL *

Contributed by STMicroelectronics

Signed-off-by: Torbjörn SVENSSON 
---
  src/w32/w32os.c | 5 +++--
  1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/w32/w32os.c b/src/w32/w32os.c
index 33828010..015b685e 100644
--- a/src/w32/w32os.c
+++ b/src/w32/w32os.c
@@ -35,6 +35,7 @@ unsigned int
  check_io_state ()
  {
    static unsigned int state = IO_UNKNOWN;
+  HANDLE outfd, errfd;

    /* We only need to compute this once per process.  */
    if (state != IO_UNKNOWN)
@@ -42,8 +43,8 @@ check_io_state ()

    /* Could have used GetHandleInformation, but that isn't supported
   on Windows 9X.  */
-  HANDLE outfd = (HANDLE)_get_osfhandle (fileno (stdout));
-  HANDLE errfd = (HANDLE)_get_osfhandle (fileno (stderr));
+  outfd = (HANDLE)_get_osfhandle (fileno (stdout));
+  errfd = (HANDLE)_get_osfhandle (fileno (stderr));

    if ((HANDLE)_get_osfhandle (fileno (stdin)) != INVALID_HANDLE_VALUE)
  state |= IO_STDIN_OK;
--
2.25.1

Re: Unable to cross build for Windows

[Torbjorn SVENSSON]

On 2023-06-09 09:11, Eli Zaretskii wrote:

From: Martin Dorey 
CC: "bug-make@gnu.org" 
Date: Fri, 9 Jun 2023 06:32:28 +
msip_labels:

#include 

intptr_t _get_osfhandle(int);
typedef void* HANDLE;

HANDLE fn() {
   return (HANDLE)_get_osfhandle(0);
}
martind@sirius:~/tmp/svensson-2023-06-08$ gcc -c -Wbad-function-cast make.c
make.c: In function ‘fn’:
make.c:7:10: warning: cast from function call of type ‘intptr_t {aka long int}’ 
to non-matching type ‘void *’ [-Wbad-function-cast]
    return (HANDLE)_get_osfhandle(0);
           ^
martind@sirius:~/tmp/svensson-2023-06-08$

That's gcc-6.3, but it's much the same in every version I tested from gcc-4.4 
to gcc-10.  A random version's man page, 
https://gcc.gnu.org/onlinedocs/gcc-9.1.0/gcc/Warning-Options.html, explains the 
flag as to:


Warn when a function call is cast to a non-matching type. For example, warn if 
a call to a function returning an integer type is cast to a pointer type.


I fear they mean exactly what they say there.  So, while you might call it a 
mistake, I don't think it's an accident.  
https://stackoverflow.com/a/35308713/18096 has some explanation of why it might 
sometimes be useful... and how it could be sidestepped by a change to Gnu Make, 
like our new friend wished for.


Yes, it sounds like -Wbad-function-cast will always emit a warning in
such cases, and one cannot use it with the likes of _get_osfhandle,
which _require_ such casts.

You can, of course, work around it; the following compiles without a
warning:

#include 

intptr_t _get_osfhandle(int);
typedef void* HANDLE;

HANDLE fn() {
   intptr_t handle = _get_osfhandle(0);
   return (HANDLE)handle;
}


Okay, lets go this way to fix the warning to allow building with 
-Wbad-function-cast.

The only thing left, that I don't know how to handle, is the use of O() where 
the 3rd parameter is not a string literal.

src/job.c: In function 'create_batch_file':
src/job.c:365:3: error: format not a string literal and no format arguments 
[-Werror=format-security]
   O (fatal, NILF, error_string);
   ^

is it okay to change this to: fatal (NILF, 0, error_string);
or is there a better way to get around this warning?

Re: Unable to cross build for Windows

[Torbjorn SVENSSON]

On 2023-06-09 14:44, Paul Smith wrote:

On Fri, 2023-06-09 at 14:16 +0200, Torbjorn SVENSSON wrote:

The only thing left, that I don't know how to handle, is the use of
O() where the 3rd parameter is not a string literal.

src/job.c: In function 'create_batch_file':
src/job.c:365:3: error: format not a string literal and no format
arguments [-Werror=format-security]
     O (fatal, NILF, error_string);
     ^

is it okay to change this to: fatal (NILF, 0, error_string);
or is there a better way to get around this warning?


I don't see how that change would get around this warning.  The result
after preprocessing is the same.  Did you try this and see the warning
go away?  That would be extremely odd.

To get rid of this warning you have to use this:

   OS (fatal, NILF, "%s", error_string);




Right, I missed to type that in my call to fatal, sorry for that.
I've sent a patch series that addresses the errors that was blocking me
from building in maintainer mode.

Re: Unable to cross build for Windows

[Torbjorn SVENSSON]

On 2023-06-08 17:05, Eli Zaretskii wrote:

Date: Thu, 8 Jun 2023 16:19:04 +0200
CC: 
From: Torbjorn SVENSSON 

On 2023-06-08 16:14, Eli Zaretskii wrote:

Date: Thu, 8 Jun 2023 14:44:35 +0200
From: Torbjorn SVENSSON 

/build/gnu-make_4.4.1-45-g07fcee35/src/function.c: In function 
'windows32_openpipe':
/build/gnu-make_4.4.1-45-g07fcee35/src/function.c:1676:12: error: cast from 
function call of type 'intptr_t {aka long long int}' to non-matching type 'void 
*' [-Werror=bad-function-cast]
  tmpErr = (HANDLE)_get_osfhandle (errfd);
   ^


/build/gnu-make_4.4.1-45-g07fcee35/src/job.c: In function 'create_batch_file':
/build/gnu-make_4.4.1-45-g07fcee35/src/job.c:365:3: error: format not a string 
literal and no format arguments [-Werror=format-security]
  O (fatal, NILF, error_string);
  ^


On the MSDN page for _get_osfhandle, they recommend to case the return value
to HANDLE, but GCC apparently thinks this is not allowed.
Any idea on how to get around this issue?


For the O()-macro; is it correct to use the macro with a variable?
Maybe it's more appropriate to fall fatal directly with the arguments?


Is this with x86_64 being the target or i686?  I'm guessing the
latter, since for 64-bit Windows 'long long' and 'void *' are of the
same width.  If it's indeed for i686, are you sure the MinGW64 headers
you have support 32-bit builds?  Because AFAIR MinGW64 tossed
supported for old Windows versions, which basically means they don't
support 32-bit Windows.


The idea is to have a 64-bit make application. I just included the i686
log for completeness.


If you get the error about casting _get_osfhandle to HANDLE only for
the 32-bit build, and you don't care about that build, then just
ignore it.  But if that error is emitted for the 64-bit build, then
there's something strange here, since on 64-bit Windows 'long long'
should be compatible with 'void *'.


The error is emitted in both builds, but let's ignore 32-bit for now.
Both the errors that I pasted above are emitted using
x86_64-w64-mingw32 and need to have some kind of fix to have a successful
build.

Any idea what could be done except disabling the warning for the cast.

Re: Unable to cross build for Windows

[Torbjorn SVENSSON]

On 2023-06-08 16:14, Eli Zaretskii wrote:

Date: Thu, 8 Jun 2023 14:44:35 +0200
From: Torbjorn SVENSSON 

/build/gnu-make_4.4.1-45-g07fcee35/src/function.c: In function 
'windows32_openpipe':
/build/gnu-make_4.4.1-45-g07fcee35/src/function.c:1676:12: error: cast from 
function call of type 'intptr_t {aka long long int}' to non-matching type 'void 
*' [-Werror=bad-function-cast]
 tmpErr = (HANDLE)_get_osfhandle (errfd);
  ^


/build/gnu-make_4.4.1-45-g07fcee35/src/job.c: In function 'create_batch_file':
/build/gnu-make_4.4.1-45-g07fcee35/src/job.c:365:3: error: format not a string 
literal and no format arguments [-Werror=format-security]
 O (fatal, NILF, error_string);
 ^


On the MSDN page for _get_osfhandle, they recommend to case the return value
to HANDLE, but GCC apparently thinks this is not allowed.
Any idea on how to get around this issue?


For the O()-macro; is it correct to use the macro with a variable?
Maybe it's more appropriate to fall fatal directly with the arguments?


Is this with x86_64 being the target or i686?  I'm guessing the
latter, since for 64-bit Windows 'long long' and 'void *' are of the
same width.  If it's indeed for i686, are you sure the MinGW64 headers
you have support 32-bit builds?  Because AFAIR MinGW64 tossed
supported for old Windows versions, which basically means they don't
support 32-bit Windows.


The idea is to have a 64-bit make application. I just included the i686
log for completeness.

If I can get a build going for x86_64-w64-mingw32, I would be okay with
ignoring whatever problems that are unique to the i686-w64-mingw32 target.