Re: [PATCH v5 00/27] Prepare the sequencer for the upcoming rebase -i patches

[Max Horn]

Hi Dscho,

> On 23 Oct 2016, at 11:54, Johannes Schindelin  
> wrote:
> 
> Hi Junio,
> 
> On Sat, 22 Oct 2016, Junio C Hamano wrote:
> 
[...]

>> There isn't enough time to include this topic in the upcoming
>> release within the current https://tinyurl.com/gitCal calendar,
>> however, which places the final on Nov 11th.
> 
> More is the pity.
> 
> Thank you, though, for being upfront with me. I will shift my focus to
> tasks that require my attention more urgently, then.

Junio did go on, though:

>> I am wondering if it makes sense to delay 2.11 by moving the final
>> by 4 weeks to Dec 9th.

I was reading this as an offer to delay things to accommodate the integration 
your work into 2.11. I.e. "within the current plan, there is no time for this, 
but we could adjust the plan". But maybe I am misinterpreting?


Cheers,
Max

Re: [ANNOUNCE] Git v2.8.0-rc2

[Max Horn]

> On 11 Mar 2016, at 00:04, Junio C Hamano  wrote:
> 
> A release candidate Git v2.8.0-rc2 is now available for testing
> at the usual places.  It is comprised of 459 non-merge commits
> since v2.7.0, contributed by 60 people, 19 of which are new faces.
> 
[...]

> Updates since v2.7
> --
> 
> UI, Workflows & Features
> 
> * It turns out "git clone" over rsync transport has been broken when
>   the source repository has packed references for a long time, and
>   nobody noticed nor complained about it.
> 
> * "branch --delete" has "branch -d" but "push --delete" does not.

This states a problem, but not whether (and how) it was resolved?

[...]

> * Across the transition at around Git version 2.0, the user used to
>   get a pretty loud warning when running "git push" without setting
>   push.default configuration variable.  We no longer warn, given that
>   the transition is over long time ago.

That last sentence sounds weird... perhaps "the transition was completed a long 
time ago" ? Or "the transition ended a long time ago" ?

> 
> * README has been renamed to README.md and its contents got tweaked
>   slightly to make it easier on the eyes.
> 
> 
> Performance, Internal Implementation, Development Support etc.
> 
> * Add a framework to spawn a group of processes in parallel, and use
>   it to run "git fetch --recurse-submodules" in parallel.
> 
> * A slight update to the Makefile to mark "phoney" targets
>   as such correctly.

phoney -> phony ? (it seems in british english you can write "phoney", but 
according to a quickly Google search this is rarely used in relation to 
Makefiles; no surprise, given that the corresponding syntax element is ".PHONY" 
and you are not allowed to spell it differently ;-)

[...]

> 
> * Some calls to strcpy(3) triggers a false warning from static
>   analysers that are less intelligent than humans, and reducing the
>   number of these false hits helps us notice real issues.  A few
>   calls to strcpy(3) in test-path-utils that are already safe has
>   been rewritten to avoid false wanings.
> 
> * Some calls to strcpy(3) triggers a false warning from static
>   analysers that are less intelligent than humans, and reducing the
>   number of these false hits helps us notice real issues.  A few
>   calls to strcpy(3) in "git rerere" that are already safe has been
>   rewritten to avoid false wanings.

The above two messages are very similar, only the end differs a bit. Perhaps
merge them into one? I.e.. "A few calls to strcpy(3) in test-path-utils and 
"git rerere" that [...]"

Also: wanings -> warnings

[...]

> 
> * Asking gitweb for a nonexistent commit left a warning in the server
>   log.
> 
>   Somebody may want to follow this up with an additional test, perhaps?
>   IIRC, we do test that no Perl warnings are given to the server log,
>   so this should have been caught if our test coverage were good.

That last paragraph seems odd for a changelog?

[...]

> * The underlying machinery used by "ls-files -o" and other commands
>   have been taught not to create empty submodule ref cache for a

have -> has (the machinery is singular)





Cheers,
Max--
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: Announcing git-cinnabar 0.3.0

[Max Horn]

Hi there,

> On 15 Jan 2016, at 10:25, Johannes Schindelin  
> wrote:
> 
> Hi Mike,
> 
> On Fri, 15 Jan 2016, Mike Hommey wrote:
> 
>> Git-cinnabar is a git remote helper to interact with mercurial
>> repositories. It allows to clone, pull and push from/to mercurial remote
>> repositories, using git.
> 
> Great news! I was really sad when Sverre's work on remote-hg was basically
> ignored and replaced (and he went away after that),

I have a bit of a different recollection of that story; in particular, his tool 
was quite limited, and quickly broke down when used. 

> and then the replacement was not maintained properly.

If you are talking about felipe's git-remote-hg: I am maintaining a fork of 
that, and it works quite well for me (I use it daily, and so do many other 
people):



That said, I certainly am not putting much energy into it, so I am very happy 
to see that Mike is active on his new tool :-).

Cheers,
Max

> 
> So I am personally very happy that there is a well-maintained alternative
> now.
> 
> Hopefully I will get a chance to test this soon, but I already have one
> comment: at
>> 1. https://github.com/glandium/git-cinnabar/wiki/Windows-Support
> 
> ... it is mentioned that...
> 
>   Git for Windows 32-bits is untested, but assumed to have the same
>   issue as MSys2 32-bits
> 
> ... but as is mentioned here:
> 
>   https://github.com/git-for-windows/git/wiki/32-bit-issues
> 
> Git for Windows executes the rebaseall step as part of the installation,
> so it should not have the same issue as MSys2 32-bits ;-)
> 
> Ciao,
> Dscho
> --
> 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
> 

--
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: How to combine git repos with similar code and keep all branches and tags?

[Max Horn]

Hi Jose,

On 14.04.2015, at 18:44, Jose de Leon  wrote:

> Hi All,
> 
> 
> I've got an interesting problem and the possible solutions I've found from 
> searching google don't seem to work for us.  In a nutshell, I need to combine 
> multiple git repositories into single repository and preserve all history, 
> branches and tags from each repository.
> 
> Does anybody have a solution for this, how do this kind of thing, is it even 
> possible?
> 
> For some unknown reason to me, our developers started a git project, called 
> Ver1, this was the first version.  Then sometime later, they created a new 
> git repository called Ver2, the initial commit for Ver2 was essentially a 
> copy of the code in Ver1 from the master.  They didn't clone it, they just 
> copied the code at the latest point.  This wasn't done by forking.  Then they 
> repeated this for Ver3 and Ver4, etc.  On top of all that, they have been 
> adding new code to Ver1, Ver2, etc. that has caused each to divert from the 
> other and each have their own branch and tag sets.  They have similar 
> directory structure and similar file names, but there are some slight 
> differences in structure.
> 
> Well, this has become unmanageable and now we want to combine them into one 
> single git repository.   
> 
> Logically, these are the same project but at different versions, basically, 
> Ver1 and Ver2, etc, should be simply branches at different times if these 
> were combined into a single repository.

Here is one possible way to go about this using grafts (I used something
similar in the past);

1) Get all the data into a single git repository.

  Since everything is already in a git repositories, you could e.g. create a
  clone of Ver1; then add remotes for Ver2 ... VerN, and fetch them all,
  along with tags. If there are conflicts between branch or tag names, deal
  with them at this point.

2) Create graft points to tie the history together.

   Identify the commit in Ver1 at which Ver2 branched off. Then, graft that
   as parent for the initial commits of Ver2. See here for some basic
   instructions  or feel
   free to ask for details (or Google, or... :)

   Repeat vor Ver2+Ver3, Ver3+Ver4, etc.

3) Finally, you can get rid of the graft points, and turn everything into a
"proper" history, by running "git filter-branch". Something like

  git filter-branch -- --all

ought to do it, but I might be forgetting something (I am sure somebody will
correct me soon in that case, though ;-). Best to have a look at
 for yourself, though.


This all is under the assumption that you want to stay as close to how
things really were (usually a good idea). But sometimes it may be desirable
to make further adjustments. E.g. you may wish to adjust committer names,
rearrange some stuff (though usually git is quite good at doing the right
thing automatically, etc. How to do that of course depends on what exactly
you want to do, but in many cases, filter-branch is your friend.

Hope that helps!

Max

--
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] doc: add some crossrefs between manual pages

[Max Horn]

On 11.11.2014, at 23:51, Junio C Hamano  wrote:

> Max Horn  writes:
> 
>> I did this because I was browsing the remote helper docs online quite a bit,
>> and was wishing for some more direct links between the pages. While I can
>> manyally edit the URL, it seems logical to offer these links directly.
> 
>> diff --git a/Documentation/git-fast-export.txt 
>> b/Documentation/git-fast-export.txt
>> ...
>> +linkgit:git-fast-import[1]
> 
>> diff --git a/Documentation/git-fast-import.txt 
>> b/Documentation/git-fast-import.txt
>> ...
>> +linkgit:git-fast-export[1]
> 
> Makes sense to have these pair refer to each other.
> 
>> diff --git a/Documentation/git-remote-ext.txt 
>> b/Documentation/git-remote-ext.txt
>> ...
>> +linkgit:gitremote-helpers[1]
> 
>> diff --git a/Documentation/git-remote-fd.txt 
>> b/Documentation/git-remote-fd.txt
>> +linkgit:gitremote-helpers[1]
> 
> Likewise.  git-remote-* are instances of gitremote-helpers.
> 
>> diff --git a/Documentation/gitremote-helpers.txt 
>> b/Documentation/gitremote-helpers.txt
>> index 64f7ad2..8edf72c 100644
>> --- a/Documentation/gitremote-helpers.txt
>> +++ b/Documentation/gitremote-helpers.txt
>> @@ -452,8 +452,14 @@ SEE ALSO
>> 
>> linkgit:git-remote[1]
>> 
>> +linkgit:git-remote-ext[1]
>> +
>> +linkgit:git-remote-fd[1]
>> +
>> linkgit:git-remote-testgit[1]
> 
> Makes sense.
> 
>> +linkgit:git-fast-import[1]
> 
> This looks somewhat out of place; fast-import is not the only or
> even the primary way to do a remote-helper, is it?

It depends on how you look at it, I'd say. If you write a remote-helper that
uses the import/export feature, it is absolutely vital.  All remote helpers
I ever worked on are of that kind, so to me it is the primary way ;-),
although of course I realize there are others. So, how would you determine
which of the various methods is the "primary" one?

In fact, this single link is the one that motivated me to write the whole
patch; all the others were afterthoughts ;-).

To elaborate on that: In the past, I did some work on various import/export
remote-helpers; and I recall wishing for this precise link several times.
More recently, I worked on some tweaks and fixes for Felipe's git-remote-hg.
Whenever doing that, the place in the docs I start to refresh my memory on
how remote helpers work is gitremote-helpers. But then at some point I
realize "ah wait, *that* particular bit is actually part of the "fastimport"
protocol". So I need to look that up. And again and again thought "dang, why
isn't there a hyperlink for that here". Fact is, I need both man pages to
understand what's going on.

Now, clearly, I can live without that link. But I feel that there is a clear
connection.  And if you say it doesn't belong here because it is only
relevant for one of multiple ways to do a remote-helper, then shouldn't one
drop the links to git-remote-ext etc., too? After all, they are only
examples for one of multiple ways, too...

In other words, I find it arbitrary to exclude one link that is useful for
some, but not all remote-helper authors, while adding some other links that
are also useful for some, but not all remote-helper authors...

That said, I certainly don't plan to hold that patch hostage to this one
line. :-)


Cheers,
Max--
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] doc: add some crossrefs between manual pages

[Max Horn]

In particular, git-fast-import and -export link to each
other, and gitremote-helpers links to existing remote
helpers, and vice versa. Also link to fast-import from the
remote helper spec, as this is relevant for remote helpers
using the fast-import format.

Signed-off-by: Max Horn 
---
I did this because I was browsing the remote helper docs online quite a bit,
and was wishing for some more direct links between the pages. While I can
manyally edit the URL, it seems logical to offer these links directly.

 Documentation/git-fast-export.txt   | 4 
 Documentation/git-fast-import.txt   | 4 
 Documentation/git-remote-ext.txt| 5 -
 Documentation/git-remote-fd.txt | 4 
 Documentation/gitremote-helpers.txt | 6 ++
 5 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-fast-export.txt 
b/Documentation/git-fast-export.txt
index 221506b..769689d 100644
--- a/Documentation/git-fast-export.txt
+++ b/Documentation/git-fast-export.txt
@@ -148,6 +148,10 @@ Since 'git fast-import' cannot tag trees, you will not be
 able to export the linux.git repository completely, as it contains
 a tag referencing a tree instead of a commit.
 
+SEE ALSO
+
+linkgit:git-fast-import[1]
+
 GIT
 ---
 Part of the linkgit:git[1] suite
diff --git a/Documentation/git-fast-import.txt 
b/Documentation/git-fast-import.txt
index 377eeaa..f71fb01 100644
--- a/Documentation/git-fast-import.txt
+++ b/Documentation/git-fast-import.txt
@@ -1441,6 +1441,10 @@ operator can use this facility to peek at the objects 
and refs from an
 import in progress, at the cost of some added running time and worse
 compression.
 
+SEE ALSO
+
+linkgit:git-fast-export[1]
+
 GIT
 ---
 Part of the linkgit:git[1] suite
diff --git a/Documentation/git-remote-ext.txt b/Documentation/git-remote-ext.txt
index cd0bb77..74817b0 100644
--- a/Documentation/git-remote-ext.txt
+++ b/Documentation/git-remote-ext.txt
@@ -72,7 +72,6 @@ GIT_EXT_SERVICE_NOPREFIX::
Set to long name (upload-pack, etc...) of service helper needs
to invoke.
 
-
 EXAMPLES:
 -
 This remote helper is transparently used by Git when
@@ -116,6 +115,10 @@ begins with `ext::`.  Examples:
determined by the helper using environment variables (see
above).
 
+SEE ALSO
+
+linkgit:gitremote-helpers[1]
+
 GIT
 ---
 Part of the linkgit:git[1] suite
diff --git a/Documentation/git-remote-fd.txt b/Documentation/git-remote-fd.txt
index bcd3766..e700baf 100644
--- a/Documentation/git-remote-fd.txt
+++ b/Documentation/git-remote-fd.txt
@@ -50,6 +50,10 @@ EXAMPLES
 `git push fd::7,8/bar master`::
Same as above.
 
+SEE ALSO
+
+linkgit:gitremote-helpers[1]
+
 GIT
 ---
 Part of the linkgit:git[1] suite
diff --git a/Documentation/gitremote-helpers.txt 
b/Documentation/gitremote-helpers.txt
index 64f7ad2..8edf72c 100644
--- a/Documentation/gitremote-helpers.txt
+++ b/Documentation/gitremote-helpers.txt
@@ -452,8 +452,14 @@ SEE ALSO
 
 linkgit:git-remote[1]
 
+linkgit:git-remote-ext[1]
+
+linkgit:git-remote-fd[1]
+
 linkgit:git-remote-testgit[1]
 
+linkgit:git-fast-import[1]
+
 GIT
 ---
 Part of the linkgit:git[1] suite
-- 
2.1.3.dirty

--
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/3] remote-helpers: move out of contrib

[Max Horn]

Hi,

I am going to step away from this painful discussion and also this mailing
list, but I owed it to send one last reply with two of the problems I am
still seeing, simply in the hope that this will benefit some future
git-remote-hg users, but also to dispel any claims I was "hoarding" bugs to
somehow hurt Felipe.

Beyond that, I refuse to further "discuss" with Felipe on anything. It leads
nowhere, and he is so full of himself that it seems impossible to reason
with him. I will also refrain from countering everything he said, even
though I am sure he'll construe this as me admitting that he is right. I
don't care enough to try to keep up the flames *shrug*.  In the end,
everybody here can interpret this in whichever way they like.

Ah well, OK, I can't resist one last retort to one point Felipe wrote:

On 24.04.2014, at 02:23, Felipe Contreras  wrote:

> Max Horn wrote:
[...]
> 
>> Out of curiosity: How do you yourself use git-remote-hg in your daily work?
> 
> I don't.

So you don't eat your own dog-food (at most you sometimes snack on it)?
That would explain a lot...



Now to the issues: The following results are based on git "next" at revision
e8486314780a4. In addition, I cobbled together fixes from Felipe's
repository, namely the commit [1] he claimed to have fixed the multi head
issue I mentioned, as well as the changes from the fc/remote/fixes branch on
his github repository [2].

It is very well possible that this is still not the latest and greatest, and
that I missed some important patch that changes everything -- it's hard to
tell given the multitude of branches in Felipe's repository. Anyway, to
avoid confusion, I put my merged version of the script into a Gist [3], so
if I made a mistake there, it should be easy to discover.

Given that, the following script (which Felipe already knows from his
issue tracker [4]) still causes a fast-import crash after which "git pull"
from the remote hg repository is impossible, and the user has no idea
how to recover.

-- 8< --
#!/bin/sh -ex
rm -rf hgrepo gitrepo

# Create a multi-head repository
hg init hgrepo
cd hgrepo
echo a > a && hg add a && hg commit -m a
echo b > b && hg add b && hg commit -m b
hg update 0
echo c > c && hg add c && hg commit -m c
cd ..

# Clone it via remote-hg
git clone "hg::hgrepo" gitrepo

cd gitrepo
git gc --prune=now
git pull  # error
-- 8< --

Which results in this:

WARNING: Branch 'default' has more than one head, consider merging
fatal: object not found: 61780798228d17af2d34fce4cfbdf35556832472
fast-import: dumping crash report to .git/fast_import_crash_78219
fatal: Error while running fast-import

Any subsequent pulls then give something like this:

WARNING: Branch 'default' has more than one head, consider merging
fatal: mark :6 not declared
fast-import: dumping crash report to .git/fast_import_crash_78834
fatal: Error while running fast-import


What happens here is that a hg branch with two heads is created; this
repository is cloned via git-remote-hg. Both heads of the hg branch are
imported to git, but only one is referenced by a git ref. We then prune, and
end up with a "missing" commit that is still referenced by the marks file.

Note that the "next" version I used has fc/transport-helper-sync-error-fix,
but this did not stop "git fast-import" from trashing the marks file. :-(



The second script is similar but uses a closed branch to trigger essentially
the same issue. Background: closed branches are ignored by git-remote-hg,
meaning that no git ref is created for them, which can again lead to a
commit without a git ref but referenced by the marks file(s). However,
reproducing the bug in this case is a bit more complicated, because the
problem is obscured by another bug (?): Namely, if a hg branch is closed,
then git-remote-hg starts ignoring it, but does not seem to remove the ref
created for that branch.  So, the git user will not see that the remote
branch was closed (which is a bug, I'd say); on the upside, since the ref is
still around, no dangling commit is produced.

But we can "work around" this by re-opening the hg branch at a different
position, i.e. as child of an unrelated commit, which does *not* have the
commits of the original branch as parents. If we do that, git-remote-hg
moves the git ref to point to the new branch tip, and again we end up with a
dangling commit (the git commit corresponding to the former hg branch tip).

Again, this is something me and also colleagues "discovered" in real life
usage. So don't be fooled by the somewhat convoluted test script. This *does*
happen.

-- 8< ---
#!/bin/sh -ex

rm -rf hgrepo gitrepo

# Create a repository with two branches
hg init hgrepo
cd hgrepo
echo a > a && hg add a && hg commit

Re: [PATCH v2 2/3] remote-helpers: move out of contrib

[Max Horn]

On 23.04.2014, at 22:54, Felipe Contreras  wrote:

> Max Horn wrote:
>> On 21.04.2014, at 22:37, Felipe Contreras  wrote:
>> 
>>> The remote-helpers in contrib/remote-helpers have proved to work, be
>>> reliable, and stable. It's time to move them out of contrib, and be
>>> distributed by default.
>> 
>> Really? While I agree that git-remote-hg by now works quite well for basic
>> usage in simple situation, there are still unresolved bugs and fundamental
>> issues with it.
> 
> s/basic usage in simple situation/complex usage in the vast majority of 
> situations/

Yeah, hm, no. We can agree to disagree, I guess. It might also depend on
what you call "basic" or "complex" usage...

For example, whenever I need to
- close a branch
- fix a branch with multiple heads
- deal with phases
I need to switch to hg. I am pretty sure there are more things that make
that necessary, but luckily they don't happen to me.

What does work, though (and what I count as basic usage, although I'd say it
probably is enough for 95% of people out there) is making a clone of "safe"
repository (i.e. no "bad" branch names like 'foo' together with 'foo/bar'),
push and pull with it; and if you are careful, you can even get branch
attribution right. This is great, but in day-to-day usage, I still regularly
need to work with a hg clone for some tasks. But I am not really complaining
about that: For most of my regular "developer" work, I can stay in git, and
that makes me happy.

Out of curiosity: How do you yourself use git-remote-hg in your daily work?
Many people I encountered are happy enough with the ability to quickly
clone a hg repository, prepare a fix / feature branch and then submit
it back to upstream. For this, git-remote-hg *usually* is good enough.
But I am worried about people hitting the edge cases where it does not
quite work -- and then people are lost. This is what concerns me -- and
this concern would be alleviate if there was a list of known things that
do not work (and perhaps cannot work, at least for now, due to fundamental
differences between hg and git which need major work to bridge over).

Anyway, despite my criticism, I'd like to emphasis that I am actually quite
happy and grateful that your git-remote-hg exists and that you continue
improving it and the surrounding infrastructure. I just wish you could do it
while not acting like an asshole most of the time, but I'll survive that,
too *shrug*.



> 
>> E.g. I recently showed you a reproducible use case involving git-remote-hg
>> that puts the helper into a broken state from which it is difficult for a
>> normal user to recover. Namely when a hg branch has multiple heads, then
>> git-remote-hg exports all of those to git, but only adds a git ref for one of
>> them; after pruning unreferenced commits, the fast-import marks file
>> references git commits that now are missing, prompting git fast-import to
>> crash and trash the marks file. Afterwards, attempts to push or pull from the
>> remote hg repository are answered with an error.
> 
> Yes, and how often does that happen? A normal user would only see this if a
> branch remains with multiple heads in Mercurial for more than one month or so.

There are projects who do exactly that, although I believe most of them use
bookmarks, so the issue should indeed not affect those. Anyway, they do the
wrong thing ;-). Still, if you are forced with such a repository, it's not
very helpful to be told that this is your own fault...

But this kind of issue also happens in any other scenario were heads are not
mapped to a git reference. At the very least, it also happens for closed hg
branches. These are quite common, and I also run into that in real life.

[And to reply to a claim you made in another mail: No, I am not deliberately
"hoarding" issues to make you look bad. But analyzing a breakage you run into
and then properly writing it up takes time; and when you know you will likely
be insulted when reporting it doesn't really help to motivate me to sit
down and do that...]


> 
> In practice that's very unlikely, and proof of that is that nobody has 
> reported
> such issues.

No, that logic is flawed. For example, It could also mean that not many
people are using the tool, and of those not many bother to report issues via
your github bug tracker.

> 
> Either way, I just fixed it [1].

That's great to hear, thanks!

> 
>> There are more issues related to unresolved clashes between the git and hg
>> ways of naming things. E.g. I am collaborating on a hg repository that has
>> branches "foo" and "foo/bar" which git-remote-hg cannot handle because it
>> translates them to git branch names, and, well,

Re: [PATCH v2 2/3] remote-helpers: move out of contrib

[Max Horn]

On 21.04.2014, at 22:37, Felipe Contreras  wrote:

> The remote-helpers in contrib/remote-helpers have proved to work, be
> reliable, and stable. It's time to move them out of contrib, and be
> distributed by default.

Really? While I agree that git-remote-hg by now works quite well for basic 
usage in simple situation, there are still unresolved bugs and fundamental 
issues with it.

E.g. I recently showed you a reproducible use case involving git-remote-hg that 
puts the helper into a broken state from which it is difficult for a normal 
user to recover. Namely when a hg branch has multiple heads, then git-remote-hg 
exports all of those to git, but only adds a git ref for one of them; after 
pruning unreferenced commits, the fast-import marks file references git commits 
that now are missing, prompting git fast-import to crash and trash the marks 
file. Afterwards, attempts to push or pull from the remote hg repository are 
answered with an error.

There are other ways to trigger variants of this, and these are not artificial, 
they occur in real life (this is how I run into the issue). There are more 
issues related to unresolved clashes between the git and hg ways of naming 
things. E.g. I am collaborating on a hg repository that has branches "foo" and 
"foo/bar" which git-remote-hg cannot handle because it translates them to git 
branch names, and, well, git cannot handle that.

It may be hard to deal with some of them, and admittedly I wouldn't necessarily 
expect that all of these are handled from the outset, i.e. "in version 1.0". 
But I think at the very least, users should be warned about these things.

More broadly speaking, there is currently no documentation at all in git.git 
for those remote helpers, which I find worrisome.

That said, I don't know what the criteria are for moving something out of 
contrib. Perhaps it is OK to move an undocumented remote-helper with known bugs 
out of contrib.


Cheers,
Max


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: Our official home page and logo for the Git project

[Max Horn]

On 11.04.2014, at 20:56, Felipe Contreras  wrote:

> Max Horn wrote:
>> On 11.04.2014, at 17:21, Felipe Contreras  wrote:
>>> Max Horn wrote:
>>>> On 11.04.2014, at 15:29, Felipe Contreras  
>>>> wrote:
>>>>> Max Horn wrote:
>>>>> 
>>>>> You don't think red represent an oldness in Git? Whereas green
>>>>> represents progress?
>>>> 
>>>> No, I don't think that.
>>> 
>>> Then you belong to the minority of Git users. Those of us that see
>>> patches day and night, red is old, green is new.
>> 
>> Hasty generalization.
> 
> You don't know what a hasty generalization is.

That is another hasty generalization...

> If you want me to explain it to
> you, send me a personal e-mail, you are polluting the discussion enough as it
> is.

... and that is pure hubris and arrogance. :-)


>> Come back when you have facts, as opposed to the illusion that you are the
>> spokesperson of the (apparently silent) majority of Git users.
> 
> Facts:
> 
> 1) A hunk that removed (-) is represented in red [1]
> 2) A hunk that added (+) is represented in green [1]
> 3) A file that is removed is represented in red [2]
> 4) A file that is added or modified is represented in green [2]
> 5) A test that fails is represented in red [3]
> 6) A test that succeeds is represented in green [3]
> 7) The current Git logo (accordo to some people) has "-" in red, "+" in green 
> [4]

I do not dispute any of that.

> 
> Given these facts, it's reasonable to assume that to the majority of Git users
> red is old and bad, green is new and good.

This is where you are making the hasty generalization. Your facts do not 
suffice to prove this conclusion. 

And even if the conclusion is true (which is possible despite your flawed 
argument, although I doubt it), then you are making another implicit 
assumption: Namely that people will automatically transfer the red/green 
principle from diffs and test results to logos. 


Look, it's exactly this kind of non-sense pseudo-rationalization that leads big 
companies to follow what "market researchers" tell them they absolutely must do 
to make their customers happy, and then fail big with it because emotional 
stuff like that doesn't work with pure logic.

If you want to know what Git users think about the various logo variants, ask 
them *exactly that*. Indeed, that might be a helpful contribution.

But do not ask them something else, and then pretend you can deduce from that 
what they will think about the logo. And in particular, please stop claiming 
that you don't even have to ask them that, because you already supposedly know 
-- you somehow being representative of the majority of Git users, while 
everybody who disagrees with you automatically is in the minority. You can do 
that if you are e.g. leader of North Korea, but nobody here is buying that.



Max


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: Our official home page and logo for the Git project

[Max Horn]

> 

On 11.04.2014, at 17:21, Felipe Contreras  wrote:

> Max Horn wrote:
>> On 11.04.2014, at 15:29, Felipe Contreras  wrote:
>>> Max Horn wrote:
>>> 
>>> You don't think red represent an oldness in Git? Whereas green
>>> represents progress?
>> 
>> No, I don't think that.
> 
> Then you belong to the minority of Git users. Those of us that see
> patches day and night, red is old, green is new.

Hasty generalization. Come back when you have facts, as opposed to the illusion 
that you are the spokesperson of the (apparently silent) majority of Git users.


Max


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: Our official home page and logo for the Git project

[Max Horn]

On 11.04.2014, at 17:39, Philippe Vaucher  wrote:

>>> You don't think red represent an oldness in Git? Whereas green
>>> represents progress?
>> 
>> No, I don't think that.
>> 
>> Perhaps you think that, but if that is the case, it is based on your own 
>> sociocultural background. Hey, and let's not forget that supposedly 8% or so 
>> of all males are red-green blind... ;-)
> 
> 
> FWIW, I think if you made a poll and asked which color is the most
> "positive" between green and red, the vast majority of people would
> say "green". Examples could be traffic green lights vs red lights, or
> that in nature quiet & peaceful usually involves green while
> danger/action involves red (tree leafs vs blood).

This is worthless, unless you (a) actually make the poll, instead of claiming 
to know its outcome, and  (b) you establish that what people answer when asked 
about the colors red and green implies what they think about the git logo on 
git-scm.com rendered in either color...

If you really want to conduct a poll, though, I think it would be more useful 
if you e.g. asked people to order several logo candidates / variants by 
preference, and/or asked them what "feelings" each logo evokes with them, etc. 
-- i.e. ask them about actual logos, as opposed to asking about something else 
(like colors) and then extrapolating without a scientific basis.


Max


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: Silly time stamps

[Max Horn]

On 11.04.2014, at 15:19, Holger Hellmuth  wrote:

> Am 11.04.2014 13:08, schrieb Michael Haggerty:
>> On 04/09/2014 10:50 PM, Mahmoud Asshole wrote:
>>> [...]
>> 
>> Please conduct your discussions here in a civil tone.  It is both more
>> pleasant for all involved and also more likely to elicit a response.  I
>> hardly think that the "waste" of 12 bytes in every commit is an act of
>> stupidity so inexcusable that it would deserve your bile, even *if* one
>> were to agree that this information is useless (which I personally don't
>> think).
> 
> I would guess he is more concerned about the unnecessary disclosure of 
> information that could be used to track or (together with other data) 
> identify you.

Perhaps, though I fully agree with Michael that calling people "assholes" 
because their opinion differs from yours is not increasing the chances that 
anybody will listen to you, or bother to try and understand your problem.

> 
> Since the reasons to include it seem to be specifically to know more about 
> the comitter this seems to me the typical conflict between privacy and 
> security.
> 


More between "privacy" (or perhaps "personal safety"? think: dissident coder?)  
vs. "feature that is useful to some people".

If one is truly concerned about spilling timezone information, how about this:

  alias git='TZ=0 git'

Seems to work for me, at least in bash.


Cheers,
Max


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: Our official home page and logo for the Git project

[Max Horn]

On 11.04.2014, at 15:29, Felipe Contreras  wrote:

> Max Horn wrote:
>> As for the logo, I think it's nice and simple,
> 
> You don't think red represent an oldness in Git? Whereas green
> represents progress?

No, I don't think that.

Perhaps you think that, but if that is the case, it is based on your own 
sociocultural background. Hey, and let's not forget that supposedly 8% or so of 
all males are red-green blind... ;-)

> 
>> and based on experience I think that for every logo you'll find people
>> who object to it.
> 
> So we should just accept any logo without thinking about it?

No. You (well, everybody) should just take a deep breath, step back, and ask 
yourself "Does this really matter that much to me and the rest of the world? Is 
it worth keeping up another long drawn discussion? Is there perhaps a chance 
for a compromise?"

Of course it is completely up to each individual to decide this! Power to you!

In the meantime, I'll watch from the sidelines, eat my popcorn, enjoy the show, 
and keep on not using a git logo for anything, indefinitely :-).


>> E.g. the red color of the log on git-scm.com looks great to me, while
>> I dislike e.g. the color variation Felipe is using.
> 
> If you don't like my variation that doesn't mean we should accept the
> red one;

Of course! That's why I marked it as only being an example.

> there are many shades of green to begin with.

Indeed. And many shades of red, blue, etc., and let's not forget about stripes, 
dots, and other patterns. So many possibilities! Oh, and can I get mine with 
ponies? :-)


> 
> Also, there's more than the color to think about; look at the order of
> the pictured commits; they don't make any sense.

I disagree. And I think you again confuse your personal sociocultural 
conditioning with an universal truth. 

In closing, let's not forget that for some things, there just is no "correct" 
solution, and I think the choice of a logo is one of these cases.


Cheers,
Max


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: Our official home page and logo for the Git project

[Max Horn]

My two cents: I like git-scm.com quite a bit. As for the logo, I think it's 
nice and simple, and based on experience I think that for every logo you'll 
find people who object to it. E.g. the red color of the log on git-scm.com 
looks great to me, while I dislike e.g. the color variation Felipe is using.

While we are at it, can I please get that bike-shed in turquoise with a hint of 
ocean blue mixed in?

Max


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: [PATCH] Add support for commit attributes

[Max Horn]

On 10.04.2014, at 19:28, Felipe Contreras  wrote:

> Diego Lago wrote:
>> Commit attributes are custom commit extra headers the user can
>> add to the commit object.
>> 
>> The motivation for this patch is that in my company we have a custom
>> continuous integration software that uses a custom formatted commit
>> message (currently in YALM format) to show several information into
>> our CI server front-end.
> 
> These attributes can be used for remote-helpers as well; to store extra
> information that cannot be stored otherwise in Git's data structures.

+1 to that. This is reminds me of what Kiln Harmony does as part of their 
effort to enable full round-robin transfer between Git and Mercurial (a goal 
from which all other tools I know of still are far away). See 
.



Cheers,
Max


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: fast-import deltas

[Max Horn]

On 01.04.2014, at 15:15, Jeff King  wrote:

> On Tue, Apr 01, 2014 at 10:07:03PM +0900, Mike Hommey wrote:
> 
>>> For my own curiosity, how does this differ from what is in
>>> contrib/remote-helpers/git-remote-hg?
>> 
>> contrib/remote-helpers/git-remote-hg does a local mercurial clone before
>> doing the git conversion. While this is not really a problem for most
>> mercurial projects, it tends to be slow with big ones, like the firefox
>> source code. What I'm aiming at is something that can talk directly to a
>> remote mercurial server.
> 
> Ah, that makes sense. Thanks for explaining.


Hm, myself, I am not quite convinced. Yes, there is an overhead, but it is 
one-time (well, the space overhead is not, but Mike only mentioned time, not 
space). I wonder if it is really worth the effort to start yet another project 
on this... Moreover, I don't see a fundamental reason why one could not modify 
git-remote-hg to work this way. At least optionally - myself, I would strongly 
prefer the current way, as translating between git and hg 100% round trip clean 
is provably impossible [1].

Thing is, there are by now more than half a dozen projects of this kind. In my 
impression, all do the low hanging fruit, some go slightly beyond that, but 
*none* solves all the tough parts and itty-gritty details...

Just to mention a few of the problems that are usually ignored, even though 
they have real world impact:

- the concept of Mercurial branches has no counterpart in git, making all kinds 
of translations hard. As a consequence, many translators ignore hg branches 
completely (e.g. hg-git -- at least it used to do that, not sure whether that 
changed) or handle them only partially (e.g. 
contrib/remote-helpers/git-remote-hg: It does not deal with multiple heads or 
with closed branches)
(this can cause sever issues with git-remote-hg, by the way, with dangling 
refs, which, when pruned by an auto-gc, can wipe your fast-import marks file, 
causing major pain...)

- in the other direction, git branches most closely correspond to hg bookmarks. 
But what if a hg repository has both a branch "foo" and a bookmark "foo"? 
git-remote-hg partially deals with that (by mapping the hg bookmark "foo" to 
the git branch "foo", and mapping the hg branch "foo" to the git branch 
"branches/foo"), but this still has issues (besides being annoying for users, 
it clearly still not avoids ref name conflicts)

- git and hg also allow different characters sets in branch and bookmark names

- in hg you can simultaneously have things called "foo" and "foo/bar". In git, 
you can't.

There is plenty more. Of course, some of this might just be impossible [1] to 
handle nicely. But I find it kind of sad that everybody seems to prefer to 
start yet another solution, then leave it as 80%, instead of trying to improve 
upon existing work :-(.

By the way, to get back to the speed bottleneck: We found that by far the 
slowest part in importing large repositories like the Mozilla one was not the 
initial cloning of the hg repository (althoug that could sometimes take ages) 
but rather an unfortunate mismatch between the hg and git storage approach. 
When creating a fast-import stream, the normal way to go about that is to 
import things commit by commit. But if you do that, then extracting file data 
from Mercurial and its revlog data format easily can degenerate into the worst 
case quadratic runtime :-/. Now, if one know that one is going to import the 
whole repository anyway, one could do better by first exporting all file 
revisions, generating many blobs and their marks, and keeping these in memory, 
*then* exporting the commits, reverting to these blob marks. 

However, this stops being a great idea once you are working in incremental 
mode. That said, it certainly would make sense to investigate this possibility 
(regardless of whether one uses a local hg clone or directly talks to the 
remote repository); at least in theory, even if one only uses this approach 
during the initial import, it should be a strict improvement over the current 
situation.

In closing, I should mention that the problems caused by translating between hg 
and git concepts are by far not the only ones; the fast-import interface itself 
still has limitations that make some things annoying. E.g. when a remote is 
renamed, the remote handler does not know that, which can lead to awkward 
situations that right now may require some trickery to resolve correctly, if it 
is possible at all. Or if a user manually removed a commit that a remote-helper 
previously referenced in a marks file, and that remote helper than uses that 
marks file, fast-import just dies, complaining about the invalid mark. As a 
result, every proper remote helper basically would need to fully parse and 
verify those marks files, detect "broken" marks, and deal with that -- there is 
no way to benefit from the existing mark verification code in fast-import right 
now.

Please don't get me wrong.

Re: What's cooking in git.git (Mar 2014, #07; Fri, 28)

[Max Horn]

On 28.03.2014, at 23:21, Junio C Hamano  wrote:

[...]

> * ap/remote-hg-skip-null-bookmarks (2014-03-25) 1 commit
>  (merged to 'next' on 2014-03-25 at a8cd922)
> + remote-hg: do not fail on invalid bookmarks
> 
> Will merge to 'master'.


Just got back and had a chance to look at the patch you queued.
It looks good to me. Considering it is already scheduled to go
into master, I'll assume no further work from me on this is needed.
If I am wrong on that, please let me know.

Max


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: [PATCH v3] remote-hg: do not fail on invalid bookmarks

[Max Horn]

Hi Torsten,

On 21.03.2014, at 21:47, Torsten Bögershausen  wrote:

> On 2014-03-21 12.36, Max Horn wrote:
> All tests passed :-),

Excellent.

> thanks from my side.
> comments inline, some are debatable

Thanks for having a close look and for the constructive feedback!
Unfortunately, I won't have time to look into this for the next 7 days
or so. I wouldn't mind if the patch gets queued with the changes you
suggest; but of course that might be a tad too much to ask for, so I'll
also be happy to do a "proper" re-roll, but then it has to wait a bit.

Cheers,
Max



signature.asc
Description: Message signed with OpenPGP using GPGMail

[PATCH v3] remote-hg: do not fail on invalid bookmarks

[Max Horn]

Mercurial can have bookmarks pointing to "nullid" (the empty root
revision), while Git can not have references to it. When cloning or
fetching from a Mercurial repository that has such a bookmark, the
import failed because git-remote-hg was not be able to create the
corresponding reference.

Warn the user about the invalid reference, and do not advertise these
bookmarks as head refs, but otherwise continue the import. In
particular, we still keep track of the fact that the remote repository
has a bookmark of the given name, in case the user wants to modify that
bookmark.

Also add some test cases for this issue.

Reported-by: Antoine Pelisse 
Signed-off-by: Max Horn 
---
This is a different fix than in my previous attempts. I thought
a bit more about the issue, and determined that the previous fix,
while working, was not really correct: It is wrong to
treat nullid bookmarks as if they are non-existent; if e.g.
the user wants to modify the bookmark from git, we need to
into account that the remote already has a bookmark with that name.
Indeed, I extended the new test cases to cover this aspect.
With the previous fix, the new tests would fail upon pushing,
with the new one, they work.

 contrib/remote-helpers/git-remote-hg |  5 ++-
 contrib/remote-helpers/test-hg.sh| 67 
 2 files changed, 71 insertions(+), 1 deletion(-)

diff --git a/contrib/remote-helpers/git-remote-hg 
b/contrib/remote-helpers/git-remote-hg
index eb89ef6..36b5261 100755
--- a/contrib/remote-helpers/git-remote-hg
+++ b/contrib/remote-helpers/git-remote-hg
@@ -643,7 +643,10 @@ def do_list(parser):
 print "? refs/heads/branches/%s" % gitref(branch)
 
 for bmark in bmarks:
-print "? refs/heads/%s" % gitref(bmark)
+if  bmarks[bmark].hex() == '':
+warn("Ignoring invalid bookmark '%s'", bmark)
+else:
+print "? refs/heads/%s" % gitref(bmark)
 
 for tag, node in repo.tagslist():
 if tag == 'tip':
diff --git a/contrib/remote-helpers/test-hg.sh 
b/contrib/remote-helpers/test-hg.sh
index a933b1e..f5d0d97 100755
--- a/contrib/remote-helpers/test-hg.sh
+++ b/contrib/remote-helpers/test-hg.sh
@@ -772,4 +772,71 @@ test_expect_success 'remote double failed push' '
)
 '
 
+test_expect_success 'clone remote with master null bookmark, then push to the 
bookmark' '
+   test_when_finished "rm -rf gitrepo* hgrepo*" &&
+
+   (
+   hg init hgrepo &&
+   cd hgrepo &&
+   echo a >a &&
+   hg add a &&
+   hg commit -m a &&
+   hg bookmark -r null master
+   ) &&
+
+   git clone "hg::hgrepo" gitrepo &&
+   check gitrepo HEAD a &&
+   cd gitrepo &&
+   git checkout --quiet -b master &&
+   echo b >b &&
+   git add b &&
+   git commit -m b &&
+   git push origin master
+'
+
+test_expect_success 'clone remote with default null bookmark, then push to the 
bookmark' '
+   test_when_finished "rm -rf gitrepo* hgrepo*" &&
+
+   (
+   hg init hgrepo &&
+   cd hgrepo &&
+   echo a >a &&
+   hg add a &&
+   hg commit -m a &&
+   hg bookmark -r null -f default
+   ) &&
+
+   git clone "hg::hgrepo" gitrepo &&
+   check gitrepo HEAD a &&
+   cd gitrepo &&
+   git checkout --quiet -b default &&
+   echo b >b &&
+   git add b &&
+   git commit -m b &&
+   git push origin default
+'
+
+test_expect_success 'clone remote with generic null bookmark, then push to the 
bookmark' '
+   test_when_finished "rm -rf gitrepo* hgrepo*" &&
+
+   (
+   hg init hgrepo &&
+   cd hgrepo &&
+   echo a >a &&
+   hg add a &&
+   hg commit -m a &&
+   hg bookmark -r null bmark
+   ) &&
+
+   git clone "hg::hgrepo" gitrepo &&
+   check gitrepo HEAD a &&
+   cd gitrepo &&
+   git checkout --quiet -b bmark &&
+   git remote -v &&
+   echo b >b &&
+   git add b &&
+   git commit -m b &&
+   git push origin bmark
+'
+
 test_done
-- 
1.9.0.7.ga299b13


--
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 v2] remote-hg: do not fail on invalid bookmarks

[Max Horn]

Mercurial can have bookmarks pointing to "nullid" (the empty root
revision), while Git can not have references to it.
When cloning or fetching from a Mercurial repository that has such a
bookmark, the import will fail because git-remote-hg will not be able to
create the corresponding reference.

Warn the user about the invalid reference, and continue the import,
instead of stopping right away.

Also add some test cases for this issue.

Reported-by: Antoine Pelisse 
Signed-off-by: Max Horn 
---
 contrib/remote-helpers/git-remote-hg |  6 +
 contrib/remote-helpers/test-hg.sh| 48 
 2 files changed, 54 insertions(+)

diff --git a/contrib/remote-helpers/git-remote-hg 
b/contrib/remote-helpers/git-remote-hg
index eb89ef6..49b2c2e 100755
--- a/contrib/remote-helpers/git-remote-hg
+++ b/contrib/remote-helpers/git-remote-hg
@@ -625,6 +625,12 @@ def list_head(repo, cur):
 def do_list(parser):
 repo = parser.repo
 for bmark, node in bookmarks.listbookmarks(repo).iteritems():
+if node == '':
+if fake_bmark == 'default' and bmark == 'master':
+pass
+else:
+warn("Ignoring invalid bookmark '%s'", bmark)
+continue
 bmarks[bmark] = repo[node]
 
 cur = repo.dirstate.branch()
diff --git a/contrib/remote-helpers/test-hg.sh 
b/contrib/remote-helpers/test-hg.sh
index a933b1e..8d01b32 100755
--- a/contrib/remote-helpers/test-hg.sh
+++ b/contrib/remote-helpers/test-hg.sh
@@ -772,4 +772,52 @@ test_expect_success 'remote double failed push' '
)
 '
 
+test_expect_success 'clone remote with master null bookmark' '
+   test_when_finished "rm -rf gitrepo* hgrepo*" &&
+
+   (
+   hg init hgrepo &&
+   cd hgrepo &&
+   echo a >a &&
+   hg add a &&
+   hg commit -m a &&
+   hg bookmark -r null master
+   ) &&
+
+   git clone "hg::hgrepo" gitrepo &&
+   check gitrepo HEAD a
+'
+
+test_expect_success 'clone remote with default null bookmark' '
+   test_when_finished "rm -rf gitrepo* hgrepo*" &&
+
+   (
+   hg init hgrepo &&
+   cd hgrepo &&
+   echo a >a &&
+   hg add a &&
+   hg commit -m a &&
+   hg bookmark -r null -f default
+   ) &&
+
+   git clone "hg::hgrepo" gitrepo &&
+   check gitrepo HEAD a
+'
+
+test_expect_success 'clone remote with generic null bookmark' '
+   test_when_finished "rm -rf gitrepo* hgrepo*" &&
+
+   (
+   hg init hgrepo &&
+   cd hgrepo &&
+   echo a >a &&
+   hg add a &&
+   hg commit -m a &&
+   hg bookmark -r null bmark
+   ) &&
+
+   git clone "hg::hgrepo" gitrepo &&
+   check gitrepo HEAD a
+'
+
 test_done
-- 
1.9.0.7.ga299b13

--
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: What's cooking in git.git (Mar 2014, #03; Fri, 14)

[Max Horn]

> Am 19.03.2014 um 18:04 schrieb Junio C Hamano :
> 
> Max Horn  writes:
> 
>>> On 17.03.2014, at 18:01, Junio C Hamano  wrote:
>>> 
>>> Torsten Bögershausen  writes:
>>> 
>>>>> On 2014-03-14 23.09, Junio C Hamano wrote:
>>>>> * ap/remote-hg-skip-null-bookmarks (2014-01-02) 1 commit
>>>>> - remote-hg: do not fail on invalid bookmarks
>>>>> 
>>>>> Reported to break tests ($gmane/240005)
>>>>> Expecting a reroll.
>>>> I wonder what should happen here.
>>>> The change breaks all the tests in test-hg-hg-git.sh
>>>> (And the breakage may prevent us from detecting other breakages)
>>>> 
>>>> The ideal situation would be to have an extra test case for the problem
>>>> which we try to fix with this patch.
>>>> 
>>>> Antoine, is there any way to make your problem reproducable ?
>>>> And based on that, to make a patch which passes all test cases ?
>>> 
>>> After re-reading the thread briefly (there're just five messages)
>>> 
>>> http://thread.gmane.org/gmane.comp.version-control.git/239797/focus=240069
>> 
>> For some reason, that link does not contain all messages from that
>> conversation (unfortunately, I have seen GMane do that on multiple
>> occasions. I hence try not to rely on it for reviewing email
>> history -- I just don't trust it). In particular, it misses this
>> crucial post:
> 
> [jc: please avoid overlong lines; I re-flowed above]

Sorry. If anybody knows a way to tech Mail.app to auto-wrap
long lines, I'd appreciate a hint. 

> 
>>  http://thread.gmane.org/gmane.comp.version-control.git/239830
> 
> Interesting.
> 
>> The (or at least "a") root cause has actually been
>> discovered. Would a patch that adds an xfail test case for it be
>> acceptable?
> 
> Do you mean a patch that only adds a new test that expects a failure
> to the current code, without touching the current code that has the
> bug it exposes?

Exactly.

>  That would be a good place to start.

Ok.

> 
>> ... As a matter of fact, I a know a few more bugs in remote-hg for
>> which I could produce xfail test cases. Of course I'd prefer to
>> put them in together with a fix, but I don't know when I can get
>> to that, if ever. So, would such changes be welcome?
> 
> Surely.  That is to keep tabs on bugs in an actionable form; it is a
> better way of bug tracking than having a bug-tracker that is not
> actively maintained, I would think.

Yeah, makes sense.

So, one more silly (bikeshedding) question: should I do this as one big
patch adding multiple xfail tests - or one commit per test, with perhaps a
brief description of the issue at hand? Or should a code comment next to
the failing test explain things?

Actually, some of those bugs might require a lengthy background
explanation, so yet another variant would be to write an email here
With an explanation, then add a gmane ref to the commit message...

> 
> 
> 
> 
--
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 2/3] remote-hg: allow invalid bookmarks in a few edge cases

[Max Horn]

Hi Antoine,

On 19.03.2014, at 14:07, Antoine Pelisse  wrote:

> Hi Max,
> 
> Thank you for working on this.
> I believe it would be fair that you forget about patch 1/3 as you fix
> it in this patch (2/3).
> Also, I think it would be best NOT to integrate a patch (mine) that
> breaks a test, as it
> would make bisect harder to use.


OK, makes sense. I didn't want to step on anybodies feet by hijacking 
previously made work (however small or big it might be -- I've been burned by 
this before). Anyway, so I'll squash the first two commits together (or all 
three even?), and edit the message. But I'd like to properly attribute that you 
discovered the issue, so perhaps I can add something like "Reported-by: Antoine 
Pelisse" or so?

Max

> 
> Thanks,
> Antoine
> 
> On Wed, Mar 19, 2014 at 1:33 PM, Max Horn  wrote:
>> Fix the previous commit to workaround issues with edge cases: Specifically,
>> remote-hg inserts a fake 'master' branch, unless the cloned hg repository
>> already contains a 'master' bookmark. If that 'master' bookmark happens
>> to reference the 'null' commit, the preceding fix ignores it. This
>> would leave us in an inconsistent state. Avoid this by NOT ignoring
>> null bookmarks named 'master' or 'default' under suitable circumstances.
>> 
>> Signed-off-by: Max Horn 
>> ---
>> contrib/remote-helpers/git-remote-hg | 7 +--
>> 1 file changed, 5 insertions(+), 2 deletions(-)
>> 
>> diff --git a/contrib/remote-helpers/git-remote-hg 
>> b/contrib/remote-helpers/git-remote-hg
>> index 12d850e..49b2c2e 100755
>> --- a/contrib/remote-helpers/git-remote-hg
>> +++ b/contrib/remote-helpers/git-remote-hg
>> @@ -626,8 +626,11 @@ def do_list(parser):
>> repo = parser.repo
>> for bmark, node in bookmarks.listbookmarks(repo).iteritems():
>> if node == '':
>> -warn("Ignoring invalid bookmark '%s'", bmark)
>> -continue
>> +if fake_bmark == 'default' and bmark == 'master':
>> +pass
>> +else:
>> +warn("Ignoring invalid bookmark '%s'", bmark)
>> +continue
>> bmarks[bmark] = repo[node]
>> 
>> cur = repo.dirstate.branch()
>> --
>> 1.9.0.7.ga299b13
>> 
> 



signature.asc
Description: Message signed with OpenPGP using GPGMail

[PATCH 1/3] remote-hg: do not fail on invalid bookmarks

[Max Horn]

From: Antoine Pelisse 

Mercurial can have bookmarks pointing to "nullid" (the empty root
revision), while Git can not have references to it.
When cloning or fetching from a Mercurial repository that has such a
bookmark, the import will fail because git-remote-hg will not be able to
create the corresponding reference.

Warn the user about the invalid reference, and continue the import,
instead of stopping right away.

Signed-off-by: Antoine Pelisse 
Signed-off-by: Max Horn 
---
 contrib/remote-helpers/git-remote-hg | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/contrib/remote-helpers/git-remote-hg 
b/contrib/remote-helpers/git-remote-hg
index eb89ef6..12d850e 100755
--- a/contrib/remote-helpers/git-remote-hg
+++ b/contrib/remote-helpers/git-remote-hg
@@ -625,6 +625,9 @@ def list_head(repo, cur):
 def do_list(parser):
 repo = parser.repo
 for bmark, node in bookmarks.listbookmarks(repo).iteritems():
+if node == '':
+warn("Ignoring invalid bookmark '%s'", bmark)
+continue
 bmarks[bmark] = repo[node]
 
 cur = repo.dirstate.branch()
-- 
1.9.0.7.ga299b13

--
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 2/3] remote-hg: allow invalid bookmarks in a few edge cases

[Max Horn]

Fix the previous commit to workaround issues with edge cases: Specifically,
remote-hg inserts a fake 'master' branch, unless the cloned hg repository
already contains a 'master' bookmark. If that 'master' bookmark happens
to reference the 'null' commit, the preceding fix ignores it. This
would leave us in an inconsistent state. Avoid this by NOT ignoring
null bookmarks named 'master' or 'default' under suitable circumstances.

Signed-off-by: Max Horn 
---
 contrib/remote-helpers/git-remote-hg | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/contrib/remote-helpers/git-remote-hg 
b/contrib/remote-helpers/git-remote-hg
index 12d850e..49b2c2e 100755
--- a/contrib/remote-helpers/git-remote-hg
+++ b/contrib/remote-helpers/git-remote-hg
@@ -626,8 +626,11 @@ def do_list(parser):
 repo = parser.repo
 for bmark, node in bookmarks.listbookmarks(repo).iteritems():
 if node == '':
-warn("Ignoring invalid bookmark '%s'", bmark)
-continue
+if fake_bmark == 'default' and bmark == 'master':
+pass
+else:
+warn("Ignoring invalid bookmark '%s'", bmark)
+continue
 bmarks[bmark] = repo[node]
 
 cur = repo.dirstate.branch()
-- 
1.9.0.7.ga299b13

--
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 3/3] remote-hg: add test cases for null bookmarks

[Max Horn]

Signed-off-by: Max Horn 
---
 contrib/remote-helpers/test-hg.sh | 48 +++
 1 file changed, 48 insertions(+)

diff --git a/contrib/remote-helpers/test-hg.sh 
b/contrib/remote-helpers/test-hg.sh
index a933b1e..8d01b32 100755
--- a/contrib/remote-helpers/test-hg.sh
+++ b/contrib/remote-helpers/test-hg.sh
@@ -772,4 +772,52 @@ test_expect_success 'remote double failed push' '
)
 '
 
+test_expect_success 'clone remote with master null bookmark' '
+   test_when_finished "rm -rf gitrepo* hgrepo*" &&
+
+   (
+   hg init hgrepo &&
+   cd hgrepo &&
+   echo a >a &&
+   hg add a &&
+   hg commit -m a &&
+   hg bookmark -r null master
+   ) &&
+
+   git clone "hg::hgrepo" gitrepo &&
+   check gitrepo HEAD a
+'
+
+test_expect_success 'clone remote with default null bookmark' '
+   test_when_finished "rm -rf gitrepo* hgrepo*" &&
+
+   (
+   hg init hgrepo &&
+   cd hgrepo &&
+   echo a >a &&
+   hg add a &&
+   hg commit -m a &&
+   hg bookmark -r null -f default
+   ) &&
+
+   git clone "hg::hgrepo" gitrepo &&
+   check gitrepo HEAD a
+'
+
+test_expect_success 'clone remote with generic null bookmark' '
+   test_when_finished "rm -rf gitrepo* hgrepo*" &&
+
+   (
+   hg init hgrepo &&
+   cd hgrepo &&
+   echo a >a &&
+   hg add a &&
+   hg commit -m a &&
+   hg bookmark -r null bmark
+   ) &&
+
+   git clone "hg::hgrepo" gitrepo &&
+   check gitrepo HEAD a
+'
+
 test_done
-- 
1.9.0.7.ga299b13

--
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: What's cooking in git.git (Mar 2014, #03; Fri, 14)

[Max Horn]

On 19.03.2014, at 11:53, Max Horn  wrote:

> 
> On 17.03.2014, at 18:01, Junio C Hamano  wrote:
> 
>> Torsten Bögershausen  writes:
>> 
>>> On 2014-03-14 23.09, Junio C Hamano wrote:
>>>> * ap/remote-hg-skip-null-bookmarks (2014-01-02) 1 commit
>>>> - remote-hg: do not fail on invalid bookmarks
>>>> 
>>>> Reported to break tests ($gmane/240005)
>>>> Expecting a reroll.
>>> I wonder what should happen here.
>>> The change breaks all the tests in test-hg-hg-git.sh
>>> (And the breakage may prevent us from detecting other breakages)
>>> 
>>> The ideal situation would be to have an extra test case for the problem
>>> which we try to fix with this patch.
> 

[...]

> The (or at least "a") root cause has actually been discovered. Would a patch 
> that adds an xfail test case for it be acceptable?
> 
> As to the why the proposed patch causes test failures: I think this is due to 
> the fact that remote-hg inserts a fake "master" branch (resp. "bookmark" in 
> the hg terminology). Now, in those test cases, a hg repository gets created 
> that actually contains a "null" bookmark named "master". When the proposed 
> fix for the problem is added, this bookmarks gets ignored. But at that point, 
> remote-hg already determined that there is a hg bookmark named "master", and 
> adjusts how it works accordingly -- when we then remove that bookmarks, 
> things go awry.
> 
> But I might be wrong here, and in any case, did not yet have time to come up 
> with a proper fix.

Actually, scratch that, I just came up with a fix, and also tests. Will submit 
shortly.

I'd still like to know whether it is OK to submit further patches with 
(x?)failing test cases?






signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: What's cooking in git.git (Mar 2014, #03; Fri, 14)

[Max Horn]

On 17.03.2014, at 18:01, Junio C Hamano  wrote:

> Torsten Bögershausen  writes:
> 
>> On 2014-03-14 23.09, Junio C Hamano wrote:
>>> * ap/remote-hg-skip-null-bookmarks (2014-01-02) 1 commit
>>> - remote-hg: do not fail on invalid bookmarks
>>> 
>>> Reported to break tests ($gmane/240005)
>>> Expecting a reroll.
>> I wonder what should happen here.
>> The change breaks all the tests in test-hg-hg-git.sh
>> (And the breakage may prevent us from detecting other breakages)
>> 
>> The ideal situation would be to have an extra test case for the problem
>> which we try to fix with this patch.
>> 
>> Antoine, is there any way to make your problem reproducable ?
>> And based on that, to make a patch which passes all test cases ?
> 
> After re-reading the thread briefly (there're just five messages)
> 
>  http://thread.gmane.org/gmane.comp.version-control.git/239797/focus=240069

For some reason, that link does not contain all messages from that conversation 
(unfortunately, I have seen GMane do that on multiple occasions. I hence try 
not to rely on it for reviewing email history -- I just don't trust it). In 
particular, it misses this crucial post:
  http://thread.gmane.org/gmane.comp.version-control.git/239830

I call it crucial because it describes how to make a reproducible test cases 
out of this, in which a legitimate hg repository leads to a remote-hg error 
preventing the user from normal operation.


> 
> I think the "breakage" the patch tries to fix seems to be of dubious
> nature in the first place ("I don't know how I ended-up with such a
> bookmark", Antoine says in $gmane/239800), and it has been in
> "Expecting a reroll" state in response to "I will try to come-up
> with an improved version" in $gmane/240069 but nothing has happened
> for a few months.
> 
> At this point I think it would be OK for me to discard the topic
> (without prejudice); if the root cause of the issue (if there is
> one) and a proper fix is discovered in the future, the topic can
> come back with a fresh patch, but it appears to me that keeping the
> above patch in my tree would not help anybody.

The (or at least "a") root cause has actually been discovered. Would a patch 
that adds an xfail test case for it be acceptable?

As to the why the proposed patch causes test failures: I think this is due to 
the fact that remote-hg inserts a fake "master" branch (resp. "bookmark" in the 
hg terminology). Now, in those test cases, a hg repository gets created that 
actually contains a "null" bookmark named "master". When the proposed fix for 
the problem is added, this bookmarks gets ignored. But at that point, remote-hg 
already determined that there is a hg bookmark named "master", and adjusts how 
it works accordingly -- when we then remove that bookmarks, things go awry.

But I might be wrong here, and in any case, did not yet have time to come up 
with a proper fix. What I do have is a test case, that I could turn into an 
xfail test. As a matter of fact, I a know a few more bugs in remote-hg for 
which I could produce xfail test cases. Of course I'd prefer to put them in 
together with a fix, but I don't know when I can get to that, if ever. So, 
would such changes be welcome?





signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: [RFC/WIP] Pluggable reference backends

[Max Horn]

On 10.03.2014, at 15:30, Shawn Pearce  wrote:

> On Mon, Mar 10, 2014 at 4:00 AM, Michael Haggerty  
> wrote:
>> I have started working on pluggable ref backends.  In this email I
>> would like to share my plans and solicit feedback.
> 
> Yay!

Yay, too!

> JGit already has pluggable ref backends, so it is good to see this
> starting in git-core.
> 
> FWIW the Gerrit Code Review community is interested in this project.
> 
>> * Store references in a SQLite database, to get correct transaction
>>  handling.
> 
> No to SQLLite in git-core. Using it from JGit requires building
> SQLLite and a JNI wrapper, which makes JGit significantly less
> portable. I know SQLLite is pretty amazing, but implementing
> compatibility with it from JGit will be a big nightmare for us.

I understood this as an example (indeed, it is listed under "Wile new ideas"), 
not a proposal to put this into the git core. It might be an interesting 
experiment in any case, and if the proposed modularity is truly achieved, it 
could (if there was any interest in it, that is) be implemented in an external 
3rd party project.


Anyway, I am quite excited about this project. Usually, I am quite skeptical 
about such large scope ideas ("Yeah, cool idea, but who will pull it off, and 
with which resources?"). But this one seems to have a good chance of being 
implemented gradually and inside the main repository, with the help of "feature 
flags". 

Thus, I am looking forward to Michael's announced initial patch series. I feel 
that I don't know enough yet about git overall to be of much help on my own at 
this point. But perhaps over time some mini- or micro-projects pop up were 
others can help (e.g. "adapt these 50 tests to work with the 'quagga' ref"); if 
they are pointed out (assuming that doing so isn't more work than just 
addressing them yourself ;-), I am willing to help out.


Cheers,
Max


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: [PATCH v3] commit.c: use skip_prefix() instead of starts_with()

[Max Horn]

On 04.03.2014, at 09:42, Tanay Abhra  wrote:

[...]

> commit.c | 17 +
> 1 file changed, 9 insertions(+), 8 deletions(-)
> 
> diff --git a/commit.c b/commit.c
> index 6bf4fe0..6c92acb 100644
> --- a/commit.c
> +++ b/commit.c

[...]

> @@ -566,14 +566,16 @@ static void record_author_date(struct author_date_slab 
> *author_date,
>buf;
>buf = line_end + 1) {
>   line_end = strchrnul(buf, '\n');
> - if (!starts_with(buf, "author ")) {
> + ident_line = skip_prefix(buf, "author ");
> + if (!ident_line) {
>   if (!line_end[0] || line_end[1] == '\n')
>   return; /* end of header */
>   continue;
>   }
> + buf = ident_line;
>   if (split_ident_line(&ident,
> -  buf + strlen("author "),
> -  line_end - (buf + strlen("author "))) ||
> +  buf,
> +  line_end - buf) ||
>   !ident.date_begin || !ident.date_end)
>   goto fail_exit; /* malformed "author" line */
>   break;

Why not get rid of that assignment to "buf", and use ident_line instead of buf 
below? That seems like it would be more readable, wouldn't it?


> @@ -1193,10 +1195,9 @@ static void parse_gpg_output(struct signature_check 
> *sigc)
>   for (i = 0; i < ARRAY_SIZE(sigcheck_gpg_status); i++) {
>   const char *found, *next;
> 
> - if (starts_with(buf, sigcheck_gpg_status[i].check + 1)) {
> - /* At the very beginning of the buffer */
> - found = buf + strlen(sigcheck_gpg_status[i].check + 1);
> - } else {
> + found = skip_prefix(buf, sigcheck_gpg_status[i].check + 1);
> + /* At the very beginning of the buffer */

Do we really need that comment, and in that spot? The code seemed clear enough 
to me without it. But if you think keeping is better, perhaps move it to 
*before* the skip_prefix, and add a trailing "?"

> + if(!found) {
>   found = strstr(buf, sigcheck_gpg_status[i].check);
>   if (!found)
>   continue;




signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: [PATCH V2] commit.c: Use skip_prefix() instead of starts_with()

[Max Horn]

On 03.03.2014, at 20:43, Junio C Hamano  wrote:

> Tanay Abhra  writes:
> 
>> @@ -1193,9 +1194,9 @@ static void parse_gpg_output(struct signature_check 
>> *sigc)
>>  for (i = 0; i < ARRAY_SIZE(sigcheck_gpg_status); i++) {
>>  const char *found, *next;
>> 
>> -if (starts_with(buf, sigcheck_gpg_status[i].check + 1)) {
>> +if (found = skip_prefix(buf, sigcheck_gpg_status[i].check + 1)) 
>> {
>>  /* At the very beginning of the buffer */
>> -found = buf + strlen(sigcheck_gpg_status[i].check + 1);
>> +;
>>  } else {
>>  found = strstr(buf, sigcheck_gpg_status[i].check);
>>  if (!found)
> 
> This hunk looks good.  It can be a separate patch but they are both
> minor changes so it is OK to have it in a single patch.

Hm, but that hunk also introduces an assignment in a conditional, and 
introduces an empty block. Maybe like this?


diff --git a/commit.c b/commit.c
index 6bf4fe0..0ee0725 100644
--- a/commit.c
+++ b/commit.c
@@ -1193,10 +1193,8 @@ static void parse_gpg_output(struct signature_check 
*sigc)
for (i = 0; i < ARRAY_SIZE(sigcheck_gpg_status); i++) {
const char *found, *next;

-   if (starts_with(buf, sigcheck_gpg_status[i].check + 1)) {
-   /* At the very beginning of the buffer */
-   found = buf + strlen(sigcheck_gpg_status[i].check + 1);
-   } else {
+   found = skip_prefix(buf, sigcheck_gpg_status[i].check + 1);
+   if (!found) {
found = strstr(buf, sigcheck_gpg_status[i].check);
if (!found)
continue;




signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: [BUG] Halt during fetch on MacOS

[Max Horn]

On 01.03.2014, at 00:26, Conley Owens  wrote:

> $ git --version  # This is just the git from MacPorts
> git version 1.8.5.5
> $ sw_vers
> ProductName:Mac OS X
> ProductVersion: 10.8.5
> BuildVersion:   12F45

I cannot reproduce this, neither with 1.8.5.5 nor with 1.9.0. I am also running 
Mac OS X 10.8.5.

Note: I tried this both with you original test.sh, and also with a version were 
I replaced the ">" by ">>", as per Jeff's suggestion. It (as predicted) didn't 
make any difference).

If this is a race condition, it might be easier to trigger it on slower 
hardware. I am running this on a 2012 MBP with 2.3 Ghz i7 and an SSD. What is 
your test machine?


Cheers,
Max

> 
> test.sh
> "
> #!/bin/bash
> rungit() {
>mkdir $1
>GIT_DIR=$1 git init --bare
>echo '[remote "aosp"]' > $1/config
>echo 'url =
> https://android.googlesource.com/platform/external/tinyxml2' >>
> $1/config
>GIT_DIR=$1 git fetch aosp +refs/heads/master:refs/remotes/aosp/master
>rm -rf $1
> }
> 
> for i in $(seq 1 100)
> do
>rungit testdir$i &
> done
> """
> $ ./test.sh  # Warning! This script fetches ~40MB of data
> 
> When everything cools, you can see that there are some fetches hanging
> (typically).
> $ ps | grep 'git fetch'
> ...
> 63310 ttys0040:00.01 git fetch aosp
> +refs/heads/master:refs/remotes/aosp/master
> 63314 ttys0040:00.01 git fetch aosp
> +refs/heads/master:refs/remotes/aosp/master
> 63319 ttys0040:00.01 git fetch aosp
> +refs/heads/master:refs/remotes/aosp/master
> 63407 ttys0040:00.00 git fetch aosp
> +refs/heads/master:refs/remotes/aosp/master
> 63414 ttys0040:00.00 git fetch aosp
> +refs/heads/master:refs/remotes/aosp/master
> 63420 ttys0040:00.00 git fetch aosp
> +refs/heads/master:refs/remotes/aosp/master
> ...
> 
> You can look at the parent process of each and see that one half
> spawned the other half, or you can look at the environment variables
> for each to see that there are two processes operating in the same
> directory for each directory where there's an issue.
> $ echo "$(for pid in $(ps | grep 'git fetch' | grep -o '^[0-9]*'); do
> ps -p $pid -wwwE | grep 'GIT_DIR=[^ ]*' -o; done)" | sort
> GIT_DIR=testdir14
> GIT_DIR=testdir14
> GIT_DIR=testdir32
> GIT_DIR=testdir32
> GIT_DIR=testdir47
> GIT_DIR=testdir47
> 
> I've searched through the mailing list, but this doesn't seem to be a
> known issue.  I've only seen this occur on macs (and with a good deal
> of regularity).  It doesn't occur on my Ubuntu box.
> 
> ~cco3
> --
> 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
> 



signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: What's cooking in git.git (Feb 2014, #06; Wed, 19)

[Max Horn]

> Am 24.02.2014 um 18:06 schrieb Junio C Hamano :
> 
> Junio C Hamano  writes:
> 
>> But I think I was worried too much into the future---I agree that
>> the code can stay as you proposed until such a remote-helper needs
>> more support, because "overwrite with zero" is necessary but is
>> probably not sufficient---it also may need to be able to tell us
>> what the final resulting commit of the push is, for example.
> 
> So, here is what I'll queue (with forged s-o-b).

Thank you, I hereby declare the forged s-o-b as legit ;-)

> 
> Thanks.
> 
> -- >8 --
> From: Max Horn 
> Date: Fri, 21 Feb 2014 10:55:59 +0100
> Subject: [PATCH] transport-helper.c: do not overwrite forced bit
> 
> If the the transport helper says it was a forced update, then it is
> a forced update.  It is however possible that an update is forced
> without the transport-helper knowing about it, namely because some
> higher up code had objections to the update and needed forcing in
> order to let it through to the transport helper.  In other words, it
> does not necessarily mean the update was *not* forced, when the
> helper did not say "forced update".
> 
> Signed-off-by: Max Horn 
> Signed-off-by: Junio C Hamano 
> ---
> transport-helper.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/transport-helper.c b/transport-helper.c
> index abe4c3c..705dce7 100644
> --- a/transport-helper.c
> +++ b/transport-helper.c
> @@ -727,7 +727,7 @@ static int push_update_ref_status(struct strbuf *buf,
>}
> 
>(*ref)->status = status;
> -(*ref)->forced_update = forced;
> +(*ref)->forced_update |= forced;
>(*ref)->remote_status = msg;
>return !(status == REF_STATUS_OK);
> }
> -- 
> 1.9.0-291-g027825b
> 
> 
--
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: What's cooking in git.git (Feb 2014, #06; Wed, 19)

[Max Horn]

On 21.02.2014, at 19:04, Junio C Hamano  wrote:

> Here is a description I wrote for a tentative commit to queue on
> 'pu' after seeing your response:
> 
>transport-helper.c: do not overwrite forced bit

I'd change "forced bit" to "forced_update bit"

> 
>It does not necessarily mean the update was *not* forced, when the
>helper did not say "forced update".  When the helper does say so, we
>know the update is forced.
> 
>A workaround to fix breakage introduced by the previous step,
>proposed by Max Horn.
> 
> It troubled me that "it does not necessarily mean" sounded really
> wishy-washy.

But it's correct. But if you want to be more precise, how about this:

  If the the transport helper says it was a forced update, then it is
  a forced update. But it is possible that an update is forced without
  the transport-helper knowing about it, namely because some higher up
  code had objections to the update and needed forcing in order to let
  it through to the transport helper.



>  Isn't it possible for some helpers to _do_ want to
> tell us that it did not have to force after all by _not_ saying
> "forced update" and overwrite ->forced_update with zero?

Yes to the first part, no to the last bit:  Yes, a transport helper can (and 
frequently does) tell us that no force happened -- by not saying "forced 
update".

But not, it should not get to reset the forced_update bit. Because it can't 
know what other reasons there might have been for requiring a --force. If you 
look at the code, right now the only place setting forced_update is 
set_ref_status_for_push() in remote.c. If it decides to set forced_update, it 
does so, without giving the transport helper any say in in the matter. So the 
update will fail. Unless the user forces it. Only then the transport helper 
gets involved, and indeed, it could happen that the transport helper happily 
pushes the changes without seeing any reason to signal a "forced update".

But the update still should be marked as "forced", because it *was* forced. The 
transport helper can't know about that, and consequently, it doesn't make sense 
to allow it to override the statement of its betters ;-).


>  How do we tell helpers that do want to do so apart from other helpers that 
> say
> "forced update" only when they noticed they are indeed forcing?

I am not completely sure I even understand that bit? So far, no remote helper 
should have ever said "forced update", as they weren't allowed to. Now, we 
allow transport helpers supporting the "force" feature to signal that, "yes, 
indeed, I had to force this update". And the only thing we use that for is to 
report this fact to the user. So, that seems fine and all cases covered. No?


BTW, I guess we could perform an extra test and raise an error if a transport 
helper dares to request the "forced update" message even though it wasn't told 
that this update is supposed to be forced, i.e. even though !ref->force && 
!(flags & TRANSPORT_PUSH_FORCE) holds -- but doing that would mostly be 
something to help transport helper developers to catch misbehavior in their 
remote helpers, and could just as well be verified by suitable test cases.



> Perhaps the logic needs to be more like:
> 
>   if (we know helper will tell us the push did not have to
>force by *not* saying "forced update") {
>   (*ref)->forced_update = forced;
>   }
> 
> i.e. for helpers that do not use the 'forced update' feature, simply
> ignore this "forced" variable altogether, no?

Huh? For helpers not using the "forced updated" feature, the local "forced" 
variable stays zero, hence "forced_update |= forced" already does nothing.



Max


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: What's cooking in git.git (Feb 2014, #06; Wed, 19)

[Max Horn]

On 20.02.2014, at 20:22, Junio C Hamano  wrote:

> Max Horn  writes:
> 
>> On 19.02.2014, at 22:41, Junio C Hamano  wrote:
>> 
>>> * fc/transport-helper-fixes (2013-12-09) 6 commits
>>> - remote-bzr: support the new 'force' option
>>> - test-hg.sh: tests are now expected to pass
>>> - transport-helper: check for 'forced update' message
>>> - transport-helper: add 'force' to 'export' helpers
>>> - transport-helper: don't update refs in dry-run
>>> - transport-helper: mismerge fix
>>> 
>>> Reported to break t5541, and has been stalled for a while without
>>> fixes.
>> ...
>> Since I somewhat care about transport-helpers, I had a closer look
>> at this failure.
> 
> Thanks.  Let's keep it a bit longer and see how your new
> investigation (and possibly help from others) develops to a
> solution.

So I had a closer look, and I now believe to now understand what the right fix 
is. Simply dropping 
  transport-helper: check for 'forced update' message
would be wrong, because it would cause the contrib/remote-helpers test cases to 
fail -- when I tested last night, I forgot that I had to run those separately. 
Silly me!

Indeed, these tests include a test with a force push, and trigger the code path 
added in that commit. The remaining problem then is that the new code should be 
changed to only tell git when a remote-helper signals a forced update; but it 
should not incorrectly reset the forced_update flag to 0 if git already 
considers the update to be forced.

In other words, the following patch should be the correct solution, as far as I 
can tell. I verified that it fixes t5541 and causes no regressions in the 
contrib/remote-helpers test suite.

-- 8< --
diff --git a/transport-helper.c b/transport-helper.c
index fa7c608..86e1679 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -734,7 +734,7 @@ static int push_update_ref_status(struct strbuf *buf,
}

(*ref)->status = status;
-   (*ref)->forced_update = forced;
+   (*ref)->forced_update |= forced;
(*ref)->remote_status = msg;
return !(status == REF_STATUS_OK);
 }
-- 8< --




signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: What's cooking in git.git (Feb 2014, #06; Wed, 19)

[Max Horn]

On 19.02.2014, at 22:41, Junio C Hamano  wrote:

> * fc/transport-helper-fixes (2013-12-09) 6 commits
> - remote-bzr: support the new 'force' option
> - test-hg.sh: tests are now expected to pass
> - transport-helper: check for 'forced update' message
> - transport-helper: add 'force' to 'export' helpers
> - transport-helper: don't update refs in dry-run
> - transport-helper: mismerge fix
> 
> Updates transport-helper, fast-import and fast-export to allow the
> ref mapping and ref deletion in a way similar to the natively
> supported transports.
> 
> Reported to break t5541, and has been stalled for a while without
> fixes.
> 
> Will discard.

Since I somewhat care about transport-helpers, I had a closer look at this 
failure. Torsten already narrowed it down to f9e3c6bebb89de12 
(transport-helper: check for 'forced update' message).

Looking at that commit, the problem is the new line

   (*ref)->forced_update = forced;

which is supposed to set forced_update to 1 in certain cases; but the code 
which sets "forced = 1" ever triggered (at least in my limited tests). Worse, 
it seems forced_update can be set to 1 before we ever get there, and in these 
casss, we end up reseting forced_update from 1 back to 0. This triggers the 
test failure.

So a simple fix for the test failure is to drop that patch. Another would be to 
change the assignment to

   (*ref)->forced_update |= forced;

But I haven't spent enough time to look at the patch to determine if one of 
these two possible changes is correct. All I can say is that dropping that 
single commit fixes the test failure for me and seems to cause now new failures.


Cheers,
Max


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: [PATCH v3 2/2] Rename suffixcmp() to has_suffix() and invert its result

[Max Horn]

> Am 07.11.2013 um 00:28 schrieb Jonathan Nieder :
> 
> Max Horn wrote:
> 
>> Well, unlike suffixcmp, it is transitive, so it could be used for sorting.
> 
> It is not antisymmetric.
> 
>prefixcmp("foo", "foobar") < 0
>prefixcmp("foobar", "foo") == 0

Right! I wasn't thinkinh :-(
> 
> I can see how it's possible to care about the sign of the return
> value, but it's equally possible to care about the sign from
> suffixcmp.  Neither is suitable as a function for passing to qsort.

Yeah, so then I'd be for changing rhis one to has_prefix.


> 
> Hoping that clarifies,
> 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 v3 2/2] Rename suffixcmp() to has_suffix() and invert its result

[Max Horn]

On 06.11.2013, at 23:17, Jonathan Nieder  wrote:

> Hi,
> 
> Christian Couder wrote:
> 
>> Now has_suffix() returns 1 when the suffix is present and 0 otherwise.
> 
> Ok.  My only worry is that the function is less discoverable since
> its name is so different from prefixcmp(), which might cause someone
> to invent yet another postfixcmp.

Well, that can always happen, no matter what, can't it?

Though personally I wouldn't mind if there was an has_prefix instead or in 
parallel to prefixcmp.

> 
>> The old name followed the pattern anything-cmp(), which suggests
>> a general comparison function suitable for e.g. sorting objects.
>> But this was not the case for suffixcmp().
> 
> It's not clear to me that prefixcmp() is usable for sorting objects,
> either.  Shouldn't it get the same treatment?

Well, unlike suffixcmp, it is transitive, so it could be used for sorting. 
Whether doing that would be sensible is another question, though :-).

For clarity, it might indeed be better to also change prefixcmp to 
has_prefix(), and if some code pops up in the future that needs something like 
the current prefixcmp, it can still be added back.



signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: [PATCH v2] Rename suffixcmp() to has_suffix() and inverse its result

[Max Horn]

+1 for the change. I find the resulting code easier to understand, too.


On 05.11.2013, at 05:57, Christian Couder  wrote:

> As suffixcmp() should not be used as an ordering comparison function,
> and anything-cmp() ought to be usable as an ordering comparison function,
> suffixcmp() should be renamed to something that doesn't end with "cmp".
> 
> has_suffix() is a straightforward name for such a function, except
> that with such a name callers will expect that it will return 1
> when the suffix is present and 0 otherwise.
> 
> So we need to also inverse the value returned by this function ti

> match what the callers will expect, because suffixcmp() like all
> anything-cmp() returns 0 when the suffix is present and 1 or -1
> otherwise.
> 
> As we inverse the value returned by the function, we also have
> to inverse the ways its callers are using its returned value.

s/inverse/invert/  (multiple times)

Taking one step back, shouldn't the commit message rather explain the new 
status, instead of referring so much to the past? If I imagine somebody reading 
this in a year, they might not even know suffixcmp (e.g. if they joined the 
project after this patch was merged).

How about something like this:

--- 8< 

Rename suffixcmp() to has_suffix() and invert its result

Now has_suffix() returns 1 when the suffix is present and 0 otherwise.

The old name followed the pattern anything-cmp(), which suggests
a general comparison function suitable for e.g. sorting objects.
But this was not the case for suffixcmp().

--- 8< 


By the way, a much stronger reason why suffixcmp is not suitable than that it 
is not clear what it would mean, is that it is not transitive. I.e. for an 
ordering you would want that if a "2"



> 
> Signed-off-by: Christian Couder 
> ---
> Hi Junio and Peff,
> 
> So here is a new version of the patch to rename
> suffixcmp() into has_suffix(). We now inverse the
> result of the function as we rename it.
> 
> This patch should be added to or squashed into the
> patch series that removes postfixcmp().
> 

[...]




signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: [PATCH v5 09/10] fast-export: add support to delete refs

[Max Horn]

On 31.10.2013, at 20:53, Felipe Contreras  wrote:

> On Thu, Oct 31, 2013 at 1:47 PM, Max Horn  wrote:
>> 
>> On 31.10.2013, at 20:41, Felipe Contreras  wrote:
>> 
>>> On Thu, Oct 31, 2013 at 1:29 PM, Max Horn  wrote:
[...]
>>> 
>>> That's what the previous patch does.
>> 
>> Right *facepalm*.
>> 
>> But then this should be documented in git-fast-import.txt, shouldn't it?
> 
> It is... in the previous patch.

Indeed, there it is. So that answer to my initial question is: Yes, I am being 
dense :-).

So, I am (still) happy with the patch series :-)



signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: [PATCH v5 09/10] fast-export: add support to delete refs

[Max Horn]

On 31.10.2013, at 20:41, Felipe Contreras  wrote:

> On Thu, Oct 31, 2013 at 1:29 PM, Max Horn  wrote:
>> Actually, I just noticed one thing that I *do* have a question about:
>> 
>> On 31.10.2013, at 10:36, Felipe Contreras  wrote:
>> 
>>> Signed-off-by: Felipe Contreras 
>>> ---
>>> builtin/fast-export.c  | 14 ++
>>> t/t9350-fast-export.sh | 11 +++
>>> 2 files changed, 25 insertions(+)
>>> 
>>> diff --git a/builtin/fast-export.c b/builtin/fast-export.c
>>> index b6f623e..8ed41b4 100644
>>> --- a/builtin/fast-export.c
>>> +++ b/builtin/fast-export.c
>>> @@ -673,6 +673,19 @@ static void import_marks(char *input_file)
>>>  fclose(f);
>>> }
>>> 
>>> +static void handle_deletes(void)
>>> +{
>>> + int i;
>>> + for (i = 0; i < refspecs_nr; i++) {
>>> + struct refspec *refspec = &refspecs[i];
>>> + if (*refspec->src)
>>> + continue;
>>> +
>>> + printf("reset %s\nfrom %s\n\n",
>>> + refspec->dst, sha1_to_hex(null_sha1));
>> 
>> If I understand it right, this issues a "reset" command in the fast-import 
>> stream, resetting a ref to an all-zero SHA1. I had a look at the 
>> git-fast-import documentation, but I found that it does not explicitly cover 
>> this case. In particular, the "reset" command does not specify that an 
>> all-zero SHA1 should be treated as "delete this ref".
> 
> That's what the previous patch does.

Right *facepalm*.

But then this should be documented in git-fast-import.txt, shouldn't it?

> 
>> On the other hand, the docs for "reset" seem to indicate that one can omit 
>> the "from" part, although I couldn't tell for sure what that would mean, 
>> either.
> 
> It means something different.

Yeah, I figured that -- just wanted to point out that this, too, is not very 
clear in the documentation and should be improved (not saying that I expect you 
to do that, just pointing it out).



signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: [PATCH v5 09/10] fast-export: add support to delete refs

[Max Horn]

Actually, I just noticed one thing that I *do* have a question about:

On 31.10.2013, at 10:36, Felipe Contreras  wrote:

> Signed-off-by: Felipe Contreras 
> ---
> builtin/fast-export.c  | 14 ++
> t/t9350-fast-export.sh | 11 +++
> 2 files changed, 25 insertions(+)
> 
> diff --git a/builtin/fast-export.c b/builtin/fast-export.c
> index b6f623e..8ed41b4 100644
> --- a/builtin/fast-export.c
> +++ b/builtin/fast-export.c
> @@ -673,6 +673,19 @@ static void import_marks(char *input_file)
>   fclose(f);
> }
> 
> +static void handle_deletes(void)
> +{
> + int i;
> + for (i = 0; i < refspecs_nr; i++) {
> + struct refspec *refspec = &refspecs[i];
> + if (*refspec->src)
> + continue;
> +
> + printf("reset %s\nfrom %s\n\n",
> + refspec->dst, sha1_to_hex(null_sha1));

If I understand it right, this issues a "reset" command in the fast-import 
stream, resetting a ref to an all-zero SHA1. I had a look at the 
git-fast-import documentation, but I found that it does not explicitly cover 
this case. In particular, the "reset" command does not specify that an all-zero 
SHA1 should be treated as "delete this ref". On the other hand, the docs for 
"reset" seem to indicate that one can omit the "from" part, although I couldn't 
tell for sure what that would mean, either.

I have not yet tried to dive into the code to figure out what actually happens, 
but I assume Felipe did that resp. tested it, and so supposedly doing this 
works, at least for git resp. existing transport helpers. But I wonder if other 
implementers of the fast-import format would handle it properly?

In any case, it seems to me that this is a gap in the documentation, isn't it? 
Or am I just being dense?



signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: [PATCH 03/16] pull: cleanup documentation

[Max Horn]

On 31.10.2013, at 20:00, Junio C Hamano  wrote:

> Felipe Contreras  writes:

[...]

> 
>> 
>> If you want to be pedantic, this is the "reality":
>> 
>>  
>>  D---E---F---G master
>>  
> 
> You are wrong again.  The "reality" is more like this:
> 
>  origin/master in your repository
>  |
>  v
>  A---B---C master at origin
> /
>D---E---F---G master in your repository
> 
> if you really want to write origin/master somewhere in this
> illustration.

Actually, I kind of like that. After just reading the existing phrasing in 
git-pull.txt, I doubt that a newbie would catch the difference between 
"origin/master" and "master at origin". With this illustration, it's very 
clearly conveyed that there is a difference.



signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: [PATCH v5 00/10] transport-helper: updates

[Max Horn]

On 31.10.2013, at 19:10, Junio C Hamano  wrote:

> Max Horn  writes:
> 
>> On 31.10.2013, at 10:36, Felipe Contreras 
>> wrote:
>> 
>>> Hi,
>>> 
>>> Here are the patches that allow transport helpers to be completely
>> transparent;
>>> renaming branches, deleting them, custom refspecs, --force,
>> --dry-run,
>>> reporting forced update, everything works.
>> 
>> I looked through this patch series in detail, and it looks fine to
>> me. Indeed, it fixes several nuisances when using remote-helpers, and
>> as such would be a definite win.
> 
> Nice.
> 
>> In other words: Would be really nice to see these applied!
> 
> The series is sitting on the 'pu' branch, and I think there were
> some "fixup" suggestions during the review, so it may need to be
> rerolled before advancing to 'next'.

You are of course right. Next time I comment, I'll make sure to check whether 
previous review suggestions have been picked up.



signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: [PATCH v5 08/10] fast-import: add support to delete refs

[Max Horn]

On 31.10.2013, at 10:36, Felipe Contreras  wrote:

> Signed-off-by: Felipe Contreras 
> ---
> Documentation/git-fast-import.txt |  3 +++
> fast-import.c | 13 ++---
> t/t9300-fast-import.sh| 18 ++
> 3 files changed, 31 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/git-fast-import.txt 
> b/Documentation/git-fast-import.txt
> index 73f9806..c49ede4 100644
> --- a/Documentation/git-fast-import.txt
> +++ b/Documentation/git-fast-import.txt
> @@ -483,6 +483,9 @@ Marks must be declared (via `mark`) before they can be 
> used.
> * Any valid Git SHA-1 expression that resolves to a commit.  See
>   ``SPECIFYING REVISIONS'' in linkgit:gitrevisions[7] for details.
> 
> +* The special null SHA-1 (40 zeros) specifices that the branch is to be

s/specifices/specifies/

(this was previously pointed out by Eric Sunshine for patch 7 of v4 of this 
series).



signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: [PATCH 00/16] Trivial patches

[Max Horn]

On 31.10.2013, at 10:25, Felipe Contreras  wrote:

> Most of these have been sent before, but were not applied for one reason or
> another.

All of these look fine and sensible to me. Some of the latter patches in the 
series might be a bit subjective (e.g. I personally don't mind "yoda" 
conditions at all), but none do harm, and most are a clear improvement. So I am 
all for applying this.

Cheers,
Max

> 
> Felipe Contreras (16):
>  merge: simplify ff-only option
>  t: replace pulls with merges
>  pull: cleanup documentation
>  fetch: add missing documentation
>  revision: add missing include
>  shortlog: add missing declaration
>  branch: trivial style fix
>  sha1-name: trivial style cleanup
>  transport-helper: trivial style fix
>  describe: trivial style fixes
>  pretty: trivial style fix
>  revision: trivial style fixes
>  diff: trivial style fix
>  run-command: trivial style fixes
>  setup: trivial style fixes
>  add: avoid yoda conditions
> 
> Documentation/git-fetch.txt|  3 +++
> Documentation/git-pull.txt |  4 ++--
> builtin/add.c  |  2 +-
> builtin/branch.c   |  3 +--
> builtin/describe.c |  7 +++
> builtin/diff.c |  2 +-
> builtin/merge.c| 11 ++-
> pretty.c   |  2 +-
> revision.c | 14 ++
> revision.h |  1 +
> run-command.c  | 13 +
> setup.c|  4 ++--
> sha1_name.c|  1 -
> shortlog.h |  2 ++
> t/annotate-tests.sh|  2 +-
> t/t4200-rerere.sh  |  2 +-
> t/t9114-git-svn-dcommit-merge.sh   |  2 +-
> t/t9500-gitweb-standalone-no-errors.sh |  2 +-
> transport-helper.c |  1 +
> 19 files changed, 35 insertions(+), 43 deletions(-)
> 
> -- 
> 1.8.4.2+fc1
> 
> --
> 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
> 



signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: [PATCH v5 00/10] transport-helper: updates

[Max Horn]

On 31.10.2013, at 10:36, Felipe Contreras  wrote:

> Hi,
> 
> Here are the patches that allow transport helpers to be completely 
> transparent;
> renaming branches, deleting them, custom refspecs, --force, --dry-run,
> reporting forced update, everything works.

I looked through this patch series in detail, and it looks fine to me. Indeed, 
it fixes several nuisances when using remote-helpers, and as such would be a 
definite win. In other words: Would be really nice to see these applied!


Cheers,
Max



signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: My patches

[Max Horn]

I guess most other people keep out of this because they are sensible enough to 
not feed the troll, and instead focus on useful things. But I can't help it, I 
have to say this.


On 17.10.2013, at 23:44, Felipe Contreras  wrote:

> Junio C Hamano wrote:
>> Felipe Contreras  writes:
>> 
>>> Junio C Hamano wrote:
 Such a review comment and the discussion that follows it after a
 patch is posted is an essential part of the collaborative
 development process in this community and it has helped the quality
 of our end product. We unfortunately saw time and again that the
 process rarely works when the discussion involves your patches.
>>> 
>>> No, you did not. What you saw was a person that unlike a trained dog, argued
>>> against you. And apparently your definition of a good discussion is one in
>>> which the other person just does what you say, and doesn't argue back.
>> 
>> That is so untrue that it is not even funny.
> 
> It is true, and there's penty of evidence that proves it.

No, it is not true, and there is plenty of evidence that proves it.

But I don't think it's helpful for either of us drag up such "evidence", as 
it'll convince nobody -- indeed, I am sure almost everybody here has already 
formed a clear opinion on this matter. And I hazard to guess that the vast 
majority agree with Junio on this (based, again, on email evidence). Not with 
you.

Of course one can't prove mathematical theorems by a majority vote, but as we 
are not talking about theorems, but rather about judging whether Junion's 
behavior is considered fair or not, I think it is appropriate to. Moreover, if 
I look at e.g. the "staging area" discussion, you also bring up the "everybody 
but Junio and one other guy" argument (incorrectly generalizing from "those 
people on this mailing list who chose to reply" to "everybody"), so I think I 
am entitled to do the same ;-).

(BTW, I am actually in favor of using the term "staging area" over "index")


> Every single time that you get mad at me, it's because I disagree with you.

I have not yet seen Junio get "mad" here, even in discussions with you were I 
think most other people would indeed have gotten "mad" at you. He stays 
remarkably polite, despite the insults and dirt you keep flinging at him... If 
at all, it would seem that you are getting mad at Junio.


> 
>> Contributors often make sensible counter-arguments and/or explain
>> the rationale better than they did in their original messages, and
>> then receive a "Thanks for a dose of sanity" (or an equivalent
>> phrased in different ways).
> 
> Yes, when there's an agreement, so you are basically proving what I said. I
> disagree with you, you disagree with me, and that means I'm the problem.

Actually, it is more like this: "Felipe disagrees with Junio, Junio disagrees 
with Felipe, Felipe insults Junio and in passing half a dozen other people." It 
is the latter point which makes the situation asymmetric, and indeed indicates 
you as the problem.

> In any healthy collaborative project that simply means there was a
> disagreement, and that's that.

If your premise was correct (that there is simply a disagreement), then this 
conclusion might be correct. As it is, though, your premise is false. The 
problem is rather a disruptive person: you. Quite sad, since you seem to have 
some good ideas and code contributions! I am in particular grateful for your 
work on remote helpers, both on specific ones (git-remote-hg) and also on 
improving the whole remote helper interface. I hope some of this work can 
eventually be merged...

But at the end of the day, we most (all?) of us here are volunteers, and unlike 
what you seem to claim a lot, for most of us, making git better is *not* the 
number 1 priority in our lives... In particular, if working with you would make 
git better, but at the same time would give me ulcers, well, my choice is clear 
to me... Perhaps it wouldn't be best for git, but I don't think anybody 
(except, perhaps, you) would blame me for it. Or, for that matter, Junio. 


@Junio: Thank you for being an opinionated but also very fair project lead, who 
listens to *constructive* feedback, and has again and again demonstrated 
through action a readiness to revise a position based on sensible discussions 
conducted on this list.



Max


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: [PATCH v2 00/14] Officially start moving to the term 'staging area'

[Max Horn]

On 17.10.2013, at 21:50, Junio C Hamano  wrote:

> Felipe Contreras  writes:

[...]

> 
> However, since you asked, I would share a couple of comments
> regarding the index, cache and staging area.
> 
> (1) "Staging area".
> 
> The phrase "staging area" is not an everyday phrase or common CS
> lingo, and that unfortunately makes it a suboptimal choice of words
> especially to those of us, to whom a large portion of their exposure
> to the English language is through the command words we use when we
> talk to our computers.

Interestingly, as a non-native speaker, I draw the exact reverse conclusion 
from this: While I had no idea what a "staging area" or "to stage" was (I did 
know the "stage" in a theater, though), I found this to not be a major problem: 
Using a dictionary and reading up on what it means in git made it clearly 
quickly enough.

To the contrary, the fact that the term was not yet overloaded with conflicting 
other meanings made it easier for me to attach the semantics git associates 
with it.

In contrast, "index" was exceedingly bad and confusing to me... I already had 
various notions of what an "index" is (e.g. the index of a book -- the same 
word actually exists in German; or more generally an index in computer science, 
as a kind of loopup table, etc.), and to this day, have a hard time 
consolidating this with the way git uses it. For me, it is yet another, seeming 
completely unrelated, meaning of the word "index" I had to memorize. Hey, just 
take a look at Wiki page  for the many 
dozens of meanings associated to this word. Ugh. And worst of it, I am actually 
not quite sure on which of the meanings listed there "the index" as used by git 
is based... I.e. I don't even see a helpful analogy that would make it easier 
to understand the choice of name. 

In summary: For me as a non-native speaker, "index" feels like about the worst 
possible choice (well, you could have called it the "file" or "thing", that 
might have been worse ;-). While staging area turned out to be surprisingly 
good, *precisely* because I was unfamiliar with it. 

So, while "staging area" might not be perfect, it seems good enough to me. If 
this matter had indeed been discussed here for years, and no better suggestions 
has come up, then perhaps it is time to end the search for the (possibly 
non-existent) perfect solution, and instead do the pragmatic?


> The index can also be thought of "like the buffer cache, where new
> contents of files are kept before they are committed to disk
> platter."  At least, non-native speaker with CS background would
> understand that, much better than "the index" (no, I am not saying
> that we should call it "the cache"; I am just saying "the index" is
> not a good word, but we may need to find a better word than "the
> staging area").

Huh? As a non-native speaker with CS background, this actually leaves me more 
confused than I was before. I think about "the staging area", and I don't see 
how this is anything like an "index" (in any of the meaning I see on 
). I can vaguely recognize a faint 
similarity to a "cache", and yet more relation to a "buffer", but in the end, 
none of these strike me as particularly illustrative.

For that matter, I never really understood of why I had to do "git diff 
--cached", I simply learned it by rote. 

On the other hand, I feel that after understanding what the staging area is, 
then writing "git diff --staged" is very logical and simple to memorize.




Cheers,
Max


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: Bug on OS X...

[Max Horn]

On 27.06.2013, at 12:17, John Szakmeister wrote:

> I wanted to look at some OpenWRT bits this morning and ran into an
> issue cloning the packages repository when setting up the package
> feed.  The feeds script executes this under the hood:
> 
>   git clone --depth 1 git://nbd.name/packages.git feeds/packages
> 
> When trying to run the command directly on OS X, I see:
>   :: git clone --depth 1 git://nbd.name/packages.git
>   Cloning into 'packages'...
>   remote: Counting objects: 4728, done.
>   remote: Compressing objects: 100% (4013/4013), done.
>   remote: Total 4728 (delta 158), reused 3339 (delta 94)
>   Receiving objects: 100% (4728/4728), 3.85 MiB | 1.79 MiB/s, done.
>   Resolving deltas: 100% (158/158), done.
>   error: unable to find 9f041557a0c81f696280bb934731786e3d009b36
>   fatal: object of unexpected type
>   fatal: index-pack failed
> 
> I tried on Linux, and it succeeded.  I tested with both 1.8.2 and
> 1.8.3.1.  Unfortunately, I don't have time to dig through what's wrong
> at the moment so I thought I'd put it out there for others.

I am unable to reproduce this on Mac OS X 10.7.5 with git 1.8.3.1 nor with 
current git maint. Command run inside /tmp, which is on a normal HFS+ volume 
(using the default settings, i.e. the FS is case insensitive).


$ git --version
git version 1.8.3.1.42.ge2652c0
$ git clone --depth 1 git://nbd.name/packages.git
Cloning into 'packages'...
remote: Counting objects: 4711, done.
remote: Compressing objects: 100% (3998/3998), done.
remote: Total 4711 (delta 157), reused 3326 (delta 94)
Receiving objects: 100% (4711/4711), 3.85 MiB | 0 bytes/s, done.
Resolving deltas: 100% (157/157), done.


Cheers,
Max

--
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 00/13] remote-hg: general updates

[Max Horn]

While I am not really interested in exchanging any further emails or any other 
form of communication with Felipe, as I find his vitriolic style of 
communication unbearable, I feel compelled to reply to a few points. I'll 
probably regret this... anyway, I promise this will be my last mail in this 
thread. Even though I fully expect to receive a barrage of hostile and 
aggressive replies by Felipe. So be it, /dev/null has plenty space left.


OK, I'll try to keep a professional tone from now on :-).


On 04.04.2013, at 08:42, Felipe Contreras wrote:

[...]

 * The gitifyhg test suite is run after each push on Travis CI against 
 several git / mercurial combinations [4].
 In particular, unlike all other remote-hg implementations I know, we 
 explicitly promise (and test) compatibility with a specific range of 
 Mercurial versions (not just the one the dev happens to have installed 
 right now). This has been a frequent issue for me with the msysgit 
 remote-hg
>>> 
>>> I've personally checked against multiple versions of Mercurial. It's
>>> possible that some error might slip by, but it would get quickly
>>> noticed.
>> 
>> Really? This sounds close to some people who say things like "I don't need a 
>> test suite, I personally run some tests every now and then on my machine."
> 
> Do you see any compatibility issues reported in the git mailing list,
> or my github[1]? No? KTHXBYE. There _were_ compatibility issues, and
> those got reported, and fixed, not any more.

Please consider that the willingness of people to collaborate with you in any 
way is directly related to how you treat them. That includes bug reports. The 
way you acted towards Jed, who was very calmly and matter-of-factly explaining 
things, was IMHO completely inappropriate and unacceptable. Indeed, I should 
augment my list of reasons why people might not want to contribute to remote-hg 
by one major bullet point: You. And please, don't feel to compelled to tell us 
that Junio is really the maintainer of remote-hg and not you: Whether this is 
true or not doesn't matter for this point.


> remote-hg certainly works on versions older than 1.9, again

That's plain wrong. Indeed, remote-hg is using hg interfaces that were only 
introduced in 1.9. As such, I would be quite surprised if remote-hg worked with 
older hg versions, but I don't see why I should bother to test... Hmm, wait, I 
see a reason:

> , I find it
> annoying that you claim to know what is important for users, as if
> somehow knowing that gitifyhg doesn't work with the user's version of
> mercurial (e.g. 1.8) is better than remote-hg's situation; where it
> *actually works*, but it's not mentioned. Yeah, mentioning that it
> doesn't work is better than working, right.

I'll leave it to everybody to read what you wrote there, and contrast it with 
the following, and draw their own conclusions...

The reason why I did not write what exactly is wrong with remote-hg in hg 1.8 
and older is that it is so obvious that I didn't think anybody would need 
handholding to verify it or find out the details :-). But since you "asked" for 
it:


$ hg --version
Mercurial Distributed SCM (version 1.8.4)
(see http://mercurial.selenic.com for more information)

Copyright (C) 2005-2011 Matt Mackall and others
This is free software; see the source for copying conditions. There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
$ git clone hg::foobar/ foobar.git
Cloning into 'foobar.git'...
Traceback (most recent call last):
  File "/Users/mhorn/bin/git-remote-hg", line 846, in 
sys.exit(main(sys.argv))
  File "/Users/mhorn/bin/git-remote-hg", line 819, in main
fix_path(alias, peer or repo, url)
  File "/Users/mhorn/bin/git-remote-hg", line 765, in fix_path
repo_url = util.url(repo.url())
AttributeError: 'module' object has no attribute 'url'

Indeed, util.url was only added in 1.9.3. OK, let's work around that. Then 
local clones work. But of course in reality, most users will want to interact 
with a remote repository. But that's still broken:

$ git clone hg::ssh://h...@bitbucket.org/fingolfin/test-gitifyhg
Cloning into 'test-gitifyhg'...
Traceback (most recent call last):
  File "/Users/mhorn/bin/git-remote-hg", line 1138, in 
sys.exit(main(sys.argv))
  File "/Users/mhorn/bin/git-remote-hg", line 1107, in main
repo = get_repo(url, alias)
  File "/Users/mhorn/bin/git-remote-hg", line 284, in get_repo
peer, dstpeer = hg.clone(myui, {}, url, local_path, update=False, pull=True)
TypeError: clone() got multiple values for keyword argument 'pull'


Right, clone() changed. And some more stuff. See 
.
 

There also was a good reason why I stopped at that point, but I don't remember 
the details right now. And quite frankly, I have zero incentive to even try to 
remember. But anyway, I don't think it's that useful to add support for

Re: [PATCH 00/13] remote-hg: general updates

[Max Horn]

On 04.04.2013, at 08:46, Felipe Contreras wrote:

> On Thu, Apr 4, 2013 at 12:42 AM, Felipe Contreras
>  wrote:
>> On Wed, Apr 3, 2013 at 6:25 PM, Max Horn  wrote:
>>> On 03.04.2013, at 03:31, Felipe Contreras wrote:
>> 
>>>> I only learned about it recently, I've looked at the history and to me
>>>> it seems rather chaotic, and a lot of the code was simply copied from
>>>> git-remote-hg without comment.
>>> 
>>> gitifyhg was scrapped and completely restarted from scratch at some point. 
>>> Based largely on your git-remote-hg code. A bit more on its history can be 
>>> read here:
>>>  http://archlinux.me/dusty/2013/01/06/gitifyhg-rewritten/
> 
> Please don't CC the gitifyhg mailing list, unlike vger mailing lists
> (or any other sane list), it doesn't accept mail from non-subscribers,
> which makes communication with outsiders much more difficult, as
> demonstrated by this.

I changed the settings of the gitifyhg list settings to accept emails from 
anybody.

Moreover, I would appreciate if you could refrain from injecting all those 
snide side remarks, such as the one you just needlessly made about how 
moderated mailing lists are insane.--
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 00/13] remote-hg: general updates

[Max Horn]

On 03.04.2013, at 03:31, Felipe Contreras wrote:

> On Tue, Apr 2, 2013 at 4:23 PM, Max Horn  wrote:
>> 
>> On 02.04.2013, at 22:09, John Keeping wrote:
>> 
>>> On Tue, Apr 02, 2013 at 01:02:49PM -0600, Felipe Contreras wrote:
>>>> Here is the next round of patches for remote-hg, some which have been
>>>> contributed through github.
>>>> 
>>>> Fortunately it seems to be working for the most part, but there are some
>>>> considerable issues while pushing branches and tags.
>>> 
>>> How does this compare to the current state of gitifyhg[1]?  That's built
>>> on top of this git-remote-hg script but seems to have been more actively
>>> developed recently.
> 
> I only learned about it recently, I've looked at the history and to me
> it seems rather chaotic, and a lot of the code was simply copied from
> git-remote-hg without comment.

gitifyhg was scrapped and completely restarted from scratch at some point. 
Based largely on your git-remote-hg code. A bit more on its history can be read 
here:
  http://archlinux.me/dusty/2013/01/06/gitifyhg-rewritten/


> 
>> * added many new test cases, sadly still including some xfails. Several of 
>> these (both passing and xfailing) also apply to remote-hg (i.e. the issue is 
>> also present in contrib's remote-hg)
> 
> I ran these test-cases with remote-hg, and the same test-cases pass. I
> only had to do minor modifications, most of the failures came from
> subtle differences such as different strategies to sanitize authors,
> and which branch to pick for HEAD.

Yeah, that's because what I wrote was based on the situation before your recent 
patch series. Glad to see that git/contrib's remote-hg is improving!


> 
>> * improved handling of hg user names (remote-hg is not able to deal with 
>> some pathological cases, failing to import commits). Sadly, mercurial allows 
>> arbitrary strings as usernames, git doesn't...
> 
> I wouldn't call it improved. In some cases the remote-hg result is
> better, in others gitifyhg is,

I'd love to learn about cases where remote-hg's result is better in your 
opinion, so that I can see if the mapping in gitifyhg could be improved for 
those cases.

That said, this part is really quite subjective I guess. In the end, since 
Mercurial names can be *anything*, one can never get a "perfect" mapping. 
Luckily, for most real repositories out there, user names will be quite sane 
and remote-hg and gitifyhg will produce identical results. (Although some hg 
repos out there have some really weird stuff going on. Yuck.)

> but there's only a single case where
> the author name becomes a significant problem. It's a trivial fix.

Excellent, looking forward to it then.

> 
>> * failed pushes to hg are cleanly rolled back (using mq.strip() from the mq 
>> extension), instead of resulting in inconsistent internal state. This is 
>> quite important in real life, and has bitten me several times with remote-hg 
>> (and was the initial reason why I switched to gitifyhg). A typical way to 
>> reproduce this is to push to a remote repository that has commits not yet in 
>> my local clone.
> 
> This is not an issue in remote-hg any more since now we force the
> push. It's not nice, but there's no other way to push multiple
> bookmarks (aka git branches) to the same branch (aka commit label).

Uhh, what? The push is forced? That sounds to me like a much, much bigger issue 
with remote-hg than anything else ever was, from my point of view. That seems 
tobe akin to making "--force" default to "git push", and I don't think anybody 
here would consider that a good idea.

> I doubt these inconsistent states can happen any more, but if they do,

Seriously? This is triggered quite frequently in real life. And it will very 
likely cause somebody to mess up a hg repository they work on. As long is this 
in, using remote-hg is a total no-go for me. Just consider the following 
scenario:
* user A clones a hg repository into a git repository
* user A commits some commits in the git clone
* meanwhile, user B pushes changes to the hg repository
* user A tries to push his changes to the hg remote

The last step causes this result in gitifyhg (similar to what one gets when the 
remote is a git repos):

 ! [rejected]master -> master (non-fast-forward)
error: failed to push some refs to 'gitifyhg::URL'
hint: Updates were rejected because the tip of your current branch is behind
hint: its remote counterpart. Merge the remote changes (e.g. 'git pull')
hint: before pushing again.
hint: See the 'Note about fast-forwards' in 'git push --help' for details.


With remote-

Re: [PATCH 00/13] remote-hg: general updates

[Max Horn]

On 02.04.2013, at 22:09, John Keeping wrote:

> On Tue, Apr 02, 2013 at 01:02:49PM -0600, Felipe Contreras wrote:
>> Here is the next round of patches for remote-hg, some which have been
>> contributed through github.
>> 
>> Fortunately it seems to be working for the most part, but there are some
>> considerable issues while pushing branches and tags.
> 
> How does this compare to the current state of gitifyhg[1]?  That's built
> on top of this git-remote-hg script but seems to have been more actively
> developed recently.

Several bugs that were fixed in gitifyhg some time ago are now fixed in this 
remote-hg, too.

I'll try to list some of remaining differences, mostly (in my biased opinion) 
improvements on the gitifyhg side. Note that some of these might be outdated 
with felipe's recent changes, i.e. I have not yet had time to review and/or 
test them all. So please bear that in mind.

* added many new test cases, sadly still including some xfails. Several of 
these (both passing and xfailing) also apply to remote-hg (i.e. the issue is 
also present in contrib's remote-hg)

* improved handling of hg user names (remote-hg is not able to deal with some 
pathological cases, failing to import commits). Sadly, mercurial allows 
arbitrary strings as usernames, git doesn't...

* failed pushes to hg are cleanly rolled back (using mq.strip() from the mq 
extension), instead of resulting in inconsistent internal state. This is quite 
important in real life, and has bitten me several times with remote-hg (and was 
the initial reason why I switched to gitifyhg). A typical way to reproduce this 
is to push to a remote repository that has commits not yet in my local clone.

* git notes are used to associate to each git commit the sha1 of the 
corresponding hg commit, to help users figure out that mapping

* internally, the marks are using the hg sha1s instead of the hg rev ids. The 
latter are not necessarily invariant, and using the sha1s makes it much easier 
to recover from semi-broken states.

* Better handling of various hg errors, see e.g. [2]. More work is still needed 
there with both tools, though [3].

* Support for creating hg tags from git (i.e. pushing light git tags to heavy 
hg tags)

* The gitifyhg test suite is run after each push on Travis CI against several 
git / mercurial combinations [4].
In particular, unlike all other remote-hg implementations I know, we explicitly 
promise (and test) compatibility with a specific range of Mercurial versions 
(not just the one the dev happens to have installed right now). This has been a 
frequent issue for me with the msysgit remote-hg

* Renaming a gitifyhg remote just works [5]. Doing that with remote-hg triggers 
a re-clone of the remote repository (if it works at all, I don't remember). 


Sadly, while working on gitifyhg, we discovered various more design problems 
(from our perspective, at least) in Mercurial, e.g. the fact that commits are 
not necessarily normalized, in the sense that "equivalent" commits (same 
author, time, changed files / code) can have different hashs, with some nasty 
implications for import. This is potentially problematic because without extra 
care, these would be mapped to the same commit on the git side.

Unfortunately, we also stumbled into various problems with the git 
remote-helper system. We are currently using the fast-import remote-helper 
type, but are encountering more and more of its limitations. This affects 
remote-hg and gitifyhg equally, and probably other remote helpers. E.g. "git 
push --dry-run" seems to be impossible to support with such a remote-helper 
(but then I might be mistaken).

Thing is, for several of these I don't feel quite competent enough to come up 
with patches that I could submit here. And in my experience just reporting a 
perceived problem with the remote-helper API is not going to trigger a response 
here [6]. I guess that's why we stopped reporting them here for now, but if 
there is interest I could try to compile an overview.


[1] https://github.com/buchuki/gitifyhg
[2] https://github.com/buchuki/gitifyhg/commit/74b71f4
[3] https://github.com/buchuki/gitifyhg/issues/66
[4] https://travis-ci.org/buchuki/gitifyhg/builds
[5] https://github.com/buchuki/gitifyhg/commit/68ce89bb32
[6] http://thread.gmane.org/gmane.comp.version-control.git/214802--
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: rebase: strange failures to apply patc 3-way

[Max Horn]

On 11.03.2013, at 23:54, Andrew Wong wrote:

> On 3/11/13, Max Horn  wrote:
>> Looking at the git config man page to check what each of my config settings
>> does, I discovered "trustctime". And adding
>>  trustctime = false
>> to .git/config made the rebase work, every single time. Huh.
>> 
>> 
>> Adding this to the fact that a clone works fine, I wonder if  something *is*
>> touching my files, but just in that directory. But what could it be? One
>> nagging suspicion is the "file versioning" feature Apple introduced as part
>> of Time Machine in OS X 10.7; it's kind of a "version control system for
>> n00bs" for arbitrary documents. It has caused me some pain in the past.
>> 
>> But I just re-checked, and problematic repos is explicitly on the Time
>> Machine exclusion list. I also used the "tmutil isexcluded FILES" to verify
>> that the problematic files are really on the TM exclusion list. Finally, I
>> moved the one of the repos subdirectory containing most of the problematic
>> files, and then run "git checkout". In other instances, this sufficed to
>> "disassociate" a file from an unwanted TM version history. But doing that
>> had no effect here, i.e. also with the freshly regenerated files, the
>> problems appear.
> 
> Would you be able to turn off Time Machine completely and do a few
> tests? If it does works, then it becomes a matter of fixing Time
> Machine...
> 

sudo launchctl unload /System/Library/LaunchDaemons/com.apple.revisiond.plist

-> this exits revisiond, and prevents launchd from respawning it. After that, 
the problem is gone, on several attempts. And once I reactivate revisions, the 
problem reoccurs.

So it seems pretty clear what the cause of this is. Now I still need to figure 
out why it affects that particular repository and not others. But at this point 
I guess it is safe to say that Apple's auto-crap-magic revisiond is the root of 
the problem, and git is purely a victim. So I'll stop spamming this list with 
this issue for now, and see if I can figure out a fix that is less invasive 
than turning off revisiond completely (which some application rely on, so 
disabling it may break them badly).

Andrew, thank you very much for your persistent guidance and help. I definitely 
owe you a beverage-of-your-choice ;-).

Cheers,
Max--
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: rebase: strange failures to apply patc 3-way

[Max Horn]

On 11.03.2013, at 23:54, Andrew Wong wrote:

> On 3/11/13, Max Horn  wrote:
>> Looking at the git config man page to check what each of my config settings
>> does, I discovered "trustctime". And adding
>>  trustctime = false
>> to .git/config made the rebase work, every single time. Huh.
>> 
>> 
>> Adding this to the fact that a clone works fine, I wonder if  something *is*
>> touching my files, but just in that directory. But what could it be? One
>> nagging suspicion is the "file versioning" feature Apple introduced as part
>> of Time Machine in OS X 10.7; it's kind of a "version control system for
>> n00bs" for arbitrary documents. It has caused me some pain in the past.
>> 
>> But I just re-checked, and problematic repos is explicitly on the Time
>> Machine exclusion list. I also used the "tmutil isexcluded FILES" to verify
>> that the problematic files are really on the TM exclusion list. Finally, I
>> moved the one of the repos subdirectory containing most of the problematic
>> files, and then run "git checkout". In other instances, this sufficed to
>> "disassociate" a file from an unwanted TM version history. But doing that
>> had no effect here, i.e. also with the freshly regenerated files, the
>> problems appear.
> 
> Would you be able to turn off Time Machine completely and do a few
> tests? If it does works, then it becomes a matter of fixing Time
> Machine...

I did turn it off just now, but no effect. Then again, daemons like revisiond 
were still running...


One more thing: I used the "fs_usage" to monitor what process accessed what 
file during one of those failing "git rebase" runs. I then searched through 
this to determine which processes accessed the "bad" file in this time. The 
result where these processes (aggregated):

  git
  revisiond
  fseventsd
  git-merge-recursive

Note that Time Machine was not running, but revisiond is... from its manpage:

 revisiond is the daemon that manages document revisions created by 
applications and system services.

 There are no configurations to revisiond, and users should not run 
revisiond manually.

This is the process I am suspecting. Specifically, a fchflags call it makes 
(see that attached excerpt of the fs_usage output). I am not sure, but my guess 
would be that it sets SF_ARCHIVED on the file. Causing its ctime to wander... 
Yuck.

But I don't know how to control it... and I am not sure if I can just kill it. 
Hrm. I'll try to dig some more into it, perhaps I can somehow confirm that this 
is *really* the cause of the problems, and somehow prevent it.


Cheers,
Max

00:59:54.349156  lstat64src/BAD_FILE
 0.50   git.623953
00:59:54.349160  open  F=5(R_)  src/BAD_FILE
 0.04   git.623953
00:59:54.350659  close F=5  
 0.07   git.623953
00:59:54.371716  lstat64src/BAD_FILE
 0.02   git.623955
00:59:54.429674  lstat64src/BAD_FILE
 0.02   git.623959
00:59:54.600060  lstat64src/BAD_FILE
 0.07   git.623959
00:59:54.600151  stat64 
/Users/mhorn/the-path-to-my-repository/src/BAD_FILE  0.06   
revisiond.623963
00:59:54.600154  stat64 
/Users/mhorn/the-path-to-my-repository/src   0.03   
revisiond.623963
00:59:54.600160  open  F=7(R_)  
/Users/mhorn/the-path-to-my-repository/src/BAD_FILE  0.06   
revisiond.623963
00:59:54.600161  fstatfs64 F=7  
 0.02   revisiond.623963
00:59:54.600163  close F=7  
 0.02   revisiond.623963
00:59:54.600387  unlink src/BAD_FILE
 0.000328 W git.623959
00:59:54.600429  open  F=5(_WC__E)  src/BAD_FILE
 0.39   git.623959
00:59:54.602910  write F=5B=0x4000  
 0.40   git.623959
00:59:54.602932  write F=5B=0x4000  
 0.17   git.623959
00:59:54.602947  write F=5B=0x4000  
 

Re: rebase: strange failures to apply patc 3-way

[Max Horn]

On 11.03.2013, at 23:10, Andrew Wong wrote:

> On 3/11/13, Max Horn  wrote:
>> PS: Just as a side note, I should mention that I have done tons of rebases
>> on various repositories on this very machine: same hard drive, same file
>> system; the git version of course has changed over time, but as I already
>> described, I can reproduce the same issue with older git versions.
> 
> What if you do a "git clone" from this repo to an entirely new repo? I
> wonder if the rebase issue still happens in the new repo...

The problem seems to be non-existent in a clone. 

> 
> Could you also post the .git/config file from the repo?

[core]
repositoryformatversion = 0
filemode = true
bare = false
logallrefupdates = true
ignorecase = true
precomposeunicode = false

Other than that, it just contains some a [remote] section and several [branch] 
sections. None of these contains any fancy (i.e. the branch sections just say 
"remote = origin" and give the name of the remote branch).

Looking at the git config man page to check what each of my config settings 
does, I discovered "trustctime". And adding
trustctime = false
to .git/config made the rebase work, every single time. Huh. 


Adding this to the fact that a clone works fine, I wonder if  something *is* 
touching my files, but just in that directory. But what could it be? One 
nagging suspicion is the "file versioning" feature Apple introduced as part of 
Time Machine in OS X 10.7; it's kind of a "version control system for n00bs" 
for arbitrary documents. It has caused me some pain in the past.

But I just re-checked, and problematic repos is explicitly on the Time Machine 
exclusion list. I also used the "tmutil isexcluded FILES" to verify that the 
problematic files are really on the TM exclusion list. Finally, I moved the one 
of the repos subdirectory containing most of the problematic files, and then 
run "git checkout". In other instances, this sufficed to "disassociate" a file 
from an unwanted TM version history. But doing that had no effect here, i.e. 
also with the freshly regenerated files, the problems appear.

> 
> If supported, git could actually make use of threading when doing
> "stat"... it should be disabled by default though, but you could try
> disabling it with this config:
>git config core.preloadindex false
> 
> But I don't know why that'd only affect this one repo and not the
> others though...
> 
This setting doesn't seem to have any effect on the issue at hand.


Cheers,
Max--
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: rebase: strange failures to apply patc 3-way

[Max Horn]

On 11.03.2013, at 22:34, Andrew Wong wrote:

> On 3/11/13, Max Horn  wrote:
>> So I tried this:
>> 
>>  git rebase branches/stable-4.6  # ... which leads to the error
>>  git ls-files -m
>> 
>> but got nothing. Perhaps you had something else in mind, though, but I am
>> not quite sure what... sorry for being dense, but if you let me know what
>> exactly you meant, I'll be happy to try that. As for the stat command:
> 
> I'm still digesting the information form the email. Just want to
> quickly mention that the "ls-files" command should be:
>$ git ls-files --debug file1 file2
> 
> You should always get output. It prints out stat-like information for
> files in the index.

Aha, I see. I inserted that into git-am, once right before git-merge-recursive 
and once right after it (just before a call to rerere).


This is what I got;


Applying: COMMIT X
error: BAD-FILE: does not match index
Using index info to reconstruct a base tree...
Falling back to patching base and 3-way merge...
GOOD-FILE
  ctime: 1362921639:0
  mtime: 1362921639:0
  dev: 234881028ino: 18665879
  uid: 502  gid: 20
  size: 25429   flags: 0
BAD-FILE
  ctime: 1363040264:0
  mtime: 1363040264:0
  dev: 234881028ino: 19170773
  uid: 502  gid: 20
  size: 52906   flags: 0
error: Your local changes to the following files would be overwritten by merge:
BAD-FILE
Please, commit your changes or stash them before you can merge.
Aborting
GOOD-FILE
  ctime: 1362921639:0
  mtime: 1362921639:0
  dev: 234881028ino: 18665879
  uid: 502  gid: 20
  size: 25429   flags: 0
BAD-FILE
  ctime: 1363040264:0
  mtime: 1363040264:0
  dev: 234881028ino: 19170773
  uid: 502  gid: 20
  size: 52906   flags: 0
Failed to merge in the changes.
Patch failed at 0015 COMMIT X
...

So the ctime/mtine for the BAD-FILE do not differ.
After this, I also run "ls-files" on the command line, and got:

GOOD-FILE gd
  ctime: 1362921639:0
  mtime: 1362921639:0
  dev: 234881028ino: 18665879
  uid: 502  gid: 20
  size: 25429   flags: 0
BAD-FILE
  ctime: 1363040266:0
  mtime: 1363040264:0
  dev: 234881028ino: 19170773
  uid: 502  gid: 20
  size: 52906   flags: 0

So now the ctime of the bad file is more recent than the mtime.


Max--
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: rebase: strange failures to apply patc 3-way

[Max Horn]

PS: Just as a side note, I should mention that I have done tons of rebases on 
various repositories on this very machine: same hard drive, same file system; 
the git version of course has changed over time, but as I already described, I 
can reproduce the same issue with older git versions.

All I want to say here is: While this may of course be a bug of OS X or my 
machine may be faulty or whatever, it is not as if this issue is occurring all 
over the place. It is so far somehow specific to this repository, or even the 
particular branch. So it is probably not something as simple as a faulty clock, 
because then I should be seeing rebases fail all over the place, right? (Hm, 
OK, admittedly I didn't try any other big rebases in the last couple days. And 
right now I have no good example at hand, i.e. a non-trivial branch that 
cleanly rebases to some other branch)


Cheers,
Max--
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: rebase: strange failures to apply patc 3-way

[Max Horn]

On 11.03.2013, at 20:15, Andrew Wong wrote:

> On 3/10/13, Max Horn  wrote:
>> I did run
>> 
>>  touch lib/*.* src/*.* && git update-index --refresh && git diff-files
>> 
>> a couple dozen times (the "problematic" files where in src/ and lib), but
>> nothing happened. I just re-checked, and the rebase still fails in the same
>> why...
>> 
>> Perhaps I should add some printfs into the git source to figure out what
>> exactly it thinks is not right about those files... i.e. how does it come to
>> the conclusion that I have local changes, exactly. I don't know how Git does
>> that -- does it take the mtime from (l)stat into account? Perhaps problems
>> with my machine's clock could be responsible?
> 
> Instead of using "touch", maybe it'd be better if you run "ls-files"
> and "stat" at the point where rebase failed. You should run the
> command as soon as rebase failed. Don't try to run any git commands,
> as they might change the index state.

So I tried this:

  git rebase branches/stable-4.6  # ... which leads to the error
  git ls-files -m

but got nothing. Perhaps you had something else in mind, though, but I am not 
quite sure what... sorry for being dense, but if you let me know what exactly 
you meant, I'll be happy to try that. As for the stat command:

> 
> And yes, git does make use of mtime and ctime from lstat to some
> degree when detecting file changes. Inserting printf's to print the
> timestamp might help, but the output might be too overwhelming to make
> out useful information, especially during a rebase.
> 
> BTW, it looks like "stat" command on OS X only prints out timestamps
> in seconds, and doesn't show you the nanoseconds part, which may be
> significant in your situation. Instead of using the "stat" command,
> try using this python command to print out the nanoseconds parts:
> python -c "import sys;import os;s=os.stat(sys.argv[1]);print('%d, %f,
> %f' % (s.st_size, s.st_ctime, s.st_mtime))" file1

I can do that, but I am not quite sure what the output should tell me? BTW, I 
think OS X just does not provide the stat information on a nanosecond level, at 
least that python command doesn't seem to yield useful extra data... Here is 
what I got after I just triggered the failing rebase (today it's again a 
different file it fails on that yesterday...). For comparision, I run stat 
first on the file with "conflicts", and then on a file that is not being 
touched by the rebase:

  File: ‘BAD-FILE’
  Size: 21058   Blocks: 48 IO Block: 4096   regular file
Device: e04h/234881028d Inode: 19108072Links: 1
Access: (0644/-rw-r--r--)  Uid: (  502/   mhorn)   Gid: (   20/   staff)
Access: 2013-03-11 21:40:14.0 +0100
Modify: 2013-03-11 21:40:12.0 +0100
Change: 2013-03-11 21:40:14.0 +0100
 Birth: 2013-03-11 21:40:12.0 +0100
  File: ‘NEUTRAL-FILE’
  Size: 1425Blocks: 8  IO Block: 4096   regular file
Device: e04h/234881028d Inode: 18180450Links: 1
Access: (0644/-rw-r--r--)  Uid: (  502/   mhorn)   Gid: (   20/   staff)
Access: 2013-03-11 17:58:30.0 +0100
Modify: 2013-03-10 14:20:39.0 +0100
Change: 2013-03-10 14:20:39.0 +0100
 Birth: 2013-02-27 16:18:57.0 +0100

Here is the output of the python script for the two files:
21058, 1363034414.00, 1363034412.00
1425, 1362921639.00, 1362921639.00


One thing I notice is that ctime equals atime and both are larger than mtime. 
But I have no clue if that has any significance... hmm



> Perhaps you could hack git-am.sh a bit to get more debugging info too.
> Hm, maybe a good place to start is un-silencing the output of "git
> apply". Inside "git-am.sh", you should see a line like:
>git apply $squelch
> Remove the $squelch, and see what output it generates.


error: BAD-FILE: does not match index

I inserted "git ls-files -m" before that but that didn't print anything nor had 
it any other effect.

> 
> Also, since you're getting the 3-way merge, you could also insert the
> "ls-files" and "stat" right after "git-merge-recursive", but before
> "die".
> 

Aha, so that was interesting: I inserted both a "stat" invocation, and also 
invoked the "md5" tool to be able to tell whether the file content matched what 
I thought it should match. Doing that caused the breaking commit to shift. So, 
I added a second stat / md5 pair on the new breaking file. After doing that, 
the rebase suddenly completed! I reset -- hard to the previous state, tried 
again, and again it worked. And this after consistently failing (albeit in 

Re: rebase: strange failures to apply patc 3-way

[Max Horn]

Sorry for taking so long to reply... :-/

On 09.03.2013, at 19:32, Andrew Wong wrote:

> On 03/09/13 06:26, Max Horn wrote:
>> It tends to fail in separate places, but eventually "stabilizes". E.g. I 
>> just did a couple test rebases, and it failed twice in commit 14, then the 
>> third time in commit 15 (which underlines once more that the failures are 
>> inappropriate).
>> 
>> The fourth time, something new and weird happened:
>> 
>> $ git rebase --abort
>> $ git rebase NEW-PARENT 
>> Cannot rebase: You have unstaged changes.
>> Please commit or stash them.
>> $
>> 
>> This is quite suspicious. It appears that git for some reason things a file 
>> is dirty when it isn't. That could explain the other rebase failures too, 
>> couldn't it? But what might cause such a thing?
> Yea, that's really suspicious. This could mean there's an issue with
> when git is checking the index. Try running these a couple times in a
> clean work tree:
>$ git update-index --refresh
>$ git diff-files
> 
> In a clean work tree, these commands should print nothing. But in your
> case, these might print random files that git thinks have been modified...

I did run

  touch lib/*.* src/*.* && git update-index --refresh && git diff-files

a couple dozen times (the "problematic" files where in src/ and lib), but 
nothing happened. I just re-checked, and the rebase still fails in the same 
why...

Perhaps I should add some printfs into the git source to figure out what 
exactly it thinks is not right about those files... i.e. how does it come to 
the conclusion that I have local changes, exactly. I don't know how Git does 
that -- does it take the mtime from (l)stat into account? Perhaps problems with 
my machine's clock could be responsible?


> 
> If the commands do print out some files, check the timestamp from the
> git index and the filesystem:
>$ git ls-files --debug file1 file2
>$ stat -f "%N %m %c" file1 file2
> 
> Is this repo on a network drive? Or is it local drive in your Mac?

Local (some more details also described in my first email on this thread, but 
I'll happily provide more data if I can).

Thanks again,
Max--
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: rebase: strange failures to apply patc 3-way

[Max Horn]

On 08.03.2013, at 20:20, Andrew Wong wrote:

> On 3/8/13, Max Horn  wrote:
>> Same result, it works fine.
> 
> Just shooting in the dark here... I wonder if there's some background
> process running in OS X that's messing with the files/directories
> while rebase is working... backup, virus scan, etc? Or maybe some
> programs that you're using at the same time? Maybe also make sure you
> don't have any programs (shells, editors, etc.) opened that's
> accessing those files/directories?

I am pretty sure no other programs are accessing those files at the same time. 
But just to make sure I quite most programs. No virus scanner running. No 
backup running -- although, OS X automatically runs hourly backups as part of 
Time Machine... So just to be triple certain, I black listed the repos dir in 
both the "Time Machine" backup service and the "Spotlight" indexing service.

No diference. In the end I even did a reboot, but that made no differences 
either (which I am quite relieved about, I must say ;-).


> 
> Does the error always happen at COMMIT A and COMMIT B? Or is it all
> over the place?

It tends to fail in separate places, but eventually "stabilizes". E.g. I just 
did a couple test rebases, and it failed twice in commit 14, then the third 
time in commit 15 (which underlines once more that the failures are 
inappropriate).

The fourth time, something new and weird happened:

$ git rebase --abort
$ git rebase NEW-PARENT 
Cannot rebase: You have unstaged changes.
Please commit or stash them.
$

This is quite suspicious. It appears that git for some reason things a file is 
dirty when it isn't. That could explain the other rebase failures too, couldn't 
it? But what might cause such a thing?


I checked with "git st" and it reported no changed files. Executing the 
"rebase" once again then "worked" as before... I.e. it got stuck in commit 15. 
The next time it got till commit 16. Then back to commit 15. Etc. Now it is 
getting stuck on commit 17 (but it doesn't always go up as it did right now).


> 
> In cases where COMMIT A succeeded, did it say it did a 3-way merge? Or
> was it exactly as the output in your original message? i.e. no message
> at all

It's always a variation of the same message as shown in my original email. I.e.:

Applying: ...
...
Applying: commit XYZ
Using index info to reconstruct a base tree...
Falling back to patching base and 3-way merge...
error: Your local changes to the following files would be overwritten by merge:
some/file
Please, commit your changes or stash them before you can merge.
Aborting
Failed to merge in the changes.
Patch failed at 0015 commit XYZ
The copy of the patch that failed is found in:
   /path/to/repos/.git/rebase-apply/patch





Thanks,
Max--
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: rebase: strange failures to apply patc 3-way

[Max Horn]

Am 08.03.2013 um 19:02 schrieb Andrew Wong :

> On 3/8/13, Max Horn  wrote:
>> I tried this a dozen times, but 'git apply' failed to fail even once. No
>> surprise there, given that the patch that throws off rebase every time is
>> clean and simple. I am flabbergasted :-(
> 
> Hm, what if you add in the "--index" flag? i.e.
>git apply --index .git/rebase-apply/patch
> 
> Wonder if that makes any difference...

Same result, it works fine.
--
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: rebase: strange failures to apply patc 3-way

[Max Horn]

Am 08.03.2013 um 16:32 schrieb Andrew Wong :

> On 3/8/13, Max Horn  wrote:
>> All in all, I suspect that Mac OS X and/or the filesystem (HFS+ with
>> journaling, not case sensitive (the default)) might be at fault. Still, this
>> is quite puzzling and annoying, and so I still wonder if anybody has any
>> insights on this.
> 
> When "rebase" errors out at COMMIT A, try manually running "git apply"
> on the patch file (rebase-apply/patch) a couple times, and see if the
> error occurs randomly. You'd have to do a "reset --hard" to revert the
> changes done by "git apply" every time before you run it again. The
> error from "git apply" might shed more light on the issue.

I tried this a dozen times, but 'git apply' failed to fail even once. No 
surprise there, given that the patch that throws off rebase every time is clean 
and simple. I am flabbergasted :-(

--
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: rebase: strange failures to apply patc 3-way

[Max Horn]

A follow up:

I was able to reproduce this behavior on my Mac with multiple git versions down 
to 1.6.0.6. On the other hand, I copied the whole work tree to a virtual 
machine running Ubuntu, and there the issue did not occur.

All in all, I suspect that Mac OS X and/or the filesystem (HFS+ with 
journaling, not case sensitive (the default)) might be at fault. Still, this is 
quite puzzling and annoying, and so I still wonder if anybody has any insights 
on this.

And: is this a known issue, I wonder?


Cheers,
Max


On 07.03.2013, at 11:16, Max Horn wrote:

> Recently I have observed very strange failures in "git rebase" that cause it 
> to fail to work automatically in situations where it should trivially be able 
> to do so.
> 
> In case it matter, here's my setup: git 1.8.2.rc2.4.g7799588 (i.e. git.git 
> master branch) on Mac OS X. The repos clone is on a HFS+ partition, not on a 
> network volume. No gitattributes are being used.  Regarding the origin of the 
> repos (I hope it doesn't matter, but just in case): The repository in 
> question used to be a CVS repository; it was decided to switch to Mercurial 
> (not my decision ;-) and I performed the conversion; I first converted it to 
> git using cvs2git (from the cvs2svn suite), then performed some history 
> cleanup, and finally used "hg convert" to convert it to git. Recently, I have 
> been accessing the repository from git via the gitifyhg tool 
> <https://github.com/buchuki/gitifyhg>. 
> 
> Anyway, several strange things just happened (and I had similar experiences 
> in the past days and weeks, but that was using an older git, and I thought I 
> was just doing something wrong).
> 
> Specifically, I wanted to rebase a branch with some experimental commits. The 
> strange things started with this:
> 
> $ git rebase MY-NEW-BASE
> First, rewinding head to replay your work on top of it...
> Applying: SOME COMMIT
> Applying: SOME OTHER COMMIT
> ...
> Applying: COMMIT A
> Applying: COMMIT B
> Using index info to reconstruct a base tree...
> Falling back to patching base and 3-way merge...
> error: Your local changes to the following files would be overwritten by 
> merge:
>   some/source.file
> Please, commit your changes or stash them before you can merge.
> Aborting
> Failed to merge in the changes.
> Patch failed at 0014 COMMIT B
> The copy of the patch that failed is found in:
>   /path/to/my/repo/.git/rebase-apply/patch
> 
> When you have resolved this problem, run "git rebase --continue".
> If you prefer to skip this patch, run "git rebase --skip" instead.
> To check out the original branch and stop rebasing, run "git rebase --abort".
> 
> 
> Now, what is strange about this is that the failed patch actually applies 
> cleanly:
> 
> $ patch -p1 < /path/to/my/repo/.git/rebase-apply/patch
> patching file some/source.file
> $
> 
> And there is no subtle merge issue here, either: That patch is the only one 
> to have touched the surrounding code since 1999! There is no source of 
> conflict there!
> 
> Anyway. The tale gets stranger, as I was trying to do this again (no changes 
> were made to the repos in between, this is a straight continuation from 
> above):
> 
> $ git rebase --abort
> $ git rebase MY-NEW-BASE
> First, rewinding head to replay your work on top of it...
> Applying: SOME COMMIT
> Applying: SOME OTHER COMMIT
> ...
> Applying: COMMIT A
> Using index info to reconstruct a base tree...
> Falling back to patching base and 3-way merge...
> error: Your local changes to the following files would be overwritten by 
> merge:
>   some/othersource.file
>   some/yetanother.file
> Please, commit your changes or stash them before you can merge.
> Aborting
> Failed to merge in the changes.
> Patch failed at 0013 COMMIT A
> The copy of the patch that failed is found in:
>   /path/to/my/repo/.git/rebase-apply/patch
> 
> When you have resolved this problem, run "git rebase --continue".
> If you prefer to skip this patch, run "git rebase --skip" instead.
> To check out the original branch and stop rebasing, run "git rebase --abort".
> 
> 
> 
> So suddenly it fails to apply the commit A, the one before the previously 
> failing commit. Huh? But again, the failing patch applies cleanly (and after 
> all, rebase was able to apply it in my previous attempt).  And again, the 
> patch actually applies cleanly. So one more try:
> 
> 
> $ git rebase --abort
> $ git rebase MY-NEW-BASE
> First, rewinding head to replay your work on top of it...
> Applying: SOME COMMIT
> Applying: SOME OTHER COMMIT
> ...
> Applying: COMMIT A
> Using index info to rec

rebase: strange failures to apply patc 3-way

[Max Horn]

Recently I have observed very strange failures in "git rebase" that cause it to 
fail to work automatically in situations where it should trivially be able to 
do so.

In case it matter, here's my setup: git 1.8.2.rc2.4.g7799588 (i.e. git.git 
master branch) on Mac OS X. The repos clone is on a HFS+ partition, not on a 
network volume. No gitattributes are being used.  Regarding the origin of the 
repos (I hope it doesn't matter, but just in case): The repository in question 
used to be a CVS repository; it was decided to switch to Mercurial (not my 
decision ;-) and I performed the conversion; I first converted it to git using 
cvs2git (from the cvs2svn suite), then performed some history cleanup, and 
finally used "hg convert" to convert it to git. Recently, I have been accessing 
the repository from git via the gitifyhg tool 
. 

Anyway, several strange things just happened (and I had similar experiences in 
the past days and weeks, but that was using an older git, and I thought I was 
just doing something wrong).

Specifically, I wanted to rebase a branch with some experimental commits. The 
strange things started with this:

$ git rebase MY-NEW-BASE
First, rewinding head to replay your work on top of it...
Applying: SOME COMMIT
Applying: SOME OTHER COMMIT
...
Applying: COMMIT A
Applying: COMMIT B
Using index info to reconstruct a base tree...
Falling back to patching base and 3-way merge...
error: Your local changes to the following files would be overwritten by merge:
some/source.file
Please, commit your changes or stash them before you can merge.
Aborting
Failed to merge in the changes.
Patch failed at 0014 COMMIT B
The copy of the patch that failed is found in:
   /path/to/my/repo/.git/rebase-apply/patch

When you have resolved this problem, run "git rebase --continue".
If you prefer to skip this patch, run "git rebase --skip" instead.
To check out the original branch and stop rebasing, run "git rebase --abort".


Now, what is strange about this is that the failed patch actually applies 
cleanly:

$ patch -p1 < /path/to/my/repo/.git/rebase-apply/patch
patching file some/source.file
$

And there is no subtle merge issue here, either: That patch is the only one to 
have touched the surrounding code since 1999! There is no source of conflict 
there!

Anyway. The tale gets stranger, as I was trying to do this again (no changes 
were made to the repos in between, this is a straight continuation from above):

$ git rebase --abort
$ git rebase MY-NEW-BASE
First, rewinding head to replay your work on top of it...
Applying: SOME COMMIT
Applying: SOME OTHER COMMIT
...
Applying: COMMIT A
Using index info to reconstruct a base tree...
Falling back to patching base and 3-way merge...
error: Your local changes to the following files would be overwritten by merge:
some/othersource.file
some/yetanother.file
Please, commit your changes or stash them before you can merge.
Aborting
Failed to merge in the changes.
Patch failed at 0013 COMMIT A
The copy of the patch that failed is found in:
   /path/to/my/repo/.git/rebase-apply/patch

When you have resolved this problem, run "git rebase --continue".
If you prefer to skip this patch, run "git rebase --skip" instead.
To check out the original branch and stop rebasing, run "git rebase --abort".



So suddenly it fails to apply the commit A, the one before the previously 
failing commit. Huh? But again, the failing patch applies cleanly (and after 
all, rebase was able to apply it in my previous attempt).  And again, the patch 
actually applies cleanly. So one more try:


$ git rebase --abort
$ git rebase MY-NEW-BASE
First, rewinding head to replay your work on top of it...
Applying: SOME COMMIT
Applying: SOME OTHER COMMIT
...
Applying: COMMIT A
Using index info to reconstruct a base tree...
Falling back to patching base and 3-way merge...
error: Your local changes to the following files would be overwritten by merge:
some/othersource.file
Please, commit your changes or stash them before you can merge.
Aborting
Failed to merge in the changes.
Patch failed at 0013 COMMIT A
The copy of the patch that failed is found in:
   /path/to/my/repo/.git/rebase-apply/patch

When you have resolved this problem, run "git rebase --continue".
If you prefer to skip this patch, run "git rebase --skip" instead.
To check out the original branch and stop rebasing, run "git rebase --abort".


Again it fails in commit A -- but this time, it only thinks one file is 
problematic. HUH? Again, the patch actually applies cleanly:

$ patch -p1 < /path/to/my/repo/.git/rebase-apply/patch
patching file some/othersource.file
patching file some/yetanother.file


At this point, things stabilized, and when I now abort and reattempt the merge, 
it always fails in the same way. This time trying to apply a change to a code 
comment that was last changed in 1997 (though for one hunk, a few lines before 
the changed lines, there is a line that w

Why is ident_is_sufficient different on Windows?

[Max Horn]

Hi there,

while trying to understand which parts of the author & committer identity are 
mandatory (name, email, or both), I ended up in ident.c, looking at 
ident_is_sufficient(), and to my surprise discovered that this seems to differ 
between Windows (were both are mandatory) and everyone else:

static int ident_is_sufficient(int user_ident_explicitly_given)
{
#ifndef WINDOWS
return (user_ident_explicitly_given & IDENT_MAIL_GIVEN);
#else
return (user_ident_explicitly_given == IDENT_ALL_GIVEN);
#endif
}


According to git blame, this was introduced here:

commit 5aeb3a3a838b2cb03d250f3376cf9c41f4d4608e
Author: Junio C Hamano 
Date:   Sun Jan 17 13:54:28 2010 -0800

user_ident_sufficiently_given(): refactor the logic to be usable from 
elsewhere


The commit message sounds as if this was only a refactoring, but the patch to 
me look as if it changes behaviour, too. Of course this could very well be 
false, say due to code elsewhere that already caused Windows to behave 
differently; I wouldn't know.


Still, I wonder: Why does this difference exist?


Cheers,
Max


--
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: Anybody know a website with up-to-date git documentation?

[Max Horn]

On 30.01.2013, at 16:59, Sitaram Chamarty wrote:

> I'm curious... what's wrong with 'git checkout html' from the git repo
> and just browsing them using a web browser?

Hm, do you mean "make html", perhaps? At least I couldn't figure out what "git 
checkout html" should do, but out of curiosity gave it a try and got an error...

But supposing that you meant "make html": There is nothing "wrong" with it. 
This is mostly a matter of convenience:

* Many people just use git and don't have the git.git repos (or any git 
sources) at hand.  And while for many things, older versions of the reference 
pages may suffice, this is not always the case.

* When I want to point somebody at something specific in the git docs while, 
say, while discussing on IRC or a mailing list, it is very convenient to point 
them at a website, like this:
   
http://git-htmldocs.googlecode.com/git/git-fast-import.html#_code_notemodify_code
 

* Similarly if I am standing physically next to somebody sitting at their 
computer and they ask me something about git, it is nice to be able to send 
them to a current version of the docs online

* I can access the web version from my tablet -- and I actually do that (use my 
tablet as "secondary screen" showing some git refs and other docs while coding 
on my laptop).

* a website can be update by one person (or ideally: one script) and serve many 
people with the same need Seems more efficient than each of those people 
setting up an appropriate clone & a cron job to keep it up-to-date on each 
machine where they need it.


But of course, the "make html" has its own clear advantages, e.g. I can use it 
online, I have full control over which exact version of the docs I get, 
including most recent changes, etc. To me, the two complement each other.


Anyway, I'll stop spamming the list, I got my answers from John and Junio:

  http://git-htmldocs.googlecode.com/git/git.html

and in addition 

  http://manned.org/git.1


Thanks again,
Max--
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: Anybody know a website with up-to-date git documentation?

[Max Horn]

On 30.01.2013, at 12:54, John Keeping wrote:

> On Wed, Jan 30, 2013 at 12:46:47PM +0100, Max Horn wrote:
>> does anybody know a website where one can view that latest git
>> documentation? Here, "latest" means "latest release" (though being
>> also able to access it for "next" would of course be a nice bonus,
>> likewise for older versions). While I do have those docs on my local
>> machine, I would like to access them online, too (e.g. easier to
>> pointer people at this, I can access it from other machines, etc.).
> 
> How about http://git-htmldocs.googlecode.com/git/ ?
> 
> It's just a directory listing of the git-htmldocs repository that Junio
> maintains - the latest update was yesterday: Autogenerated HTML docs for
> v1.8.1.2-422-g08c0e.
> 
> [I didn't know Google Code let you view the repository like that, but I
> got there by clicking the "raw" link against one of the files so I
> assume it's not likely to go away.]
> 

Thanks John, that looks pretty good!

In addition, I just discovered

   http://manned.org/git-remote-helpers/2b9e4c86

which contains git docs from Arch Linux, Debian, FreeBSD and Ubuntu packages. 
And since Arch tends to have the latest, so does manned.org. And best, it even 
lets me browser to older versions of a file.

So, taken together, I guess that solves my problem -- with John's link, I can 
see the bleeding edge versions, with manned.org the latest released version (as 
soon as Arch Linux catches up, which seems to be pretty quick :-).


Thanks!
Max--
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: Anybody know a website with up-to-date git documentation?

[Max Horn]

Hi Sebastian,

On 30.01.2013, at 12:56, Sebastian Staudt wrote:

> Hello Max,
> 
> git-scm.com is the best source and it's not outdated.

Then it seems you are using the word "outdated" in a different way than me 
which I don't understand :-). Sure, it strives to be up-to-date, but fact is 
that it fails to do that, due to a bug (I guess). The end result (failure to 
update at all, vs. failure in an attempted update) sadly amount to the same.

> It gets an
> update after every single release of Git.
> See e.g. http://git-scm.com/docs/git-config which was updated in the
> current stable version.
> It seems that git-remote-helper's documentation was just not updated
> since version 1.7.12.3.

Yes, and it is not alone in that, which makes the site somewhat unreliable, 
sadly. Some more examples of pages tuck at version 1.7.12.3 and showing 
outdated content:

http://git-scm.com/docs/git-log
http://git-scm.com/docs/git-merge
http://git-scm.com/docs/git-merge-base
http://git-scm.com/docs/git-mergetool
http://git-scm.com/docs/git-reset
http://git-scm.com/docs/git-rm
http://git-scm.com/docs/git-status
http://git-scm.com/docs/git-symbolic-ref

I did not bother to check every single page, though, and I am pretty sure there 
are plenty more. Because there definitely is a plethora of other pages that are 
stuck at 1.7.12.3. Several of them still show the latest version due to not 
having had updates since the 1.7.12.3, but that is not always easy to tell due 
to included files (e.g. git-log.txt was not changed v1.7.12.2, but it includes 
rev-list-options.txt which was last changed in 1.8.1).


So, yeah, I do think git-scm.com is outdated -- in the sense that for many 
pages, it does not show the latest officially released version of the page.


Best regards,
Max--
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

Anybody know a website with up-to-date git documentation?

[Max Horn]

Hi,

does anybody know a website where one can view that latest git documentation? 
Here, "latest" means "latest release" (though being also able to access it for 
"next" would of course be a nice bonus, likewise for older versions). While I 
do have those docs on my local machine, I would like to access them online, too 
(e.g. easier to pointer people at this, I can access it from other machines, 
etc.).


My problem is that all sites I know of are outdated, and thus don't show recent 
improvements. Also, for many it is hard to determine for which version of git 
they carry documentation. Here are the contenders I know, and the problems they 
have:


* The closest I know is http://git-scm.com/ -- they fit the bill almost 
perfectly. Except that sadly, some pages that are crucial for me are 
permanently stuck at outdated versions, like 
http://git-scm.com/docs/git-remote-helpers which is stuck at 1.7.12.3. I tried 
contacting them about this for two months now, but to no avail (multiple bug 
reports, direct emails, etc. all went w/o reaction). Of course time and 
resources are limited, so I fully understand and respect that the people behind 
it (Scott Chacon in particular, who did an awesome job creating that site in 
the first place) have other priorities.

* http://www.kernel.org/pub/software/scm/git/docs/ was last updated in May 
2012. No hints on who maintains this and how to contact them. Attempts to 
contact kernel.org webadmins to find out more were not answered either :-(. 
Anybody know more?

* http://schacon.github.com/git/git-remote-helpers.html was lasted updated in 
May 2011. I assume git-scm.com is supposed to replace it, though, as Scott 
Chacon made git-scm.com. (In that case, a redirect to git-scm.com might be nice 
*g* but of course is extra work) 

* http://www.manpagez.com/man/1/git/ and http://man.he.net/man1/git at least 
document on each page from which git version it is taken. Unfortunately, both 
are stuck at the 1.7.x series.

* http://linux.die.net/man/1/git does not indicate the git version, but it 
seems to be a 1.7.x, too


Anybody know an up-to-date alternative? Or do I have to setup my own? :-(.


Cheers,
Max--
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: Updating shared ref from remote helper, or fetch hook

[Max Horn]

Hi Jed, all,


On 28.01.2013, at 06:19, Jed Brown wrote:

> I'm working on an hg remote helper that uses git notes for the sha1
> revision, so that git users can more easily refer to specific commits
> when communicating with hg users.

For the record, I am also working on that very same thing; it is yet another 
git-remote-hg alternative, based on Felipe's code but with many improvements. 
You can find it here: .

Anyway, back to Jed's (and also my) question:

>  Since there may be multiple
> concurrent fast-import streams, I write the notes to a private ref
> (refs/notes/hg-REMOTE), to be merged eventually using
> 
>  git notes --ref hg merge hg-REMOTE*
> 
> There will never be conflicts because each hg commit translates to a
> unique git commit, thus even if multiple concurrent remotes process the
> same commit, the corresponding note will match.
> 
> Unfortunately, I couldn't find a safe way to get this run after a fetch
> or clone.  Of course I can ask the user to arrange to have this command
> run, but it would be a better interface to have it run automatically
> since it is a natural responsibility of the remote helper.  Am I missing
> a way to do this or a viable alternative approach?

One idea we (well, Jed :-) had when brain storming about this was that perhaps 
one could (or even should?) (ab)use the "checkpoint" feature for that.

Basically, when the remote-helper is almost done with everything, issue a 
"checkpoint" command, to flush out everything we just imported to the HD. 

Then once this completed, we can perform the notes merge. The main remaining 
problem with that is: How would we know when the "checkpoint" actually 
completed? Any ideas?

Perhaps a way to do that would be to make use of the new "bidi-import" remote 
helper capability -- if I understand it right, then this effectively connects 
the fast-import stdout to the stdin of the remote helper. Thus, if one were to 
follow the "checkpoint" by a "progress" command, then by waiting for that 
progress command's output to appear back on stdin, the remote helper could 
determine whether the import succeeded, and perform finalization actions (like 
merging notes, as in our case).


Does that sound viable? Crazy? Anybody got better a idea?


Cheers,
Max--
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: git pull - reporting that I modified files, but I did not

[Max Horn]

On 17.01.2013, at 20:29, Jay Vee wrote:

> When I do a git pull, I am getting a messages that changes to local
> files would be overwritten by a merge, but  I have not changed these
> files locally at all, I have not opened them in my IDE.
> This happens every now and then.
> 
> 1) Why does this happen?

This is hard to tell given the little information you provide. But clearly 
*something* modified those files, whether consciously triggered by you or not. 
But files don't magically change themselves :-). I recommend that you run "git 
diff" on those files to figure out in what way they changed -- this will likely 
provide a clue to the cause for this.

All I can say is that it is extremely unlikely that git did this, unless it 
happens to be something you (or somebody who has access to your git config 
resp. the repository config) explicitly activated and thus requested from git 
(e.g. via clean or smudge filters in gitattributes).



> 2) How do I prevent this from happening in the future?

As this largely depends on the cause, it can't be answered before 1) is 
answered...

> 
> 3) How do I get out of this state so that I can do a git pull and
> rebuild my code?

At least over here, when I do a "git status", it actually prints a nice message 
that explains how to do this. For example, I see something like this:

# On branch next
# Changes to be committed:
#   (use "git reset HEAD ..." to unstage)
#
#   modified:   src/MyClass.java
#   deleted:src/AnotherClass.java
#
# Changes not staged for commit:
#   (use "git add ..." to update what will be committed)
#   (use "git checkout -- ..." to discard changes in working directory)
#
#   modified:   src/MyModifiedClass.java
#   deleted:src/YetAnotherClass.java
...


But I would strongly urge you to first review those changes, to make sure that 
they are really OK to discard. For example, you wouldn't want to throw away a 
change you did make on purpose but forgot to commit.


> 
> ---
> In other instances, when I do a git pull (not getting the message
> above,  I will see something like:
> M  src/MyClass.java  <= a file that I did not touch or modify
> D   src/AnotherClass.java   <= a file that I did not delete or touch
> M src/MyModifiedClass.java   <= a file that I indeed modified for
> which in the pull there are no merge conflicts.

Hmm, where is this output from?

> 
> and the pull is successful, (then I want to push my changes), but I
> did not change either of the above two files

Did you try to find out what the change in them is? E.g. using "git diff" ? My 
guess would be that the nature of the changes would give a big clue as to their 
cause. E.g. did indention change? Line breaks? Where RCS keywords expanded / 
contracted? Was a random piece of code inserted somewhere?


> 
> If I see the above, am I OK to push?  My thinking is that git thinks I
> changed 'src/MyClass.java' and if I do a diff there are differences,
> but I do not want to push because I NEVER TOUCHED THAT FILE IN ANY
> WAY.

Shouting doesn't help :-). Something on your computer *did* touch the file. Git 
does not magically change your stuff -- unless in very special cases, but then 
only when *explicitly* configured to do so by somebody.

> 
> What is going on here?  Maybe this is normal and I simply do not
> understand correctly.

It doesn't sound "normal", but it doesn't sound like an issue with git either, 
more like one with your particular setup. It is hard to say more since you give 
us very little concrete information. E.g. what kind of changes are in those 
files? Which git version, and what OS are you on? Do you use git via the 
command line exclusively, or do you use a frontend(s) for it? etc.


> 
> What is happening?  I would expect to see only line items 'M' and 'D'
> for files that I personally have modified and deleted.

What do you mean by "personally"? Is a cron job you setup and forgot counting 
towards this, too? What about automatic changes caused by an IDE or a Git 
frontend? What about automatic changes caused by a git config setting you or a 
friendly co-worker setup in your gitconfig but you forgot about? 


> If I push at this point, will I overwrite changes in the repo pushed
> by others and muck things up?

A push will only push any commits you made. And a commit will only include 
changes you explicitly staged via "git add", or told "git commit" to include by 
listing files when doing the commit... So if you worry that what you are about 
to push is broken, I'd recommend to a) review all local commits and their 
contents before pushing them, and b) testing your work before pushing it.


Cheers,
Max--
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 some clang warnings

[Max Horn]

On 16.01.2013, at 18:18, John Keeping wrote:

> On Wed, Jan 16, 2013 at 06:12:57PM +0100, Antoine Pelisse wrote:
>> FWIW, I also happen to have the warning:
>> 
>> advice.c:69:2: warning: expression result unused [-Wunused-value]
>>error("'%s' is not possible because you have unmerged files.", me);
>>^~
>> ./git-compat-util.h:314:55: note: expanded from:
>> #define error(fmt, ...) (error((fmt), ##__VA_ARGS__), -1)
>>  ^~
>> 
>> with clang: Ubuntu clang version 3.0-6ubuntu3 (tags/RELEASE_30/final)
>> (based on LLVM 3.0)
> 
> I have the same output with:
> 
> clang version 3.2 (tags/RELEASE_32/final)

Sorry for not being more specific in my message. I have this with 

Apple clang version 4.1 (tags/Apple/clang-421.11.66) (based on LLVM 3.1svn)


Max
--
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] fix some clang warnings

[Max Horn]

Signed-off-by: Max Horn 
---
 cache.h   | 2 +-
 git-compat-util.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/cache.h b/cache.h
index c257953..5c8440b 100644
--- a/cache.h
+++ b/cache.h
@@ -1148,7 +1148,7 @@ extern int check_repository_format_version(const char 
*var, const char *value, v
 extern int git_env_bool(const char *, int);
 extern int git_config_system(void);
 extern int config_error_nonbool(const char *);
-#ifdef __GNUC__
+#if defined(__GNUC__) && ! defined(__clang__)
 #define config_error_nonbool(s) (config_error_nonbool(s), -1)
 #endif
 extern const char *get_log_output_encoding(void);
diff --git a/git-compat-util.h b/git-compat-util.h
index 4f022a3..cc2abee 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -310,7 +310,7 @@ extern void warning(const char *err, ...) 
__attribute__((format (printf, 1, 2)))
  * behavior. But since we're only trying to help gcc, anyway, it's OK; other
  * compilers will fall back to using the function as usual.
  */
-#ifdef __GNUC__
+#if defined(__GNUC__) && ! defined(__clang__)
 #define error(fmt, ...) (error((fmt), ##__VA_ARGS__), -1)
 #endif
 
-- 
1.8.1.1.435.g4e2ebdf

--
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 v6 0/8] push: update remote tags only with force

[Max Horn]

Hi there,

I was just working on improving git-remote-helper.txt by documenting how remote 
helper can signal error conditions to git. This lead me to notice a (to me) 
surprising change in behavior between master and next that I traced back to 
this patch series.

Specifically:

On 30.11.2012, at 02:41, Chris Rorvick wrote:

> This patch series originated in response to the following thread:
> 
>  http://thread.gmane.org/gmane.comp.version-control.git/208354
> 
> I made some adjustments based on Junio's last round of feedback
> including a new patch reworking the "push rules" comment in remote.c.
> Also refined some of the log messages--nothing major.  Finally, took a
> stab at putting something together for the release notes, see below.

>From the discussion in that gmane thread and from the commits in this series, 
>I had the impression that it should mostly affect pushing tags. However, this 
>is not the case: It also changes messages upon regular push "conflicts. 
>Consider this test script:


#!/bin/sh -ex
git init repo_orig
cd repo_orig
echo a > a
git add a
git commit -m a
cd ..

git clone repo_orig repo_clone

cd repo_orig
echo b > b
git add b
git commit -m b
cd ..

cd repo_clone
echo B > b
git add b
git commit -m B
git push


With git 1.8.1, I get this message:

 ! [rejected]master -> master (non-fast-forward)
error: failed to push some refs to 
'/Users/mhorn/Projekte/foreign/gitifyhg/bugs/git-push-conflict/repo_orig'
hint: Updates were rejected because the tip of your current branch is behind
hint: its remote counterpart. Merge the remote changes (e.g. 'git pull')
hint: before pushing again.
hint: See the 'Note about fast-forwards' in 'git push --help' for details.



But with next, I get this:


 ! [rejected]master -> master (already exists)
error: failed to push some refs to 
'/Users/mhorn/Projekte/foreign/gitifyhg/bugs/git-push-conflict/repo_orig'
hint: Updates were rejected because the destination reference already exists
hint: in the remote.


This looks like a regression to me. No tags were involve, and the new message 
is very confusing if not outright wrong -- at least in my mind, but perhaps I 
am missing a way to interpret it "correctly" ? What am I missing?


Cheers,
Max

--
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] remote-hg: store converted URL

[Max Horn]

On 15.01.2013, at 17:51, Junio C Hamano wrote:

> Junio C Hamano  writes:
> 
>> Max Horn  writes:
>> ...
>>> See also the discussion (yeah, this time a real one ;-) leading to this:
>>>  https://github.com/felipec/git/issues/2
>>> ...
> 
> If I understand correctly, the $backend::$opaqueToken is a contract
> between the remote-helper and the remote-$backend

Just to clarify: What is the "remote-helper" ? So far, I thought of this being 
a contract between git (or some part of git), and remote-$backend (i.e. 
remote-hg in this case). 


> that says "When
> user wants to interact with the same (foreign) repository, we agreed
> to let her use 'origin' nickname.  The remote-helper looks up this
> opaque token that corresponds to 'origin' and gives it to the
> remote-$backend, and whatever is in the opaque token should be
> sufficient for the remote-$backend to figure out how to proceed from
> there".

Supposing that I interpret "remote-helper" correctly, that sounds about right 
to me.

> 
> But in this hg::../over/there case, it seems that string is not
> sufficient for remote-hg to do so and the contract is broken.

One could put it that way.

> 
> When "git clone $backend::$opaqueToken repo" is run in /dir/ecto/ry,
> and then subsequent "git fetch origin" will be run in (a
> subdirectory of) /dir/ecto/ry/repo, but anything relative to
> /dir/ecto/ry will not work once you go inside /dir/ecto/ry/repo.
> The "create a new repository here" argument could even be an
> absolute path to a totally different place, so if the
> remote-$backend wants to use $opaqueToken as anything relative to
> the $(cwd) when "git clone" was invoked, that original location
> needs to be available somehow.

That would be one option. Another is to do what the proposed patch does, and 
what git itself does: Change the relative path into an absolute one. This 
requires remote-$backend to be able to modify the opaque token supplied by the 
user.

Yet another would be to be more strict in remote-$backend as to which opaque 
tokens to accept: When it contains a relative path, simply always refuse to 
work, even if the PWD happens to be set the right way. Of course this would be 
quite undesirable from a user's perspective.


> 
> Would a new helper protocol message be necessary, so that the
> backend can rewrite the $opaqueToken at "clone" time and tell the
> helper what to store as URL instead of the original?  I do not think
> that is much different from remote-$backend updating the value of the
> remote.origin.URL using "git config".
> 
> An alternative approach may be for somebody (either the "git clone"
> or the remote-$backend) to store a "base directory" when "git clone"
> was invoked in remote.origin.dirAtCloneTime variable, so that the
> next time remote-$backend runs, it can read that directory and
> interpret the $opaqueToken as a relative path to that directory if
> it wants to.  That way, nobody needs to rewrite $opaqueToken.

As I said above, this would be an option. However, I would prefer rewriting the 
$opaqueToken, as that would be closer to what git does for "native" tokens 
passed to "git clone" Specifically, I am talking about get_repo_path() in 
builtin/clone.c which is called by cmd_clone. What this does is in my eyes 
essentially the equivalent of what the patch discussed here is doing.

Anyway, at the end of the day, I mainly care about relative paths working, 
somehow :-). But I think it would be important to make the issue easy to 
resolve for all remote-$backend authors, as many of them are affected (see 
below).

> 
> How do other remote helpers solve this, I have to wonder, though.
> By not allowing relative paths to a directory?

So far, all I look at do not deal with this at all. Any attempts to deal with 
it should be pretty easy to recognize: The remote-$backend would have to store 
something into the git config, or else, verify the opaque token and refuse to 
work with it under certain conditions (e.g. when it contains a relative path). 
But they don't. E.g. git-remote-testgit has the exact same problem. Doing

   git init repo && cd repo && echo a > a && git add a && git ci -m a a
   git clone testgit::repo clone
   cd clone

results in a .git/config file containing

[remote "origin"]
url = testgit::repo
fetch = +refs/heads/*:refs/remotes/origin/*

Trying to do a "git push" from within the clone then gives this:

fatal: 'repo/.git' does not appear to be a git repository
fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the reposito

Re: [PATCH] remote-hg: store converted URL

[Max Horn]

On 15.01.2013, at 17:05, Junio C Hamano wrote:

> Max Horn  writes:
> 
>> On 14.01.2013, at 19:14, Junio C Hamano wrote:
>> 
>>> What is lacking from this description is why it even needs to work
>>> from a different working directory
> 
> In your rewrite below, this is still lacking, I think.

Hm, I thought I made it clear: It has to change because relative paths only 
make sense when you know the reference point they are relative with.

Typically. This is the pwd. But when I 

  git clone repo newrepo
  cd newrepo

I just changed the PWD. The clone command was given the relative path "repo". 
If git were to use that, it would suddenly refer to a directory inside newrepo, 
not next to it. Bang. Hence, git expands the relative path to an absolute one 
in the above example.

But git cannot do that for URLs in the form HELPER::PATH, because such a string 
is necessarily opaque to git.



>> Thus when latter attempting to, say, "git pull"
>> from inside gitrepo, remote-hg cannot resolve the relative path correctly,
>> and the user sees an unexpected error.
> 
> ... "cannot resolve the relative path correctly" above sound like a
> bug in remote-hg.  Something like:
> 
>Cloning a local hg repository using a relative path, e.g.
> 
>  git clone hg::hgrepo gitrepo
> 
>stores "hg::hgrepo" in gitrepo/.git/config as its URL.  When
>remote-hg is invoked by "git fetch", it chdirs to X (which is
>different from the "gitrepo" directory) and uses the URL (which
>is not correct, as it is a relative path but the cwd is
>different when it is used) to interact with the original
>"hgrepo", which will fail.
> 
> is needed, but you didn't explain what that X is.  Perhaps it is a
> temporary directory.  Perhaps it is a hidden Hg repository somewhere
> in gitrepo/.git directory.  Or something else.

None of the above. Nor does the remote helper chdir anywhere. It is the user 
who has done the chdir: Away from the location he invoked "git clone" at, and 
into the new repository directory that previously did not even exist.



Cheers,
Max--
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] remote-hg: fix handling of file perms when pushing

[Max Horn]

On 15.01.2013, at 14:02, Max Horn wrote:

> Previously, when changing and committing an executable file, the file
> would loose its executable bit on the hg side. Likewise, symlinks ended
> up as "normal" files". This was not immediately apparent on the git side
> unless one did a fresh clone.

Sorry, forgot to sign off, please add:

Signed-off-by: Max Horn 

Max
--
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] remote-hg: fix handling of file perms when pushing

[Max Horn]

Previously, when changing and committing an executable file, the file
would loose its executable bit on the hg side. Likewise, symlinks ended
up as "normal" files". This was not immediately apparent on the git side
unless one did a fresh clone.
---
 contrib/remote-helpers/git-remote-hg |  2 +-
 contrib/remote-helpers/test-hg-hg-git.sh | 68 
 2 files changed, 69 insertions(+), 1 deletion(-)

diff --git a/contrib/remote-helpers/git-remote-hg 
b/contrib/remote-helpers/git-remote-hg
index 7c74d8b..328c2dc 100755
--- a/contrib/remote-helpers/git-remote-hg
+++ b/contrib/remote-helpers/git-remote-hg
@@ -53,7 +53,7 @@ def gittz(tz):
 return '%+03d%02d' % (-tz / 3600, -tz % 3600 / 60)
 
 def hgmode(mode):
-m = { '0100755': 'x', '012': 'l' }
+m = { '100755': 'x', '12': 'l' }
 return m.get(mode, '')
 
 def get_config(config):
diff --git a/contrib/remote-helpers/test-hg-hg-git.sh 
b/contrib/remote-helpers/test-hg-hg-git.sh
index 3e76d9f..7e3967f 100755
--- a/contrib/remote-helpers/test-hg-hg-git.sh
+++ b/contrib/remote-helpers/test-hg-hg-git.sh
@@ -109,6 +109,74 @@ setup () {
 
 setup
 
+test_expect_success 'executable bit' '
+   mkdir -p tmp && cd tmp &&
+   test_when_finished "cd .. && rm -rf tmp" &&
+
+   (
+   git init -q gitrepo &&
+   cd gitrepo &&
+   echo alpha > alpha &&
+   chmod 0644 alpha &&
+   git add alpha &&
+   git commit -m "add alpha" &&
+   chmod 0755 alpha &&
+   git add alpha &&
+   git commit -m "set executable bit" &&
+   chmod 0644 alpha &&
+   git add alpha &&
+   git commit -m "clear executable bit"
+   ) &&
+
+   for x in hg git; do
+   (
+   hg_clone_$x gitrepo hgrepo-$x &&
+   cd hgrepo-$x &&
+   hg_log . &&
+   hg manifest -r 1 -v &&
+   hg manifest -v
+   ) > output-$x &&
+
+   git_clone_$x hgrepo-$x gitrepo2-$x &&
+   git_log gitrepo2-$x > log-$x
+   done &&
+   cp -r log-* output-* /tmp/foo/ &&
+
+   test_cmp output-hg output-git &&
+   test_cmp log-hg log-git
+'
+
+test_expect_success 'symlink' '
+   mkdir -p tmp && cd tmp &&
+   test_when_finished "cd .. && rm -rf tmp" &&
+
+   (
+   git init -q gitrepo &&
+   cd gitrepo &&
+   echo alpha > alpha &&
+   git add alpha &&
+   git commit -m "add alpha" &&
+   ln -s alpha beta &&
+   git add beta &&
+   git commit -m "add beta"
+   ) &&
+
+   for x in hg git; do
+   (
+   hg_clone_$x gitrepo hgrepo-$x &&
+   cd hgrepo-$x &&
+   hg_log . &&
+   hg manifest -v
+   ) > output-$x &&
+
+   git_clone_$x hgrepo-$x gitrepo2-$x &&
+   git_log gitrepo2-$x > log-$x
+   done &&
+
+   test_cmp output-hg output-git &&
+   test_cmp log-hg log-git
+'
+
 test_expect_success 'merge conflict 1' '
mkdir -p tmp && cd tmp &&
test_when_finished "cd .. && rm -rf tmp" &&
-- 
1.8.1.448.g79c577a.dirty

--
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] remote-hg: store converted URL

[Max Horn]

On 14.01.2013, at 19:14, Junio C Hamano wrote:

> Max Horn  writes:
> 
>> From: Felipe Contreras 
>> 
>> Mercurial might convert the URL to something more appropriate, like an
>> absolute path.
> 
> "What it is converted *TO*" is fairly clear with ", like an ...",
> but from the first reading it was unclear to me "what it is
> converted *FROM*" and "*WHEN* the conversion happens".  Do you mean
> that the user gives "git clone" an URL "../hg-repo" via the command
> line (e.g. the argument to "git clone" is spelled something like
> "hg::../hg-repo"), and that "../hg-repo" is rewritten to something
> else (an absolute path, e.g. "/srv/project/hg-repo")?

Yes, that was meant. 

> 
>> Lets store that instead of the original URL, which won't
>> work from a different working directory if it's relative.
> 
> What is lacking from this description is why it even needs to work
> from a different working directory.  I am guessing that remote-hg
> later creates a hidden Hg repository or something in a different
> place and still tries to use the URL to interact with the upstream,
> and that is what breaks, but with only the above description without
> looking at your original report, people who will read the "git log"
> output and find this change will not be able to tell why this was
> needed, I am afraid.
> 
> Of course, the above guess of mine may even be wrong, but then that
> is yet another reason that the log needs to explain the change
> better.

Fully agreed. How about this commit message:

-- >8 --
remote-hg: store converted URL of hg repo in git config

When remote-hg is invoked, read the remote repository URL from the git config,
give Mercurial a chance to expand it, and if changed, store it back into
the git config.

This fixes the following problem: Suppose you clone a local hg repository
using a relative path, e.g.
  git clone hg::hgrepo gitrepo
This stores "hg::hgrepo" in gitrepo/.git/config. However, no information
about the PWD is stored, making it impossible to correctly interpret the
relative path later on. Thus when latter attempting to, say, "git pull"
from inside gitrepo, remote-hg cannot resolve the relative path correctly,
and the user sees an unexpected error.

With this commit, the URL "hg::hgrepo" gets expanded (during cloning,
but also during any other remote operation) and the resulting absolute
URL (e.g. "hg::/abspath/hgrepo") is stored in gitrepo/.git/config.
Thus the git clone of hgrepo becomes usable.
-- >8 --

> 
>> Suggested-by: Max Horn 
>> Signed-off-by: Felipe Contreras 
>> Signed-off-by: Max Horn 
>> ---
>> For a discussion of the problem, see also
>>  http://article.gmane.org/gmane.comp.version-control.git/210250
> 
> I do not see any discussion; only your problem report.

Aha, an english language issue on my side I guess: For me, a single person can 
"discuss" a problem (often, a research paper is said to be "discussing a 
problem"). Sorry for that. Anyway, the reason I gave that link was because it 
attempts explains the problem and one solution (which this patch ended up 
implementing), but also express that I feel a bit uncomfortable with this. 
Which I still do. Relying on the remote helper to invoke "git config" feels 
like a hack and I was wondering whether this is deemed an acceptable solution 
-- or whether one should instead extend the remote-helper protocol, allowing 
the remote helper to signal a rewritten remote URL (perhaps only directly after 
a clone?). As it is, the remote helper seems (?) to have no way to distinguish 
whether it is being called during a clone or a pull; hence it has to "expand" 
and rewrite the URL every time it is called, just in case.


Anyway, as long as this particular command works somehow, I am fine:

  git clone hg::../relative/path/to/hg-repo  git-repo


> Was this work done outside the list?  I just want to make sure this
> patch is not something Felipe did not want to sign off for whatever
> reason but you are passing it to the list as a patch signed off by
> him.

The work was done by Felipe's and published in his github repository:
  https://github.com/felipec/git/commit/605bad5b52d2fcf3d8f5fd782a87d7c97d1b040a
See also the discussion (yeah, this time a real one ;-) leading to this:
  https://github.com/felipec/git/issues/2

I took his sign-off from there and interpreted it as saying that Felipe was OK 
with this being pushed to git.git. But perhaps this is not what I should have 
done? In that case I am very sorry :-(. It's just that I feel this patch is 
quite useful and important for daily use (which is why I suggested it in the 
first place ;-), so I w

[PATCH] remote-hg: store converted URL

[Max Horn]

From: Felipe Contreras 

Mercurial might convert the URL to something more appropriate, like an
absolute path. Lets store that instead of the original URL, which won't
work from a different working directory if it's relative.

Suggested-by: Max Horn 
Signed-off-by: Felipe Contreras 
Signed-off-by: Max Horn 
---
For a discussion of the problem, see also
  http://article.gmane.org/gmane.comp.version-control.git/210250
While I am not quite happy with using "git config" to solve it, there
doesn't seem to be a better way right now.

 contrib/remote-helpers/git-remote-hg | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/contrib/remote-helpers/git-remote-hg 
b/contrib/remote-helpers/git-remote-hg
index c700600..7c74d8b 100755
--- a/contrib/remote-helpers/git-remote-hg
+++ b/contrib/remote-helpers/git-remote-hg
@@ -720,6 +720,14 @@ def do_export(parser):
 if peer:
 parser.repo.push(peer, force=False)
 
+def fix_path(alias, repo, orig_url):
+repo_url = util.url(repo.url())
+url = util.url(orig_url)
+if str(url) == str(repo_url):
+return
+cmd = ['git', 'config', 'remote.%s.url' % alias, "hg::%s" % repo_url]
+subprocess.call(cmd)
+
 def main(args):
 global prefix, dirname, branches, bmarks
 global marks, blob_marks, parsed_refs
@@ -766,6 +774,9 @@ def main(args):
 repo = get_repo(url, alias)
 prefix = 'refs/hg/%s' % alias
 
+if not is_tmp:
+fix_path(alias, peer or repo, url)
+
 if not os.path.exists(dirname):
 os.makedirs(dirname)
 
-- 
1.8.0.1.525.gaaf5ad5

--
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: cvsps, parsecvs, svn2git and the CVS exporter mess

[Max Horn]

On 03.01.2013, at 21:53, Eric S. Raymond wrote:

> Michael Haggerty :
>> There are two good reasons that the output is written to two separate files:
> 
> Those are good reasons to write to a pair of tempfiles, and I was able
> to deduce in advance most of what your explanation would be from the
> bare fact that you did it that way.
> 
> They are *not* good reasons for having an interface that exposes this
> implementation detail to the caller - that choice I consider a failure
> of interface-design judgment.  But I know how to fix this in a simple and
> backward-compatible way, and will do so when I have time to write you
> a patch.  Next week or the week after, most likely.
> 
> Also, the cvs2git manual page is still rather half-baked and careless,
> with several fossil references to cvs2svn that shouldn't be there and
> obviously incomplete feature coverage. Fixing these bugs is also on my
> to-do list for sometime this month.
> 
> I'd be willing to put in this work anyway, but it still in the back of
> my mind that if cvs2git wins the test-suite competition I might
> officially end-of-life both cvsps and parsecvs.  One of the features
> of the new git-cvsimport is direct support for using cvs2git as a
> conversion engine.
> 
>> A potentially bigger problem is that if you want to handle such
>> blob/dump output, you have to deal with git-fast-import format's "blob"
>> command as opposed to only handling inline blobs.
> 
> Not a problem.  All of the main potential consumers for this output,
> including reposurgeon, handle the blob command just fine.

Hm, you snipped this part of Michael's mail:

>> However, if that is a
>> problem, it is possible to configure cvs2git to write the blobs inline
>> with the rest of the dumpfile (this mode is supported because "hg
>> fast-import" doesn't support detached blobs).

I would call "hg fast-import" a main potential customer, given that there 
"cvs2hg" is another part of the cvs2svn suite. So I can't quite see how you can 
come to your conclusion above...



Cheers,
Max--
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: What's cooking in git.git (Dec 2012, #03; Wed, 12)

[Max Horn]

Felipe,

please stop referring to "facts" and "obvious". You pretend to be a being of 
pure reason and that everything you say is logical, drawn from facts. But you 
forget or perhaps do not know that logic by itself proofs nothing, it all 
depends on the axioms you impose. And yours are quite different from what 
others consider as such, and apparently also inconsistent. 

So, instead of trying to twist things around so that broken things in your code 
are not broken after all, why not simply re-roll your patch with the "obvious" 
fixes applies? As you write yourself, time is not pressing at all -- so I don't 
see why your patch should be merged now, and fixed later, contrary to how other 
people's patches are treated? Why not fix them first, and then apply? We do 
have time, after all! And nobody is expecting you to do that while you are on 
vacation, either. Nor that you do it instantly.

Just say: "OK, I see there is a problem with the patches; even though I 
consider it unimportant, I will play by the rules everybody here has to follow, 
and re-roll the patch series. But this is of low priority for me, so I cannot 
say right now when it will happen".

Everybody would be happy then. Except perhaps the hypothetical users, who would 
have to wait a bit longer -- but oh, not really, because they can just use 
remote-bzr from your repo, yay :-). I really like that about it, it lives in 
contrib, so one can use it w/o it being merged into git.git.

Instead, you make claims that make you look like a foolish and arrogant ass, 
all the while insulting Junio and me implicitly. Why do you do that??? It 
delays acceptance of your nice work. As you write, this hurts the users. So why 
do it?


Since you keep complaining that nobody ever really can point to anything wrong 
your said, I'll do you the favor by deconstructing one of the claims you made:


On 13.12.2012, at 20:06, Felipe Contreras wrote:

> On Thu, Dec 13, 2012 at 6:04 AM, Max Horn  wrote:
>> 
>> On 13.12.2012, at 11:08, Felipe Contreras wrote:
>> 
>>> On Thu, Dec 13, 2012 at 2:11 AM, Junio C Hamano  wrote:
>>>> Felipe Contreras  writes:
>>> 
>>>>>> New remote helper for bzr (v3).  With minor fixes, this may be ready
>>>>>> for 'next'.
>>>>> 
>>>>> What minor fixes?
>>>> 
>>>> Lookng at the above (fixup), $gmane/210744 comes to mind
>>> 
>>> That doesn't matter. The code and the tests would work just fine.
>> 
>> 
>> It doesn't matter? I find that statement hard to align with what the 
>> maintainer of git, and thus the person who decides whether your patch series 
>> gets merged or not, wrote just above? In fact, it seems to me that what 
>> Junio said matters a great deal...
> 
> So you think Junio knows more about remote-bzr than I do?


This is a classical straw man argument. No, I do not think that. But I do think 
that Junio knows enough to review your code, and I do think that the point he 
raised is valid. You disagree with the importance of his point

> I repeat; it
> doesn't affect the tests, it doesn't affect the code, it doesn't cause
> any problem. remote-bzr could be merged today, in fact, it could have
> been merged a month ago.
> 
> You don't trust me? Here, look:
> 
[..]

> All this code is a no-op, because, as Junio pointed out, cmd is null.
> How is that a problem? It's not.

It is a problem. Because either the code inside the if is important, and then 
this is a bug. Or it is not important -- then it should not be there in the 
first place.

Either way, the patch series should be re-rolled. Of course in a whatever time 
frame suits you. If you are not willing to do that, this is sad, but of course 
also your right!

[...]

>> This is a very strange attitude...
>> 
>> In another email, you complained about nobody reviewing your patches 
>> respectively nobody voicing any constructive criticism. Yet Junio did just 
>> that, and again in $gmane/210745 -- and you replied to neither, and acted on 
>> neither (not even by refuting the points brought up), and now summarily 
>> dismiss them as irrelevant. I find that quite disturbing :-(.
> 
> I didn't say it was irrelevant, it should be fixed,

Actually, you wrote:

 "That doesn't matter."

So I paraphrased. In any case, I am glad to hear you finally agree that it 
should be fixed (which you did *not* say in your initial reply). So the problem 
we have seems to be that you do not understand how patches typically handled in 
git.git. Well, based on my observation: If reviews point out things in a patch 
series that are not optimal or even broken, it is expected that the submitter 
fixes this locally 

Re: What's cooking in git.git (Dec 2012, #03; Wed, 12)

[Max Horn]

On 13.12.2012, at 11:08, Felipe Contreras wrote:

> On Thu, Dec 13, 2012 at 2:11 AM, Junio C Hamano  wrote:
>> Felipe Contreras  writes:
> 
 New remote helper for bzr (v3).  With minor fixes, this may be ready
 for 'next'.
>>> 
>>> What minor fixes?
>> 
>> Lookng at the above (fixup), $gmane/210744 comes to mind
> 
> That doesn't matter. The code and the tests would work just fine.


It doesn't matter? I find that statement hard to align with what the maintainer 
of git, and thus the person who decides whether your patch series gets merged 
or not, wrote just above? In fact, it seems to me that what Junio said matters 
a great deal...

This is a very strange attitude...

In another email, you complained about nobody reviewing your patches 
respectively nobody voicing any constructive criticism. Yet Junio did just 
that, and again in $gmane/210745 -- and you replied to neither, and acted on 
neither (not even by refuting the points brought up), and now summarily dismiss 
them as irrelevant. I find that quite disturbing :-(.

> 
>> but there may be others.  It is the responsibility of a contributor to keep
>> track of review comments others give to his or her patches and
>> reroll them, so I do not recall every minor details, sorry.
> 
> There is nothing that prevents remote-bzr from being merged.

Well, I think that is up to Junio to decide in the end, though :-). He wrote 


Cheers,
Max--
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 1/6] git-remote-helpers.txt: document invocation before input format

[Max Horn]

On 13.12.2012, at 00:13, Junio C Hamano wrote:

> Max Horn  writes:
> 
>> Of course I can also re-roll, if that is necessary/preferred.
> 
> No, you can't.  The topic has been cooking in 'next' for some days
> now already.

Ah, right, I somehow missed that :-/. Well, I guess it's at most a tiny minor 
cleanup anyway and thus not important :-).


Cheers,
Max--
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 1/6] git-remote-helpers.txt: document invocation before input format

[Max Horn]

On 13.12.2012, at 00:00, Felipe Contreras wrote:
[...]

>>> 
>>> I find all this text a bit confusing. First argument, second argument,
>>> etc. Personally, I would describe everything in the terms of alias
>>> (1st arg), and URL (2nd arg).
>> 
>> Yeah, I also thought about that, but as above, deliberately did not touch it 
>> here, but only moved it around. I'll be happy to revisit this on a future 
>> date, though.
> 
> Oh, in that case it's fine, but I would have named it "move invocation
> before input format", or something that has *move*, or *shuffle*.

Agreed. It explicit says move in the body of the commit message, but not in the 
summary line.. That would be an improvement, I gueess. Junio, if you want, feel 
free to reword the summary line of the patch accordingly, e.g. changing it from

  git-remote-helpers.txt: document invocation before input format

to something like

  git-remote-helpers.txt: move 'invocation' section before 'input format'

Of course I can also re-roll, if that is necessary/preferred.


Cheers,
Max--
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 1/6] git-remote-helpers.txt: document invocation before input format

[Max Horn]

On 12.12.2012, at 23:14, Felipe Contreras wrote:

> On Tue, Nov 27, 2012 at 5:03 PM, Max Horn  wrote:
> 
>> index 5ce4cda..9a7e583 100644
>> --- a/Documentation/git-remote-helpers.txt
>> +++ b/Documentation/git-remote-helpers.txt
>> @@ -35,6 +35,37 @@ transport protocols, such as 'git-remote-http', 
>> 'git-remote-https',
>> 'git-remote-ftp' and 'git-remote-ftps'. They implement the capabilities
>> 'fetch', 'option', and 'push'.
>> 
>> +INVOCATION
>> +--
>> +
>> +Remote helper programs are invoked with one or (optionally) two
>> +arguments. The first argument specifies a remote repository as in git;
>> +it is either the name of a configured remote or a URL. The second
>> +argument specifies a URL; it is usually of the form
>> +'://', but any arbitrary string is possible.
>> +The 'GIT_DIR' environment variable is set up for the remote helper
>> +and can be used to determine where to store additional data or from
>> +which directory to invoke auxiliary git commands.
>> +
>> +When git encounters a URL of the form '://', where
>> +'' is a protocol that it cannot handle natively, it
>> +automatically invokes 'git remote-' with the full URL as
>> +the second argument. If such a URL is encountered directly on the
>> +command line, the first argument is the same as the second, and if it
>> +is encountered in a configured remote, the first argument is the name
>> +of that remote.
> 
> Maybe it's worth mentioning that if the alias of the remote is not
> specified, the URL is used instead.

Worth a thought yeah -- but beyond the scope of this patch: I merely moved this 
text around, but did not touch it otherwise.

> 
>> +A URL of the form '::' explicitly instructs git to
>> +invoke 'git remote-' with '' as the second
>> +argument. If such a URL is encountered directly on the command line,
>> +the first argument is '', and if it is encountered in a
>> +configured remote, the first argument is the name of that remote.
>> +
>> +Additionally, when a configured remote has 'remote..vcs' set to
>> +'', git explicitly invokes 'git remote-' with
>> +'' as the first argument. If set, the second argument is
>> +'remote..url'; otherwise, the second argument is omitted.
> 
> I find all this text a bit confusing. First argument, second argument,
> etc. Personally, I would describe everything in the terms of alias
> (1st arg), and URL (2nd arg).

Yeah, I also thought about that, but as above, deliberately did not touch it 
here, but only moved it around. I'll be happy to revisit this on a future date, 
though.


Thanks for the feedback,
Max--
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] Improve remote helper documentation

[Max Horn]

On 07.12.2012, at 22:52, Junio C Hamano wrote:

> Max Horn  writes:
> 
>> On 07.12.2012, at 20:09, Junio C Hamano wrote:
>> 
>>> Except for a minor nit in 6/6; I think "defined options" should be
>>> "defined attributes".
>>> 
>>>   --- a/Documentation/git-remote-helpers.txt
>>>   +++ b/Documentation/git-remote-helpers.txt
>>>   @@ -227,6 +227,8 @@ Support for this command is mandatory.
>>>   the name; unrecognized attributes are ignored. The list ends
>>>   with a blank line.
>>>+
>>>   +See REF LIST ATTRIBUTES for a list of currently defined options.
>>>   ++
>> 
>> Indeed, my mistake. Dumb question since I am still not completely
>> familiar with the procedures here: Would you just fix this
>> yourself as you apply this, or is it better (or even required)
>> that I reroll for this? (Of course if I ned to re-roll for other
>> reasons, I'd fix this one as well).
> 
> I didn't see any other issues myself, and I didn't see anybody
> raising issues on the series, either, so I could fix it up by
> amending what is already queued, as long as you are OK with it.

Sure, I'd appreciate if you could make that fix.

Thanks,
Max
--
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] Improve remote helper documentation

[Max Horn]

On 07.12.2012, at 20:09, Junio C Hamano wrote:

> Max Horn  writes:
> 
>> Various remote helper capabilities and commands were not
>> documented, in particular 'export', or documented in a misleading
>> way (e.g. 'for-push' was listed as a ref attribute understood by
>> git, which is not the case). This patch series changes that, and
>> also address some other things in the remote helper documentation
>> that I found jarring when reading through it.
>> 
>> Note that the description of export and (im|ex)port-marks probably can be
>> improved, and I hope that somebody who knows more about them
>> than me and/or is better at writing documentation will do just that.
>> But I felt it was better to provide something than to do nothing
>> and only complain, as I did previously on this subject ;-).
> 
> A second ping to people who have touched transport-helper.c,
> remote-testsvn.c, git-remote-testgit, and contrib/remote-helpers/ in
> the past 18 months for comments.  I've re-read the documentation
> updates myself and didn't find anything majorly objectionable.
> 
> Except for a minor nit in 6/6; I think "defined options" should be
> "defined attributes".
> 
>--- a/Documentation/git-remote-helpers.txt
>+++ b/Documentation/git-remote-helpers.txt
>@@ -227,6 +227,8 @@ Support for this command is mandatory.
>the name; unrecognized attributes are ignored. The list ends
>with a blank line.
> +
>+See REF LIST ATTRIBUTES for a list of currently defined options.
>++

Indeed, my mistake. Dumb question since I am still not completely familiar with 
the procedures here: Would you just fix this yourself as you apply this, or is 
it better (or even required) that I reroll for this? (Of course if I ned to 
re-roll for other reasons, I'd fix this one as well).


Thanks,
Max--
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: [Query] Can we ignore case for commiters name in shortlog?

[Max Horn]

On 30.11.2012, at 04:35, viresh kumar wrote:

> On 30 November 2012 09:03, Nicolas Pitre  wrote:
> 
>> Have a look at the .mailmap file in the top directory of your repo.
> 
> Repeating what i said to David in other mail:
> 
> I have my name there :)
> 
> I thought using names with different case is actually different then 
> misspelling
> it. And so, everybody must not be required to update their names in mailmap
> with different case. So, with same email id and same name (that may be in
> different case), we can show commits together in shortlog.

I don't see how wrong case is different from any other form of misspelling. And 
mailmap is there precisely to handle such problems. Now, if these case issues 
were for some reasons very frequent, it might be worth adding dedicated support 
for it. But this seems dubious to me -- do you have any evidence for this? 
Indeed, do you have more than just the one example?

In a nutshell, there seem to be two options here, and I know which *I* find 
more appealing ;)

1) continue this discussion over several emails to design a new feature (an 
option, config setting, whatever) to handle your special case, then sit down 
and write code for it, add documentation, add test cases to test it.

2) You just add a single entry to your mailmap to solve your problem at hand. 
:-)


Cheers,
Max--
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 v7 p2 1/2] fast-export: don't handle uninteresting refs

[Max Horn]

On 28.11.2012, at 23:23, Felipe Contreras wrote:

> They have been marked as UNINTERESTING for a reason, lets respect that.
> 
> Currently the first ref is handled properly, but not the rest:
> 
>  % git fast-export master ^uninteresting ^foo ^bar

All these refs are assumed to point to the same object, right? I think it would 
be better if the commit message stated that explicitly. To make up for the lost 
space, you could then get rid of one of the four refs, I think three are 
sufficient to drive the message home ;-).




> The reason this happens is that before traversing the commits,
> fast-export checks if any of the refs point to the same object, and any
> duplicated ref gets added to a list in order to issue 'reset' commands
> after the traversing. Unfortunately, it's not even checking if the
> commit is flagged as UNINTERESTING. The fix of course, is to do
> precisely that.

Hm... So this might be me being a stupid n00b (I am not yet that familiar with 
the internal rep of things in git and all...)... but I found the "precisely 
that" par very confusing, because right afterwards, you say:

> 
> However, in order to do it properly we need to get the UNINTERESTING flag
> from the command line ref, not from the commit object.

So this sounds like you are saying "we do *precisely* that, except we don't, 
because it is more complicated, so we actually don't do this *precisely*, just 
manner of speaking..."

Some details here are beyond my knowledge, I am afraid, so I have to resort to 
guess: In particular it is not clear to me why the "however" part pops up: 
Reading it makes it sound as if the commit object also carries an UNINTERESTING 
flag, but we can't use it because of some reason (perhaps it doesn't have the 
semantics we need?), so we have to look at revs.pending instead. Right? Wrong? 
Or is it because the commit objects actually do *not* carry the UNINTERESTING 
bits, hence we need to look at revs.pending. Or is it due to yet another reason?

I would find it helpful if that could be clarified. E.g. like so:

 "The fix is to add such a check. However, we cannot just use the UNINTERESTING 
flag of the commit object, because INSERT-REASON."

or

 "The fix is to add such a check. However, the commit object does not contain 
the UNINTERESTING flag directly."

or something.

Anyway, other than these nitpicky questions, this whole thing looks very 
logical to me, description and code alike. I also played around with tons of 
"fast-export" invocations, with and without this patch, and it seems to do what 
the description says. Finally, I went to the various long threads discussion 
prior versions of this patch, in particular those starting at
  http://thread.gmane.org/gmane.comp.version-control.git/208725
and
  http://thread.gmane.org/gmane.comp.version-control.git/209355/focus=209370

These contained some concerns. Sadly, several of those discussions ultimately 
degenerated into not-so-pleasant exchanges :-(, and my impression is that as a 
result some people are not so inclined to comment on these patches anymore at 
all. Which is a pity :-(. But overall, it seems this patch makes nothing worse, 
but fixes some things; and it is simple enough that it shouldn't make future 
improvements harder.

So *I* at least am quite happy with this, it helps me! My impression is that 
Felipe's latest patch addresses most concerns people raised by means of an 
improved description. I couldn't find any in those threads that I feel still 
applies -- but of course those people should speak for themselves, I am simply 
afraid they don't want to be part of this anymore :-(.


Still, for what little it might be worth, I think this patch is good and a real 
improvement. I hope it can be merged soon.


Cheers,
Max


> Fortunately we
> can simply use revs.pending, which contains all the information we need
> for get_tags_and_duplicates(), plus the ref flag. This way the rest of
> the positive refs will remain untouched; it's only the negative ones
> that change in behavior.
> 
> Signed-off-by: Felipe Contreras 
> ---
> builtin/fast-export.c | 11 +++
> t/t5801-remote-helpers.sh |  8 
> t/t9350-fast-export.sh| 30 ++
> 3 files changed, 45 insertions(+), 4 deletions(-)
> 
> diff --git a/builtin/fast-export.c b/builtin/fast-export.c
> index 191936c..2547e6c 100644
> --- a/builtin/fast-export.c
> +++ b/builtin/fast-export.c
> @@ -474,18 +474,21 @@ static void handle_tag(const char *name, struct tag 
> *tag)
>  (int)message_size, (int)message_size, message ? message : "");
> }
> 
> -static void get_tags_and_duplicates(struct object_array *pending,
> +static void get_tags_and_duplicates(struct rev_cmdline_info *info,
>   struct string_list *extra_refs)
> {
>   struct tag *tag;
>   int i;
> 
> - for (i = 0; i < pending->nr; i++) {
> - struct object_array_entry *e = pending->objects + i;
> + for (i = 0; i < 

Re: [PATCH] configure.ac: fix pthreads detection on Mac OS X

[Max Horn]

On 28.11.2012, at 07:38, Junio C Hamano wrote:

> Max Horn  writes:
> 
>> The configure script checks whether certain flags are required to use
>> pthreads. But it did not consider that *none* might be needed (as is the
>> case on Mac OS X). This lead to configure adding "-mt" to the list of
>> flags (which does nothing on OS X except producing a warning). This in
>> turn triggered a compiler warning on every single file.
>> 
>> To solve this, we now first check if pthreads work without extra flags.
>> This means the check is now order dependant, hence a comment is added
>> explaining this, and the reasons for it.
>> 
>> Note that it might be possible to write an order independent test, but
>> it does not seem worth the extra effort required for implementing and
>> testing such a solution, when this simple solution exists and works.
>> 
>> Signed-off-by: Max Horn 
>> ---
>> 
>> This is actually a revised version from my patch
>> "Change configure to check if pthreads are usable without any extra flags"
>> from July. I simply had forgotten all about it :-(.
> 
> Will queue,

OK

> but we would need wider testing to avoid "compiles well
> without an option but fails to link" issues similar to cea13a8
> (Improve test for pthreads flag, 2011-03-28) on other people's
> platforms (I know you tested on Mac OS X and over there it compiles
> and links well---I am worried about others).

Sure, understood. Though note that the test in question performs a compile & 
link test. So I have a hard time to see how this could break something. Then 
again, I dabbled in portable code long enough to never say never ;-).

BTW, is there such a thing as a build farm for git which automatically compiles 
and runs tests for pu / next / main, across a variety of platforms? Or does it 
all rely on devs test building everything regularly?


Cheers,
Max--
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] configure.ac: fix pthreads detection on Mac OS X

[Max Horn]

The configure script checks whether certain flags are required to use
pthreads. But it did not consider that *none* might be needed (as is the
case on Mac OS X). This lead to configure adding "-mt" to the list of
flags (which does nothing on OS X except producing a warning). This in
turn triggered a compiler warning on every single file.

To solve this, we now first check if pthreads work without extra flags.
This means the check is now order dependant, hence a comment is added
explaining this, and the reasons for it.

Note that it might be possible to write an order independent test, but
it does not seem worth the extra effort required for implementing and
testing such a solution, when this simple solution exists and works.

Signed-off-by: Max Horn 
---

This is actually a revised version from my patch
 "Change configure to check if pthreads are usable without any extra flags"
from July. I simply had forgotten all about it :-(.

Chers,
Max

 configure.ac | 12 +++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/configure.ac b/configure.ac
index ad215cc..41ac9a5 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1021,7 +1021,17 @@ if test -n "$USER_NOPTHREAD"; then
 # -D_REENTRANT' or some such.
 elif test -z "$PTHREAD_CFLAGS"; then
   threads_found=no
-  for opt in -mt -pthread -lpthread; do
+  # Attempt to compile and link some code using pthreads to determine
+  # required linker flags. The order is somewhat important here: We
+  # first try it without any extra flags, to catch systems where
+  # pthreads are part of the C library, then go on testing various other
+  # flags. We do so to avoid false positives. For example, on Mac OS X
+  # pthreads are part of the C library; moreover, the compiler allows us
+  # to add "-mt" to the CFLAGS (although it will do nothing except
+  # trigger a warning about an unused flag). Hence if we checked for
+  # "-mt" before "" we would end up picking it. But unfortunately this
+  # would then trigger compiler warnings on every single file we compile.
+  for opt in "" -mt -pthread -lpthread; do
  old_CFLAGS="$CFLAGS"
  CFLAGS="$opt $CFLAGS"
  AC_MSG_CHECKING([for POSIX Threads with '$opt'])
-- 
1.8.0.393.gcc9701d

--
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 6/6] git-remote-helpers.txt: clarify ref list attributes, link to subsections

[Max Horn]

Ouch. This one should *not* have been sent (the "[PATCH v2 6/6]" one is the 
correct one). Very sorry :(. I'll triple check next time.
Max

On 28.11.2012, at 00:03, Max Horn wrote:

> The documentation was misleading in that it gave the impression that
> 'for-push' could be used as a ref attribute in the output of the
> 'list' command. That is wrong.
> 
> Also, explicitly point out the connection between the commands
> 'list' and 'options' on the one hand, and the sections
> 'REF LIST ATTRIBUTES' and 'OPTIONS' on the other hand.
> 
> Signed-off-by: Max Horn 
> ---
> Documentation/git-remote-helpers.txt | 17 -
> 1 file changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/git-remote-helpers.txt 
> b/Documentation/git-remote-helpers.txt
> index 023dcca..e1df01d 100644
> --- a/Documentation/git-remote-helpers.txt
> +++ b/Documentation/git-remote-helpers.txt
> @@ -227,6 +227,8 @@ Support for this command is mandatory.
>   the name; unrecognized attributes are ignored. The list ends
>   with a blank line.
> +
> +See REF LIST ATTRIBUTES for a list of currently defined options.
> ++
> Supported if the helper has the "fetch" or "import" capability.
> 
> 'list for-push'::
> @@ -248,6 +250,8 @@ Supported if the helper has the "push" or "export" 
> capability.
>   for it).  Options should be set before other commands,
>   and may influence the behavior of those commands.
> +
> +See OPTIONS for a list of currently defined options.
> ++
> Supported if the helper has the "option" capability.
> 
> 'fetch'  ::
> @@ -256,7 +260,7 @@ Supported if the helper has the "option" capability.
>   per line, terminated with a blank line.
>   Outputs a single blank line when all fetch commands in the
>   same batch are complete. Only objects which were reported
> - in the ref list with a sha1 may be fetched this way.
> + in the output of 'list' with a sha1 may be fetched this way.
> +
> Optionally may output a 'lock ' line indicating a file under
> GIT_DIR/objects/pack which is keeping a pack until refs can be
> @@ -360,10 +364,9 @@ capabilities reported by the helper.
> REF LIST ATTRIBUTES
> ---
> 
> -'for-push'::
> - The caller wants to use the ref list to prepare push
> - commands.  A helper might chose to acquire the ref list by
> - opening a different type of connection to the destination.
> +The 'list' command produces a list of refs in which each ref
> +may be followed by a list of attributes. The following ref list
> +attributes are defined.
> 
> 'unchanged'::
>   This ref is unchanged since the last import or fetch, although
> @@ -371,6 +374,10 @@ REF LIST ATTRIBUTES
> 
> OPTIONS
> ---
> +
> +The following options are defined and (under suitable circumstances)
> +set by git if the remote helper has the 'option' capability.
> +
> 'option verbosity' ::
>   Changes the verbosity of messages displayed by the helper.
>   A value of 0 for  means that processes operate
> -- 
> 1.8.0.393.gcc9701d
> 
> --
> 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
> 

--
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 v2 0/6] Improve remote helper documentation

[Max Horn]

Various remote helper capabilities and commands were not
documented, in particular 'export', or documented in a misleading
way (e.g. 'for-push' was listed as a ref attribute understood by
git, which is not the case). This patch series changes that, and
also address some other things in the remote helper documentation
that I found jarring when reading through it.

Note that the description of export and (im|ex)port-marks probably can be
improved, and I hope that somebody who knows more about them
than me and/or is better at writing documentation will do just that.
But I felt it was better to provide something than to do nothing
and only complain, as I did previously on this subject ;-).

Max Horn (6):
  git-remote-helpers.txt: document invocation before input format
  git-remote-helpers.txt: document missing capabilities
  git-remote-helpers.txt: minor grammar fix
  git-remote-helpers.txt: rearrange description of capabilities
  git-remote-helpers.txt: clarify command <-> capability correspondences
  git-remote-helpers.txt: clarify options & ref list attributes

 Documentation/git-remote-helpers.txt | 245 ---
 1 file changed, 140 insertions(+), 105 deletions(-)

-- 
1.8.0.393.gcc9701d

--
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 v2 5/6] git-remote-helpers.txt: clarify command <-> capability correspondences

[Max Horn]

In particular, document 'list for-push' separately from 'list', as
the former needs only be supported for the push/export
capabilities, and the latter only for fetch/import. Indeed, a
hypothetically 'push-only' helper would only need to support the
former, not the latter.

Signed-off-by: Max Horn 
---
 Documentation/git-remote-helpers.txt | 21 -
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-remote-helpers.txt 
b/Documentation/git-remote-helpers.txt
index 7ac1461..023dcca 100644
--- a/Documentation/git-remote-helpers.txt
+++ b/Documentation/git-remote-helpers.txt
@@ -216,6 +216,8 @@ Commands are given by the caller on the helper's standard 
input, one per line.
which marks them mandatory for git versions using the remote
helper to understand. Any unknown mandatory capability is a
fatal error.
++
+Support for this command is mandatory.
 
 'list'::
Lists the refs, one per line, in the format " 
@@ -225,9 +227,18 @@ Commands are given by the caller on the helper's standard 
input, one per line.
the name; unrecognized attributes are ignored. The list ends
with a blank line.
 +
-If 'push' is supported this may be called as 'list for-push'
-to obtain the current refs prior to sending one or more 'push'
-commands to the helper.
+Supported if the helper has the "fetch" or "import" capability.
+
+'list for-push'::
+   Similar to 'list', except that it is used if and only if
+   the caller wants to the resulting ref list to prepare
+   push commands.
+   A helper supporting both push and fetch can use this
+   to distinguish for which operation the output of 'list'
+   is going to be used, possibly reducing the amount
+   of work that needs to be performed.
++
+Supported if the helper has the "push" or "export" capability.
 
 'option'  ::
Sets the transport helper option  to .  Outputs a
@@ -306,7 +317,7 @@ sequence has to be buffered before starting to send data to 
fast-import
 to prevent mixing of commands and fast-import responses on the helper's
 stdin.
 +
-Supported if the helper has the 'import' capability.
+Supported if the helper has the "import" capability.
 
 'export'::
Instructs the remote helper that any subsequent input is
@@ -322,7 +333,7 @@ fast-export', which then will load/store a table of marks 
for
 local objects. This can be used to implement for incremental
 operations.
 +
-Supported if the helper has the 'export' capability.
+Supported if the helper has the "export" capability.
 
 'connect' ::
Connects to given service. Standard input and standard output
-- 
1.8.0.393.gcc9701d

--
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 v2 3/6] git-remote-helpers.txt: minor grammar fix

[Max Horn]

Signed-off-by: Max Horn 
---
 Documentation/git-remote-helpers.txt | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-remote-helpers.txt 
b/Documentation/git-remote-helpers.txt
index db63541..7eb43d7 100644
--- a/Documentation/git-remote-helpers.txt
+++ b/Documentation/git-remote-helpers.txt
@@ -235,9 +235,9 @@ Commands are given by the caller on the helper's standard 
input, one per line.
 'capabilities'::
Lists the capabilities of the helper, one per line, ending
with a blank line. Each capability may be preceded with '*',
-   which marks them mandatory for git version using the remote
-   helper to understand (unknown mandatory capability is fatal
-   error).
+   which marks them mandatory for git versions using the remote
+   helper to understand. Any unknown mandatory capability is a
+   fatal error.
 
 'list'::
Lists the refs, one per line, in the format " 
-- 
1.8.0.393.gcc9701d

--
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 6/6] git-remote-helpers.txt: clarify ref list attributes, link to subsections

[Max Horn]

The documentation was misleading in that it gave the impression that
'for-push' could be used as a ref attribute in the output of the
'list' command. That is wrong.

Also, explicitly point out the connection between the commands
'list' and 'options' on the one hand, and the sections
'REF LIST ATTRIBUTES' and 'OPTIONS' on the other hand.

Signed-off-by: Max Horn 
---
 Documentation/git-remote-helpers.txt | 17 -
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-remote-helpers.txt 
b/Documentation/git-remote-helpers.txt
index 023dcca..e1df01d 100644
--- a/Documentation/git-remote-helpers.txt
+++ b/Documentation/git-remote-helpers.txt
@@ -227,6 +227,8 @@ Support for this command is mandatory.
the name; unrecognized attributes are ignored. The list ends
with a blank line.
 +
+See REF LIST ATTRIBUTES for a list of currently defined options.
++
 Supported if the helper has the "fetch" or "import" capability.
 
 'list for-push'::
@@ -248,6 +250,8 @@ Supported if the helper has the "push" or "export" 
capability.
for it).  Options should be set before other commands,
and may influence the behavior of those commands.
 +
+See OPTIONS for a list of currently defined options.
++
 Supported if the helper has the "option" capability.
 
 'fetch'  ::
@@ -256,7 +260,7 @@ Supported if the helper has the "option" capability.
per line, terminated with a blank line.
Outputs a single blank line when all fetch commands in the
same batch are complete. Only objects which were reported
-   in the ref list with a sha1 may be fetched this way.
+   in the output of 'list' with a sha1 may be fetched this way.
 +
 Optionally may output a 'lock ' line indicating a file under
 GIT_DIR/objects/pack which is keeping a pack until refs can be
@@ -360,10 +364,9 @@ capabilities reported by the helper.
 REF LIST ATTRIBUTES
 ---
 
-'for-push'::
-   The caller wants to use the ref list to prepare push
-   commands.  A helper might chose to acquire the ref list by
-   opening a different type of connection to the destination.
+The 'list' command produces a list of refs in which each ref
+may be followed by a list of attributes. The following ref list
+attributes are defined.
 
 'unchanged'::
This ref is unchanged since the last import or fetch, although
@@ -371,6 +374,10 @@ REF LIST ATTRIBUTES
 
 OPTIONS
 ---
+
+The following options are defined and (under suitable circumstances)
+set by git if the remote helper has the 'option' capability.
+
 'option verbosity' ::
Changes the verbosity of messages displayed by the helper.
A value of 0 for  means that processes operate
-- 
1.8.0.393.gcc9701d

--
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 v2 6/6] git-remote-helpers.txt: clarify options & ref list attributes

[Max Horn]

The documentation was misleading in that it gave the impression that
'for-push' could be used as a ref attribute in the output of the
'list' command. That is wrong.

Also, explicitly point out the connection between the commands
'list' and 'options' on the one hand, and the sections
'REF LIST ATTRIBUTES' and 'OPTIONS' on the other hand.

Signed-off-by: Max Horn 
---
 Documentation/git-remote-helpers.txt | 17 -
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-remote-helpers.txt 
b/Documentation/git-remote-helpers.txt
index 023dcca..e1df01d 100644
--- a/Documentation/git-remote-helpers.txt
+++ b/Documentation/git-remote-helpers.txt
@@ -227,6 +227,8 @@ Support for this command is mandatory.
the name; unrecognized attributes are ignored. The list ends
with a blank line.
 +
+See REF LIST ATTRIBUTES for a list of currently defined options.
++
 Supported if the helper has the "fetch" or "import" capability.
 
 'list for-push'::
@@ -248,6 +250,8 @@ Supported if the helper has the "push" or "export" 
capability.
for it).  Options should be set before other commands,
and may influence the behavior of those commands.
 +
+See OPTIONS for a list of currently defined options.
++
 Supported if the helper has the "option" capability.
 
 'fetch'  ::
@@ -256,7 +260,7 @@ Supported if the helper has the "option" capability.
per line, terminated with a blank line.
Outputs a single blank line when all fetch commands in the
same batch are complete. Only objects which were reported
-   in the ref list with a sha1 may be fetched this way.
+   in the output of 'list' with a sha1 may be fetched this way.
 +
 Optionally may output a 'lock ' line indicating a file under
 GIT_DIR/objects/pack which is keeping a pack until refs can be
@@ -360,10 +364,9 @@ capabilities reported by the helper.
 REF LIST ATTRIBUTES
 ---
 
-'for-push'::
-   The caller wants to use the ref list to prepare push
-   commands.  A helper might chose to acquire the ref list by
-   opening a different type of connection to the destination.
+The 'list' command produces a list of refs in which each ref
+may be followed by a list of attributes. The following ref list
+attributes are defined.
 
 'unchanged'::
This ref is unchanged since the last import or fetch, although
@@ -371,6 +374,10 @@ REF LIST ATTRIBUTES
 
 OPTIONS
 ---
+
+The following options are defined and (under suitable circumstances)
+set by git if the remote helper has the 'option' capability.
+
 'option verbosity' ::
Changes the verbosity of messages displayed by the helper.
A value of 0 for  means that processes operate
-- 
1.8.0.393.gcc9701d

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