Re: Regression on Cygwin: Problems with parallel make in 4.4

[Paul Smith]

On Mon, 2023-02-20 at 20:21 -0500, Ken Brown wrote:
> Parallel make is still not working reliably.  I've just discovered
> that my TeX Live build logs have several occurrences of the following
> warning:
> 
>    jobserver unavailable: using -j1.  Add '+' to parent make rule.
> 
> This has been going on since I first started using 4.4.0.90 with the 
> jobserver style set to "pipe".  It *only* occurs when the jobserver
> uses a pipe.  Two others in the Cygwin community are reporting the
> same thing when they build projects using cmake (with Unix Makefiles
> and make 4.4.0.91).

Just to note, I do run the regression test suite with the equivalent of
"jobserver-style=pipe" (basically I force the configure to believe that
the system doesn't support mkfifo() so it falls back to "pipe" mode).

Can you run builds with GNU Make 4.4 and --jobserver-style=pipe?  Do
you see the same warnings there?  I just want to know if the problem
happened in 4.4 or post-4.4.  If the latter, that's very odd.  If the
former, it's more understandable because we did make the "pipe" mode
more strict in 4.4.

This message means only one thing: the parent make didn't provide the
proper environment (that is, the correct file descriptors) to its sub-
make.  However, there could be different ways in which the environment
is wrong.

If you have the ability, it would be helpful if you could apply the
attached patch to your build of GNU Make and try the build again, and
let me know which of these different messages are generated right
before the above error.

It would also be very helpful if someone could provide me with the
complete rule that the parent make uses to invoke the sub-make, where
that message appears.  And, if you can include the output that make
generates to invoke the submake (the command it prints) that would be
excellent as well.
diff --git a/src/posixos.c b/src/posixos.c
index 66f6e49c..39b4a3ec 100644
--- a/src/posixos.c
+++ b/src/posixos.c
@@ -116,6 +116,9 @@ make_job_rfd ()
   EINTRLOOP (job_rfd, dup (job_fds[0]));
   if (job_rfd >= 0)
 fd_noinherit (job_rfd);
+  else
+ONS (error, NILF,
+ "cannot dup jobserver FD %d: %s", job_fds[0], strerror (errno));
 
   return job_rfd;
 #endif
@@ -256,11 +259,18 @@ jobserver_parse_auth (const char *auth)
 {
   /* The parent overrode our FDs because we aren't a recursive make.  */
   if (rfd == -2 || wfd == -2)
-return 0;
+{
+  ONN (error, NILF, "not recursive %d,%d", rfd, wfd);
+  return 0;
+}
 
   /* Make sure our pipeline is valid.  */
   if (!FD_OK (rfd) || !FD_OK (wfd))
-return 0;
+{
+  error (NILF, strlen (strerror (errno)) + INTSTR_LENGTH*2,
+ "not OK %d,%d: %s", rfd, wfd, strerror (errno));
+  return 0;
+}
 
   job_fds[0] = rfd;
   job_fds[1] = wfd;

[bug #63821] Switch -r fails to disable default suffix rules.

[Paul D. Smith]

Update of bug #63821 (project make):

  Status:None => Fixed  
 Open/Closed:Open => Closed 
   Component Version: SCM => 4.3
   Fixed Release:None => SCM
   Triage Status:None => Medium Effort  

___

Follow-up Comment #6:

Hm, I didn't see your reply before I pushed my changes.  We can reconsider
it.

I see what you're saying, it's just that I always consider that all rules
should be defined before we snap_deps(): once snap_deps() happens we're not
allowed to manipulate the DAG (other than adding implicit rule matches).

Probably it doesn't matter either way in this case.


___

Reply to this item at:

  

___
Message sent via Savannah
https://savannah.gnu.org/

[bug #63821] Switch -r fails to disable default suffix rules.

[Dmitry Goncharov]

Follow-up Comment #5, bug #63821 (project make):

> The title is slightly misleading

Hopefully, subsequent description made it clear.

> Curious why you decided to install the default suffix rules after
snap_deps() @dgoncharov .

i wanted to install default suffix rules as close to convert_to_pattern as
possible. i think, it makes it easier to follow the logic in main.
install_default_suffix_rules, convert_to_pattern,
install_default_implicit_rules all belong together.

>  That doesn't feel right to me,

Are you thinking about the prior impl of snap_deps? We modified snap_deps and
removed that code that expanded prerequisites of .SUFFIXES. See
07eea3aa496184bb763b7306e9de6a40a94605c9.
Or do you see a use case that requires install_default_suffix_rules before
snap_deps?

> was there something that didn't work right if you did it between defining
the makeflags and snap_deps(), rather than after?

No. Either way it works the same, as far as i can see.


___

Reply to this item at:

  

___
Message sent via Savannah
https://savannah.gnu.org/

Re: Regression on Cygwin: Problems with parallel make in 4.4

[Ken Brown]

On 2/19/2023 9:29 AM, Paul Smith wrote:

I will change the default setting of the jobserver to use "pipe" on
Cygwin, at least for now.


Parallel make is still not working reliably.  I've just discovered that 
my TeX Live build logs have several occurrences of the following warning:


  jobserver unavailable: using -j1.  Add '+' to parent make rule.

This has been going on since I first started using 4.4.0.90 with the 
jobserver style set to "pipe".  It *only* occurs when the jobserver uses 
a pipe.  Two others in the Cygwin community are reporting the same thing 
when they build projects using cmake (with Unix Makefiles and make 
4.4.0.91).


I don't know if this is Cygwin-specific, since other platforms (except 
GNU/Hurd) are using fifos.  So the "pipe" jobserver style might not have 
gotten a lot of testing since the switch to fifos was made.


I guess the first step in tracking this down is for people working on 
other platforms to see if they can reproduce the problem when they set 
the jobserver style to "pipe".


Unfortunately, I don't have a simple reproduction recipe.  I myself have 
seen it when building TeX Live, as I've said, and also when building 
doxygen (which uses cmake with Unix Makefiles).  The other Cygwin users 
who have reported the problem (both CC'd) haven't yet said what cmake 
projects they were building.


Ken

[bug #63821] Switch -r fails to disable default suffix rules.

[Paul D. Smith]

Follow-up Comment #4, bug #63821 (project make):

Curious why you decided to install the default suffix rules after snap_deps()
@dgoncharov .  That doesn't feel right to me, was there something that didn't
work right if you did it between defining the makeflags and snap_deps(),
rather than after?


___

Reply to this item at:

  

___
Message sent via Savannah
https://savannah.gnu.org/

RE: Handling references to invalid variables

[rsbecker]

On Monday, February 20, 2023 2:50 PM, Paul Smith wrote:
>On Mon, 2023-02-20 at 14:20 -0500, rsbec...@nexbridge.com wrote:
>> I think you need to be able to return to a compatible mode for some
>> users. Having an option like --undefined-variables=warn or --
>> undefined-variables=error (the default) or --undefined-
>> variables=ignore would be prudent.
>
>Hm.  I'm not sure about the "ignore" option.  I'm not a fan of adding lots of 
>options, it
>just makes things that much more complicated to use and test.  Is it important 
>to
>allow these checks to be completely ignored?  If the default is to leave them 
>as
>warnings they won't cause make to fail or change what it will build.

I added =ignore as an afterthought. But build engineers tend to like things to 
stay they way they are for presentation purposes. If new messages show up in 
build streams, there will be questions and inquiries for centuries (cryptic Q 
reference). =ignore would allow the output to stay the same as it is now.

Re: Handling references to invalid variables

[Paul Smith]

On Mon, 2023-02-20 at 14:20 -0500, rsbec...@nexbridge.com wrote:
> I think you need to be able to return to a compatible mode for some
> users. Having an option like --undefined-variables=warn or --
> undefined-variables=error (the default) or --undefined-
> variables=ignore would be prudent.

Hm.  I'm not sure about the "ignore" option.  I'm not a fan of adding
lots of options, it just makes things that much more complicated to use
and test.  Is it important to allow these checks to be completely
ignored?  If the default is to leave them as warnings they won't cause
make to fail or change what it will build.

RE: Handling references to invalid variables

[rsbecker]

On February 20, 2023 2:11 PM, Paul Smith wrote:
>In the next major release (not the upcoming 4.4.1 release but the one after 
>that) I
>plan to implement notifying users of invalid variable references; for example
>variable names containing whitespace.
>
>So, a makefile like this for example:
>
>  all: ; echo $(cat foo)
>
>will notify the user about the illegal variable reference "cat foo", instead 
>if silently
>expanding to the empty string.
>
>My intent is that this is always enabled, not requiring an extra option like 
>--warn-
>undefined-variables, since it's never legal to have a variable name containing
>whitespace.
>
>The question is, should this notification be a warning?  Or should it be a 
>fatal error?
>Originally I thought it should be fatal but now I'm leaning towards making it a
>warning, at least for a release or two, because I worry about makefiles that 
>might
>have these references that are silently and innocuously expanding to empty 
>strings,
>suddenly stop working completely.

Having worked on (insert large number) of build engines, there are likely users 
who depend on bad or empty strings behaving in a particular way (prior to this 
feature). I like your idea of turning this detection on by default from a 
personal standpoint; however, I think you need to be able to return to a 
compatible mode for some users. Having an option like 
--undefined-variables=warn or --undefined-variables=error (the default) or 
--undefined-variables=ignore would be prudent.

Just my $0.02
--Randall

--
Brief whoami: NonStop&UNIX developer since approximately
UNIX(421664400)
NonStop(2112884442)
-- In real life, I talk too much.

Re: Handling references to invalid variables

[David A. Wheeler]

> On Feb 20, 2023, at 2:11 PM, Paul Smith  wrote:
> 
> In the next major release (not the upcoming 4.4.1 release but the one
> after that) I plan to implement notifying users of invalid variable
> references; for example variable names containing whitespace.
> ...
> 
> The question is, should this notification be a warning?  Or should it
> be a fatal error?

Definitely start by making it a warning. I have no idea how common they are,
but they're probably more common than we'd like, so people will need time
to fix the problems it reveals.

--- David A. Wheeler

Handling references to invalid variables

[Paul Smith]

In the next major release (not the upcoming 4.4.1 release but the one
after that) I plan to implement notifying users of invalid variable
references; for example variable names containing whitespace.

So, a makefile like this for example:

  all: ; echo $(cat foo)

will notify the user about the illegal variable reference "cat foo",
instead if silently expanding to the empty string.

My intent is that this is always enabled, not requiring an extra option
like --warn-undefined-variables, since it's never legal to have a
variable name containing whitespace.

The question is, should this notification be a warning?  Or should it
be a fatal error?  Originally I thought it should be fatal but now I'm
leaning towards making it a warning, at least for a release or two,
because I worry about makefiles that might have these references that
are silently and innocuously expanding to empty strings, suddenly stop
working completely.

[bug #63821] Switch -r fails to disable default suffix rules.

[Paul D. Smith]

Follow-up Comment #3, bug #63821 (project make):

The title is slightly misleading because MAKEFLAGS += -r DOES disable default
suffix rules, it just doesn't remove them if you add back the extensions to
the .SUFFIXES target.

See this test from tests/scripts/options/dash-r which passes:


# Make sure we can set it from within the makefile too
run_make_test(q!
COMPILE.c = echo
MAKEFLAGS += -r
!,
  'xxx.o',
  "#MAKE#: *** No rule to make target 'xxx.o'.  Stop.", 512);


Since we don't set .SUFFIXES we get the answer we want.  I do agree that your
change is needed though: we shouldn't "recover" the rules by setting .SUFFIXES
without redefining them.


___

Reply to this item at:

  

___
Message sent via Savannah
https://savannah.gnu.org/

[bug #63821] Switch -r fails to disable default suffix rules.

[Dmitry Goncharov]

Follow-up Comment #2, bug #63821 (project make):

A correction: After parsing makefiles the patch appends the default suffix
rules to suffix_file->deps only if there is no user defined rules in the
makefile.


___

Reply to this item at:

  

___
Message sent via Savannah
https://savannah.gnu.org/

[bug #63821] Switch -r fails to disable default suffix rules.

[Dmitry Goncharov]

Additional Item Attachment, bug #63821 (project make):

File name: sv63821_fix.diff   Size:3 KB


File name: sv63821_test.diff  Size:2 KB




___

Reply to this item at:

  

___
Message sent via Savannah
https://savannah.gnu.org/

[bug #63821] Switch -r fails to disable default suffix rules.

[Dmitry Goncharov]

Follow-up Comment #1, bug #63821 (project make):

Setting -r in MAKEFLAGS in makefile fails to the disable default suffix
rules.


$ ls
hello.c  makefile
$ cat makefile
MAKEFLAGS+=-r
.SUFFIXES: .c .o
all: hello.o
$ make-4.4
cc-c -o hello.o hello.c
$
$ rm hello.o 
$ make-4.4 -r
make-4.4: *** No rule to make target 'hello.o', needed by 'all'.  Stop.
$


This happens, because make install default suffix rules before parsing
makefiles. Once the files are added they stay in hash table "files" and also
in suffix_file->deps. Make then creates pattern rules from them.
The patch moves setup of default suffix rules after parsing makefiles. The
patch also prepends suffix rules defined in the makefile before the default
suffix rules in suffix_file->deps in order to give user defined rules
precedence over the default rules.

This ensures that when built-in rules are disabled in the makefile, but
suffixes are added to .SUFFIXES, make exits with the 'No rule...' error
message. Just like when -r is specified on the command line.
This also relieves make from creating and then destroying file objects for
default suffix rules and keeps hash table "files" free of the related entries
when built-in rules are disabled in the makefile.



___

Reply to this item at:

  

___
Message sent via Savannah
https://savannah.gnu.org/

[bug #63821] Switch -r fails to disable default suffix rules.

[Dmitry Goncharov]

URL:
  

 Summary: Switch -r fails to disable default suffix rules.
   Group: make
   Submitter: dgoncharov
   Submitted: Mon 20 Feb 2023 05:52:08 PM UTC
Severity: 3 - Normal
  Item Group: Bug
  Status: None
 Privacy: Public
 Assigned to: None
 Open/Closed: Open
 Discussion Lock: Any
   Component Version: SCM
Operating System: Any
   Fixed Release: None
   Triage Status: None


___

Follow-up Comments:


---
Date: Mon 20 Feb 2023 05:52:08 PM UTC By: Dmitry Goncharov 
.







___

Reply to this item at:

  

___
Message sent via Savannah
https://savannah.gnu.org/

Re: GNU Make 4.4.0.91 on Cygwin

[Eli Zaretskii]

> From: Paul Smith 
> Cc: br...@clisp.org, bug-make@gnu.org
> Date: Mon, 20 Feb 2023 10:36:07 -0500
> 
> Of course "testing on Windows" can mean many different things as there
> are so many possible environments "on Windows".  I only run one Windows
> environment: (a) Windows 10/11 in a VM, (b) using Visual Studio as the
> compiler, (c) using .\build_w32.bat in cmd.exe to do the build, (d)
> running tests from within cmd.exe via ".\WinRel\gnumake.exe check", (e)
> with Git for Windows bin directory on my PATH so I have access to a
> bunch of POSIX tools during the test run.
> 
> It definitely doesn't fail for me in that configuration.
> 
> > Maybe it could work if you link Make statically, or if you copy the
> > dependency DLLs into a system directory where Windows looks
> > regardless of PATH.  But in general, emptying PATH on Windows is not
> > very useful.
> 
> That's missing the point of the test.  Even on POSIX it's useless to
> start a process with an empty PATH, but that's not what the test is
> for.  See https://savannah.gnu.org/bugs/index.php?57674 to read the
> background: on a POSIX system if PATH is not set a default PATH is used
> when we run execvp() and that's what this is testing.
> 
> I don't really know what happens in this situation on Windows, but
> something must happen.

Is your VS-built Make statically linked, or does it have dependencies
on DLLs, such as Guile?  If the latter, in which directories are those
DLLs kept?

My problem with an empty PATH is that the dependency DLLs cannot be
found, and Make simply won't start, because the Windows loader won't
let it.  And even if I work around this by copying the DLLs to the
same directory as the Make executable, there's no point to this test
on Windows, since there's no "default PATH used by execvp" on
MS-Windows.  Moreover, execvp on Windows is emulated (quite poorly,
btw) anyway.

IOW, this Posix feature makes no sense on Windows, so I decided long
ago not to try too hard to run this test.

> It's possible that it works for me because I'm using cmd.exe as the
> shell, and echo is a shell built-in there, and so the fact that PATH is
> empty is irrelevant.

I also use cmd.exe, I just don't have any Git-related Unix programs on
PATH, including Bash and MSYS executables that come with it.  The test
suite runs Perl, so it is very much possible that in your environment,
the Perl that runs is an MSYS perl, not a native Windows perl, in
which case what you get is the Cygwin emulation of Posix (since MSYS
is nothing other than a fork of Cygwin).

In my environment, the perl being used for the test suite is a native
Windows perl (ActiveState Perl).  And any Unix tools the test suite
uses, like 'touch' etc. are all native Windows builds, not MSYS
builds.

Re: GNU Make 4.4.0.91 on Cygwin

[Paul Smith]

On Mon, 2023-02-20 at 14:47 +0200, Eli Zaretskii wrote:
> How do you conclude that this "works on Windows"?

Before I make a release I test on various systems and the regression
test suite must pass with no failures.  One of the systems I test on is
Windows, so if a release comes out and a test is not conditionalized to
not run on that system in that release, you can assume it worked for
me.

Of course "testing on Windows" can mean many different things as there
are so many possible environments "on Windows".  I only run one Windows
environment: (a) Windows 10/11 in a VM, (b) using Visual Studio as the
compiler, (c) using .\build_w32.bat in cmd.exe to do the build, (d)
running tests from within cmd.exe via ".\WinRel\gnumake.exe check", (e)
with Git for Windows bin directory on my PATH so I have access to a
bunch of POSIX tools during the test run.

It definitely doesn't fail for me in that configuration.

> Maybe it could work if you link Make statically, or if you copy the
> dependency DLLs into a system directory where Windows looks
> regardless of PATH.  But in general, emptying PATH on Windows is not
> very useful.

That's missing the point of the test.  Even on POSIX it's useless to
start a process with an empty PATH, but that's not what the test is
for.  See https://savannah.gnu.org/bugs/index.php?57674 to read the
background: on a POSIX system if PATH is not set a default PATH is used
when we run execvp() and that's what this is testing.

I don't really know what happens in this situation on Windows, but
something must happen.

It's possible that it works for me because I'm using cmd.exe as the
shell, and echo is a shell built-in there, and so the fact that PATH is
empty is irrelevant.

Re: GNU Make 4.4.0.91 on Cygwin

[Eli Zaretskii]

> From: Paul Smith 
> Date: Sun, 19 Feb 2023 14:59:35 -0500
> 
> On Sun, 2023-02-19 at 20:32 +0100, Bruno Haible wrote:
> > All "Device or resource busy" failures are gone. Only the 1 failure
> > in category 'misc/general4' is still present.
> 
> Hm.  This is a test of this:
> https://savannah.gnu.org/bugs/index.php?57674
> 
> which explicitly removes PATH from the environment completely.  It
> should use a default PATH, which should mean "echo" works even so.  I'm
> not really sure how this should react on a cygwin system, but I guess
> since it behaves properly on Windows and POSIX it should work on cygwin
> as well.  I'll have to figure out why only cygwin isn't working.

How do you conclude that this "works on Windows"?  In my testing, it
doesn't, and I therefore disable this test on Windows:

  # SV 57674: ensure we use a system default PATH if one is not set
  # This cannot work on MS-Windows, because PATH is used to find
  # shared libraries against which Make was built.
  if ($port_type ne 'W32') {
  delete $ENV{PATH};
  run_make_test(q!
  a: ; @echo hi
  !,
'', "hi\n");
  }

Maybe it could work if you link Make statically, or if you copy the
dependency DLLs into a system directory where Windows looks regardless
of PATH.  But in general, emptying PATH on Windows is not very useful.