Re: [PATCH 0/6] Make 'git help everyday' work

2014-01-10 Thread Philip Oakley

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

I think we already use a nicer way to set up a page alias to keep
old links working than making a copy in Documentation/; please mimic
that if possible.


This was mainly about ensuring that the 'git help' command could access 
these extra extra guides that it currently misses. (Tt also misses the 
'user-manual', which isn't a man page, but could have a link page to 
guide the seeker of truth between 'git help' and the actual user-manual)


The only method I can see for that (via help.c) is to get the filename 
format correct.  Where you thinking of something else?




It may be overdue to refresh the suggested set of top 20 commands,
as things have vastly changed over the past 8 years.  Perhaps we
should do that after reorganizing with something like this series.


Agreed.


--


--
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 0/6] Make 'git help everyday' work

2014-01-10 Thread Stefan Näwe
Am 10.01.2014 00:49, schrieb Junio C Hamano:
 I think we already use a nicer way to set up a page alias to keep
 old links working than making a copy in Documentation/; please mimic
 that if possible.
 
 It may be overdue to refresh the suggested set of top 20 commands,
 as things have vastly changed over the past 8 years.  Perhaps we
 should do that after reorganizing with something like this series.

I'd really like to see 'git help relnotes' working as well...

Stefan
-- 

/dev/random says: Despite the high cost of living, it remains popular.
python -c print 
'73746566616e2e6e616577654061746c61732d656c656b74726f6e696b2e636f6d'.decode('hex')
--
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 v3 3/5] refs: teach for_each_ref a flag to avoid recursion

2014-01-10 Thread Michael Haggerty
On 01/09/2014 10:49 PM, Jeff King wrote:
 On Wed, Jan 08, 2014 at 12:29:51PM +0100, Michael Haggerty wrote:
 
 Here's a fixed version of patch 3/5.

 v2 4/5 doesn't apply cleanly on top of v3 3/5.  So I'm basing my review
 on the branch you have at GitHub peff/git jk/cat-file-warn-ambiguous;
 I hope it is the same.
 
 Hrmph. I didn't have to do any conflict resolution during the rebase, so
 I would think it would apply at least with am -3.

That may be; I didn't try with -3.

 [...]
  static int do_for_each_entry(struct ref_cache *refs, const char *base,
 -each_ref_entry_fn fn, void *cb_data)
 +each_ref_entry_fn fn, void *cb_data,
 +int flags)
  {
 struct packed_ref_cache *packed_ref_cache;
 struct ref_dir *loose_dir;

 A few lines after this, do_for_each_entry() calls
 prime_ref_dir(loose_dir) to ensure that all of the loose references that
 will be iterated over are read before the packed-refs file is checked.
 It seems to me that prime_ref_dir() should also get a flags parameter to
 prevent it reading more loose references than necessary, something like
 this:
 
 Hmm. I hadn't considered that, but yeah, it definitely nullifies part of
 the purpose of the optimization.
 
 However, is it safe to prime only part of the loose ref namespace? The
 point of that priming is to avoid the race fixed in 98eeb09, which
 depends on us caching the loose refs before the packed refs. But when we
 read packed-refs, we will be reading and storing _all_ of it, even if we
 do not touch it in this traversal. So it does not affect the race for
 this traversal, but have we setup a cache situation where a subsequent
 for_each_ref in the same process would be subject to the race?

prime_ref_dir() is called by do_for_each_entry(), which all the
iteration functions pass through.  It is always called before the
iteration starts, and it primes only the subtree of the refs hierarchy
that is being iterated over.  For example, if iterating over
refs/heads then it only primes references with that prefix.

This is OK, because if later somebody iterates over a broader part of
the refs hierarchy (say, refs), then priming is done again, including
re-checking the packed refs.  If the packed-refs file was changed
between the iterations, then the first iteration (if it is still
running) continues using the old packed-refs cache while the second
iteration uses the new packed-refs cache.  (So the first iteration will
have a stale, but self-consistent, view of the references.)

If do_for_each_entry() gets the DO_FOR_EACH_NO_RECURSE option, then it
knows that it will only traverse one level of the refs hierarchy.  So if
it passes the option to prime_ref_dir(), then the same level will be
primed.  If somebody later iterates over the same part of the hierarchy
without DO_FOR_EACH_NO_RECURSE, they will re-prime without
DO_FOR_EACH_NO_RECURSE then, if the packed-refs file has been changed,
load a new version of it.  So they will also get a self-consistent view
of the references and I think everything will be OK.

 I'm starting to wonder if this optimization is worth it.

It's true that this is quite a special-case optimization.

I think reference handling will have to move in the direction of
transactions, to remove one or two known race conditions.  That is why I
described the alternative of having the DWIM function do its lookups in
a ref cache.  It would move in the direction of consciously taking a
snapshot of the ref tree and using it for a whole transaction, which I
think is a style that we will want to use in more places.  It's just
hard to judge whether this alternative would actually solve the
performance problem that you were originally trying to address--a point
that Junio discussed elsewhere in this thread.

(This is something I would be willing to work on if you feel like I am
pushing you to enlarge the scope of your work beyond what you are
interested in.)

 [...]
 @@ -1718,7 +1732,7 @@ static int do_for_each_ref(struct ref_cache *refs, 
 const char *base,
 data.fn = fn;
 data.cb_data = cb_data;
  
 -   return do_for_each_entry(refs, base, do_one_ref, data);
 +   return do_for_each_entry(refs, base, do_one_ref, data, flags);
  }

 This change makes the DO_FOR_EACH_NO_RECURSE option usable with
 do_for_each_ref() (even though it is never in fact used).  It should
 either be mentioned in the docstring or (if there is a reason not to
 allow it) explicitly prohibited.
 
 Hrm, yeah. I guess there are no callers, and there is no plan for any.
 So we could just pass 0 here, and then flags passed to
 do_for_each_ref really is _just_ for the callback data that goes to
 do_one_ref. That clears up the weird combined namespace stuff I
 mentioned in the commit message, and is a bit cleaner. I'll take it in
 that direction.

It would also be possible to swing in the other direction.  I don't
remember a particular reason why I left the DO_FOR_EACH_INCLUDE_BROKEN

Re: [PATCH v3 3/5] refs: teach for_each_ref a flag to avoid recursion

2014-01-10 Thread Jeff King
On Fri, Jan 10, 2014 at 09:59:25AM +0100, Michael Haggerty wrote:

  However, is it safe to prime only part of the loose ref namespace?
 [...]
 
 prime_ref_dir() is called by do_for_each_entry(), which all the
 iteration functions pass through.  It is always called before the
 iteration starts, and it primes only the subtree of the refs hierarchy
 that is being iterated over.  For example, if iterating over
 refs/heads then it only primes references with that prefix.
 
 This is OK, because if later somebody iterates over a broader part of
 the refs hierarchy (say, refs), then priming is done again, including
 re-checking the packed refs.

Ah, right. This is the part I was forgetting: the next for_each_ref will
re-prime with the expanded view. Thanks for a dose of sanity.

I'll fix that in my re-roll.

 It would also be possible to swing in the other direction.  I don't
 remember a particular reason why I left the DO_FOR_EACH_INCLUDE_BROKEN
 handling at the do_for_each_ref() level rather than handling it at the
 do_for_each_entry() level.  But now that you are passing the flags
 parameter all the way down the call stack, it wouldn't cost anything to
 support both of the DO_FOR_EACH flags everywhere and just document it
 that way.

I think it was simply that it was an option that the traversal did not
need to know about (just like the trim option), so you kept it as
encapsulated as possible. I think I'll introduce it as a separate flag
namespace, as discussed in the previous email. It is the same amount of
refactoring work to merge them later as it is now, if we so choose.

-Peff
--
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 v2 4/5] get_sha1: speed up ambiguous 40-hex test

2014-01-10 Thread Jeff King
On Wed, Jan 08, 2014 at 05:09:25PM +0100, Michael Haggerty wrote:

 It's not only racy WRT other processes.  If the current git process
 would create a new reference, it wouldn't be reflected in the cache.
 
 It's true that the main ref_cache doesn't invalidate itself
 automatically either when a new reference is created, so it's not really
 a fair complaint.  However, as we add places where the cache is
 invalidated, it is easy to overlook this cache that is stuck in static
 variables within a function definition and it is impossible to
 invalidate it.  Might it not be better to attach the cache to the
 ref_cache structure instead, and couple its lifetime to that object?

Yeah, I noticed that we don't ever invalidate the loose ref cache. I
think that's mostly fine, as we rely on resolve_ref to cover the cases
that need to be ordered properly. And in this particular case, it's
only a warning (and a rather obscure one, at that), so I think the
stakes are low.

That being said, it should not be hard at all to attach the cache to the
ref_cache. Since we are generated from that cache, the lifetimes should
be the same.

 Alternatively, the cache could be created and managed on the caller
 side, since the caller would know when the cache would have to be
 invalidated.  Also, different callers are likely to have different
 performance characteristics.  It is unlikely that the time to initialize
 the cache will be amortized in most cases; in fact, rev-list --stdin
 might be the *only* plausible use case.

The two I know of are rev-list --stdin and cat-file --batch-check.

 Regarding the overall strategy: you gather all refnames that could be
 confused with an SHA-1 into a sha1_array, then later look up SHA-1s in
 the array to see if they are ambiguous.  This is a very special-case
 optimization for SHA-1s.

Yes, it is very sha1-specific. Part of my goal was that in the common
case, the check would collapse to O(# of ambiguous refs), which is
typically 0.

That may be premature optimization, though. As you note below, doing a
few binary searches through the in-memory ref cache is _probably_ fine,
too. And we can do that without a separate index.

 I wonder whether another approach would gain almost the same amount of
 performance but be more general.  We could change dwim_ref() (or a
 version of it?) to read its data out of a ref_cache instead of going to
 disk every time.  Then, at the cost of populating the relevant parts of
 the ref_cache once, we would have fast dwim_ref() calls for all strings.

I'm very nervous about turning dwim_ref into a cache. As we noted above,
we never invalidate the cache, so any write-then-read operations could
get stale data. That is not as risky as caching, say, resolve_ref, but
it still makes me nervous. Caching just the warning has much lower
stakes.

 It's true that the lookups wouldn't be quite so fast--they would require
 a few bisects per refname lookup (one for each level in the refname
 hierarchy) and several refname lookups (one for each ref_rev_parse_rule)
 for every dwim_ref() call, vs. a single bisect in your current design.
 But this approach it would bring us most of the gain, it might
 nevertheless be preferable.

I don't think this would be all that hard to measure. I'll see what I
can do.

  I wanted to make the ref traversal as cheap as possible, hence the
  NO_RECURSE flag I added. I thought INCLUDE_BROKEN used to not open up
  the refs at all, but it looks like it does these days. I wonder if that
  is worth changing or not.
 
 What do you mean by open up the refs?  The loose reference files are
 read when populating the cache.  (Was that ever different?)

I meant actually open the ref files and read the sha1. But that is me
being dumb. We have always done that, as we must provide the sha1 via
to the for_each_ref callback.

That being said, we could further optimize this by not opening the files
at all (and make that the responsibility of do_one_ref, which we are
avoiding here). I am slightly worried about the open() cost of my
solution. It's amortized away in a big call, but it is probably
noticeable for something like `git rev-parse 40-hex`.

 This doesn't correctly handle the rule
 
   refs/remotes/%.*s/HEAD

 We might be willing to accept this limitation, but it should at least be
 mentioned somewhere.  OTOH if we want to handle this pattern as well, we
 could do use a technique like that of shorten_unambiguous_ref().

Yes, you're right. I considered this, but for some reason convinced
myself that it would be OK to just look for refs/remotes/40-hex in
this case (which is what my patch does). But obviously that's wrong.
The ref traversal won't find a _directory_ with that name.

I'll see how painful it is to make it work. I have to say I was tempted
to simply manually write the rules. It's a duplication that could go out
of sync with the ref_rev_parse rules, and that's nasty. But that list does
not change often, and the reverse-parsing of the rules is 

Subject: Something like cat-file for the index?

2014-01-10 Thread Enno Weichert
Hi,

I'd like to have a more technical look into the index file and what/how it
stores data; call it educational spelunking.

I know the index-format.txt but I'd really like to save me the work to
implement a pretty-printed output based on it.
I know ls-files but that's obviously not the whole thing.

So: is there something like cat-file, that basically gives me a readable
version of the information (version number and all...) in the index already
implemented or did nobody care until 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: Subject: Something like cat-file for the index?

2014-01-10 Thread Thomas Gummerer

Hi,

Enno Weichert enno.weich...@gmail.com writes:
 Hi,

 I'd like to have a more technical look into the index file and what/how it
 stores data; call it educational spelunking.

 I know the index-format.txt but I'd really like to save me the work to
 implement a pretty-printed output based on it.
 I know ls-files but that's obviously not the whole thing.

 So: is there something like cat-file, that basically gives me a readable
 version of the information (version number and all...) in the index already
 implemented or did nobody care until now?

You can use `git ls-files --debug` and `git ls-files --stage` to get all
the information about the files in the index.  The meaning of the flags
is the only thing that's not shown by the command, and I don't think
there is a tool yet to examine them.

The undocumented --resolve-undo flag to git ls-files shows you the
resolve undo data that is stored in the index.

If you build git yourself, the `test-dump-cache-tree` helper can be used
to show all information about the cache-tree that is stored in the
index.

The you can get the version of the index either by using
`test-index-version` when you build git yourself, or by using `file
.git/index`, which in addition will give you the number of entries that
are in the index.

--
Thomas
--
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


git-log --cherry-pick gives different results when using tag or tag^{}

2014-01-10 Thread Francis Moreau
Hello,

In mykernel repository, I'm having 2 different behaviours with git-log
but I don't understand why:

Doing:

$ git log --oneline --cherry-pick --left-right v3.4.71-1^{}...next

and

$ git log --oneline --cherry-pick --left-right v3.4.71-1...next

give something different (where v3.4.71-1 is a tag).

The command using ^{} looks the one that gives correct result I think.

Could anybody enlight me ?

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 3/3] shorten_unambiguous_ref(): tighten up pointer arithmetic

2014-01-10 Thread Michael Haggerty
On 01/10/2014 12:01 AM, Junio C Hamano wrote:
 Michael Haggerty mhag...@alum.mit.edu writes:
 
 As long as we're being pathologically stingy with mallocs, we might as
 well do the math right and save 6 (!) bytes.

 Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
 ---
 It is left to the reader to show how another 7 bytes could be saved
 (11 bytes on a 64-bit architecture!)

 It probably wouldn't kill performance to use a string_list here
 instead.

  refs.c | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)

 diff --git a/refs.c b/refs.c
 index ef9cdea..63b3a71 100644
 --- a/refs.c
 +++ b/refs.c
 @@ -3351,10 +3351,10 @@ char *shorten_unambiguous_ref(const char *refname, 
 int strict)
  size_t total_len = 0;
  size_t offset = 0;
  
 -/* the rule list is NULL terminated, count them first */
 +/* the rule list is NUL terminated, count them first */
 
 I think this _is_ wrong; it talks about the NULL termination of the
 ref_rev_parse_rules[] array, not each string that is an element of
 the array being NUL terminated.

Yes, you're right.  Thanks for catching my sloppiness.  Would you mind
squashing the fix onto my patch?

 Output from git grep -e refname_match -e ref_rev_parse_rules
 suggests me that we actually could make ref_rev_parse_rules[] a
 file-scope static to refs.c, remove its NULL termination and convert
 all the iterators of the array to use ARRAY_SIZE() on it, after
 dropping the third parameter to refname_match().  That way, we do
 not have to count them first here.
 
 But that is obviously a separate topic.
 
  for (nr_rules = 0; ref_rev_parse_rules[nr_rules]; nr_rules++)
 -/* no +1 because strlen(%s)  strlen(%.*s) */
 -total_len += strlen(ref_rev_parse_rules[nr_rules]);
 +/* -2 for strlen(%.*s) - strlen(%s); +1 for NUL */
 +total_len += strlen(ref_rev_parse_rules[nr_rules]) - 2 
 + 1;
  
  scanf_fmts = xmalloc(nr_rules * sizeof(char *) + total_len);

The way the code is written now (e.g., as long as it is not converted to
use a string_list or something) needs this loop not only to count the
number of rules but also to compute the total_len of the string into
which will be written all of the scanf format strings.

As for removing the third argument of refname_match(): although all
callers pass it ref_ref_parse_rules, that array is sometimes passed to
the function via the alias ref_fetch_rules.  So I suppose somebody
wanted to leave the way open to make these two rule sets diverge (though
I don't know how likely that is to occur).  If we discard the third
argument to refname_match(), then we loose the distinction.

Thanks for your feedback,
Michael

-- 
Michael Haggerty
mhag...@alum.mit.edu
http://softwareswirl.blogspot.com/
--
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: Subject: Something like cat-file for the index?

2014-01-10 Thread Enno Weichert
Thank you :)

On 1/10/14, Thomas Gummerer t.gumme...@gmail.com wrote:

 Hi,

 Enno Weichert enno.weich...@gmail.com writes:
 Hi,

 I'd like to have a more technical look into the index file and what/how
 it
 stores data; call it educational spelunking.

 I know the index-format.txt but I'd really like to save me the work to
 implement a pretty-printed output based on it.
 I know ls-files but that's obviously not the whole thing.

 So: is there something like cat-file, that basically gives me a readable
 version of the information (version number and all...) in the index
 already
 implemented or did nobody care until now?

 You can use `git ls-files --debug` and `git ls-files --stage` to get all
 the information about the files in the index.  The meaning of the flags
 is the only thing that's not shown by the command, and I don't think
 there is a tool yet to examine them.

 The undocumented --resolve-undo flag to git ls-files shows you the
 resolve undo data that is stored in the index.

 If you build git yourself, the `test-dump-cache-tree` helper can be used
 to show all information about the cache-tree that is stored in the
 index.

 The you can get the version of the index either by using
 `test-index-version` when you build git yourself, or by using `file
 .git/index`, which in addition will give you the number of entries that
 are in the index.

 --
 Thomas

--
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 3/3] shorten_unambiguous_ref(): tighten up pointer arithmetic

2014-01-10 Thread Junio C Hamano
Michael Haggerty mhag...@alum.mit.edu writes:

 As for removing the third argument of refname_match(): although all
 callers pass it ref_ref_parse_rules, that array is sometimes passed to
 the function via the alias ref_fetch_rules.  So I suppose somebody
 wanted to leave the way open to make these two rule sets diverge (though
 I don't know how likely that is to occur).

It is the other way around.  We used to use two separate rules for
the normal ref resolution dwimming and dwimming done to decide which
remote ref to grab.  For example, 'x' in 'git log x' can mean
'refs/remotes/x/HEAD', but 'git fetch origin x' would not grab
'refs/remotes/x/HEAD' at 'origin'.  Also, 'git fetch origin v1.0'
did not look into 'refs/tags/v1.0' in the 'origin' repository back
when these two rules were different.

This was originally done very much on purpose, in order to avoid
surprises and to discourage meaningless usage---you would not expect
us to be interested in the state of a third repository that was
observed by our 'origin' the last time (which we do not even know
when).

When we harmonized these two rules later and we #defined out
ref_fetch_rules away to avoid potential breakages for in-flight
topics.  The old synonym can now go safely.
--
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 0/6] Make 'git help everyday' work

2014-01-10 Thread Junio C Hamano
Philip Oakley philipoak...@iee.org writes:

 From: Junio C Hamano gits...@pobox.com
I think we already use a nicer way to set up a page alias to keep
 old links working than making a copy in Documentation/; please mimic
 that if possible.

 This was mainly about ensuring that the 'git help' command could
 access these extra extra guides that it currently misses. (Tt also
 misses the 'user-manual', which isn't a man page, but could have a
 link page to guide the seeker of truth between 'git help' and the
 actual user-manual)

 The only method I can see for that (via help.c) is to get the filename
 format correct.  Where you thinking of something else?

I do not have an objection against the creation of giteveryday.txt;
I was questioning the way the original everyday.txt was left behind
to bit-rot.  It is good to keep _something_ there, because there may
be old URLs floating around that point at Documentation/everyday.txt,
but the contents of that file does not have to be a stale copy.

Cf. bd4a3d61 (Rename {git- = git}remote-helpers.txt, 2013-01-31)
for how we renamed git-remote-helpers.txt to gitremote-helpers.txt
--
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


[PATCH] git-p4: Do not include diff in spec file when just preparing p4

2014-01-10 Thread Maxime Coste
The diff information render the spec file unusable as is by p4,
do not include it when run with --prepare-p4-only so that the
given file can be directly passed to p4.
---
 git-p4.py | 70 +++
 1 file changed, 39 insertions(+), 31 deletions(-)

diff --git a/git-p4.py b/git-p4.py
index 5ea8bb8..7c65340 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -1397,38 +1397,14 @@ class P4Submit(Command, P4UserMap):
 submitTemplate +=  Use option --preserve-user to modify 
authorship.\n
 submitTemplate +=  Variable git-p4.skipUserNameCheck 
hides this message.\n
 
-separatorLine =  everything below this line is just the diff 
###\n
-
-# diff
-if os.environ.has_key(P4DIFF):
-del(os.environ[P4DIFF])
-diff = 
-for editedFile in editedFiles:
-diff += p4_read_pipe(['diff', '-du',
-  wildcard_encode(editedFile)])
-
-# new file diff
-newdiff = 
-for newFile in filesToAdd:
-newdiff +=  new file \n
-newdiff += --- /dev/null\n
-newdiff += +++ %s\n % newFile
-f = open(newFile, r)
-for line in f.readlines():
-newdiff += + + line
-f.close()
-
-# change description file: submitTemplate, separatorLine, diff, newdiff
-(handle, fileName) = tempfile.mkstemp()
-tmpFile = os.fdopen(handle, w+)
-if self.isWindows:
-submitTemplate = submitTemplate.replace(\n, \r\n)
-separatorLine = separatorLine.replace(\n, \r\n)
-newdiff = newdiff.replace(\n, \r\n)
-tmpFile.write(submitTemplate + separatorLine + diff + newdiff)
-tmpFile.close()
-
 if self.prepare_p4_only:
+(handle, fileName) = tempfile.mkstemp()
+tmpFile = os.fdopen(handle, w+)
+if self.isWindows:
+submitTemplate = submitTemplate.replace(\n, \r\n)
+tmpFile.write(submitTemplate)
+tmpFile.close()
+
 #
 # Leave the p4 tree prepared, and the submit template around
 # and let the user decide what to do next
@@ -1463,6 +1439,38 @@ class P4Submit(Command, P4UserMap):
 print
 return True
 
+else:
+separatorLine =  everything below this line is just the 
diff ###\n
+
+# diff
+if os.environ.has_key(P4DIFF):
+del(os.environ[P4DIFF])
+diff = 
+for editedFile in editedFiles:
+diff += p4_read_pipe(['diff', '-du',
+  wildcard_encode(editedFile)])
+
+# new file diff
+newdiff = 
+for newFile in filesToAdd:
+newdiff +=  new file \n
+newdiff += --- /dev/null\n
+newdiff += +++ %s\n % newFile
+f = open(newFile, r)
+for line in f.readlines():
+newdiff += + + line
+f.close()
+
+# change description file: submitTemplate, separatorLine, diff, 
newdiff
+(handle, fileName) = tempfile.mkstemp()
+tmpFile = os.fdopen(handle, w+)
+if self.isWindows:
+submitTemplate = submitTemplate.replace(\n, \r\n)
+separatorLine = separatorLine.replace(\n, \r\n)
+newdiff = newdiff.replace(\n, \r\n)
+tmpFile.write(submitTemplate + separatorLine + diff + newdiff)
+tmpFile.close()
+
 #
 # Let the user edit the change description, then submit it.
 #
-- 
1.8.5.2


--
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


A question about the error: svn_fspath__is_canonical

2014-01-10 Thread Dan Kaplan
I found this mailing list thread discussing the problem I'm currently
experiencing: 
http://git.661346.n2.nabble.com/Fwd-Error-with-git-svn-pushing-a-rename-td7599382.html

Apparently a patch was submitted to fix this bug and I'm trying to
figure out what version of what I need to fix this bug.  The inability
to rename a class is a pretty limiting.

My environment is probably different from most.  I'm using cygwin.
This makes it very difficult to use different versions of
git/svn/git-svn, but I'm interested in learning git more so I'm
willing to try whatever it takes.

$ git version
git version 1.8.3.4

$ svn --version
svn, version 1.8.5 (r1542147)
   compiled Nov 25 2013, 10:45:07 on x86_64-unknown-cygwin

Copyright (C) 2013 The Apache Software Foundation.
This software consists of contributions made by many people;
see the NOTICE file for more information.
Subversion is open source software, see http://subversion.apache.org/

The following repository access (RA) modules are available:

* ra_svn : Module for accessing a repository using the svn network protocol.
  - with Cyrus SASL authentication
  - handles 'svn' scheme
* ra_local : Module for accessing a repository on local disk.
  - handles 'file' scheme
* ra_serf : Module for accessing a repository via WebDAV protocol using serf.
  - using serf 1.3.3
  - handles 'http' scheme
  - handles 'https' scheme

Thanks for the help

--
Thanks,
Dan

-- 
CONFIDENTIALITY NOTICE: The information contained in this electronic 
transmission may be confidential. If you are not an intended recipient, be 
aware that any disclosure, copying, distribution or use of the information 
contained in this transmission is prohibited and may be unlawful. If you 
have received this transmission in error, please notify us by email reply 
and then erase it from your computer system.
--
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: A question about the error: svn_fspath__is_canonical

2014-01-10 Thread Jonathan Nieder
Hi Dan,

Dan Kaplan wrote:

 My environment is probably different from most.  I'm using cygwin.
 This makes it very difficult to use different versions of
 git/svn/git-svn, but I'm interested in learning git more so I'm
 willing to try whatever it takes.

 $ git version
 git version 1.8.3.4

 $ svn --version
 svn, version 1.8.5 (r1542147)
compiled Nov 25 2013, 10:45:07 on x86_64-unknown-cygwin

You have three choices:

 A) upgrade git to latest master
 B) upgrade subversion to latest trunk
 C) downgrade subversion to a version before that bug was introduced

(A) is probably simplest.  E.g., something like the following should work:

  git clone https://kernel.googlesource.com/pub/scm/git/git.git
  cd git
  make -j8
  make test; # optional, to verify that the git you built works ok
  export PATH=$(pwd)/bin-wrappers:$PATH

Now the updated git is in your $PATH and you can use it.

See INSTALL in the git source tree for more details.

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 mm/mv-file-to-no-such-dir-with-slash] mv: let 'git mv file no-such-dir/' error out on Windows, too

2014-01-10 Thread Johannes Sixt
Am 09.01.2014 23:42, schrieb Junio C Hamano:
 Johannes Sixt j...@kdbg.org writes:
 
 The previous commit c57f628 (mv: let 'git mv file no-such-dir/' error out)
 relies on that rename(src, dst/) fails if directory dst does not
 exist (note the trailing slash). This does not work as expected on Windows:
 This rename() call is successful. Insert an explicit check for this case.
 
 Could you care to explain Successful how a bit here?  Do we see
 no-such-dir mkdir'ed and then no-such-dir/file created?  Do we see
 file moved to a new file whose name is no-such-dir/?  I am guessing
 that it would be the latter, but if that is the case we would need
 at least an air-quote around successful.

The file is renamed to no-such-dir without the slash at the end. The
updated commit message would be:

mv: let 'git mv file no-such-dir/' error out on Windows, too

The previous commit c57f628 (mv: let 'git mv file no-such-dir/' error out)
relies on that rename(src, dst/) fails if directory dst does not
exist (note the trailing slash). This does not work as expected on Windows:
The rename() call does not fail, but renames src to dst (without the
trailing slash). Insert an explicit check for this case to force an error.

This changes the error message from

   $ git mv file no-such-dir/
   fatal: renaming 'file' failed: Not a directory

to

   $ git mv file no-such-dir/
   fatal: destination directory does not exist, source=file, 
destination=no-such-dir/

Signed-off-by: Johannes Sixt j...@kdbg.org

 
 This changes the error message from

$ git mv file no-such-dir/
fatal: renaming 'file' failed: Not a directory

 to

$ git mv file no-such-dir/
fatal: destination directory does not exist, source=file, 
 destination=no-such-dir/

 Signed-off-by: Johannes Sixt j...@kdbg.org
 ---
  builtin/mv.c | 2 ++
  1 file changed, 2 insertions(+)

 diff --git a/builtin/mv.c b/builtin/mv.c
 index 08fbc03..21c46d1 100644
 --- a/builtin/mv.c
 +++ b/builtin/mv.c
 @@ -214,6 +214,8 @@ int cmd_mv(int argc, const char **argv, const char 
 *prefix)
  }
  } else if (string_list_has_string(src_for_dst, dst))
  bad = _(multiple sources for the same target);
 +else if (is_dir_sep(dst[strlen(dst) - 1]))
 +bad = _(destination directory does not exist);
  else
  string_list_insert(src_for_dst, dst);
 

--
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: A question about the error: svn_fspath__is_canonical

2014-01-10 Thread Dan Kaplan
Because I'm on cygwin, that's a little intimidating to me.  I've never
compiled sources on cygwin.  Do you think it'll still work?

On Fri, Jan 10, 2014 at 11:16 AM, Jonathan Nieder jrnie...@gmail.com wrote:
 Hi Dan,

 Dan Kaplan wrote:

 My environment is probably different from most.  I'm using cygwin.
 This makes it very difficult to use different versions of
 git/svn/git-svn, but I'm interested in learning git more so I'm
 willing to try whatever it takes.

 $ git version
 git version 1.8.3.4

 $ svn --version
 svn, version 1.8.5 (r1542147)
compiled Nov 25 2013, 10:45:07 on x86_64-unknown-cygwin

 You have three choices:

  A) upgrade git to latest master
  B) upgrade subversion to latest trunk
  C) downgrade subversion to a version before that bug was introduced

 (A) is probably simplest.  E.g., something like the following should work:

   git clone https://kernel.googlesource.com/pub/scm/git/git.git
   cd git
   make -j8
   make test; # optional, to verify that the git you built works ok
   export PATH=$(pwd)/bin-wrappers:$PATH

 Now the updated git is in your $PATH and you can use it.

 See INSTALL in the git source tree for more details.

 Hope that helps,
 Jonathan



-- 
Thanks,
Dan

-- 
CONFIDENTIALITY NOTICE: The information contained in this electronic 
transmission may be confidential. If you are not an intended recipient, be 
aware that any disclosure, copying, distribution or use of the information 
contained in this transmission is prohibited and may be unlawful. If you 
have received this transmission in error, please notify us by email reply 
and then erase it from your computer system.
--
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: A question about the error: svn_fspath__is_canonical

2014-01-10 Thread Jonathan Nieder
Dan Kaplan wrote:

  Do you think it'll still work?

Yes, that's why I suggested it. ;-)

You might need to install the gcc-core, libcurl-devel, openssl-devel,
and subversion-perl packages first.

Regards,
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: A question about the error: svn_fspath__is_canonical

2014-01-10 Thread Torsten Bögershausen
On 2014-01-10 20.28, Jonathan Nieder wrote:
 Dan Kaplan wrote:
 
  Do you think it'll still work?
 
 Yes, that's why I suggested it. ;-)
 
 You might need to install the gcc-core, libcurl-devel, openssl-devel,
 and subversion-perl packages first.
 
 Regards,
 Jonathan
Out of my head:
You probably need to install even:
make, expat-devel (or similar)

--
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


[PATCH sb/diff-orderfile-config] diff test: reading a directory as a file need not error out

2014-01-10 Thread Jonathan Nieder
There is no guarantee that strbuf_read_file must error out for
directories.  On some operating systems (e.g., Debian GNU/kFreeBSD
wheezy), reading a directory gives its raw content:

$ head -c5  / | cat -A
^AM-|^_^@^L$

As a result, 'git diff -O/' succeeds instead of erroring out on
these systems, causing t4056.5 orderfile is a directory to fail.

On some weird OS it might even make sense to pass a directory to the
-O option and this is not a common user mistake that needs catching.
Remove the test.

Signed-off-by: Jonathan Nieder jrnie...@gmail.com
---
Hi,

t4056 is failing on systems using glibc with the kernel of FreeBSD[1]:

| expecting success: 
|   test_must_fail git diff -O/ --name-only HEAD^..HEAD
|
| a.h
| b.c
| c/Makefile
| d.txt
| test_must_fail: command succeeded: git diff -O/ --name-only HEAD^..HEAD
| not ok 5 - orderfile is a directory

How about this patch?

Thanks,
Jonathan

[1] 
https://buildd.debian.org/status/fetch.php?pkg=gitarch=kfreebsd-amd64ver=1%3A2.0~next.20140107-1stamp=1389379274

 t/t4056-diff-order.sh | 4 
 1 file changed, 4 deletions(-)

diff --git a/t/t4056-diff-order.sh b/t/t4056-diff-order.sh
index 1ddd226..9e2b29e 100755
--- a/t/t4056-diff-order.sh
+++ b/t/t4056-diff-order.sh
@@ -68,10 +68,6 @@ test_expect_success POSIXPERM,SANITY 'unreadable orderfile' '
test_must_fail git diff -Ounreadable_file --name-only HEAD^..HEAD
 '
 
-test_expect_success 'orderfile is a directory' '
-   test_must_fail git diff -O/ --name-only HEAD^..HEAD
-'
-
 for i in 1 2
 do
test_expect_success orderfile using option ($i) '
-- 
1.8.5.1

--
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 0/6] Make 'git help everyday' work

2014-01-10 Thread Jonathan Nieder
Hi,

Philip Oakley wrote:

 The Everyday GIT With 20 Commands Or So guide is not accessible
 via the git help system. Fix that.

Neat. :)

Junio covered everything I'd want to say about patch 1/6.

After fixing that, I'd suggest squashing all 6 patches into a single
patch.  They all are part of accomplishing the same task, they are not
too hard to read together, and the intermediate state after applying a
few but not the rest doesn't make much sense.  The details of patches
2-6/6 look good to me.

Alternatively, this could be two patches:

 1 - modify everyday.txt in place to be a suitable manpage
 2 - rename it, add a placeholder for the old name, and modify the
 build rules to treat it as an actual manpage

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: A question about the error: svn_fspath__is_canonical

2014-01-10 Thread Dan Kaplan
I had to convert every file from windows line endings to unix line
endings with dos2unix (dos2unix was a separate install).  I did that
with this command: find . -type f | xargs dos2unix

I also had to install: libiconv, gettext, expat, gettext-devel, expat-devel

That got my make -j8 to run without error.

I then ran make test and it looked like most of the passed.  I added
it to my path and now my bug is fixed!  Thanks for the help guys.


On Fri, Jan 10, 2014 at 11:34 AM, Torsten Bögershausen tbo...@web.de wrote:
 On 2014-01-10 20.28, Jonathan Nieder wrote:
 Dan Kaplan wrote:

  Do you think it'll still work?

 Yes, that's why I suggested it. ;-)

 You might need to install the gcc-core, libcurl-devel, openssl-devel,
 and subversion-perl packages first.

 Regards,
 Jonathan
 Out of my head:
 You probably need to install even:
 make, expat-devel (or similar)




-- 
Thanks,
Dan

-- 
CONFIDENTIALITY NOTICE: The information contained in this electronic 
transmission may be confidential. If you are not an intended recipient, be 
aware that any disclosure, copying, distribution or use of the information 
contained in this transmission is prohibited and may be unlawful. If you 
have received this transmission in error, please notify us by email reply 
and then erase it from your computer system.
--
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 0/6] Make 'git help everyday' work

2014-01-10 Thread Philip Oakley

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

Philip Oakley philipoak...@iee.org writes:


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

I think we already use a nicer way to set up a page alias to keep
old links working than making a copy in Documentation/; please mimic
that if possible.


This was mainly about ensuring that the 'git help' command could
access these extra extra guides that it currently misses. (Tt also
misses the 'user-manual', which isn't a man page, but could have a
link page to guide the seeker of truth between 'git help' and the
actual user-manual)

The only method I can see for that (via help.c) is to get the 
filename

format correct.  Where you thinking of something else?


I do not have an objection against the creation of giteveryday.txt;
I was questioning the way the original everyday.txt was left behind
to bit-rot.  It is good to keep _something_ there, because there may
be old URLs floating around that point at Documentation/everyday.txt,
but the contents of that file does not have to be a stale copy.


Ah, OK. I had indicated it would be deprecated, but had resisted stating 
a date for deletion (e.g. git 2.0).


I was thinking of a moderate two step deprecation period with the next 
step being a severely cut down residual stub, before it's removal.




Cf. bd4a3d61 (Rename {git- = git}remote-helpers.txt, 2013-01-31)
for how we renamed git-remote-helpers.txt to gitremote-helpers.txt


I'll have a look at re-using that approach.

Anything else needed before a re-roll?

Philip

--
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


What's cooking in git.git (Jan 2014, #02; Fri, 10)

2014-01-10 Thread Junio C Hamano
What's cooking in git.git (Jan 2014, #02; Fri, 10)
--

Here are the topics that have been cooking.  Commits prefixed with
'-' are only in 'pu' (proposed updates) while commits prefixed with
'+' are in 'next'.

Quite a few topics have graduated to 'master' in this round and the
upcoming release is starting to take shape.

You can find the changes described here in the integration branches
of the repositories listed at

http://git-blame.blogspot.com/p/git-public-repositories.html

--
[Graduated to master]

* ap/path-max (2013-12-16) 1 commit
  (merged to 'next' on 2014-01-03 at affc620)
 + Prevent buffer overflows when path is too long


* bc/log-decoration (2013-12-20) 1 commit
  (merged to 'next' on 2014-01-03 at ff8873f)
 + log: properly handle decorations with chained tags

 git log --decorate did not handle a tag pointed by another tag
 nicely.


* bm/merge-base-octopus-dedup (2013-12-30) 2 commits
  (merged to 'next' on 2014-01-06 at 355d62b)
 + merge-base --octopus: reduce the result from get_octopus_merge_bases()
 + merge-base: separate --independent codepath into its own helper

 git merge-base --octopus used to leave cleaning up suboptimal
 result to the caller, but now it does the clean-up itself.


* bs/mirbsd (2014-01-02) 1 commit
  (merged to 'next' on 2014-01-06 at d5cecbb)
 + Add MirBSD support to the build system.


* cc/replace-object-info (2013-12-30) 11 commits
  (merged to 'next' on 2014-01-03 at 4473803)
 + replace info: rename 'full' to 'long' and clarify in-code symbols
  (merged to 'next' on 2013-12-17 at aeb9e18)
 + Documentation/git-replace: describe --format option
 + builtin/replace: unset read_replace_refs
 + t6050: add tests for listing with --format
 + builtin/replace: teach listing using short, medium or full formats
 + sha1_file: perform object replacement in sha1_object_info_extended()
 + t6050: show that git cat-file --batch fails with replace objects
 + sha1_object_info_extended(): add an unsigned flags parameter
 + sha1_file.c: add lookup_replace_object_extended() to pass flags
 + replace_object: don't check read_replace_refs twice
 + rename READ_SHA1_FILE_REPLACE flag to LOOKUP_REPLACE_OBJECT

 read_sha1_file() that is the workhorse to read the contents given
 an object name honoured object replacements, but there is no
 corresponding mechanism to sha1_object_info() that is used to
 obtain the metainfo (e.g. type  size) about the object, leading
 callers to weird inconsistencies.


* jh/rlimit-nofile-fallback (2013-12-18) 1 commit
  (merged to 'next' on 2014-01-03 at b56ae0c)
 + get_max_fd_limit(): fall back to OPEN_MAX upon getrlimit/sysconf failure

 When we figure out how many file descriptors to allocate for
 keeping packfiles open, a system with non-working getrlimit() could
 cause us to die(), but because we make this call only to get a
 rough estimate of how many is available and we do not even attempt
 to use up all file descriptors available ourselves, it is nicer to
 fall back to a reasonable low value rather than dying.


* jk/credential-plug-leak (2014-01-02) 1 commit
  (merged to 'next' on 2014-01-06 at 88e29a3)
 + Revert prompt: clean up strbuf usage

 An earlier clean-up introduced an unnecessary memory leak.


* jk/http-auth-tests-robustify (2014-01-02) 1 commit
  (merged to 'next' on 2014-01-06 at 7e87bba)
 + use distinct username/password for http auth tests

 Using the same username and password during the tests would not
 catch a potential breakage of sending one when we should be sending
 the other.


* jk/oi-delta-base (2013-12-26) 2 commits
  (merged to 'next' on 2014-01-06 at 8cf3ed2)
 + cat-file: provide %(deltabase) batch format
 + sha1_object_info_extended: provide delta base sha1s

 Teach cat-file --batch to show delta-base object name for a
 packed object that is represented as a delta.


* jk/sha1write-void (2013-12-26) 1 commit
  (merged to 'next' on 2014-01-06 at d8cd8ff)
 + do not pretend sha1write returns errors

 Code clean-up.


* jk/test-framework-updates (2014-01-02) 3 commits
  (merged to 'next' on 2014-01-06 at f81f373)
 + t: drop known breakage test
 + t: simplify HARNESS_ACTIVE hack
 + t: set TEST_OUTPUT_DIRECTORY for sub-tests

 The basic test used to leave unnecessary trash directories in the
 t/ directory.


* js/lift-parent-count-limit (2013-12-27) 1 commit
  (merged to 'next' on 2014-01-06 at b74133c)
 + Remove the line length limit for graft files

 There is no reason to have a hardcoded upper limit of the number of
 parents for an octopus merge, created via the graft mechanism.


* km/gc-eperm (2014-01-02) 1 commit
  (merged to 'next' on 2014-01-06 at fe107de)
 + gc: notice gc processes run by other users

 A gc process running as a different user should be able to stop a
 new gc process from starting.


* mh/path-max (2013-12-18) 2 commits
  (merged to 'next' on 2014-01-03 at 5227c9b)
 + builtin/prune.c: use 

Re: [PATCH sb/diff-orderfile-config] diff test: reading a directory as a file need not error out

2014-01-10 Thread Junio C Hamano
Jonathan Nieder jrnie...@gmail.com writes:

 There is no guarantee that strbuf_read_file must error out for
 directories.  On some operating systems (e.g., Debian GNU/kFreeBSD
 wheezy), reading a directory gives its raw content:

   $ head -c5  / | cat -A
   ^AM-|^_^@^L$

 As a result, 'git diff -O/' succeeds instead of erroring out on
 these systems, causing t4056.5 orderfile is a directory to fail.

 On some weird OS it might even make sense to pass a directory to the
 -O option and this is not a common user mistake that needs catching.
 Remove the test.

 Signed-off-by: Jonathan Nieder jrnie...@gmail.com
 ---
 Hi,

 t4056 is failing on systems using glibc with the kernel of FreeBSD[1]:

 | expecting success: 
 | test_must_fail git diff -O/ --name-only HEAD^..HEAD
 |
 | a.h
 | b.c
 | c/Makefile
 | d.txt
 | test_must_fail: command succeeded: git diff -O/ --name-only HEAD^..HEAD
 | not ok 5 - orderfile is a directory

 How about this patch?

Sounds sensible. Thanks.



 Thanks,
 Jonathan

 [1] 
 https://buildd.debian.org/status/fetch.php?pkg=gitarch=kfreebsd-amd64ver=1%3A2.0~next.20140107-1stamp=1389379274

  t/t4056-diff-order.sh | 4 
  1 file changed, 4 deletions(-)

 diff --git a/t/t4056-diff-order.sh b/t/t4056-diff-order.sh
 index 1ddd226..9e2b29e 100755
 --- a/t/t4056-diff-order.sh
 +++ b/t/t4056-diff-order.sh
 @@ -68,10 +68,6 @@ test_expect_success POSIXPERM,SANITY 'unreadable 
 orderfile' '
   test_must_fail git diff -Ounreadable_file --name-only HEAD^..HEAD
  '
  
 -test_expect_success 'orderfile is a directory' '
 - test_must_fail git diff -O/ --name-only HEAD^..HEAD
 -'
 -
  for i in 1 2
  do
   test_expect_success orderfile using option ($i) '
--
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] t5531: further matching fixups

2014-01-10 Thread Junio C Hamano
Jeff King p...@peff.net writes:

 ... but the
 failing test is actually somewhat broken in 'next' already.

Hmph, in what way?  I haven't seen t5531 breakage on 'next', with or
without your series...

 fixes it, and should be done regardless of the other series.

  t/t5531-deep-submodule-push.sh | 1 +
  1 file changed, 1 insertion(+)

 diff --git a/t/t5531-deep-submodule-push.sh b/t/t5531-deep-submodule-push.sh
 index 8c16e04..445bb5f 100755
 --- a/t/t5531-deep-submodule-push.sh
 +++ b/t/t5531-deep-submodule-push.sh
 @@ -12,6 +12,7 @@ test_expect_success setup '
   (
   cd work 
   git init 
 + git config push.default matching 
   mkdir -p gar/bage 
   (
   cd gar/bage 
--
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 v2 2/4] t7505: ensure cleanup after hook blocks merge

2014-01-10 Thread Junio C Hamano
Matthieu Moy matthieu@grenoble-inp.fr writes:

 Ryan Biesemeyer r...@yaauie.com writes:

 +  test_when_finished git merge --abort 
 +  (
 +git checkout -B other HEAD@{1} 

 Weird indentation (space/tab mix).

Also I do not quite see why the body has to be in a subshell.
--
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 v2 3/4] merge: make prepare_to_commit responsible for write_merge_state

2014-01-10 Thread Junio C Hamano
Ryan Biesemeyer r...@yaauie.com writes:

 When merging, make the prepare-commit-msg hook have access to the merge
 state in order to make decisions about the commit message it is preparing.

What information is currently not available, and if available how
would that help the hook to formulate a better message?

I am not asking you to answer the question to me in an
e-mail response. The above is an example of a natural
question a reader of the above statement would have and
would wish the log message already answered when the reader
read it.

 Since `abort_commit` is *only* called after `write_merge_state`, and a
 successful commit *always* calls `drop_save` to clear the saved state, this
 change effectively ensures that the merge state is also available to the
 prepare-commit-msg hook and while the message is being edited.

 Signed-off-by: Ryan Biesemeyer r...@yaauie.com
 ---

OK.  The most important part is that this makes sure that these
intermediate state files never remains after the normal codepath
finishes what it does.

You seem to be only interested in prepare-commit-msg hook, but
because of your calling write_merge_state() early, these state files
will persist while we call finish() and they are also visible while
the post-merge hook is run.  While I think it may be a good thing
that the post-merge hook too can view that information, the log
message makes it sound as if it is an unintended side effect of the
change.

  builtin/merge.c|  3 ++-
  t/t7505-prepare-commit-msg-hook.sh | 21 +
  2 files changed, 23 insertions(+), 1 deletion(-)

 diff --git a/builtin/merge.c b/builtin/merge.c
 index 4941a6c..b7bfc9c 100644
 --- a/builtin/merge.c
 +++ b/builtin/merge.c
 @@ -802,7 +802,6 @@ static void abort_commit(struct commit_list *remoteheads, 
 const char *err_msg)
   error(%s, err_msg);
   fprintf(stderr,
   _(Not committing merge; use 'git commit' to complete the 
 merge.\n));
 - write_merge_state(remoteheads);
   exit(1);
  }
  
 @@ -816,6 +815,8 @@ N_(Please enter a commit message to explain why this 
 merge is necessary,\n
  static void prepare_to_commit(struct commit_list *remoteheads)
  {
   struct strbuf msg = STRBUF_INIT;
 +
 + write_merge_state(remoteheads);
   strbuf_addbuf(msg, merge_msg);
   strbuf_addch(msg, '\n');
   if (0  option_edit)
 diff --git a/t/t7505-prepare-commit-msg-hook.sh 
 b/t/t7505-prepare-commit-msg-hook.sh
 index 697ecc0..7ccf870 100755
 --- a/t/t7505-prepare-commit-msg-hook.sh
 +++ b/t/t7505-prepare-commit-msg-hook.sh
 @@ -183,4 +183,25 @@ test_expect_success 'with failing hook (merge)' '
)
  '
  
 +test_expect_success 'should have MERGE_HEAD (merge)' '
 +
 + git checkout -B other HEAD@{1} 
 + echo more  file 

Style: no SP between the redirection operator and its target, i.e.

echo more file 

 + git add file 
 + rm -f $HOOK 
 + git commit -m other 
 + git checkout - 
 + write_script $HOOK -EOF 
 + if [ -s $(git rev-parse --git-dir)/MERGE_HEAD ]; then

Style: no [], and no semicolon to join two lines of control
statement into one, i.e.

if test -s ...
then

 + exit 0
 + else
 + exit 1
 + fi
 + EOF
 + git merge other 
 + test `git log -1 --pretty=format:%s` = Merge branch '''other''' 
 

Style:

- After sh t7505-*.sh v -i fails for whatever reason, being
  able to inspect the trash directory to see what actually was
  produced is much easier way to debug than having to re-run the
  command with sh -x to peek into what the test statement is
  getting.

- $(...) is much easier to read than `...`, but you do not have
  to use either if you follow the above.

i.e.

git log -1 --format=%s actual 
echo Merge branch '\''other'\'' expect 
test_cmp expect actual 

 + test ! -s $(git rev-parse --git-dir)/MERGE_HEAD
 +
 +'
 +
  test_done
--
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 v2 3/4] merge: make prepare_to_commit responsible for write_merge_state

2014-01-10 Thread Junio C Hamano
Ryan Biesemeyer r...@yaauie.com writes:

 + write_script $HOOK -EOF 
 + if [ -s $(git rev-parse --git-dir)/MERGE_HEAD ]; then
 + exit 0
 + else
 + exit 1
 + fi
 + EOF

The script can be a one-liner

write_scirpt $HOOK -\EOF 
test -s $(git rev-parse --git-dir)/MERGE_HEAD
EOF

can't it?  I also do not think you want to have the rev-parse run
while writing the script (rather, you would want it run inside the
script, no?)

 + git merge other 
 + test `git log -1 --pretty=format:%s` = Merge branch '''other''' 
 
 + test ! -s $(git rev-parse --git-dir)/MERGE_HEAD
 +
 +'
 +
  test_done
--
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] Fix typesetting in Bugs section of 'git-rebase' man page (web version)

2014-01-10 Thread Jason St. John
On Tue, Nov 19, 2013 at 11:43 PM, Junio C Hamano gits...@pobox.com wrote:
 Jason St. John jstj...@purdue.edu writes:

 Documentation/git-rebase.txt: add a blank line after the two AsciiDoc
 listing blocks

 That looks funnily formatted, out of place and redundant.

 Without these blank lines, AsciiDoc thinks the opening - is a
 section heading and typesets the word to as such, which causes
 cascading formatting/typesetting issues until the end of the document.


 Signed-off-by: Jason St. John jstj...@purdue.edu
 ---
 You can see the carnage here:
 http://git-scm.com/docs/git-rebase#_bugs

 This fixes GitHub issue github/gitscm-next#281

 Hmph. https://git-htmldocs.googlecode.com/git/git.html has HTML
 documentation pages I preformat, but as far as I can see, the bugs
 section of git-rebase(1) does not have such a carnage.

 Perhaps git-scm.com uses some buggy formatter?

This does seem to be an issue with git-scm.com only, so this is
probably an issue with the AsciiDoc formatter they use.

What AsciiDoc formatter (and version) do you use?

Sorry for the long delay in replying!

Jason
--
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:

2014-01-10 Thread Mr. Jerry Natai
I have a business Proposal for you.You can contact me on my private email: 
(mrjerrynatai2...@manager.in.th)
--
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] t5531: further matching fixups

2014-01-10 Thread Jeff King
On Fri, Jan 10, 2014 at 03:34:59PM -0800, Junio C Hamano wrote:

 Jeff King p...@peff.net writes:
 
  ... but the
  failing test is actually somewhat broken in 'next' already.
 
 Hmph, in what way?  I haven't seen t5531 breakage on 'next', with or
 without your series...

The test still passes, but it is not testing the right thing anymore.

On 'next', run t5531. Test 6 is push fails when commit on
multiple branches if one branch has no remote and ends with:

  test_must_fail git push --recurse-submodules=check ../pub.git

But the output ends with:

  warning: push.default is unset; its implicit value has changed in
  Git 2.0 from 'matching' to 'simple'. To squelch this message
  [...]

  fatal: The current branch branch2 has no upstream branch.
  To push the current branch and set the remote as upstream, use

  git push --set-upstream ../pub.git branch2

When not merged with b2ed944 (push: switch default from matching to
simple), or with my patch to set push.default=matching explicitly, the
output is:

  The following submodule paths contain changes that can
  not be found on any remote:
gar/bage

  Please try

  git push --recurse-submodules=on-demand

  or cd to the path and use

  git push

  to push them to a remote.

  fatal: Aborting.

which is what the test is actually trying to check. So the push fails,
as we expect, but not for the right reason.

My other series for @{publish} had a bug that caused the push to
succeed. So that series was buggy (and I posted the fix already), but we
only noticed it because this test was not working (it should not care
about upstream/triangular config at all, but it accidentally did).

Does that clarify the situation?

-Peff
--
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