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

[Simon J. Gerraty]

Taylor R Campbell  wrote:
> Can you please back this out promptly, add automatic tests for
> whatever the underlying issue is, and redo it another way?

I did a scan of miss-use of <>, and looks like libc might
have an issue:

lib/libc/arch/arm/Makefile.inc:28:.include 

expects to find lib/libc/softfloat/Makefile.inc

via .CURDIR, so have re-enabled that search.

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

[Simon J. Gerraty]

Taylor R Campbell  wrote:
> 
> --- cleandir-libterminfo ---
> nbmake[5]: "/tmp/build/2024.05.19.20.09.40-i386/src/lib/libterminfo/Makefile" 
> line 50: Could not find Makefile.hash
> 
> Can you please back this out promptly, add automatic tests for
> whatever the underlying issue is, and redo it another way?

Sorry, for the break.
The makefile in lib/libterminfo was using <> when it should use "".
Martin already fixed it.

I'm checking for similar 

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

[Taylor R Campbell]

> Module Name:src
> Committed By:   sjg
> Date:   Sun May 19 20:09:40 UTC 2024
> 
> Modified Files:
> src/usr.bin/make: dir.c dir.h parse.c
> 
> Log Message:
> make: use separate function to include makefiles.
> 
> Have Dir_FindFile and Dir_FindInclude call FindFile with a
> bool flag to indicate whether .CURDIR should be be searched at all.

This appears to have broken all of the builds:

https://releng.netbsd.org/builds/HEAD/202405200920Z/
https://releng.netbsd.org/b5reports/i386/commits-2024.05.html#2024.05.19.20.09.40
https://releng.netbsd.org/b5reports/i386/2024/2024.05.19.20.09.40/build.log.tail

--- cleandir-libterminfo ---
nbmake[5]: "/tmp/build/2024.05.19.20.09.40-i386/src/lib/libterminfo/Makefile" 
line 50: Could not find Makefile.hash

Can you please back this out promptly, add automatic tests for
whatever the underlying issue is, and redo it another way?

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

[Simon Gerraty]

On Thu, 25 Jan 2024 20:02:00 +0100, Roland Illig writes:
>>> Modified Files:
>>> src/usr.bin/make: make.1
>>>
>>> Log Message:
>>> Indicate that for :U newval is optional
>>
>> I think this is more confusing than helpful.
>
>I agree. Make doesn't distinguish between an empty string and an absent
>string here, so the wording in the manual page should be kept equally
>simple.

Not a question of what make likes or not, but what users glean from the
man page (those that don't also read the source).

The change was prompted by a user (not a make novice by any means)
complaining that :Unewval and the description provides no clue
that just :U is actually valid.

>If anything, you might mention that newval may be empty, but then you'd

I think I started with that, but then figured someone sufficiently
pedantic might consider that not the same as no value being provided ;-)

>have to add the same wording in many other places, for consistency:

>* the variable name in ${:Uvalue}

This is covered by the above no?

>* the two branches of the ':?' modifier in ${cond:?}

true

>* the pattern matching modifiers after ':S,from,to' (1gW)
>* the variable name in the variable assignment '!=3Dcmd'
>* the argument of a function call in '.if defined()'
>
>Most of these cases don't need this wording, and the '!=3Dcmd' case

Agreed, the :S usage at least is sufficiently familiar to anyone who's
used sed(1) to not need elaboration.

Is there perhaps a general statement somewhere (I may have missed it)
that could cover all these and be cited to pedantic users?
Eg to the effect of perhaps, unless stated otherwise arguments to
modifiers are optional or can be empty.

I don't think its there and is hard to word succinctly.

Thanks
--sjg

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

[Simon Gerraty]

On Thu, 25 Jan 2024 21:12:20 +0100, Roland Illig writes:
>them all. Due to this, I'd go with:
>
>> .It Cm \&:U\| Ns Ar newval
>> If the variable is undefined,
>> .Ar newval
>> (which may be empty) is the value.

That's almost exactly what I had in my 1st cut ;-)
Will do.

>Then, add the same wording to ':D' and maybe to ':N' (since ':N' is a
>shortcut for the more common ':M*') but not to ':M' itself (as that
>would always result in an empty string, making it uninteresting).

Will take a look, but not so sure.

--sjg

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

[Roland Illig]

About optional arguments to modifiers, such as in ${VAR:U}:

Am 25.01.2024 um 20:54 schrieb Simon Gerraty:
> Is there perhaps a general statement somewhere (I may have missed it)
> that could cover all these and be cited to pedantic users?
> Eg to the effect of perhaps, unless stated otherwise arguments to
> modifiers are optional or can be empty.
>
> I don't think its there and is hard to word succinctly.

No, I searched through the manual page but didn't find one.

Also, there are so many syntactic quirks and edge cases in make that I
don't think there's an easy to grasp single-line statement that covers
them all. Due to this, I'd go with:

> .It Cm \&:U\| Ns Ar newval
> If the variable is undefined,
> .Ar newval
> (which may be empty) is the value.

Then, add the same wording to ':D' and maybe to ':N' (since ':N' is a
shortcut for the more common ':M*') but not to ':M' itself (as that
would always result in an empty string, making it uninteresting).

Roland

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

[Roland Illig]

Am 25.01.2024 um 14:25 schrieb Valery Ushakov:
> On Thu, Jan 25, 2024 at 07:35:46 +, Simon J. Gerraty wrote:
>
>> Modified Files:
>>  src/usr.bin/make: make.1
>>
>> Log Message:
>> Indicate that for :U newval is optional
>
> I think this is more confusing than helpful.

I agree. Make doesn't distinguish between an empty string and an absent
string here, so the wording in the manual page should be kept equally
simple.

If anything, you might mention that newval may be empty, but then you'd
have to add the same wording in many other places, for consistency:

* the variable name in ${:Uvalue}
* the two branches of the ':?' modifier in ${cond:?}
* the pattern matching modifiers after ':S,from,to' (1gW)
* the variable name in the variable assignment '!=cmd'
* the argument of a function call in '.if defined()'

Most of these cases don't need this wording, and the '!=cmd' case
shouldn't be documented at all, as it is a dirty hack.

Roland

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

[bch]

On Fri, Sep 8, 2023 at 21:38 matthew green  wrote:

> Module Name:src
> Committed By:   mrg
> Date:   Sat Sep  9 04:38:49 UTC 2023
>
> Modified Files:
> src/usr.bin/make: main.c
>
> Log Message:
> add explicit cast for long -> int truncation warning-as-error.
>
> as this is number of CPUs, i don't think we have to care about
> it for a long, long, *long* time...


Pun intended?



>
>
> To generate a diff of this commit:
> cvs rdiff -u -r1.594 -r1.595 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.
>
>

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

[Simon J. Gerraty]

Taylor R Campbell  wrote:
> That said, I don't see any reason why this should be a macro in the
> first place.  If there is a compelling reason, please write it down;
> if not, please change it to a static function:
> 

Sure.

> static BuildMon *
> BM(Job *job)
> {
> 
> return (job != NULL ? >bm : );
> }

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

[Taylor R Campbell]

> Module Name:src
> Committed By:   sjg
> Date:   Sat Feb 25 22:52:22 UTC 2023
> 
> Modified Files:
> src/usr.bin/make: meta.c
> 
> Log Message:
> meta.c: use macro to access job->bm
> 
> and if job is NULL use Mybm.

> +#define BM(job) (job != NULL) ? >bm : 

If this must be a macro, it should have more parentheses to avoid
confusion at use sites:

#define BM(job) (((job) != NULL) ? &(job)->bm : )

As is, for instance,

BM(jobarray + i)

or

BM(job)->member

will do the wrong thing (perhaps with a compile error but needlessly
confusing nonetheless).

That said, I don't see any reason why this should be a macro in the
first place.  If there is a compelling reason, please write it down;
if not, please change it to a static function:

static BuildMon *
BM(Job *job)
{

return (job != NULL ? >bm : );
}

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

[Robert Elz]

Date:Mon, 10 Oct 2022 17:33:35 +
From:"Roland Illig" 
Message-ID:  <20221010173335.c3cccf...@cvs.netbsd.org>

  | Document only the POSIX requirement for now, as I didn't find
  | information about _which_ ancient UNIX systems would corrupt the
  | filesystem on unlinking a directory.

Everthinng before the rename() mkdir() and rmdir() sys calls
were added, which was about 4.2bsd I think - and as I recall
the ability remained for some time after those were added.

Not sure when the UTS stream caught up, probably about the
time when Sun switched SunOS to being SysV based.

  | http://man.cat-v.org/unix-1st/2/sys-unlink (1971) says:
  | > It is also illegal to unlink a directory (except for the super-user).

That has always been true ... the function in make is for when make
is being run as root - not needed otherwise.

kre

ps: I agree that sys calls and emulations thereof should always return
non-zero (and hence true) in the error case (but not necessarily always
0 when successful).  That is simply the way they work and always have.

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

[Christos Zoulas]

In article <20220928163447.b0bf6f...@cvs.netbsd.org>,
Simon J. Gerraty  wrote:
>-=-=-=-=-=-
>
>Module Name:   src
>Committed By:  sjg
>Date:  Wed Sep 28 16:34:47 UTC 2022
>
>Modified Files:
>   src/usr.bin/make: main.c meta.c
>
>Log Message:
>Don't ignore return from snprintf or getcwd

But if you check, you should also check < 0

christos

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

[Roland Illig]

Am 14.08.2022 um 12:14 schrieb Valeriy E. Ushakov:

Module Name:src
Committed By:   uwe
Date:   Sun Aug 14 10:14:58 UTC 2022

Modified Files:
src/usr.bin/make: make.1


Thanks for your corrections and improvements, now that I see them they
are obvious. :)

Roland

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

[Christos Zoulas]

> On Jan 30, 2022, at 2:41 PM, Roland Illig  wrote:
> 
> Am 30.01.2022 um 14:21 schrieb Christos Zoulas:
>> Module Name: src
>> Committed By:christos
>> Date:Sun Jan 30 13:21:09 UTC 2022
>> 
>> Modified Files:
>>  src/usr.bin/make: dir.c job.c make.h
>> 
>> Log Message:
>> Make the GNode lineno unsigned to fix lint warning in var.c calling
>> PrintLocation()
> 
> Hi Christos,
> 
> Did you get that warning on i386?  On x86_64 I didn't get any lint
> warnings, as these would have made my pre-commit hook fail.

Yes: https://releng.netbsd.org/builds/HEAD-lint/202201300710Z/i386.build.failed 

Do you know it would only fail on i386?

> Since usr.bin/make is also used in tools/make, it needs to follow the
> rules in tools/README, which say that all tools should stick to C89.
> The format specifier %zu comes from C99 though.

Yes, %zu is annoying because windows does not have it either (until very
recently I think). But cygwin does have it(?) so there is no issue?

> Or can we drop the C89 rule from tools/README?  In the past few years,
> usr.bin/make used several features from C99 (end-of-line comments, long
> long, %zu, snprintf), and nobody cared to fix the C99-isms, so either
> nobody needed the C89 restriction or we simply ignored the complaints.

I think that we should ask in tech-toolchain. Yes, I prefer to require c99
at this point.

Best,

christos


signature.asc
Description: Message signed with OpenPGP

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

[Roland Illig]

Am 30.01.2022 um 20:54 schrieb Christos Zoulas:



On Jan 30, 2022, at 2:41 PM, Roland Illig mailto:roland.il...@gmx.de>> wrote:

Am 30.01.2022 um 14:21 schrieb Christos Zoulas:

Module Name:src
Committed By:christos
Date:Sun Jan 30 13:21:09 UTC 2022

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

Log Message:
Make the GNode lineno unsigned to fix lint warning in var.c calling
PrintLocation()


Hi Christos,

Did you get that warning on i386?  On x86_64 I didn't get any lint
warnings, as these would have made my pre-commit hook fail.


Yes:
https://releng.netbsd.org/builds/HEAD-lint/202201300710Z/i386.build.failed
Do you know it would only fail on i386?


No, I didn't know, and I also couldn't guess the lint warning, that's
why I asked.  The lint warning from the log is:

var.c(4527): warning: argument #4 is converted from 'int' to 'unsigned
int' due to prototype [259]

The warning makes sense, and it doesn't occur on x86_64 because size_t
is a 64-bit integer type there, which can hold any value of int.  But
still, the representation might change, so lint should warn.  Not about
potential value truncation, but about prototype compatibility.  In
lint's should_warn_about_prototype_conversion, I already wrote a comment
saying 'XXX: Investigate whether this rule makes sense', so it's
probably a conceptual bug in lint that this warning didn't occur on x86_64.

Roland

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

[Simon Gerraty]

On Sun, 30 Jan 2022 14:54:25 -0500, Christos Zoulas writes:
>> Since usr.bin/make is also used in tools/make, it needs to follow the
>> rules in tools/README, which say that all tools should stick to C89.
>> The format specifier %zu comes from C99 though.
>
>Yes, %zu is annoying because windows does not have it either (until very
>recently I think). But cygwin does have it(?) so there is no issue?

size_t is annoying ;-)

>> Or can we drop the C89 rule from tools/README?  In the past few years,
>> usr.bin/make used several features from C99 (end-of-line comments, =
>long
>> long, %zu, snprintf), and nobody cared to fix the C99-isms, so either
>> nobody needed the C89 restriction or we simply ignored the complaints.
>
>I think that we should ask in tech-toolchain. Yes, I prefer to require =
>c99
>at this point.

For bmake I have converted the %zu to %u.
I no longer have access to even a tiny fraction of the systems bmake is
used on, but I recall %z being an issue in the past.

I cannot find docs on gcc-2.95 (which is the oldest compiler I've
recently had to use to port bmake), and the license for that VM has
expired, it probably should support c99 but I cannot confirm.

--sjg

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

[Roland Illig]

Am 30.01.2022 um 14:21 schrieb Christos Zoulas:

Module Name:src
Committed By:   christos
Date:   Sun Jan 30 13:21:09 UTC 2022

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

Log Message:
Make the GNode lineno unsigned to fix lint warning in var.c calling
PrintLocation()


Hi Christos,

Did you get that warning on i386?  On x86_64 I didn't get any lint
warnings, as these would have made my pre-commit hook fail.

Since usr.bin/make is also used in tools/make, it needs to follow the
rules in tools/README, which say that all tools should stick to C89.
The format specifier %zu comes from C99 though.

Or can we drop the C89 rule from tools/README?  In the past few years,
usr.bin/make used several features from C99 (end-of-line comments, long
long, %zu, snprintf), and nobody cared to fix the C99-isms, so either
nobody needed the C89 restriction or we simply ignored the complaints.

Roland

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

[Roland Illig]

Am 09.01.2022 um 23:04 schrieb Joerg Sonnenberger:

On Fri, Jan 07, 2022 at 08:04:49PM +, Roland Illig wrote:

Module Name:src
Committed By:   rillig
Date:   Fri Jan  7 20:04:49 UTC 2022

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

Log Message:
make: use simpler code for handling .for loops


This seems backwards to me. For simpler logic I would expect computing
the var length first and then skipping variables that don't have the
same length, falling back to memcmp otherwise.


As it is now, the body of the .for loop is a simple string.  As such,
there are no markers where variables start and end, therefore "the same
length" is not known in advance.

It would be possible to parse the body of the .for loop first, so that
each part is either:

* a plain string that is copied verbatim
* '$' + a one-character variable name
* '$' + (brace or parenthesis) + variable name

After preparing the .for loop in this way, the lengths of the variable
names in the body would be known, and it would make sense to match them
with the variable names from the .for loop.  This would have to be done
only once though since the names of the iteration variables don't
change.  Then again, I don't see why it would be beneficial to compute
the length of a variable name in advance.  If I missed a point here,
please tell me. :)

It's definitely an interesting idea to avoid parsing the loop body on
each iteration.  I will experiment on that idea to see whether it makes
a noticeable difference.

Since everything else in makefiles is parsed and evaluated only once, I
don't see an immediate benefit of first parsing constructs other than
.for loops into data structures and then evaluating them.

Another possible optimization is multiple-inclusion guards.  That's
completely independent though.

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

[Joerg Sonnenberger]

On Fri, Jan 07, 2022 at 08:04:49PM +, Roland Illig wrote:
> Module Name:  src
> Committed By: rillig
> Date: Fri Jan  7 20:04:49 UTC 2022
> 
> Modified Files:
>   src/usr.bin/make: for.c
> 
> Log Message:
> make: use simpler code for handling .for loops

This seems backwards to me. For simpler logic I would expect computing
the var length first and then skipping variables that don't have the
same length, falling back to memcmp otherwise. De facto inlining a
memcmp loop is not an improvement.

Joerg

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

[nia]

On Sun, Jan 09, 2022 at 01:18:32AM +0100, Roland Illig wrote:
> If I were to extract the newly added code into a function call
> cpp_skip_string(, varname), would that help?  That function could be
> used in var.c as well, for example in ModMatch and ModMatchEq.

Yes, it should be a utility function IMO.

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

[Roland Illig]

Am 08.01.2022 um 23:54 schrieb nia:

On Fri, Jan 07, 2022 at 08:04:49PM +, Roland Illig wrote:

Using memcmp for comparing the variable name was probably overkill since
the variable names are usually very short, so rather compare them byte
by byte.


I don't see the point of this change - it makes the code harder to
read. We should trust the optimizing compiler, or alternatively trust
that comparing short byte strings isn't particularly expensive.


My idea was that it is usually enough to compare the first byte of the
variable name to see the mismatch, and that the variable names from .for
loops are usually shorter than 8 bytes.  Therefore I wanted to save the
function call to memcmp, as well as the extra comparison for length > 7
that is in x86_64's libc.a(memcmp.o).

I didn't measure whether the speed improvement is noticeable though.
Throwing away 12 lines of code plus a struct looked good to me though,
therefore I went this way.

If I were to extract the newly added code into a function call
cpp_skip_string(, varname), would that help?  That function could be
used in var.c as well, for example in ModMatch and ModMatchEq.

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

[nia]

On Fri, Jan 07, 2022 at 08:04:49PM +, Roland Illig wrote:
> Using memcmp for comparing the variable name was probably overkill since
> the variable names are usually very short, so rather compare them byte
> by byte.

I don't see the point of this change - it makes the code harder to
read. We should trust the optimizing compiler, or alternatively trust
that comparing short byte strings isn't particularly expensive.

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

[Robert Elz]

Date:Fri, 10 Dec 2021 01:36:10 +0100 (GMT+01:00)
From:Roland Illig 
Message-ID:  

  | I guess there's really no way around running the whole build before each
  | commit, to reach a build success rate of 99.9 %.

What I tend to do, where possible, is make a bunch of changes during
the day starting from a known buildable HEAD (in my case, day does not
necessarily, or even often, match daylight very closely at all, but that's
irrelevant), related changes or just plain random ones, then do a release
build while I sleep.

If it works, then after I am properly awake, go about doing all the appropriate
commits, fairly close together.  Then if no-one else left the build in a broken
state, cvs update everything, and start another build, just to verify it still
all builds cleanly.   While that happens, I can be doing whatever else needs
doing, more changes for the next set, or just walking the dog...

If I go for a while not making any changes, I do the cvs update and build
cycle again before starting back working on the code.

This handles all the "change broke everything" issues (including set list
issues) but not always the "change broke my port" problems, so you also
need to watch the releng buildbot stats, and if some ports stop building,
go look at the logs to see if my change might have caused tha

The b5 i386 build broke e-mail helps, but i386 is just one alt port.

Do remember that there is very rarely and pressing urgency for
changes you are making to get committed.   A delay of a few days
from change complete to change committed generally hurts no-one.

kre

ps changes to things that cannot break the build, like man page
sources, TODO, CHANGES,..   and often comments, if you are fairly
convinced that nothing else will care are not much helped by build
testing, so that can often be ommitted.   Tests however are not
in that category ... not working properly can be ok, not building
is not.

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

[Roland Illig]

10.12.2021 01:07:22 Robert Elz :

>     Date:    Thu, 9 Dec 2021 22:25:58 +
>     From:    "Roland Illig" 
>     Message-ID:  <20211209222558.cdf22f...@cvs.netbsd.org>
>   | No functional change.
>
> That obviously wasn't true.   That means that you just guessed at that,

I cannot defend against this argument. Indeed, I guessed, but that guess was 
backed up by several hundred lines of test code, essentially all cond-*.mk and 
especially cond-short.mk.  These tests made me reasonably sure that the 
behavior of the code wouldn't change.  The behavior did change though, and the 
change went unnoticed due to my lack of testing.  So even 100 % code coverage 
does not prevent all possible bugs.

I guess there's really no way around running the whole build before each 
commit, to reach a build success rate of 99.9 %.

Roland

Re: CVS commit: src/usr.bin/make/unit-tests

[Robert Elz]

Date:Thu, 9 Dec 2021 23:57:19 +
From:"Roland Illig" 
Message-ID:  <20211209235719.cde20f...@cvs.netbsd.org>


  | Log Message:
  | tests/make: prevent the bug from cond.c 1.283 from happening again

This new test (while OK of itself) would not have done that.
I suspect this test would have passed with that broken version.

The actual condition that failed was (effectively)

.if 1 && (defined(VAR) || ${VAR} != "string")

not .if 0

In that, the "i &&" part is largely irrelevant, you didn't
touch && parsing in the change that broke things, just ||
parsing, so implementing a test that tests short circuit
eval of && will not help (this one, it could help others).

This is actually not a good text of short circuit eval of
&& though, it is complicated by the complex (and breakable,
as seen) condition on the rhs, better would be

.if defined(VAR) && ${VAR} == "yes"

or something simple like that.

With simple tests for basic short circuit eval, you can then add
tests for more complex cases

.if 1 && 1 && defined(VAR) && ${VAR} == "yes" 

(and similar for ||), and after those pass, tests for various complicated
combinations of || and && (all with something relying on short circuit eval
to verify that things are not tested which should not be).

kre

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

[Robert Elz]

Date:Thu, 9 Dec 2021 22:25:58 +
From:"Roland Illig" 
Message-ID:  <20211209222558.cdf22f...@cvs.netbsd.org>

  | make: avoid recursion in CondParser_Or
  |
  | Previously, a long chain of '1 || 1 || 1 || 1 || ...' led to a deep
  | recursion.  Furhermore, the code didn't match the grammar on superficial
  | reading: the grammar said "or || and", the code said "and || or".
  |
  | No functional change.

That obviously wasn't true.   That means that you just guessed at that,
so the comment should have been "No functional change intended" instead,
but more importantly, for changes this significant you should *always*
be doing a test release build before committing.

I know that you have reverted this now, which is fine, but for the record
the problem was that your change broke short circuit evaluation.

Once an || test returns true, you don't do any more of them, the answer
is already known.   The expression that failed depended upon that, as is
allowed.

kre

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

[matthew green]

Roland Illig writes:
> Am 09.12.2021 um 21:01 schrieb matthew green:
> > i'm not asking that you make sun2 or vax stuff work, but
> > some of us choose to use jemalloc 100 on all builds and
> > since this is a supported option, i wanted to make sure
> > you were aware of it.
>
> I added back the support for jemalloc 100, the few extra lines didn't
> hurt as much as I had thought. :)
>
> Thank you for suggesting and explaining it.

thanks!


.mrg.

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

[Roland Illig]

Am 09.12.2021 um 21:01 schrieb matthew green:

i'm not asking that you make sun2 or vax stuff work, but
some of us choose to use jemalloc 100 on all builds and
since this is a supported option, i wanted to make sure
you were aware of it.


I added back the support for jemalloc 100, the few extra lines didn't
hurt as much as I had thought. :)

Thank you for suggesting and explaining it.

Roland

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

[matthew green]

Roland Illig writes:
> Am 08.12.2021 um 02:09 schrieb matthew green:
> >> Module Name:   src
> >> Committed By:  rillig
> >> Date:  Sun Dec  5 14:57:36 UTC 2021
> >>
> >> Modified Files:
> >>src/usr.bin/make: test-variants.sh
> >>src/usr.bin/make/unit-tests: Makefile export.mk opt-file.mk
> >>
> >> Log Message:
> >> tests/make: migrate to jemalloc > 100
> >
> > note that the build allows choosing jemalloc 100 or 510, and
> > we default to 100 on vax and sun2 currently.
>
> The code of usr.bin/make is as platform-independent as possible,
> therefore my idea was to run the tests with memory debugging on the main
> development platforms, assuming that if the memory management would be
> broken, either x86_64 or i386 or sparc would fail first, and these are
> included in https://releng.netbsd.org/test-results.html.
>
> I don't think adding memory management debugging for vax or sun2
> provides much additional value.  Sure, if you insist, I can add that,
> but right now I feel it would be redundant.  On the plus side, omitting
> memory debugging on these platforms makes the tests run a bit faster.

i'm not asking that you make sun2 or vax stuff work, but
some of us choose to use jemalloc 100 on all builds and
since this is a supported option, i wanted to make sure
you were aware of it.

thanks.


.mrg.

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

[Roland Illig]

Am 08.12.2021 um 02:09 schrieb matthew green:

Module Name:src
Committed By:   rillig
Date:   Sun Dec  5 14:57:36 UTC 2021

Modified Files:
src/usr.bin/make: test-variants.sh
src/usr.bin/make/unit-tests: Makefile export.mk opt-file.mk

Log Message:
tests/make: migrate to jemalloc > 100


note that the build allows choosing jemalloc 100 or 510, and
we default to 100 on vax and sun2 currently.


The code of usr.bin/make is as platform-independent as possible,
therefore my idea was to run the tests with memory debugging on the main
development platforms, assuming that if the memory management would be
broken, either x86_64 or i386 or sparc would fail first, and these are
included in https://releng.netbsd.org/test-results.html.

I don't think adding memory management debugging for vax or sun2
provides much additional value.  Sure, if you insist, I can add that,
but right now I feel it would be redundant.  On the plus side, omitting
memory debugging on these platforms makes the tests run a bit faster.

Roland

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

[matthew green]

> Module Name:  src
> Committed By: rillig
> Date: Sun Dec  5 14:57:36 UTC 2021
>
> Modified Files:
>   src/usr.bin/make: test-variants.sh
>   src/usr.bin/make/unit-tests: Makefile export.mk opt-file.mk
>
> Log Message:
> tests/make: migrate to jemalloc > 100

note that the build allows choosing jemalloc 100 or 510, and
we default to 100 on vax and sun2 currently.

see ${HAVE_JEMALLOC}.

thanks.


.mrg.

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

[Roland Illig]

On 27.01.2021 20:54, Reinoud Zandijk wrote:

Hi,

On Tue, Jan 26, 2021 at 11:44:56PM +, Roland Illig wrote:

Module Name:src
Committed By:   rillig
Date:   Tue Jan 26 23:44:56 UTC 2021

Modified Files:
src/usr.bin/make: parse.c
src/usr.bin/make/unit-tests: include-main.exp include-subsub.mk

Log Message:
make(1): in -dp mode, print stack trace with each diagnostic


Maybe related but could you make printing of the 1st failing error message
easier to find in a parallel build? Say recording the command and output of
the offending command and print it at the end?


Sounds possible.  I'll look into it but cannot promise anything right
now.  Ideally this diagnostic information would be passed up to the
top-level make process, which would then print it at the very end.

Roland

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

[Reinoud Zandijk]

Hi,

On Tue, Jan 26, 2021 at 11:44:56PM +, Roland Illig wrote:
> Module Name:  src
> Committed By: rillig
> Date: Tue Jan 26 23:44:56 UTC 2021
> 
> Modified Files:
>   src/usr.bin/make: parse.c
>   src/usr.bin/make/unit-tests: include-main.exp include-subsub.mk
> 
> Log Message:
> make(1): in -dp mode, print stack trace with each diagnostic

Maybe related but could you make printing of the 1st failing error message
easier to find in a parallel build? Say recording the command and output of
the offending command and print it at the end ?

Reinoud

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