Re: [PATCH 4/4] Move initialization of Git::SVN variables into Git::SVN.

2012-07-27 Thread Junio C Hamano
Jonathan Nieder jrnie...@gmail.com writes:

 In short:

  - I didn't see anything questionable in 1/4;

  - Calling up ::opt_prefix() from module in 2/4 looked ugly to me
but I suspect it should be easy to fix;

  - 3/4 was a straight move and I didn't see anything questionable in
it, but I think it would be nicer if intermediate steps can be
made to still work by making 4/4 come first or something
similarly simple.

 If the issues in 2/4 and 3/4 are easily fixable by going the route I
 handwaved above, the result of doing so based on this round is ready
 to be applied, I think.

 Eric, Jonathan, what do you think?

 I think this is pretty good already, though I also like your
 suggestion re 2/4.

 I haven't reviewed the tests these introduce and assume Eric has that
 covered.

I didn't mean to say Unless you prove that the two suggestions are
not easy to implement, I will veto the series until they are fixed.
Especially, I consider that the ordering between 3 and 4 falls into
the it would be nicer if this wart weren't there category.

The result will be queued tentatively near the tip of 'pu', but as
this is primarily about git-svn, I would prefer a copy that is
vetted by Eric to be fed from him.

Thanks.

P.S.

t91XX series seem to fail in 'pu' with Can't locate Git/SVN.pm in
@INC for me.  I see perl/blib/lib/Git/SVN/ directory and files
under it, but there is no perl/blib/lib/Git/SVN.pm installed.  I see
Git/I18N.pm and Git/SVN/Ra.pm (and friends) mentioned in
perl/perl.mak generated by MakeMaker, but Git/SVN.pm does not appear
anywhere.

I think it is some interaction with other topics, as the tip of
ms/git-svn-pm topic that parks this series does not exhibit the
symptom, but it is getting late for me already, so I won't dig into
this further.

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 4/4] Move initialization of Git::SVN variables into Git::SVN.

2012-07-27 Thread Junio C Hamano
Junio C Hamano gits...@pobox.com writes:

 t91XX series seem to fail in 'pu' with Can't locate Git/SVN.pm in
 @INC for me.  I see perl/blib/lib/Git/SVN/ directory and files
 under it, but there is no perl/blib/lib/Git/SVN.pm installed.  I see
 Git/I18N.pm and Git/SVN/Ra.pm (and friends) mentioned in
 perl/perl.mak generated by MakeMaker, but Git/SVN.pm does not appear
 anywhere.

 I think it is some interaction with other topics, as the tip of
 ms/git-svn-pm topic that parks this series does not exhibit the
 symptom, but it is getting late for me already, so I won't dig into
 this further.

Actually there is no difference between ms/git-svn-pm and pu in perl/
directory. I _think_ there is some dependency missing that makes
this sequence break:

(in one repository)
git checkout pu ;# older pu without ms/git-svn-pm
make ; make test

(in another repository that shares the refs)
git checkout pu
git merge ms/git-svn-pm

(in the first repository)
git reset --hard ;# update the working tree
make ; make test



--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 4/4] Move initialization of Git::SVN variables into Git::SVN.

2012-07-27 Thread Junio C Hamano
Junio C Hamano gits...@pobox.com writes:

 Junio C Hamano gits...@pobox.com writes:

 t91XX series seem to fail in 'pu' with Can't locate Git/SVN.pm in
 @INC for me.  I see perl/blib/lib/Git/SVN/ directory and files
 under it, but there is no perl/blib/lib/Git/SVN.pm installed.  I see
 Git/I18N.pm and Git/SVN/Ra.pm (and friends) mentioned in
 perl/perl.mak generated by MakeMaker, but Git/SVN.pm does not appear
 anywhere.
 
 I think it is some interaction with other topics, as the tip of
 ms/git-svn-pm topic that parks this series does not exhibit the
 symptom, but it is getting late for me already, so I won't dig into
 this further.

 Actually there is no difference between ms/git-svn-pm and pu in perl/
 directory. I _think_ there is some dependency missing that makes
 this sequence break:

   (in one repository)
   git checkout pu ;# older pu without ms/git-svn-pm
 make ; make test

   (in another repository that shares the refs)
   git checkout pu
 git merge ms/git-svn-pm

   (in the first repository)
 git reset --hard ;# update the working tree
 make ; make test

What was happening was that originally, pu had ms/makefile-pl but
not ms/git-svn-pm.  Hence, perl/Git/SVN.pm did not exist.  I ran
make and it created perl/perl.mak that does not know about
Git/SVN.pm;

Then ms/git-svn-pm is merged to pu and now we have perl/Git/SVN.pm.
But there is nothing in ms/makefile-pl that says on what files
perl.mak depends on.

I think there needs to be a dependency in to recreate perl/perl.mak
when any of the *.pm files are changed, perhaps like this.

I am not sure why perl/perl.mak is built by the top-level Makefile,
instead of just using $(MAKE) -C perl/, though...

 Makefile  | 7 +++
 perl/Makefile | 1 +
 2 files changed, 8 insertions(+)

diff --git a/Makefile b/Makefile
index b0b3493..e2a4ac7 100644
--- a/Makefile
+++ b/Makefile
@@ -2090,6 +2090,13 @@ $(SCRIPT_LIB) : % : %.sh GIT-SCRIPT-DEFINES
 ifndef NO_PERL
 $(patsubst %.perl,%,$(SCRIPT_PERL)): perl/perl.mak
 
+perl/perl.mak: perl/PM.stamp
+
+perl/PM.stamp: FORCE
+   $(QUIET_GEN)find perl -type f -name '*.pm' | sort $@+  \
+   { cmp $@+ $@ /dev/null 2/dev/null || mv $@+ $@; }  \
+   $(RM) $@+
+
 perl/perl.mak: GIT-CFLAGS GIT-PREFIX perl/Makefile perl/Makefile.PL
$(QUIET_SUBDIR0)perl $(QUIET_SUBDIR1) PERL_PATH='$(PERL_PATH_SQ)' 
prefix='$(prefix_SQ)' $(@F)
 
diff --git a/perl/Makefile b/perl/Makefile
index 6ca7d47..d6f8478 100644
--- a/perl/Makefile
+++ b/perl/Makefile
@@ -20,6 +20,7 @@ clean:
$(RM) ppport.h
$(RM) $(makfile)
$(RM) $(makfile).old
+   $(RM) PM.stamp
 
 ifdef NO_PERL_MAKEMAKER
 instdir_SQ = $(subst ','\'',$(prefix)/lib)
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 4/4] Move initialization of Git::SVN variables into Git::SVN.

2012-07-27 Thread Michael G Schwern
On 2012.7.26 10:18 PM, Junio C Hamano wrote:
 If you swap the order of steps 3/4 and 4/4 by creating Git/SVN.pm
 that only has these variable definitions (i.e. our $X and use
 vars $X) and make git-svn.perl use them from Git::SVN in the first
 step, and then do the bulk-moving (equivalent of your 3/4) in the
 second step, would it free you from having to say it's doubtful it
 will compile by itself?

If it wasn't clear, all tests pass with every patch using SVN 1.6.

Compile on its own wasn't entirely clear.  I meant that Git::SVN doesn't
depend on git-svn to set its defaults.  Git::SVN still depends on it for A LOT
of other things, and will likely remain that way for a long time, so it's
kinda splitting hairs to worry about it.

4/4 was done last to ensure the phase of git-svn when the Git::SVN globals are
initialized remains basically the same.  If they were moved into Git::SVN
before it was split out they'd be getting initialized *after* the git-svn
command has been executed.  I didn't want to expend the energy or risk the
bugs to get around that.


 In short:
 
  - I didn't see anything questionable in 1/4;
 
  - Calling up ::opt_prefix() from module in 2/4 looked ugly to me
but I suspect it should be easy to fix;

Originally I tried to refactor new().  It rapidly turned into a lot of work on
undocumented code with no unit tests for no use to the SVN 1.7 issue for one
variable.  This is a very cheap way to let far more important work move
forward and it has a very narrow effect.  It could be made a Git::SVN global
that git-svn grabs at, but that's not really any better.  I'd rather leave it 
be.


-- 
91. I am not authorized to initiate Jihad.
-- The 213 Things Skippy Is No Longer Allowed To Do In The U.S. Army
   http://skippyslist.com/list/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 4/4] Move initialization of Git::SVN variables into Git::SVN.

2012-07-27 Thread Eric Wong
Junio C Hamano gits...@pobox.com wrote:
 The result will be queued tentatively near the tip of 'pu', but as
 this is primarily about git-svn, I would prefer a copy that is
 vetted by Eric to be fed from him.

OK.  I've signed-off on the 7 patches you have in pu.  Will look at the
other series in a few hours.

The following changes since commit cdd159b2f56c9e69e37bbb8f5af301abd93e5407:

  Merge branch 'jc/test-lib-source-build-options-early' (2012-07-25 15:47:08 
-0700)

are available in the git repository at:

  git://bogomips.org/git-svn master

for you to fetch changes up to 8d1ddbdc877cf1f430ea8e79bf800ce806875565:

  Move initialization of Git::SVN variables into Git::SVN. (2012-07-27 11:29:21 
+)


Michael G. Schwern (7):
  Quiet warning if Makefile.PL is run with -w and no --localedir
  Don't lose Error.pm if $@ gets clobbered.
  The Makefile.PL will now find .pm files itself.
  Extract some utilities from git-svn to allow extracting Git::SVN.
  Prepare Git::SVN for extraction into its own file.
  Extract Git::SVN from git-svn into its own .pm file.
  Move initialization of Git::SVN variables into Git::SVN.

 git-svn.perl   | 2340 +---
 perl/Git/SVN.pm| 2324 +++
 perl/Git/SVN/Utils.pm  |   59 +
 perl/Makefile  |2 +
 perl/Makefile.PL   |   35 +-
 t/Git-SVN/00compile.t  |9 +
 t/Git-SVN/Utils/can_compress.t |   11 +
 t/Git-SVN/Utils/fatal.t|   34 +
 8 files changed, 2476 insertions(+), 2338 deletions(-)
 create mode 100644 perl/Git/SVN.pm
 create mode 100644 perl/Git/SVN/Utils.pm
 create mode 100644 t/Git-SVN/00compile.t
 create mode 100644 t/Git-SVN/Utils/can_compress.t
 create mode 100644 t/Git-SVN/Utils/fatal.t

 Thanks.
 
 P.S.
 
 t91XX series seem to fail in 'pu' with Can't locate Git/SVN.pm in
 @INC for me.  I see perl/blib/lib/Git/SVN/ directory and files
 under it, but there is no perl/blib/lib/Git/SVN.pm installed.  I see
 Git/I18N.pm and Git/SVN/Ra.pm (and friends) mentioned in
 perl/perl.mak generated by MakeMaker, but Git/SVN.pm does not appear
 anywhere.
 
 I think it is some interaction with other topics, as the tip of
 ms/git-svn-pm topic that parks this series does not exhibit the
 symptom, but it is getting late for me already, so I won't dig into
 this further.

I think your proposed patch in the followup should work.  We should
probably squash that into this series avoid breaking bisect in the
future.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 4/4] Move initialization of Git::SVN variables into Git::SVN.

2012-07-27 Thread Eric Wong
Junio C Hamano gits...@pobox.com wrote:
 What was happening was that originally, pu had ms/makefile-pl but
 not ms/git-svn-pm.  Hence, perl/Git/SVN.pm did not exist.  I ran
 make and it created perl/perl.mak that does not know about
 Git/SVN.pm;
 
 Then ms/git-svn-pm is merged to pu and now we have perl/Git/SVN.pm.
 But there is nothing in ms/makefile-pl that says on what files
 perl.mak depends on.
 
 I think there needs to be a dependency in to recreate perl/perl.mak
 when any of the *.pm files are changed, perhaps like this.
 
 I am not sure why perl/perl.mak is built by the top-level Makefile,
 instead of just using $(MAKE) -C perl/, though...

I'm not sure why perl/perl.mak is built by the top-level Makefile,
either.

I'll put the following after ms/makefile-pl but before ms/git-svn-pm:

From a6ea2301d1bb6fd7c7415fed3aa7673542a563bd Mon Sep 17 00:00:00 2001
From: Junio C Hamano gits...@pobox.com
Date: Fri, 27 Jul 2012 20:04:20 +
Subject: [PATCH] perl: detect new files in MakeMaker builds

While Makefile.PL now finds .pm files on its own, it does not
detect new files after it generates perl/perl.mak.

[ew: commit message]

ref: http://mid.gmane.org/7vlii51xz4@alter.siamese.dyndns.org

Signed-off-by: Eric Wong normalper...@yhbt.net
---
 Makefile  | 7 +++
 perl/Makefile | 1 +
 2 files changed, 8 insertions(+)

diff --git a/Makefile b/Makefile
index b0b3493..e2a4ac7 100644
--- a/Makefile
+++ b/Makefile
@@ -2090,6 +2090,13 @@ $(SCRIPT_LIB) : % : %.sh GIT-SCRIPT-DEFINES
 ifndef NO_PERL
 $(patsubst %.perl,%,$(SCRIPT_PERL)): perl/perl.mak
 
+perl/perl.mak: perl/PM.stamp
+
+perl/PM.stamp: FORCE
+   $(QUIET_GEN)find perl -type f -name '*.pm' | sort $@+  \
+   { cmp $@+ $@ /dev/null 2/dev/null || mv $@+ $@; }  \
+   $(RM) $@+
+
 perl/perl.mak: GIT-CFLAGS GIT-PREFIX perl/Makefile perl/Makefile.PL
$(QUIET_SUBDIR0)perl $(QUIET_SUBDIR1) PERL_PATH='$(PERL_PATH_SQ)' 
prefix='$(prefix_SQ)' $(@F)
 
diff --git a/perl/Makefile b/perl/Makefile
index 8493d76..4969ef8 100644
--- a/perl/Makefile
+++ b/perl/Makefile
@@ -20,6 +20,7 @@ clean:
$(RM) ppport.h
$(RM) $(makfile)
$(RM) $(makfile).old
+   $(RM) PM.stamp
 
 ifdef NO_PERL_MAKEMAKER
 instdir_SQ = $(subst ','\'',$(prefix)/lib)
-- 
Eric Wong
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 4/4] Move initialization of Git::SVN variables into Git::SVN.

2012-07-27 Thread Michael G Schwern
On 2012.7.27 1:07 PM, Eric Wong wrote:
 While Makefile.PL now finds .pm files on its own, it does not
 detect new files after it generates perl/perl.mak.

Are you saying this doesn't work?

perl Makefile.PL
make -f perl.mak
touch Git/Foo.pm
perl Makefile.PL
make -f perl.mak

or this?

perl Makefile.PL
make -f perl.mak
touch Git/Foo.pm
make -f perl.mak

The former should work.  The latter is a MakeMaker limitation.  Makefile.PL
hard codes the list of .pm files into the Makefile.


-- 
Who invented the eponym?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 4/4] Move initialization of Git::SVN variables into Git::SVN.

2012-07-27 Thread Eric Wong
Michael G Schwern schw...@pobox.com wrote:
 On 2012.7.27 1:07 PM, Eric Wong wrote:
  While Makefile.PL now finds .pm files on its own, it does not
  detect new files after it generates perl/perl.mak.
 
 Are you saying this doesn't work?
 
 perl Makefile.PL
 make -f perl.mak
 touch Git/Foo.pm
 perl Makefile.PL
 make -f perl.mak

This works.

 or this?
 
 perl Makefile.PL
 make -f perl.mak
 touch Git/Foo.pm
 make -f perl.mak
 
 The former should work.  The latter is a MakeMaker limitation.  Makefile.PL
 hard codes the list of .pm files into the Makefile.

Yup, Junio's patch works around the MM limitation so the latter works.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 4/4] Move initialization of Git::SVN variables into Git::SVN.

2012-07-27 Thread Junio C Hamano
Michael G Schwern schw...@pobox.com writes:

 On 2012.7.27 1:07 PM, Eric Wong wrote:
 While Makefile.PL now finds .pm files on its own, it does not
 detect new files after it generates perl/perl.mak.

 Are you saying this doesn't work?

 perl Makefile.PL
 make -f perl.mak
 touch Git/Foo.pm
 perl Makefile.PL
 make -f perl.mak

 or this?

 perl Makefile.PL
 make -f perl.mak
 touch Git/Foo.pm
 make -f perl.mak

Neither of the above.  Nobody should be typing perl Makefile.PL
inside our source tree unless he is trying to debug our Makefiles
anyway.

What does not work is this sequence:

make
perl/Git/Foo.pm
make

Makefile at the top-level, which builds perl/perl.mak by running
perl Makefile.PL in perl/ subdirectory, doesn't have dependencies
[*1*], so in the above sequence, the second invocation of make
fails to rebuild perl/perl.mak, which causes Git/Foo.pm forgotten
from the build/installation step.

And that is what happened to Git/SVN.pm.


[Footnote]

*1* I also suspect perl/Makefile lacks this dependency even though
it has its own rule to build perl/perl.mak---don't they need to be
cleaned-up and merged???


--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 4/4] Move initialization of Git::SVN variables into Git::SVN.

2012-07-27 Thread Junio C Hamano
Eric Wong normalper...@yhbt.net writes:

 I'll put the following after ms/makefile-pl but before ms/git-svn-pm:

OK, it seems that you haven't pushed out the result yet (which is
fine); how do we want to proceed?

I generally prefer pulling from maintainer trees directly to my
'master' without staging them in 'next', so when the above is done
and you feel the tip of your tree is good for 1.7.12-rc1 without
regression, please throw me a pull, now!.

Thanks.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 4/4] Move initialization of Git::SVN variables into Git::SVN.

2012-07-27 Thread Eric Wong
Junio C Hamano gits...@pobox.com wrote:
 Eric Wong normalper...@yhbt.net writes:
 
  I'll put the following after ms/makefile-pl but before ms/git-svn-pm:
 
 OK, it seems that you haven't pushed out the result yet (which is
 fine); how do we want to proceed?

Oops, got sidetracked into something else.  Before I got sidetracked,
my application of another patch in Michael's 3rd series failed
(even with your updated Makefile patch):

  [PATCH 7/8] Extract Git::SVN::GlobSpec from git-svn

GlobSpec.pm did not get picked up and placed into blib/ and some tests
(t9118-git-svn-funky-branch-names.sh) failed as a result

So I'll hold off until we can fix the build regressions (working on it
now)
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 4/4] Move initialization of Git::SVN variables into Git::SVN.

2012-07-27 Thread Eric Wong
Eric Wong normalper...@yhbt.net wrote:
 Junio C Hamano gits...@pobox.com wrote:
  Eric Wong normalper...@yhbt.net writes:
  
   I'll put the following after ms/makefile-pl but before ms/git-svn-pm:
  
  OK, it seems that you haven't pushed out the result yet (which is
  fine); how do we want to proceed?

 So I'll hold off until we can fix the build regressions (working on it
 now)

OK, all fixed, all I needed was this (squashed in):

--- a/perl/Makefile
+++ b/perl/Makefile
@@ -22,6 +22,8 @@ clean:
$(RM) $(makfile).old
$(RM) PM.stamp
 
+$(makfile): PM.stamp
+
 ifdef NO_PERL_MAKEMAKER
 instdir_SQ = $(subst ','\'',$(prefix)/lib)

-
The redundant dependencies are biting us :  I agree there presence in
the top-level Makefile needs to be reviewed.

Anyways, you can pull this now from my master:

The following changes since commit cdd159b2f56c9e69e37bbb8f5af301abd93e5407:

  Merge branch 'jc/test-lib-source-build-options-early' (2012-07-25 15:47:08 
-0700)

are available in the git repository at:

  git://bogomips.org/git-svn master

for you to fetch changes up to 5c71028fced46d03bf81b8625680d9ac87c8f4f0:

  Move initialization of Git::SVN variables into Git::SVN. (2012-07-27 22:14:54 
+)


Junio C Hamano (1):
  perl: detect new files in MakeMaker builds

Michael G. Schwern (7):
  Quiet warning if Makefile.PL is run with -w and no --localedir
  Don't lose Error.pm if $@ gets clobbered.
  The Makefile.PL will now find .pm files itself.
  Extract some utilities from git-svn to allow extracting Git::SVN.
  Prepare Git::SVN for extraction into its own file.
  Extract Git::SVN from git-svn into its own .pm file.
  Move initialization of Git::SVN variables into Git::SVN.

 Makefile   |7 +
 git-svn.perl   | 2340 +---
 perl/.gitignore|1 +
 perl/Git/SVN.pm| 2324 +++
 perl/Git/SVN/Utils.pm  |   59 +
 perl/Makefile  |5 +
 perl/Makefile.PL   |   35 +-
 t/Git-SVN/00compile.t  |9 +
 t/Git-SVN/Utils/can_compress.t |   11 +
 t/Git-SVN/Utils/fatal.t|   34 +
 10 files changed, 2487 insertions(+), 2338 deletions(-)
 create mode 100644 perl/Git/SVN.pm
 create mode 100644 perl/Git/SVN/Utils.pm
 create mode 100644 t/Git-SVN/00compile.t
 create mode 100644 t/Git-SVN/Utils/can_compress.t
 create mode 100644 t/Git-SVN/Utils/fatal.t

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 4/4] Move initialization of Git::SVN variables into Git::SVN.

2012-07-27 Thread Junio C Hamano
Eric Wong normalper...@yhbt.net writes:

 OK, all fixed, all I needed was this (squashed in):

 --- a/perl/Makefile
 +++ b/perl/Makefile
 @@ -22,6 +22,8 @@ clean:
   $(RM) $(makfile).old
   $(RM) PM.stamp
  
 +$(makfile): PM.stamp
 +
  ifdef NO_PERL_MAKEMAKER
  instdir_SQ = $(subst ','\'',$(prefix)/lib)

 -
 The redundant dependencies are biting us :  I agree there presence in
 the top-level Makefile needs to be reviewed.

Do you feel confident enough that we can leave that question hanging
around and still merge this before 1.7.12 safely?

I do not think it is a regression at the Makefile level per-se---we
didn't have right dependencies to keep perl.mak up to date, which
was the root cause of what we observed.

But the lack of dependencies did not matter before this series
because the list of *.pm files never changed, so in that sense the
series is what introduced the build regression, and I do not have a
solid feeling that we squashed it.

 Anyways, you can pull this now from my master:

 The following changes since commit cdd159b2f56c9e69e37bbb8f5af301abd93e5407:

   Merge branch 'jc/test-lib-source-build-options-early' (2012-07-25 15:47:08 
 -0700)

 are available in the git repository at:

   git://bogomips.org/git-svn master

 for you to fetch changes up to 5c71028fced46d03bf81b8625680d9ac87c8f4f0:

   Move initialization of Git::SVN variables into Git::SVN. (2012-07-27 
 22:14:54 +)

 
 Junio C Hamano (1):
   perl: detect new files in MakeMaker builds

 Michael G. Schwern (7):
   Quiet warning if Makefile.PL is run with -w and no --localedir
   Don't lose Error.pm if $@ gets clobbered.
   The Makefile.PL will now find .pm files itself.
   Extract some utilities from git-svn to allow extracting Git::SVN.
   Prepare Git::SVN for extraction into its own file.
   Extract Git::SVN from git-svn into its own .pm file.
   Move initialization of Git::SVN variables into Git::SVN.

  Makefile   |7 +
  git-svn.perl   | 2340 
 +---
  perl/.gitignore|1 +
  perl/Git/SVN.pm| 2324 +++
  perl/Git/SVN/Utils.pm  |   59 +
  perl/Makefile  |5 +
  perl/Makefile.PL   |   35 +-
  t/Git-SVN/00compile.t  |9 +
  t/Git-SVN/Utils/can_compress.t |   11 +
  t/Git-SVN/Utils/fatal.t|   34 +
  10 files changed, 2487 insertions(+), 2338 deletions(-)
  create mode 100644 perl/Git/SVN.pm
  create mode 100644 perl/Git/SVN/Utils.pm
  create mode 100644 t/Git-SVN/00compile.t
  create mode 100644 t/Git-SVN/Utils/can_compress.t
  create mode 100644 t/Git-SVN/Utils/fatal.t
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 4/4] Move initialization of Git::SVN variables into Git::SVN.

2012-07-27 Thread Eric Wong
Junio C Hamano gits...@pobox.com wrote:
 Eric Wong normalper...@yhbt.net writes:
  The redundant dependencies are biting us :  I agree there presence in
  the top-level Makefile needs to be reviewed.
 
 Do you feel confident enough that we can leave that question hanging
 around and still merge this before 1.7.12 safely?

Yes.

 I do not think it is a regression at the Makefile level per-se---we
 didn't have right dependencies to keep perl.mak up to date, which
 was the root cause of what we observed.
 
 But the lack of dependencies did not matter before this series
 because the list of *.pm files never changed, so in that sense the
 series is what introduced the build regression, and I do not have a
 solid feeling that we squashed it.

Right, I agree the original dependencies are not good and it's not
a recent regression in the Makefile level.

I do feel our patch deals with the problem for now.  I've been going
between commits in Michael's 3rd series and haven't noticed new issues
when running the tests.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 4/4] Move initialization of Git::SVN variables into Git::SVN.

2012-07-27 Thread Junio C Hamano
Eric Wong normalper...@yhbt.net writes:

 Eric Wong normalper...@yhbt.net wrote:
 So I'll hold off until we can fix the build regressions (working on it
 now)

 OK, all fixed, all I needed was this (squashed in):

 --- a/perl/Makefile
 +++ b/perl/Makefile
 @@ -22,6 +22,8 @@ clean:
   $(RM) $(makfile).old
   $(RM) PM.stamp
  
 +$(makfile): PM.stamp
 +
  ifdef NO_PERL_MAKEMAKER
  instdir_SQ = $(subst ','\'',$(prefix)/lib)

Another thing I noticed but didn't say was that the top-level
Makefile seems to think without NO_PERL the way to regenerate
perl/perl.mak is to run perl/Makefile.PL, which is not true if the
build is done with NO_PERL_MAKEMAKER.

I do not offhand know why we even need to have dependency on
perl/perl.mak in the toplevel Makefile (other than otherwise nobody
descends into perl/ and run make in it, which is a bogus
reason---there should be a rule to run $(MAKE) -C perl/ $@ when
doing make all at the top-level if that is the case), but I think
at least the duplicated rule in the toplevel Makefile should read
something like:

perl/perl.mak: ... (the dependencies) ...
$(QUIET_SUBDIR0)perl ... (make variables) ... perl.mak

so that the real knowledge of how to rebuild it (with or without
NO_PERL_MAKEMAKER) should be in perl/Makefile.


--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 4/4] Move initialization of Git::SVN variables into Git::SVN.

2012-07-27 Thread Junio C Hamano
Eric Wong normalper...@yhbt.net writes:

 Junio C Hamano gits...@pobox.com wrote:
 Eric Wong normalper...@yhbt.net writes:
  The redundant dependencies are biting us :  I agree there presence in
  the top-level Makefile needs to be reviewed.
 
 Do you feel confident enough that we can leave that question hanging
 around and still merge this before 1.7.12 safely?

 Yes.

 I do not think it is a regression at the Makefile level per-se---we
 didn't have right dependencies to keep perl.mak up to date, which
 was the root cause of what we observed.
 
 But the lack of dependencies did not matter before this series
 because the list of *.pm files never changed, so in that sense the
 series is what introduced the build regression, and I do not have a
 solid feeling that we squashed it.

 Right, I agree the original dependencies are not good and it's not
 a recent regression in the Makefile level.

 I do feel our patch deals with the problem for now.  I've been going
 between commits in Michael's 3rd series and haven't noticed new issues
 when running the tests.

Ok, please don't forget to add necessary .gitignore rule for the new
stamp file.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 4/4] Move initialization of Git::SVN variables into Git::SVN.

2012-07-27 Thread Eric Wong
Junio C Hamano gits...@pobox.com wrote:
 Ok, please don't forget to add necessary .gitignore rule for the new
 stamp file.

I noticed/remembered that, but I forgot to mention I squashed that in,
too :)
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 4/4] Move initialization of Git::SVN variables into Git::SVN.

2012-07-26 Thread Junio C Hamano
Michael G. Schwern schw...@pobox.com writes:

 From: Michael G. Schwern schw...@pobox.com

 Also it can compile on its own now, yay!

Hmmm.

If you swap the order of steps 3/4 and 4/4 by creating Git/SVN.pm
that only has these variable definitions (i.e. our $X and use
vars $X) and make git-svn.perl use them from Git::SVN in the first
step, and then do the bulk-moving (equivalent of your 3/4) in the
second step, would it free you from having to say it's doubtful it
will compile by itself?

In short:

 - I didn't see anything questionable in 1/4;

 - Calling up ::opt_prefix() from module in 2/4 looked ugly to me
   but I suspect it should be easy to fix;

 - 3/4 was a straight move and I didn't see anything questionable in
   it, but I think it would be nicer if intermediate steps can be
   made to still work by making 4/4 come first or something
   similarly simple.

If the issues in 2/4 and 3/4 are easily fixable by going the route I
handwaved above, the result of doing so based on this round is ready
to be applied, I think.

Eric, Jonathan, what do you think?

 ---
  git-svn.perl  | 4 
  perl/Git/SVN.pm   | 9 +++--
  t/Git-SVN/00compile.t | 3 ++-
  3 files changed, 9 insertions(+), 7 deletions(-)

 diff --git a/git-svn.perl b/git-svn.perl
 index 4c77f69..ef10f6f 100755
 --- a/git-svn.perl
 +++ b/git-svn.perl
 @@ -20,10 +20,7 @@ my $cmd_dir_prefix = eval {
  
  my $git_dir_user_set = 1 if defined $ENV{GIT_DIR};
  $ENV{GIT_DIR} ||= '.git';
 -$Git::SVN::default_repo_id = 'svn';
 -$Git::SVN::default_ref_id = $ENV{GIT_SVN_ID} || 'git-svn';
  $Git::SVN::Ra::_log_window_size = 100;
 -$Git::SVN::_minimize_url = 'unset';
  
  if (! exists $ENV{SVN_SSH}  exists $ENV{GIT_SSH}) {
   $ENV{SVN_SSH} = $ENV{GIT_SSH};
 @@ -114,7 +111,6 @@ my ($_stdin, $_help, $_edit,
  # This is a refactoring artifact so Git::SVN can get at this git-svn switch.
  sub opt_prefix { return $_prefix || '' }
  
 -$Git::SVN::_follow_parent = 1;
  $Git::SVN::Fetcher::_placeholder_filename = .gitignore;
  $_q ||= 0;
  my %remote_opts = ( 'username=s' = \$Git::SVN::Prompt::_username,
 diff --git a/perl/Git/SVN.pm b/perl/Git/SVN.pm
 index c71c041..2e0d7f0 100644
 --- a/perl/Git/SVN.pm
 +++ b/perl/Git/SVN.pm
 @@ -3,9 +3,9 @@ use strict;
  use warnings;
  use Fcntl qw/:DEFAULT :seek/;
  use constant rev_map_fmt = 'NH40';
 -use vars qw/$default_repo_id $default_ref_id $_no_metadata $_follow_parent
 +use vars qw/$_no_metadata
  $_repack $_repack_flags $_use_svm_props $_head
 -$_use_svnsync_props $no_reuse_existing $_minimize_url
 +$_use_svnsync_props $no_reuse_existing
   $_use_log_author $_add_author_from $_localtime/;
  use Carp qw/croak/;
  use File::Path qw/mkpath/;
 @@ -30,6 +30,11 @@ BEGIN {
   $can_use_yaml = eval { require Git::SVN::Memoize::YAML; 1};
  }
  
 +our $_follow_parent  = 1;
 +our $_minimize_url   = 'unset';
 +our $default_repo_id = 'svn';
 +our $default_ref_id  = $ENV{GIT_SVN_ID} || 'git-svn';
 +
  my ($_gc_nr, $_gc_period);
  
  # properties that we do not log:
 diff --git a/t/Git-SVN/00compile.t b/t/Git-SVN/00compile.t
 index a7aa85a..97475d9 100644
 --- a/t/Git-SVN/00compile.t
 +++ b/t/Git-SVN/00compile.t
 @@ -3,6 +3,7 @@
  use strict;
  use warnings;
  
 -use Test::More tests = 1;
 +use Test::More tests = 2;
  
  require_ok 'Git::SVN::Utils';
 +require_ok 'Git::SVN';
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 4/4] Move initialization of Git::SVN variables into Git::SVN.

2012-07-26 Thread Jonathan Nieder
Junio C Hamano wrote:
 Michael G. Schwern schw...@pobox.com writes:

 Also it can compile on its own now, yay!

 Hmmm.

I agree with Michael's yay and also think it's fine that after
patch 3 it isn't there yet.

That's because git-svn.perl doesn't use Git::SVN on its own but helps
it out a little.  So even if we only applied patches 1-3, git-svn
would still work (maybe it's worth testing perl -MGit::SVN by hand
to avoid the it's doubtful about whether Git::SVN is self-contained
and replace it with a more certain statement?), and patch 4 just makes
it even better.

[...]
 In short:

  - I didn't see anything questionable in 1/4;

  - Calling up ::opt_prefix() from module in 2/4 looked ugly to me
but I suspect it should be easy to fix;

  - 3/4 was a straight move and I didn't see anything questionable in
it, but I think it would be nicer if intermediate steps can be
made to still work by making 4/4 come first or something
similarly simple.

 If the issues in 2/4 and 3/4 are easily fixable by going the route I
 handwaved above, the result of doing so based on this round is ready
 to be applied, I think.

 Eric, Jonathan, what do you think?

I think this is pretty good already, though I also like your
suggestion re 2/4.

I haven't reviewed the tests these introduce and assume Eric has that
covered.

Thanks,
Jonathan
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html