Re: CVS commit: src/usr.bin/make

[Martin Husemann]

On Tue, Jan 26, 2021 at 11:39:52PM +0100, Roland Illig wrote:
> The code of usr.bin/make gets distributed to a wider audience by Simon's
> bmake distribution (http://www.crufty.net/help/sjg/bmake.html), that's
> where the requirement of supporting C89 compilers comes from.  At the
> time I committed this fix, Simon had managed to dig out an old Solaris 9
> installation with GCC, and these few changes were the only ones needed
> to let bmake run on that platform.  That sounded easy enough to me.

Also note that we need bmake during bootstrap of pkgsrc, and besides
finding a working compiler it is one of the early things you need to
make work on an ancient platform if you try to bring it to new use (I
have been there with Solaris 2.6 at one point, but gave up for other
reasons - and of course the machine in question now runs NetBSD (again)
[the Solaris adventure was a temporary thing anyway and as pkgsrc did
not help as a "plug and play" way to get a usable dev/debug environment
it was not worth pushing]).

But I must admit that the *commit log* of that change sounded way more scary
than the actual change is:

replace %zu with %u in printf calls

would be plain wrong and of course break (either at runtime or if lucky
at compile time) many, many platforms.

But

-   (void)fprintf(f, "\"%s\" line %zu: ", fname, lineno);
+   (void)fprintf(f, "\"%s\" line %u: ", fname, (unsigned)lineno);

is ok for portable code and "lineno" referencing (I guess) a makefile.
It could have been (long unsigned) and "%lu", but maybe portability to
systems where that would make a difference is a bit far stretched.

Martin

Re: CVS commit: src/usr.bin/make

[Roland Illig]

On 26.01.2021 21:04, Christos Zoulas wrote:

Yes, and I did not push for it for the exact reasons stated below:
There was only a handful of cases and those can be handled with casts or a 
macro for now.

I am questioning though the utility of maintaining compatibility with platforms 
that
do not support C99 20 years later, vs. using %u and casting or using a 
formatting macro.


The support for ancient platforms is not only with regards to "%zu" vs.
"%u", in util.c there is even support for platforms that lack getenv().
 Apparently nobody cared enough to remove these bits, even though
getenv is guaranteed to exist since C89 already.

The code of usr.bin/make gets distributed to a wider audience by Simon's
bmake distribution (http://www.crufty.net/help/sjg/bmake.html), that's
where the requirement of supporting C89 compilers comes from.  At the
time I committed this fix, Simon had managed to dig out an old Solaris 9
installation with GCC, and these few changes were the only ones needed
to let bmake run on that platform.  That sounded easy enough to me.

Roland

Re: CVS commit: src/usr.bin/make

[Christos Zoulas]

Yes, and I did not push for it for the exact reasons stated below:
There was only a handful of cases and those can be handled with casts or a 
macro for now.

I am questioning though the utility of maintaining compatibility with platforms 
that
do not support C99 20 years later, vs. using %u and casting or using a 
formatting macro.

christos

> On Jan 26, 2021, at 2:00 PM, Roland Illig  wrote:
> 
> On 26.01.2021 11:19, Rin Okuyama wrote:
>> Ping?
>> 
>> I don't think this is correct fix either.
>> Can you please revert this commit?
> 
> Sorry, I didn't get the first mail from Christos back in December,
> that's why I didn't take any action.
> 
> Why shouldn't the fix I did be correct?  I carefully checked the few
> places where I added the casts, and none of the numbers that are
> involved will ever be more than a billion.
> 
> 1.  The number of variables in a .for loop
> 
> 2.  The number of items in a .for loop
> 
> 3.  A single line of a text file in meta mode.  (Only in debug mode.)
> 
> 4.  The number of lines in a single makefile.
> 
> 5.  The capturing subexpression in a :C variable modifier, which only
> ever ranges from 0 to 9.
> 
> 6.  The number of words into which a variable value is split. (Only in
> debug mode.)
> 
> Given these, why should a simple %u not be appropriate?  I don't want to
> clutter the code with another preprocessor macro, therefore I'd like to
> keep the code the way it is right now.
> 
> I also don't see why this could ever have the slightest chance of
> "breaking everyone else".
> 
> For reference and easy viewing, here is the corresponding commit on
> GitHub:
> https://github.com/NetBSD/src/commit/ff11c7d3497a40c90ec70101ad72612e2f0884b2
> 
> Roland
> 
>> On 2020/12/15 7:57, Christos Zoulas wrote:
>>> In article <20201213212746.3cfc3f...@cvs.netbsd.org>,
>>> Roland Illig  wrote:
 -=-=-=-=-=-
 
 Module Name:src
 Committed By:rillig
 Date:Sun Dec 13 21:27:46 UTC 2020
 
 Modified Files:
 src/usr.bin/make: for.c meta.c parse.c var.c
 
 Log Message:
 make(1): replace %zu with %u in printf calls
 
 This is needed to compile bmake with GCC 2.8.1 on SunOS 5.9.
 
 To support ancient systems like this, the whole code of usr.bin/make is
 supposed to use only ISO C90 features, except for filemon, which is not
 used on these systems.
>>> 
>>> Please revert! This breaks everyone else. %zu is the format to
>>> print size_t.  Last I checked SunOS 5.9 has been dead since 2014
>>> and whoever is still using it might as well install a new compiler,
>>> or tie the box at the end of a long chain so it can find its true
>>> calling. If you really want to support it instead define MAKE_FMT_SIZE_T
>>> and conditionalize it properly for "ancient OS", windows, cygwin,
>>> mingwin, and regular folks (this does not even handle "ancient os"):
>>> 
>>>  https://github.com/file/file/blob/master/src/file.h#L55
>>> 
>>> I am still against it though...
>>> 
>>> christos
>>> 



signature.asc
Description: Message signed with OpenPGP

Re: CVS commit: src/usr.bin/make

[Roland Illig]

On 26.01.2021 11:19, Rin Okuyama wrote:

Ping?

I don't think this is correct fix either.
Can you please revert this commit?


Sorry, I didn't get the first mail from Christos back in December,
that's why I didn't take any action.

Why shouldn't the fix I did be correct?  I carefully checked the few
places where I added the casts, and none of the numbers that are
involved will ever be more than a billion.

1.  The number of variables in a .for loop

2.  The number of items in a .for loop

3.  A single line of a text file in meta mode.  (Only in debug mode.)

4.  The number of lines in a single makefile.

5.  The capturing subexpression in a :C variable modifier, which only
ever ranges from 0 to 9.

6.  The number of words into which a variable value is split. (Only in
debug mode.)

Given these, why should a simple %u not be appropriate?  I don't want to
clutter the code with another preprocessor macro, therefore I'd like to
keep the code the way it is right now.

I also don't see why this could ever have the slightest chance of
"breaking everyone else".

For reference and easy viewing, here is the corresponding commit on
GitHub:
https://github.com/NetBSD/src/commit/ff11c7d3497a40c90ec70101ad72612e2f0884b2

Roland


On 2020/12/15 7:57, Christos Zoulas wrote:

In article <20201213212746.3cfc3f...@cvs.netbsd.org>,
Roland Illig  wrote:

-=-=-=-=-=-

Module Name:    src
Committed By:    rillig
Date:    Sun Dec 13 21:27:46 UTC 2020

Modified Files:
src/usr.bin/make: for.c meta.c parse.c var.c

Log Message:
make(1): replace %zu with %u in printf calls

This is needed to compile bmake with GCC 2.8.1 on SunOS 5.9.

To support ancient systems like this, the whole code of usr.bin/make is
supposed to use only ISO C90 features, except for filemon, which is not
used on these systems.


Please revert! This breaks everyone else. %zu is the format to
print size_t.  Last I checked SunOS 5.9 has been dead since 2014
and whoever is still using it might as well install a new compiler,
or tie the box at the end of a long chain so it can find its true
calling. If you really want to support it instead define MAKE_FMT_SIZE_T
and conditionalize it properly for "ancient OS", windows, cygwin,
mingwin, and regular folks (this does not even handle "ancient os"):

 https://github.com/file/file/blob/master/src/file.h#L55

I am still against it though...

christos

Re: CVS commit: src/usr.bin/make

[Rin Okuyama]

Ping?

I don't think this is correct fix either.
Can you please revert this commit?

Thanks,
rin

On 2020/12/15 7:57, Christos Zoulas wrote:

In article <20201213212746.3cfc3f...@cvs.netbsd.org>,
Roland Illig  wrote:

-=-=-=-=-=-

Module Name:src
Committed By:   rillig
Date:   Sun Dec 13 21:27:46 UTC 2020

Modified Files:
src/usr.bin/make: for.c meta.c parse.c var.c

Log Message:
make(1): replace %zu with %u in printf calls

This is needed to compile bmake with GCC 2.8.1 on SunOS 5.9.

To support ancient systems like this, the whole code of usr.bin/make is
supposed to use only ISO C90 features, except for filemon, which is not
used on these systems.


Please revert! This breaks everyone else. %zu is the format to
print size_t.  Last I checked SunOS 5.9 has been dead since 2014
and whoever is still using it might as well install a new compiler,
or tie the box at the end of a long chain so it can find its true
calling. If you really want to support it instead define MAKE_FMT_SIZE_T
and conditionalize it properly for "ancient OS", windows, cygwin,
mingwin, and regular folks (this does not even handle "ancient os"):

 https://github.com/file/file/blob/master/src/file.h#L55

I am still against it though...

christos

re: CVS commit: src/usr.bin/make

[matthew green]

"Roland Illig" writes:
> Module Name:  src
> Committed By: rillig
> Date: Tue Dec 22 08:10:39 UTC 2020
>
> Modified Files:
>   src/usr.bin/make: parse.c

i'm not sure which change did it (but before this one for sure),
but my builds crash early now (still trying to bootstrap nbmake):

cc -o nbmake *.o
assertion "*line_end == '\n'" failed: file "/usr/src/usr.bin/make/parse.c", 
line 2844, function "ParseGetLine"
[1]   Abort trap (core dumped) "${make}" -m ${TOP}/share/mk -s -B -f- _x_ <<...

ERROR: bomb_getmakevar MAKECONF: /tmp/nbbuild14989/nbmake failed

please fix :-)


.mrg.

Re: CVS commit: src/usr.bin/make

[Christos Zoulas]

In article <20201213212746.3cfc3f...@cvs.netbsd.org>,
Roland Illig  wrote:
>-=-=-=-=-=-
>
>Module Name:   src
>Committed By:  rillig
>Date:  Sun Dec 13 21:27:46 UTC 2020
>
>Modified Files:
>   src/usr.bin/make: for.c meta.c parse.c var.c
>
>Log Message:
>make(1): replace %zu with %u in printf calls
>
>This is needed to compile bmake with GCC 2.8.1 on SunOS 5.9.
>
>To support ancient systems like this, the whole code of usr.bin/make is
>supposed to use only ISO C90 features, except for filemon, which is not
>used on these systems.

Please revert! This breaks everyone else. %zu is the format to
print size_t.  Last I checked SunOS 5.9 has been dead since 2014
and whoever is still using it might as well install a new compiler,
or tie the box at the end of a long chain so it can find its true
calling. If you really want to support it instead define MAKE_FMT_SIZE_T
and conditionalize it properly for "ancient OS", windows, cygwin,
mingwin, and regular folks (this does not even handle "ancient os"):

https://github.com/file/file/blob/master/src/file.h#L55

I am still against it though...

christos

Re: CVS commit: src/usr.bin/make

[David Holland]

On Sun, Oct 25, 2020 at 05:37:36PM +, Simon J. Gerraty wrote:
 > Modified Files:
 >  src/usr.bin/make: main.c
 >  src/usr.bin/make/unit-tests: varmod-match-escape.exp
 > 
 > Log Message:
 > Skip reading .MAKE.DEPENDFILE if set to
 > "/dev/null" or anything starting with "no".
 > 
 > Ref: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=223564

I object to this, particularly the "no" part but also making /dev/null
magic.

This is an unnecessary workaround for a bug in freebsd's build
system.

If there's going to be a change, it should be removing the :T from the
generated reference; there is no reason to remove the directory name
from a user-supplied .MAKE.DEPENDFILE, and the default doesn't have a
directory part. That would also remove the problem freebsd's seeing,
and would be, y'know, actually correct.

(also the application of :T is undocumented as well as wrong)


kept for reference:

 > To generate a diff of this commit:
 > cvs rdiff -u -r1.388 -r1.389 src/usr.bin/make/main.c
 > cvs rdiff -u -r1.1 -r1.2 src/usr.bin/make/unit-tests/varmod-match-escape.exp

-- 
David A. Holland
dholl...@netbsd.org

Re: CVS commit: src/usr.bin/make

[Roland Illig]

On 24.09.2020 18:02, Jonathan A. Kollasch wrote:

cvs rdiff -u -r1.65 -r1.66 src/usr.bin/make/lst.h


breaks build on Debian 9:

In file included from /local/jakllsch/netbsd/src/usr.bin/make/make.h:158:0,
  from /local/jakllsch/netbsd/src/usr.bin/make/arch.c:130:
/local/jakllsch/netbsd/src/usr.bin/make/lst.h:92:5: error: unknown type name 
'uint8_t'
  uint8_t priv_useCount; /* Count of functions using the node.


Thank you.  Fixed in lst.h 1.67 from a minute ago.

Re: CVS commit: src/usr.bin/make

[Jonathan A. Kollasch]

> cvs rdiff -u -r1.65 -r1.66 src/usr.bin/make/lst.h

breaks build on Debian 9:

In file included from /local/jakllsch/netbsd/src/usr.bin/make/make.h:158:0,
 from /local/jakllsch/netbsd/src/usr.bin/make/arch.c:130:
/local/jakllsch/netbsd/src/usr.bin/make/lst.h:92:5: error: unknown type name 
'uint8_t'
 uint8_t priv_useCount; /* Count of functions using the node.
 ^~~

Re: CVS commit: src/usr.bin/make

[Roland Illig]

On 14.09.2020 21:05, Robert Elz wrote:

 Date:Mon, 14 Sep 2020 16:16:52 +
 From:"Roland Illig" 
 Message-ID:  <20200914161652.d4eb5f...@cvs.netbsd.org>

   | make(1): inline LPAREN in parse.c
   |
   | It's shorter and more readable, and the other characters don't have
   | named constants as well.

Most likely the reason for that was for parentheses matching editors.

Using '(' creates a ( that (a non C syntax savvy) editor will match
against the next (otherwise unmatched) ')' it finds.   LPAREN doesn't
have that effect.   An alternative is to add a /*)*/ comment on the line,
but that starts getting obtrusive, and difficult to justify.


I've seen all 3 variants in usr.bin/make/*.c, even in the code from
-D2020.01.01.00.00.00, before I started the big rewrite:

$ grep "'[(){}]'" *.c | wc -l
  58

$ grep -Ew 'LPAREN|RPAREN|BROPEN|BRCLOSE|PROPEN|PRCLOSE' *.c \
| wc -l
  19

$ grep '/\*[[:space:]]*[(){}][[:space:]]*\*/' *.c | wc -l
   2

Therefore I had the impression that there was no hard preference, and
that it would be ok to change this.  If it had been consistent over the
majority of the code in usr.bin/make, I would have left it that way.

Re: CVS commit: src/usr.bin/make

[Robert Elz]

Date:Mon, 14 Sep 2020 16:16:52 +
From:"Roland Illig" 
Message-ID:  <20200914161652.d4eb5f...@cvs.netbsd.org>

  | make(1): inline LPAREN in parse.c
  |
  | It's shorter and more readable, and the other characters don't have
  | named constants as well.

Most likely the reason for that was for parentheses matching editors.

Using '(' creates a ( that (a non C syntax savvy) editor will match
against the next (otherwise unmatched) ')' it finds.   LPAREN doesn't
have that effect.   An alternative is to add a /*)*/ comment on the line,
but that starts getting obtrusive, and difficult to justify.

kre

Re: CVS commit: src/usr.bin/make

[Roland Illig]

On 02.08.2020 13:06, Simon Burge wrote:
> "Roland Illig" wrote:
>
>> Module Name: src
>> Committed By:rillig
>> Date:Sun Aug  2 09:43:22 UTC 2020
>>
>> Modified Files:
>>
>>  src/usr.bin/make: var.c
>>
>> Log Message:
>>
>> make(1): use shorter local variable names
>>
>> The c in cp was redundant since the context makes it obvious that this
>> is a character pointer. In a tight loop where lots of characters are
>> compared, every letter counts.
>
> I don't understand the intent of this commit.  Are you saying the length
> of a C variable name has some sort of impact on code execution speed?

No, I know that it has absolutely no influence on execution speed.

My concern was about the ease of reading by humans.  In var.c, there are
several functions that do low-level parsing by reading the input byte by
byte and comparing the current character.  These functions are
ParseModifierPart, ApplyModifier_Defined, ApplyModifier_Match,
ModifyWord_SubstRegex.

In these functions, there are often 3 to 4 comparisons in a single line.
The comparisons all have the same left-hand side, so that expression
becomes uninteresting to the reader and should therefore be as short as
possible, to leave room for the more interesting details, which are the
right-hand sides of the comparisons.

Therefore I renamed the cp to p, since the c did not add any valuable
information to the code.  It's pretty obvious that in a comparison like
*p == '$', the p must be a pointer to a character.

Only in these very tight loops, and only because these have only a
single interesting pointer, do I prefer this short variable name.  For
comparison, in ApplyModifier_Match, in the second big loop, I named the
variables src and dst, to keep them apart.

Another reason for renaming cp and cp2 is that the make(1) source code
uses these names in a lot of places, and these names do not add any
valuable information to the reader, while a more expressive variable
name could do that very well.  Therefore I'm generally against this
variable name, especially when these variables are used as
general-purpose storage that serves 3 or 4 completely distinct purposes
during its lifetime, happily mixing between storing const char * and
char * and maybe a void * in between, followed by UNCONST to free the cp
at the end.  How should a casual reader know at the end what exactly is
freed here?  It's obviously cp, but what does this mean?

In summary: By the variable name p, I mean a moving pointer in a parsing
function, therefore it's usually the only moving pointer in that
function.  I'm using that variable name consistently for this single
purpose.  This is unlike the original make(1) code, which uses cp and
cp2 for everything.

I notice I'm getting too much into rant mode right now, therefore I'd
rather stop here. :)

Roland

Re: CVS commit: src/usr.bin/make

[Simon Burge]

"Roland Illig" wrote:

> Module Name:  src
> Committed By: rillig
> Date: Sun Aug  2 09:43:22 UTC 2020
>
> Modified Files:
>
>   src/usr.bin/make: var.c
>
> Log Message:
>
> make(1): use shorter local variable names
>
> The c in cp was redundant since the context makes it obvious that this
> is a character pointer. In a tight loop where lots of characters are
> compared, every letter counts.

I don't understand the intent of this commit.  Are you saying the length
of a C variable name has some sort of impact on code execution speed?

Cheers,
Simon.

Re: CVS commit: src/usr.bin/make

[Jared McNeill]

On Sun, 2 Aug 2020, Roland Illig wrote:


Module Name:src
Committed By:   rillig
Date:   Sun Aug  2 09:43:22 UTC 2020

Modified Files:
src/usr.bin/make: var.c

Log Message:
make(1): use shorter local variable names

The c in cp was redundant since the context makes it obvious that this
is a character pointer. In a tight loop where lots of characters are
compared, every letter counts.


I don't follow the rationale here. Can you expand on this?

Re: CVS commit: src/usr.bin/make

[Christos Zoulas]

In article <20200726200457.f2522f...@cvs.netbsd.org>,
Roland Illig  wrote:
>-=-=-=-=-=-
>
>Module Name:   src
>Committed By:  rillig
>Date:  Sun Jul 26 20:04:57 UTC 2020
>
>Modified Files:
>   src/usr.bin/make: Makefile
>
>Log Message:
>make(1): explicitly add dependencies on headers
>
>This prevents partial builds after changing a header.  The declared
>dependencies are more than strictly necessary, but that's still better
>than having inconsistent partial builds because too few dependencies are
>declared.

Isn't that what make depend is for?

christos

re: CVS commit: src/usr.bin/make

[matthew green]

> In a follow-up commit, I adjusted the build scripts, and I'm doing a
> full build right now. I'm pretty confident that everything works now
> again, and if not, I'll fix it immediately.

thanks!


.mrg.

Re: CVS commit: src/usr.bin/make

[Roland Illig]

On 26.07.2020 10:29, matthew green wrote:
> "Roland Illig" writes:
>> Module Name: src
>> Committed By:rillig
>> Date:Sun Jul 26 07:15:26 UTC 2020
>>
>> Modified Files:
>>  src/usr.bin/make: Makefile
>> Added Files:
>>  src/usr.bin/make: lst.c
>> Removed Files:
>>  src/usr.bin/make/lst.lib: Makefile lstAppend.c lstAtEnd.c lstAtFront.c
>>  lstClose.c lstConcat.c lstDatum.c lstDeQueue.c lstDestroy.c
>>  lstDupl.c lstEnQueue.c lstFind.c lstFindFrom.c lstFirst.c
>>  lstForEach.c lstForEachFrom.c lstInit.c lstInsert.c lstInt.h
>>  lstIsAtEnd.c lstIsEmpty.c lstLast.c lstMember.c lstNext.c lstOpen.c
>>  lstPrev.c lstRemove.c lstReplace.c lstSucc.c
>>
>> Log Message:
>> make(1): condense the list library into a single file
>
> hi Roland.
>
>
> this has broken the build:
>
> cc: error: /usr/src3/usr.bin/make/lst.lib/*.c: No such file or directory
>
> please be more careful with make changes they affect everyone.

Ouch, I'm sorry for that.

I had planned to do a full build with this change before committing it,
but I simply forgot it at the right moment.

In a follow-up commit, I adjusted the build scripts, and I'm doing a
full build right now. I'm pretty confident that everything works now
again, and if not, I'll fix it immediately.

Roland

re: CVS commit: src/usr.bin/make

[matthew green]

"Roland Illig" writes:
> Module Name:  src
> Committed By: rillig
> Date: Sun Jul 26 07:15:26 UTC 2020
> 
> Modified Files:
>   src/usr.bin/make: Makefile
> Added Files:
>   src/usr.bin/make: lst.c
> Removed Files:
>   src/usr.bin/make/lst.lib: Makefile lstAppend.c lstAtEnd.c lstAtFront.c
>   lstClose.c lstConcat.c lstDatum.c lstDeQueue.c lstDestroy.c
>   lstDupl.c lstEnQueue.c lstFind.c lstFindFrom.c lstFirst.c
>   lstForEach.c lstForEachFrom.c lstInit.c lstInsert.c lstInt.h
>   lstIsAtEnd.c lstIsEmpty.c lstLast.c lstMember.c lstNext.c lstOpen.c
>   lstPrev.c lstRemove.c lstReplace.c lstSucc.c
> 
> Log Message:
> make(1): condense the list library into a single file

hi Roland.


this has broken the build:

cc: error: /usr/src3/usr.bin/make/lst.lib/*.c: No such file or directory

please be more careful with make changes they affect everyone.

thanks.


.mrg.

Re: CVS commit: src/usr.bin/make

[Izumi Tsutsui]

kardel@ wrote:

> nbmake currently fails to build the tree.
> 
> config.status: creating buildmake.sh
>  compile  arch.c
>  compile  buf.c
>  compile  compat.c
>  compile  cond.c
>  compile  dir.c
>  compile  for.c
>  compile  hash.c
>  compile  job.c
>  compile  main.c
>  compile  make.c
>  compile  make_malloc.c
>  compile  meta.c
>  compile  metachar.c
>  compile  parse.c
>  compile  str.c
>  compile  strlist.c
>  compile  suff.c
>  compile  targ.c
>  compile  trace.c
>  compile  util.c
>  compile  var.c
>  compile  lstAppend.c
>  compile  lstAtEnd.c
>  compile  lstAtFront.c
>  compile  lstClose.c
>  compile  lstConcat.c
>  compile  lstDatum.c
>  compile  lstDeQueue.c
>  compile  lstDestroy.c
>  compile  lstDupl.c
>  compile  lstEnQueue.c
>  compile  lstFind.c
>  compile  lstFindFrom.c
>  compile  lstFirst.c
>  compile  lstForEach.c
>  compile  lstForEachFrom.c
>  compile  lstInit.c
>  compile  lstInsert.c
>  compile  lstIsAtEnd.c
>  compile  lstIsEmpty.c
>  compile  lstLast.c
>  compile  lstMember.c
>  compile  lstNext.c
>  compile  lstOpen.c
>  compile  lstPrev.c
>  compile  lstRemove.c
>  compile  lstReplace.c
>  compile  lstSucc.c
> link  nbmake
> ===> MAKECONF file:   /etc/mk.conf
>   objdir  /src/NetBSD/native/src/obj.evbarm/tools
> nbmake: "/src/NetBSD/native/src/share/mk/bsd.nls.mk" line 18: Unknown 
> directive
> nbmake: "/src/NetBSD/native/src/share/mk/bsd.nls.mk" line 19: Unknown 
> directive
> nbmake: Fatal errors encountered -- cannot continue
> ERROR: bomb_getmakevar TOOLDIR: /tmp/nbbuild851/nbmake failed

Also reported:
 https://mail-index.netbsd.org/current-users/2020/07/19/msg039168.html
 https://releng.netbsd.org/builds/HEAD/202007191220Z/
and still broken here.

Could you please fix or revert changes?

Thanks,
---
Izumi Tsutsui

Re: CVS commit: src/usr.bin/make

[Frank Kardel]

nbmake currently fails to build the tree.

config.status: creating buildmake.sh
compile  arch.c
compile  buf.c
compile  compat.c
compile  cond.c
compile  dir.c
compile  for.c
compile  hash.c
compile  job.c
compile  main.c
compile  make.c
compile  make_malloc.c
compile  meta.c
compile  metachar.c
compile  parse.c
compile  str.c
compile  strlist.c
compile  suff.c
compile  targ.c
compile  trace.c
compile  util.c
compile  var.c
compile  lstAppend.c
compile  lstAtEnd.c
compile  lstAtFront.c
compile  lstClose.c
compile  lstConcat.c
compile  lstDatum.c
compile  lstDeQueue.c
compile  lstDestroy.c
compile  lstDupl.c
compile  lstEnQueue.c
compile  lstFind.c
compile  lstFindFrom.c
compile  lstFirst.c
compile  lstForEach.c
compile  lstForEachFrom.c
compile  lstInit.c
compile  lstInsert.c
compile  lstIsAtEnd.c
compile  lstIsEmpty.c
compile  lstLast.c
compile  lstMember.c
compile  lstNext.c
compile  lstOpen.c
compile  lstPrev.c
compile  lstRemove.c
compile  lstReplace.c
compile  lstSucc.c
   link  nbmake
===> MAKECONF file:   /etc/mk.conf
 objdir  /src/NetBSD/native/src/obj.evbarm/tools
nbmake: "/src/NetBSD/native/src/share/mk/bsd.nls.mk" line 18: Unknown 
directive
nbmake: "/src/NetBSD/native/src/share/mk/bsd.nls.mk" line 19: Unknown 
directive

nbmake: Fatal errors encountered -- cannot continue
ERROR: bomb_getmakevar TOOLDIR: /tmp/nbbuild851/nbmake failed

Frank

On 07/19/20 14:26, Roland Illig wrote:

Module Name:src
Committed By:   rillig
Date:   Sun Jul 19 12:26:17 UTC 2020

Modified Files:
src/usr.bin/make: arch.c compat.c cond.c for.c job.c main.c make.c
meta.c nonints.h parse.c suff.c var.c

Log Message:
make(1): rename Varf_Flags to VarEvalFlags

In var.c there are lots of different flag types.  To make any accidental
mixture obvious, each flag group gets its own prefix.

The only flag group that is visible outside of var.c is concerned with
evaluating variables, therefore the "e", which replaces the former "f"
that probably just meant "flag".


To generate a diff of this commit:
cvs rdiff -u -r1.73 -r1.74 src/usr.bin/make/arch.c
cvs rdiff -u -r1.113 -r1.114 src/usr.bin/make/compat.c
cvs rdiff -u -r1.79 -r1.80 src/usr.bin/make/cond.c src/usr.bin/make/nonints.h
cvs rdiff -u -r1.54 -r1.55 src/usr.bin/make/for.c
cvs rdiff -u -r1.201 -r1.202 src/usr.bin/make/job.c
cvs rdiff -u -r1.280 -r1.281 src/usr.bin/make/main.c
cvs rdiff -u -r1.99 -r1.100 src/usr.bin/make/make.c
cvs rdiff -u -r1.86 -r1.87 src/usr.bin/make/meta.c
cvs rdiff -u -r1.236 -r1.237 src/usr.bin/make/parse.c
cvs rdiff -u -r1.88 -r1.89 src/usr.bin/make/suff.c
cvs rdiff -u -r1.258 -r1.259 src/usr.bin/make/var.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Re: CVS commit: src/usr.bin/make

[Joerg Sonnenberger]

On Sat, Jul 04, 2020 at 03:44:07PM +, Roland Illig wrote:
> Module Name:  src
> Committed By: rillig
> Date: Sat Jul  4 15:44:07 UTC 2020
> 
> Modified Files:
>   src/usr.bin/make: var.c
> 
> Log Message:
> make(1): fix :hash modifier on 16-bit platforms
> 
> On platforms where int has only 16 bits the shifts would have been in
> 16-bit arithmetic, which would invoke undefined behavior for "ustr[3] <<
> 24" as well as "ustr[2] << 16" (C99, 6.5.7p3).

WTF should we care? This is just making things more complicated without
adding any value.

Joerg

Re: CVS commit: src/usr.bin/make

[Simon J. Gerraty]

J. Hannken-Illjes  wrote:
> After this commit parallel builds take much longer.  Building
> amd64 release with -j16 for example goes from 45 to 380 minutes.

Interesting.
Removing the sleep would help there - the busy waiting issue
isn't new anyway.

Re: CVS commit: src/usr.bin/make

[J. Hannken-Illjes]

> On 12. May 2018, at 20:17, Simon J. Gerraty  wrote:
> 
> Module Name:  src
> Committed By: sjg
> Date: Sat May 12 18:17:04 UTC 2018
> 
> Modified Files:
>   src/usr.bin/make: job.c
> 
> Log Message:
> Skip setting wantToken.
> 
> polling the job token pipe adds a lot of overhead
> for little gain.
> For now, just leave wantToken=0
> 
> And avoid busy waiting when no tokens are available and
> no jobs are running.
> 
> Reviewed by: christos
> 
> 
> To generate a diff of this commit:
> cvs rdiff -u -r1.192 -r1.193 src/usr.bin/make/job.c
> 
> Please note that diffs are not public domain; they are subject to the
> copyright notices on the relevant files.
> 

After this commit parallel builds take much longer.  Building
amd64 release with -j16 for example goes from 45 to 380 minutes.

--
J. Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany)

Re: CVS commit: src/usr.bin/make

[David Holland]

On Wed, Apr 04, 2018 at 08:31:11PM -0400, Christos Zoulas wrote:
 > Module Name: src
 > Committed By:christos
 > Date:Thu Apr  5 00:31:11 UTC 2018
 > 
 > Modified Files:
 >  src/usr.bin/make: parse.c
 > 
 > Log Message:
 > Be more selective about detecting a SYSV include as opposed to a dependency
 > line. Dependency lines should contain a '::' operator or ':'.

This is wrong: it's perfectly legal to write "foo.o:foo.c".

It needs to scan for variables, or at least not look inside matching
sets of () {}.

-- 
David A. Holland
dholl...@netbsd.org

re: CVS commit: src/usr.bin/make

[matthew green]

> > And it still does. You cannot use -VV because of getopt(3). You can use
> > a different letter. The complexity is when I get this long string instead
> > of the evaluated variable.
> 
> Please do not unilaterally change behavior. Especially if it has been
> discussed in the past. This is rude at best and not everyone shares your
> opinion.

i agree.  we discussed this before and decided *not* to go with
the freebsd version.  please revert.


.mrg.

Re: CVS commit: src/usr.bin/make

[Christos Zoulas]

On Jun 19,  6:58am, mar...@duskware.de (Martin Husemann) wrote:
-- Subject: Re: CVS commit: src/usr.bin/make

| On Sun, Jun 18, 2017 at 04:26:38PM -0400, Christos Zoulas wrote:
| > As I said above "reasons other than debugging", and you can still
| > get the previous "debugging output" by '\var'
| 
| There was a discussion way back, exactly about what the default
| behaviour should be:
| 
| http://mail-index.netbsd.org/tech-toolchain/2012/08/06/msg001900.html
| 
| If we now want to change the default (as you did), we should at least
| have a follow-up discussion on tech-toolchain.

I removed the \VAR hack and send a message to tech-toolchain.

christos

Re: CVS commit: src/usr.bin/make

[Christos Zoulas]

On Jun 19,  6:58am, mar...@duskware.de (Martin Husemann) wrote:
-- Subject: Re: CVS commit: src/usr.bin/make

| There was a discussion way back, exactly about what the default
| behaviour should be:
| 
| http://mail-index.netbsd.org/tech-toolchain/2012/08/06/msg001900.html
| 
| If we now want to change the default (as you did), we should at least
| have a follow-up discussion on tech-toolchain.

I just read all 4 messages and there was no strong consensus either way:

Iain:
I don't like the thought of changing the result of options that are
already present, since it is not beyond conceivable that doing so would
break somebodys setup somewhere..

but I agree that -V can be a bit irritating, and providing a modifier to
cause it to produce the expanded value directly does sound useful...

joerg:

My usage distribution is far less screwed into either direction, so I am
not too sure about changing it.

simon:

Being able to get the raw value of a variable is definitely useful,
but I'm wondering if it makes a good default.
99% of the time I end up doing:

make -V VAR
make -V '${VAR}


I ask again: Do you want to have 'make -V VAR' do something useful by
default, or do you want it to produce debugging output because this
is what it did historically?

I am convinced that this behavior is an accident; whoever wrote
the code thought that Var_Value() returns the value of a variable,
and did not realize that it returns the unexpanded value (because
it worked for the cases she/he checked). It is my fault because I
I introduced -V in 1.31 in 1996 without realizing that.

christos

Re: CVS commit: src/usr.bin/make

[Martin Husemann]

On Sun, Jun 18, 2017 at 02:21:42PM -0400, Christos Zoulas wrote:
> 1. What needs the intermediate representation, and how it can be used?

For debuging makefiles.

Martin

Re: CVS commit: src/usr.bin/make

[Christos Zoulas]

On Jun 18,  7:01pm, jo...@bec.de (Joerg Sonnenberger) wrote:
-- Subject: Re: CVS commit: src/usr.bin/make

| On Sat, Jun 17, 2017 at 10:40:25PM -0400, Christos Zoulas wrote:
| > On Jun 18, 12:36am, jo...@bec.de (Joerg Sonnenberger) wrote:
| > -- Subject: Re: CVS commit: src/usr.bin/make
| > 
| > | Please do not unilaterally change behavior. Especially if it has been
| > | discussed in the past. This is rude at best and not everyone shares your
| > | opinion.
| > 
| > Please explain the use case (aside for looking at the internal 
representation
| > of a variable). For example, if .OBJDIR, LIB, etc. ended up unexpanded, many
| > things would break. I just don't think it is useful to be the default, and
| > using -V '${var}' to get it to expand is counter-intuitive. Why should it
| > behave differently in the first place?
| 
| They haven't been expanded for years, if ever. You have broken any user
| of the unexpanded variables already. ${var} is a pretty intuitive
| semantics for "show me this expression". It even works for complex
| expressions involving modifiers. Your change has increased the
| complexity of the argument format without providing any new value. It
| has broken compatibility. It was not discussed. I think those are more
| than enough reasons to ask for a revert.

This is not what I asked; I asked for a use case. The reason we have not
made this change sooner is because the majority of the variables produce
already expanded values. I don't think that it is reasonable for someone
in a Makefile to have to write:

foo!=${MAKE} -V '$${var}'

which is the common use case to get the expanded value. Again, what
is the use case? I don't buy the argument that this will break
compatibility since the majority of the variables produce the same
results. The users also expect the expanded value, not an intermediate
representation (which is again non portable since it can change
anytime; so we can't depend on its form to parse it).  Bottom line:
please explain the use case (aside from debugging). I.e.

1. What needs the intermediate representation, and how it can be used?
2. When is the intermediate representation preferable?

christos

Re: CVS commit: src/usr.bin/make

[Christos Zoulas]

On Jun 18, 12:36am, jo...@bec.de (Joerg Sonnenberger) wrote:
-- Subject: Re: CVS commit: src/usr.bin/make

| Please do not unilaterally change behavior. Especially if it has been
| discussed in the past. This is rude at best and not everyone shares your
| opinion.

Please explain the use case (aside for looking at the internal representation
of a variable). For example, if .OBJDIR, LIB, etc. ended up unexpanded, many
things would break. I just don't think it is useful to be the default, and
using -V '${var}' to get it to expand is counter-intuitive. Why should it
behave differently in the first place?

christos

Re: CVS commit: src/usr.bin/make

[Joerg Sonnenberger]

On Sun, Jun 18, 2017 at 12:40:20AM +0200, Kamil Rytarowski wrote:
> Can we reuse show-var from pkgsrc?
> 
> $ make show-var VARNAME=MACHINE_CPU
> x86_64

It's no better than just using -V '${expr}' directly.

Joerg

Re: CVS commit: src/usr.bin/make

[Kamil Rytarowski]

On 18.06.2017 00:31, Christos Zoulas wrote:
> In article <20170617222558.ga24...@britannica.bec.de>,
> Joerg Sonnenberger  <jo...@bec.de> wrote:
>> On Sun, Jun 18, 2017 at 12:22:13AM +0200, Kamil Rytarowski wrote:
>>> On 18.06.2017 00:16, Christos Zoulas wrote:
>>>> In article <20170617213136.ga21...@britannica.bec.de>,
>>>> Joerg Sonnenberger  <jo...@bec.de> wrote:
>>>>> On Sat, Jun 17, 2017 at 05:28:07PM -0400, Christos Zoulas wrote:
>>>>>> On Jun 17,  9:38pm, jo...@bec.de (Joerg Sonnenberger) wrote:
>>>>>> -- Subject: Re: CVS commit: src/usr.bin/make
>>>>>>
>>>>>> | Agreed, please revert. This was discussed at the time and FreeBSD
>>>>>> | behavior you have now implemented is much less useful.
>>>>>>
>>>>>> You can get the original with -V '\VAR'
>>>>>
>>>>> That's no better than the behavior before.
>>>>
>>>> Now you get:
>>>>
>>>> $ make -V MACHINE_CPU
>>>> arm
>>>> $ make -V \\MACHINE_CPU
>>>>
>> ${MACHINE_ARCH:C/mipse[bl]/mips/:C/mips64e[bl]/mips/:C/sh3e[bl]/sh3/:S/coldfire/m68k/:S/m68000/m68k/:C/arm.*/arm/:C/earm.*/arm/:S/earm/arm/:S/powerpc64/powerpc/:S/aarch64eb/aarch64/:S/or1knd/or1k/:C/riscv../riscv/}
>>>>
>>>> The second is the original version.
>>>>
>>>> christos
>>>>
>>>
>>> How about "make -V" getting the original behavior and "make -VV"
>>> resulting with evaluated one?
>>
>> I find -V '${foo}' a perfectly reasonable way to spell it, especially
>> since it works consistently with modifiers. No need for more complexity.
> 
> And it still does. You cannot use -VV because of getopt(3). You can use
> a different letter. The complexity is when I get this long string instead
> of the evaluated variable.
> 
> christos
> 

Can we reuse show-var from pkgsrc?

$ make show-var VARNAME=MACHINE_CPU
x86_64

# show-var:
# show-vars:
# show-subdir-var:
#   Convenience targets, to display make variables from the command
#   line. Examples:
#
#   make show-var VARNAME=PKGNAME
#   make show-vars VARNAMES="PKGNAME PKGVERSION PKGREVISION"
#   make show-subdir-var VARNAME=DISTFILES
#
#   In category directories, show-var and show-vars descend
#   recursively into each subdirectory, printing the variables of


#   the individual packages. To show a variable from the category
#   itself, use show-subdir-var.
.PHONY: show-var
show-var:
@${ECHO} ${${VARNAME}:Q

 -- /usr/pkgsrc/mk/bsd.pkg.mk



signature.asc
Description: OpenPGP digital signature

Re: CVS commit: src/usr.bin/make

[Christos Zoulas]

In article <20170617222558.ga24...@britannica.bec.de>,
Joerg Sonnenberger  <jo...@bec.de> wrote:
>On Sun, Jun 18, 2017 at 12:22:13AM +0200, Kamil Rytarowski wrote:
>> On 18.06.2017 00:16, Christos Zoulas wrote:
>> > In article <20170617213136.ga21...@britannica.bec.de>,
>> > Joerg Sonnenberger  <jo...@bec.de> wrote:
>> >> On Sat, Jun 17, 2017 at 05:28:07PM -0400, Christos Zoulas wrote:
>> >>> On Jun 17,  9:38pm, jo...@bec.de (Joerg Sonnenberger) wrote:
>> >>> -- Subject: Re: CVS commit: src/usr.bin/make
>> >>>
>> >>> | Agreed, please revert. This was discussed at the time and FreeBSD
>> >>> | behavior you have now implemented is much less useful.
>> >>>
>> >>> You can get the original with -V '\VAR'
>> >>
>> >> That's no better than the behavior before.
>> > 
>> > Now you get:
>> > 
>> > $ make -V MACHINE_CPU
>> > arm
>> > $ make -V \\MACHINE_CPU
>> >
>${MACHINE_ARCH:C/mipse[bl]/mips/:C/mips64e[bl]/mips/:C/sh3e[bl]/sh3/:S/coldfire/m68k/:S/m68000/m68k/:C/arm.*/arm/:C/earm.*/arm/:S/earm/arm/:S/powerpc64/powerpc/:S/aarch64eb/aarch64/:S/or1knd/or1k/:C/riscv../riscv/}
>> > 
>> > The second is the original version.
>> > 
>> > christos
>> > 
>> 
>> How about "make -V" getting the original behavior and "make -VV"
>> resulting with evaluated one?
>
>I find -V '${foo}' a perfectly reasonable way to spell it, especially
>since it works consistently with modifiers. No need for more complexity.

And it still does. You cannot use -VV because of getopt(3). You can use
a different letter. The complexity is when I get this long string instead
of the evaluated variable.

christos

Re: CVS commit: src/usr.bin/make

[Kamil Rytarowski]

On 18.06.2017 00:16, Christos Zoulas wrote:
> In article <20170617213136.ga21...@britannica.bec.de>,
> Joerg Sonnenberger  <jo...@bec.de> wrote:
>> On Sat, Jun 17, 2017 at 05:28:07PM -0400, Christos Zoulas wrote:
>>> On Jun 17,  9:38pm, jo...@bec.de (Joerg Sonnenberger) wrote:
>>> -- Subject: Re: CVS commit: src/usr.bin/make
>>>
>>> | Agreed, please revert. This was discussed at the time and FreeBSD
>>> | behavior you have now implemented is much less useful.
>>>
>>> You can get the original with -V '\VAR'
>>
>> That's no better than the behavior before.
> 
> Now you get:
> 
> $ make -V MACHINE_CPU
> arm
> $ make -V \\MACHINE_CPU
> ${MACHINE_ARCH:C/mipse[bl]/mips/:C/mips64e[bl]/mips/:C/sh3e[bl]/sh3/:S/coldfire/m68k/:S/m68000/m68k/:C/arm.*/arm/:C/earm.*/arm/:S/earm/arm/:S/powerpc64/powerpc/:S/aarch64eb/aarch64/:S/or1knd/or1k/:C/riscv../riscv/}
> 
> The second is the original version.
> 
> christos
> 

How about "make -V" getting the original behavior and "make -VV"
resulting with evaluated one?



signature.asc
Description: OpenPGP digital signature

Re: CVS commit: src/usr.bin/make

[Christos Zoulas]

In article <20170617213136.ga21...@britannica.bec.de>,
Joerg Sonnenberger  <jo...@bec.de> wrote:
>On Sat, Jun 17, 2017 at 05:28:07PM -0400, Christos Zoulas wrote:
>> On Jun 17,  9:38pm, jo...@bec.de (Joerg Sonnenberger) wrote:
>> -- Subject: Re: CVS commit: src/usr.bin/make
>> 
>> | Agreed, please revert. This was discussed at the time and FreeBSD
>> | behavior you have now implemented is much less useful.
>> 
>> You can get the original with -V '\VAR'
>
>That's no better than the behavior before.

Now you get:

$ make -V MACHINE_CPU
arm
$ make -V \\MACHINE_CPU
${MACHINE_ARCH:C/mipse[bl]/mips/:C/mips64e[bl]/mips/:C/sh3e[bl]/sh3/:S/coldfire/m68k/:S/m68000/m68k/:C/arm.*/arm/:C/earm.*/arm/:S/earm/arm/:S/powerpc64/powerpc/:S/aarch64eb/aarch64/:S/or1knd/or1k/:C/riscv../riscv/}

The second is the original version.

christos

Re: CVS commit: src/usr.bin/make

[Joerg Sonnenberger]

On Sat, Jun 17, 2017 at 05:28:07PM -0400, Christos Zoulas wrote:
> On Jun 17,  9:38pm, jo...@bec.de (Joerg Sonnenberger) wrote:
> -- Subject: Re: CVS commit: src/usr.bin/make
> 
> | Agreed, please revert. This was discussed at the time and FreeBSD
> | behavior you have now implemented is much less useful.
> 
> You can get the original with -V '\VAR'

That's no better than the behavior before.

Joerg

Re: CVS commit: src/usr.bin/make

[Christos Zoulas]

On Jun 17,  9:38pm, jo...@bec.de (Joerg Sonnenberger) wrote:
-- Subject: Re: CVS commit: src/usr.bin/make

| Agreed, please revert. This was discussed at the time and FreeBSD
| behavior you have now implemented is much less useful.

You can get the original with -V '\VAR'

christos

re: CVS commit: src/usr.bin/make

[matthew green]

"Christos Zoulas" writes:
> Module Name:  src
> Committed By: christos
> Date: Sat Jun 17 15:49:56 UTC 2017
> 
> Modified Files:
>   src/usr.bin/make: main.c
> 
> Log Message:
> -V: try to expand the variable again if the value contains a variable.

how do i get the original behaviour?  i like having both
that and the -V "${FOO}" version to expand.  they're both
useful in different ways.


.mrg.

Re: CVS commit: src/usr.bin/make

[Christos Zoulas]

In article <20170421221544.6b4eaf...@cvs.netbsd.org>,
Simon J. Gerraty  wrote:
>-=-=-=-=-=-
>
>Module Name:   src
>Committed By:  sjg
>Date:  Fri Apr 21 22:15:44 UTC 2017
>
>Modified Files:
>   src/usr.bin/make: str.c
>   src/usr.bin/make/unit-tests: modmatch.exp modmatch.mk
>
>Log Message:
>Str_Match: fix closure tests for [^] and add unit-test.

I think this is just another bad copy of:

https://nxr.netbsd.org/xref/src/sys/lib/libkern/pmatch.c

and it is useful to make it available from userland, so others
can use it without fixing bug after bug in it :-)

christos

Re: CVS commit: src/usr.bin/make

[Simon J. Gerraty]

Simon J. Gerraty  wrote:

>  wrote:
> 
> > On Tue, Apr 11, 2017 at 05:30:13PM +, Simon J. Gerraty wrote:
> > > Log Message:
> > > Str_Match: allow [^a-z] to behave as expected.
> > 
> > % env A=NetBSD-7-x86_64 make -V '${A:MNetBSD-[^0-1]-i386}'
> > NetBSD-7-x86_64
> 
> Well that's interesting - thanks...

Fixed, and a better unit-test added.

Thanks

Re: CVS commit: src/usr.bin/make

[Simon J. Gerraty]

 wrote:

> On Tue, Apr 11, 2017 at 05:30:13PM +, Simon J. Gerraty wrote:
> > Log Message:
> > Str_Match: allow [^a-z] to behave as expected.
> 
> % env A=NetBSD-7-x86_64 make -V '${A:MNetBSD-[^0-1]-i386}'
> NetBSD-7-x86_64

Well that's interesting - thanks...

Re: CVS commit: src/usr.bin/make

[coypu]

On Tue, Apr 11, 2017 at 05:30:13PM +, Simon J. Gerraty wrote:
> Log Message:
> Str_Match: allow [^a-z] to behave as expected.

% env A=NetBSD-7-x86_64 make -V '${A:MNetBSD-[^0-1]-i386}'
NetBSD-7-x86_64

Re: CVS commit: src/usr.bin/make

[Christos Zoulas]

In article <20170420035727.ba900f...@cvs.netbsd.org>,
Simon J. Gerraty  wrote:
>-=-=-=-=-=-
>
>Module Name:   src
>Committed By:  sjg
>Date:  Thu Apr 20 03:57:27 UTC 2017
>
>Modified Files:
>   src/usr.bin/make: main.c
>
>Log Message:
>We cannot tollerate things like trailing /.. etc in .CURDIR
>so only accept -C arg "as is" if it contains no relative components.
>

Why?

christos

Re: CVS commit: src/usr.bin/make

[Christos Zoulas]

In article <20170131065424.246a2f...@cvs.netbsd.org>,
Simon J. Gerraty  wrote:
>-=-=-=-=-=-
>
>Module Name:   src
>Committed By:  sjg
>Date:  Tue Jan 31 06:54:24 UTC 2017
>
>Modified Files:
>   src/usr.bin/make: dir.c main.c
>
>Log Message:
>Partially initialize Dir before MainParseArgs can be called.
>
>The rest can be done once curdir is finalized.
>
>
>To generate a diff of this commit:
>cvs rdiff -u -r1.68 -r1.69 src/usr.bin/make/dir.c
>cvs rdiff -u -r1.254 -r1.255 src/usr.bin/make/main.c
>
>Please note that diffs are not public domain; they are subject to the
>copyright notices on the relevant files.
>
>
>-=-=-=-=-=-
>
>Modified files:
>
>Index: src/usr.bin/make/dir.c
>diff -u src/usr.bin/make/dir.c:1.68 src/usr.bin/make/dir.c:1.69
>--- src/usr.bin/make/dir.c:1.68Tue Jun  7 00:40:00 2016
>+++ src/usr.bin/make/dir.c Tue Jan 31 06:54:23 2017
>@@ -1,4 +1,4 @@
>-/*$NetBSD: dir.c,v 1.68 2016/06/07 00:40:00 sjg Exp $ */
>+/*$NetBSD: dir.c,v 1.69 2017/01/31 06:54:23 sjg Exp $ */
> 
> /*
>  * Copyright (c) 1988, 1989, 1990 The Regents of the University of California.
>@@ -70,14 +70,14 @@
>  */
> 
> #ifndef MAKE_NATIVE
>-static char rcsid[] = "$NetBSD: dir.c,v 1.68 2016/06/07 00:40:00 sjg Exp $";
>+static char rcsid[] = "$NetBSD: dir.c,v 1.69 2017/01/31 06:54:23 sjg Exp $";
> #else
> #include 
> #ifndef lint
> #if 0
> static char sccsid[] = "@(#)dir.c 8.2 (Berkeley) 1/2/94";
> #else
>-__RCSID("$NetBSD: dir.c,v 1.68 2016/06/07 00:40:00 sjg Exp $");
>+__RCSID("$NetBSD: dir.c,v 1.69 2017/01/31 06:54:23 sjg Exp $");
> #endif
> #endif /* not lint */
> #endif
>@@ -346,11 +346,13 @@ cached_lstat(const char *pathname, void 
> void
> Dir_Init(const char *cdname)
> {
>-dirSearchPath = Lst_Init(FALSE);
>-openDirectories = Lst_Init(FALSE);
>-Hash_InitTable(, 0);
>-Hash_InitTable(, 0);
>-
>+if (!cdname) {
>+  dirSearchPath = Lst_Init(FALSE);
>+  openDirectories = Lst_Init(FALSE);
>+  Hash_InitTable(, 0);
>+  Hash_InitTable(, 0);
>+  return;
>+}

Perhaps it is better to protect against calling this again like:

if (!dirSearchPath) {
dirSearchPath = Lst_Init(FALSE);
openDirectories = Lst_Init(FALSE);
Hash_InitTable(, 0);
Hash_InitTable(, 0);
}
if (!cdname)
return;

christos

Re: CVS commit: src/usr.bin/make

[Paul Goyette]

Module Name:src
Committed By:   sjg
Date:   Sat Jan 14 22:58:04 UTC 2017

Modified Files:
src/usr.bin/make: make.1 var.c
src/usr.bin/make/unit-tests: varmisc.exp varmisc.mk

Log Message:
Allow providing a utc value to :{gm,local}time

Reviewed by: christos


The man-page descriptions for these options are rather awkward:

 :gmtime[=utc]
  The value is a format string for strftime(3), using gmtime(3).
  If a utc value is not provided or is 0, the current time is
  used.

It's not really clear that "the value" refers to the value of the 
variable being modified, rather than the value of the modification.


Perhaps using an example here could make it more clear?  (The example 
from the updated unit tests could be used, but with a utc value of 1 
representing '1970-01-01 00:00:01 UTC'.)  Or reword to something along 
the lines of


Convert the specified utc value using the variable's value as
a format string for strftime(3).  If no utc value is given or
is 0, the current gmtime(3) / localtime(3) is used.

Also, is the value passed to :localtime[=utc] really a utc value?  Or is 
it a "corrected" localtime(3) value?



+--+--++
| Paul Goyette | PGP Key fingerprint: | E-mail addresses:  |
| (Retired)| FA29 0E3B 35AF E8AE 6651 | paul at whooppee.com   |
| Kernel Developer | 0786 F758 55DE 53BA 7731 | pgoyette at netbsd.org |
+--+--++

Re: CVS commit: src/usr.bin/make

[Simon J. Gerraty]

Joerg Sonnenberger  wrote:
> > This improves the worst case performance (eg examining libc with
> > nothing to do) by a factor of 4.
> 
> ...except now build.sh on !NetBSD fails immediately due to missing
> strlcpy.

Should be fixed...

Don't know if you want to get carried away with tools/make/configure*
to build things like strlcpy which are available in the tree if
necessary.

Re: CVS commit: src/usr.bin/make

[Simon J. Gerraty]

Joerg Sonnenberger  wrote:
> > This improves the worst case performance (eg examining libc with
> > nothing to do) by a factor of 4.
> 
> ...except now build.sh on !NetBSD fails immediately due to missing
> strlcpy.

Gah, bmake provides strlcpy... sorry about that.

Re: CVS commit: src/usr.bin/make

[Joerg Sonnenberger]

On Fri, Jun 03, 2016 at 01:21:59AM +, Simon J. Gerraty wrote:
> Module Name:  src
> Committed By: sjg
> Date: Fri Jun  3 01:21:59 UTC 2016
> 
> Modified Files:
>   src/usr.bin/make: main.c make.h meta.c nonints.h var.c
> 
> Log Message:
> Add cached_realpath()
> 
> realpath(3) is expensive, and meta mode at least uses it extensively.
> We use cached_realpath() to save the result of successful calls to
> realpath(3) in a private variable context.
> 
> This improves the worst case performance (eg examining libc with
> nothing to do) by a factor of 4.

...except now build.sh on !NetBSD fails immediately due to missing
strlcpy.

Joerg

Re: CVS commit: src/usr.bin/make

[Christos Zoulas]

In article <20160603012159.a51b6f...@cvs.netbsd.org>,
Simon J. Gerraty  wrote:
>-=-=-=-=-=-
>
>Module Name:   src
>Committed By:  sjg
>Date:  Fri Jun  3 01:21:59 UTC 2016
>
>Modified Files:
>   src/usr.bin/make: main.c make.h meta.c nonints.h var.c
>
>Log Message:
>Add cached_realpath()
>
>realpath(3) is expensive, and meta mode at least uses it extensively.
>We use cached_realpath() to save the result of successful calls to
>realpath(3) in a private variable context.
>
>This improves the worst case performance (eg examining libc with
>nothing to do) by a factor of 4.

I forgot to ask if it is worth it to cache the misses too...

christos

Re: CVS commit: src/usr.bin/make

[Joerg Sonnenberger]

On Sat, Jan 16, 2016 at 04:15:21PM -0800, Simon J. Gerraty wrote:
> Joerg Sonnenberger  wrote:
> > I suspect this change broke editors/xemacs-current, which is now failing
> > with:
> > 
> > make[1]: make[1]: don't know how to make insert-data-in-exec. Stop
> 
> I'm guessing you are talking about a makefile that comes with emacs?
> I don't see anything in editors/xemacs-current itself.
> Do you happen to have an accessible machine with the makefile(s) in
> question?

Build log and content of the build directory can be found in
http://www.netbsd.org/~joerg/xemacs-21.5.27nb22.tar.gz

Joerg

Re: CVS commit: src/usr.bin/make

[Simon J. Gerraty]

Joerg Sonnenberger  wrote:
> I suspect this change broke editors/xemacs-current, which is now failing
> with:
> 
> make[1]: make[1]: don't know how to make insert-data-in-exec. Stop

I'm guessing you are talking about a makefile that comes with emacs?
I don't see anything in editors/xemacs-current itself.
Do you happen to have an accessible machine with the makefile(s) in
question?

Re: CVS commit: src/usr.bin/make

[Joerg Sonnenberger]

On Sun, Dec 20, 2015 at 10:44:10PM +, Simon J. Gerraty wrote:
> Module Name:  src
> Committed By: sjg
> Date: Sun Dec 20 22:44:10 UTC 2015
> 
> Modified Files:
>   src/usr.bin/make: suff.c
> 
> Log Message:
> Suff_ClearSuffixes() needs to re-initialize suffNull,
> otherwise its children retain old suffixes.
> Have Suff_Init() call Suff_ClearSuffixes() for consistency.

I suspect this change broke editors/xemacs-current, which is now failing
with:

make[1]: make[1]: don't know how to make insert-data-in-exec. Stop

Joerg

Re: CVS commit: src/usr.bin/make

[Joerg Sonnenberger]

On Tue, Dec 01, 2015 at 07:26:08AM +, Simon J. Gerraty wrote:
> Module Name:  src
> Committed By: sjg
> Date: Tue Dec  1 07:26:08 UTC 2015
> 
> Modified Files:
>   src/usr.bin/make: var.c
> 
> Log Message:
> Avoid calling brk_string() and hence Var_Export1() on
> empty strings.

I'm not sure which change, but now I get fallout in the clang builds
from constructs like:

. if defined(HAVE_GCC) && ${HAVE_GCC} > 0

in src/tools/Makefile since HAVE_GCC is undefined here.

Joerg

Re: CVS commit: src/usr.bin/make

[Simon J. Gerraty]

Joerg Sonnenberger  wrote:
> > Log Message:
> > Avoid calling brk_string() and hence Var_Export1() on
> > empty strings.
> 
> I'm not sure which change, but now I get fallout in the clang builds
> from constructs like:

Shouldn't this one since only affects exporting of vars.
More likely change to cond.c - will dig...

Re: CVS commit: src/usr.bin/make

[Simon J. Gerraty]

Thanks - fixed.

> /work/src/usr.bin/make/var.c:2772:7: error: variable 'emsg' is used 
> uninitialized whenever 'if' condition is false 
> [-Werror,-Wsometimes-uninitialized]
>   if (wantit)
> /work/src/usr.bin/make/var.c:2777:7: note: uninitialized use occurs here
>   if (emsg)

Re: CVS commit: src/usr.bin/make

[Tobias Nygren]

On Mon, 12 Oct 2015 16:48:13 +
"Simon J. Gerraty"  wrote:

> Module Name:  src
> Committed By: sjg
> Date: Mon Oct 12 16:48:13 UTC 2015
> 
> Modified Files:
>   src/usr.bin/make: var.c
> 
> Log Message:
> The conditional expressions used with ':?' can be expensive
> eg. exists() does stat(2).
> If 'wantit' is FALSE, we are going to discard everything anyway,
> so skip evaluating the conditional and expanding either lhs or rhs.

/work/src/usr.bin/make/var.c:2772:7: error: variable 'emsg' is used 
uninitialized whenever 'if' condition is false 
[-Werror,-Wsometimes-uninitialized]
  if (wantit)
/work/src/usr.bin/make/var.c:2777:7: note: uninitialized use occurs here
  if (emsg)
/work/src/usr.bin/make/var.c:2772:3: note: remove the 'if' if its condition is 
always true
  if (wantit)
/work/src/usr.bin/make/var.c:2760:19: note: initialize the variable 'emsg' to 
silence this warning
  const char *emsg;
1 error generated.

Re: CVS commit: src/usr.bin/make

[Marc Balmer]

> Am 06.10.2015 um 19:36 schrieb Christos Zoulas :
> 
> Module Name:  src
> Committed By: christos
> Date: Tue Oct  6 17:36:25 UTC 2015
> 
> Modified Files:
>   src/usr.bin/make: var.c
> 
> Log Message:
> don't check for NULL before free() (Tilman Sauerbeck)
> 
> 
> To generate a diff of this commit:
> cvs rdiff -u -r1.195 -r1.196 src/usr.bin/make/var.c
> 
> Please note that diffs are not public domain; they are subject to the
> copyright notices on the relevant files.
> 
> Modified files:
> 
> Index: src/usr.bin/make/var.c
> diff -u src/usr.bin/make/var.c:1.195 src/usr.bin/make/var.c:1.196
> --- src/usr.bin/make/var.c:1.195  Fri Jun 19 10:25:16 2015
> +++ src/usr.bin/make/var.cTue Oct  6 13:36:25 2015
> @@ -1,4 +1,4 @@
> -/*   $NetBSD: var.c,v 1.195 2015/06/19 14:25:16 christos Exp $   */
> +/*   $NetBSD: var.c,v 1.196 2015/10/06 17:36:25 christos Exp $   */
> 
> /*
>  * Copyright (c) 1988, 1989, 1990, 1993
> @@ -69,14 +69,14 @@
>  */
> 
> #ifndef MAKE_NATIVE
> -static char rcsid[] = "$NetBSD: var.c,v 1.195 2015/06/19 14:25:16 christos 
> Exp $";
> +static char rcsid[] = "$NetBSD: var.c,v 1.196 2015/10/06 17:36:25 christos 
> Exp $";
> #else
> #include 
> #ifndef lint
> #if 0
> static char sccsid[] = "@(#)var.c 8.3 (Berkeley) 3/19/94";
> #else
> -__RCSID("$NetBSD: var.c,v 1.195 2015/06/19 14:25:16 christos Exp $");
> +__RCSID("$NetBSD: var.c,v 1.196 2015/10/06 17:36:25 christos Exp $");
> #endif
> #endif /* not lint */
> #endif
> @@ -982,8 +982,7 @@ Var_Set(const char *name, const char *va
> }
>   
>  out:
> -if (expanded_name != NULL)
> - free(expanded_name);
> +free(expanded_name);
> if (v != NULL)
>   VarFreeEnv(v, TRUE);
> }
> @@ -1061,8 +1060,7 @@ Var_Append(const char *name, const char 
>   Hash_SetValue(h, v);
>   }
> }
> -if (expanded_name != NULL)
> - free(expanded_name);
> +free(expanded_name);
> }
> 
> /*-
> @@ -1092,9 +1090,7 @@ Var_Exists(const char *name, GNode *ctxt
>   cp = Var_Subst(NULL, name, ctxt, FALSE);
> }
> v = VarFind(cp ? cp : name, ctxt, FIND_CMD|FIND_GLOBAL|FIND_ENV);
> -if (cp != NULL) {
> - free(cp);
> -}
> +free(cp);
> if (v == NULL) {
>   return(FALSE);
> } else {
> @@ -2192,8 +2188,7 @@ VarGetPattern(GNode *ctxt, Var_Parse_Sta
>*/
>   cp2 = Var_Parse(cp, ctxt, errnum, , );
>   Buf_AddBytes(, strlen(cp2), cp2);
> - if (freeIt)
> - free(freeIt);
> + free(freeIt);
>   cp += len - 1;
>   } else {
>   const char *cp2 = [1];
> @@ -2505,8 +2500,7 @@ ApplyModifiers(char *nstr, const char *t
>   (c = tstr[rlen]) != '\0' &&
>   c != ':' &&
>   c != endc) {
> - if (freeIt)
> - free(freeIt);
> + free(freeIt);
>   goto apply_mods;
>   }
> 
> @@ -2526,13 +2520,11 @@ ApplyModifiers(char *nstr, const char *t
>   if (nstr == var_Error
>   || (nstr == varNoError && errnum == 0)
>   || strlen(rval) != (size_t) used) {
> - if (freeIt)
> - free(freeIt);
> + free(freeIt);
>   goto out;   /* error already reported */
>   }
>   }
> - if (freeIt)
> - free(freeIt);
> + free(freeIt);
>   if (*tstr == ':')
>   tstr++;
>   else if (!*tstr && endc) {
> @@ -2621,8 +2613,7 @@ ApplyModifiers(char *nstr, const char *t
>   Error(emsg, nstr);
>   else
>   Var_Set(v->name, newStr,  v_ctxt, 0);
> - if (newStr)
> - free(newStr);
> + free(newStr);
>   break;
>   case '?':
>   if ((v->flags & VAR_JUNK) == 0)
> @@ -2704,8 +2695,7 @@ ApplyModifiers(char *nstr, const char *t
> 
>   cp2 = Var_Parse(cp, ctxt, errnum, , );
>   Buf_AddBytes(, strlen(cp2), cp2);
> - if (freeIt)
> - free(freeIt);
> + free(freeIt);
>   cp += len - 1;
>   } else {
>   Buf_AddByte(, *cp);
> @@ -3543,10 +3533,8 @@ ApplyModifiers(char *nstr, const char *t
> if (delim != '\0')
>   Error("Unclosed substitution for %s (%c missing)",
> v->name, delim);
> -if (*freePtr) {
> - free(*freePtr);
> - *freePtr = NULL;
> -}
> +free(*freePtr);
> +*freePtr = NULL;

is this sentinel, setting *freePtr to NULL, really needed?

> return (var_Error);
> }
> 
> @@ -3689,8 +3677,7 @@ Var_Parse(const char *str, GNode *ctxt, 
>   if (rval != NULL) {
>  

Re: CVS commit: src/usr.bin/make

[Christos Zoulas]

In article <3c6cf536-97db-4dae-966e-a3c02c505...@msys.ch>,
Marc Balmer   wrote:

>>v->name, delim);
>> -if (*freePtr) {
>> -free(*freePtr);
>> -*freePtr = NULL;
>> -}
>> +free(*freePtr);
>> +*freePtr = NULL;
>
>is this sentinel, setting *freePtr to NULL, really needed?

Maybe not, but the point of the patch was to just remove the tests, not alter
the code.

christos

Re: CVS commit: src/usr.bin/make

[Joerg Sonnenberger]

On Tue, Sep 09, 2014 at 06:18:17AM +, David A. Holland wrote:
 Module Name:  src
 Committed By: dholland
 Date: Tue Sep  9 06:18:17 UTC 2014
 
 Modified Files:
   src/usr.bin/make: main.c
 
 Log Message:
 Restore apb's 20140820 commit (-r1.228 of main.c):
 It should not be an error to have VAR != command that prints no output
 
 Joerg reverted a bit too enthusiastically.

Thanks. I will go over the various changes after Tuesday if noone beats
me to it. I am aware that there are some non-problematic ones in the
patch as well as subtile issues with others.

Joerg

Re: CVS commit: src/usr.bin/make

[David Holland]

On Tue, Sep 09, 2014 at 02:19:02PM +0200, Joerg Sonnenberger wrote:
   Log Message:
   Restore apb's 20140820 commit (-r1.228 of main.c):
   It should not be an error to have VAR != command that prints no output
   
   Joerg reverted a bit too enthusiastically.
  
  Thanks. I will go over the various changes after Tuesday if noone beats
  me to it. I am aware that there are some non-problematic ones in the
  patch as well as subtile issues with others.

Yeah, that's what I was getting started on.

There's a lot of changes. There's three main changesets, of which the
big one was over 5000 lines of diff; and the changes include things
like braces changes as well as substance so it's a pain to review in
detail.

I've been thinking I should create a secondary repo for this with a
branch for each of the changesets, so they can be handled
incrementally. If you're interested in sharing such a thing, let me
know.

-- 
David A. Holland
dholl...@netbsd.org

Re: CVS commit: src/usr.bin/make

[Christos Zoulas]

In article 20140907205534.98d4...@cvs.netbsd.org,
Joerg Sonnenberger source-changes-d@NetBSD.org wrote:
-=-=-=-=-=-

Module Name:   src
Committed By:  joerg
Date:  Sun Sep  7 20:55:34 UTC 2014

Modified Files:
   src/usr.bin/make: compat.c lst.h main.c make.c make.h nonints.h parse.c
   suff.c targ.c var.c
   src/usr.bin/make/lst.lib: lstInt.h lstRemove.c

Log Message:
Revert all make changes except the unit tests to the state of three
weeks ago. Individual changes can be reapplied after review.

Did I miss some discussion here? Was there something broken that you
could not wait fixing?

christos

Re: CVS commit: src/usr.bin/make

[Christos Zoulas]

On Aug 31, 12:36am, u...@stderr.spb.ru (Valery Ushakov) wrote:
-- Subject: Re: CVS commit: src/usr.bin/make

| On Fri, Aug 29, 2014 at 05:27:44 -0400, Christos Zoulas wrote:
| 
|  Module Name:src
|  Committed By:   christos
|  Date:   Fri Aug 29 09:27:44 UTC 2014
|  
|  Modified Files:
|  src/usr.bin/make: parse.c
|  
|  Log Message:
|  undo eating the trailing backslash now that the shell has been fixed.
| 
| IIRC, we use native shell when crossbuilding, so we will run with
| unfixed shell, don't we?

This bug only affects Makefiles that use rules that end with a backslash.
We don't have any of those. If we ever get any, we can put it back. Good
point though.

christos

Re: CVS commit: src/usr.bin/make

[Valery Ushakov]

On Fri, Aug 29, 2014 at 05:27:44 -0400, Christos Zoulas wrote:

 Module Name:  src
 Committed By: christos
 Date: Fri Aug 29 09:27:44 UTC 2014
 
 Modified Files:
   src/usr.bin/make: parse.c
 
 Log Message:
 undo eating the trailing backslash now that the shell has been fixed.

IIRC, we use native shell when crossbuilding, so we will run with
unfixed shell, don't we?

-uwe

Re: CVS commit: src/usr.bin/make

[David Holland]

On Tue, Sep 03, 2013 at 09:58:55PM +0200, Alan Barrett wrote:
  On Mon, 02 Sep 2013, Simon J. Gerraty wrote:
  Modified Files:
  src/usr.bin/make: compat.c
  
  Log Message:
  Do not apply shellErrFlag unless errCheck is true.
  
  
  To generate a diff of this commit:
  cvs rdiff -u -r1.92 -r1.93 src/usr.bin/make/compat.c
  
  Will this fix PR 45356?

Don't think so. Roots of that one are deeper; it's (I expect) caused
by running the whole recipe with one shell command.

-- 
David A. Holland
dholl...@netbsd.org

Re: CVS commit: src/usr.bin/make

[Alan Barrett]

On Mon, 02 Sep 2013, Simon J. Gerraty wrote:

Modified Files:
src/usr.bin/make: compat.c

Log Message:
Do not apply shellErrFlag unless errCheck is true.


To generate a diff of this commit:
cvs rdiff -u -r1.92 -r1.93 src/usr.bin/make/compat.c


Will this fix PR 45356?

--apb (Alan Barrett)

Re: CVS commit: src/usr.bin/make

[Ryo ONODERA]

Hi,

From: Christos Zoulas chris...@netbsd.org, Date: Tue, 16 Jul 2013 10:00:53 
-0400

 Module Name:  src
 Committed By: christos
 Date: Tue Jul 16 14:00:53 UTC 2013
 
 Modified Files:
   src/usr.bin/make: main.c make.1 var.c
 
 Log Message:
 More gmake compatibility:
 1. add -w flag to print Entering and Leaving directory name the the beginning
and the end of processing.
 2. export MAKELEVEL=$((MAKELEVEL + 1)) only in the child environment.
 3. when printing error messages, prefix them with the program name 
 [$MAKELEVEL]
for $MAKELEVEL  0
 4. if $MAKEFLAGS consists only of letters assume it is a set of flags (as
allowed by posix), convert them to -f -l -a -g -s, so that they get parsed
properly.
 With those fixes gmake - bmake - gmake - bmake etc. works as expected.
 
 
 To generate a diff of this commit:
 cvs rdiff -u -r1.219 -r1.220 src/usr.bin/make/main.c
 cvs rdiff -u -r1.218 -r1.219 src/usr.bin/make/make.1
 cvs rdiff -u -r1.181 -r1.182 src/usr.bin/make/var.c
 
 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.
 

With this change, I have gotten following errors.
I think -lutil is needed.

cc  -O -DDEFSHELL_CUSTOM=/bin/sh -DHAVE_SETENV=1 -DHAVE_STRDUP=1 
-DHAVE_STRERROR=1 -DHAVE_STRFTIME=1 -DHAVE_VSNPRINTF=1  -c 
/usr/src/tools/make/../../usr.bin/make/lst.lib/lstReplace.c
cc  -O -DDEFSHELL_CUSTOM=/bin/sh -DHAVE_SETENV=1 -DHAVE_STRDUP=1 
-DHAVE_STRERROR=1 -DHAVE_STRFTIME=1 -DHAVE_VSNPRINTF=1  -c 
/usr/src/tools/make/../../usr.bin/make/lst.lib/lstSucc.c
cc  -O -o nbmake *.o
main.o: In function `main':
main.c:(.text+0x2362): undefined reference to `estrdup'
main.c:(.text+0x23ca): undefined reference to `estrdup'
main.c:(.text+0x2d6d): undefined reference to `emalloc'

ERROR: Build of nbmake failed
*** BUILD ABORTED ***
r

--
Ryo ONODERA // ryo...@yk.rim.or.jp
PGP fingerprint = 82A2 DC91 76E0 A10A 8ABB  FD1B F404 27FA C7D1 15F3

Re: CVS commit: src/usr.bin/make

[Christos Zoulas]

On Jul 16, 11:20pm, ryo...@yk.rim.or.jp (Ryo ONODERA) wrote:
-- Subject: Re: CVS commit: src/usr.bin/make

| With this change, I have gotten following errors.
| I think -lutil is needed.

No, I fixed it... I was supposed to use bmake_foo() not efoo().

christos

Re: CVS commit: src/usr.bin/make

[Ryo ONODERA]

From: chris...@zoulas.com (Christos Zoulas), Date: Tue, 16 Jul 2013 10:23:04 
-0400

 On Jul 16, 11:20pm, ryo...@yk.rim.or.jp (Ryo ONODERA) wrote:
 -- Subject: Re: CVS commit: src/usr.bin/make
 
 | With this change, I have gotten following errors.
 | I think -lutil is needed.
 
 No, I fixed it... I was supposed to use bmake_foo() not efoo().

Thank you very much!

--
Ryo ONODERA // ryo...@yk.rim.or.jp
PGP fingerprint = 82A2 DC91 76E0 A10A 8ABB  FD1B F404 27FA C7D1 15F3

Re: CVS commit: src/usr.bin/make

[David Holland]

On Tue, Jul 16, 2013 at 08:00:56PM +, Simon J. Gerraty wrote:
  Modified Files:
   src/usr.bin/make: var.c
  
  Log Message:
  When a var is set in the CMD context, it prevents the same name
  being set in GLOBAL context.  We should also delete any such
  variable in GLOBAL context, else make -V will show the wrong value.

Example?

-- 
David A. Holland
dholl...@netbsd.org

Re: CVS commit: src/usr.bin/make

[Christos Zoulas]

In article krbu04$2j1$1...@ger.gmane.org,
Christos Zoulas chris...@astron.com wrote:
Module Name:  src
Committed By: sjg
Date: Sat Jul  6 18:19:17 UTC 2013

Modified Files:
  src/usr.bin/make: main.c var.c

Log Message:
If using gmake's MAKELEVEL; use it the same way

Now you put it back the way it was before which is wrong. Gmake does not
behave this way. Before your change the following Makefile printed:

This is still broken. Renaming the variable is not a fix either.
Consider the case where we switched from bmake to gmake as the
pkgsrc wrapper. The packages that depend on $MAKELEVEL being 0 or
unset would still break. So the proper fix for the 2 packages that
broke is to make pkgsrc unset MAKELEVEL before invoking gmake.

So to fix those:
- revert the changes so that MAKELEVEL again works the same way as in gmake.
- add glue to invoke gmake with MAKELEVEL unset.

christos


--- Makefile ---
all:
   echo makelevel=${MAKELEVEL} make=${MAKE} makeflags=${MAKEFLAGS}
   echo .makelevel=${.MAKELEVEL}
   ${MAKE} l1

l1:
   echo makelevel=${MAKELEVEL} make=${MAKE} makeflags=${MAKEFLAGS}
   echo .makelevel=${.MAKELEVEL}
--- Output gmake ---
echo makelevel=0 make=gmake makeflags=
makelevel=0 make=gmake makeflags=
echo .makelevel=
.makelevel=
gmake l1
gmake[1]: Entering directory `/u/christos'
echo makelevel=1 make=gmake makeflags=w
makelevel=1 make=gmake makeflags=w
echo .makelevel=
.makelevel=
gmake[1]: Leaving directory `/u/christos'
--- Output make before your fixes ---
echo makelevel=0 make=make makeflags= 
makelevel=0 make=make makeflags=
echo .makelevel=0
.makelevel=0
make l1
echo makelevel=1 make=make makeflags= 
makelevel=1 make=make makeflags=
echo .makelevel=1
.makelevel=1
--- Output make after your fixes --
echo makelevel=1 make=/usr/src/usr.bin/make/obj.amd64/make makeflags= 
makelevel=1 make=/usr/src/usr.bin/make/obj.amd64/make makeflags=
echo .makelevel=
.makelevel=
/usr/src/usr.bin/make/obj.amd64/make l1
echo makelevel=2 make=/usr/src/usr.bin/make/obj.amd64/make makeflags= 
makelevel=2 make=/usr/src/usr.bin/make/obj.amd64/make makeflags=
echo .makelevel=
.makelevel=

As I mentioned before, it makes no sense to keep the variable internally at
X and pass it to the program at X + 1, and as demonstrated, this is not
what gmake does.

christos

Re: CVS commit: src/usr.bin/make

[Ryo ONODERA]

From: chris...@astron.com (Christos Zoulas), Date: Wed, 10 Jul 2013 13:08:49 
+ (UTC)

 In article krbu04$2j1$1...@ger.gmane.org,
 Christos Zoulas chris...@astron.com wrote:
Module Name: src
Committed By:sjg
Date:Sat Jul  6 18:19:17 UTC 2013

Modified Files:
 src/usr.bin/make: main.c var.c

Log Message:
If using gmake's MAKELEVEL; use it the same way

Now you put it back the way it was before which is wrong. Gmake does not
behave this way. Before your change the following Makefile printed:
 
 This is still broken. Renaming the variable is not a fix either.
 Consider the case where we switched from bmake to gmake as the
 pkgsrc wrapper. The packages that depend on $MAKELEVEL being 0 or
 unset would still break. So the proper fix for the 2 packages that
 broke is to make pkgsrc unset MAKELEVEL before invoking gmake.
 
 So to fix those:
 - revert the changes so that MAKELEVEL again works the same way as in gmake.
 - add glue to invoke gmake with MAKELEVEL unset.

Hi,

I feel second idea is not so good.
Because pkgsrc creates symlink, work/.tools/bin/make, for our make and gmake.
So with gmake in USE_TOOLS, it is symlink for /usr/pkg/bin/gmake,
and without gmake in USE_TOOLS, it is for /usr/bin/make.
I cannot distinguish them with filename.

Thank you.

--
Ryo ONODERA // ryo...@yk.rim.or.jp
PGP fingerprint = 82A2 DC91 76E0 A10A 8ABB  FD1B F404 27FA C7D1 15F3

Re: CVS commit: src/usr.bin/make

[Christos Zoulas]

In article 20130710.232445.202823964487691297.ryo...@yk.rim.or.jp,
Ryo ONODERA  ryo...@yk.rim.or.jp wrote:
From: chris...@astron.com (Christos Zoulas), Date: Wed, 10 Jul 2013
13:08:49 + (UTC)

 In article krbu04$2j1$1...@ger.gmane.org,
 Christos Zoulas chris...@astron.com wrote:
Module Name:src
Committed By:   sjg
Date:   Sat Jul  6 18:19:17 UTC 2013

Modified Files:
src/usr.bin/make: main.c var.c

Log Message:
If using gmake's MAKELEVEL; use it the same way

Now you put it back the way it was before which is wrong. Gmake does not
behave this way. Before your change the following Makefile printed:
 
 This is still broken. Renaming the variable is not a fix either.
 Consider the case where we switched from bmake to gmake as the
 pkgsrc wrapper. The packages that depend on $MAKELEVEL being 0 or
 unset would still break. So the proper fix for the 2 packages that
 broke is to make pkgsrc unset MAKELEVEL before invoking gmake.
 
 So to fix those:
 - revert the changes so that MAKELEVEL again works the same way as in gmake.
 - add glue to invoke gmake with MAKELEVEL unset.

Hi,

I feel second idea is not so good.
Because pkgsrc creates symlink, work/.tools/bin/make, for our make and gmake.
So with gmake in USE_TOOLS, it is symlink for /usr/pkg/bin/gmake,
and without gmake in USE_TOOLS, it is for /usr/bin/make.
I cannot distinguish them with filename.

It is simple:

1. You unset it for all (easiest+safest)
2. You create a shell script instead of a symlink that does:
unset MAKELEVEL  exec /usr/pkg/bin/gmake $@

christos

Re: CVS commit: src/usr.bin/make

[Ryo ONODERA]

From: chris...@astron.com (Christos Zoulas), Date: Wed, 10 Jul 2013 15:08:39 
+ (UTC)

 In article 20130710.232445.202823964487691297.ryo...@yk.rim.or.jp,
 Ryo ONODERA  ryo...@yk.rim.or.jp wrote:
From: chris...@astron.com (Christos Zoulas), Date: Wed, 10 Jul 2013
13:08:49 + (UTC)

 In article krbu04$2j1$1...@ger.gmane.org,
 Christos Zoulas chris...@astron.com wrote:
Module Name:   src
Committed By:  sjg
Date:  Sat Jul  6 18:19:17 UTC 2013

Modified Files:
   src/usr.bin/make: main.c var.c

Log Message:
If using gmake's MAKELEVEL; use it the same way

Now you put it back the way it was before which is wrong. Gmake does not
behave this way. Before your change the following Makefile printed:
 
 This is still broken. Renaming the variable is not a fix either.
 Consider the case where we switched from bmake to gmake as the
 pkgsrc wrapper. The packages that depend on $MAKELEVEL being 0 or
 unset would still break. So the proper fix for the 2 packages that
 broke is to make pkgsrc unset MAKELEVEL before invoking gmake.
 
 So to fix those:
 - revert the changes so that MAKELEVEL again works the same way as in gmake.
 - add glue to invoke gmake with MAKELEVEL unset.

Hi,

I feel second idea is not so good.
Because pkgsrc creates symlink, work/.tools/bin/make, for our make and gmake.
So with gmake in USE_TOOLS, it is symlink for /usr/pkg/bin/gmake,
and without gmake in USE_TOOLS, it is for /usr/bin/make.
I cannot distinguish them with filename.
 
 It is simple:
 
 1. You unset it for all (easiest+safest)
 2. You create a shell script instead of a symlink that does:
 unset MAKELEVEL  exec /usr/pkg/bin/gmake $@

Thanks for your explanation.

In pkgsrc case, shell script may work.
Outside pkgsrc, some program may be broken by this make(1) change.
I vote to revert the changes so that MAKELEVEL again works
the same way as in gmake.

Thank you.

--
Ryo ONODERA // ryo...@yk.rim.or.jp
PGP fingerprint = 82A2 DC91 76E0 A10A 8ABB  FD1B F404 27FA C7D1 15F3

Re: CVS commit: src/usr.bin/make

[Christos Zoulas]

Module Name:   src
Committed By:  sjg
Date:  Sat Jul  6 18:19:17 UTC 2013

Modified Files:
   src/usr.bin/make: main.c var.c

Log Message:
If using gmake's MAKELEVEL; use it the same way

Now you put it back the way it was before which is wrong. Gmake does not
behave this way. Before your change the following Makefile printed:

--- Makefile ---
all:
echo makelevel=${MAKELEVEL} make=${MAKE} makeflags=${MAKEFLAGS}
echo .makelevel=${.MAKELEVEL}
${MAKE} l1

l1:
echo makelevel=${MAKELEVEL} make=${MAKE} makeflags=${MAKEFLAGS}
echo .makelevel=${.MAKELEVEL}
--- Output gmake ---
echo makelevel=0 make=gmake makeflags=
makelevel=0 make=gmake makeflags=
echo .makelevel=
.makelevel=
gmake l1
gmake[1]: Entering directory `/u/christos'
echo makelevel=1 make=gmake makeflags=w
makelevel=1 make=gmake makeflags=w
echo .makelevel=
.makelevel=
gmake[1]: Leaving directory `/u/christos'
--- Output make before your fixes ---
echo makelevel=0 make=make makeflags= 
makelevel=0 make=make makeflags=
echo .makelevel=0
.makelevel=0
make l1
echo makelevel=1 make=make makeflags= 
makelevel=1 make=make makeflags=
echo .makelevel=1
.makelevel=1
--- Output make after your fixes --
echo makelevel=1 make=/usr/src/usr.bin/make/obj.amd64/make makeflags= 
makelevel=1 make=/usr/src/usr.bin/make/obj.amd64/make makeflags=
echo .makelevel=
.makelevel=
/usr/src/usr.bin/make/obj.amd64/make l1
echo makelevel=2 make=/usr/src/usr.bin/make/obj.amd64/make makeflags= 
makelevel=2 make=/usr/src/usr.bin/make/obj.amd64/make makeflags=
echo .makelevel=
.makelevel=

As I mentioned before, it makes no sense to keep the variable internally at
X and pass it to the program at X + 1, and as demonstrated, this is not
what gmake does.

christos

Re: CVS commit: src/usr.bin/make

[Christos Zoulas]

In article 20130618200609.a234...@cvs.netbsd.org,
Simon J. Gerraty source-changes-d@NetBSD.org wrote:
-=-=-=-=-=-

Module Name:   src
Committed By:  sjg
Date:  Tue Jun 18 20:06:09 UTC 2013

Modified Files:
   src/usr.bin/make: main.c make.h var.c

Log Message:
Use a #define for the variable name we put in environment to pass .MAKE.LEVEL
in case we don't want to use gmake's MAKELEVEL in a different way.



Can you do this for all the environment strings. I think there are some
onconsistencies.

christos

Re: CVS commit: src/usr.bin/make

[Christos Zoulas]

On Mar 8,  5:21pm, lu...@netbsd.org (Luke Mewburn) wrote:
-- Subject: Re: CVS commit: src/usr.bin/make

| Removing the source files would be an epic,
| if brutal, method of fixing the problem. :-)

I think it is still possible to do this with the code I added :-)

christos

Re: CVS commit: src/usr.bin/make

[Luke Mewburn]

On Tue, Mar 05, 2013 at 08:35:42PM +, Christos Zoulas wrote:
  | In article 
CADbF7eeV2AcmiVZJzuxsN8gvwQHfMLYVN63gRdXpv8-jV=x...@mail.gmail.com,
  | Masao Uebayashi  uebay...@gmail.com wrote:
  | -=-=-=-=-=-
  | 
  | I like make(1) to exit when stale depend is found.
  | 
  | I was thinking of adding a .STALE target, so you can do what you like,
  | for example:
  | 
  | .STALE:
  | rm -f ${.ALLSRC}
  | ${MAKE} depend

Removing the source files would be an epic,
if brutal, method of fixing the problem. :-)

cheers,
Luke.


pgpnkvHs9rnXE.pgp
Description: PGP signature

Re: CVS commit: src/usr.bin/make

[Christos Zoulas]

In article 20130306073342.gg4...@apb-laptoy.apb.alt.za,
Alan Barrett  a...@cequrux.com wrote:
On Tue, 05 Mar 2013, Christos Zoulas wrote:
Modified Files:
  src/usr.bin/make: dir.c job.c job.h make.1 parse.c

Log Message:
Add a .STALE special target that gets invoked when dependency files contain
stail entries.

Please discuss this sort of thing first.  There may be a way to 
solve the underlying problem without adding a new special target, 
such as using the newish meta mode.  Also, a better name than 
.STALE might have been suggested.

It is not enabled, so now is a good time to provide alternatives.

christos

Re: CVS commit: src/usr.bin/make

[Masao Uebayashi]

I like make(1) to exit when stale depend is found.

On Tue, Mar 5, 2013 at 11:04 AM, Christos Zoulas chris...@netbsd.orgwrote:

 Module Name:src
 Committed By:   christos
 Date:   Tue Mar  5 02:04:11 UTC 2013

 Modified Files:
 src/usr.bin/make: dir.c job.c parse.c

 Log Message:
 Keep track of the location where a dependency is defined, so we can report
 about it.


 To generate a diff of this commit:
 cvs rdiff -u -r1.65 -r1.66 src/usr.bin/make/dir.c
 cvs rdiff -u -r1.170 -r1.171 src/usr.bin/make/job.c
 cvs rdiff -u -r1.185 -r1.186 src/usr.bin/make/parse.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

Re: CVS commit: src/usr.bin/make

[Christos Zoulas]

In article CADbF7eeV2AcmiVZJzuxsN8gvwQHfMLYVN63gRdXpv8-jV=x...@mail.gmail.com,
Masao Uebayashi  uebay...@gmail.com wrote:
-=-=-=-=-=-

I like make(1) to exit when stale depend is found.

I was thinking of adding a .STALE target, so you can do what you like,
for example:

.STALE:
rm -f ${.ALLSRC}
${MAKE} depend

christos

Re: CVS commit: src/usr.bin/make

[Alan Barrett]

On Tue, 05 Mar 2013, Christos Zoulas wrote:

Modified Files:
src/usr.bin/make: dir.c job.c job.h make.1 parse.c

Log Message:
Add a .STALE special target that gets invoked when dependency files contain
stail entries.


Please discuss this sort of thing first.  There may be a way to 
solve the underlying problem without adding a new special target, 
such as using the newish meta mode.  Also, a better name than 
.STALE might have been suggested.


--apb (Alan Barrett)

Re: CVS commit: src/usr.bin/make

[David Laight]

On Sat, Jan 26, 2013 at 10:53:00AM -0500, Christos Zoulas wrote:
 Module Name:  src
 Committed By: christos
 Date: Sat Jan 26 15:53:00 UTC 2013
 
 Modified Files:
   src/usr.bin/make: job.c main.c
 
 Log Message:
 Check read and write errors to avoid warnings from linux.
 XXX: Should we print an error and exit instead?
 
 
 To generate a diff of this commit:
 cvs rdiff -u -r1.164 -r1.165 src/usr.bin/make/job.c
 cvs rdiff -u -r1.204 -r1.205 src/usr.bin/make/main.c

I'd have been tempted to do:

int stupid_glibc_wont_let_us_ignore_the_result_of_write(int fd, const void 
*buf, size_t len)
{
return write(fd, buf, len);
}

#define write(fd, buf,len) 
stupid_glibc_wont_let_us_ignore_the_result_of_write(fd, buf, len)

Certainly the accesses to the pipes are very boring.

The read(childExitJob.inPipe, ...) definitely doesn't need checking at all.
The writes to it are NON_BLOCK (I think) - it is only used to wake from
poll() - the read is there to clear POLLIN.

The job token pipe is slightly more interesting - but it will only be full
if you try to do more jobs than the pipe size - and then you need to discard
a job token, not block.

I'm not sure, but I think that read/write can only return EAGAIN if they
are blocking, have transferred no data, and take a signal that is
restartable.

If they have transferred some data they they have to return a partial count.

So if you care about the result you have to do far more than loop
for EAGAIN - adding such a loop is a bogus fix.

Not much point writing an error is you'vejust failed to write to stderr!

David

-- 
David Laight: da...@l8s.co.uk

Re: CVS commit: src/usr.bin/make

[Christos Zoulas]

In article 20130126205325.gd28...@snowdrop.l8s.co.uk,
David Laight  da...@l8s.co.uk wrote:

I'd have been tempted to do:

int stupid_glibc_wont_let_us_ignore_the_result_of_write(int fd, const
void *buf, size_t len)
{
   return write(fd, buf, len);
}

It is the linux headers :-)

I'm not sure, but I think that read/write can only return EAGAIN if they
are blocking, have transferred no data, and take a signal that is
restartable.

The other way around O_NONBLOCK reads and writes can return EAGAIN on
non-files.

If they have transferred some data they they have to return a partial count.

But if they are empty(read) or full(write) and non-blocking they have to
return EAGAIN (or the old EWOULDBLOCK).

So if you care about the result you have to do far more than loop
for EAGAIN - adding such a loop is a bogus fix.

If they are non blocking it is correct; if they are blocking it is a noop.

Not much point writing an error is you'vejust failed to write to stderr!

correct.

christos

re: CVS commit: src/usr.bin/make

[matthew green]

 Not much point writing an error is you'vejust failed to write to stderr!

i disagree -- ktrace sees it and thus it is not entirely lost :)

Re: CVS commit: src/usr.bin/make

[David Laight]

On Sat, Nov 03, 2012 at 02:25:14AM +, Simon J. Gerraty wrote:
 Module Name:  src
 Committed By: sjg
 Date: Sat Nov  3 02:25:13 UTC 2012
 
 Modified Files:
   src/usr.bin/make: cond.c
 
 Log Message:
 Allow cond_state[] to grow.
 The need is rare, but real.

128 nested conditional was expected to be far more than imaginable.

There is also an off-by-one error is the new code.
The original code had:
static enum if_states cond_state[MAXIF + 1] = { IF_ACTIVE };
(note the +1)
This isn't allowed for in the new code - and it will overwrite
the end of the malloced area.

You need to move the realloc() below the cond_depth+++

I'm then not sure that 128+n*32 is a sane sequence.
Possibly just n*32 - since I suspect 32 is plenty for most makefiles.

David

-- 
David Laight: da...@l8s.co.uk

Re: CVS commit: src/usr.bin/make

[Christos Zoulas]

In article 20120831070037.6fb7a17...@cvs.netbsd.org,
Simon J. Gerraty source-changes-d@NetBSD.org wrote:
-=-=-=-=-=-

Module Name:   src
Committed By:  sjg
Date:  Fri Aug 31 07:00:36 UTC 2012

Modified Files:
   src/usr.bin/make: main.c

Log Message:
Cast sizeof() to int, which is sufficent and consistent with other use
within make.

snprintf() can return -1 so the second part is wrong.

christos

Re: CVS commit: src/usr.bin/make

[Matt Thomas]

On Jun 10, 2011, at 4:57 PM, Simon J. Gerraty wrote:

 Module Name:  src
 Committed By: sjg
 Date: Fri Jun 10 23:57:39 UTC 2011
 
 Modified Files:
   src/usr.bin/make: meta.c
 
 Log Message:
 size_t on amd64 doesn't like %u, use a cast.

why not use %zu?

Re: CVS commit: src/usr.bin/make

[Jukka Ruohonen]

On Fri, Jun 10, 2011 at 11:57:39PM +, Simon J. Gerraty wrote:
 Module Name:  src
 Committed By: sjg
 Date: Fri Jun 10 23:57:39 UTC 2011
 
 Modified Files:
   src/usr.bin/make: meta.c
 
 Log Message:
 size_t on amd64 doesn't like %u, use a cast.

%zu?

- Jukka.

re: CVS commit: src/usr.bin/make

[matthew green]

 On Fri, Jun 10, 2011 at 11:57:39PM +, Simon J. Gerraty wrote:
  Module Name:src
  Committed By:   sjg
  Date:   Fri Jun 10 23:57:39 UTC 2011
  
  Modified Files:
  src/usr.bin/make: meta.c
  
  Log Message:
  size_t on amd64 doesn't like %u, use a cast.
 
 %zu?

%z = size_t
%u = unsigned
%d = signed

thus

%zd = ssize_t
%zu = size_t


.mrg.

Re: CVS commit: src/usr.bin/make

[David Laight]

On Wed, May 18, 2011 at 02:43:03AM +, David Holland wrote:
 On Tue, May 17, 2011 at 09:56:52PM +, David Laight wrote:
   cvs rdiff -u -r1.51 -r1.52 src/usr.bin/make/Makefile
 
 uh, did you mean to commit that?

No - I've changed it back.

David

-- 
David Laight: da...@l8s.co.uk

Re: CVS commit: src/usr.bin/make

[David Holland]

On Tue, May 17, 2011 at 09:56:52PM +, David Laight wrote:
  cvs rdiff -u -r1.51 -r1.52 src/usr.bin/make/Makefile

uh, did you mean to commit that?

-- 
David A. Holland
dholl...@netbsd.org

Re: CVS commit: src/usr.bin/make

[Joerg Sonnenberger]

On Thu, Apr 07, 2011 at 02:49:31AM +0100, Mindaugas Rasiukevicius wrote:
 Joerg Sonnenberger jo...@netbsd.org wrote:
  Module Name:src
  Committed By:   joerg
  Date:   Thu Apr  7 01:40:02 UTC 2011
  
  Modified Files:
  src/usr.bin/make: make.1 var.c
  src/usr.bin/make/unit-tests: Makefile test.exp
  Added Files:
  src/usr.bin/make/unit-tests: hash
  
  Log Message:
  Add the :hash modifier to compute a 32bit hash of an variable.
  This uses MurmurHash3 to get a reasonable collission-free hash with
  small code. The result is endian neutral.
 
 Cool.  Can you please move MurmurHash3 hash into src/common?  I have
 been planning to use it in NPF, and potentially there are more uses in
 the kernel, since it has a great speed and mixing trade-off.

No. make has to be self-contained, so that doesn't make sense.

Joerg

Re: CVS commit: src/usr.bin/make

[Martin Husemann]

On Thu, Nov 25, 2010 at 04:31:10PM -0500, Christos Zoulas wrote:
 Module Name:  src
 Committed By: christos
 Date: Thu Nov 25 21:31:09 UTC 2010
 
 Modified Files:
   src/usr.bin/make: arch.c compat.c make.c make.h targ.c
 
 Log Message:
 Instead of keeping around the mtime of the youngest child, keep a pointer
 to it, so that we can print it when we do the out of date determination.

This makes -current fail on amd64:

--- dependall-gnu ---
[1]   Segmentation fault (core dumped) /usr/tools/bin/n...
*** [dependall-host-libiberty] Error code 139

Martin

Re: CVS commit: src/usr.bin/make

[Christos Zoulas]

In article 20101126105706.ga18...@homeworld.netbsd.org,
Martin Husemann  mar...@homeworld.netbsd.org wrote:
On Thu, Nov 25, 2010 at 04:31:10PM -0500, Christos Zoulas wrote:
 Module Name: src
 Committed By:christos
 Date:Thu Nov 25 21:31:09 UTC 2010
 
 Modified Files:
  src/usr.bin/make: arch.c compat.c make.c make.h targ.c
 
 Log Message:
 Instead of keeping around the mtime of the youngest child, keep a pointer
 to it, so that we can print it when we do the out of date determination.

This makes -current fail on amd64:

--- dependall-gnu ---
[1]   Segmentation fault (core dumped) /usr/tools/bin/n...
*** [dependall-host-libiberty] Error code 139

I must have forgotten to test for cmgn NULL somewhere; where does gdb say
that it dies?

christos

Re: CVS commit: src/usr.bin/make

[Christoph Egger]

 Original-Nachricht 
 Datum: Fri, 26 Nov 2010 13:56:22 + (UTC)
 Von: chris...@astron.com
 An: source-changes-d@NetBSD.org
 Betreff: Re: CVS commit: src/usr.bin/make

 In article 20101126105706.ga18...@homeworld.netbsd.org,
 Martin Husemann  mar...@homeworld.netbsd.org wrote:
 On Thu, Nov 25, 2010 at 04:31:10PM -0500, Christos Zoulas wrote:
  Module Name:   src
  Committed By:  christos
  Date:  Thu Nov 25 21:31:09 UTC 2010
  
  Modified Files:
 src/usr.bin/make: arch.c compat.c make.c make.h targ.c
  
  Log Message:
  Instead of keeping around the mtime of the youngest child, keep a
 pointer
  to it, so that we can print it when we do the out of date
 determination.
 
 This makes -current fail on amd64:
 
 --- dependall-gnu ---
 [1]   Segmentation fault (core dumped) /usr/tools/bin/n...
 *** [dependall-host-libiberty] Error code 139

BTW: It works on a clean build.

 I must have forgotten to test for cmgn NULL somewhere; where
 does gdb say that it dies?

Core was generated by `nbmake'.
Program terminated with signal 11, Segmentation fault.
#0  0x0040261b in Arch_LibOODate ()
(gdb) bt
#0  0x0040261b in Arch_LibOODate ()
#1  0x0040f919 in Make_OODate ()
#2  0x0040fcde in MakeStartJobs ()
#3  0x0040ff6c in Make_Run ()
#4  0x0040e7b3 in main ()
(gdb) 

Re: CVS commit: src/usr.bin/make

[Christos Zoulas]

On Nov 26,  3:34pm, christoph_eg...@gmx.de (Christoph Egger) wrote:
-- Subject: Re: CVS commit: src/usr.bin/make

| BTW: It works on a clean build.
| 
|  I must have forgotten to test for cmgn NULL somewhere; where
|  does gdb say that it dies?
| 
| Core was generated by `nbmake'.
| Program terminated with signal 11, Segmentation fault.
| #0  0x0040261b in Arch_LibOODate ()
| (gdb) bt
| #0  0x0040261b in Arch_LibOODate ()
| #1  0x0040f919 in Make_OODate ()
| #2  0x0040fcde in MakeStartJobs ()
| #3  0x0040ff6c in Make_Run ()
| #4  0x0040e7b3 in main ()
| (gdb) 

Thanks, try now!

christos

Re: CVS commit: src/usr.bin/make

[Christoph Egger]

 On Nov 26,  3:34pm, christoph_eg...@gmx.de (Christoph Egger) wrote:
 -- Subject: Re: CVS commit: src/usr.bin/make
 
 | BTW: It works on a clean build.
 | 
 |  I must have forgotten to test for cmgn NULL somewhere; where
 |  does gdb say that it dies?
 | 
 | Core was generated by `nbmake'.
 | Program terminated with signal 11, Segmentation fault.
 | #0  0x0040261b in Arch_LibOODate ()
 | (gdb) bt
 | #0  0x0040261b in Arch_LibOODate ()
 | #1  0x0040f919 in Make_OODate ()
 | #2  0x0040fcde in MakeStartJobs ()
 | #3  0x0040ff6c in Make_Run ()
 | #4  0x0040e7b3 in main ()
 | (gdb) 
 
 Thanks, try now!

Works for me. Tnx for fixing.

Christoph

Re: CVS commit: src/usr.bin/make

[Joerg Sonnenberger]

On Wed, Jun 09, 2010 at 12:58:23PM -0400, Christos Zoulas wrote:
 Log Message:
 Explain variable expansion better. Requested by Aleksey Cheusov

This is wrong. Loop variables are not exapnded on each loop iteration.
Each loop iteration effectively creates a new variable. The rest of his
confusion comes from two simple facts:
(1) += is lazy in bmake. This is different from FreeBSD, where it forces
expansion.
(2) The evaluation of j is lazy as well. That's why he sees the last
loop iteration.

Therefore I don't agree with the content and it is actually making
things a lot more complicated than necessary.

Joerg

Re: CVS commit: src/usr.bin/make

[Christos Zoulas]

In article 20100609171621.ga23...@britannica.bec.de,
Joerg Sonnenberger  jo...@britannica.bec.de wrote:
On Wed, Jun 09, 2010 at 12:58:23PM -0400, Christos Zoulas wrote:
 Log Message:
 Explain variable expansion better. Requested by Aleksey Cheusov

This is wrong. Loop variables are not exapnded on each loop iteration.
Each loop iteration effectively creates a new variable. The rest of his
confusion comes from two simple facts:
(1) += is lazy in bmake. This is different from FreeBSD, where it forces
expansion.
(2) The evaluation of j is lazy as well. That's why he sees the last
loop iteration.

Well, it was true when I wrote the for code (more than 16 years
ago!). I guess dsl re-wrote it, but the net effect is the same.

When did FreeBSD changed += not to be lazy? That would break a lot
of existing Makefiles I presume.

Therefore I don't agree with the content and it is actually making
things a lot more complicated than necessary.

How is the net effect different than what I am describing? Feel free
to improve the explanation, to match the actual implementation.

christos