Perl / perl5

đŸȘ The Perl programming language
https://dev.perl.org/perl5/
Other
1.98k stars 559 forks source link

[PATCH] Fix Parallel Building #15593

Closed p5pRT closed 8 years ago

p5pRT commented 8 years ago

Migrated from rt.perl.org#129229 (status was 'resolved')

Searchable as RT129229$

p5pRT commented 8 years ago

From @tomhukins

Created by @tomhukins

Currently\, I see parallel builds fail every time on FreeBSD. The attached patch might not be the most elegant solution but it fixes the problem as described in the commit message.

Perl Info ``` Flags: category=install severity=low Type=Patch PatchStatus=HasPatch Site configuration information for perl 5.25.5: Configured by tom at Thu Sep 8 15:58:32 UTC 2016. Summary of my perl5 (revision 5 version 25 subversion 5) configuration: Commit id: 1455a756264f698d1aa66c4ada9f431497f31d3c Platform: osname=freebsd osvers=9.3-release-p13 archname=amd64-freebsd uname='freebsd canet.scrubhole.org 9.3-release-p13 freebsd 9.3-release-p13 #1 r282738: mon may 11 01:25:08 utc 2015 root@tyne.exonetric.net:usrobjusrsrcsysgeneric amd64 ' config_args='-des -Dusedevel' hint=recommended useposix=true d_sigaction=define useithreads=undef usemultiplicity=undef use64bitint=define use64bitall=define uselongdouble=undef usemymalloc=n bincompat5005=undef Compiler: cc='cc' ccflags ='-DHAS_FPSETMASK -DHAS_FLOATINGPOINT_H -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include -D_FORTIFY_SOURCE=2' optimize='-O' cppflags='-DHAS_FPSETMASK -DHAS_FLOATINGPOINT_H -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include' ccversion='' gccversion='4.2.1 20070831 patched [FreeBSD]' gccosandvers='' intsize=4 longsize=8 ptrsize=8 doublesize=8 byteorder=12345678 doublekind=3 d_longlong=define longlongsize=8 d_longdbl=define longdblsize=16 longdblkind=3 ivtype='long' ivsize=8 nvtype='double' nvsize=8 Off_t='off_t' lseeksize=8 alignbytes=8 prototype=define Linker and Libraries: ld='cc' ldflags ='-Wl,-E -fstack-protector -L/usr/local/lib' libpth=/usr/lib /usr/local/lib /usr/include/gcc/4.2 /usr/lib libs=-lpthread -lm -lcrypt -lutil -lc perllibs=-lpthread -lm -lcrypt -lutil -lc libc= so=so useshrplib=false libperl=libperl.a gnulibc_version='' Dynamic Linking: dlsrc=dl_dlopen.xs dlext=so d_dlsymun=undef ccdlflags=' ' cccdlflags='-DPIC -fPIC' lddlflags='-shared -L/usr/local/lib -fstack-protector' @INC for perl 5.25.5: lib /usr/local/lib/perl5/site_perl/5.25.5/amd64-freebsd /usr/local/lib/perl5/site_perl/5.25.5 /usr/local/lib/perl5/5.25.5/amd64-freebsd /usr/local/lib/perl5/5.25.5 /usr/local/lib/perl5/site_perl Environment for perl 5.25.5: HOME=/home/tom LANG=en_GB.UTF-8 LANGUAGE (unset) LD_LIBRARY_PATH (unset) LOGDIR (unset) PATH=/sbin:/bin:/usr/sbin:/usr/bin:/usr/games:/usr/local/sbin:/usr/local/bin:/home/tom/bin PERL_BADLANG (unset) SHELL=/usr/local/bin/zsh ```
p5pRT commented 8 years ago

From @tomhukins

0001-Fix-parallel-building.patch ```diff From 309830d1990f6125d1fed459cf15f8e424b7dfab Mon Sep 17 00:00:00 2001 From: Tom Hukins Date: Thu, 8 Sep 2016 16:01:36 +0000 Subject: [PATCH] Fix parallel building Commit 3dfcef7e97bf1b516f added a dependency on Porting/pod_lib.pl to Porting/manisort. Porting/pod_lib.pl depends on File::Find which in turn depends on Cwd. Stating Cwd as a requirement ensures the $(MANIFEST_SRT) target builds successfully with "make -jX" where X > 1. Without this fix, I see this target fail every time because perl can't load Cwd. --- Makefile.SH | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile.SH b/Makefile.SH index 42beb81..27f1e66 100755 --- a/Makefile.SH +++ b/Makefile.SH @@ -600,7 +600,7 @@ all: $(FIRSTMAKEFILE) $(MINIPERL_EXE) $(generated_pods) $(private) $(unidatafile @echo " "; @echo " Everything is up to date. Type '$(MAKE) test' to run test suite." -$(MANIFEST_SRT): MANIFEST $(PERL_EXE) +$(MANIFEST_SRT): MANIFEST $(PERL_EXE) lib/auto/Cwd/Cwd$(DLSUFFIX) @$(RUN_PERL) Porting/manisort -q || (echo "WARNING: re-sorting MANIFEST"; \ $(RUN_PERL) Porting/manisort -q -o MANIFEST; sh -c true) @touch $(MANIFEST_SRT) -- 2.9.2 ```
p5pRT commented 8 years ago

From @jkeenan

On Thu Sep 08 09​:05​:09 2016\, tomhukins wrote​:

This is a bug report for perl from tom@​eborcom.com\, generated with the help of perlbug 1.40 running under perl 5.25.5.

----------------------------------------------------------------- [Please describe your issue here]

Currently\, I see parallel builds fail every time on FreeBSD. The attached patch might not be the most elegant solution but it fixes the problem as described in the commit message.

I'm a bit puzzled by this report. I have been smoke-testing blead on FreeBSD approximately twice a week for the past month\, typically with '-j8'\, and I have yet to see evidence of this problem. Could you provide more evidence of what you're seeing?

Also\, AFAICT there was nothing FreeBSD-specific in commit 3dfcef7e97bf1b516f. Nor is there anything FreeBSD-specific in your patch. 3dfcef7e97bf1b516f was committed to blead on June 21. Hence\, if there were a problem with parallel builds I would have expected it to show up on multiple OSes\, not just FreeBSD\, and to have started to appear three months ago. Can you clarify?

Thank you very much. -- James E Keenan (jkeenan@​cpan.org)

p5pRT commented 8 years ago

The RT System itself - Status changed from 'new' to 'open'

p5pRT commented 8 years ago

From @tomhukins

On Sun\, Sep 18\, 2016 at 04​:52​:00PM -0700\, James E Keenan via RT wrote​:

I'm a bit puzzled by this report. I have been smoke-testing blead on FreeBSD approximately twice a week for the past month\, typically with '-j8'\, and I have yet to see evidence of this problem.

In case I was unclear\, I wouldn't expect a dependency error to always occur under parallel builds in every environment. However\, in one of my environments\, it reliably does.

Could you provide more evidence of what you're seeing?

Here's the end of the output I see when running sh -c 'sh Configure -des -Dusedevel && make -j4'

Processing PropertyAliases.txt cc -c -DHAS_FPSETMASK -DHAS_FLOATINGPOINT_H -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include -D_FORTIFY_SOURCE=2 -Wall -Werror=declaration-after-statement -Wextra -Wc++-compat -Wwrite-strings -O -DVERSION=\"1.39\" -DXS_VERSION=\"1.39\" "-I../.." -DLIBC="" DynaLoader.c Finishing property setup Processing PropValueAliases.txt rm -rf ../../DynaLoader.o cp DynaLoader.o ../../DynaLoader.o rm -f libperl.a /usr/bin/ar rc libperl.a op.o perl.o gv.o toke.o perly.o pad.o regcomp.o dump.o util.o mg.o reentr.o mro_core.o keywords.o hv.o av.o run.o pp_hot.o sv.o pp.o scope.o pp_ctl.o pp_sys.o doop.o doio.o regexec.o utf8.o taint.o deb.o universal.o globals.o perlio.o perlapi.o numeric.o mathoms.o locale.o pp_pack.o pp_sort.o caretx.o dquote.o time64.o DynaLoader.o cc -o perl -Wl\,-E -fstack-protector -L/usr/local/lib perlmain.o libperl.a `cat ext.libs` -lpthread -lm -lcrypt -lutil -lc Processing extracted/DGeneralCategory.txt Can't locate Cwd.pm in @​INC (you may need to install the Cwd module) (@​INC contains​: lib /usr/local/lib/perl5/site_perl/5.25.5/amd64-freebsd /usr/local/lib/perl5/site_perl/5.25.5 /usr/local/lib/perl5/5.25.5/amd64-freebsd /usr/local/lib/perl5/5.25.5 /usr/local/lib/perl5/site_perl .) at lib/File/Find.pm line 8. Compilation failed in require at Porting/pod_lib.pl line 4. BEGIN failed--compilation aborted at Porting/pod_lib.pl line 4. Compilation failed in require at Porting/manisort line 17. WARNING​: re-sorting MANIFEST Can't locate Cwd.pm in @​INC (you may need to install the Cwd module) (@​INC contains​: lib /usr/local/lib/perl5/site_perl/5.25.5/amd64-freebsd /usr/local/lib/perl5/site_perl/5.25.5 /usr/local/lib/perl5/5.25.5/amd64-freebsd /usr/local/lib/perl5/5.25.5 /usr/local/lib/perl5/site_perl .) at lib/File/Find.pm line 8. Compilation failed in require at Porting/pod_lib.pl line 4. BEGIN failed--compilation aborted at Porting/pod_lib.pl line 4. Compilation failed in require at Porting/manisort line 17. *** [MANIFEST.srt] Error code 2 magic​: PERL_MAGIC_qr already provided by misc including ppphdoc including ppphbin including version including threads including limits including uv including memory including misc including variables including mPUSH including call including newRV including newCONSTSUB including MY_CXT including format including SvREFCNT including newSV_type including newSVpv including SvPV including Sv_set including sv_xpvf including shared_pv including HvNAME including gv including warn including pvs including magic including cop including grok including snprintf including sprintf including exception including strlfuncs including pv_tools running "/usr/home/tom/checkouts/perl5/miniperl" -I../../lib ppport_h.PL Processing extracted/DCombiningClass.txt installing ppport.h for cpan/DB_File installing ppport.h for cpan/IPC-SysV installing ppport.h for cpan/Scalar-List-Utils installing ppport.h for cpan/Win32API-File installing ppport.h for dist/PathTools installing ppport.h for dist/Time-HiRes removing temporary file PPPort.pm removing temporary file ppport.h Processing extracted/DNumType.txt Processing extracted/DEastAsianWidth.txt Processing extracted/DLineBreak.txt Processing extracted/DBidiClass.txt Processing extracted/DDecompositionType.txt Processing extracted/DBinaryProperties.txt Processing extracted/DNumValues.txt Processing extracted/DJoinGroup.txt Processing extracted/DJoinType.txt Processing Jamo.txt Processing UnicodeData.txt Processing ArabicShaping.txt Processing Blocks.txt Processing PropList.txt Processing SpecialCasing.txt Processing LineBreak.txt Processing EastAsianWidth.txt Processing CompositionExclusions.txt Processing BidiMirroring.txt Processing CaseFolding.txt Processing DCoreProperties.txt Processing Scripts.txt Processing DNormalizationProps.txt Processing DAge.txt Processing HangulSyllableType.txt Processing auxiliary/WordBreakProperty.txt Processing auxiliary/GraphemeBreakProperty.txt Processing auxiliary/GCBTest.txt Processing auxiliary/SBTest.txt Processing auxiliary/WBTest.txt Processing auxiliary/SentenceBreakProperty.txt Processing NamedSequences.txt Processing NameAliases.txt Processing auxiliary/LBTest.txt Processing ScriptExtensions.txt Processing IndicSyllabicCategory.txt Processing BidiBrackets.txt Processing IndicPositionalCategory.txt Finishing processing Unicode properties Compiling Perl properties Creating Perl synonyms Writing tables Making pod file Making test script Updating 'mktables.lst' 1 error

Also\, AFAICT there was nothing FreeBSD-specific in commit 3dfcef7e97bf1b516f. Nor is there anything FreeBSD-specific in your patch.

I agree with both your statements.

3dfcef7e97bf1b516f was committed to blead on June 21. Hence\, if there were a problem with parallel builds I would have expected it to show up on multiple OSes\, not just FreeBSD\, and to have started to appear three months ago. Can you clarify?

Yes\, I don't mean to suggest that the build will fail every time time in every environment. I have one particular environment where I can make it occur reliably\, but it may not do so elsewhere.

I noticed that setting "-Doptimize='-O2'" makes the problem go away in my environment\, for example.

As I wrote in my commit message\, I believe the cause of these symptoms is an additional dependency on Cwd added in 3dfcef7e97bf1b516f that has not been reflected in the build system. I believe my patch addresses that\, although I recognise it might do so suboptimally or inelegantly.

Tom

p5pRT commented 8 years ago

From @Leont

On Mon\, Sep 19\, 2016 at 1​:52 AM\, James E Keenan via RT \< perlbug-followup@​perl.org> wrote​:

I'm a bit puzzled by this report. I have been smoke-testing blead on FreeBSD approximately twice a week for the past month\, typically with '-j8'\, and I have yet to see evidence of this problem. Could you provide more evidence of what you're seeing?

Which make do you use? Gnu make or BSD make? Same question to Tom obviously. Such a difference may explain the difference in observed behavior.

Leon

p5pRT commented 8 years ago

From @tomhukins

On Mon\, Sep 19\, 2016 at 04​:10​:59PM +0200\, Leon Timmermans wrote​:

Which make do you use? Gnu make or BSD make?

I'm using the default BSD make that ships with FreeBSD 9.3.

Tom

p5pRT commented 8 years ago

From @jkeenan

On Mon Sep 19 06​:50​:57 2016\, tomhukins wrote​:

On Sun\, Sep 18\, 2016 at 04​:52​:00PM -0700\, James E Keenan via RT wrote​:

I'm a bit puzzled by this report. I have been smoke-testing blead on FreeBSD approximately twice a week for the past month\, typically with '-j8'\, and I have yet to see evidence of this problem.

In case I was unclear\, I wouldn't expect a dependency error to always occur under parallel builds in every environment. However\, in one of my environments\, it reliably does.

Could you provide more evidence of what you're seeing?

Here's the end of the output I see when running sh -c 'sh Configure -des -Dusedevel && make -j4'

Processing PropertyAliases.txt cc -c -DHAS_FPSETMASK -DHAS_FLOATINGPOINT_H -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include -D_FORTIFY_SOURCE=2 -Wall -Werror=declaration-after-statement -Wextra -Wc++-compat -Wwrite- strings -O -DVERSION=\"1.39\" -DXS_VERSION=\"1.39\" "-I../.." -DLIBC="" DynaLoader.c Finishing property setup Processing PropValueAliases.txt rm -rf ../../DynaLoader.o cp DynaLoader.o ../../DynaLoader.o rm -f libperl.a /usr/bin/ar rc libperl.a op.o perl.o gv.o toke.o perly.o pad.o regcomp.o dump.o util.o mg.o reentr.o mro_core.o keywords.o hv.o av.o run.o pp_hot.o sv.o pp.o scope.o pp_ctl.o pp_sys.o doop.o doio.o regexec.o utf8.o taint.o deb.o universal.o globals.o perlio.o perlapi.o numeric.o mathoms.o locale.o pp_pack.o pp_sort.o caretx.o dquote.o time64.o DynaLoader.o cc -o perl -Wl\,-E -fstack-protector -L/usr/local/lib perlmain.o libperl.a `cat ext.libs` -lpthread -lm -lcrypt -lutil -lc Processing extracted/DGeneralCategory.txt Can't locate Cwd.pm in @​INC (you may need to install the Cwd module) (@​INC contains​: lib /usr/local/lib/perl5/site_perl/5.25.5/amd64- freebsd /usr/local/lib/perl5/site_perl/5.25.5 /usr/local/lib/perl5/5.25.5/amd64-freebsd /usr/local/lib/perl5/5.25.5 /usr/local/lib/perl5/site_perl .) at lib/File/Find.pm line 8. Compilation failed in require at Porting/pod_lib.pl line 4. BEGIN failed--compilation aborted at Porting/pod_lib.pl line 4. Compilation failed in require at Porting/manisort line 17. WARNING​: re-sorting MANIFEST Can't locate Cwd.pm in @​INC (you may need to install the Cwd module) (@​INC contains​: lib /usr/local/lib/perl5/site_perl/5.25.5/amd64- freebsd /usr/local/lib/perl5/site_perl/5.25.5 /usr/local/lib/perl5/5.25.5/amd64-freebsd /usr/local/lib/perl5/5.25.5 /usr/local/lib/perl5/site_perl .) at lib/File/Find.pm line 8. Compilation failed in require at Porting/pod_lib.pl line 4. BEGIN failed--compilation aborted at Porting/pod_lib.pl line 4. Compilation failed in require at Porting/manisort line 17. *** [MANIFEST.srt] Error code 2 magic​: PERL_MAGIC_qr already provided by misc including ppphdoc including ppphbin including version including threads including limits including uv including memory including misc including variables including mPUSH including call including newRV including newCONSTSUB including MY_CXT including format including SvREFCNT including newSV_type including newSVpv including SvPV including Sv_set including sv_xpvf including shared_pv including HvNAME including gv including warn including pvs including magic including cop including grok including snprintf including sprintf including exception including strlfuncs including pv_tools running "/usr/home/tom/checkouts/perl5/miniperl" -I../../lib ppport_h.PL Processing extracted/DCombiningClass.txt installing ppport.h for cpan/DB_File installing ppport.h for cpan/IPC-SysV installing ppport.h for cpan/Scalar-List-Utils installing ppport.h for cpan/Win32API-File installing ppport.h for dist/PathTools installing ppport.h for dist/Time-HiRes removing temporary file PPPort.pm removing temporary file ppport.h Processing extracted/DNumType.txt Processing extracted/DEastAsianWidth.txt Processing extracted/DLineBreak.txt Processing extracted/DBidiClass.txt Processing extracted/DDecompositionType.txt Processing extracted/DBinaryProperties.txt Processing extracted/DNumValues.txt Processing extracted/DJoinGroup.txt Processing extracted/DJoinType.txt Processing Jamo.txt Processing UnicodeData.txt Processing ArabicShaping.txt Processing Blocks.txt Processing PropList.txt Processing SpecialCasing.txt Processing LineBreak.txt Processing EastAsianWidth.txt Processing CompositionExclusions.txt Processing BidiMirroring.txt Processing CaseFolding.txt Processing DCoreProperties.txt Processing Scripts.txt Processing DNormalizationProps.txt Processing DAge.txt Processing HangulSyllableType.txt Processing auxiliary/WordBreakProperty.txt Processing auxiliary/GraphemeBreakProperty.txt Processing auxiliary/GCBTest.txt Processing auxiliary/SBTest.txt Processing auxiliary/WBTest.txt Processing auxiliary/SentenceBreakProperty.txt Processing NamedSequences.txt Processing NameAliases.txt Processing auxiliary/LBTest.txt Processing ScriptExtensions.txt Processing IndicSyllabicCategory.txt Processing BidiBrackets.txt Processing IndicPositionalCategory.txt Finishing processing Unicode properties Compiling Perl properties Creating Perl synonyms Writing tables Making pod file Making test script Updating 'mktables.lst' 1 error

Aha! You are not alone! I reported this (albeit\, without much detail or insight) on list a little over two weeks ago​:

http​://www.nntp.perl.org/group/perl.perl5.porters/2016/09/msg239501.html

And because the errors were only intermittent\, I forgot about them.

And the OS in question was FreeBSD 10.3; I don't see this on Ubuntu Linux 16.04 LTS.

Replying to Leon​:

The version of make which I use there is whatever the default version of make is on FreeBSD. There doesn't appear to be a --version switch for make\, but 'man make' contains the following​:

##### Other make dialects (GNU make\, SVR4 make\, POSIX make\, etc.) do not support most of the features of make as described in this manual. ... This make implementationis based on Adam De Boor's pmake program which was written for Sprite at Berkeley. #####

Also\, AFAICT there was nothing FreeBSD-specific in commit 3dfcef7e97bf1b516f. Nor is there anything FreeBSD-specific in your patch.

I agree with both your statements.

3dfcef7e97bf1b516f was committed to blead on June 21. Hence\, if there were a problem with parallel builds I would have expected it to show up on multiple OSes\, not just FreeBSD\, and to have started to appear three months ago. Can you clarify?

Yes\, I don't mean to suggest that the build will fail every time time in every environment. I have one particular environment where I can make it occur reliably\, but it may not do so elsewhere.

I noticed that setting "-Doptimize='-O2'" makes the problem go away in my environment\, for example.

As I wrote in my commit message\, I believe the cause of these symptoms is an additional dependency on Cwd added in 3dfcef7e97bf1b516f that has not been reflected in the build system. I believe my patch addresses that\, although I recognise it might do so suboptimally or inelegantly.

Tom

I will try to build some perls on FreeBSD and see if I can capture error output like yours.

Thank you very much.

-- James E Keenan (jkeenan@​cpan.org)

p5pRT commented 8 years ago

From @jkeenan

On Mon Sep 19 09​:33​:14 2016\, jkeenan wrote​:

On Mon Sep 19 06​:50​:57 2016\, tomhukins wrote​: [snip]

As I wrote in my commit message\, I believe the cause of these symptoms is an additional dependency on Cwd added in 3dfcef7e97bf1b516f that has not been reflected in the build system. I believe my patch addresses that\, although I recognise it might do so suboptimally or inelegantly.

Tom

I will try to build some perls on FreeBSD and see if I can capture error output like yours.

Well\, I've now on my 43rd build of blead on FreeBSD 10.3 -- and have yet to get a build failure!

In any event\, I agree with the logic in Tom's patch and believe it is worth trying out. I have pushed it to this smoke-testing branch​:

smoke-me/jkeenan/129229-cwd-dependency

Please try this out on multiple OSes\, as the fix will apply to all platforms\, not just FreeBSD.

Thank you very much.

-- James E Keenan (jkeenan@​cpan.org)

p5pRT commented 8 years ago

From @kentfredric

On 19 September 2016 at 11​:52\, James E Keenan via RT \perlbug\-followup@&#8203;perl\.org wrote​:

Hence\, if there were a problem with parallel builds I would have expected it to show up on multiple OSes\, not just FreeBSD\, and to have started to appear three months ago. Can you clarify?

Its been showing up since at least 5.24.0 for me on linux ( That is\, before 3dfcef7e97bf1b516f ) \, but I have special circumstances that cause the frequency of appearing​: One of our patches changed the MANIFEST\, which causes the *need* to resort the MANIFEST\, which causes the build order problem about 10% of the time.

( Granted whether or not we need to patch the manifest is a separate issue\, however\, its just how we came to know of it\, and will be glad to see fixed )

If there were other things that happened which caused the manifest to need re-sorting\, it would cause the same problem.

3dfcef7e97bf1b516f just appears to have shuffled the deck enough that other people see it now as well.

-- Kent

KENTNL - https://metacpan.org/author/KENTNL

p5pRT commented 8 years ago

From @cpansprout

On Mon Sep 19 23​:50​:58 2016\, kentfredric@​gmail.com wrote​:

On 19 September 2016 at 11​:52\, James E Keenan via RT \perlbug\-followup@&#8203;perl\.org wrote​:

Hence\, if there were a problem with parallel builds I would have expected it to show up on multiple OSes\, not just FreeBSD\, and to have started to appear three months ago. Can you clarify?

Its been showing up since at least 5.24.0 for me on linux ( That is\, before 3dfcef7e97bf1b516f ) \, but I have special circumstances that cause the frequency of appearing​: One of our patches changed the MANIFEST\, which causes the *need* to resort the MANIFEST\, which causes the build order problem about 10% of the time.

( Granted whether or not we need to patch the manifest is a separate issue\, however\, its just how we came to know of it\, and will be glad to see fixed )

If there were other things that happened which caused the manifest to need re-sorting\, it would cause the same problem.

3dfcef7e97bf1b516f just appears to have shuffled the deck enough that other people see it now as well.

What I want to know is why the build is trying to sort MANIFEST at all. Ideally\, ‘make’ should *never* touch checked-in files\, because then ‘make distclean’ doesn’t work. You can’t get a clean slate unless you are using a git repository.

--

Father Chrysostomos

p5pRT commented 8 years ago

From @cpansprout

On Tue Sep 20 00​:36​:46 2016\, sprout wrote​:

What I want to know is why the build is trying to sort MANIFEST at all. Ideally\, ‘make’ should *never* touch checked-in files\, because then ‘make distclean’ doesn’t work. You can’t get a clean slate unless you are using a git repository.

Does the attached diff also fix the problem? (Instead of working around the problem\, it removes the cause.)

--

Father Chrysostomos

p5pRT commented 8 years ago

From @cpansprout

Inline Patch ```diff diff --git a/Makefile.SH b/Makefile.SH index 42beb81..8007e43 100755 --- a/Makefile.SH +++ b/Makefile.SH @@ -596,7 +596,7 @@ splintfiles = $(c1) @echo `$(CCCMDSRC)` -S $*.c @`$(CCCMDSRC)` -S $*.c -all: $(FIRSTMAKEFILE) $(MINIPERL_EXE) $(generated_pods) $(private) $(unidatafiles) $(public) $(dynamic_ext) $(nonxs_ext) extras.make $(MANIFEST_SRT) +all: $(FIRSTMAKEFILE) $(MINIPERL_EXE) $(generated_pods) $(private) $(unidatafiles) $(public) $(dynamic_ext) $(nonxs_ext) extras.make @echo " "; @echo " Everything is up to date. Type '$(MAKE) test' to run test suite." ```
p5pRT commented 8 years ago

From @cpansprout

On Tue Sep 20 00​:40​:13 2016\, sprout wrote​:

On Tue Sep 20 00​:36​:46 2016\, sprout wrote​:

What I want to know is why the build is trying to sort MANIFEST at all. Ideally\, ‘make’ should *never* touch checked-in files\, because then ‘make distclean’ doesn’t work. You can’t get a clean slate unless you are using a git repository.

Does the attached diff also fix the problem? (Instead of working around the problem\, it removes the cause.)

Digging a little further\, I see this commit​:

commit 19bf1007743b4337230ad3a4538df4bd94311fc4 Author​: Yves Orton \demerphq@&#8203;gmail\.com Date​: Thu Dec 25 15​:44​:14 2014 +0100

  automatically sort the MANIFEST if necessary  
  Instead of harrasing people to sort the manifest in our   tests\, we can just automatically sort the manifest when it   changes.  
  That way the tests are actually testing that the auto-sort   worked\, and not that our devs put the new file in the right   place.

The problem with automatically sorting the MANIFEST is what I mentioned above​: ‘make’ should not make changes to files tracked by git. I think that commit was ill-advised.

--

Father Chrysostomos

p5pRT commented 8 years ago

From @tomhukins

On Tue\, Sep 20\, 2016 at 12​:40​:13AM -0700\, Father Chrysostomos via RT wrote​:

Does the attached diff also fix the problem? (Instead of working around the problem\, it removes the cause.)

Yes\, I ran "make -j4" three times with this patch and saw no failures.

Tom

p5pRT commented 8 years ago

From @kentfredric

On 20 September 2016 at 19​:43\, Father Chrysostomos via RT \perlbug\-followup@&#8203;perl\.org wrote​:

The problem with automatically sorting the MANIFEST is what I mentioned above​: ‘make’ should not make changes to files tracked by git. I think that commit was ill-advised.

Before that commit\, the patching the manifest causes a handful of porting tests to fail *every* time\, instead of only failing due to Cwd.pm 10% of the time.

And given we're patching\, compiling\, testing\, and then installing on the users machine ( Gentoo )\, failing simply because the manifest isn't sorted is not great for the deployment side of the equation.

But I'll have to see if the fix works ( hard to know for sure because 10% is low enough you could do dozens of builds without one failing ).

I suspect it will\, because I'd have suggested a similar patch.

-- Kent

KENTNL - https://metacpan.org/author/KENTNL

p5pRT commented 8 years ago

From @iabyn

On Tue\, Sep 20\, 2016 at 10​:59​:42PM +1200\, Kent Fredric wrote​:

On 20 September 2016 at 19​:43\, Father Chrysostomos via RT \perlbug\-followup@&#8203;perl\.org wrote​:

The problem with automatically sorting the MANIFEST is what I mentioned above​: ‘make’ should not make changes to files tracked by git. I think that commit was ill-advised.

Before that commit\, the patching the manifest causes a handful of porting tests to fail *every* time\, instead of only failing due to Cwd.pm 10% of the time.

And given we're patching\, compiling\, testing\, and then installing on the users machine ( Gentoo )\, failing simply because the manifest isn't sorted is not great for the deployment side of the equation.

But I'll have to see if the fix works ( hard to know for sure because 10% is low enough you could do dozens of builds without one failing ).

I suspect it will\, because I'd have suggested a similar patch.

How about we just don't bother keeping MANIFEST strictly sorted during development\, (and so don't automatically test it for sortedness)\, and instead have it as an extra manual step in the release process ("step 97​: run 'make manisort' and commit if MANIFEST has changed"). So it just gets done once per month and doesn't get in the way of everyone else.

That does however leave open the issue of whether we should still automatically check for correctness (i.e. that MANFEST == git ls-files).

-- Wesley Crusher gets beaten up by his classmates for being a smarmy git\, and consequently has a go at making some friends of his own age for a change.   -- Things That Never Happen in "Star Trek" #18

p5pRT commented 8 years ago

From @cpansprout

On Tue Sep 20 05​:07​:29 2016\, davem wrote​:

How about we just don't bother keeping MANIFEST strictly sorted during development\, (and so don't automatically test it for sortedness)\, and instead have it as an extra manual step in the release process ("step 97​: run 'make manisort' and commit if MANIFEST has changed"). So it just gets done once per month and doesn't get in the way of everyone else.

That makes sense. I was thinking of proposing that next if anyone objected to a simple revert of the change. I have been wondering since I started hacking on perl why we needed to keep it sorted anyway.

The best way to solve something that shouldn’t be a problem to begin with is to remove the problem instead of working around it.

That does however leave open the issue of whether we should still automatically check for correctness (i.e. that MANFEST == git ls- files).

manifest.t should continue to perform its other tests.

--

Father Chrysostomos

p5pRT commented 8 years ago

From @nawglan

On Tue Sep 20 05​:59​:20 2016\, sprout wrote​:

On Tue Sep 20 05​:07​:29 2016\, davem wrote​:

How about we just don't bother keeping MANIFEST strictly sorted during development\, (and so don't automatically test it for sortedness)\, and instead have it as an extra manual step in the release process ("step 97​: run 'make manisort' and commit if MANIFEST has changed"). So it just gets done once per month and doesn't get in the way of everyone else.

That makes sense. I was thinking of proposing that next if anyone objected to a simple revert of the change. I have been wondering since I started hacking on perl why we needed to keep it sorted anyway.

The best way to solve something that shouldn’t be a problem to begin with is to remove the problem instead of working around it.

That does however leave open the issue of whether we should still automatically check for correctness (i.e. that MANFEST == git ls- files).

manifest.t should continue to perform its other tests.

Can the MANIFEST file be sorted via a git hook?

-- Dez.

p5pRT commented 8 years ago

From @dcollinsn

If MANIFEST is procedurally generated\, is there such a thing as a pre-commit hook\, either on the client side (for every commit) or on the server side (for pushing to blead/maint-5.*) that would prompt a committer to run manisort\, or do so automatically\, if necessary? That might help prevent the "MANIFEST is not sorted" situation to begin with.

-- Dan Collins

On Tue\, Sep 20\, 2016 at 8​:59 AM\, Father Chrysostomos via RT \< perlbug-followup@​perl.org> wrote​:

On Tue Sep 20 05​:07​:29 2016\, davem wrote​:

How about we just don't bother keeping MANIFEST strictly sorted during development\, (and so don't automatically test it for sortedness)\, and instead have it as an extra manual step in the release process ("step 97​: run 'make manisort' and commit if MANIFEST has changed"). So it just gets done once per month and doesn't get in the way of everyone else.

That makes sense. I was thinking of proposing that next if anyone objected to a simple revert of the change. I have been wondering since I started hacking on perl why we needed to keep it sorted anyway.

The best way to solve something that shouldn’t be a problem to begin with is to remove the problem instead of working around it.

That does however leave open the issue of whether we should still automatically check for correctness (i.e. that MANFEST == git ls- files).

manifest.t should continue to perform its other tests.

--

Father Chrysostomos

--- via perlbug​: queue​: perl5 status​: open https://rt-archive.perl.org/perl5/Ticket/Display.html?id=129229

p5pRT commented 8 years ago

From @demerphq

On 20 Sep 2016 8​:59 a.m.\, "Father Chrysostomos via RT" \< perlbug-followup@​perl.org> wrote​:

On Tue Sep 20 05​:07​:29 2016\, davem wrote​:

How about we just don't bother keeping MANIFEST strictly sorted during development\, (and so don't automatically test it for sortedness)\, and instead have it as an extra manual step in the release process ("step 97​: run 'make manisort' and commit if MANIFEST has changed"). So it just gets done once per month and doesn't get in the way of everyone else.

That makes sense. I was thinking of proposing that next if anyone objected to a simple revert of the change. I have been wondering since I started hacking on perl why we needed to keep it sorted anyway.

I object to your change so long as we test for sorted manifests routinely. â˜ș

As the comment shows I only made the change to eliminate manual build steps which were mandated by these tests.

The best way to solve something that shouldn’t be a problem to begin with is to remove the problem instead of working around it.

That assumes there is consensus about what the problem is. Historically when these kinds of tests have generated developer frustration there has not been agreement on removing the test or changing our build process to remove the frustration. For instance I have in the past proposed much the same build test policy for porting tests as you have here without achieving consensus that it was a good idea.

So when this issue came up for me I did what hackers always do and automated a solution to my frustrations. I'm sorry it caused issues\, but not sorry I tried to eliminate make-work from the build process.

That does however leave open the issue of whether we should still automatically check for correctness (i.e. that MANFEST == git ls- files).

manifest.t should continue to perform its other tests.

I think manifest.t shouldn't be an automated test. We should have a make test_porting target to run them prerelease.

Yves

p5pRT commented 8 years ago

From @demerphq

On 20 Sep 2016 3​:43 a.m.\, "Father Chrysostomos via RT" \< perlbug-followup@​perl.org> wrote​:

On Tue Sep 20 00​:40​:13 2016\, sprout wrote​:

On Tue Sep 20 00​:36​:46 2016\, sprout wrote​:

What I want to know is why the build is trying to sort MANIFEST at all. Ideally\, ‘make’ should *never* touch checked-in files\, because then ‘make distclean’ doesn’t work. You can’t get a clean slate unless you are using a git repository.

Does the attached diff also fix the problem? (Instead of working around the problem\, it removes the cause.)

Digging a little further\, I see this commit​:

commit 19bf1007743b4337230ad3a4538df4bd94311fc4 Author​: Yves Orton \demerphq@&#8203;gmail\.com Date​: Thu Dec 25 15​:44​:14 2014 +0100

automatically sort the MANIFEST if necessary

Instead of harrasing people to sort the manifest in our
tests\, we can just automatically sort the manifest when it
changes\.

That way the tests are actually testing that the auto\-sort
worked\, and not that our devs put the new file in the right
place\.

The problem with automatically sorting the MANIFEST is what I mentioned above​: ‘make’ should not make changes to files tracked by git. I think that commit was ill-advised.

I think we have to differ on that one. The only way a patch ends up upstream in an incorrect order is if someone made changes to it and never ran make at all. Thus the scenario you describe would never occur on an unmatched perl.

And I don't agree about your make clean argument either in your other post . Make clean does not undo changes to source code. The only way you end up in the scenario you describe as undesirable is when you have made changes to files in the first place and make clean is not intended to undo such changes.

That make might correct an innocuous error you made in a source file while making changes seems quite reasonable to me.

Anyway my view is that if we routinely test for something that can be fixed with a make command then make should just do that command automatically and save a lot of fuss. (all things being equal etc)

Yves

p5pRT commented 8 years ago

From @cpansprout

On Tue Sep 20 07​:26​:04 2016\, demerphq wrote​:

On 20 Sep 2016 3​:43 a.m.\, "Father Chrysostomos via RT" \< perlbug-followup@​perl.org> wrote​:

On Tue Sep 20 00​:40​:13 2016\, sprout wrote​:

On Tue Sep 20 00​:36​:46 2016\, sprout wrote​:

What I want to know is why the build is trying to sort MANIFEST at all. Ideally\, ‘make’ should *never* touch checked-in files\, because then ‘make distclean’ doesn’t work. You can’t get a clean slate unless you are using a git repository.

Does the attached diff also fix the problem? (Instead of working around the problem\, it removes the cause.)

Digging a little further\, I see this commit​:

commit 19bf1007743b4337230ad3a4538df4bd94311fc4 Author​: Yves Orton \demerphq@&#8203;gmail\.com Date​: Thu Dec 25 15​:44​:14 2014 +0100

automatically sort the MANIFEST if necessary

Instead of harrasing people to sort the manifest in our
tests\, we can just automatically sort the manifest when it
changes\.

That way the tests are actually testing that the auto\-sort
worked\, and not that our devs put the new file in the right
place\.

The problem with automatically sorting the MANIFEST is what I mentioned above​: ‘make’ should not make changes to files tracked by git. I think that commit was ill-advised.

I think we have to differ on that one. The only way a patch ends up upstream in an incorrect order is if someone made changes to it and never ran make at all. Thus the scenario you describe would never occur on an unmatched perl.

1. Make changes 2. git commit 3. make test 4. All tests pass\, so push.

I follow that routine routinely. If I get ‘Can’t rebase because you have unstaged changes’ when I *know* I have committed\, I am going to get very frustrated. :-)

And I don't agree about your make clean argument either in your other post . Make clean does not undo changes to source code. The only way you end up in the scenario you describe as undesirable is when you have made changes to files in the first place

But what if I haven’t made any changes\, but simply tried to build perl?

and make clean is not intended to undo such changes.

That make might correct an innocuous error you made in a source file while making changes seems quite reasonable to me.

Anyway my view is that if we routinely test for something that can be fixed with a make command then make should just do that command automatically

No\, instead of doing that command automatically\, have a ‘make porting’ target that gets the tests passing\, so that the hacker has more control over when things get run. For non-critical things like manifest sorting\, we can remove the tests and have ‘make porting’ as a standard thing to run when making a release.

(If people want to run ‘make porting’ from time to time themselves\, they can.)

and save a lot of fuss. (all things being equal etc)

I hope my suggestions will do the same\, but without causing problems.

--

Father Chrysostomos

p5pRT commented 8 years ago

From @demerphq

On 20 Sep 2016 12​:51\, "Father Chrysostomos via RT" \< perlbug-followup@​perl.org> wrote​:

On Tue Sep 20 07​:26​:04 2016\, demerphq wrote​:

On 20 Sep 2016 3​:43 a.m.\, "Father Chrysostomos via RT" \< perlbug-followup@​perl.org> wrote​:

On Tue Sep 20 00​:40​:13 2016\, sprout wrote​:

On Tue Sep 20 00​:36​:46 2016\, sprout wrote​:

What I want to know is why the build is trying to sort MANIFEST at all. Ideally\, ‘make’ should *never* touch checked-in files\, because then ‘make distclean’ doesn’t work. You can’t get a clean slate unless you are using a git repository.

Does the attached diff also fix the problem? (Instead of working around the problem\, it removes the cause.)

Digging a little further\, I see this commit​:

commit 19bf1007743b4337230ad3a4538df4bd94311fc4 Author​: Yves Orton \demerphq@&#8203;gmail\.com Date​: Thu Dec 25 15​:44​:14 2014 +0100

automatically sort the MANIFEST if necessary

Instead of harrasing people to sort the manifest in our
tests\, we can just automatically sort the manifest when it
changes\.

That way the tests are actually testing that the auto\-sort
worked\, and not that our devs put the new file in the right
place\.

The problem with automatically sorting the MANIFEST is what I mentioned above​: ‘make’ should not make changes to files tracked by git. I think that commit was ill-advised.

I think we have to differ on that one. The only way a patch ends up upstream in an incorrect order is if someone made changes to it and never ran make at all. Thus the scenario you describe would never occur on an unmatched perl.

1. Make changes 2. git commit 3. make test 4. All tests pass\, so push.

I follow that routine routinely. If I get ‘Can’t rebase because you have unstaged changes’ when I *know* I have committed\, I am going to get very frustrated. :-)

Not as frustrated as I get when I make a change and it fails test because of failing manifest.t

I mean has it happened even once yet?

Also personally I think committing before you run make test is a bit optimistic. I mean it's like that line about the doctor... So don't do that... â˜ș

And I don't agree about your make clean argument either in your other post . Make clean does not undo changes to source code. The only way you end up in the scenario you describe as undesirable is when you have made changes to files in the first place

But what if I haven’t made any changes\, but simply tried to build perl?

Lots of what ifs here\, but have any of them happened?

I mean basically you are saying I should be frustrated about something so that you don't get frustrated while using an arguably broken workflow. Why does your preference about something that hasn't happened trump my preference (AFAIK shared with others)?

Luckily there is an option where neither of us have to be frustrated. â˜ș

and make clean is not intended to undo such changes.

That make might correct an innocuous error you made in a source file while making changes seems quite reasonable to me.

Anyway my view is that if we routinely test for something that can be fixed with a make command then make should just do that command automatically

No\, instead of doing that command automatically\, have a ‘make porting’ target that gets the tests passing\, so that the hacker has more control over when things get run. For non-critical things like manifest sorting\, we can remove the tests and have ‘make porting’ as a standard thing to run when making a release.

(If people want to run ‘make porting’ from time to time themselves\, they can.)

I am fully supportive of this option\, and for more than just manifest.t

and save a lot of fuss. (all things being equal etc)

I hope my suggestions will do the same\, but without causing problems.

I do too.

Yves

p5pRT commented 8 years ago

From @demerphq

On 21 Sep 2016 09​:53\, "demerphq" \demerphq@&#8203;gmail\.com wrote​:

On 20 Sep 2016 12​:51\, "Father Chrysostomos via RT" \< perlbug-followup@​perl.org> wrote​:

On Tue Sep 20 07​:26​:04 2016\, demerphq wrote​:

On 20 Sep 2016 3​:43 a.m.\, "Father Chrysostomos via RT" \< perlbug-followup@​perl.org> wrote​:

On Tue Sep 20 00​:40​:13 2016\, sprout wrote​:

On Tue Sep 20 00​:36​:46 2016\, sprout wrote​:

What I want to know is why the build is trying to sort MANIFEST at all. Ideally\, ‘make’ should *never* touch checked-in files\, because then ‘make distclean’ doesn’t work. You can’t get a clean slate unless you are using a git repository.

Does the attached diff also fix the problem? (Instead of working around the problem\, it removes the cause.)

Digging a little further\, I see this commit​:

commit 19bf1007743b4337230ad3a4538df4bd94311fc4 Author​: Yves Orton \demerphq@&#8203;gmail\.com Date​: Thu Dec 25 15​:44​:14 2014 +0100

automatically sort the MANIFEST if necessary

Instead of harrasing people to sort the manifest in our
tests\, we can just automatically sort the manifest when it
changes\.

That way the tests are actually testing that the auto\-sort
worked\, and not that our devs put the new file in the right
place\.

The problem with automatically sorting the MANIFEST is what I mentioned above​: ‘make’ should not make changes to files tracked by git. I think that commit was ill-advised.

I think we have to differ on that one. The only way a patch ends up upstream in an incorrect order is if someone made changes to it and never ran make at all. Thus the scenario you describe would never occur on an unmatched perl.

1. Make changes 2. git commit 3. make test 4. All tests pass\, so push.

I follow that routine routinely. If I get ‘Can’t rebase because you have unstaged changes’ when I *know* I have committed\, I am going to get very frustrated. :-)

Not as frustrated as I get when I make a change and it fails test because of failing manifest.t

I mean has it happened even once yet?

Also personally I think committing before you run make test is a bit optimistic. I mean it's like that line about the doctor... So don't do that... â˜ș

And I don't agree about your make clean argument either in your other post . Make clean does not undo changes to source code. The only way you end up in the scenario you describe as undesirable is when you have made changes to files in the first place

But what if I haven’t made any changes\, but simply tried to build perl?

Lots of what ifs here\, but have any of them happened?

I mean basically you are saying I should be frustrated about something so that you don't get frustrated while using an arguably broken workflow. Why does your preference about something that hasn't happened trump my preference (AFAIK shared with others)?

Luckily there is an option where neither of us have to be frustrated. â˜ș

and make clean is not intended to undo such changes.

That make might correct an innocuous error you made in a source file while making changes seems quite reasonable to me.

Anyway my view is that if we routinely test for something that can be fixed with a make command then make should just do that command automatically

No\, instead of doing that command automatically\, have a ‘make porting’ target that gets the tests passing\, so that the hacker has more control over when things get run. For non-critical things like manifest sorting\, we can remove the tests and have ‘make porting’ as a standard thing to run when making a release.

(If people want to run ‘make porting’ from time to time themselves\, they can.)

I am fully supportive of this option\, and for more than just manifest.t

and save a lot of fuss. (all things being equal etc)

I hope my suggestions will do the same\, but without causing problems.

I do too.

By the way there is an option available that we did not discuss. We could simply make the manifest sort contingent on being in a git repository with uncommitted changes to MANIFEST.

That would make both of us happy right? In my workflow the test would not annoy and I wouldn't have to worry about changes to MANIFEST\, and in yours tests would fail if you made an unsorted modification of the file.

Yves

p5pRT commented 8 years ago

From @cpansprout

On Wed Sep 21 06​:54​:38 2016\, demerphq wrote​:

Also personally I think committing before you run make test is a bit optimistic. I mean it's like that line about the doctor... So don't do that... â˜ș

I used not to\, but the tests that make sure that files are actually listed in MANIFEST would blithely pass\, and then I would commit and push and get smoke failures. This happened again\, and again\, and again\, so I changed my workflow.

--

Father Chrysostomos

p5pRT commented 8 years ago

From @cpansprout

On Wed Sep 21 07​:00​:45 2016\, demerphq wrote​:

By the way there is an option available that we did not discuss. We could simply make the manifest sort contingent on being in a git repository with uncommitted changes to MANIFEST.

That would make both of us happy right?

Would it not still cause the problem that was the subject of this ticket to begin with? It seems to me that we are using fragile fixes to fix fragile fixes when we could just ‘not do that’ to begin with.

In my workflow the test would not annoy and I wouldn't have to worry about changes to MANIFEST\, and in yours tests would fail if you made an unsorted modification of the file.

--

Father Chrysostomos

p5pRT commented 8 years ago

From @bulk88

On Tue Sep 20 00​:43​:37 2016\, sprout wrote​:

The problem with automatically sorting the MANIFEST is what I mentioned above​: ‘make’ should not make changes to files tracked by git. I think that commit was ill-advised.

I think so too. I never submit a patch to p5p without a "make test_porting" first (1-2 minutes for all the tests\, excluding build time) which covers the manifest sorting. That patch sounds like its CYA for commiters who think "this can't possibly fail tests so why waste my time running any tests or regen.pl?" I'd rather not see the makefile dependency tree made into more of a ball of yarn that limits build speed on high core count machine (unable to max out all cores).

-- bulk88 ~ bulk88 at hotmail.com

p5pRT commented 8 years ago

From @ap

* Dave Mitchell \davem@&#8203;iabyn\.com [2016-09-20 14​:12]​:

How about we just don't bother keeping MANIFEST strictly sorted during development\, (and so don't automatically test it for sortedness)\, and instead have it as an extra manual step in the release process ("step 97​: run 'make manisort' and commit if MANIFEST has changed"). So it just gets done once per month and doesn't get in the way of everyone else.

I had the same thought to begin with.

Then I thought on it a little further\, and ended up disagreeing.

If we want MANIFEST to be sorted at all\, then we should make sure that additions to it go in the right place within the commit that adds them\, and we should avoid commits that reorder the file in bulk\, because that is the only way to minimise spurious merge conflicts on that file. Try to rebase a branch to a new release? Here\, hand-clean the MANIFEST!

But writing that down served as my talk-to-the-teddybear​: it reminded me that you can set up custom merge drivers for particular files in Git. We could write one for MANIFEST which diffs against the base on both sides\, applies all deletions regardless of original context\, de-dupes additions from both sides\, appends them all\, and sorts the result. The only source of conflicts (which it has to check for) would be if both sides happened to add the same path but with different comments (which never happens in practice but must be caught if it does).

But if we’re writing tooling anyway\, it would not be hard to also write some to keep the file sorted in the ordinary course of development. That would further keep the noise in commitdiffs to a minimum.

Then we should move the test for proper sorting to the release process\, so it stops bothering vendors patching in stuff during *their* build.

The alternative to all this is to say we don’t care about the order in MANIFEST at all\, ever. Not during development\, not in release tarballs\, never.

That does however leave open the issue of whether we should still automatically check for correctness (i.e. that MANFEST == git ls-files).

That should stay. There should maybe even be a hook for it.

* Father Chrysostomos via RT \perlbug\-followup@&#8203;perl\.org [2016-09-21 18​:36]​:

On Wed Sep 21 07​:00​:45 2016\, demerphq wrote​:

By the way there is an option available that we did not discuss. We could simply make the manifest sort contingent on being in a git repository with uncommitted changes to MANIFEST.

That would make both of us happy right?

Would it not still cause the problem that was the subject of this ticket to begin with? It seems to me that we are using fragile fixes to fix fragile fixes when we could just ‘not do that’ to begin with.

It seems not a fragile fix to me but exactly the right idea. Though it could be a pre-commit hook – any time MANIFEST is dirty\, just sort it before committing it.

That would not address people who run tests prior to committing\, but if the automation takes care of this well enough that we feel free to move the check for sortedness to the release process\, they will no longer be nagged in the first place.

* Desmond Daignault via RT \perlbug\-followup@&#8203;perl\.org [2016-09-21 01​:48]​:

Can the MANIFEST file be sorted via a git hook?

:-)

Regards\, -- Aristotle Pagaltzis // \<http​://plasmasturm.org/>

p5pRT commented 8 years ago

From @kentfredric

On 23 September 2016 at 18​:18\, Aristotle Pagaltzis \pagaltzis@&#8203;gmx\.de wrote​:

That would not address people who run tests prior to committing\, but if the automation takes care of this well enough that we feel free to move the check for sortedness to the release process\, they will no longer be nagged in the first place.

* Desmond Daignault via RT \perlbug\-followup@&#8203;perl\.org [2016-09-21 01​:48]​:

Can the MANIFEST file be sorted via a git hook?

There's another option .... server side push hooks that assert manifest is sorted to accept the push.

This at least means people who don't have the magic locally can't push commits that would be known to be broken.

Ideally there should also be a way to make it force\, in the event somebody ships a perl with an unsorted ... nah\, if that happens\, then we've already got oh so much bigger problems.

-- Kent

KENTNL - https://metacpan.org/author/KENTNL

p5pRT commented 8 years ago

From @tonycoz

On Thu Sep 08 09​:05​:09 2016\, tomhukins wrote​:

Currently\, I see parallel builds fail every time on FreeBSD. The attached patch might not be the most elegant solution but it fixes the problem as described in the commit message.

In general this type of problem is easy to reproduce once you identify the target with missing dependencies - just build the target missing its deps​:

$ ./Configure -des -Dusedevel && make -j6 MANIFEST.srt ... Can't locate Cwd.pm in @​INC (you may need to install the Cwd module) (@​INC contains​: lib ... ... $

Whether or not we should have a MANIFEST file\, or if we have the file whether it should be sorted\, the patch supplied depends on the dynamic Cwd$(DLSUFFIX) even when Cwd is being built statically\, as with a -Uusedl or -Dstatic_ext=Cwd build.

Something like the attached fixes that.

Tony

p5pRT commented 8 years ago

From @tonycoz

0001-perl-129229-fix-dependency-for-MANIFEST_SRT.patch ```diff From b635474a1457d7bc466fed8d0223210a6d873686 Mon Sep 17 00:00:00 2001 From: Tony Cook Date: Wed, 12 Oct 2016 09:56:43 +1100 Subject: (perl #129229) fix dependency for $(MANIFEST_SRT) Correctly handles -Uusedl and -Dstatic_ext=Cwd builds --- Makefile.SH | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/Makefile.SH b/Makefile.SH index 511d6e3..487231d 100755 --- a/Makefile.SH +++ b/Makefile.SH @@ -461,9 +461,20 @@ SH_to_target() { SH='Makefile.SH cflags.SH config_h.SH makedepend.SH myconfig.SH runtests.SH pod/Makefile.SH' shextract=`SH_to_target $SH` +case "$usedl$static_cwd" in +defineundef) + manifest_deps="lib/auto/Cwd/Cwd\$(DLSUFFIX)" + ;; +*) + manifest_deps= + ;; +esac + ## In the following dollars and backticks do not need the extra backslash. $spitshell >>$Makefile <
p5pRT commented 8 years ago

From @arc

Tony Cook via RT \perlbug\-followup@&#8203;perl\.org wrote​:

In general this type of problem is easy to reproduce once you identify the target with missing dependencies - just build the target missing its deps​:

$ ./Configure -des -Dusedevel && make -j6 MANIFEST.srt ... Can't locate Cwd.pm in @​INC (you may need to install the Cwd module) (@​INC contains​: lib ... ...

Thanks\, that's a very useful tip.

Whether or not we should have a MANIFEST file\, or if we have the file whether it should be sorted\, the patch supplied depends on the dynamic Cwd$(DLSUFFIX) even when Cwd is being built statically\, as with a -Uusedl or -Dstatic_ext=Cwd build.

Something like the attached fixes that.

diff --git a/Makefile.SH b/Makefile.SH index 511d6e3..487231d 100755 --- a/Makefile.SH +++ b/Makefile.SH @​@​ -461\,9 +461\,20 @​@​ SH_to_target() { SH='Makefile.SH cflags.SH config_h.SH makedepend.SH myconfig.SH runtests.SH pod/Makefile.SH' shextract=`SH_to_target $SH`

+case "$usedl$static_cwd" in +defineundef) + manifest_deps="lib/auto/Cwd/Cwd\$(DLSUFFIX)" + ;; +*) + manifest_deps= + ;; +esac

I started wondering why Porting/manisort needs Cwd at all. It turns out that the sort_manifest() routine that does the heavy lifting is defined in Porting/pod_lib.pl\, which imports File​::Find (for the benefit of pods_to_install()\, used by installman and pod/buildtoc); and it's File​::Find that actually needs Cwd.

So an alternative would be to restructure Porting/pod_lib.pl​: either by breaking sort_manifest() out into a separate library\, or by changing compile-time "use File​::Find" into a run-time "require File​::Find" solely in pods_to_install(). I'm inclined to think either of those would be a little better\, from the point of view of reducing the number of cross-dependencies (especially hidden ones) between various parts of the build system. The latter option would be a smaller patch\, but I think putting it in its own library is actually the clearest option.

See attached.

-- Aaron Crane ** http​://aaroncrane.co.uk/

p5pRT commented 8 years ago

From @arc

0001-RT-129229-move-sort_manifest-into-its-own-library.patch ```diff From 0cd563745f7dff93ad04af9f9117623474a42a4e Mon Sep 17 00:00:00 2001 From: Aaron Crane Date: Wed, 12 Oct 2016 10:34:13 +0100 Subject: [PATCH] RT#129229: move sort_manifest() into its own library This means that the MANIFEST.srt target in the Makefile no longer needs to load a library that depends on Cwd (and other potentially-dynamic modules). That in turn fixes a missing-dependency bug in the Makefile. --- MANIFEST | 1 + Porting/README.pod | 4 ++++ Porting/manifest_lib.pl | 53 +++++++++++++++++++++++++++++++++++++++++++++++++ Porting/manisort | 2 +- Porting/pod_lib.pl | 29 --------------------------- Porting/pod_rules.pl | 1 + 6 files changed, 60 insertions(+), 30 deletions(-) create mode 100644 Porting/manifest_lib.pl diff --git a/MANIFEST b/MANIFEST index 065ca84..f56a336 100644 --- a/MANIFEST +++ b/MANIFEST @@ -4998,6 +4998,7 @@ Porting/make_snapshot.pl Make a tgz snapshot of our tree with a .patch file in i Porting/makemeta Create the top-level META.yml Porting/makerel Release making utility Porting/manicheck Check against MANIFEST +Porting/manifest_lib.pl Library for checking and sorting the MANIFEST Porting/manisort Sort the MANIFEST Porting/new-perldelta.pl Generate a new perldelta Porting/newtests-perldelta.pl Generate Perldelta stub for newly added tests diff --git a/Porting/README.pod b/Porting/README.pod index 21a0414..c00096e 100644 --- a/Porting/README.pod +++ b/Porting/README.pod @@ -245,6 +245,10 @@ web page to use to generate the snapshot files. This script outputs a list of files in F which don't exist and a list of files that exist and aren't in F. +=head2 F + +This library provides functions used in checking and sorting the F. + =head2 F This script sorts the files in F. diff --git a/Porting/manifest_lib.pl b/Porting/manifest_lib.pl new file mode 100644 index 0000000..0b63046 --- /dev/null +++ b/Porting/manifest_lib.pl @@ -0,0 +1,53 @@ +#!/usr/bin/perl + +use strict; + +=head1 NAME + +Porting/manifest_lib.pl - functions for managing manifests + +=head1 SYNOPSIS + + require './Porting/manifest_lib.pl'; + +=head1 DESCRIPTION + +=head2 C + +Treats its arguments as (chomped) lines from a MANIFEST file, and returns that +listed sorted appropriately. + +=cut + +# Try to get a sane sort. case insensitive, more or less +# sorted such that path components are compared independently, +# and so that lib/Foo/Bar sorts before lib/Foo-Alpha/Baz +# and so that lib/Foo/Bar.pm sorts before lib/Foo/Bar/Alpha.pm +# and so that configure and Configure sort together. +sub sort_manifest { + return + # case insensitive sorting of directory components independently. + map { $_->[0] } # extract the full line + sort { + $a->[1] cmp $b->[1] || # sort in order of munged filename + $a->[0] cmp $b->[0] # then by the exact text in full line + } + map { + # split out the filename and the description + my ($f) = split /\s+/, $_, 2; + # lc the filename so Configure and configure sort together in the list + my $m= lc $f; # $m for munged + # replace slashes by nulls, this makes short directory names sort before + # longer ones, such as "foo/" sorting before "foo-bar/" + $m =~ s!/!\0!g; + # replace the extension (only one) by null null extension. + # this puts any foo/blah.ext before any files in foo/blah/ + $m =~ s!(\.[^.]+\z)!\0\0$1!; + # return the original string, and the munged filename + [ $_, $m ]; + } @_; +} + +1; + +# ex: set ts=8 sts=4 sw=4 et: diff --git a/Porting/manisort b/Porting/manisort index 72cbb9c..3d698e2 100644 --- a/Porting/manisort +++ b/Porting/manisort @@ -14,7 +14,7 @@ $| = 1; # Get command line options use Getopt::Long; -require "Porting/pod_lib.pl"; +require "Porting/manifest_lib.pl"; my $outfile; my $check_only = 0; my $quiet = 0; diff --git a/Porting/pod_lib.pl b/Porting/pod_lib.pl index 4ad204c..6eaacde 100644 --- a/Porting/pod_lib.pl +++ b/Porting/pod_lib.pl @@ -663,35 +663,6 @@ sub get_pod_metadata { return \%state; } -# Try to get a sane sort. case insensitive, more or less -# sorted such that path components are compared independently, -# and so that lib/Foo/Bar sorts before lib/Foo-Alpha/Baz -# and so that lib/Foo/Bar.pm sorts before lib/Foo/Bar/Alpha.pm -# and so that configure and Configure sort together. -sub sort_manifest { - return - # case insensitive sorting of directory components independently. - map { $_->[0] } # extract the full line - sort { - $a->[1] cmp $b->[1] || # sort in order of munged filename - $a->[0] cmp $b->[0] # then by the exact text in full line - } - map { - # split out the filename and the description - my ($f) = split /\s+/, $_, 2; - # lc the filename so Configure and configure sort together in the list - my $m= lc $f; # $m for munged - # replace slashes by nulls, this makes short directory names sort before - # longer ones, such as "foo/" sorting before "foo-bar/" - $m =~ s!/!\0!g; - # replace the extension (only one) by null null extension. - # this puts any foo/blah.ext before any files in foo/blah/ - $m =~ s!(\.[^.]+\z)!\0\0$1!; - # return the original string, and the munged filename - [ $_, $m ]; - } @_; -} - 1; # ex: set ts=8 sts=4 sw=4 et: diff --git a/Porting/pod_rules.pl b/Porting/pod_rules.pl index af5550e..37cbb9c 100644 --- a/Porting/pod_rules.pl +++ b/Porting/pod_rules.pl @@ -33,6 +33,7 @@ if (ord("A") == 193) { ); require 'Porting/pod_lib.pl'; +require 'Porting/manifest_lib.pl'; sub my_die; # process command-line switches -- 2.7.4 ```
p5pRT commented 8 years ago

From @jkeenan

On Wed Oct 12 03​:04​:01 2016\, arc wrote​:

Tony Cook via RT \perlbug\-followup@&#8203;perl\.org wrote​:

In general this type of problem is easy to reproduce once you identify the target with missing dependencies - just build the target missing its deps​:

$ ./Configure -des -Dusedevel && make -j6 MANIFEST.srt ... Can't locate Cwd.pm in @​INC (you may need to install the Cwd module) (@​INC contains​: lib ... ...

Thanks\, that's a very useful tip.

Whether or not we should have a MANIFEST file\, or if we have the file whether it should be sorted\, the patch supplied depends on the dynamic Cwd$(DLSUFFIX) even when Cwd is being built statically\, as with a -Uusedl or -Dstatic_ext=Cwd build.

Something like the attached fixes that.

diff --git a/Makefile.SH b/Makefile.SH index 511d6e3..487231d 100755 --- a/Makefile.SH +++ b/Makefile.SH @​@​ -461\,9 +461\,20 @​@​ SH_to_target() { SH='Makefile.SH cflags.SH config_h.SH makedepend.SH myconfig.SH runtests.SH pod/Makefile.SH' shextract=`SH_to_target $SH`

+case "$usedl$static_cwd" in +defineundef) + manifest_deps="lib/auto/Cwd/Cwd\$(DLSUFFIX)" + ;; +*) + manifest_deps= + ;; +esac

I started wondering why Porting/manisort needs Cwd at all. It turns out that the sort_manifest() routine that does the heavy lifting is defined in Porting/pod_lib.pl\, which imports File​::Find (for the benefit of pods_to_install()\, used by installman and pod/buildtoc); and it's File​::Find that actually needs Cwd.

So an alternative would be to restructure Porting/pod_lib.pl​: either by breaking sort_manifest() out into a separate library\, or by changing compile-time "use File​::Find" into a run-time "require File​::Find" solely in pods_to_install(). I'm inclined to think either of those would be a little better\, from the point of view of reducing the number of cross-dependencies (especially hidden ones) between various parts of the build system. The latter option would be a smaller patch\, but I think putting it in its own library is actually the clearest option.

See attached.

In order to move the discussion forward\, I've created a smoke-testing branch for Aaron Crane's latest patch​:

smoke-me/jkeenan/arc/129229-manifest-lib

Since I've gotten the 'make' failure quite a few times on my FreeBSD VMs\, I'm motivated to get this straightened out. I'm attaching an excerpt from a build failure today on FreeBSD-11.0.

Thank you very much.

-- James E Keenan (jkeenan@​cpan.org)

p5pRT commented 8 years ago

From @jkeenan

cc -o perl -pthread -Wl\,-E -fstack-protector-strong -L/usr/local/lib perlmain.o libperl.a `cat ext.libs` -lpthread -lm -lcrypt -lutil --- MANIFEST.srt --- --- extras.make --- --- MANIFEST.srt --- Can't locate Cwd.pm in @​INC (you may need to install the Cwd module) (@​INC contains​: lib /usr/local/lib/perl5/site_perl/5.25.7/amd64-freebsd-thread-multi /usr/local/lib/perl5/site_perl/5.25.7 /usr/local/lib/perl5/5.25.7/amd64-freebsd-thread-multi /usr/local/lib/perl5/5.25.7 .) at lib/File/Find.pm line 8. Compilation failed in require at Porting/pod_lib.pl line 4. BEGIN failed--compilation aborted at Porting/pod_lib.pl line 4. Compilation failed in require at Porting/manisort line 17. WARNING​: re-sorting MANIFEST Can't locate Cwd.pm in @​INC (you may need to install the Cwd module) (@​INC contains​: lib /usr/local/lib/perl5/site_perl/5.25.7/amd64-freebsd-thread-multi /usr/local/lib/perl5/site_perl/5.25.7 /usr/local/lib/perl5/5.25.7/amd64-freebsd-thread-multi /usr/local/lib/perl5/5.25.7 .) at lib/File/Find.pm line 8. Compilation failed in require at Porting/pod_lib.pl line 4. BEGIN failed--compilation aborted at Porting/pod_lib.pl line 4. Compilation failed in require at Porting/manisort line 17. *** [MANIFEST.srt] Error code 2

make​: stopped in /usr/home/jkeenan/gitwork/perl --- uni.data --- Processing extracted/DCombiningClass.txt Processing extracted/DNumType.txt Processing extracted/DEastAsianWidth.txt --- makeppport --- including ppphdoc including ppphbin including version including threads including limits including uv including memory including misc including variables including mPUSH including call including newRV including newCONSTSUB including MY_CXT including format including SvREFCNT including newSV_type including newSVpv including SvPV including Sv_set including sv_xpvf including shared_pv including HvNAME including gv including warn including pvs including magic including cop including grok including snprintf including sprintf including exception including strlfuncs including pv_tools running "/usr/home/jkeenan/gitwork/perl/miniperl" -I../../lib ppport_h.PL installing ppport.h for cpan/DB_File installing ppport.h for cpan/IPC-SysV installing ppport.h for cpan/Scalar-List-Utils installing ppport.h for cpan/Win32API-File installing ppport.h for dist/PathTools installing ppport.h for dist/Time-HiRes removing temporary file PPPort.pm removing temporary file ppport.h --- uni.data --- Processing extracted/DLineBreak.txt Processing extracted/DBidiClass.txt Processing extracted/DDecompositionType.txt Processing extracted/DBinaryProperties.txt Processing extracted/DNumValues.txt Processing extracted/DJoinGroup.txt Processing extracted/DJoinType.txt Processing Jamo.txt Processing UnicodeData.txt Processing ArabicShaping.txt Processing Blocks.txt Processing PropList.txt Processing SpecialCasing.txt Processing LineBreak.txt Processing EastAsianWidth.txt Processing CompositionExclusions.txt Processing BidiMirroring.txt Processing CaseFolding.txt Processing DCoreProperties.txt Processing Scripts.txt Processing DNormalizationProps.txt Processing DAge.txt Processing HangulSyllableType.txt Processing auxiliary/WordBreakProperty.txt Processing auxiliary/GraphemeBreakProperty.txt Processing auxiliary/GCBTest.txt Processing auxiliary/SBTest.txt Processing auxiliary/WBTest.txt Processing auxiliary/SentenceBreakProperty.txt Processing NamedSequences.txt Processing NameAliases.txt Processing auxiliary/LBTest.txt Processing ScriptExtensions.txt Processing IndicSyllabicCategory.txt Processing BidiBrackets.txt Processing IndicPositionalCategory.txt Finishing processing Unicode properties Compiling Perl properties Creating Perl synonyms Writing tables Making pod file Making test script Updating 'mktables.lst' 1 error

make​: stopped in /usr/home/jkeenan/gitwork/perl [perl] $ make -j${TEST_JOBS} test_harness

p5pRT commented 8 years ago

From @tonycoz

On Wed Oct 12 03​:04​:01 2016\, arc wrote​:

So an alternative would be to restructure Porting/pod_lib.pl​: either by breaking sort_manifest() out into a separate library\, or by changing compile-time "use File​::Find" into a run-time "require File​::Find" solely in pods_to_install(). I'm inclined to think either of those would be a little better\, from the point of view of reducing the number of cross-dependencies (especially hidden ones) between various parts of the build system. The latter option would be a smaller patch\, but I think putting it in its own library is actually the clearest option.

Inline Patch ```diff diff --git a/Porting/manifest_lib.pl b/Porting/manifest_lib.pl new file mode 100644 index 0000000..0b63046 --- /dev/null +++ b/Porting/manifest_lib.pl @@ -0,0 +1,53 @@ +#!/usr/bin/perl + ```

Why does this file have a #! line?

(I don't know why Nicholas added one to pod_lib.pl when he created it either.)

Tony

p5pRT commented 8 years ago

From @jkeenan

On Sat Oct 22 17​:32​:17 2016\, jkeenan wrote​:

In order to move the discussion forward\, I've created a smoke-testing branch for Aaron Crane's latest patch​:

smoke-me/jkeenan/arc/129229-manifest-lib

Since I've gotten the 'make' failure quite a few times on my FreeBSD VMs\, I'm motivated to get this straightened out. I'm attaching an excerpt from a build failure today on FreeBSD-11.0.

I'm attaching a different instance of what I suspect is the same kind of build failure\, again from FreeBSD (though I don't recall whether this was 10.3 or 11.0).

Thank you very much.

-- James E Keenan (jkeenan@​cpan.org)

p5pRT commented 8 years ago

From @jkeenan

lib/unicore/mktables​: Files seem to be ok\, not bothering to rebuild. Add '-w' option to force build --- DynaLoader.o --- --- manifypods --- --- all --- --- MANIFEST.srt --- --- extras.make --- --- MANIFEST.srt --- Can't locate Cwd.pm in @​INC (you may need to install the Cwd module) (@​INC contains​: lib /usr/local/lib/perl5/site_perl/5.25.7/amd64-freebsd-thread-multi /usr/local/lib/perl5/site_perl/5.25.7 /usr/local/lib/perl5/5.25.7/amd64-freebsd-thread-multi /usr/local/lib/perl5/5.25.7 .) at lib/File/Find.pm line 8. Compilation failed in require at Porting/pod_lib.pl line 4. BEGIN failed--compilation aborted at Porting/pod_lib.pl line 4. Compilation failed in require at Porting/manisort line 17. WARNING​: re-sorting MANIFEST --- makeppport --- including ppphdoc including ppphbin including version including threads including limits including uv including memory including misc including variables including mPUSH including call including newRV including newCONSTSUB including MY_CXT including format including SvREFCNT including newSV_type including newSVpv including SvPV including Sv_set including sv_xpvf including shared_pv including HvNAME including gv including warn including pvs including magic including cop including grok including snprintf including sprintf including exception including strlfuncs including pv_tools --- MANIFEST.srt --- Can't locate Cwd.pm in @​INC (you may need to install the Cwd module) (@​INC contains​: lib /usr/local/lib/perl5/site_perl/5.25.7/amd64-freebsd-thread-multi /usr/local/lib/perl5/site_perl/5.25.7 /usr/local/lib/perl5/5.25.7/amd64-freebsd-thread-multi /usr/local/lib/perl5/5.25.7 .) at lib/File/Find.pm line 8. Compilation failed in require at Porting/pod_lib.pl line 4. BEGIN failed--compilation aborted at Porting/pod_lib.pl line 4. Compilation failed in require at Porting/manisort line 17. *** [MANIFEST.srt] Error code 2

make​: stopped in /usr/home/jkeenan/gitwork/perl --- makeppport --- running "/usr/home/jkeenan/gitwork/perl/miniperl" -I../../lib ppport_h.PL ppport.h in cpan/DB_File is up-to-date ppport.h in cpan/IPC-SysV is up-to-date ppport.h in cpan/Scalar-List-Utils is up-to-date ppport.h in cpan/Win32API-File is up-to-date ppport.h in dist/PathTools is up-to-date ppport.h in dist/Time-HiRes is up-to-date removing temporary file PPPort.pm removing temporary file ppport.h 1 error

make​: stopped in /usr/home/jkeenan/gitwork/perl

p5pRT commented 8 years ago

From @tomhukins

On Mon\, Oct 24\, 2016 at 08​:24​:54AM -0700\, James E Keenan via RT wrote​:

I'm attaching a different instance of what I suspect is the same kind of build failure

Yes\, this looks like the same problem to me.

--- MANIFEST.srt --- Can't locate Cwd.pm in @​INC (you may need to install the Cwd module) (@​INC contains​: lib /usr/local/lib/perl5/site_perl/5.25.7/amd64-freebsd-thread-multi /usr/local/lib/perl5/site_perl/5.25.7 /usr/local/lib/perl5/5.25.7/amd64-freebsd-thread-multi /usr/local/lib/perl5/5.25.7 .) at lib/File/Find.pm line 8. Compilation failed in require at Porting/pod_lib.pl line 4. BEGIN failed--compilation aborted at Porting/pod_lib.pl line 4. Compilation failed in require at Porting/manisort line 17. WARNING​: re-sorting MANIFEST

The MANIFEST.srt make target fails to find Cwd.pm. The original patch I sent for this ticket made the MANIFEST.srt target depend on the presence of this file.

Tom

p5pRT commented 8 years ago

From @demerphq

On 24 October 2016 at 17​:35\, Tom Hukins \tom@&#8203;eborcom\.com wrote​:

On Mon\, Oct 24\, 2016 at 08​:24​:54AM -0700\, James E Keenan via RT wrote​:

I'm attaching a different instance of what I suspect is the same kind of build failure

Yes\, this looks like the same problem to me.

--- MANIFEST.srt --- Can't locate Cwd.pm in @​INC (you may need to install the Cwd module) (@​INC contains​: lib /usr/local/lib/perl5/site_perl/5.25.7/amd64-freebsd-thread-multi /usr/local/lib/perl5/site_perl/5.25.7 /usr/local/lib/perl5/5.25.7/amd64-freebsd-thread-multi /usr/local/lib/perl5/5.25.7 .) at lib/File/Find.pm line 8. Compilation failed in require at Porting/pod_lib.pl line 4. BEGIN failed--compilation aborted at Porting/pod_lib.pl line 4. Compilation failed in require at Porting/manisort line 17. WARNING​: re-sorting MANIFEST

The MANIFEST.srt make target fails to find Cwd.pm. The original patch I sent for this ticket made the MANIFEST.srt target depend on the presence of this file.

I will merge James' patch in a few minutes if it passes test. Which should fix this. We need introduce no dependencies with this feature.

The arguments from FC are a bit different and should be dealt with in a different ticket IMO.

Yves

-- perl -Mre=debug -e "/just|another|perl|hacker/"

p5pRT commented 8 years ago

From @demerphq

On 24 October 2016 at 17​:42\, demerphq \demerphq@&#8203;gmail\.com wrote​:

On 24 October 2016 at 17​:35\, Tom Hukins \tom@&#8203;eborcom\.com wrote​:

On Mon\, Oct 24\, 2016 at 08​:24​:54AM -0700\, James E Keenan via RT wrote​:

I'm attaching a different instance of what I suspect is the same kind of build failure

Yes\, this looks like the same problem to me.

--- MANIFEST.srt --- Can't locate Cwd.pm in @​INC (you may need to install the Cwd module) (@​INC contains​: lib /usr/local/lib/perl5/site_perl/5.25.7/amd64-freebsd-thread-multi /usr/local/lib/perl5/site_perl/5.25.7 /usr/local/lib/perl5/5.25.7/amd64-freebsd-thread-multi /usr/local/lib/perl5/5.25.7 .) at lib/File/Find.pm line 8. Compilation failed in require at Porting/pod_lib.pl line 4. BEGIN failed--compilation aborted at Porting/pod_lib.pl line 4. Compilation failed in require at Porting/manisort line 17. WARNING​: re-sorting MANIFEST

The MANIFEST.srt make target fails to find Cwd.pm. The original patch I sent for this ticket made the MANIFEST.srt target depend on the presence of this file.

I will merge James' patch in a few minutes if it passes test. Which should fix this. We need introduce no dependencies with this feature.

So I have done this.

Yves

-- perl -Mre=debug -e "/just|another|perl|hacker/"

p5pRT commented 8 years ago

From @demerphq

On 24 October 2016 at 17​:57\, demerphq \demerphq@&#8203;gmail\.com wrote​:

On 24 October 2016 at 17​:42\, demerphq \demerphq@&#8203;gmail\.com wrote​:

On 24 October 2016 at 17​:35\, Tom Hukins \tom@&#8203;eborcom\.com wrote​:

On Mon\, Oct 24\, 2016 at 08​:24​:54AM -0700\, James E Keenan via RT wrote​:

I'm attaching a different instance of what I suspect is the same kind of build failure

Yes\, this looks like the same problem to me.

--- MANIFEST.srt --- Can't locate Cwd.pm in @​INC (you may need to install the Cwd module) (@​INC contains​: lib /usr/local/lib/perl5/site_perl/5.25.7/amd64-freebsd-thread-multi /usr/local/lib/perl5/site_perl/5.25.7 /usr/local/lib/perl5/5.25.7/amd64-freebsd-thread-multi /usr/local/lib/perl5/5.25.7 .) at lib/File/Find.pm line 8. Compilation failed in require at Porting/pod_lib.pl line 4. BEGIN failed--compilation aborted at Porting/pod_lib.pl line 4. Compilation failed in require at Porting/manisort line 17. WARNING​: re-sorting MANIFEST

The MANIFEST.srt make target fails to find Cwd.pm. The original patch I sent for this ticket made the MANIFEST.srt target depend on the presence of this file.

I will merge James' patch in a few minutes if it passes test. Which should fix this. We need introduce no dependencies with this feature.

So I have done this.

To dispel some confusion​: I merged the following​:

commit 4a59181454f23dbf43f396b924ff7434b63c9d98 Author​: Aaron Crane \arc@&#8203;cpan\.org Date​: Wed Oct 12 10​:34​:13 2016 +0100

  RT#129229​: move sort_manifest() into its own library

  This means that the MANIFEST.srt target in the Makefile no longer needs   to load a library that depends on Cwd (and other potentially-dynamic   modules). That in turn fixes a missing-dependency bug in the Makefile.

-- perl -Mre=debug -e "/just|another|perl|hacker/"

p5pRT commented 8 years ago

From @tomhukins

On Mon\, Oct 24\, 2016 at 05​:42​:01PM +0200\, demerphq wrote​:

The arguments from FC are a bit different and should be dealt with in a different ticket IMO.

The applied patch has fixed the problem I encountered. If the additional concerns raised belong in a new ticket\, please close this one.

Thank you all\, Tom

p5pRT commented 8 years ago

@xsawyerx - Status changed from 'open' to 'resolved'