Re: [PATCH v7 2/2] Verify index file before we opportunistically update it
Junio C Hamano writes: > What is the race under discussion about? It is about the index, > which corresponds one-to-one to the working tree, so in order for > the "race" to matter, you need to be racing against another process > that is not cooperating with you (e.g. a continuous and uncontrolled > "git add" updating the index while you are doing a real work), > mucking with the index in the same working tree. How could such a > workflow any useful in the real life? > > In the spectrum between useful and insane, there is a point where we > should just tell the insane: don't do it then. > > Having said that... > >>> So in that sense that can be done as a less urgent follow-up for this topic. >> >> Yeah if racing at refresh time is a real problem, sure we should solve it >> first. > > ... In order to solve the write-vs-write competition in a useful > way, you must serialize the competing accesses. E.g. "git add" > would first take a write lock on the index before read_cache(), and > then do its operation and finally release the lock by the usual > write-to-lock-close-then-rename dance. Extending on this a bit. Here I didn't mean the traditional ".lock" we create via the lockfile() API. Rather, we would want to use fcntl/flock style lock that lets others _wait_, not _fail_. Because... > The lazy "read and refresh in-core first, hoping that we did not > compete with anybody, and then check just before writing out after > taking the lock" is a very good solution for the opportunistic > update codepath, because it is an option not to write the result out > when there actually was an update by somebody else. But such an > opportunistic locking scheme does not work for write-vs-write > competition. Upon a real conflict, you need to fail the entire > operation. ...having multiple conflicting writers on the single index file is what you thought about worth protecting against. When somebody else is pounding on the index file you are trying to prepare your next commmit in, with his writing that can unexpectedly overwrite what you prepared, you would at least want the accesses serialized, without getting half of your attempt to "git add" fail (and having to redo). For that, you would want your "git add" to wait while the other party is mucking with the index under lock, instead of failing, which is what you would get from the traditional lockfile API based locking. But that still takes us back to the "don't do it then". It is true that, with serialization, you may be able to guarantee that one "git add" you do would start from one state (which may record the state of your previous "git add", or which may have further been modified by the other process) and ends with whatever that "git add" added, without any other change. But even in that case, when you finally do a "git commit", can you say you know what is in the index? I do not think so. After all, the root cause of the "race" issue is that the other process pounds at the same index while you are working on it, without any coordination with you. So... -- 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 2/2] Verify index file before we opportunistically update it
On Sat, Apr 12, 2014 at 11:19 AM, Junio C Hamano wrote: > Duy Nguyen writes: > >> On Sat, Apr 12, 2014 at 3:43 AM, Junio C Hamano wrote: >>> Having said that, nobody sane would be running two simultaneous >>> operations that are clearly write-oriented competing with each other >>> against the same index file. >> >> When it comes to racing, sanity does not matter much. People could >> just do it without thinking what exactly is happening behind the >> scene. > > Well, "a race is a race is a race" would also be my knee-jerk > reaction when anybody utters the word "race", but you need to > realize that we are not talking about races like object creation > while "gc --auto" running in the background or two pushes trying to > update the same ref to different values, which are meaningful use > cases. > > What is the race under discussion about? It is about the index, > which corresponds one-to-one to the working tree, so in order for > the "race" to matter, you need to be racing against another process > that is not cooperating with you (e.g. a continuous and uncontrolled > "git add" updating the index while you are doing a real work), > mucking with the index in the same working tree. How could such a > workflow any useful in the real life? > > In the spectrum between useful and insane, there is a point where we > should just tell the insane: don't do it then. The thing is people do not usually have that level of knowledge of how git works. They could write a cron job to do something in some repos, not aware at all about these non-cooperations. Telling people not to automatically touch a git directory at all is a bit extreme. I think this patch is about guarding the user from shooting themselves. Either a command works correctly, not not work at all. A process' dying is a way of telling people "this is insane" without having to draw a line between dos and dont's in documents. Serializing write access to make both competing processes is nice, but that's a separate step that may or may not be worth pursuing. And I'm towards not worth pursuing. As you metioned in the next mail, serializing helps two competing processes, but not two command sequences. -- Duy -- 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 6/9] branch: display publish branch
On Fri, Apr 11, 2014 at 08:48:01AM -0500, Felipe Contreras wrote:
> I think of @{publish} as "the branch the user has configured to push
> to"; it overrides all other configurations (push.default and push
> refspecs). I wouldn't mind having a @{push} *in addition* to @{publish}
> that would have the behavior you mention, but for @{publish} I'm pretty
> certain the behavior I want is that it maps *directly* to what the user
> has configured.
I guess I don't understand why we need "branch.*.push" when we already
have push refspecs that do the same thing (and are more powerful, as
they can glob).
The behavior you describe is not useful to me, as it would mean having
to manage branch.*.push as a duplicate of the information that is
already found in "remote.$(git config remote.pushdefault).push". I do
not mind if the two co-exist, but please do not take the short @{p} as
your series does (not only because of @{push}, but also because @{pull}
has been mentioned elsewhere as a possible other name for @{upstream}).
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: wrong handling of text git attribute leading to files incorrectly reported as modified
Am 11.04.2014 um 22:38 schrieb Torsten Bögershausen : > On 2014-04-11 22.20, Frank Ammeter wrote: >> I’m not a git expert and this might be the wrong place to ask this question, >> so please send me somewhere else if I’m in the wrong place. >> >> I asked the same question on stack overflow, but didn’t get any response: >> http://stackoverflow.com/questions/22823004/files-incorrectly-reported-modified-git-attributes-buggy-leading-to-inconsist >> >> If a file is committed with crlf line endings with the text attribute unset >> in the working tree, but the text attribute is set in the repo, the file >> will be incorrectly shown as modified - for all users checking out the file. >> Resetting or manually modifying the file will not help - The only remedy is >> to commit the .gitattributes with the text attribute set for the file. >> >> Wouldn’t it be better to only consider the checked-in gitattributes instead >> of the attributes in the working tree? > No. > If you change stuff in your working tree (and .gitattributes is a part of the > working tree) > how should Git know what you want? I don’t see that argument. I don’t know why at the time of a commit git should read unstaged files from my working tree - that affect my commit. > The primary assumption is that you know what you are doing in the working > tree. >> Is this a bug in git handling gitattributes or is this wrong usage? > I thinkk No, yes. > > If it is wrong usage, is it documented anywhere? > Please have a look here: > https://www.kernel.org/pub/software/scm/git/docs/gitattributes.html I’ve read this, can’t see anything about my problem in this document. No offense, but because I don’t understand the reasoning behind this, I can’t really help improve the documentation. I don’t think it makes much sense if I as a non-git-developer add something like „please apologize the git developers didn’t really think far enough when they invented git attributes, because they don't care if your repo gets inconsistent…" -- 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 6/9] branch: display publish branch
On Fri, Apr 11, 2014 at 12:24:35PM -0700, Junio C Hamano wrote:
> > But the branch.master.push setting does not do
> > anything to "git push".
>
> I am not sure I understand this. I thought that the desire behind
> the branch.*.push is to allow something like:
>
> ... other things in the config ...
> [remote]
> pushdefault = foo
> [remote "foo"]
> url = ...
> push = +refs/heads/*:refs/remotes/satellite/*
> fetch = +refs/heads/*:refs/remotes/foo/*
> [branch "master"]
> ; pushremote = foo
> push = refs/heads/bar
>
> so that "git push" on 'master' will override the more generic "all
> local branches here will go to their remote-tracking hierarchy for
> this satellite" refspec. And in that sense branch.master.push would
> do something to "git push".
Ah, I see. If I set "push.default" to "upstream", then the config I
showed before _does_ affect "git push". But I do not usually do that. I
have push.default set to "current", and sometimes override it using push
refspecs on certain repositories.
And that is why I find branch.*.push and Felipe's @{publish} useless for
my workflow. Pushes already go where I want them to, and I just want a
way to ask git to perform that config resolution for me. Whereas
Felipe's workflow is (I think) something like:
# make a new branch...
git checkout -b topic origin/master
# now publish our branch, and remember our publishing point
git push -p my-repo topic
# and now further pushes automatically go to my-repo/topic
git push
I can see there is some value in that override if you do things like:
git push -p my-repo topic:some-other-name
because the "-p" means "remember this other name I gave".
I would think in such a workflow that most of your branches would end up
with publish config, though. And therefore @{publish} would become
equivalent to "where you would push". But Felipe indicated that he would
not want "branch -vv" to match where all branches would be pushed, but
rather only those that were specifically configured. So maybe I do not
understand his workflow after all.
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Our official home page and logo for the Git project
On Fri, Apr 11, 2014 at 12:25:17PM -0700, Junio C Hamano wrote: > The mention of "dev.git-scm.com" gives me a mixed feeling. The > chasm between the developer community and casual end-users who know > about Git primarily via their perusal of git-scm.com is one of the > root causes of this confusion. I do not think you can get rid of that split, though. Different people want different content from a site. Somebody who wants to download and run git does not care about our Summer of Code ideas page. Somebody who wants to get a logo does not care about seeing an in-progress logo contest, or discussion on which logos people are working on. Historically most of the "dev" information has been on the mailing list. But sometimes it is more helpful to have a web page showing the "current state" of some content (e.g., the list of SoC ideas) and just periodically update it, rather than having each reader assemble the current state from whatever has been posted to the list. We have used the kernel.org wiki for this in the past. What I was suggesting is that those things could fall under the name "dev.git-scm.com" (which could even just point to the k.org wiki, or some other wiki, or a site to which many devs had push access). The wiki has _also_ been used for user-facing content. E.g., the list of tools that build on git. That kind of content would make sense to me on git-scm.com, and perhaps it could be ported there to give it better exposure. > The one on the left-top corner was one of the alternatives that > received favorable reactions from multiple people (I am not sure if > there was a clear "majority" though) submitted when we briefly had a > poll to come up with an updated logo. Do you have a link to the poll or its results? I could not find one in the list archive. Not that it necessarily matters to the current discussion, but I was interested for historical curiosity. I have also seen that logo receive unfavorable reactions from people, but my recollection is probably biased because I was one of those people. :) > In any case, this motion is not about "let's declare the logo we see > on git-scm.com today as _the_ official one". It is not about "that > logo on git-scm.com sucks; let's come up with a better one". People > are welcome to do that discussion elsewhere, and I do not mind a > repository of contestants created somewhere, but personally I think > the project is too mature for that and it is too late, even though > the "bleeding-red fork" logo may not be my favorite. Thanks, this is what I was trying to say in my earlier message. > The motion is about this: > > Outside people, like the party who approached us about putting > our logo on their trinket, seem to associate that logo we see on > git-scm.com today with our project, but we never officially said > it was our logo (we did not endorse that git-scm.com is our > official home page, either, for that matter). > > It is silly for us to have to say "Ehh, that is a logo that was > randomly done and slapped on git-scm.com which is not even our > official home page, and the logo is licensed CC-BY by somebody > else. Go talk to them.", every time such a request comes. > > Please help us by letting us answer "Yup, that is a logo (among > others) that represents our project, and we are OK with you > using it to help promote our project" instead. > > That is what I meant by "our official logo" in the first message. > > So,... seconds? I do not know if I count, as I am listed as one of the proposers in your original message. But yes, I agree with this. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Our official home page and logo for the Git project
On Fri, Apr 11, 2014 at 08:24:48AM -0500, Felipe Contreras wrote: > I would actually like you (everyone) to be honest and answer this > question; > > Have you actually analized the logo? Or are you just arguing against > change, because the logo is already used by git-scm.com, and related > stuff? Is this rhetorical? If not... Yes, I really thought about the logo and like it. Many of your complaints are about how git concepts map onto the logo (for instance, the direction of the graph nodes). That is _one_ way of evaluating the logo. But there are other criteria, as well. For example, is the logo pleasing to the eye? Is it memorable and recognizable? Things like "pleasing" are subjective, but there are patterns across humanity. Graphic artists have studied this for some time and have guidelines for layouts, contrast, balance, proportionality, etc. For example, in the git-fc logo you mentioned, you rotated the logo from git-scm.com. I find it less visually pleasing than the original. It seems somehow more "wobbly" to me with the two branches sticking up. Now, that is my completely subjective opinion. I do not know very much about graphic design, and whether guidelines could help there, nor did I conduct any empirical research. So maybe it is just me, or maybe one design is universally more pleasing than the other. But I think that visual art considerations should be at least as important in a logo as whether the logo pedantically matches the tool's output. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Clear an invalid password out of the credential-cache?
Is it me or is the only way to clear a single invalid password out of the credential-cache is by "git credential-cache exit"? -Jason -- -=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=- - - - Jason Pyeron PD Inc. http://www.pdinc.us - - Principal Consultant 10 West 24th Street #100- - +1 (443) 269-1555 x333Baltimore, Maryland 21218 - - - -=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=- This message is copyright PD Inc, subject to license 20080407P00. -- 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: Our official home page and logo for the Git project
Jeff King wrote: > On Fri, Apr 11, 2014 at 08:24:48AM -0500, Felipe Contreras wrote: > > > I would actually like you (everyone) to be honest and answer this > > question; > > > > Have you actually analized the logo? Or are you just arguing against > > change, because the logo is already used by git-scm.com, and related > > stuff? > > Is this rhetorical? If not... It was, because I was pretty sure the answer was mostly the later. > Yes, I really thought about the logo and like it. > > Many of your complaints are about how git concepts map onto the logo > (for instance, the direction of the graph nodes). That is _one_ way of > evaluating the logo. There are many ways of evaluating the logo, and they are not exclusive. > But there are other criteria, as well. For example, is the logo pleasing > to the eye? Is it memorable and recognizable? Things like "pleasing" are > subjective, but there are patterns across humanity. Graphic artists have > studied this for some time and have guidelines for layouts, contrast, > balance, proportionality, etc. Yes, that is _also_ important, but so is the fact that the logo should have correct Git concepts, because the main target audience for the logo is programmers. > For example, in the git-fc logo you mentioned, you rotated the logo from > git-scm.com. I find it less visually pleasing than the original. It > seems somehow more "wobbly" to me with the two branches sticking up. > Now, that is my completely subjective opinion. I do not know very much > about graphic design, and whether guidelines could help there, nor did I > conduct any empirical research. So maybe it is just me, or maybe one > design is universally more pleasing than the other. I've been playing with different logos myself, trying to see if I can come up with something different (rather than modifying the one done by GitHub). I've yet to come with something that I think might be superior, but I think I might be able to do more improvements now. So I have to agree on this; the direction of the nodes in the current logo does seem to be more aesthetically pleasing than my own. However, you left the colour of the logo completely untouched by your analysis, and the colour is extremely important. > But I think that visual art considerations should be at least as > important in a logo as whether the logo pedantically matches the tool's > output. *Both* are important, as are many other considerations. In short my concern is that *if* we are to pick an official logo, we shouldn't do it blindly, as it appears the logo done by GitHub wasn't reviewed at all by the community. Fortunately as Junio clarified; this is not a discussion to officialize the logo (albeit the title implying so). -- Felipe Contreras -- 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 6/9] branch: display publish branch
Jeff King wrote:
> On Fri, Apr 11, 2014 at 08:48:01AM -0500, Felipe Contreras wrote:
>
> > I think of @{publish} as "the branch the user has configured to push
> > to"; it overrides all other configurations (push.default and push
> > refspecs). I wouldn't mind having a @{push} *in addition* to @{publish}
> > that would have the behavior you mention, but for @{publish} I'm pretty
> > certain the behavior I want is that it maps *directly* to what the user
> > has configured.
>
> I guess I don't understand why we need "branch.*.push" when we already
> have push refspecs that do the same thing (and are more powerful, as
> they can glob).
Really? I think it's pretty simple.
I have a configuration similar to the following
[remote "gh"]
url = g...@github.com:felipec/git.git
[branch "master"]
pushremote = gh
push = refs/heads/master
[branch "fc/master"]
pushremote = gh
push = refs/heads/fc/master
[branch "up/publish"]
pushremote = gh
push = refs/heads/fc/publish
Now, if I didn't, I could create it easily with something like this:
% git push -p gh master
% git push -p gh fc/master
% git push -p gh up/publish:fc/publish
How would that translate to push refspecs?
[remote "gh"]
url = g...@github.com:felipec/git.git
push = refs/heads/master:refs/heads/master
push = refs/heads/fc/master:refs/heads/fc/master
push = refs/heads/up/master:refs/heads/fc/publish
[branch "master"]
pushremote = gh
[branch "fc/master"]
pushremote = gh
[branch "up/publish"]
pushremote = gh
So in theory it could be possible to make it work without branch.x.push.
If I run `git branch -v`, it would show the publish branch correctly.
master cee0c27 [origin/master, gh/master] Git 1.9.1
However, if I have a configuration like this:
[remote "gh"]
url = g...@github.com:felipec/git.git
push = refs/heads/*:refs/heads/*
Git would still show the same publish branch, even though the user never
explicitely told it do to so (e.g. with `git push -p`). To me this is not OK;
if I haven't specifically set a publish branch, it shouldn't be shown.
And then, what happens when I do:
% git branch --unset-upstream up/publish
Or:
% git branch -d up/publish
Should Git remove the appropriate push refspec? What if that refspec was
manually added by the user before the concept of the publish branch even
existed?
To me using the refspec just adds more complications, and it's not symetric
with @{upstream}. With branch.x.push, the symetry is very much there; the code
is similar to @{upstream}, the configuration as well, and what the user expects
too; it's much simpler. Also, the user can clearly see what are his manually
configured refspecs, as opposed to the ones added by `git push -p`.
It's just neat.
--
Felipe Contreras
--
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 6/9] branch: display publish branch
Jeff King wrote:
> On Fri, Apr 11, 2014 at 12:24:35PM -0700, Junio C Hamano wrote:
>
> > > But the branch.master.push setting does not do
> > > anything to "git push".
> >
> > I am not sure I understand this. I thought that the desire behind
> > the branch.*.push is to allow something like:
> >
> > ... other things in the config ...
> > [remote]
> > pushdefault = foo
> > [remote "foo"]
> > url = ...
> > push = +refs/heads/*:refs/remotes/satellite/*
> > fetch = +refs/heads/*:refs/remotes/foo/*
> > [branch "master"]
> > ; pushremote = foo
> > push = refs/heads/bar
> >
> > so that "git push" on 'master' will override the more generic "all
> > local branches here will go to their remote-tracking hierarchy for
> > this satellite" refspec. And in that sense branch.master.push would
> > do something to "git push".
>
> Ah, I see. If I set "push.default" to "upstream", then the config I
> showed before _does_ affect "git push". But I do not usually do that. I
> have push.default set to "current", and sometimes override it using push
> refspecs on certain repositories.
>
> And that is why I find branch.*.push and Felipe's @{publish} useless for
> my workflow. Pushes already go where I want them to, and I just want a
> way to ask git to perform that config resolution for me. Whereas
> Felipe's workflow is (I think) something like:
>
> # make a new branch...
> git checkout -b topic origin/master
>
> # now publish our branch, and remember our publishing point
> git push -p my-repo topic
>
> # and now further pushes automatically go to my-repo/topic
> git push
>
> I can see there is some value in that override if you do things like:
>
> git push -p my-repo topic:some-other-name
>
> because the "-p" means "remember this other name I gave".
>
> I would think in such a workflow that most of your branches would end up with
> publish config, though. And therefore @{publish} would become equivalent to
> "where you would push".
> But Felipe indicated that he would not want "branch -vv" to match where all
> branches would be pushed, but rather only those that were specifically
> configured. So maybe I do not understand his workflow after all.
It's a pretty typical triangular workflow, with a touch of fork maintainership.
Here are some types of branches I have:
* master [origin/master, gh/master] Git 1.9.1
My main master branch, I use it as a base point for many other branches. I
don't use origin/master because that's a moving target.
* dev/remote/hg-extra [master] remote-hg: store extra hg information in notes
A development branch. I don't publish those, therefore no @{publish}.
* fc/publish [fc/branch/nice-verbose, gh/fc/publish] sha1_name: add support for
@{publish} marks
A branch that is all good. I publish those, and use them for git-fc (my fork).
I think they should be in Git's core, but haven't been merged for some reason
or another.
Notice that the upstream branch is another local branch, not master. Strictly
speaking it's not an "upstream" branch, but I want 'git rebase' to use that as
the base point. Another @{base} concept might be more appropriate, but those
patches are a different story.
* up/publish [master] sha1_name: add support for @{publish} marks
A branch that should be sent upstream. I don't publish those.
Notice up/publish is different from fc/publish because the later depends on
another fc/* branch, which wasn't accepted upstream.
* fc/master [gh/fc/master] prompt: fix missing file errors in zsh
My main branch, used for git-fc. I merge Git's master, and cherry-pick various
fixes, so it has always the latest and greatest stuff.
Notice that 'gh/fc/master' is the publish branch, there is no upstream.
* pu [] Merge branch 'travis-ci' into pu
Similar to Junio's pu, I use `git reintegrate` to generate this branch using
'master' as the baseline, and merging all the fc/* branches. The result should
be identical to fc/master, if not, we are missing something from upstream, or
there's something missing in the fc/* branches.
It's not published, and has no upstream.
As you can see; some branches are published, others are not. The ones that are
not published don't have a @{publish}, and `git branch -v` doesn't show them.
Why is that hard to understand?
--
Felipe Contreras
--
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 1/3] rebase: avoid non-function use of "return" on FreeBSD
"Kyle J. McKay" writes:
> On Apr 11, 2014, at 10:30, Matthieu Moy wrote:
>> "Kyle J. McKay" writes:
>>
>>> There are already nested functions with file inclusion between both
>>> levels of nesting in git-rebase--interactive.sh and git-rebase--
>>> merge.sh now, so it's not introducing anything new.
>>
>> OK, so it's less serious than I thought. But still, we're
>> introducing a
>> function with 3 levels of nesting, split accross files, in an area
>> where
>> we know that at least one shell is buggy ...
>
> Currently in maint:
>
> The current code in maint does this:
>
> git-rebase.sh: top-level
> git-rebase.sh: run_specific_rebase()
> git-rebase.sh: run_specific_rebase_internal() -- contains "dot"
> git-rebase--interactive.sh: top-level (using --continue or --
> skip)
> git-rebase--interactive.sh: do_rest
> git-rebase--interactive.sh: do_next
You're confusing function calls and function nesting. do_rest calls
do_next, but the definition of do_next is not nested within do_rest.
When I talk about nested function, I mean
f() {
g() {
...
}
}
Obviously, having functions call each other is not an issue. That's what
functions are meant to be.
Now, having run_specific_rebase_internal include a file which defines
functions which contain nested functions _is_ something I find weird. It
both stresses the shell in a buggy area and makes the code harder to
understand.
> The problem with these changes, particularly the git-rebase--
> interactive.sh one is that a bunch of code is still run when the file
> is "dot" included.
Function definitions, and variables assignments. Is it so much of an
issue?
What's the difference between a function definition or variable
assignment within git-rebase--*.sh and within git-rebase.sh?
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
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 (Apr 2014, #03; Fri, 11)
On 11/04/14 23:22, Junio C Hamano wrote:
[...]
> [New Topics]
>
> * nd/index-pack-one-fd-per-thread (2014-04-09) 1 commit
> - index-pack: work around thread-unsafe pread()
>
> Enable threaded index-pack on platforms without thread-unsafe
> pread() emulation.
>
> Will merge to 'next' and keep it there for the remainder of the cycle.
The commit message for commit 512ebe5d ("index-pack: work around
thread-unsafe pread()", 25-03-2014) is a little misleading.
The pread() implementation is only thread-unsafe on older versions of
cygwin (anything prior to v1.7 - aka v1.5). Indeed, index-pack has been
threaded on cygwin (v1.7) since commit 103d530f ("Cygwin 1.7 has thread-
safe pread", 19-07-2013). (So, it was released in v1.8.4, I guess).
Since I upgraded my cygwin installation long ago (well about 9 months
ago), I can no longer test patches on cygwin v1.5. Also, since commit
a50dec22 ("Makefile: update defaults for modern Cygwin", 01-04-2010)
tells us that cygwin v1.5 is no longer being actively supported, it
may be difficult to find anyone who can do that testing. (v1.5 was
the last version supported on windows 95, 98 and ME, so maybe ... :)
Yesterday, I briefly compared the performance of the master and pu
branches (respectively with and without this patch) on cygwin.
(There seems to be little difference in the performance, as expected).
$ uname -a
CYGWIN_NT-5.1 toshiba 1.7.29(0.272/5/3) 2014-04-07 13:44 i686 Cygwin
First, on the master branch:
$ ./git version
git version 1.9.2.459.g68773ac
$ git log -1 --decorate --oneline
68773ac (HEAD, origin/master, origin/HEAD, master) Sync with 1.9.2
$ cd t
$ time ./t5302-pack-index.sh
ok 1 - setup
...
ok 31 - running index-pack in the object store
# passed all 31 test(s)
1..31
real1m11.984s
user1m24.986s
sys 0m41.521s
$
$ cd perf
$ ./p5302-pack-index.sh
warning: $GIT_PERF_LARGE_REPO is $GIT_BUILD_DIR.
warning: This will work, but may not be a sufficiently large repo
warning: for representative measurements.
ok 1 - repack
perf 2 - index-pack 0 threads: 1 2 3 ok
perf 3 - index-pack 1 thread : 1 2 3 ok
perf 4 - index-pack 2 threads: 1 2 3 ok
perf 5 - index-pack 4 threads: 1 2 3 ok
perf 6 - index-pack 8 threads: 1 2 3 ok
perf 7 - index-pack default number of threads: 1 2 3 ok
# passed all 7 test(s)
1..7
Can't locate Git.pm in @INC (@INC contains:
/usr/lib/perl5/site_perl/5.14/i686-cygwin-threads-64int
/usr/lib/perl5/site_perl/5.14
/usr/lib/perl5/vendor_perl/5.14/i686-cygwin-threads-64int
/usr/lib/perl5/vendor_perl/5.14 /usr/lib/perl5/5.14/i686-cygwin-threads-64int
/usr/lib/perl5/5.14 /usr/lib/perl5/site_perl/5.10
/usr/lib/perl5/vendor_perl/5.10 /usr/lib/perl5/site_perl/5.8 .) at
./aggregate.perl line 5.
BEGIN failed--compilation aborted at ./aggregate.perl line 5.
$ PERL5LIB=/home/ramsay/lib/perl5/site_perl/5.14 ./aggregate.perl
./p5302-pack-index.sh
Test this tree
5302.2: index-pack 0 threads 38.60(34.38+0.93)
5302.3: index-pack 1 thread36.48(34.74+0.90)
5302.4: index-pack 2 threads 26.60(36.57+1.87)
5302.5: index-pack 4 threads 27.32(37.46+2.38)
5302.6: index-pack 8 threads 27.18(38.38+3.01)
5302.7: index-pack default number of threads 25.40(36.07+2.09)
$
NOTE: that t/perf/aggregate.perl can't locate Git.pm; I haven't investigated
that.
Now, on the pu branch:
$ ./git version
git version 1.9.2.667.ge5b74e1
$ git log -1 --decorate --oneline
e5b74e1 (HEAD, origin/pu, pu) Merge branch 'jc/graph-post-root-gap' into pu
$ cd t
$ time ./t5302-pack-index.sh
ok 1 - setup
...
ok 31 - running index-pack in the object store
# passed all 31 test(s)
1..31
real1m8.063s
user1m26.151s
sys 0m40.477s
$
$ cd perf
$ PERL5LIB=/home/ramsay/lib/perl5/site_perl/5.14 ./p5302-pack-index.sh
warning: $GIT_PERF_LARGE_REPO is $GIT_BUILD_DIR.
warning: This will work, but may not be a sufficiently large repo
warning: for representative measurements.
ok 1 - repack
perf 2 - index-pack 0 threads: 1 2 3 ok
perf 3 - index-pack 1 thread : 1 2 3 ok
perf 4 - index-pack 2 threads: 1 2 3 ok
perf 5 - index-pack 4 threads: 1 2 3 ok
perf 6 - index-pack 8 threads: 1 2 3 ok
perf 7 - index-pack default number of threads: 1 2 3 ok
# passed all 7 test(s)
1..7
Test this tree
5302.2: index-pack 0 threads 37.90(34.40+0.82)
5302.3: index-pack 1 thread37.93(34.79+0.96)
5302.4: index-pack 2 threads 27.75(36.46+1.98)
5302.5: index-pack 4 threads 27.93(37.24+2.82)
5302.6: index-pack 8 threads 29.34(38.23+3.04)
5302.7:
Re: On "interpret-trailers" standalone tool
From: Junio C Hamano > > So far I've mostly been ignoring how the command line would look > like, I don't really feel this way ;-) > because the intermediate goal to my mind was to have it as a > hook that are added by people better versed with Git than an average > end-user, and if the command line interface had to change then they > are capable of updating it, so it is more acceptable than the usual > end-user tools to break compatibility between an early prototype and > later versions, and because the final goal would be to libify the > internal logic and integrate it into places we would invoke hooks, > making the standalone command irrelevant. > > However, I started to care ;-) For example, wouldn't it be nice if > you can do > > $ git format-patch -5 --cover-letter -o +my-series/ my-topic > $ git interpret-trailers "some args" ./+my-series/0*.patch > > to fix-up the "trailers" portion of the proposed log message in the > formatted patches? There may be other possible uses that having a > standalone tool would be helpful, even after we removed the need for > such a tool from commit, rebase, etc. by integrating the internal > logic to the implementation of these commands. > > However, I am wondering if the current "everything on the command > line is instruction to the command" is too limiting to allow the use > of the tool both as a filter and as a tool that can work on one or > more files named on the command line. If we start from there, the > only way to later add "these arguments are names of the files to be > operated on" is to add "--file --file ..." options, > which feels quite backwards as a UNIX tool. Yeah, except that we could add for example a '-o' option that would take a directory as argument and that would mean that the command should operate on all the files in this directory. It would be like the -o option of the format-patch command. > It would be easier to explain and understand if the command line > option set is modeled after things like "cat" or "sed", where > non-option arguments are filenames, instructions are given in the > form of "--option " (e.g. "-e 's/foo/bar/'" given to sed), and > having no non-option arguments on the command line signals that the > tool is working as a filter. Yeah, that's an interesting idea. I am not against making yet another number of changes to "git interpret-trailers" to make something like the above possible. But I think there are a few things that should be discussed first. First, if you think that the command might often be used along with format-patch, then it might be interesting for the user to have the command behave like format-patch instead of like cat or sed. This means that we could add the -o option I suggest above. And we don't need to do it now. We could add this option later instead of having to make the command work on many files now. Second, if the command should accept a patch as input instead of just a commit message, or both, this means that the command should have a way to tell if it is passed a patch, and then locate the commit message part in the patch. This means yet other changes to the command. Maybe these changes could be made later, in another series, or when the need arises to use it on full patches. Third, if trailers arguments are passed to the command using an option like "-z token=value" or "-z token:value", it would be nice to the user for consistency if the same option could be used when passing the same arguments to "git commit" and perhaps other commands like "git rebase", "git cherry-pick" and so on. This means that we now have to choose carefully the name of this option. Perhaps we can just give it a long name now like --trailer and care later about a short name, but I think it would not be very nice to the user to only have a long name for this option as it will very often be used. Fourth, some users might want the command to be passed some files as input, but they might not want the command to modify these input files. They might prefer the command to write its ouput into another set of output files. Maybe a syntax like cat or sed is not very well suited for this kind of use, while having a -o option for the output directory and a -i option for the input directory (if different from the output dir) would be nicer. Thanks, Christian. -- 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] git-remote-hg : Enable use of, $GIT_DIR/hg/origin/clone/.hg/hgrc
Daniel Liew wrote:
> git-remote-hg : Enable use of, $GIT_DIR/hg/origin/clone/.hg/hgrc
>
> Use the hgrc configuration file in the internal mercurial repository in
> addition to the other system wide hgrc files. This is done by using the
> 'ui' object from the 'repository' object which will have loaded the
> repository hgrc file if it exists.
What is the problem you are trying to solve? Is there a way to test that this
code is working correctly?
> Prior to this patch the mercurial repository's hgrc file was ignored
> which I consider to be a bug.
It might be, although the internal repository is not supposed to be used by the
user.
> Signed-off-by: Dan Liew
> ---
> contrib/remote-helpers/git-remote-hg | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/contrib/remote-helpers/git-remote-hg
> b/contrib/remote-helpers/git-remote-hg
> index eb89ef6..451842a 100755
> --- a/contrib/remote-helpers/git-remote-hg
> +++ b/contrib/remote-helpers/git-remote-hg
> @@ -421,7 +421,7 @@ def get_repo(url, alias):
>
> repo = hg.repository(myui, local_path)
> try:
> -peer = hg.peer(myui, {}, url)
> +peer = hg.peer(repo._unfilteredrepo.ui, {}, url)
Why not repo.unfiltered.ui? Or just repo.ui.
Cheers.
--
Felipe Contreras
--
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 v9 3/6] transport-helper: add 'force' to 'export' helpers
Otherwise they cannot know when to force the push or not (other than
hacks).
Tests-by: Richard Hansen
Documentation-by: Richard Hansen
Signed-off-by: Felipe Contreras
Signed-off-by: Junio C Hamano
---
Documentation/gitremote-helpers.txt | 4
git-remote-testgit.sh | 18 ++
t/t5801-remote-helpers.sh | 13 +
transport-helper.c | 5 +
4 files changed, 40 insertions(+)
diff --git a/Documentation/gitremote-helpers.txt
b/Documentation/gitremote-helpers.txt
index c2908db..c24c0c2 100644
--- a/Documentation/gitremote-helpers.txt
+++ b/Documentation/gitremote-helpers.txt
@@ -444,6 +444,10 @@ set by Git if the remote helper has the 'option'
capability.
'option update-shallow \{'true'|'false'\}::
Allow to extend .git/shallow if the new refs require it.
+'option force' \{'true'|'false'\}::
+ Request the helper to perform a force update. Defaults to
+ 'false'.
+
SEE ALSO
linkgit:git-remote[1]
diff --git a/git-remote-testgit.sh b/git-remote-testgit.sh
index 6d2f282..1c006a0 100755
--- a/git-remote-testgit.sh
+++ b/git-remote-testgit.sh
@@ -15,6 +15,8 @@ test -z "$refspec" && prefix="refs"
export GIT_DIR="$url/.git"
+force=
+
mkdir -p "$dir"
if test -z "$GIT_REMOTE_TESTGIT_NO_MARKS"
@@ -39,6 +41,7 @@ do
fi
test -n "$GIT_REMOTE_TESTGIT_SIGNED_TAGS" && echo "signed-tags"
test -n "$GIT_REMOTE_TESTGIT_NO_PRIVATE_UPDATE" && echo
"no-private-update"
+ echo 'option'
echo
;;
list)
@@ -93,6 +96,7 @@ do
before=$(git for-each-ref --format=' %(refname) %(objectname) ')
git fast-import \
+ ${force:+--force} \
${testgitmarks:+"--import-marks=$testgitmarks"} \
${testgitmarks:+"--export-marks=$testgitmarks"} \
--quiet
@@ -115,6 +119,20 @@ do
echo
;;
+ option\ *)
+ read cmd opt val <<-EOF
+ $line
+ EOF
+ case $opt in
+ force)
+ test $val = "true" && force="true" || force=
+ echo "ok"
+ ;;
+ *)
+ echo "unsupported"
+ ;;
+ esac
+ ;;
'')
exit
;;
diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh
index 613f69a..c33cc25 100755
--- a/t/t5801-remote-helpers.sh
+++ b/t/t5801-remote-helpers.sh
@@ -94,6 +94,19 @@ test_expect_failure 'push new branch with old:new refspec' '
compare_refs local HEAD server refs/heads/new-refspec
'
+test_expect_success 'forced push' '
+ (cd local &&
+ git checkout -b force-test &&
+ echo content >> file &&
+ git commit -a -m eight &&
+ git push origin force-test &&
+ echo content >> file &&
+ git commit -a --amend -m eight-modified &&
+ git push --force origin force-test
+ ) &&
+ compare_refs local refs/heads/force-test server refs/heads/force-test
+'
+
test_expect_success 'cloning without refspec' '
GIT_REMOTE_TESTGIT_REFSPEC="" \
git clone "testgit::${PWD}/server" local2 2>error &&
diff --git a/transport-helper.c b/transport-helper.c
index 4b3e38e..f50e84f 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -861,6 +861,11 @@ static int push_refs_with_export(struct transport
*transport,
die("helper %s does not support dry-run", data->name);
}
+ if (flags & TRANSPORT_PUSH_FORCE) {
+ if (set_helper_option(transport, "force", "true") != 0)
+ warning("helper %s does not support 'force'",
data->name);
+ }
+
helper = get_helper(transport);
write_constant(helper->in, "export\n");
--
1.9.1+fc3.9.gc73078e
--
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 v9 0/6] transport-helper: fixes
Hi, These patches add support for remote helpers --force, --dry-run, and reporting forced update. Changes since v8: --- 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); } Felipe Contreras (4): transport-helper: mismerge fix transport-helper: don't update refs in dry-run transport-helper: add 'force' to 'export' helpers transport-helper: check for 'forced update' message Richard Hansen (2): test-hg.sh: tests are now expected to pass remote-bzr: support the new 'force' option Documentation/gitremote-helpers.txt | 4 contrib/remote-helpers/git-remote-bzr | 31 ++- contrib/remote-helpers/test-bzr.sh| 22 +- contrib/remote-helpers/test-hg.sh | 4 ++-- git-remote-testgit.sh | 18 ++ t/t5801-remote-helpers.sh | 13 + transport-helper.c| 25 + 7 files changed, 105 insertions(+), 12 deletions(-) -- 1.9.1+fc3.9.gc73078e -- 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 v9 5/6] test-hg.sh: tests are now expected to pass
From: Richard Hansen Signed-off-by: Richard Hansen Signed-off-by: Junio C Hamano Signed-off-by: Felipe Contreras --- contrib/remote-helpers/test-hg.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/contrib/remote-helpers/test-hg.sh b/contrib/remote-helpers/test-hg.sh index 5d128a5..a933b1e 100755 --- a/contrib/remote-helpers/test-hg.sh +++ b/contrib/remote-helpers/test-hg.sh @@ -680,7 +680,7 @@ test_expect_success 'remote big push fetch first' ' ) ' -test_expect_failure 'remote big push force' ' +test_expect_success 'remote big push force' ' test_when_finished "rm -rf hgrepo gitrepo*" && setup_big_push @@ -710,7 +710,7 @@ test_expect_failure 'remote big push force' ' check_bookmark hgrepo new_bmark six ' -test_expect_failure 'remote big push dry-run' ' +test_expect_success 'remote big push dry-run' ' test_when_finished "rm -rf hgrepo gitrepo*" && setup_big_push -- 1.9.1+fc3.9.gc73078e -- 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 v9 6/6] remote-bzr: support the new 'force' option
From: Richard Hansen
Signed-off-by: Richard Hansen
Acked-by: Felipe Contreras
Signed-off-by: Junio C Hamano
Signed-off-by: Felipe Contreras
---
contrib/remote-helpers/git-remote-bzr | 31 ++-
contrib/remote-helpers/test-bzr.sh| 22 +-
2 files changed, 51 insertions(+), 2 deletions(-)
diff --git a/contrib/remote-helpers/git-remote-bzr
b/contrib/remote-helpers/git-remote-bzr
index 332aba7..5f4b2e3 100755
--- a/contrib/remote-helpers/git-remote-bzr
+++ b/contrib/remote-helpers/git-remote-bzr
@@ -684,7 +684,8 @@ def do_export(parser):
peer = bzrlib.branch.Branch.open(peers[name],
possible_transports=transports)
try:
-peer.bzrdir.push_branch(branch, revision_id=revid)
+peer.bzrdir.push_branch(branch, revision_id=revid,
+overwrite=force)
except bzrlib.errors.DivergedBranches:
print "error %s non-fast forward" % ref
continue
@@ -718,8 +719,32 @@ def do_capabilities(parser):
print "*import-marks %s" % path
print "*export-marks %s" % path
+print "option"
print
+class InvalidOptionValue(Exception):
+pass
+
+def get_bool_option(val):
+if val == 'true':
+return True
+elif val == 'false':
+return False
+else:
+raise InvalidOptionValue()
+
+def do_option(parser):
+global force
+opt, val = parser[1:3]
+try:
+if opt == 'force':
+force = get_bool_option(val)
+print 'ok'
+else:
+print 'unsupported'
+except InvalidOptionValue:
+print "error '%s' is not a valid value for option '%s'" % (val, opt)
+
def ref_is_valid(name):
return not True in [c in name for c in '~^: \\']
@@ -882,6 +907,7 @@ def main(args):
global is_tmp
global branches, peers
global transports
+global force
marks = None
is_tmp = False
@@ -904,6 +930,7 @@ def main(args):
branches = {}
peers = {}
transports = []
+force = False
if alias[5:] == url:
is_tmp = True
@@ -936,6 +963,8 @@ def main(args):
do_import(parser)
elif parser.check('export'):
do_export(parser)
+elif parser.check('option'):
+do_option(parser)
else:
die('unhandled command: %s' % line)
sys.stdout.flush()
diff --git a/contrib/remote-helpers/test-bzr.sh
b/contrib/remote-helpers/test-bzr.sh
index 1e53ff9..4f379c2 100755
--- a/contrib/remote-helpers/test-bzr.sh
+++ b/contrib/remote-helpers/test-bzr.sh
@@ -66,13 +66,33 @@ test_expect_success 'pushing' '
test_cmp expected actual
'
+test_expect_success 'forced pushing' '
+ (
+ cd gitrepo &&
+ echo three-new >content &&
+ git commit -a --amend -m three-new &&
+ git push -f
+ ) &&
+
+ (
+ cd bzrrepo &&
+ # the forced update overwrites the bzr branch but not the bzr
+ # working directory (it tries to merge instead)
+ bzr revert
+ ) &&
+
+ echo three-new >expected &&
+ cat bzrrepo/content >actual &&
+ test_cmp expected actual
+'
+
test_expect_success 'roundtrip' '
(
cd gitrepo &&
git pull &&
git log --format="%s" -1 origin/master >actual
) &&
- echo three >expected &&
+ echo three-new >expected &&
test_cmp expected actual &&
(cd gitrepo && git push && git pull) &&
--
1.9.1+fc3.9.gc73078e
--
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 v9 1/6] transport-helper: mismerge fix
Commit 9c51558 (transport-helper: trivial code shuffle) moved these
lines above, but 99d9ec0 (Merge branch 'fc/transport-helper-no-refspec')
had a wrong merge conflict and readded them.
Reported-by: Richard Hansen
Signed-off-by: Felipe Contreras
Signed-off-by: Junio C Hamano
---
transport-helper.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/transport-helper.c b/transport-helper.c
index ad72fbd..ea34d0c 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -881,9 +881,6 @@ static int push_refs_with_export(struct transport
*transport,
}
free(private);
- if (ref->deletion)
- die("remote-helpers do not support ref deletion");
-
if (ref->peer_ref) {
if (strcmp(ref->peer_ref->name, ref->name))
die("remote-helpers do not support old:new
syntax");
--
1.9.1+fc3.9.gc73078e
--
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 v9 2/6] transport-helper: don't update refs in dry-run
The remote helper namespace should not be updated.
Signed-off-by: Felipe Contreras
Signed-off-by: Junio C Hamano
---
transport-helper.c | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/transport-helper.c b/transport-helper.c
index ea34d0c..4b3e38e 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -734,7 +734,8 @@ static int push_update_ref_status(struct strbuf *buf,
}
static void push_update_refs_status(struct helper_data *data,
- struct ref *remote_refs)
+ struct ref *remote_refs,
+ int flags)
{
struct strbuf buf = STRBUF_INIT;
struct ref *ref = remote_refs;
@@ -748,7 +749,7 @@ static void push_update_refs_status(struct helper_data
*data,
if (push_update_ref_status(&buf, &ref, remote_refs))
continue;
- if (!data->refspecs || data->no_private_update)
+ if (flags & TRANSPORT_PUSH_DRY_RUN || !data->refspecs ||
data->no_private_update)
continue;
/* propagate back the update to the remote namespace */
@@ -839,7 +840,7 @@ static int push_refs_with_push(struct transport *transport,
sendline(data, &buf);
strbuf_release(&buf);
- push_update_refs_status(data, remote_refs);
+ push_update_refs_status(data, remote_refs, flags);
return 0;
}
@@ -893,7 +894,7 @@ static int push_refs_with_export(struct transport
*transport,
if (finish_command(&exporter))
die("Error while running fast-export");
- push_update_refs_status(data, remote_refs);
+ push_update_refs_status(data, remote_refs, flags);
return 0;
}
--
1.9.1+fc3.9.gc73078e
--
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 v9 4/6] transport-helper: check for 'forced update' message
So the remote-helpers can tell us when a forced push was needed.
Helped-by: Max Horn
Signed-off-by: Felipe Contreras
Signed-off-by: Junio C Hamano
---
transport-helper.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/transport-helper.c b/transport-helper.c
index f50e84f..86e1679 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -650,7 +650,7 @@ static int push_update_ref_status(struct strbuf *buf,
struct ref *remote_refs)
{
char *refname, *msg;
- int status;
+ int status, forced = 0;
if (starts_with(buf->buf, "ok ")) {
status = REF_STATUS_OK;
@@ -708,6 +708,11 @@ static int push_update_ref_status(struct strbuf *buf,
free(msg);
msg = NULL;
}
+ else if (!strcmp(msg, "forced update")) {
+ forced = 1;
+ free(msg);
+ msg = NULL;
+ }
}
if (*ref)
@@ -729,6 +734,7 @@ static int push_update_ref_status(struct strbuf *buf,
}
(*ref)->status = status;
+ (*ref)->forced_update |= forced;
(*ref)->remote_status = msg;
return !(status == REF_STATUS_OK);
}
--
1.9.1+fc3.9.gc73078e
--
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] l10n: de.po: correct translation of "completed" after resolving deltas
Signed-off-by: Ralf Thielow --- I'll queue this fix as part of the German l10n changes for the next release. po/de.po | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/po/de.po b/po/de.po index 773ae87..3e01e72 100644 --- a/po/de.po +++ b/po/de.po @@ -6108,7 +6108,7 @@ msgstr "Fehler beim Auflösen der Unterschiede" #: builtin/index-pack.c:1138 #, c-format msgid "completed with %d local objects" -msgstr "vervollständigt mit %d lokalen Objekten" +msgstr "abgeschlossen mit %d lokalen Objekten" #: builtin/index-pack.c:1148 #, c-format -- 1.9.2.649.g5ae4ba5 -- 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 0/5] transport-helper: serious crash fix
One of the most serious recurring issues[1][2][3] with remote helpers is that marks get out of sync. The way to analize and reproduce these wasn't trivial, but the culprit seems to be a crash while doing `git push`. It has been known already how exactly that happens, but no simple way how to fix it. This is the simplest way so far; tell `git fast-export` to export the marks to a temporary file, and move it to the right location only *after* the remote helper has finished its job without errors. Since the code wasn't prepared for a change like this, some reorganization changes are needed. More changes might be welcome to further propagate the errors properly through the code, but for the moment the errors are propagated to the right location, in order to fix this specific problem. [1] http://article.gmane.org/gmane.comp.version-control.git/223962 [2] http://article.gmane.org/gmane.comp.version-control.git/241707 [3] https://github.com/felipec/git/issues/56 Felipe Contreras (5): transport-helper: remove barely used xchgline() remote-helpers: make recvline return an error transport-helper: propagate recvline() error pushing transport-helper: trivial cleanup transport-helper: fix sync issue on crashes t/t5801-remote-helpers.sh | 17 ++- transport-helper.c| 72 --- 2 files changed, 59 insertions(+), 30 deletions(-) -- 1.9.1+fc3.9.gc73078e -- 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 1/5] transport-helper: remove barely used xchgline()
It's only used once, we can just call the two functions inside directly.
Signed-off-by: Felipe Contreras
---
transport-helper.c | 9 ++---
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/transport-helper.c b/transport-helper.c
index ad72fbd..bf329fd 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -71,12 +71,6 @@ static int recvline(struct helper_data *helper, struct
strbuf *buffer)
return recvline_fh(helper->out, buffer, helper->name);
}
-static void xchgline(struct helper_data *helper, struct strbuf *buffer)
-{
- sendline(helper, buffer);
- recvline(helper, buffer);
-}
-
static void write_constant(int fd, const char *str)
{
if (debug)
@@ -307,7 +301,8 @@ static int set_helper_option(struct transport *transport,
quote_c_style(value, &buf, NULL, 0);
strbuf_addch(&buf, '\n');
- xchgline(data, &buf);
+ sendline(data, &buf);
+ recvline(data, &buf);
if (!strcmp(buf.buf, "ok"))
ret = 0;
--
1.9.1+fc3.9.gc73078e
--
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/5] remote-helpers: make recvline return an error
Instead of exiting directly, make it the duty of the caller to do so.
Signed-off-by: Felipe Contreras
---
transport-helper.c | 21 ++---
1 file changed, 14 insertions(+), 7 deletions(-)
diff --git a/transport-helper.c b/transport-helper.c
index bf329fd..1432a6d 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -58,7 +58,7 @@ static int recvline_fh(FILE *helper, struct strbuf *buffer,
const char *name)
if (strbuf_getline(buffer, helper, '\n') == EOF) {
if (debug)
fprintf(stderr, "Debug: Remote helper quit.\n");
- exit(128);
+ return 1;
}
if (debug)
@@ -157,7 +157,8 @@ static struct child_process *get_helper(struct transport
*transport)
while (1) {
const char *capname;
int mandatory = 0;
- recvline(data, &buf);
+ if (recvline(data, &buf))
+ exit(128);
if (!*buf.buf)
break;
@@ -302,7 +303,8 @@ static int set_helper_option(struct transport *transport,
strbuf_addch(&buf, '\n');
sendline(data, &buf);
- recvline(data, &buf);
+ if (recvline(data, &buf))
+ exit(128);
if (!strcmp(buf.buf, "ok"))
ret = 0;
@@ -374,7 +376,8 @@ static int fetch_with_fetch(struct transport *transport,
sendline(data, &buf);
while (1) {
- recvline(data, &buf);
+ if (recvline(data, &buf))
+ exit(128);
if (starts_with(buf.buf, "lock ")) {
const char *name = buf.buf + 5;
@@ -558,7 +561,9 @@ static int process_connect_service(struct transport
*transport,
goto exit;
sendline(data, &cmdbuf);
- recvline_fh(input, &cmdbuf, name);
+ if (recvline_fh(input, &cmdbuf, name))
+ exit(128);
+
if (!strcmp(cmdbuf.buf, "")) {
data->no_disconnect_req = 1;
if (debug)
@@ -736,7 +741,8 @@ static void push_update_refs_status(struct helper_data
*data,
for (;;) {
char *private;
- recvline(data, &buf);
+ if (recvline(data, &buf))
+ exit(128);
if (!buf.len)
break;
@@ -960,7 +966,8 @@ static struct ref *get_refs_list(struct transport
*transport, int for_push)
while (1) {
char *eov, *eon;
- recvline(data, &buf);
+ if (recvline(data, &buf))
+ exit(128);
if (!*buf.buf)
break;
--
1.9.1+fc3.9.gc73078e
--
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 4/5] transport-helper: trivial cleanup
It's simpler to store the file names directly, and form the fast-export
arguments only when needed, and re-use the same strbuf with a format.
Signed-off-by: Felipe Contreras
---
transport-helper.c | 23 +++
1 file changed, 11 insertions(+), 12 deletions(-)
diff --git a/transport-helper.c b/transport-helper.c
index b068ea5..2747f98 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -195,15 +195,9 @@ static struct child_process *get_helper(struct transport
*transport)
} else if (!strcmp(capname, "signed-tags")) {
data->signed_tags = 1;
} else if (starts_with(capname, "export-marks ")) {
- struct strbuf arg = STRBUF_INIT;
- strbuf_addstr(&arg, "--export-marks=");
- strbuf_addstr(&arg, capname + strlen("export-marks "));
- data->export_marks = strbuf_detach(&arg, NULL);
+ data->export_marks = xstrdup(capname +
strlen("export-marks "));
} else if (starts_with(capname, "import-marks")) {
- struct strbuf arg = STRBUF_INIT;
- strbuf_addstr(&arg, "--import-marks=");
- strbuf_addstr(&arg, capname + strlen("import-marks "));
- data->import_marks = strbuf_detach(&arg, NULL);
+ data->import_marks = xstrdup(capname +
strlen("import-marks "));
} else if (starts_with(capname, "no-private-update")) {
data->no_private_update = 1;
} else if (mandatory) {
@@ -429,6 +423,7 @@ static int get_exporter(struct transport *transport,
struct child_process *helper = get_helper(transport);
int argc = 0, i;
memset(fastexport, 0, sizeof(*fastexport));
+ struct strbuf tmp = STRBUF_INIT;
/* we need to duplicate helper->in because we want to use it after
* fastexport is done with it. */
@@ -438,10 +433,14 @@ static int get_exporter(struct transport *transport,
fastexport->argv[argc++] = "--use-done-feature";
fastexport->argv[argc++] = data->signed_tags ?
"--signed-tags=verbatim" : "--signed-tags=warn-strip";
- if (data->export_marks)
- fastexport->argv[argc++] = data->export_marks;
- if (data->import_marks)
- fastexport->argv[argc++] = data->import_marks;
+ if (data->export_marks) {
+ strbuf_addf(&tmp, "--export-marks=%s", data->export_marks);
+ fastexport->argv[argc++] = strbuf_detach(&tmp, NULL);
+ }
+ if (data->import_marks) {
+ strbuf_addf(&tmp, "--import-marks=%s", data->import_marks);
+ fastexport->argv[argc++] = strbuf_detach(&tmp, NULL);
+ }
for (i = 0; i < revlist_args->nr; i++)
fastexport->argv[argc++] = revlist_args->items[i].string;
--
1.9.1+fc3.9.gc73078e
--
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/5] transport-helper: propagate recvline() error pushing
It's cleaner, and will allow us to do something sensible on errors
later.
Signed-off-by: Felipe Contreras
---
transport-helper.c | 18 +++---
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/transport-helper.c b/transport-helper.c
index 1432a6d..b068ea5 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -733,16 +733,21 @@ static int push_update_ref_status(struct strbuf *buf,
return !(status == REF_STATUS_OK);
}
-static void push_update_refs_status(struct helper_data *data,
+static int push_update_refs_status(struct helper_data *data,
struct ref *remote_refs)
{
struct strbuf buf = STRBUF_INIT;
struct ref *ref = remote_refs;
+ int ret = 0;
+
for (;;) {
char *private;
- if (recvline(data, &buf))
- exit(128);
+ if (recvline(data, &buf)) {
+ ret = 1;
+ break;
+ }
+
if (!buf.len)
break;
@@ -760,6 +765,7 @@ static void push_update_refs_status(struct helper_data
*data,
free(private);
}
strbuf_release(&buf);
+ return ret;
}
static int push_refs_with_push(struct transport *transport,
@@ -840,8 +846,7 @@ static int push_refs_with_push(struct transport *transport,
sendline(data, &buf);
strbuf_release(&buf);
- push_update_refs_status(data, remote_refs);
- return 0;
+ return push_update_refs_status(data, remote_refs);
}
static int push_refs_with_export(struct transport *transport,
@@ -897,8 +902,7 @@ static int push_refs_with_export(struct transport
*transport,
if (finish_command(&exporter))
die("Error while running fast-export");
- push_update_refs_status(data, remote_refs);
- return 0;
+ return push_update_refs_status(data, remote_refs);
}
static int push_refs(struct transport *transport,
--
1.9.1+fc3.9.gc73078e
--
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 5/5] transport-helper: fix sync issue on crashes
When a remote helper crashes while pushing we should revert back to the
state before the push, however, it's possible that `git fast-export`
already finished its job, and therefore has exported the marks already.
This creates a synchronization problem because from that moment on
`git fast-{import,export}` will have marks that the remote helper is not
aware of and all further commands fail (if those marks are referenced).
The fix is to tell `git fast-export` to export to a temporary file, and
only after the remote helper has finishes successfully, move to the
final destination.
Signed-off-by: Felipe Contreras
---
t/t5801-remote-helpers.sh | 17 -
transport-helper.c| 13 +++--
2 files changed, 27 insertions(+), 3 deletions(-)
diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh
index 613f69a..cf7fd43 100755
--- a/t/t5801-remote-helpers.sh
+++ b/t/t5801-remote-helpers.sh
@@ -207,6 +207,17 @@ test_expect_success 'push update refs failure' '
)
'
+clean_mark () {
+ cut -f 2 -d ' ' $1 | git cat-file --batch-check | grep commit | sort >
$(basename $1)
+}
+
+cmp_marks () {
+ test_when_finished "rm -rf git.marks testgit.marks" &&
+ clean_mark .git/testgit/$1/git.marks &&
+ clean_mark .git/testgit/$1/testgit.marks &&
+ test_cmp git.marks testgit.marks
+}
+
test_expect_success 'proper failure checks for fetching' '
(GIT_REMOTE_TESTGIT_FAILURE=1 &&
export GIT_REMOTE_TESTGIT_FAILURE &&
@@ -221,7 +232,11 @@ test_expect_success 'proper failure checks for pushing' '
(GIT_REMOTE_TESTGIT_FAILURE=1 &&
export GIT_REMOTE_TESTGIT_FAILURE &&
cd local &&
- test_must_fail git push --all
+ git checkout -b crash master &&
+ echo crash >> file &&
+ git commit -a -m crash &&
+ test_must_fail git push --all &&
+ cmp_marks origin
)
'
diff --git a/transport-helper.c b/transport-helper.c
index 2747f98..090c863 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -434,7 +434,7 @@ static int get_exporter(struct transport *transport,
fastexport->argv[argc++] = data->signed_tags ?
"--signed-tags=verbatim" : "--signed-tags=warn-strip";
if (data->export_marks) {
- strbuf_addf(&tmp, "--export-marks=%s", data->export_marks);
+ strbuf_addf(&tmp, "--export-marks=%s.tmp", data->export_marks);
fastexport->argv[argc++] = strbuf_detach(&tmp, NULL);
}
if (data->import_marks) {
@@ -901,7 +901,16 @@ static int push_refs_with_export(struct transport
*transport,
if (finish_command(&exporter))
die("Error while running fast-export");
- return push_update_refs_status(data, remote_refs);
+ if (push_update_refs_status(data, remote_refs))
+ return 1;
+
+ if (data->export_marks) {
+ strbuf_addf(&buf, "%s.tmp", data->export_marks);
+ rename(buf.buf, data->export_marks);
+ strbuf_release(&buf);
+ }
+
+ return 0;
}
static int push_refs(struct transport *transport,
--
1.9.1+fc3.9.gc73078e
--
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-remote-bzr: fatal: mark :399654 not declared
Rüdiger Sonderfeld wrote: > I'm using git-remote-bzr to access the GNU Emacs Bazaar repo. I followed the > guideline described here: > https://lists.gnu.org/archive/html/emacs-devel/2013-05/msg8.html > > Pulling and pushing worked flawless for several month. But recently git > bzr stopped working with the following error message: Sorry for the late reply. So basically your repository is screwed up. I just sent patches to the Git's core so it doesn't happen again. To recover the easiest is to remove these files: .git/bzr/origin/marks-*, in your repository, and then run `git fetch`. However, when you do that basically you have to fetch everything again. You could also try this script I wrote[1]. If you run `git marks-check -mbzr -v -f origin` it will find all the issues in the mark files and fix them. Then run `git fetch` again. Cheers. [1] https://gist.github.com/felipec/10551806 -- Felipe Contreras-- 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: Cannot push anything via export transport helper after push fails.
Felipe Contreras wrote: > On Sat, May 11, 2013 at 7:29 AM, Andrey Borzenkov wrote: > > I noticed that using git-remote-bzr, but as far as I can tell this is > > generic for all transport helpers using fast-export. > > > > > > > > What happened was "git push" failed due to merge conflict. So far so > > good - but from now on git assumes everything is up to date. > > > > bor@opensuse:/tmp/test/git> git push origin master > > To bzr::bzr+ssh://bor@localhost/tmp/test/bzr > > ! [rejected]master -> master (non-fast-forward) > > error: failed to push some refs to > > 'bzr::bzr+ssh://bor@localhost/tmp/test/bzr' > > 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. > > bor@opensuse:/tmp/test/git> git push origin master > > Everything up-to-date > > bor@opensuse:/tmp/test/git> > > > > The problem seems to be that git fast-export updates marks > > unconditionally, whether export actually applied or not. So next time > > it assumes everything is already exported and does nothing. > > > > Is it expected behavior? > > Indeed, this is the way it currently works, and it's not easy to fix. > We would need some way to make fast-export wait until we know the exit > status of the remote helper, and then tell it when it failed, so the > marks are not updated. > > However, the way remote-bzr/hg work is that the commits are still > there anyway. So if you merge the next time you push those commits are > already converted, so it's not a problem if fast-export is not > exporting them again. > > So even though it's not ideal, it should work. > > The problem is when the remote-helper crashes and the marks of > fast-export and the remote-helper are out of sync, and then the user > is really screwed. I sent patches that should fix this problem: http://article.gmane.org/gmane.comp.version-control.git/246187 -- Felipe Contreras -- 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: Cannot push anything via export transport helper after push fails.
Andrey Borzenkov wrote: > В Sat, 11 May 2013 08:57:14 -0500 > Felipe Contreras пишет: > > > On Sat, May 11, 2013 at 7:29 AM, Andrey Borzenkov > > wrote: > > > I noticed that using git-remote-bzr, but as far as I can tell this is > > > generic for all transport helpers using fast-export. > > > > > > > > > > > > What happened was "git push" failed due to merge conflict. So far so > > > good - but from now on git assumes everything is up to date. > > > > > > bor@opensuse:/tmp/test/git> git push origin master > > > To bzr::bzr+ssh://bor@localhost/tmp/test/bzr > > > ! [rejected]master -> master (non-fast-forward) > > > error: failed to push some refs to > > > 'bzr::bzr+ssh://bor@localhost/tmp/test/bzr' > > > 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. > > > bor@opensuse:/tmp/test/git> git push origin master > > > Everything up-to-date > > > bor@opensuse:/tmp/test/git> > > > > > > The problem seems to be that git fast-export updates marks > > > unconditionally, whether export actually applied or not. So next time > > > it assumes everything is already exported and does nothing. > > > > > > Is it expected behavior? > > > > Indeed, this is the way it currently works, and it's not easy to fix. > > We would need some way to make fast-export wait until we know the exit > > status of the remote helper, and then tell it when it failed, so the > > marks are not updated. > > > > Hmm ... actually as far as I understand transport-helper keeps track of > which revisions to push in "remote helper ref" (for the lack of better > word). This makes use of marks as tracking means rather redundant. > > What about the idea below? This relies on transport helper to provide > correct revisions and uses marks exclusively as cross-reference between > GIT and remote SCM. It is on top of next branch. This is one way of using it, but not ideal, and I think the patch series I sent should work for all remote helpers. -- Felipe Contreras-- 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] test: fix t7001 cp to use POSIX options
Am 11.04.2014 21:23, schrieb Junio C Hamano: > Jeff King writes: > >> On Fri, Apr 11, 2014 at 01:24:02AM -0700, Kyle J. McKay wrote: >> >>> Since 11502468 and 04c1ee57 (both first appearing in v1.8.5), the >>> t7001-mv test has used "cp -a" to perform a copy in several of the >>> tests. >>> >>> However, the "-a" option is not required for a POSIX cp utility and >>> some platforms' cp utilities do not support it. >>> >>> The POSIX equivalent of -a is -R -P -p. >>> >>> Change "cp -a" to "cp -R -P -p" so that the t7001-mv test works >>> on systems with a cp utility that only implements the POSIX >>> required set of options and not the "-a" option. >> >> I wonder if the "-R" is the part that we actually care about here. >> Including the others does not hurt in that case, but using only "-R" >> would perhaps make it more obvious to a later reader of the code exactly >> what we are trying to do. > > These calls to "cp" are about "We know that earlier 'update' created > the GIT_DIR of the submodule in the top-level superproject, because > we are running a modern Git. But we want to make sure the command > we are testing, "git mv", would work correctly if the repository > were made with an older Git that created the GIT_DIR embedded in the > working tree of the submodule, so let's emulate that case." As we > create files and directories in GIT_DIR with explicit permission > bits, we may care about > > (1) making sure "git mv" can actually move the directory, with some > paths having mode bits different from the umasked default; and > > (2) checking that the GIT_DIR after "git mv" has the permission > bits right. When writing these tests I didn't care about (2), but - in addition to the first half of (1) - I had another thing in mind: (3) "git mv" shouldn't try to update the 'core.worktree' setting when the GIT_DIR is embedded in the submodule's work tree. > and if we cared, "-R -p" would be required here, not just "-R". > > If core.prefersymlinkrefs is set, GIT_DIR would have a symbolic link > HEAD pointing at the current branch, and "-P" may become relevant, > but manually running "cp -R .git git && ls -l git/HEAD" in such an > old repository tells me that symbolic link HEAD is not dereferenced > without an explicit "-L", so I dunno. > > Because we do not check anything inside GIT_DIR of the moved > submodule after "git mv" is done, the more correct use of "cp" is > moot for the purpose of (2), but it could be possible that "git mv" > fails to move a submodule with GIT_DIR created embedded in its > working tree by an older version of Git, while successfully copying > an emulated one, due to differences such as modes and symlinks. > > The current implementation just does rename(2) on the whole > submodule working tree and let its contents move together, so I do > not think it matters at the moment for the purpose of (1); use of > flags other than "-R" are purely for future-proofing, I would think. Thanks for your detailed analysis and sorry to all parties involved for the hassle caused by my knee-jerk reaction to just use "cp -a" when I wanted to have an exact copy of 'that' directory 'there'. Given that all other tests just use "cp -R" too in that situation I'm all for the second version of Kyle's patch, so an "Acked-by" from me on that one. -- 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-remote-hg
Christophe wrote: > I am using git-remote-hg to access to projects on bitbucket. I can clone the > master branch fine and push to it. I also see hg branches as > remotes/origin/branches/«branch». However, if I create a local branch > "branches/x" and want to push it to remotes/origin/branches/x, it gets pushed > to the remote master (aka default) branch. Which version of the script and Git are you using? If I do `git push origin branches/test` it works fine, I can see the branch in bitbucket. > What is the recommended way of working with multiple branches when > interacting with a remote hg repository? My recommendation is to avoid them (I don't see the point of them), but they should work. -- Felipe Contreras-- 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] commit: add --ignore-submodules[=] parameter
On 8. 4. 2014 20:43, Jens Lehmann wrote:
Useful values for commit are 'all' (default) or 'none'. The others
('dirty' and 'untracked') have same effect as 'none', as commit is only
interested in whether the submodule's HEAD differs from what is commited
in the superproject.
Unless it outputs a status message, then 'dirty' and 'untracked' do
influence what is shown there. Apart from that (and maybe tests for
these two cases ;-) this is looking good to me.
OK, I updated the patch for commit to take that into account. Also, I
rebased both patches onto current master. Sending them in a moment.
If you don't have any more complaints, can I add "Acked-by: " and
resend the patches to Junio?
--
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] checkout: Fix grammar in inline comment.
Inline comment had incorrect grammar. Fix grammatical mistakes and reflect actual behavior of the function. Signed-off-by: Brian Gesiak --- builtin/checkout.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/builtin/checkout.c b/builtin/checkout.c index 63151e0..abe1161 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -888,8 +888,8 @@ static int parse_branchname_arg(int argc, const char **argv, * * case 3: git checkout [--] * -* (a) If is a commit, that is to -* switch to the branch or detach HEAD at it. As a special case, +* (a) If is a commit, switch to that branch or +* detach HEAD at that commit. As a special case, * if is A...B (missing A or B means HEAD but you can * omit at most one side), and if there is a unique merge base * between A and B, A...B names that merge base. -- 1.9.0.259.gc5d75e8.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
[PATCH v4 1/2] add: add --ignore-submodules[=] parameter
Allow ignoring submodules (or not) by command line switch, like diff
and status do.
Git add currently doesn't honor ignore from .gitmodules or .git/config,
which is related functionality, however I'd like to change that in
another patch, coming soon.
This commit is also a prerequisite for the next one in series, which
adds the --ignore-submodules switch to git commit. That's why signature
of function add_files_to_cache was changed. That also requires compilo
fixes in checkout.c and commit.c
Signed-off-by: Ronald Weiss
---
Documentation/git-add.txt| 7 ++-
builtin/add.c| 16 --
builtin/checkout.c | 2 +-
builtin/commit.c | 2 +-
cache.h | 2 +-
t/t3704-add-ignore-submodules.sh | 45
6 files changed, 68 insertions(+), 6 deletions(-)
create mode 100644 t/t3704-add-ignore-submodules.sh
diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt
index 9631526..b2c936f 100644
--- a/Documentation/git-add.txt
+++ b/Documentation/git-add.txt
@@ -11,7 +11,7 @@ SYNOPSIS
'git add' [-n] [-v] [--force | -f] [--interactive | -i] [--patch | -p]
[--edit | -e] [--[no-]all | --[no-]ignore-removal | [--update | -u]]
[--intent-to-add | -N] [--refresh] [--ignore-errors]
[--ignore-missing]
- [--] [...]
+ [--ignore-submodules[=]] [--] [...]
DESCRIPTION
---
@@ -164,6 +164,11 @@ for "git add --no-all ...", i.e. ignored removed
files.
be ignored, no matter if they are already present in the work
tree or not.
+--ignore-submodules[=]::
+ Can be used to override any settings of the 'submodule.*.ignore'
+ option in linkgit:git-config[1] or linkgit:gitmodules[5].
+can be either "none" or "all", which is the default.
+
\--::
This option can be used to separate command-line options from
the list of files, (useful when filenames might be mistaken
diff --git a/builtin/add.c b/builtin/add.c
index 459208a..85f2110 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -83,7 +83,8 @@ static void update_callback(struct diff_queue_struct *q,
}
int add_files_to_cache(const char *prefix,
- const struct pathspec *pathspec, int flags)
+ const struct pathspec *pathspec, int flags,
+ const char *ignore_submodules_arg)
{
struct update_callback_data data;
struct rev_info rev;
@@ -99,6 +100,12 @@ int add_files_to_cache(const char *prefix,
rev.diffopt.format_callback = update_callback;
rev.diffopt.format_callback_data = &data;
rev.max_count = 0; /* do not compare unmerged paths with stage #2 */
+
+ if (ignore_submodules_arg) {
+ DIFF_OPT_SET(&rev.diffopt, OVERRIDE_SUBMODULE_CONFIG);
+ handle_ignore_submodules_arg(&rev.diffopt,
ignore_submodules_arg);
+ }
+
run_diff_files(&rev, DIFF_RACY_IS_MODIFIED);
return !!data.add_errors;
}
@@ -237,6 +244,8 @@ static int ignore_add_errors, intent_to_add, ignore_missing;
static int addremove = ADDREMOVE_DEFAULT;
static int addremove_explicit = -1; /* unspecified */
+static char *ignore_submodule_arg;
+
static int ignore_removal_cb(const struct option *opt, const char *arg, int
unset)
{
/* if we are told to ignore, we are not adding removals */
@@ -262,6 +271,9 @@ static struct option builtin_add_options[] = {
OPT_BOOL( 0 , "refresh", &refresh_only, N_("don't add, only refresh the
index")),
OPT_BOOL( 0 , "ignore-errors", &ignore_add_errors, N_("just skip files
which cannot be added because of errors")),
OPT_BOOL( 0 , "ignore-missing", &ignore_missing, N_("check if - even
missing - files are ignored in dry run")),
+ { OPTION_STRING, 0, "ignore-submodules", &ignore_submodule_arg,
N_("when"),
+ N_("ignore changes to submodules, optional when: all, none. (Default:
all)"),
+ PARSE_OPT_OPTARG, NULL, (intptr_t)"all" },
OPT_END(),
};
@@ -434,7 +446,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
plug_bulk_checkin();
- exit_status |= add_files_to_cache(prefix, &pathspec, flags);
+ exit_status |= add_files_to_cache(prefix, &pathspec, flags,
ignore_submodule_arg);
if (add_new_files)
exit_status |= add_files(&dir, flags);
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 07cf555..607af47 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -525,7 +525,7 @@ static int merge_working_tree(const struct checkout_opts
*opts,
* entries in the index.
*/
- add_files_to_cache(NULL, NULL, 0);
+ add_files_to_cache(NULL, NULL, 0, NULL);
/*
* NEEDSWORK: carrying over local changes
* when branches have d
[PATCH v4 2/2] commit: add --ignore-submodules[=] parameter
Allow ignoring submodules (or not) by command line switch, like diff
and status do.
Git commit honors the 'ignore' setting from .gitmodules or .git/config,
but didn't allow to override it from command line.
This patch depends on Jens Lehmann's patch "commit -m: commit staged
submodules regardless of ignore config". Without it,
"commit -m --ignore-submodules" would not work and tests introduced
here would fail.
Signed-off-by: Ronald Weiss
---
Documentation/git-commit.txt| 7
builtin/commit.c| 8 +++-
t/t7513-commit-ignore-submodules.sh | 78 +
3 files changed, 92 insertions(+), 1 deletion(-)
create mode 100644 t/t7513-commit-ignore-submodules.sh
diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index 0bbc8f5..de0e8fe 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt
@@ -13,6 +13,7 @@ SYNOPSIS
[-F | -m ] [--reset-author] [--allow-empty]
[--allow-empty-message] [--no-verify] [-e] [--author=]
[--date=] [--cleanup=] [--[no-]status]
+ [--ignore-submodules[=]]
[-i | -o] [-S[]] [--] [...]
DESCRIPTION
@@ -277,6 +278,12 @@ The possible options are:
The default can be changed using the status.showUntrackedFiles
configuration variable documented in linkgit:git-config[1].
+--ignore-submodules[=]::
+ Can be used to override any settings of the 'submodule.*.ignore'
+ option in linkgit:git-config[1] or linkgit:gitmodules[5].
+can be either "none", "dirty, "untracked" or "all", which
+ is the default.
+
-v::
--verbose::
Show unified diff between the HEAD commit and what
diff --git a/builtin/commit.c b/builtin/commit.c
index a148e28..8c4d05e 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -361,7 +361,7 @@ static char *prepare_index(int argc, const char **argv,
const char *prefix,
*/
if (all || (also && pathspec.nr)) {
fd = hold_locked_index(&index_lock, 1);
- add_files_to_cache(also ? prefix : NULL, &pathspec, 0, NULL);
+ add_files_to_cache(also ? prefix : NULL, &pathspec, 0,
ignore_submodule_arg);
refresh_cache_or_die(refresh_flags);
update_main_cache_tree(WRITE_TREE_SILENT);
if (write_cache(fd, active_cache, active_nr) ||
@@ -1526,6 +1526,9 @@ int cmd_commit(int argc, const char **argv, const char
*prefix)
OPT_BOOL(0, "amend", &amend, N_("amend previous commit")),
OPT_BOOL(0, "no-post-rewrite", &no_post_rewrite, N_("bypass
post-rewrite hook")),
{ OPTION_STRING, 'u', "untracked-files", &untracked_files_arg,
N_("mode"), N_("show untracked files, optional modes: all, normal, no.
(Default: all)"), PARSE_OPT_OPTARG, NULL, (intptr_t)"all" },
+ { OPTION_STRING, 0, "ignore-submodules", &ignore_submodule_arg,
N_("when"),
+ N_("ignore changes to submodules, optional when: all, none.
(Default: all)"),
+ PARSE_OPT_OPTARG, NULL, (intptr_t)"all" },
/* end commit contents options */
OPT_HIDDEN_BOOL(0, "allow-empty", &allow_empty,
@@ -1564,6 +1567,9 @@ int cmd_commit(int argc, const char **argv, const char
*prefix)
argc = parse_and_validate_options(argc, argv, builtin_commit_options,
builtin_commit_usage,
prefix, current_head, &s);
+
+ s.ignore_submodule_arg = ignore_submodule_arg;
+
if (dry_run)
return dry_run_commit(argc, argv, prefix, current_head, &s);
index_file = prepare_index(argc, argv, prefix, current_head, 0);
diff --git a/t/t7513-commit-ignore-submodules.sh
b/t/t7513-commit-ignore-submodules.sh
new file mode 100644
index 000..52ab37d
--- /dev/null
+++ b/t/t7513-commit-ignore-submodules.sh
@@ -0,0 +1,78 @@
+#!/bin/sh
+#
+# Copyright (c) 2014 Ronald Weiss
+#
+
+test_description='Test of git commit --ignore-submodules'
+
+. ./test-lib.sh
+
+test_expect_success 'create submodule' '
+ test_create_repo sm && (
+ cd sm &&
+ >foo &&
+ git add foo &&
+ git commit -m "Add foo"
+ ) &&
+ git submodule add ./sm &&
+ git commit -m "Add sm"
+'
+
+update_sm () {
+ (cd sm &&
+ echo bar >> foo &&
+ git add foo &&
+ git commit -m "Updated foo"
+ )
+}
+
+test_expect_success 'commit -a --ignore-submodules=all ignores dirty
submodule' '
+ update_sm &&
+ test_must_fail git commit -a --ignore-submodules=all -m "Update sm"
+'
+
+test_expect_success 'commit -a --ignore-submodules=none overrides ignore=all
setting' '
+ update_sm &&
+ git config submodule.sm.ignore all &&
+ git commit -a --ignore-submodules=none -m "Update sm" &&
+ git diff --exit-code --ignore-submodules=none &&
+
[PATCH 2/2] Makefile: allow static linking against libcurl
Signed-off-by: Dave Borowitz --- Makefile | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index d6330bc..3c151d3 100644 --- a/Makefile +++ b/Makefile @@ -37,6 +37,9 @@ all:: # Define CURL_CONFIG to the path to a curl-config binary other than the # default 'curl-config'. # +# Define CURL_STATIC to statically link libcurl. Only applies if +# CURL_CONFIG is used. +# # Define CURLDIR=/foo/bar if your curl header and library files are in # /foo/bar/include and /foo/bar/lib directories. This overrides CURL_CONFIG, # but is less robust. @@ -1139,7 +1142,11 @@ else else CURL_CONFIG ?= curl-config BASIC_CFLAGS += $(shell $(CURL_CONFIG) --cflags) - CURL_LIBCURL = $(shell $(CURL_CONFIG) --libs) + ifdef CURL_STATIC + CURL_LIBCURL = $(shell $(CURL_CONFIG) --static-libs) + else + CURL_LIBCURL = $(shell $(CURL_CONFIG) --libs) + endif ifeq "$(CURL_LIBCURL)" "" $(error curl not detected; try setting CURLDIR) endif -- 1.9.1.423.g4596e3a -- 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 1/2] Makefile: use curl-config to determine curl flags
curl-config should always be installed alongside a curl distribution, and its purpose is to provide flags for building against libcurl, so use it instead of guessing flags and dependent libraries. Allow overriding CURL_CONFIG to a custom path to curl-config, to compile against a curl installation other than the first in PATH. Use this only when CURLDIR is not explicitly specified, to continue supporting older builds. Signed-off-by: Dave Borowitz --- Makefile | 35 +++ 1 file changed, 23 insertions(+), 12 deletions(-) diff --git a/Makefile b/Makefile index 2128ce3..d6330bc 100644 --- a/Makefile +++ b/Makefile @@ -34,8 +34,12 @@ all:: # git-http-push are not built, and you cannot use http:// and https:// # transports (neither smart nor dumb). # +# Define CURL_CONFIG to the path to a curl-config binary other than the +# default 'curl-config'. +# # Define CURLDIR=/foo/bar if your curl header and library files are in -# /foo/bar/include and /foo/bar/lib directories. +# /foo/bar/include and /foo/bar/lib directories. This overrides CURL_CONFIG, +# but is less robust. # # Define NO_EXPAT if you do not have expat installed. git-http-push is # not built, and you cannot push using http:// and https:// transports (dumb). @@ -143,9 +147,11 @@ all:: # # Define NEEDS_SSL_WITH_CRYPTO if you need -lssl when using -lcrypto (Darwin). # -# Define NEEDS_SSL_WITH_CURL if you need -lssl with -lcurl (Minix). +# Define NEEDS_SSL_WITH_CURL if you need -lssl with -lcurl (Minix). Only used +# if CURLDIR is set. # -# Define NEEDS_IDN_WITH_CURL if you need -lidn when using -lcurl (Minix). +# Define NEEDS_IDN_WITH_CURL if you need -lidn when using -lcurl (Minix). Only +# used if CURLDIR is set. # # Define NEEDS_LIBICONV if linking with libc is not enough (Darwin). # @@ -1121,18 +1127,23 @@ else # Try "-Wl,-rpath=$(CURLDIR)/$(lib)" in such a case. BASIC_CFLAGS += -I$(CURLDIR)/include CURL_LIBCURL = -L$(CURLDIR)/$(lib) $(CC_LD_DYNPATH)$(CURLDIR)/$(lib) -lcurl + ifdef NEEDS_SSL_WITH_CURL + CURL_LIBCURL += -lssl + ifdef NEEDS_CRYPTO_WITH_SSL + CURL_LIBCURL += -lcrypto + endif + endif + ifdef NEEDS_IDN_WITH_CURL + CURL_LIBCURL += -lidn + endif else - CURL_LIBCURL = -lcurl - endif - ifdef NEEDS_SSL_WITH_CURL - CURL_LIBCURL += -lssl - ifdef NEEDS_CRYPTO_WITH_SSL - CURL_LIBCURL += -lcrypto + CURL_CONFIG ?= curl-config + BASIC_CFLAGS += $(shell $(CURL_CONFIG) --cflags) + CURL_LIBCURL = $(shell $(CURL_CONFIG) --libs) + ifeq "$(CURL_LIBCURL)" "" + $(error curl not detected; try setting CURLDIR) endif endif - ifdef NEEDS_IDN_WITH_CURL - CURL_LIBCURL += -lidn - endif REMOTE_CURL_PRIMARY = git-remote-http$X REMOTE_CURL_ALIASES = git-remote-https$X git-remote-ftp$X git-remote-ftps$X -- 1.9.1.423.g4596e3a -- 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] commit: add --ignore-submodules[=] parameter
On 8. 4. 2014 20:26, Jens Lehmann wrote:
> Am 07.04.2014 23:46, schrieb Ronald Weiss:
>> Then, on top of that, I'll prepare patches for add to honor ignore
>> from .gitmodules, and -f implying --ignore-submodules. That might need
>> more discussion, let's see.
>
> Makes sense.
I thought more about that, and also played with the code a bit.
First, I was confused when I wrote that git add doesn't honor
submodules' ignore setting only from .gitmodules, but it does from
.git/config. It doesn't, from neither. Sorry for the confusion. However,
that doesn't change anything on the fact that it would be nice if add
would honor the ignore setting, from both places.
Second, there are some differences between adding standard ignored
files, and ignored submodules:
1) Already tracked files are never ignored, regardless of .gitignore.
However, tracked submodules should be ignored by "add -u", if told so
by their ignore setting.
2) So .gitignore seems to only do something when adding new files to
the repo. However, when adding new submodules, they are probably never
ignored (setting the ignore setting for non existent submodule seems
like non-sense, although possible).
3) Ignored files can be ignored less explicitely (in global gitignore,
or using a wildcard, or by ignoring parent folder). So it makes sense
to warn the user if he tries to explicitely add an ignored file, as he
might not be aware that the file is ignored. Submodules, however, can
only be ignored explicitely. And when user explicitely specifies the
submodule in an add command, he most probably really wants to add it,
so I don't see the point in warning him and requiring the -f option.
So, I think that the use cases are completely different, for submodules
and ignored files. So trying to make add behave the same for both, might
not be that good idea.
I would propose - let's make add honor the ignore setting by just
parsing if from config like the other commands do, and pass it to
underlying diff invocations. And at the same the, let's override it for
submodules explicitely specified on the command line, to never ignore
such submodules, without requiring the -f option. That seems to be
pretty easy, see below.
diff --git a/builtin/add.c b/builtin/add.c
index 85f2110..f19e6c8 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -284,6 +284,10 @@ static int add_config(const char *var, const char *value,
void *cb)
ignore_add_errors = git_config_bool(var, value);
return 0;
}
+
+ if (starts_with(var, "submodule."))
+ return parse_submodule_config_option(var, value);
+
return git_default_config(var, value, cb);
}
@@ -320,6 +324,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
char *seen = NULL;
git_config(add_config, NULL);
+ gitmodules_config();
argc = parse_options(argc, argv, prefix, builtin_add_options,
builtin_add_usage, PARSE_OPT_KEEP_ARGV0);
@@ -425,6 +430,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
PATHSPEC_EXCLUDE);
for (i = 0; i < pathspec.nr; i++) {
+ int cachepos;
const char *path = pathspec.items[i].match;
if (pathspec.items[i].magic & PATHSPEC_EXCLUDE)
continue;
@@ -440,6 +446,18 @@ int cmd_add(int argc, const char **argv, const char
*prefix)
die(_("pathspec '%s' did not match any
files"),
pathspec.items[i].original);
}
+
+ /* disable ignore setting for any submodules specified
explicitly in the pathspec */
+ if (path[0] &&
+ (cachepos = cache_name_pos(path,
pathspec.items[i].len)) >= 0 &&
+ S_ISGITLINK(active_cache[cachepos]->ce_mode)) {
+ char *optname;
+ int optnamelen = pathspec.items[i].len + 17;
+ optname = xcalloc(optnamelen + 1, 1);
+ snprintf(optname, optnamelen + 1,
"submodule.%s.ignore", path);
+ parse_submodule_config_option(optname, "none");
+ free(optname);
+ }
}
free(seen);
}
--
--
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 1/3] rebase: avoid non-function use of "return" on FreeBSD
On Apr 12, 2014, at 10:07, Matthieu Moy wrote:
> "Kyle J. McKay" writes:
>
>> On Apr 11, 2014, at 10:30, Matthieu Moy wrote:
>>> "Kyle J. McKay" writes:
>>>
There are already nested functions with file inclusion between both
levels of nesting in git-rebase--interactive.sh and git-rebase--
merge.sh now, so it's not introducing anything new.
>>>
>>> OK, so it's less serious than I thought. But still, we're
>>> introducing a
>>> function with 3 levels of nesting, split accross files, in an area
>>> where
>>> we know that at least one shell is buggy ...
>>
>> Currently in maint:
>>
>> The current code in maint does this:
>>
>> git-rebase.sh: top-level
>> git-rebase.sh: run_specific_rebase()
>>git-rebase.sh: run_specific_rebase_internal() -- contains "dot"
>> git-rebase--interactive.sh: top-level (using --continue or --
>> skip)
>>git-rebase--interactive.sh: do_rest
>> git-rebase--interactive.sh: do_next
>
> You're confusing function calls and function nesting. do_rest calls
> do_next, but the definition of do_next is not nested within do_rest.
>
> When I talk about nested function, I mean
>
> f() {
> g() {
> ...
> }
> }
>
> Obviously, having functions call each other is not an issue. That's
> what
> functions are meant to be.
>
> Now, having run_specific_rebase_internal include a file which defines
> functions which contain nested functions _is_ something I find
> weird. It
> both stresses the shell in a buggy area and makes the code harder to
> understand.
I meant: "nested functions" = "nested function calls"
You meant: "nested functions" = "nested function definitions"
Okay. But nested function definitions is not something new to the
rebase code.
>> The problem with these changes, particularly the git-rebase--
>> interactive.sh one is that a bunch of code is still run when the file
>> is "dot" included.
>
> Function definitions, and variables assignments. Is it so much of an
> issue?
>
> What's the difference between a function definition or variable
> assignment within git-rebase--*.sh and within git-rebase.sh?
As I said, this is the issue:
On Apr 11, 2014, at 16:08, Kyle J. McKay wrote:
> The problem with these changes, particularly the git-rebase--
> interactive.sh one is that a bunch of code is still run when the
> file is "dot" included. With the changes to git-rebase.sh, that
> code will now run regardless of the action and it will run before it
> would have now. So if any of the variables it sets affect the
> functions like read_basic_state or finish_rebase (they don't
> currently appear to), then there's a potential for new bugs. That
> initial code would not previously have run in the --abort case at all.
Let me rephrase.
Patch 1/3 does not change the order in which individual statements are
executed in the rebase code. Nor does it change the logic. It simply
introduces one additional function callstack level, but the same
individual statements are executed in the same order for all control
flows. No additional statements other than the introduced callstack
level. Nothing's executed in a different order. No control paths
execute additional statements they did not before.
The changes you propose introduce exactly the same additional function
callstack level. Then they proceed to alter the order in which
statements are executed. Statements that did not execute in some
control paths before are now executed in those control paths. Other
statements are moved around to execute earlier/later than they did
before.
My point is not that this is wrong. It's nice, really, to move the
"dot" include to the top level. The point is that's not necessary to
fix the problem and moving statements around and adding statements to
some control paths increases the risk of introducing an uncaught bug
when it's not necessary to fix the problem.
I would like to get a fix in so that rebase works out-of-the-box when
Git version 1.8.4 or later is built on FreeBSD.
So I'm not going to belabor the point anymore.
There's a follow-up patch 4/3 attached to the end of this message that
makes the changes you have proposed in your earlier email on top of
patches 1/3 and 2/3. The log message and all changes are taken from
your emails and so this patch is assigned to you and has a
'Needs-Signed-off-by:' line.
On Apr 11, 2014, at 10:30, Matthieu Moy wrote:
> The real patch is a bit more tricky though, because we need to run the
> ". git-rebase--$type" after computing type properly. A patch passing
> the
> tests but requiring cleanup is given below.
Unfortunately, after applying the below patch, some of the rebase
tests (3403, 3407, 3418, 3420, 3421) start failing for me on systems
where they did not fail previously. Even though some of the previously
failing tests on FreeBSD now pass with the patch, 3421 now fails on
FreeBSD where it did not before.
Here's a test summary of t
