Trepan-Debuggers / remake

Enhanced GNU Make - tracing, error reporting, debugging, profiling and more
http://bashdb.sf.net/remake
GNU General Public License v3.0
795 stars 75 forks source link

GNU remake 4.3 #51

Closed rocky closed 4 years ago

rocky commented 4 years ago

In issue #40 @boretom reports:

regarding make 4.3: I did a diff and I'm up for the task - not like from 3.8 to 4 :) ... it may take a week or two. If you're ok with that timeline I would take on the task.

@boretom thanks, the project can wait a week or two.

Note that regarding make 4.3, I recently read in the news (and can't find it now) that Linus Torvalds has found a way to take advantange of the speedup that was proposed over a year ago in 4.3 to parallel builds. And with this, there can be something like a 10x speedup in building the Linux Kernel on multicore machines by using the --jobs option along with his customization in the kernel that recently went out.

As a result, the prioritiy of getting this done has increased.

Another (somewhat accidental) benefit of remake over GNU make is that since it is not in the critical path of a lot of things, OS's that have 4.2 (or 3.x) can also install remake and that gives them an easy way to instal a newer GNU make to try it out without impacting existing programs.

So I had planned on doing this, but I'd rather if someone else did. That way, there are two people in the world that may understand this. And in the end I know the code will get much much better. (See my comment make previously somewhere else about making a lot of mistakes).

However... painful experience has shown that the right way to do this is to make two new branches, make-4-3 with the GNU make source, and another branch from that remake-4-3. And then with the 'diff"s from make-4-2 HEAD to remake-4-2 HEAD, "rebase" them onto the remake-4-3 branch. In other words, do not build off of the exisiting remake-4-2 branch at HEAD.

In the last paragraph, I put "rebase" in quotes because I don't necessariliy mean using git rebase. For such largish changes that might be too difficult. Of course, It is a possibility if that helps, but again; I just don't think it will, but I could be wrong.

So shortly I will be creating the two branches to work off of, although I'll leave the work from the branch from remake-4-3 to a fully featured remake 4.3 up to you.

rocky commented 4 years ago

@boretom The branch remake-4-3 has been created and is a branch of make-4-3 which is a copy of the GNU Make 4.3 sources. However I added a couple of .gitignore files so as to make sure the branch is there and has a couple of the new directories. You can squash that last commit onto whatever follows or leave it as you so desire.

But please build off of the remake-4-3 branch.

Looking at this, I think it will not be as easy as you seem to think. What's gone on between 4.2 and 4.3 is that a more conventional build organization has occurred (which of course is very welcome). But that by necessity adds a bit of upheaval.

Some things to keep in mind. First , remake doesn't support VMS, Atari, riscos, OS2, DOS (with or without EMX) or native MS Windows, and I have no desire to start doing so now (even though that pisses off the 3 VMS developers that may still exist in the world). I find the code to support this makes it even harder to read the already hard-to-read code.

Second, although GNU Make code has always been a bit back-level in terms of using best practice C style and conventions, resist the temptation to try to fix this up. Eventually GNU Make does make forward progress, albeit at a snails pace, and invariablly the GNU make fixups will be different from those in you've decided on in remake. This adds additional work when the next release comes out.

Of course if the back-leveness is in strictly remake code, such as in the debugger, then by all means go and fix up my crappy C code.

boretom commented 4 years ago

@rocky thanks for the detailed info. I'll work on it as you recommend, working off branch remake-4-3 and so on. I highly doubt that rebase will work, but worth a shot :).

I realized that I had a quick look at the diff from the latest make 4.2.93 and make HEAD. Which is not the right tag/commit since remake is based on make 4.2.1. But I'll follow you recommendation and do my best :)

Did you have a chance to try to verify the 10x speedup for compiling the kernel sources with make 4.3? I'll surely will looking into it, it seems like a good indicator if I f*ed up or not.

rocky commented 4 years ago

The article on GNU make 4.3 and GNU/Linux: https://www.phoronix.com/scan.php?page=news_item&px=Linux-Pipe-Parallel-Job-Opt

No, I haven't verified the speedup results.

The part of the article I find amusing:

the patch was partially delayed ... which ended up being resolved back in 2017 but only saw a new release with GNU Make 4.3

Yep, 3 years between resolution of a problem for a feature and when people can start to use it. That matches my experience too. And you say you are concerned about the possibility of a two week delay?

With respect to rebasing...

Git is awesome and a really powerfull tool, but it helps if you keep in mind what's going on so you can use git to its full advantage.

The remake git repostiory does not aim to capture the full GNU Make history. If you are interested in GNU Make version-control history, use GNU make's version control for that.

Also I am not interested in tracking the full remake version control history across major GNU Make/remake releases. When you go from remake-4-2 to remake-4-3, the only thing that matters is the entire set of changes as one big patch from the most recent remake-4-2 to get to the most-recent remake-4-3. Of course, if you are interested in the ins-an-outs of how you got from make-4-2 to remake-4-2 you can use the git history within the remake-4-2 branch. Having that history also be in the remake-4-3 branch is not interesting.

Using this as a guide, the first thing to note is that the file moves between GNU Make 4.2 and GNU make 4.3 such as from job.c to src/job.c don't appear in remake git anywhere.

Therefore a simple rebase isn't going to be able track this. However what you can do and what I'd do is take the patch and manually edit file names so that the diff works on the moved file, e.g. src/job.c where the diff patch lists file job.c.

boretom commented 4 years ago

Thanks for the link to Linus' article, seems I won't be able to test it easily with my 6 core laptop :)

tracking full history: When it is working I'll great one big diff/commit, does make sense. Right know the debugging phase is starting.

rocky commented 4 years ago

Right know the debugging phase is starting.

Exciting!

boretom commented 4 years ago

Turns out there are more stopchar_map character and that let remake fail pretty nice. Thanks to gdb that's solved.

Todo: ./configure maintainer mode, make check, make install and the docs are taken from make instead of remake (Plus what I forgot :) )

branch remake-4-3-tk in case you're bored or curious :).

rocky commented 4 years ago

WOW!

I cloned and tried this. All of the basic functionality (tracing, the debugger, profiling) seems to work!

As you reported in the "Todo" above, there were failures in trying to run the tests make check-local and the profiling test in unittest, and some po-files seem to need adjusting. It looks like there are a number of packging issues if you try to run make dist or make distcheck, but this is stuff is small potatoes compared to the bulk of what's there now.

Many thanks!

boretom commented 4 years ago

Hey, I'm happy 4.3 appears to work that smooth, I only had to apply the diff.

I will look a the remaining issues and report back.

rocky commented 4 years ago

I only had to apply the diff.

That's good to hear. I guess this is the approach then to use in the future as well.

The branch organization and method of patching may seem obvious now how to do, but it in fact took a while (and lots of painful merges after the few times a GNU make release came out). So I am also happy to hear that this aspect has improved and is not as painful as it was in the past.

boretom commented 4 years ago

You have definitely done a great job and have lots patients. Extending existing, "old" software seems sometimes to be quite mess.

While starting to fix check-local errors I see that remake has a slightly different formating of an exit string. Which leads to check errors since it diffs the output. And I'm only taking about the strings that make outputs too, not the additional lines.

The particular case I'm talking about is in src/job.c, line 467: remake:

  if (exit_sig == 0)
    err_with_stack(p_call_stack,
           _("%s[%s] error %d%s"),
           pre, f->name, exit_code, post);
 else
    err_with_stack(p_call_stack, "%s[%s] %s%s%s",
           pre, f->name, strsignal (exit_sig), dump, post);

make, src/job.c, line 571:

  if (exit_sig == 0)
    error (NILF, l + INTSTR_LENGTH,
           _("%s[%s: %s] Error %d%s"), pre, nm, f->name, exit_code, post);
  else
    {
      const char *s = strsignal (exit_sig);
      error (NILF, l + strlen (s) + strlen (dump),
             "%s[%s: %s] %s%s%s", pre, nm, f->name, s, dump, post);
    }

That seems to be a difference since at least 4.2.Would you be ok if I use the make formatting? Still keeping err_with_stack, only the printf-ke formatting. Or do you have a specific reason for using that formatting?

Edit: Add the output difference

Output of make

tests git:(remake-4-3-tk) ✗ ../../make/make -f work/features/errors.mk.2 one
./foobarbazbozblat xx yy
make: ./foobarbazbozblat: No such file or directory
make: [work/features/errors.mk.2:2: one] Error 127 (ignored)

Output of remake

tests git:(remake-4-3-tk) ✗ ../make -f work/features/errors.mk.2 one
./foobarbazbozblat xx yy
make: ./foobarbazbozblat: No such file or directory
work/features/errors.mk.2:2: [one] error 127 (ignored)
rocky commented 4 years ago

Interesting. I guess it is okay to go with make's current format although it will shift the hassle for me (and possibly others) who have tools that parse remake's output.

It seems between 4.1 and 4.3 remake has shamed the GNU Make into adding the file, line and column information. Here is what make 4.1 (and 4.2?) gave for the last line:

make: [one] Error 127 (ignored)

So if I have this right, they have expanded the stuff in square brackets to include position information. I guess this makes sense, coming from where they do, in that it expands upon the format they had before.

The reason I chose the format that I did, is that it matches gcc (and other compiler) error output. Here, error messages are in the format

filename colon linenumber [colon column] [space function-name] colon error-message

(In remake a GNU make target is analogous to a function.)

In other words I was trying to be as unoriginal as possible. But I have found every project feels that their format is clearly superior in some way and has to be different.

This causes problems for tools that want to parse the output. I guess though that such tools will already have to have some upheaval because GNU Make 4.3 is different from GNU Make 4.1. (I just checked how GNU Emacs 26.2 tries to interpret that kind of GNU Make error in a "compilation" buffer, and basically it doesn't.)

The question then becomes should remake change to be compatible with make's changed incompatible format?

I don't have a strong feeling either way, since each has its downside.

But this is the kind of stuff I am referring to when I write about limiting the fixups in GNU Make because eventually it will get around to doing something, minimally, and in a different way that is incompatible with remake.

boretom commented 4 years ago

I did look into it a bit more and came to agree with you.

My original idea was that future patches for tests/* would be simpler and patching would fail for less hunks. But since it may change again in the future, let's keep it the way you have it.

boretom commented 4 years ago

check-local runs now successful. In features/output-sync I disabled the three failing tests - the same that where disabled before, including the FIXME tag.

The check-local runs fine on macos 10.14.6 Mojave, ArchLinux and FreeBSD 12.0.

rocky commented 4 years ago

check-local runs now successful[ly]. ... The check-local runs fine on macos 10.14.6 Mojave, ArchLinux and FreeBSD 12.0.

Awesome!

In my opinion some of the GNU make tests are a bit fragile. Basically, I look at the failures and if they indicate something serious I investigate more. If not, I'll leave with a "FIXME" to indicate I might deal with later, or give a heads up to someone down the line that things might be flaky, as seemed to happen here.

I looked a little at the unittest failure, and that seems to be because lib/fcntl.o is not getting added to lib/libgnu.a.

I noticed that the person who was doing Amiga support, Aaron "Optimizer" Digulla (see README.Amiga), seems to have cleaned up his code so the changes are isolated and not a holistic mess the way VMS #ifdefs were.

When OS-specfiic code goes in cleanly, and there is someone who will responsibly test the code, then it's okay for the OS go be in there. So down the line we might contact to see if he's interested. What do you think?

I see a typo in my Make target comment in to top-level Makefile.am

: create ChangeLog for distirbution from 'git log'

. ^^

I was looking at that because I would like to add a comment to the "check_local" target to indicate it does (and to suggest it can be used). In general when fixing up the code helps me figure out the code, such as adding comments to the Makefile.am file, I am in favor of that. I notice that there are a lot of targets in maintMakefile that are no longer relevant and so similarly it might be nice to remove those so people don't trip over them. What do you think?

Of course, all of this little stuff might be done after the code is merged in.

As it is so close, I am getting anxious for this to go in. So hurry up! :-)

In sum great work!

boretom commented 4 years ago

About the GNU make tests : I haven't looked at the failed output-sync tests too much. They add sleep to make sure the timing adds up, surely a bit fragile :).

unittest failure : I don't understand right away what changed for unittest between 4.2.1 and 4.3. Do you have an idea what do add to unittest/Makefile.am?

Amiga Port/Aaron Digulla : I would absolutely contact him, maybe he is up to it, my fellow Swiss - not that I know him :)

typo #: : Is corrected

cleanup maintMakefile : To be honest I haven't look at maintaince mode yet and don't feel I know enough about it to know what to remove. But I'm totally for cleanup what's possible!

So hurry up : I do my best :)

rocky commented 4 years ago

Do you have an idea what do add to unittest/Makefile.am?

Commit f3ee2748 gets a little closer to getting this working again. Now that there is lib/libgnu.a that has to be used and this commit pulls that in. However there is something in the libgnu.a that is incomplete:

$ make check
make  test_profile
make[1]: Entering directory '/src/external-vcs/github/rocky/remake-tk/unittest'
gcc  -g -O2 -Wl,--export-dynamic  -o test_profile test_profile.o ../src/mock.o ../src/version.o ../src/alloc.o ../src/globals.o ../src/misc.o ../src/output.o ../src/file_basic.o ../src/hash.o ../src/profile.o ../src/strcache.o ../lib/libgnu.a  -ldl -lreadline 
/usr/bin/ld: ../src/output.o: in function `setup_tmpfile':
/src/external-vcs/github/rocky/remake-tk/src/output.c:324: undefined reference to `fd_noinherit'
/usr/bin/ld: /src/external-vcs/github/rocky/remake-tk/src/output.c:337: undefined reference to `fd_noinherit'
collect2: error: ld returned 1 exit status
make[1]: *** [Makefile:1091: test_profile] Error 1
make[1]: Leaving directory '/src/external-vcs/github/rocky/remake-tk/unittest'
make: *** [Makefile:1372: check-am] Error 2
rocky commented 4 years ago

BTW when I run make check-local I see stuff like:

features/errors ......................................... "my" variable $answer masks earlier declaration in same scope at /src/external-vcs/github/rocky/remake-tk/tests/scripts/features/errors line 25.
"my" variable $answer masks earlier declaration in same scope at /src/external-vcs/github/rocky/remake-tk/tests/scripts/features/errors line 39.
"my" variable $answer masks earlier declaration in same scope at /src/external-vcs/github/rocky/remake-tk/tests/scripts/features/errors line 63.
"my" variable $answer masks earlier declaration in same scope at /src/external-vcs/github/rocky/remake-tk/tests/scripts/features/errors line 78.
"my" variable $answer masks earlier declaration in same scope at /src/external-vcs/github/rocky/remake-tk/tests/scripts/features/errors line 92.
"my" variable $answer masks earlier declaration in same scope at /src/external-vcs/github/rocky/remake-tk/tests/scripts/features/errors line 103.
"my" variable $answer masks earlier declaration in same scope at /src/external-vcs/github/rocky/remake-tk/tests/scripts/features/errors line 119.

What this generally means is that you have Perl code like:

my $answer = ...; # line 25
...
my $answer = ...;  # line 39

and this should be instead:


my $answer = ...; # line 25
$answer = ... ;  # line 39
boretom commented 4 years ago

Good morning,

You see my lack of mastery of Perl (and lots of other topics) :/ ... I corrected the multiple declarations of `$myanswers.

Regarding unittest: I had a blond moment yesterday and a nights sleep did well :) ... grep-ping through the files I saw that fd_noinherit is provided by src/posixos. Including it in the unittest/Makefile.am did the trick - of course plus lib/libgnu.a which you add.

Not working with the latest commit 3728539 are make po-update and make distcheck while compling unittest, not finding gettext.h. The rest does what it should.

Maybe you have an idea for these two issues, especially regaring make distcheck. make succeeds with make po-update failing.

rocky commented 4 years ago

make po-update is addressed in the PR I put in. Basically all of that has changed between 4.2 and 4.3 an it is is in make git only. See comments in PR . https://github.com/boretom/remake/pull/1

As for make dist and getting CI testing working, that's better left for doing after the merge is in. This kind of stuff is easier for me to do and then you can look at the commit than to try to explain all the (in my opinion arcane) bits of stuff that go with that.

boretom commented 4 years ago

Sounds good to me thumbsup

rocky commented 4 years ago

Actually, if you want to handle CI testing (inside remake) that'd be fine with me. I kind of hate doing that kind of stuff when it comes down to the mechanics, although I like benefits of CI.

boretom commented 4 years ago

I haven't done any CI related stuff, so it depends on how fast you want it done :). Would have to read into a bit.

rocky commented 4 years ago

For CI, I am happy to wait :-)

boretom commented 4 years ago

Hehe, you hate it that much :) ... I'll look into it.

Does everything works for you with your changes? make dist does complain that there is no rule to make build.sh.in. I added that again to maintMakefile.make distcheck` fails too. If it's working on your side I'll have to check my system.

On a sidemark: compiling on macOS fails (now the same as make 4.3) and also on FreeBSD.

rocky commented 4 years ago

I'll look into it. Thanks - I appreciate it. I'm sure you'll do fine.

make dist fails just as you report. However again, my take for you this is to put in a PR, we'll merge and then we'll continue with the administrivia such as dist, distcheck, CI, etc.

But since you took the initiative to undertake this, I'm just waiting on you to decide whether it's okay to merge. (In my opinion though right now is fine.)

boretom commented 4 years ago

It's absolutely ok to merge and continue with PRs. The move to version 4.3 doesn't have to appear all perfect in one commit.

Shall I rebase so that it only is one commit?

rocky commented 4 years ago

Shall I rebase so that it only is one commit?

Up to you. If you think there is value in preserving various steps, to guide/remind folks in the future then several steps makes sense and you could decide what to squash. But other than that I don't have an opinion.

In addition, it might be cool to write down say on the wiki the steps that you took while it is fresh in your mind as to what to do. I'll start such a thing for the things I remember, and copy some information from here.

It had occurred to me that that in addition to the one big patch from make 4.2-last to remake-4.2 last, it might be useful to consult make-4.2-last make-4.3 to help understand what's gone on. However since you did the work, you'd be in a better position to know that.

rocky commented 4 years ago

@boretom https://github.com/rocky/remake/wiki/Updating-for-a-major-GNU-Make-release has what I think may be the useful information here to guide someone. Please add or correct this as you see fit.

boretom commented 4 years ago

I didn't put much effort into the commit message tbh so they are of no great use. One big commit it shall be. Updating the wiki is certainly something I'll do and taking into account the commit message.

Regarding comparing make-4.2-last to make-4.3: For some parts I did that to see why a patch failed. Let's see if I can still remember details.

The PR is created and CI failed (not suprise I assume)

boretom commented 4 years ago

Question regarding how to proceed with fixes where I'm not sure if my approach is right.

Example: Compilng remake on macOS fails because clang is not options-compatible to gcc. That would be only warnings but make uses -Wall in AM_FLAGS.

Do I open an issue to discuss stuff like that before submitting a PR or create an PR right away and discuss it in the PR?

rocky commented 4 years ago

Do I open an issue to discuss stuff like that before submitting a PR or create an PR right away and discuss it in the PR?

Do it in the PR. I would like to be as unbureuacratic as possible here.

As for clang vs gcc, configure should be detecting stuff like this and adjusting.

I never had a problem in remake before with macOS. So consult how it's done in remake 4.2. I seem to recall there I had to make a change in C flags, at least to get CircleCI to work.

rocky commented 4 years ago

@boretom now that you have access to work in remake, please move over the new branches to remake. Thanks.

boretom commented 4 years ago

@rocky sure, I'll move the fix branches over.

And regarding clang vs gcc: -C & -Werror was also removed from MAKE_FLAGS in remake-4-2.

boretom commented 4 years ago

@rocky I do have a more general question: The time I have available varies a lot, sometimes I won't have time for stuff like this for a week or more. How is that handled in general? How would I know the urgency of a topic/issue? How to you syncronize in such cases?

For the next two weeks I will have about 3 or 4 days during which I'll have plenty of time. So getting to know and make CI will take some time and I don't know if that will take too long or not for you.

rocky commented 4 years ago

In my experience in the open-source world, people just come and go as they please and rarely give notice.

I am always prepared to take over where others leave off. If you don't get to CI, or leave it unfinished, I can take over.

How would I know the urgency of a topic/issue? How to you syncronize in such cases?

Dunno. Make a suggestion that works for you.

rocky commented 4 years ago

Release 4.3 is out .