Re: [PATCH/RFC v4 01/16] GSOC remote-svn

2012-08-20 Thread Florian Achleitner
On Saturday 18 August 2012 23:35:38 Junio C Hamano wrote:
 Junio C Hamano gits...@pobox.com writes:
 [..]
 Just to show how, here is what I did just now.
 [..] 
 Thanks.

Thanks for you guidance!
I'll base a new version on your fixups.

Florian
--
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/RFC v4 01/16] GSOC remote-svn

2012-08-20 Thread Junio C Hamano
Florian Achleitner florian.achleitner.2.6...@gmail.com writes:

 On Saturday 18 August 2012 23:35:38 Junio C Hamano wrote:
 Junio C Hamano gits...@pobox.com writes:
 [..]
 Just to show how, here is what I did just now.
 [..] 
 Thanks.

 Thanks for you guidance!
 I'll base a new version on your fixups.

Just to make sure...

Please do _not_ use fixups as-is, but squash them into the
appropriate places where the problems they fix are first introduced.

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/RFC v4 01/16] GSOC remote-svn

2012-08-20 Thread Florian Achleitner
On Saturday 18 August 2012 13:13:47 Junio C Hamano wrote:
 That indicates that one necessary patch to add logic to Makefile to
 go and build that subdirectory, at least before running the test,
 but possibly as part of the all target, is missing, isn't it?
 
 Or you can add, at the beginning of your tests files that require
 the contrib bit, to have something like
 
 if test -e $GIT_BUILD_DIR/remote-svn
 then
 test_set_prereq REMOTE_SVN
 fi
 
 and protect your tests with the prerequisite, e.g.
 
 test_expect_success REMOTE_SVN 'test svn:// URL' '
 ...
 '
 
 without changing the top-level Makefile.

What version would you prefer? Currently nothing in contrib/ is built by the 
toplevel 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/RFC v4 01/16] GSOC remote-svn

2012-08-20 Thread Junio C Hamano
Florian Achleitner florian.achleitner.2.6...@gmail.com writes:

 On Saturday 18 August 2012 13:13:47 Junio C Hamano wrote:
 That indicates that one necessary patch to add logic to Makefile to
 go and build that subdirectory, at least before running the test,
 but possibly as part of the all target, is missing, isn't it?
 
 Or you can add, at the beginning of your tests files that require
 the contrib bit, to have something like
 
 if test -e $GIT_BUILD_DIR/remote-svn
 then
 test_set_prereq REMOTE_SVN
 fi
 
 and protect your tests with the prerequisite, e.g.
 
 test_expect_success REMOTE_SVN 'test svn:// URL' '
 ...
 '
 
 without changing the top-level Makefile.

 What version would you prefer? Currently nothing in contrib/ is built by the 
 toplevel Makefile..

The latter approach of running tests only for people who went to
contrib/vcs-svn and built things there may be more conservative.  I
could be persuaded either way.
--
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/RFC v4 01/16] GSOC remote-svn

2012-08-20 Thread Jonathan Nieder
Hi,

Florian Achleitner wrote:

 What version would you prefer?

I think git-remote-svn should move out of contrib to the toplevel of
git.git (as I think I've mentioned before).  Since it's just a new git
command in incubation, I don't want the maintenance hassle of keeping
it in contrib/svn-fe.  It can be a test program for now (e.g.,
git-remote-testsvn).

Hope that helps,
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


Re: [PATCH/RFC v4 01/16] GSOC remote-svn

2012-08-19 Thread Junio C Hamano
Junio C Hamano gits...@pobox.com writes:

 Florian Achleitner florian.achleitner.2.6...@gmail.com writes:

 On Friday 17 August 2012 21:16:59 Junio C Hamano wrote:
 Comments from mentors and people interested in remote helpers?
 
 I did minimum line wrapping, typofix and small compilation fixes
 and queued these on 'pu'; I think I saw one commit whose message
 I didn't quite get what it was trying to say, and another that was
 missing S-o-b (I left them untouched).

 Should I provide a better version? I found the commit that I forgot to sign-
 off, but I'm not sure which message you mean.

 There was a one with E.g: followed by an incomplete sentence that
 did not parse for me.  Can you fetch 'pu', run format-patch on your
 topic and compare the output with what you sent to the list?

Just to show how, here is what I did just now.

  (0) Store your 16-patch series and 5-patch series in a mbox;

  (1) Check where the tip of fa/vcs-svn topic is at.

  $ git log --oneline --first-parent master..pu | grep fa/
  2ce959b Merge branch 'fa/vcs-svn' into pu
  574ffe1 Merge branch 'fa/remote-svn' into pu

  (2) Check where the topic was based on.

  $ git log --oneline --first-parent master..2ce959b^2
  ...
  1385a48 Implement a remote helper for svn in C

  (3) Detach at the same base and apply the mbox from step (0).

  $ git checkout 1385a48^
  $ git am --whitespace=nowarn mbox

  (4) Format them (i.e. as the way you sent them, without my fixup)

  $ git format-patch --stdout master ./+fa-0

  (5) Format with my fixup

  $ git format-patch --stdout master..2ce959b^2 ./+fa-1

  (6) Compare them.  The differences are my fixups.

  $ diff -u ./+fa-0 ./+fa-1 | less

Patch #17 vcs-svn: Add sha1 calculation to fast_export ... was the
one with a sentence whose purpose was unclear to me.

By the time you see this message, the tip of pu may have been
updated with further updates, so please do not trust 2ce959b above.

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/RFC v4 01/16] GSOC remote-svn

2012-08-18 Thread Florian Achleitner
On Friday 17 August 2012 21:16:59 Junio C Hamano wrote:
 Comments from mentors and people interested in remote helpers?
 
 I did minimum line wrapping, typofix and small compilation fixes
 and queued these on 'pu'; I think I saw one commit whose message
 I didn't quite get what it was trying to say, and another that was
 missing S-o-b (I left them untouched).

Should I provide a better version? I found the commit that I forgot to sign-
off, but I'm not sure which message you mean.

 
 The result merged to 'pu' seems to fail 9020, by the way.

That's because contrib/svn-fe isn't built automatically if you call make in 
the toplevel dir. 
It dies with fatal: Unable to find remote helper for 'svn', because the 
helper is not built. We currently need to run make in contrib/svn-fe 
seperately.
That's a bit awkward.

Just checked how it works for svn-fe. It has a seperate test program (test-
svn-fe.c) which is in the toplevel dir and built here, while for svn-fe 
itself, it's the same as for remote-svn. 

Don't know what to do about that.

 
 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/RFC v4 01/16] GSOC remote-svn

2012-08-18 Thread Junio C Hamano
Florian Achleitner florian.achleitner.2.6...@gmail.com writes:

 On Friday 17 August 2012 21:16:59 Junio C Hamano wrote:
 Comments from mentors and people interested in remote helpers?
 
 I did minimum line wrapping, typofix and small compilation fixes
 and queued these on 'pu'; I think I saw one commit whose message
 I didn't quite get what it was trying to say, and another that was
 missing S-o-b (I left them untouched).

 Should I provide a better version? I found the commit that I forgot to sign-
 off, but I'm not sure which message you mean.

There was a one with E.g: followed by an incomplete sentence that
did not parse for me.  Can you fetch 'pu', run format-patch on your
topic and compare the output with what you sent to the list?

 The result merged to 'pu' seems to fail 9020, by the way.

 That's because contrib/svn-fe isn't built automatically if you call make in 
 the toplevel dir. 
 It dies with fatal: Unable to find remote helper for 'svn', because the 
 helper is not built. We currently need to run make in contrib/svn-fe 
 seperately.
 That's a bit awkward.

That indicates that one necessary patch to add logic to Makefile to
go and build that subdirectory, at least before running the test,
but possibly as part of the all target, is missing, isn't it?

Or you can add, at the beginning of your tests files that require
the contrib bit, to have something like

if test -e $GIT_BUILD_DIR/remote-svn
then
test_set_prereq REMOTE_SVN
fi

and protect your tests with the prerequisite, e.g.

test_expect_success REMOTE_SVN 'test svn:// URL' '
...
'

without changing the top-level 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