Re: [PATCH v4] Makefile: replace perl/Makefile.PL with simple make rules

2017-12-19 Thread Todd Zullinger
Hi Ævar,

Ævar Arnfjörð Bjarmason wrote:
> Here's a hopefully final version. The only difference with v3 is:
> 
> -local @_ = ($caller, @_);
> +unshift @_, $caller;
> 
> As it turns out localizing @_ isn't something that worked properly
> until
> https://github.com/Perl/perl5/commit/049bd5ffd62b73325d4b2e75e59ba04b3569137d
> 
> That commit isn't part of the 5.16.3 version that ships with CentOS 7,
> which explains why Michael J Gruber had issues with it. I've tested
> this on CentOS 7 myself, it passes all tests now.

Thanks for tracking this down!

FWIW, I applied this version to next and tested it with
CentOS 6 and 7.  The tests pass on both (though there are
some unrelated failures on CentOS 6 in t5700-protocol-v1,
which I haven't looked into further yet).

I also applied this patch to 2.15.1 and ran the tests in the
Fedora build system for all fedora and epel releases, which
also passed (though with some spurious git-svn failures on
x86_64 in fedora 28, AKA rawhide).

The .pmc extensions seem to cause rpm to fail to parse the
files for rpm 'provides' as it normally would.  This causes
scripts like git-send-email which generates a 'requires' on
Git::Error to fail to find anything which provides it.

I'm not familiar with the .pmc extenstion.  Searching the
fedora repositories, there is only one other package - and
one file within it - which has a .pmc extension.

(The package is perl-test, the file is
/usr/libexec/perl5-tests/perl-tests/t/run/flib/t2.pmc.)

Perhaps it's a bug in rpm's perl dependency generator, but
I'd like to think that git wouldn't be the first package to
find it.

Is the .pmc extension important to ensure these files are
loaded in the right order?  Since they're all in the Git
namespace, I don't imagine there should be anything else in
@INC which would be provided by the system or another
package.

Pardon my ignorance if I've missed the obvious (I haven't
fully read "perldoc -f require" which you referenced in the
commit message).

-- 
Todd
~~
Suppose I were a member of Congress, and suppose I were an idiot. But,
I repeat myself.
-- Mark Twain



[PATCH v4] Makefile: replace perl/Makefile.PL with simple make rules

2017-12-19 Thread Ævar Arnfjörð Bjarmason
Replace the perl/Makefile.PL and the fallback perl/Makefile used under
NO_PERL_MAKEMAKER=NoThanks with a much simpler implementation heavily
inspired by how the i18n infrastructure's build process works[1].

The reason for having the Makefile.PL in the first place is that it
was initially[2] building a perl C binding to interface with libgit,
this functionality, that was removed[3] before Git.pm ever made it to
the master branch.

We've since since started maintaining a fallback perl/Makefile, as
MakeMaker wouldn't work on some platforms[4]. That's just the tip of
the iceberg. We have the PM.stamp hack in the top-level Makefile[5] to
detect whether we need to regenerate the perl/perl.mak, which I fixed
just recently to deal with issues like the perl version changing from
under us[6].

There is absolutely no reason for why this needs to be so complex
anymore. All we're getting out of this elaborate Rube Goldberg machine
was copying perl/* to perl/blib/* as we do a string-replacement on
the *.pm files to hardcode @@LOCALEDIR@@ in the source, as well as
pod2man-ing Git.pm & friends.

So replace the whole thing with something that's pretty much a copy of
how we generate po/build/**.mo from po/*.po, just with a small sed(1)
command instead of msgfmt. As that's being done rename the files
from *.pm to *.pmc just to indicate that they're generated (see
"perldoc -f require").

While I'm at it, change the fallback for Error.pm from being something
where we'll ship our own Error.pm if one doesn't exist at build time
to one where we just use a Git::Error wrapper that'll always prefer
the system-wide Error.pm, only falling back to our own copy if it
really doesn't exist at runtime. It's now shipped as
Git::FromCPAN::Error, making it easy to add other modules to
Git::FromCPAN::* in the future if that's needed.

Functional changes:

 * This will not always install into perl's idea of its global
   "installsitelib". This only potentially matters for packagers that
   need to expose Git.pm for non-git use, and as explained in the
   INSTALL file there's a trivial workaround.

 * The scripts themselves will 'use lib' the target directory, but if
   INSTLIBDIR is set it overrides it. It doesn't have to be this way,
   it could be set in addition to INSTLIBDIR, but my reading of [7] is
   that this is the desired behavior.

 * We don't build man pages for all of the perl modules as we used to,
   only Git(3pm). As discussed on-list[8] that we were building
   installed manpages for purely internal APIs like Git::I18N or
   private-Error.pm was always a bug anyway, and all the Git::SVN::*
   ones say they're internal APIs.

   There are apparently external users of Git.pm, but I don't expect
   there to be any of the others.

   As a side-effect of these general changes the perl documentation
   now only installed by install-{doc,man}, not a mere "install" as
   before.

1. 5e9637c629 ("i18n: add infrastructure for translating Git with
   gettext", 2011-11-18)

2. b1edc53d06 ("Introduce Git.pm (v4)", 2006-06-24)

3. 18b0fc1ce1 ("Git.pm: Kill Git.xs for now", 2006-09-23)

4. f848718a69 ("Make perl/ build procedure ActiveState friendly.",
   2006-12-04)

5. ee9be06770 ("perl: detect new files in MakeMaker builds",
   2012-07-27)

6. c59c4939c2 ("perl: regenerate perl.mak if perl -V changes",
   2017-03-29)

7. 0386dd37b1 ("Makefile: add PERLLIB_EXTRA variable that adds to
   default perl path", 2013-11-15)

8. 87bmjjv1pu@evledraar.booking.com ("Re: [PATCH] Makefile:
   replace perl/Makefile.PL with simple make rules"

Signed-off-by: Ævar Arnfjörð Bjarmason 
---

Here's a hopefully final version. The only difference with v3 is:

-local @_ = ($caller, @_);
+unshift @_, $caller;

As it turns out localizing @_ isn't something that worked properly
until
https://github.com/Perl/perl5/commit/049bd5ffd62b73325d4b2e75e59ba04b3569137d

That commit isn't part of the 5.16.3 version that ships with CentOS 7,
which explains why Michael J Gruber had issues with it. I've tested
this on CentOS 7 myself, it passes all tests now.

 INSTALL  | 17 -
 Makefile | 67 ++
 contrib/examples/git-difftool.perl   |  2 +-
 git-send-email.perl  |  2 +-
 perl/.gitignore  |  9 +--
 perl/Git.pm  |  2 +-
 perl/Git/Error.pm| 46 
 perl/{private-Error.pm => Git/FromCPAN/Error.pm} |  0
 perl/Git/I18N.pm |  2 +-
 perl/Makefile| 90 
 perl/Makefile.PL | 62 
 t/perf/aggregate.perl|  2 +-
 t/test-lib.sh|  2 +-
 wrap-for-bin.sh  |  2 +-
 14 files changed,