Perl / perl5

🐪 The Perl programming language
https://dev.perl.org/perl5/
Other
1.91k stars 542 forks source link

Multiple bugs in GNUMakefile #20749

Open khwilliamson opened 1 year ago

khwilliamson commented 1 year ago

Description

miniperl is first made. If a file that depends on is changed, it doesn't know to recompile miniperl.
It only runs to completion the first time a work space is compiled. This is because it does a rename ..\config.sh.tmp config.sh . rename fails if the destination file exists.

It runs to completion in spite of something failing. For example, if the dll can't be generated (say because of a missing symbol), gmake will continue to go through the modules, compiling each one. The original error, like a needle in a haystack, ends up in the midst of the output. This may not be a problem if the -j option isn't specified.

Steps to Reproduce

1st bug: touch locale.c; recompile; miniperl isn't remade.

2nd bug: gmake; gmake

3rd bug: Introduce a syntax error in a file. Then gmake -jN where N is some number, like the number of cores on your system

Expected behavior

None of these should happen Perl configuration


This is a longstanding bug, and the configuration is irrelevant
jkeenan commented 1 year ago

The Subject of this ticket refers to GNUMakefile. I don't see that file in the core distribution. I do see:

$ find . -type f -iname '*GNUmakefile*'
./win32/GNUmakefile

Is that the file you're referring to? If so, is this bug (and bug ticket) Win32-specific?

Description

miniperl is first made. If a file that depends on is changed, it doesn't know to recompile miniperl. It only runs to completion the first time a work space is compiled. This is because it does a rename ..\config.sh.tmp config.sh . rename fails if the destination file exists.

It runs to completion in spite of something failing. For example, if the dll can't be generated (say because of a missing symbol), gmake will continue to go through the modules, compiling each one. The original error, like a needle in a haystack, ends up in the midst of the output. This may not be a problem if the -j option isn't specified.

Steps to Reproduce

1st bug: touch locale.c; recompile; miniperl isn't remade.

I see that happening on Linux, which implies that this problem is not Win32-specific. Is that correct?

2nd bug: gmake; gmake

What problem am I supposed to see in this case?

3rd bug: Introduce a syntax error in a file. Then gmake -jN where N is some number, like the number of cores on your system

What problem am I supposed to see in this case?

demerphq commented 1 year ago

I see that happening on Linux, which implies that this problem is not Win32-specific. Is that correct?

That is surprising to me. I see it getting rebuilt.

$ touch locale.c
$ make
ccache gcc -c -DPERL_CORE -D_REENTRANT -D_GNU_SOURCE -fwrapv -DDEBUGGING -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -std=c99 -g -Wall -Werror=pointer-arith -Werror=vla -Wextra -Wno-long-long -Wno-declaration-after-statement -Wc++-compat -Wwrite-strings locale.c
ccache gcc -fstack-protector-strong -L/usr/local/lib -o miniperl \
    opmini.o perlmini.o universalmini.o  gv.o toke.o perly.o pad.o regcomp.o regcomp_debug.o regcomp_invlist.o regcomp_study.o regcomp_trie.o regexec.o dump.o util.o mg.o reentr.o mro_core.o keywords.o builtin.o hv.o av.o run.o pp_hot.o sv.o pp.o scope.o pp_ctl.o pp_sys.o peep.o doop.o doio.o utf8.o taint.o deb.o globals.o perlio.o numeric.o mathoms.o locale.o pp_pack.o pp_sort.o caretx.o dquote.o time64.o  miniperlmain.o  -lpthread -ldl -lm -lcrypt -lutil -lc 

from that output I would say this does not affect *nix builds.

demerphq commented 1 year ago

@khwilliamson can you try 20751 please? I don't have win32 to test on.

demerphq commented 1 year ago

Note that the win32 makefiles all guard the rename command with a del command first, except for the case that @khwilliamson specified here.

jkeenan commented 1 year ago

I see that happening on Linux, which implies that this problem is not Win32-specific. Is that correct?

That is surprising to me. I see it getting rebuilt.

$ touch locale.c
$ make
ccache gcc -c -DPERL_CORE -D_REENTRANT -D_GNU_SOURCE -fwrapv -DDEBUGGING -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -std=c99 -g -Wall -Werror=pointer-arith -Werror=vla -Wextra -Wno-long-long -Wno-declaration-after-statement -Wc++-compat -Wwrite-strings locale.c
ccache gcc -fstack-protector-strong -L/usr/local/lib -o miniperl \
    opmini.o perlmini.o universalmini.o  gv.o toke.o perly.o pad.o regcomp.o regcomp_debug.o regcomp_invlist.o regcomp_study.o regcomp_trie.o regexec.o dump.o util.o mg.o reentr.o mro_core.o keywords.o builtin.o hv.o av.o run.o pp_hot.o sv.o pp.o scope.o pp_ctl.o pp_sys.o peep.o doop.o doio.o utf8.o taint.o deb.o globals.o perlio.o numeric.o mathoms.o locale.o pp_pack.o pp_sort.o caretx.o dquote.o time64.o  miniperlmain.o  -lpthread -ldl -lm -lcrypt -lutil -lc 

from that output I would say this does not affect *nix builds.

Correct.

khwilliamson commented 1 year ago

Most portions of this issue are still broken

demerphq commented 1 year ago

@khwilliamson "most parts of this issue are still broken" isnt a very helpful way to reopen the ticket. What parts are still broken? What parts aren't. Did the delete before rename work or not?

I dont understand bug 2. All you say is "gmake; gmake". What does that mean?

bug 3, i dont think is a bug in the perl build process. If you make various types of error in linux with a parallel build you will get various artifacts interspersed together.

mauke commented 1 year ago

Isn't "bug 2" a workaround for the problem in "bug 3"? That is, if you run make -j8 and one of the subprocesses fails but the error message gets lost because other stuff is still being compiled, you can simply run make again and only the failing bits of the build will rerun.

(If not, then I don't understand the problem description.)

sisyphus commented 1 year ago

I'm not disputing that there's some sort of buggery going on here, but I think we need a better description of what those bugs actually are. AIUI, we're talking about the Windows-specific win32/GNUmakefile here

1st bug: touch locale.c; recompile; miniperl isn't remade.

That's true ... on my Windows builds (in cmd.exe shell) I get:

D:\comp-1220\perl-5.37.8>touch locale.c
'touch' is not recognized as an internal or external command, operable program or batch file.

which is exactly the output I expect, and not something that I regard as buggy. IOW, the 'touch' command does nothing, and no recompilation of miniperl is therefore expected.

2nd bug: gmake; gmake

I don't have any problems running the exact command 'gmake' unless the GNUmakefile has been incorrectly edited.

3rd bug: Introduce a syntax error in a file. Then gmake -jN where N is some number, like the number of cores on your system

I don't even know how many cores my Windows system has. (How do I find out ?) And I avoid running 'gmake' with a '-jN' argument because Windows builds are already fragile enough without asking for more trouble (and because I don't know what value to assign to 'N', anyway).

I'd love to get a better understanding of the problems @khw is seeing, but I (like others) need a better description of the issue(s).

Cheers, Rob

khwilliamson commented 1 year ago

Sorry for the lack of clarity.

The first bug is that the miniperl depends doesn't work. I used 'touch' presuming it is a windows command. But the point is, if you change one of the base level .c files, miniperl doesn't get rebuilt. I used the example of locale.c. But if you edit any of those files and save the result, miniperl doesn't get rebuilt. It should.

The second bug is if you type on the command line 'gmake; gmake', the second gmake will fail I haven't had a chance to test the already committed patch that fixes this.

The third bug is that the make doesn't stop when a fatal error occurs. It continues to start new tasks without checking the exit status of previous ones. It isn't simply a matter of something failing after something else is started. It doesn't generally stop from creating new subbuilds after a failure

Now, it may be that some of these aren't fixable; that the build is necessarily precarious. But someone who is more adept at makefiles than I should make that determination

I also note that it fails to place a copy or link of the dll and the exe in the t directory. That messes up harness.

I have ended up doing a full clean between compilations, depending on what gets changed. Even when it seems to be working, the generated code is sometimes different from what a build-from-scratch yields.

khwilliamson commented 1 year ago

@mauke, Glad you're around. One should be able to compile twice without the second attempt guaranteed to fail. And if something did fail to compile, and you fix it, the 2nd compile will fail.

@sisyphus to find the number of CPUs, one method is to open the Windows "Resource Monitor" and click on the CPU tab There is a panel on the R that has a left facing arrow (WIndows 11) that expands its view. When expanded it shows the CPU usage in total, as well as that of each individual core. as well as "Service CPU Usage". Scroll down to see the highest numbered CORE. Someone more familiar with Windows could tell you a better way, I'm sure.

sisyphus commented 1 year ago

According to what appears in Task Manager, I have 8 cores, and 16 logical processors in my Windows 11 machine. @khw, I take it that you're building perl in the Windows Powershell as, in cmd.exe shell I get:

D:\comp-1130\perl-5.37.8\win32>gmake; gmake
gmake: *** No rule to make target ';'.  Stop.

In the Powershell, the latest devel version of perl (5.37.8), which I think pre-dates the committing of https://github.com/Perl/perl5/commit/c0a24b11a0434ad8c8b1e758b952f8546d69c216, seems to be handled correctly by running gmake; gmake - so long as CCTYPE and CCHOME are specified correctly in GNUmakefile. The first run of 'gmake' compiles and builds perl as normal. The second run, as expected, does not re-compile or re-build anything, but simply verifies that everything is in order.

The perl executable and dll are not copied into the t/ directory until gmake test is run. Both of those files are then transferred to t/ before any tests are run.

If I "touch" locale.c, then it gets re-compiled in the next gmake run. But touching locale.c seems to have no impact on the miniperl.exe commands that are run. Is that an issue ? I also don't see anything indicating that miniperl.exe gets rebuilt. Maybe that's not as it should be ?

I'll try repeating these tests with current blead ... and see if that makes a difference. Are there other specific tests that I should do ?

Cheers, Rob

sisyphus commented 1 year ago

@khw wrote:

But the point is, if you change one of the base level .c files, miniperl doesn't get rebuilt. I used the example of locale.c. But if you edit any of those files and save the result, miniperl doesn't get rebuilt. It should.

Looking more closely at what happens on Windows (with current blead), when locale.c has its contents (or even just its timestamp) altered, the next gmake run will firstly recompile locale.c and then rebuild perl537.dll by running:

g++ -shared -o ..\perl537.dll  -s -L"c:\perl\lib\CORE" -L"C:\winlibs-gcc-1130\mingw64\lib" -L"C:\winlibs-gcc-1130\mingw64\x86_64-w64-mingw32\lib" -L"C:\winlibs-gcc-1130\mingw64\lib\gcc\x86_64-w64-mingw32\11.3.0" \
   ..\toke.o ..\regcomp.o ..\regcomp_trie.o ..\regcomp_debug.o ..\regcomp_invlist.o ..\regcomp_study.o ..\regexec.o ..\op.o ..\sv.o ..\pp.o ..\pp_ctl.o ..\pp_sys.o ..\pp_pack.o ..\pp_hot.o ..\gv.o ..\perl.o ..\utf8.o ..\dump.o ..\hv.o ..\av.o ..\builtin.o ..\caretx.o ..\deb.o ..\doio.o ..\doop.o ..\dquote.o ..\globals.o ..\mro_core.o ..\locale.o ..\keywords.o ..\mathoms.o ..\mg.o ..\numeric.o ..\pad.o ..\peep.o ..\perly.o ..\pp_sort.o ..\reentr.o ..\run.o ..\scope.o ..\taint.o ..\time64.o ..\universal.o ..\util.o perllib.o ..\perlio.o .\win32.o .\win32sck.o .\win32thread.o .\fcrypt.o ..\DynaLoader.o ..\lib\auto\Win32CORE\Win32CORE.a   -lmoldname -lkernel32 -luser32 -lgdi32 -lwinspool -lcomdlg32 -ladvapi32 -lshell32 -lole32 -loleaut32 -lnetapi32 -luuid -lws2_32 -lmpr -lwinmm -lversion -lodbc32 -lodbccp32 -lcomctl32 ..\lib\CORE\perl537.exp

I notice that the rebuilt locale.o is mentioned in there, and I'm thinking that nothing else needs to be done. (I assume that miniperl.exe is going to pull in that rebuilt perl537.dll.) But I don't really know.

Cheers, Rob

demerphq commented 1 year ago

On Sun, 5 Feb 2023, 11:44 sisyphus, @.***> wrote:

@khw https://github.com/khw wrote:

But the point is, if you change one of the base level .c files, miniperl doesn't get rebuilt. I used the example of locale.c. But if you edit any of those files and save the result, miniperl doesn't get rebuilt. It should.

Looking more closely at what happens on Windows (with current blead), when locale.c has its contents (or even just its timestamp) altered, the next gmake run will firstly recompile locale.c and then rebuild perl537.dll by running:

g++ -shared -o ..\perl537.dll -s -L"c:\perl\lib\CORE" -L"C:\winlibs-gcc-1130\mingw64\lib" -L"C:\winlibs-gcc-1130\mingw64\x86_64-w64-mingw32\lib" -L"C:\winlibs-gcc-1130\mingw64\lib\gcc\x86_64-w64-mingw32\11.3.0" \ ..\toke.o ..\regcomp.o ..\regcomp_trie.o ..\regcomp_debug.o ..\regcomp_invlist.o ..\regcomp_study.o ..\regexec.o ..\op.o ..\sv.o ..\pp.o ..\pp_ctl.o ..\pp_sys.o ..\pp_pack.o ..\pp_hot.o ..\gv.o ..\perl.o ..\utf8.o ..\dump.o ..\hv.o ..\av.o ..\builtin.o ..\caretx.o ..\deb.o ..\doio.o ..\doop.o ..\dquote.o ..\globals.o ..\mro_core.o ..\locale.o ..\keywords.o ..\mathoms.o ..\mg.o ..\numeric.o ..\pad.o ..\peep.o ..\perly.o ..\pp_sort.o ..\reentr.o ..\run.o ..\scope.o ..\taint.o ..\time64.o ..\universal.o ..\util.o perllib.o ..\perlio.o .\win32.o .\win32sck.o .\win32thread.o .\fcrypt.o ..\DynaLoader.o ..\lib\auto\Win32CORE\Win32CORE.a -lmoldname -lkernel32e -luser32 -lgdi32 -lwinspool -lcomdlg32 -ladvapi32 -lshell32 -lole32 -loleaut32 -lnetapi32 -luuid -lws2_32 -lmpr -lwinmm -lversion -lodbc32 -lodbccp32 -lcomctl32 ..\lib\CORE\perl537.exp

I notice that the rebuilt locale.o is mentioned in there, and I'm thinking that nothing else needs to be done. (I assume that miniperl.exe is going to pull in that rebuilt perl-537.exe.) But I don't really know

I doubt it. Miniperl is a separate perl that is statically linked, hence the reason it doesn't support XS code. Miniperl.exe should depend on the .o files, just as perl-537 would Touching a .c file used in core should result in both being built. Miniperl is used to build the XS files that are part of the final package, it is what converts the .XS files to .Pl files, among other tasks. I am not sure if it builds anything that is directly compiled as part of the final perl, but it definitely is used to build ancillary files like those in ext/dist/cpan.

Yves

sisyphus commented 1 year ago

Miniperl.exe should depend on the .o files, just as perl-537 would.

Quite right - and when I look carefully at the Windows build, I can see that miniperl.exe depends upon locale.o. Silly thinking on my part. This should probably be fixed in the Windows makefiles by someone that knows how to. AFAICS, the same issue arises with nmake and win32/Makefile.

Mind you, if make has just run successfully to completion, and I then decide to alter one of the .c files, I would hope that I had the common sense to take the defensive step of running make distclean before re-running make.

Cheers, Rob

demerphq commented 1 year ago

, I would hope that I had the common sense to take the defensive step of running make distclean before re-running make.

I think most *nix users wouldn't. I typically do so rarely, and am generally annoyed when I have to. The make framework should notice most cases where it needs to rebuild. I generally only have to when I have broken the regex engine and clobbered a generated file that is needed for the final build, and no error was triggered. I tend to git clean -dfx though, been a long time since I used any of the clean targets in our makefiles.

khwilliamson commented 1 year ago

On 2/5/23 06:17, Yves Orton wrote:

, I would hope that I had the common sense to take the defensive
step of running make distclean before re-running make.

I think most *nix users wouldn't. I typically do so rarely, and am generally annoyed when I have to. The make framework should notice most cases where it needs to rebuild. I generally only have to when I have broken the regex engine and clobbered a generated file that is needed for the final build, and no error was triggered. I tend to |git clean -dfx| though, been a long time since I used any of the clean targets in our makefiles.

— file The whole point of a make file is to know how to build things, and to keep track of what needs to be rebuilt when something changes. It's hard for me to grok that Windows doesn't have that capability.

I wouldn't have filed this ticket except I kept getting bitten by miniperl being outdated and things subtly not working.

demerphq commented 1 year ago

I think windows does have that capability, I think we just haven't been maintaining the makefile properly.

sisyphus commented 1 year ago

I wouldn't have filed this ticket except I kept getting bitten by miniperl being outdated and things subtly not working.

Quite the right thing to do. I've probably been bitten by the same subtleties in the past, but have mistakenly put them down to "user bugs" rather than "makefile bugs" - whereupon I've developed the somewhat unimaginative defensive technique of running gmake distclean beforehand, when in doubt. (Running gmake -B might be a slightly smarter alternative.)

Looking in the GNUmakefile itself, I see the following:

#do not put $(MINIPERL) as a dep/prereq in a rule, instead put $(HAVEMINIPERL)
#$(MINIPERL) is not a buildable target, use "gmake mp" if you want to just build
#miniperl alone
MINIPERL    = ..\miniperl.exe
HAVEMINIPERL    = ..\lib\buildcustomize.pl
.....

This raises the possibility that fixing the issue might not be so easy - something already touted by @khw.

I tried touching buildcustomize.pl but, although that leads to a plethora of miniperl.exe commands being re-run during gmake, it didn't actually cause miniperl.exe to be rebuilt AFAICS. It also caused numerous warnings about out of date Makefiles.

Running gmake mp is not of much help here. Naturally, it will rebuild miniperl only if the relevant target(s) cannot met - and touching/altering one of the core .c files is not presently sufficient to activate that trigger. (I don't know if that's something that could be changed.)

I notice that the perl537.dll gets rebuilt every time gmake is run, even if nothing has changed. This is not such a big issue as the rebuild proceeds quickly.

I recall that there was a period (maybe back as far as last century) when running dmake (as it was then) or nmake would cause everything to be rebuilt. Hopefully there's a better solution than that ;-) It's heartening that @demerphq believes that such is possible.

Cheers, Rob

demerphq commented 1 year ago

@sisyphus I think we can review the differences between the Win32 Makefiles and the nix Makefile and see why it works on nix and not on Win32. As I understand it the Win32 makefiles were created by Sarathy years ago as part of the merger of the ActivePerl fork back into core. I suspect that over the years fixes and improvements have been made to the *nix Makefiles that were not applied to the Win32 files.

I think that the following code snippet is close to an analog of the one in Win32, it wouldnt surprise me if we dug into this that at one point they were the same and that the win32 version did not get updated because we have/had relatively few Win32 builders on the project. Back in the pre-5.10 period when I started doing perl I was one of the few Win32 devs regularly hacking on the project, and I think over the years we have had less than a handful doing win32 stuff at any one time. I also remember that when I migrated to linux that the process was MUCH nicer. So I suspect we just have to do the work to get parity. It might be that win32 make tools are feature impoverished, or buggy, but until we can prove it we should assume that this is an issue in the Makefile itself. We seem to be quite conservative with the Makefile features we use in gnu make, so it shouldnt be that hard to figure out if we set our minds to it. Unfortunately I dont have a win32 dev environment set up right now, and im not likely for a few weeks, but i dont mind trying to help if you and @khwilliamson and any other win32 hackers can help out.

# The seemingly cranky ordering of having $(MINIPERL_EXE) depend on
# lib/buildcustomize.pl despite the reality that lib/buildcustomize.pl needs
# miniperl to exist first permits us to
# a) have one rule generate both miniperl and lib/buildcustomize.pl
#    (so that lib/buildcustomize.pl is always available. This simplifies things)
# b) have the rest of the Makefile depend on the more obvious $(MINIPERL_EXE)

$(MINIPERL_EXE): lib/buildcustomize.pl
        @touch $(MINIPERL_EXE)

lib/buildcustomize.pl: $& $(miniperl_dep) write_buildcustomize.pl
        -@rm -f miniperl.xok
        $(CC) $(CLDFLAGS) -o $(MINIPERL_EXE) \
            $(miniperl_objs) $(libs)
        $(LDLIBPTH) ./miniperl$(HOST_EXE_EXT) -w -Ilib -Idist/Exporter/lib -MExporter -e '<?>' || sh -c 'echo >&2 Faile$d to build miniperl.  Please run make minitest; exit 1'
        $(MINIPERL) -f write_buildcustomize.pl

$(PERL_EXE): $& $(perlmain_dep) $(LIBPERL) $(static_ext) ext.libs $(PERLEXPORT) write_buildcustomize.pl
        -@rm -f miniperl.xok
        $(SHRPENV) $(CC) -o perl $(CLDFLAGS) $(CCDLFLAGS) $(perlmain_objs) $(static_ext) $(LLIBPERL) `cat ext.libs` $(l$ibs)
tonycoz commented 1 year ago

I believe the PR will fix most of the miniperl build issues.

The "keeps on building" issue is present on Unix too, but the way extensions are built on Win32 makes it worse.

On Unix, each extension is built with a single make_ext.pl invocation, so make is only waiting for a single module build, so typically there isn't a huge amount of noise between make detecting the error and any other module builds from finishing, though I do know that if Encode is that build, there can still be quite a lot of noise (which isn't unreasonable.)

On Win32 all of the XS extensions are built with a single invocation of make_ext.pl, so the build fails elsewhere after XS extensions have started building there can be a huge delay and significant noise between the error and when the build finally winds down.

I think this could be improved by generating a separate extensions makefile and invoking that from the main GNUmakefile (there's no point for the nmake win32/Makefile), and it could improve build parallelism too.