* Re: Working with remotes with (too) many branches
From: Junio C Hamano @ 2012-02-21 22:47 UTC (permalink / raw)
To: Philip Jägenstedt; +Cc: Junio C Hamano, git, Jonathan Nieder
In-Reply-To: <CAKHWUkY_bEtKoPOiWFz6i2bp-tbekapROOaCYR0k5Ymh0HYMdA@mail.gmail.com>
Philip Jägenstedt <philip@foolip.org> writes:
> This looks a bit odd to me, shouldn't the new setting file under
> [remote "origin"]?
Yeah, remote.$name.fetchDefault or something.
> Thanks for the tip. (I did look at the original commit and search the
> archives, but found no answers.)
Heh, it does say --delete # not implemented ;-)
^ permalink raw reply
* Re: how do you review auto-resolved files
From: Neal Kreitzinger @ 2012-02-21 23:22 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Neal Kreitzinger, git
In-Reply-To: <7vhayjga0a.fsf@alter.siamese.dyndns.org>
On 2/21/2012 3:19 PM, Junio C Hamano wrote:
> "Neal Kreitzinger"<neal@rsss.com> writes:
>
>> When git does a merges (merge/rebase/cherry-pick) it auto-resolves same-file
>> changes that do not conflict on the same line(s).
>>
>> Technical Question: What are the recommended commands for reviewing the
>> files that auto-resolved after a "merge"?
>
> Imagine that you are the maintainer of the mainline and are reviewing the
> work made on a side branch that you just merged, but pretend that the
> contribution came as a patch instead. How would you assess the damage to
> your mainline?
>
> You would use "git show --first-parent $commit" for that.
>
> And then look at what the sideline wanted to do to the old baseline:
>
> git log -p $commit^..$commit
>
> which would, unless the person who worked on the side branch did a shoddy
> job describing his work, explain what the side branch wanted to achieve
> and also _how_ it wanted to achieve it.
>
> And then re-read the first "git show" output with that knowledge, together
> with the knowledge you have on your mainline codebase, and decide if the
> solution used by the side branch is still valid. If it makes sense, you
> are done. If the advance in your mainline since the side branch forked
> invalidated some assumption the side branch made (e.g. a helper function
> the side branch used has changed its meaning, a helper function the side
> branch changed its meaning gained more callsite on the mainline, etc.),
> you have a semantic conflict that you would need to address.
>
> It is unclear what exactly you consider "auto-resolve" in your message, so
> I'd refrain from commenting on the "Philosophical" part, at least for now.
Context: (git-merge manpage definition of merge-conflict) "During a
merge, the working tree files are updated to reflect the result of the
merge... When both sides made changes to the same area, however, git
cannot randomly pick one side over the other, and asks you to resolve it
by leaving what both sides did to that area."
My definition for "auto-resolve": "During a merge, the working tree
files are updated to reflect the result of the merge... When both sides
made changes to different areas of the same file, git picks both sides
automatically, and leaves its up to you to make sure you review those
merge results for correctness after git has made the merge commit."
IOW, an "auto-resolve" specifically means that both sides (ours and
theirs) made changes to file(a) since the common-ancestor version of
fila(a), and git picked both sides without raising a merge-conflict.
(The reason I came up with the term "auto-resolve" is because in the
git-merge output the term "Auto-merging" can also indicate that only one
side (theirs) changed file(a) since the common-ancestor and that git is
just "fast-forwarding" theirs file(a) on top of common-ancestor file(a).)
v/r,
neal
^ permalink raw reply
* Re: [RFC] pre-rebase: Refuse to rewrite commits that are reachable from upstream
From: Johan Herland @ 2012-02-21 23:23 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, jnareb, philipoakley
In-Reply-To: <7vehtoiqae.fsf@alter.siamese.dyndns.org>
On Tue, Feb 21, 2012 at 08:44, Junio C Hamano <gitster@pobox.com> wrote:
> Johan Herland <johan@herland.net> writes:
>
>> What you are describing here may be a common workflow, but
>> "rebase.bottomLimit" is still very specific to that kind of workflow.
>> What I'm after is a much more workflow-agnostic concept of:
>>
>> "If I have pushed something, I should probably not rebase it"
>
> Your "this branch pushes directly to that remote branch, so I can check if
> it will result in rewrite of published commit" is even *less* generic than
> having a single bottomLimit in my illustration.
>
> I may not push out my topic branches directly, only the aggregate of them
> in 'next', but once 'next' is pushed out, they are not eligible for
> rebasing. A per-branch bottom, e.g. rebase.$branch.bottomLimit, might
> make it more flexible to cover such a case, though.
But you would have to maintain rebase.$branch.bottomLimit separately
for each topic branch, updating it whenever the topic branch is merged
to 'next'. Is that practical?
> On the other hand, without any such safety, a merge to 'next' would give
> many conflicts and "shortlog master..next" will show many duplicates after
> any topic that are already merged to it are accidentally rewritten, and it
> is just the matter of using reflog on topic branches to recover from such
> a mistake.
All true, but my experience is that novice users will not enjoy this
experience, and rather become frustrated with Git for "putting them in
this situation" (i.e. giving them more rope than they can handle).
Maybe I'm doing something wrong...
>>> I wonder if it would be a more direct solution to the issue you are
>>> raising to give them a good tool to help them to be more careful with less
>>> effort on their part before they publish (not before they rebase).
>>
>> ..., I'm not sure how we can help the user _not_ publish the
>> branch until it's ready.
>
> I think we are in agreement that we do not think of a good solution
> offhand to the real cause of the issue, except by encouraging the use of
> throw-away review branches, perhaps.
Agreed. We should probably focus more on this when teaching novice users.
>> I think the following decribes what often happens for many users:
>>
>> 1. User A pushes the branch to a public repo.
>> 2. User B points out a simple mistake in the branch.
>
> That's the CVS workflow, and it is not "a" public repo but "the" public
> repo shared between A and B (and also with all the project participants).
>
>> 3. User A makes a fix
>> 4. User A squashes the fix into the (already-published) history.
>> 5. User A attempts to push the "fixed" history (but is rejected by
>> the public repo because of non-fast-forward).
>> At this point, the damage is already done,...
>
> Which is probably a sufficient safety which the user can learn from. If
> this happens too often, that probably means we are not helping them enough
> to learn not to "commit --amend" or "rebase" if they are using Git as a
> better CVS.
>
>> You could say that User A should be more careful and push to a "less
>> public" repo in step #1 (thus allowing the fix to be squashed before
>> pushing to a "more public" repo in step #5),...
>
> That is essentially a workflow that uses throw-away review branches in a
> distributed environment, and at that point, we are not constrained by the
> limitation of the CVS workflow. While still in early review cycles (which
> corresponds to being in our 'pu'), "commit --amend" and "rebase" are fine
> tool to be used. And...
>
>> but how "public" is
>> "public" enough to have someone point out the bug, but still
>> "unpublic" enough to allow rebase?
>
> ... I can imagine that currently that is determined purely by project
> convention. Perhaps there needs a way to mark throw-away review branches
> like 'pu' (or saying the same thing from the different perspective, to
> mark cast-in-stone integration branches like 'next') so that tools can
> mechanically decide what should and should not be rewritten.
Yes. Maybe it should be a tri-state thing:
- "rebase-prone" will allow rewrites without asking.
- unset will warn, but still allow rewrites. The warning should
explain the potential problem that may arise from rewriting, and
should also explain how to get rid of the warning by setting this
config to either of the above/below options. Ideally the warning
should also explain how the user may undo the rewrite. Obviously, most
of this warning should be controlled by a corresponding advice.*
config.
- "cast-in-stone" will refuse rewrites.
I notice that this config is not really per-branch, but rather per
upstream branch. I wonder how to best encode that in the config...
Also, how a remote repo may best communicate which branches are
"rebase-prone"/"cast-in-stone".
> To extend the idea of promoting throw-away review branches further,
> perhaps it might help if there is an easy way to let the users publish
> their "master" to a branch that is not the "master" of the central shared
> repository even in the CVS workflow (e.g. by default a push from user A
> always goes to refs/review/A/master), and to have an option to "git push"
> that makes it go to the "master" when the user really means the branch is
> ready (and it would move refs/review/A/master to attic to be later gc'ed).
This goes against many workflows, and many users' expectations.
Although it may be a Good(tm) practice, I don't think it's universal
enough to be worth "breaking" 'push'. What about adding a new 'git
review' command which defaults to this behavior (but can be overridden
by Gerrit and other code-review systems to do what's appropriate in
their cases)?
>> ... And I
>> think that refusing rewrites of commits that are already present in
>> the @{upstream} remote-tracking branch is good enough to help most
>> users avoid steps #4 through #6 (in a push-based workflow[1]).
>
> See above regarding branches that should not be rebased even if they are
> not directly pushed out.
True. Is it universally acceptable to assume that a topic branch that
has been merged to a "cast-in-stone" branch, should not be
rebased/rewritten (unless forced by the user)?
There are several aspects to this: First, any commits on the topic
branch that have been made _since_ the merge should obviously be
rewritable (since they have yet to be merged). And commits made
_before_ the merge should probably not be rewritable. I.e. in the
following figure, the 'y' commits should be rewritable, but the 'x'
commits (preceding the merge 'M') should probably not.
--o---o---o---o---o---o---M <- master (cast-in-stone)
\ /
x---x---x---x---x---x---y---y <- topicA
But what if we have two topic branches with shared history, like this:
--o---o---o---o---o---o---M <- master (cast-in-stone)
\ /
x---x---x---x---x---x---y---y <- topicA
\
z---z <- topicB
We cannot say whether the 'x' commits "belong" to topicA or topicB.
Also, we cannot determine whether the merge M happened with topicA or
topicB (unless the default merge message has been preserved). Now,
given that we try to rewrite the entire topicB branch (including the
'x' commits): Should we refuse rewriting the 'x' commits because they
are reachable from master (regardless of whether topicA or topicB was
merged), or should we allow rewriting the 'x' commits on the basis
that topicA may be the branch that was merged in 'M', and since topicB
are "unaware" that the 'x' commits have been merged, it should thus be
allowed to rewrite them?
Now, what if you want to backport 'topicA' from 'master' to 'maint'.
You could do so like this:
git checkout -b topicA_maint topicA
git rebase --onto maint master
(note that topicA_maint is a special case of topicB in the above
graph) In this case, we would expect the 'x' commits to be rewritable,
or we could not perform the backport.
The conclusion seems to be that we _cannot_ refuse rewriting commits
merely on the basis that they have been merged to a cast-in-stone
branch.
>> In
>> fact, from a pedagogical POV, I think step #4 is probably the best
>> spot for novice users to learn exactly the distinction between
>> acceptable and unacceptable history rewrites (instead of having it
>> explained to them as a consequence of the step #5).
>
> I doubt you have enough information at point #4, unless you restrict the
> workflow you allow your novice users to use fairly severely, to give
> appropriate advice. While I agree with you that it would be the best if
> we could do so at step #4 without stopping the user from doing what s/he
> needs to do with false positive, I think it is not pedagogical POV but
> dreaming if the world were ideal without knowing what it would take to
> make it ideal.
>
> At least I don't know offhand what kind of changes are needed to restrict
> the user actions to an "approved" workflow so that step #4 can make a
> useful decision (that is, no false positives and small enough false
> negatives).
Ok, trying to make as few assumptions about user workflows as
possible: Assuming there were a (machine-parsable) way to mark
branches as either "rebase-prone" or "cast-in-stone". Could we then
assume that rebasing commits that exist on a cast-in-stone branch's
remote-tracking @{upstream} should be refused by default? I'm trying
(without much success, it seems) to find _something_ that will help
the 1525 users that want Git to "warn before/when rewriting published
history"[1], but won't cause any false positives (and small enough
false negatives). If there really is no way to implement this, then we
shouldn't give users false hopes by putting it in the survey...
Have fun! :)
...Johan
[1]: Question 17 in https://www.survs.com/results/Q5CA9SKQ/P7DE07F0PL
--
Johan Herland, <johan@herland.net>
www.herland.net
^ permalink raw reply
* Re: [RFC] pre-rebase: Refuse to rewrite commits that are reachable from upstream
From: Junio C Hamano @ 2012-02-21 23:43 UTC (permalink / raw)
To: Johan Herland; +Cc: Junio C Hamano, git, jnareb, philipoakley
In-Reply-To: <CALKQrgfLBKG5ssL8ua_EtfyQeOp0h9HPC7vdOM_Yc_AVaa39xw@mail.gmail.com>
Johan Herland <johan@herland.net> writes:
> history"[1], but won't cause any false positives (and small enough
> false negatives). If there really is no way to implement this, then we
> shouldn't give users false hopes by putting it in the survey...
I think that question should be "warn before pushing out a commit that the
user may later regret to have pushed out" ;-)
^ permalink raw reply
* Re: how do you review auto-resolved files
From: Junio C Hamano @ 2012-02-21 23:52 UTC (permalink / raw)
To: Neal Kreitzinger; +Cc: git
In-Reply-To: <ji0vik$e48$1@dough.gmane.org>
"Neal Kreitzinger" <neal@rsss.com> writes:
> If I expect/assume you are going to review the
> auto-resolutions after-the-fact then you can neglect this because you:
>
> - have become complacent that git usually does-what-you-want so "you don't
> really need to do it",
> - are lazy and do it half-way,
> - forget to do it,
> - think "git magically does your work for you",
> - don't know how to do it,
> - don't even realize that anything auto-resolved or what auto-resolved,
> - decide you don't have to do it because that is what testing if for,
> - you think that your time is so valuable that an ounce-of-prevention on
> your part is not worth a pound-of-cure on the part of others.
A couple more bullet points I can think of off the top of my head, after
making sure that you do not count what "rerere" does as part of the
"auto-resolution", to add to the above list are:
- know git is stupid and errs on the safe side, punting anything remotely
complex;
- know that textual non-conflicts that occur in the same file have the
same risk of having semantic conflict across different files, so
singling out "touched the same file but did not conflict" any special
is pointless, but in either case, the chance of having such a conflict
is small enough that completing the merge (and other merges) first and
then checking the overall result is more efficient use of your time,
because you have to eyeball the result at least once anyway before
pushing it out.
^ permalink raw reply
* Re: [RFC] pre-rebase: Refuse to rewrite commits that are reachable from upstream
From: Dave Zarzycki @ 2012-02-21 23:59 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johan Herland, git, jnareb, philipoakley
In-Reply-To: <7vsji3eoqs.fsf@alter.siamese.dyndns.org>
On Feb 21, 2012, at 6:43 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Johan Herland <johan@herland.net> writes:
>
>> history"[1], but won't cause any false positives (and small enough
>> false negatives). If there really is no way to implement this, then we
>> shouldn't give users false hopes by putting it in the survey...
>
> I think that question should be "warn before pushing out a commit that the
> user may later regret to have pushed out" ;-)
Why limit this proposal to just the commits that are reachable from upstream? What if somebody pulls from your repo?
In other words, wouldn't it be better to have a git track "unshared" commits and only let those be rewritten? The theory being that if the given commits haven't been pushed or pulled anywhere, then they are safe to rewrite.
davez
^ permalink raw reply
* Re: [PATCH] completion: remote set-* <name> and <branch>
From: SZEDER Gábor @ 2012-02-22 0:03 UTC (permalink / raw)
To: Junio C Hamano
Cc: Philip Jägenstedt, git, SZEDER Gábor, Felipe Contreras,
Teemu Likonen
In-Reply-To: <7v8vjvg71s.fsf@alter.siamese.dyndns.org>
On Tue, Feb 21, 2012 at 02:23:11PM -0800, Junio C Hamano wrote:
> >>> {
> >>> local cur_="$cur" cmd="${words[1]}"
> >>> local i c=2 remote="" pfx="" lhs=1 no_complete_refspec=0
> >>> + if [ "$cmd" = "remote" ]; then
> >>> + c=$((++c))
> >>> + fi
> >>
> >> I don't know about others, but auto-incrementing a variable and assigning
> >> the result to the same variable, while not wrong at all, hurts my brain.
> >>
> >> c=$(($c + 1))
> >>
> >> is far more readable and does not suggest there is any funky magic
> >> involved. Also it is a good habit to get into not to omit $ from
> >> variables inside arithmetic substitution, even though bash allows it and
> >> this script is meant to be consumed only by shells that understand this
> >> bash-ism.
> >>
> >> I do not know offhand if zsh groks it, but the point is that you do not
> >> have to worry about it if you write "$(($c+1))" instead of "$((c+1))".
This c=$((++c)) style increment is used in several completion
functions, even in the main entry function _git() itself. If zsh
didn't grok it, then completion wouldn't work at all on zsh, because
the while loop in _git() would be an endless loop.
> > CodingGuidlines suggests to follow local convention, which was
> > "c=$((++c))". This file also uses "++n", "i++" and "((i++))". I will
> > send a v2 patch that normalizes these, open to discussion of course.
>
> OK, it was my mistake to suggest $c++ in this file; it liberally uses
> dollar-less variables, and I agree that it is a good idea to stick to that
> local convention.
>
> But I think you went too far in your follow-up patch to make the increment
> and decrement uniform.
>
> "i++" is so much easier on the eye unless you must use "++i" in order to
> use the value of the incremented "i" in an expression, and the changes to
> turn existing instances of free-standing "i++" to "++i" done only for the
> side effect of incrementing the variables look totally backwards.
>
> Although I do not deeply care. Just leaving the new one as you originally
> wrote, i.e.
>
> c=$((++c))
>
> would have been easier to review for the area experts, I would think.
The issue of this weird increment came up before
http://thread.gmane.org/gmane.comp.version-control.git/87650/focus=87806
and the conclusion back then was that we kept the existing style. But
otherwise I agree, it hurts my brain, too.
Best,
Gábor
^ permalink raw reply
* Re: [PATCH v2] completion: remote set-* <name> and <branch>
From: SZEDER Gábor @ 2012-02-22 0:17 UTC (permalink / raw)
To: Philip Jägenstedt; +Cc: git, Felipe Contreras, Teemu Likonen
In-Reply-To: <1329861274-21418-1-git-send-email-philip@foolip.org>
Hi,
On Tue, Feb 21, 2012 at 10:54:34PM +0100, Philip Jägenstedt wrote:
> Complete <name> only for set-url. For set-branches and
> set-head, complete <name> and <branch> over the network,
> like e.g. git pull already does.
>
> The style used for incrementing and decrementing variables was fairly
> inconsistenty and was normalized to use ++x, or ((++x)) in contexts
> where the former would otherwise be interpreted as a command. This is a
> bash-ism, but for obvious reasons this script is already bash-specific.
>
> Finally, remove out-of-date documentation for how to submit patches and
> (silently) defer to Documentation/SubmittingPatches like all other code.
>
> Signed-off-by: Philip Jägenstedt <philip@foolip.org>
> ---
> contrib/completion/git-completion.bash | 42 +++++++++++++------------------
> 1 files changed, 18 insertions(+), 24 deletions(-)
>
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index 1505cff..9dac084 100755
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -60,18 +60,6 @@
> # per-repository basis by setting the bash.showUpstream config
> # variable.
> #
> -#
> -# To submit patches:
> -#
> -# *) Read Documentation/SubmittingPatches
Um, well... Did you actually read it? I mean the part that talks
about making separate commits for logically separate changes? ;)
You clearly squashed three separate changes into a single patch.
Your proposed changes to __git_complete_remote_or_refspec() and
__git_remote() make sense, modulo the increment/decrement part. I
don't have strong preference either way, but please submit those
changes in separate patches. Otherwise if ever bisect points to this
commit during a hunt for a 'git remote' completion bug, we'll need to
think about whether the increment style change or the functional
change is causing the trouble.
Thanks,
Gábor
^ permalink raw reply
* Re: [PATCH] cherry-pick: No advice to commit if --no-commit
From: Phil Hord @ 2012-02-22 0:18 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Git List, Junio C Hamano, Ramkumar Ramachandra, Phil Hord
In-Reply-To: <20120221222049.GA31934@burratino>
Jonathan Nieder <jrnieder@gmail.com> wrote:
>> - if (show_hint)
>> + if (show_hint) {
>> advise(_("after resolving the conflicts, mark the corrected paths\n"
>> - "with 'git add <paths>' or 'git rm <paths>'\n"
>> - "and commit the result with 'git commit'"));
>> + "with 'git add <paths>' or 'git rm <paths>'"));
>> + if (!opts->no_commit)
>> + advise(_( "and commit the result with 'git commit'"));
>
> "cherry-pick --no-commit" was not about to commit, but the user might
> have been. I think the hint is intended to convey that authorship
> will be correctly preserved if the user continues with "git commit"
> and no special -c option is necessary.
If that were the case, the hint would also appear when there is no conflict.
> Could you say a little more about the motivation for this patch? For
> example, did the existing message confuse someone, or was it grating
> in the context of some particular workflow?
I found it mildly confusing myself. I cherry-picked a commit with
--no-commit with no intention of committing it. I was testing how the
changes would build, but I do not need them on my branch yet. After I
resolved the conflict and tested them, I wanted to make sure there was
no lingering effects, leaving git thinking a CP was still in progress.
$ git cherry-pick --abort
error: no cherry-pick or revert in progress
Ok, so the sequencer was smart enough to leave me on my own. But just
in case, I wondered what the hint was. And I found it was not telling me
how to clean up at all, but instead telling me how to commit. It seemed
incongruous, and I assumed it was only someone's forgetting to consider
the --no-commit case.
It smelled like a bug. I started to ask about it, but it seemed easier
to just correct it and lower the list noise.
> A smaller detail: splitting the message into two like this gives
> translators less control over how to phrase the message and where to
> wrap lines. Luckily that is easy to fix with
>
> if (opts->no_commit)
> advise(...);
> else
> advise(...);
>
> which means more flexibility in phrasing the message with pertinent
> advice for each case. ;-)
I did this at first and didn't like it. I started to ask, but -- you
know, list noise.
I'll fix it in v2.
Thanks,
Phil
^ permalink raw reply
* Re: [PATCH] git-svn.perl: fix a false-positive in the "already exists" test
From: Eric Wong @ 2012-02-22 0:33 UTC (permalink / raw)
To: Steven Walter; +Cc: gitster, git
In-Reply-To: <1329747474-17976-1-git-send-email-stevenrwalter@gmail.com>
Steven Walter <stevenrwalter@gmail.com> wrote:
> open_or_add_dir checks to see if the directory already exists or not.
> If it already exists and is not a directory, then we fail. However,
> open_or_add_dir did not previously account for the possibility that the
> path did exist as a file, but is deleted in the current commit.
>
> In order to prevent this legitimate case from failing, open_or_add_dir
> needs to know what files are deleted in the current commit.
> Unfortunately that information has to be plumbed through a couple of
> layers.
>
> Signed-off-by: Steven Walter <stevenrwalter@gmail.com>
Thanks, will push.
Acked-by: Eric Wong <normalperson@yhbt.net>
^ permalink raw reply
* [PULL git-svn] various git svn updates
From: Eric Wong @ 2012-02-22 0:38 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Steven Walter, Steven Walter, Wei-Yin Chen,
Ævar Arnfjörð Bjarmason, Frederic Heitzmann
The following changes since commit b3a769dc355b32c95783dc07f59e4dfebdd8bdc7:
Update draft release notes to 1.7.10 (2012-02-20 00:29:40 -0800)
are available in the git repository at:
git://bogomips.org/git-svn master
for you to fetch changes up to 379862ec5a413e636d977a6ea3d618f0b3eafceb:
git-svn.perl: fix a false-positive in the "already exists" test (2012-02-21 21:37:31 +0000)
----------------------------------------------------------------
Frederic Heitzmann (1):
completion: add --interactive option to git svn dcommit
Steven Walter (2):
git-svn.perl: perform deletions before anything else
git-svn.perl: fix a false-positive in the "already exists" test
Wei-Yin Chen (陳威尹) (1):
git-svn: Fix time zone in --localtime
Ævar Arnfjörð Bjarmason (2):
git-svn: remove redundant porcelain option to rev-list
git-svn: un-break "git svn rebase" when log.abbrevCommit=true
contrib/completion/git-completion.bash | 2 +-
git-svn.perl | 68 +++++++++++++++++++-------------
t/t9100-git-svn-basic.sh | 33 +++++++++-------
3 files changed, 59 insertions(+), 44 deletions(-)
^ permalink raw reply
* [PATCH v2] cherry-pick: No advice to commit if --no-commit
From: Phil Hord @ 2012-02-22 0:44 UTC (permalink / raw)
To: Git List, Junio C Hamano, Ramkumar Ramachandra, Jonathan Nieder,
Phil Hord
Cc: Phil Hord
When cherry-pick fails it offers a helpful hint about how to
proceed. The hint tells the user how to do the cleanup
needed by the conflicted cherry-pick and finish the job after
conflict resolution. In case of cherry-pick --no-commit, the
hint goes too far. It tells the user to finish up with
'git commit'. That is not what this git-cherry-pick was
trying to do in the first place.
Restrict the hint in case of --no-commit to avoid giving this
extra advice.
Also, add a test verifying the reduced hint for the --no-commit
version of cherry-pick.
Signed-off-by: Phil Hord <hordp@cisco.com>
---
sequencer.c | 17 +++++++++++------
t/t3507-cherry-pick-conflict.sh | 14 ++++++++++++++
2 files changed, 25 insertions(+), 6 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index 5fcbcb8..9716c63 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -123,7 +123,7 @@ static void write_cherry_pick_head(struct commit *commit, const char *pseudoref)
strbuf_release(&buf);
}
-static void print_advice(int show_hint)
+static void print_advice(int show_hint, struct replay_opts *opts)
{
char *msg = getenv("GIT_CHERRY_PICK_HELP");
@@ -138,10 +138,15 @@ static void print_advice(int show_hint)
return;
}
- if (show_hint)
- advise(_("after resolving the conflicts, mark the corrected paths\n"
- "with 'git add <paths>' or 'git rm <paths>'\n"
- "and commit the result with 'git commit'"));
+ if (show_hint) {
+ if (opts->no_commit)
+ advise(_("after resolving the conflicts, mark the corrected paths\n"
+ "with 'git add <paths>' or 'git rm <paths>'"));
+ else
+ advise(_("after resolving the conflicts, mark the corrected paths\n"
+ "with 'git add <paths>' or 'git rm <paths>'\n"
+ "and commit the result with 'git commit'"));
+ }
}
static void write_message(struct strbuf *msgbuf, const char *filename)
@@ -423,7 +428,7 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
: _("could not apply %s... %s"),
find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV),
msg.subject);
- print_advice(res == 1);
+ print_advice(res == 1, opts);
rerere(opts->allow_rerere_auto);
} else {
if (!opts->no_commit)
diff --git a/t/t3507-cherry-pick-conflict.sh b/t/t3507-cherry-pick-conflict.sh
index ee1659c..0c81b3c 100755
--- a/t/t3507-cherry-pick-conflict.sh
+++ b/t/t3507-cherry-pick-conflict.sh
@@ -59,6 +59,20 @@ test_expect_success 'advice from failed cherry-pick' "
test_i18ncmp expected actual
"
+test_expect_success 'advice from failed cherry-pick --no-commit' "
+ pristine_detach initial &&
+
+ picked=\$(git rev-parse --short picked) &&
+ cat <<-EOF >expected &&
+ error: could not apply \$picked... picked
+ hint: after resolving the conflicts, mark the corrected paths
+ hint: with 'git add <paths>' or 'git rm <paths>'
+ EOF
+ test_must_fail git cherry-pick --no-commit picked 2>actual &&
+
+ test_i18ncmp expected actual
+"
+
test_expect_success 'failed cherry-pick sets CHERRY_PICK_HEAD' '
pristine_detach initial &&
test_must_fail git cherry-pick picked &&
--
1.7.9.267.gda172.dirty
^ permalink raw reply related
* git-p4 uses -h for host instead of -H
From: Russell Myers @ 2012-02-22 0:49 UTC (permalink / raw)
To: git
In-Reply-To: <CAA5tD2sYSqtGTwW1PmFMB_mP_xG24VS6hPXTLD33bJsMaj4MWg@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 425 bytes --]
In attempting to use the git-p4 plugin I ran into an issue when the host
argument was specified using the -h argument instead of the -H argument
for the host. As a result, I found that instead of git-p4 specifying a
host, it gets help information. Correcting this issue allowed me to
clone without issue. I've attached a patch of what I did to fix the
issue if this is indeed believed to be an issue.
Thanks,
Russell
[-- Attachment #2: host_flag_fix.patch --]
[-- Type: text/x-patch, Size: 884 bytes --]
>From 2f6c91282c98ca0a45269524ec74655f76921ec9 Mon Sep 17 00:00:00 2001
From: Russell Myers <mezner@russellmyers.com>
Date: Tue, 21 Feb 2012 19:18:54 -0500
Subject: [PATCH] Changing host argument to -H from -h. Based on
http://www.perforce.com/perforce/doc.current/manuals/p4guide/03_using.html
'-H' is the apporpriate flag while '-h' is a flag passed
for help content.
---
contrib/fast-import/git-p4 | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
index a78d9c5..d2fd265 100755
--- a/contrib/fast-import/git-p4
+++ b/contrib/fast-import/git-p4
@@ -38,7 +38,7 @@ def p4_build_cmd(cmd):
host = gitConfig("git-p4.host")
if len(host) > 0:
- real_cmd += ["-h", host]
+ real_cmd += ["-H", host]
client = gitConfig("git-p4.client")
if len(client) > 0:
--
1.7.5.4
^ permalink raw reply related
* Re: [PATCH v2] cherry-pick: No advice to commit if --no-commit
From: Jonathan Nieder @ 2012-02-22 0:51 UTC (permalink / raw)
To: Phil Hord; +Cc: Git List, Junio C Hamano, Ramkumar Ramachandra, Phil Hord
In-Reply-To: <1329871457-12890-1-git-send-email-hordp@cisco.com>
Phil Hord wrote:
> In case of cherry-pick --no-commit, the
> hint goes too far. It tells the user to finish up with
> 'git commit'. That is not what this git-cherry-pick was
> trying to do in the first place.
Especially since if I do try to commit as it says, it will not
reuse the old commit message and timestamp like the advice made me
suspect it would. :)
[...]
> --- a/t/t3507-cherry-pick-conflict.sh
> +++ b/t/t3507-cherry-pick-conflict.sh
> @@ -59,6 +59,20 @@ test_expect_success 'advice from failed cherry-pick' "
> test_i18ncmp expected actual
> "
>
> +test_expect_success 'advice from failed cherry-pick --no-commit' "
> + pristine_detach initial &&
> +
> + picked=\$(git rev-parse --short picked) &&
The escaping here is obnoxiously tricky. Not your fault, though.
For what it's worth,
Acked-by: Jonathan Nieder <jrnieder@gmail.com>
Thanks.
^ permalink raw reply
* Re: git-p4 uses -h for host instead of -H
From: Pete Wyckoff @ 2012-02-22 1:23 UTC (permalink / raw)
To: Russell Myers; +Cc: git, luke, ggibbons
In-Reply-To: <CAA5tD2sYSqtGTwW1PmFMB_mP_xG24VS6hPXTLD33bJsMaj4MWg@mail.gmail.com>
mezner@russellmyers.com wrote on Tue, 21 Feb 2012 19:35 -0500:
> In attempting to use the git-p4 plugin I ran into an issue when the host
> argument was specified using the -h argument instead of the -H argument for
> the host. As a result, I found that instead of git-p4 specifying a host, it
> gets help information. Correcting this issue allowed me to clone without
> issue. I've attached a patch of what I did to fix the issue if this is
> indeed believed to be an issue.
Ack to this. Looks like the bug has been in there since the
feature of git-p4.host was introduced in abcaf07 (If the user has
configured various parameters, use them., 2008-08-10).
Thanks for the fix.
-- Pete
> From 2f6c91282c98ca0a45269524ec74655f76921ec9 Mon Sep 17 00:00:00 2001
> From: Russell Myers <mezner@russellmyers.com>
> Date: Tue, 21 Feb 2012 19:18:54 -0500
> Subject: [PATCH] Changing host argument to -H from -h. Based on
> http://www.perforce.com/perforce/doc.current/manuals/p4guide/03_using.html
> '-H' is the apporpriate flag while '-h' is a flag passed
> for help content.
>
> ---
> contrib/fast-import/git-p4 | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
> index a78d9c5..d2fd265 100755
> --- a/contrib/fast-import/git-p4
> +++ b/contrib/fast-import/git-p4
> @@ -38,7 +38,7 @@ def p4_build_cmd(cmd):
>
> host = gitConfig("git-p4.host")
> if len(host) > 0:
> - real_cmd += ["-h", host]
> + real_cmd += ["-H", host]
>
> client = gitConfig("git-p4.client")
> if len(client) > 0:
> --
> 1.7.5.4
>
^ permalink raw reply
* [PATCHv3 0/5] Re: tag: make list exclude !<pattern>
From: Tom Grennan @ 2012-02-22 1:28 UTC (permalink / raw)
To: git; +Cc: gitster, peff, jasampler, pclouds
In-Reply-To: <20120211190856.GB4903@tgrennan-laptop>
It has taken me a while to get back to this; the following series rebase
and rework my proposed "!<pattern>" feature. The first patch extracts
match_pattern() from builtin/tag.c and updates this to replace similar
functions in builtin/{branch,for-each-ref,ls-remote}.c
The second patch uses the OPT_CALLBACK wrapper on the recently added
"--points-at" option of git-tag. I should have implemented it this way
to begin with but I missed Jeff King's suggestion within that thread.
The remaining patches add "--exclude <pattern>" options (vs.
"!<pattern>") to: git-tag, git-branch, and git-for-each-ref.
For example,
$ git tag -l --exclude "*-rc?" "v1.7.8*"
$ git branch -r --exclude \*HEAD
$ git for-each-ref --format="%(refname)" --exclude "*HEAD"
refs/remotes/origin
Instead of,
$ git tag -l "!*-rc?" "v1.7.8*"
$ git branch -r "!*HEAD"
$ git for-each-ref --format="%(refname)" "!*HEAD" refs/remotes/origin
Note that I haven't yet added an "--exclude" feature to git-ls-remote
because I think that I should first update its option parsing.
Thanks,
Tom Grennan (5):
refs: add match_pattern()
tag --points-at option wrapper
tag --exclude option
branch --exclude option
for-each-ref --exclude option
Documentation/git-branch.txt | 7 ++++-
Documentation/git-for-each-ref.txt | 7 ++++-
Documentation/git-tag.txt | 6 +++-
builtin/branch.c | 30 ++++++++-----------
builtin/for-each-ref.c | 27 +++++------------
builtin/ls-remote.c | 12 ++------
builtin/tag.c | 35 ++++++++++------------
refs.c | 36 +++++++++++++++++++++++
refs.h | 12 ++++++++
t/t3200-branch.sh | 23 +++++++++++++++
t/t6300-for-each-ref.sh | 11 +++++++
t/t7004-tag.sh | 56 ++++++++++++++++++++++++++++++++++++
12 files changed, 195 insertions(+), 67 deletions(-)
--
1.7.8
^ permalink raw reply
* [PATCHv3 1/5] refs: add match_pattern()
From: Tom Grennan @ 2012-02-22 1:28 UTC (permalink / raw)
To: git; +Cc: gitster, peff, jasampler, pclouds
In-Reply-To: <20120211190856.GB4903@tgrennan-laptop>
Used-by: git-branch, git-for-each-ref, git-ls-remote, and git-tag
Signed-off-by: Tom Grennan <tmgrennan@gmail.com>
---
builtin/branch.c | 16 ++--------------
builtin/for-each-ref.c | 21 ++-------------------
builtin/ls-remote.c | 12 +++---------
builtin/tag.c | 13 +------------
refs.c | 36 ++++++++++++++++++++++++++++++++++++
refs.h | 12 ++++++++++++
6 files changed, 56 insertions(+), 54 deletions(-)
diff --git a/builtin/branch.c b/builtin/branch.c
index cb17bc3..e46ed58 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -266,18 +266,6 @@ struct append_ref_cb {
int ret;
};
-static int match_patterns(const char **pattern, const char *refname)
-{
- if (!*pattern)
- return 1; /* no pattern always matches */
- while (*pattern) {
- if (!fnmatch(*pattern, refname, 0))
- return 1;
- pattern++;
- }
- return 0;
-}
-
static int append_ref(const char *refname, const unsigned char *sha1, int flags, void *cb_data)
{
struct append_ref_cb *cb = (struct append_ref_cb *)(cb_data);
@@ -312,7 +300,7 @@ static int append_ref(const char *refname, const unsigned char *sha1, int flags,
if ((kind & ref_list->kinds) == 0)
return 0;
- if (!match_patterns(cb->pattern, refname))
+ if (!match_pattern(refname, cb->pattern, NULL, 0))
return 0;
commit = NULL;
@@ -542,7 +530,7 @@ static int print_ref_list(int kinds, int detached, int verbose, int abbrev, stru
qsort(ref_list.list, ref_list.index, sizeof(struct ref_item), ref_cmp);
detached = (detached && (kinds & REF_LOCAL_BRANCH));
- if (detached && match_patterns(pattern, "HEAD"))
+ if (detached && match_pattern("HEAD", pattern, NULL, 0))
show_detached(&ref_list);
for (i = 0; i < ref_list.index; i++) {
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index b01d76a..bd6a114 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -781,25 +781,8 @@ static int grab_single_ref(const char *refname, const unsigned char *sha1, int f
struct refinfo *ref;
int cnt;
- if (*cb->grab_pattern) {
- const char **pattern;
- int namelen = strlen(refname);
- for (pattern = cb->grab_pattern; *pattern; pattern++) {
- const char *p = *pattern;
- int plen = strlen(p);
-
- if ((plen <= namelen) &&
- !strncmp(refname, p, plen) &&
- (refname[plen] == '\0' ||
- refname[plen] == '/' ||
- p[plen-1] == '/'))
- break;
- if (!fnmatch(p, refname, FNM_PATHNAME))
- break;
- }
- if (!*pattern)
- return 0;
- }
+ if (!match_pattern(refname, cb->grab_pattern, NULL, FNM_PATHNAME))
+ return 0;
/*
* We do not open the object yet; sort may only need refname
diff --git a/builtin/ls-remote.c b/builtin/ls-remote.c
index 41c88a9..29f2b38 100644
--- a/builtin/ls-remote.c
+++ b/builtin/ls-remote.c
@@ -2,6 +2,7 @@
#include "cache.h"
#include "transport.h"
#include "remote.h"
+#include "refs.h"
static const char ls_remote_usage[] =
"git ls-remote [--heads] [--tags] [-u <exec> | --upload-pack <exec>]\n"
@@ -13,19 +14,12 @@ static const char ls_remote_usage[] =
*/
static int tail_match(const char **pattern, const char *path)
{
- const char *p;
char pathbuf[PATH_MAX];
- if (!pattern)
- return 1; /* no restriction */
-
if (snprintf(pathbuf, sizeof(pathbuf), "/%s", path) > sizeof(pathbuf))
return error("insanely long ref %.*s...", 20, path);
- while ((p = *(pattern++)) != NULL) {
- if (!fnmatch(p, pathbuf, 0))
- return 1;
- }
- return 0;
+
+ return match_pattern(pathbuf, pattern, NULL, 0);
}
int cmd_ls_remote(int argc, const char **argv, const char *prefix)
diff --git a/builtin/tag.c b/builtin/tag.c
index fe7e5e5..9dcd7d2 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -34,17 +34,6 @@ struct tag_filter {
static struct sha1_array points_at;
-static int match_pattern(const char **patterns, const char *ref)
-{
- /* no pattern means match everything */
- if (!*patterns)
- return 1;
- for (; *patterns; patterns++)
- if (!fnmatch(*patterns, ref, 0))
- return 1;
- return 0;
-}
-
static const unsigned char *match_points_at(const char *refname,
const unsigned char *sha1)
{
@@ -149,7 +138,7 @@ static int show_reference(const char *refname, const unsigned char *sha1,
{
struct tag_filter *filter = cb_data;
- if (match_pattern(filter->patterns, refname)) {
+ if (match_pattern(refname, filter->patterns, NULL, 0)) {
if (filter->with_commit) {
struct commit *commit;
diff --git a/refs.c b/refs.c
index c9f6835..0c50e81 100644
--- a/refs.c
+++ b/refs.c
@@ -3,6 +3,7 @@
#include "object.h"
#include "tag.h"
#include "dir.h"
+#include "string-list.h"
/* ISSYMREF=0x01, ISPACKED=0x02 and ISBROKEN=0x04 are public interfaces */
#define REF_KNOWS_PEELED 0x10
@@ -2127,3 +2128,38 @@ char *shorten_unambiguous_ref(const char *refname, int strict)
free(short_name);
return xstrdup(refname);
}
+
+static int match_path(const char *name, const char *pattern, int nlen)
+{
+ int plen = strlen(pattern);
+
+ return ((plen <= nlen) &&
+ !strncmp(name, pattern, plen) &&
+ (name[plen] == '\0' ||
+ name[plen] == '/' ||
+ pattern[plen-1] == '/'));
+}
+
+int match_pattern(const char *name, const char **match,
+ struct string_list *exclude, int flags)
+{
+ int nlen = strlen(name);
+
+ if (exclude) {
+ struct string_list_item *x;
+ for_each_string_list_item(x, exclude) {
+ if (!fnmatch(x->string, name, 0))
+ return 0;
+ }
+ }
+ if (!match || !*match)
+ return 1;
+ for (; *match; match++) {
+ if (flags == FNM_PATHNAME)
+ if (match_path(name, *match, nlen))
+ return 1;
+ if (!fnmatch(*match, name, flags))
+ return 1;
+ }
+ return 0;
+}
diff --git a/refs.h b/refs.h
index 33202b0..dd059d8 100644
--- a/refs.h
+++ b/refs.h
@@ -144,4 +144,16 @@ int update_ref(const char *action, const char *refname,
const unsigned char *sha1, const unsigned char *oldval,
int flags, enum action_on_err onerr);
+/*
+ * match_pattern() - compares a name with pattern match and ignore lists
+ * This returns in highest to lowest precedence:
+ * 0 if <name> fnmatch() an <exclude> pattern
+ * 1 if <match> is NULL or empty
+ * 1 if <flags> is FNM_PATHNAME and <name> is an exact match of a listed
+ * pattern upto and including a trailing '/'
+ * 1 <name> fnmatch() a <match> pattern
+ * 0 otherwise
+ */
+int match_pattern(const char *name, const char **match, struct string_list *exclude, int flags);
+
#endif /* REFS_H */
--
1.7.8
^ permalink raw reply related
* [PATCHv3 3/5] tag --exclude option
From: Tom Grennan @ 2012-02-22 1:28 UTC (permalink / raw)
To: git; +Cc: gitster, peff, jasampler, pclouds
In-Reply-To: <20120211190856.GB4903@tgrennan-laptop>
Example,
$ git tag -l --exclude "*-rc?" "v1.7.8*"
v1.7.8
v1.7.8.1
v1.7.8.2
v1.7.8.3
v1.7.8.4
Which is equivalent to,
$ git tag -l "v1.7.8*" | grep -v \\-rc.
v1.7.8
v1.7.8.1
v1.7.8.2
v1.7.8.3
v1.7.8.4
Signed-off-by: Tom Grennan <tmgrennan@gmail.com>
---
Documentation/git-tag.txt | 6 ++++-
builtin/tag.c | 17 ++++++++++---
t/t7004-tag.sh | 56 +++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 74 insertions(+), 5 deletions(-)
diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
index 8d32b9a..470bd80 100644
--- a/Documentation/git-tag.txt
+++ b/Documentation/git-tag.txt
@@ -13,7 +13,7 @@ SYNOPSIS
<tagname> [<commit> | <object>]
'git tag' -d <tagname>...
'git tag' [-n[<num>]] -l [--contains <commit>] [--points-at <object>]
- [<pattern>...]
+ [--exclude <pattern>] [<pattern>...]
'git tag' -v <tagname>...
DESCRIPTION
@@ -90,6 +90,10 @@ OPTIONS
--points-at <object>::
Only list tags of the given object.
+--exclude <pattern>::
+ Don't list tags matching the given pattern. This has precedence
+ over any other match pattern arguments.
+
-m <msg>::
--message=<msg>::
Use the given tag message (instead of prompting).
diff --git a/builtin/tag.c b/builtin/tag.c
index 4a016d5..547f97d 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -21,7 +21,7 @@ static const char * const git_tag_usage[] = {
"git tag [-a|-s|-u <key-id>] [-f] [-m <msg>|-F <file>] <tagname> [<head>]",
"git tag -d <tagname>...",
"git tag -l [-n[<num>]] [--contains <commit>] [--points-at <object>] "
- "\n\t\t[<pattern>...]",
+ "\n\t\t[--exclude <pattern>] [<pattern>...]",
"git tag -v <tagname>...",
NULL
};
@@ -30,6 +30,7 @@ struct tag_filter {
const char **patterns;
int lines;
struct commit_list *with_commit;
+ struct string_list *exclude;
};
static struct sha1_array points_at;
@@ -138,7 +139,7 @@ static int show_reference(const char *refname, const unsigned char *sha1,
{
struct tag_filter *filter = cb_data;
- if (match_pattern(refname, filter->patterns, NULL, 0)) {
+ if (match_pattern(refname, filter->patterns, filter->exclude, 0)) {
if (filter->with_commit) {
struct commit *commit;
@@ -165,13 +166,15 @@ static int show_reference(const char *refname, const unsigned char *sha1,
}
static int list_tags(const char **patterns, int lines,
- struct commit_list *with_commit)
+ struct commit_list *with_commit,
+ struct string_list *exclude)
{
struct tag_filter filter;
filter.patterns = patterns;
filter.lines = lines;
filter.with_commit = with_commit;
+ filter.exclude = exclude;
for_each_tag_ref(show_reference, (void *) &filter);
@@ -428,6 +431,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
const char *msgfile = NULL, *keyid = NULL;
struct msg_arg msg = { 0, STRBUF_INIT };
struct commit_list *with_commit = NULL;
+ struct string_list exclude = STRING_LIST_INIT_NODUP;
struct option options[] = {
OPT_BOOLEAN('l', "list", &list, "list tag names"),
{ OPTION_INTEGER, 'n', NULL, &lines, "n",
@@ -459,6 +463,9 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
OPT_CALLBACK(0, "points-at", NULL, "object",
"print only tags of the object",
parse_opt_points_at),
+ OPT_CALLBACK(0, "exclude", &exclude, "pattern",
+ "ignore pattern matching tags",
+ parse_opt_string_list),
OPT_END()
};
@@ -485,13 +492,15 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
usage_with_options(git_tag_usage, options);
if (list)
return list_tags(argv, lines == -1 ? 0 : lines,
- with_commit);
+ with_commit, &exclude);
if (lines != -1)
die(_("-n option is only allowed with -l."));
if (with_commit)
die(_("--contains option is only allowed with -l."));
if (points_at.nr)
die(_("--points-at option is only allowed with -l."));
+ if (exclude.nr)
+ die(_("--exclude option is only allowed with -l."));
if (delete)
return for_each_tag_name(argv, delete_tag);
if (verify)
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index f8c247a..4f1cf48 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -82,6 +82,10 @@ test_expect_success \
'listing tags using a non-matching pattern should output nothing' \
'test `git tag -l xxx | wc -l` -eq 0'
+test_expect_success \
+ 'listing tags excluding "mytag" should output nothing' \
+ 'test `git tag -l --exclude mytag | wc -l` -eq 0'
+
# special cases for creating tags:
test_expect_success \
@@ -202,6 +206,15 @@ test_expect_success \
'
cat >expect <<EOF
+v0.2.1
+EOF
+test_expect_success \
+ 'listing tags with a suffix as pattern and prefix exclusion' '
+ git tag -l --exclude "v1.*" "*.1" > actual &&
+ test_cmp expect actual
+'
+
+cat >expect <<EOF
t210
t211
EOF
@@ -212,6 +225,15 @@ test_expect_success \
'
cat >expect <<EOF
+t210
+EOF
+test_expect_success \
+ 'listing tags with a prefix as pattern and suffix exclusion' '
+ git tag -l --exclude "*1" "t21*" > actual &&
+ test_cmp expect actual
+'
+
+cat >expect <<EOF
a1
EOF
test_expect_success \
@@ -239,6 +261,15 @@ test_expect_success \
test_cmp expect actual
'
+cat >expect <<EOF
+v1.0.1
+EOF
+test_expect_success \
+ 'listing tags with ? in the pattern and exclusion' '
+ git tag -l --exclude "v1.?.3" "v1.?.?" > actual &&
+ test_cmp expect actual
+'
+
>expect
test_expect_success \
'listing tags using v.* should print nothing because none have v.' '
@@ -263,6 +294,31 @@ test_expect_success 'tag -l can accept multiple patterns' '
test_cmp expect actual
'
+test_expect_success 'tag -l can cancel exclusions' '
+ git tag -l --exclude "v*.3" --no-exclude "v1*" "v0*" >actual &&
+ test_cmp expect actual
+'
+
+cat >expect <<EOF
+v0.2.1
+v1.0.1
+EOF
+test_expect_success 'tag -l can accept multiple patterns and exclusions' '
+ git tag -l --exclude "v*.3" --exclude "v1.0" "v1*" "v0*" >actual &&
+ test_cmp expect actual
+'
+
+cat >expect <<EOF
+v0.2.1
+v1.0.1
+v1.1.3
+EOF
+test_expect_success \
+ 'tag -l can cancel then reapply exclusions' '
+ git tag -l --exclude "v*.3" --no-exclude --exclude "v1.0" \
+ "v1*" "v0*" >actual &&
+ test_cmp expect actual
+'
# creating and verifying lightweight tags:
test_expect_success \
--
1.7.8
^ permalink raw reply related
* [PATCHv3 4/5] branch --exclude option
From: Tom Grennan @ 2012-02-22 1:28 UTC (permalink / raw)
To: git; +Cc: gitster, peff, jasampler, pclouds
In-Reply-To: <20120211190856.GB4903@tgrennan-laptop>
Example,
$ ./git-branch -r --exclude \*HEAD
origin/maint
origin/master
origin/next
origin/pu
origin/todo
Signed-off-by: Tom Grennan <tmgrennan@gmail.com>
---
Documentation/git-branch.txt | 7 ++++++-
builtin/branch.c | 18 +++++++++++++-----
t/t3200-branch.sh | 23 +++++++++++++++++++++++
3 files changed, 42 insertions(+), 6 deletions(-)
diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
index 0427e80..ef08872 100644
--- a/Documentation/git-branch.txt
+++ b/Documentation/git-branch.txt
@@ -10,7 +10,8 @@ SYNOPSIS
[verse]
'git branch' [--color[=<when>] | --no-color] [-r | -a]
[--list] [-v [--abbrev=<length> | --no-abbrev]]
- [(--merged | --no-merged | --contains) [<commit>]] [<pattern>...]
+ [(--merged | --no-merged | --contains) [<commit>]]
+ [--exclude <pattern>] [<pattern>...]
'git branch' [--set-upstream | --track | --no-track] [-l] [-f] <branchname> [<start-point>]
'git branch' (-m | -M) [<oldbranch>] <newbranch>
'git branch' (-d | -D) [-r] <branchname>...
@@ -166,6 +167,10 @@ start-point is either a local or remote-tracking branch.
--contains <commit>::
Only list branches which contain the specified commit.
+--exclude <pattern>::
+ Don't list branches matching the given pattern. This has
+ precedence over other match pattern arguments.
+
--merged [<commit>]::
Only list branches whose tips are reachable from the
specified commit (HEAD if not specified).
diff --git a/builtin/branch.c b/builtin/branch.c
index e46ed58..ec06f66 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -243,6 +243,7 @@ struct ref_list {
int index, alloc, maxwidth, verbose, abbrev;
struct ref_item *list;
struct commit_list *with_commit;
+ struct string_list *exclude;
int kinds;
};
@@ -300,7 +301,7 @@ static int append_ref(const char *refname, const unsigned char *sha1, int flags,
if ((kind & ref_list->kinds) == 0)
return 0;
- if (!match_pattern(refname, cb->pattern, NULL, 0))
+ if (!match_pattern(refname, cb->pattern, ref_list->exclude, 0))
return 0;
commit = NULL;
@@ -498,7 +499,10 @@ static void show_detached(struct ref_list *ref_list)
}
}
-static int print_ref_list(int kinds, int detached, int verbose, int abbrev, struct commit_list *with_commit, const char **pattern)
+static int print_ref_list(int kinds, int detached, int verbose, int abbrev,
+ struct commit_list *with_commit,
+ struct string_list *exclude,
+ const char **pattern)
{
int i;
struct append_ref_cb cb;
@@ -509,6 +513,7 @@ static int print_ref_list(int kinds, int detached, int verbose, int abbrev, stru
ref_list.verbose = verbose;
ref_list.abbrev = abbrev;
ref_list.with_commit = with_commit;
+ ref_list.exclude = exclude;
if (merge_filter != NO_FILTER)
init_revisions(&ref_list.revs, NULL);
cb.ref_list = &ref_list;
@@ -530,7 +535,7 @@ static int print_ref_list(int kinds, int detached, int verbose, int abbrev, stru
qsort(ref_list.list, ref_list.index, sizeof(struct ref_item), ref_cmp);
detached = (detached && (kinds & REF_LOCAL_BRANCH));
- if (detached && match_pattern("HEAD", pattern, NULL, 0))
+ if (detached && match_pattern("HEAD", pattern, exclude, 0))
show_detached(&ref_list);
for (i = 0; i < ref_list.index; i++) {
@@ -665,7 +670,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
enum branch_track track;
int kinds = REF_LOCAL_BRANCH;
struct commit_list *with_commit = NULL;
-
+ struct string_list exclude = STRING_LIST_INIT_NODUP;
struct option options[] = {
OPT_GROUP("Generic options"),
OPT__VERBOSE(&verbose,
@@ -689,6 +694,9 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
PARSE_OPT_HIDDEN | PARSE_OPT_LASTARG_DEFAULT,
parse_opt_with_commit, (intptr_t) "HEAD",
},
+ OPT_CALLBACK(0, "exclude", &exclude, "pattern",
+ "ignorepattern matching branches",
+ parse_opt_string_list),
OPT__ABBREV(&abbrev),
OPT_GROUP("Specific git-branch actions:"),
@@ -753,7 +761,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
return delete_branches(argc, argv, delete > 1, kinds);
else if (list)
return print_ref_list(kinds, detached, verbose, abbrev,
- with_commit, argv);
+ with_commit, &exclude, argv);
else if (edit_description) {
const char *branch_name;
struct strbuf branch_ref = STRBUF_INIT;
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index dd1aceb..8144bc8 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -160,6 +160,29 @@ test_expect_success 'git branch --list -d t should fail' '
test_path_is_missing .git/refs/heads/t
'
+>expect
+test_expect_success \
+ 'git branch --list --exclude "t*" "t*" should be empty' '
+ git branch ta &&
+ git branch tb &&
+ git branch --list --exclude "t*" "t*" > actual &&
+ cmp expect actual
+'
+
+cat >expect <<EOF
+ ta
+EOF
+test_expect_success \
+ 'git branch --list --exclude "tb" "t*" should be "ta"' '
+ git branch --list --exclude "tb" "t*" > actual &&
+ cmp expect actual
+'
+
+test_expect_success \
+ 'git branch -d ta tb should succeed' '
+ git branch -d ta tb
+'
+
mv .git/config .git/config-saved
test_expect_success 'git branch -m q q2 without config should succeed' '
--
1.7.8
^ permalink raw reply related
* [PATCHv3 2/5] tag --points-at option wrapper
From: Tom Grennan @ 2012-02-22 1:28 UTC (permalink / raw)
To: git; +Cc: gitster, peff, jasampler, pclouds
In-Reply-To: <20120211190856.GB4903@tgrennan-laptop>
Use the OPT_CALLBACK wrapper with "--points-at" instead of an
OPTION_CALLBACK block.
Signed-off-by: Tom Grennan <tmgrennan@gmail.com>
---
builtin/tag.c | 7 +++----
1 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/builtin/tag.c b/builtin/tag.c
index 9dcd7d2..4a016d5 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -456,10 +456,9 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
PARSE_OPT_LASTARG_DEFAULT,
parse_opt_with_commit, (intptr_t)"HEAD",
},
- {
- OPTION_CALLBACK, 0, "points-at", NULL, "object",
- "print only tags of the object", 0, parse_opt_points_at
- },
+ OPT_CALLBACK(0, "points-at", NULL, "object",
+ "print only tags of the object",
+ parse_opt_points_at),
OPT_END()
};
--
1.7.8
^ permalink raw reply related
* [PATCHv3 5/5] for-each-ref --exclude option
From: Tom Grennan @ 2012-02-22 1:28 UTC (permalink / raw)
To: git; +Cc: gitster, peff, jasampler, pclouds
In-Reply-To: <20120211190856.GB4903@tgrennan-laptop>
Example,
$ git for-each-ref --format="%(refname)" refs/remotes/origin
refs/remotes/origin/HEAD
refs/remotes/origin/maint
refs/remotes/origin/master
refs/remotes/origin/next
refs/remotes/origin/pu
refs/remotes/origin/todo
$ ./git-for-each-ref --format="%(refname)" --exclude "*/HEAD" refs/remotes/origin
refs/remotes/origin/maint
refs/remotes/origin/master
refs/remotes/origin/next
refs/remotes/origin/pu
refs/remotes/origin/todo
Signed-off-by: Tom Grennan <tmgrennan@gmail.com>
---
Documentation/git-for-each-ref.txt | 7 ++++++-
builtin/for-each-ref.c | 8 +++++++-
t/t6300-for-each-ref.sh | 11 +++++++++++
3 files changed, 24 insertions(+), 2 deletions(-)
diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
index c872b88..5f19a8b 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -9,7 +9,8 @@ SYNOPSIS
--------
[verse]
'git for-each-ref' [--count=<count>] [--shell|--perl|--python|--tcl]
- [(--sort=<key>)...] [--format=<format>] [<pattern>...]
+ [(--sort=<key>)...] [--format=<format>]
+ [--exclude=<pattern>] [<pattern>...]
DESCRIPTION
-----------
@@ -47,6 +48,10 @@ OPTIONS
`xx`; for example `%00` interpolates to `\0` (NUL),
`%09` to `\t` (TAB) and `%0a` to `\n` (LF).
+--exclude <pattern>::
+ Ignore refs matching the given pattern. This has precedence
+ over any other pattern match arguments.
+
<pattern>...::
If one or more patterns are given, only refs are shown that
match against at least one pattern, either using fnmatch(3) or
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index bd6a114..783f59f 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -91,6 +91,9 @@ static const char **used_atom;
static cmp_type *used_atom_type;
static int used_atom_cnt, sort_atom_limit, need_tagged, need_symref;
+/* list of ref patterns and ref groups (i.e. foo/) to ignore */
+static struct string_list exclude = STRING_LIST_INIT_NODUP;
+
/*
* Used to parse format string and sort specifiers
*/
@@ -781,7 +784,7 @@ static int grab_single_ref(const char *refname, const unsigned char *sha1, int f
struct refinfo *ref;
int cnt;
- if (!match_pattern(refname, cb->grab_pattern, NULL, FNM_PATHNAME))
+ if (!match_pattern(refname, cb->grab_pattern, &exclude, FNM_PATHNAME))
return 0;
/*
@@ -985,6 +988,9 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
OPT_STRING( 0 , "format", &format, "format", "format to use for the output"),
OPT_CALLBACK(0 , "sort", sort_tail, "key",
"field name to sort on", &opt_parse_sort),
+ OPT_CALLBACK(0, "exclude", &exclude, "pattern",
+ "ignore pattern matching refs",
+ parse_opt_string_list),
OPT_END(),
};
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 1721784..26df442 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -243,6 +243,17 @@ test_expect_success 'Verify descending sort' '
'
cat >expected <<\EOF
+refs/tags/testtag
+refs/heads/master
+EOF
+
+test_expect_success 'Verify exclusion with sort' '
+ git for-each-ref --format="%(refname)" --sort=-refname \
+ --exclude "*origin*" >actual &&
+ test_cmp expected actual
+'
+
+cat >expected <<\EOF
'refs/heads/master'
'refs/remotes/origin/master'
'refs/tags/testtag'
--
1.7.8
^ permalink raw reply related
* Re: [RFC/PATCH 0/3] push: add 'prune' option
From: Nguyen Thai Ngoc Duy @ 2012-02-22 1:45 UTC (permalink / raw)
To: Jeff King; +Cc: Felipe Contreras, git, Junio C Hamano
In-Reply-To: <20120221173548.GB32376@sigill.intra.peff.net>
On Wed, Feb 22, 2012 at 12:35 AM, Jeff King <peff@peff.net> wrote:
> Huh? Don't we already have "fetch --prune"?
We do indeed. I went to git-fetch.txt, searched for prune but missed
many "include" directives in there.
--
Duy
^ permalink raw reply
* Ambiguous reference weirdness
From: Phil Hord @ 2012-02-22 1:46 UTC (permalink / raw)
To: git, Ramkumar Ramachandra, Junio C Hamano, Jonathan Nieder
I accidentally ran into this today:
$ git cherry-pick 1147
fatal: BUG: expected exactly one commit from walk
git log shows no output:
$ git log 1147
Neither of these are very helpful or reassuring. I tried a few things
but I haven't looked the code yet. I found lots of inconsistencies
along the way.
$ git clone https://github.com/git/git
$ cd git
$git log 114
fatal: ambiguous argument '114': unknown revision or path not in the
working tree.
Use '--' to separate paths from revisions
$ git checkout 114
error: pathspec '114' did not match any file(s) known to git.
$ git merge 114
fatal: '114' does not point to a commit
$ git cherry-pick 114
fatal: ambiguous argument '114': unknown revision or path not in the
working tree.
Use '--' to separate paths from revisions
$ git checkout 1147
fatal: reference is not a tree: 1147
$ git merge 1147
error: 1147: expected commit type, but the object dereferences to blob type
fatal: '1147' does not point to a commit
$ git cherry-pick 1147
fatal: BUG: expected exactly one commit from walk
$ git log 1147
$ git checkout 1146
error: short SHA1 1146 is ambiguous.
error: pathspec '1146' did not match any file(s) known to git.
$ git merge 1146
error: short SHA1 1146 is ambiguous.
fatal: '1146' does not point to a commit
$ git cherry-pick 1146
error: short SHA1 1146 is ambiguous.
error: short SHA1 1146 is ambiguous.
fatal: ambiguous argument '1146': unknown revision or path not in the
working tree.
Use '--' to separate paths from revisions
$ git log 1146
error: short SHA1 1146 is ambiguous.
error: short SHA1 1146 is ambiguous.
fatal: ambiguous argument '1146': unknown revision or path not in the
working tree.
Use '--' to separate paths from revisions
I can understand some of the inconsistent error reporting (checkout
may expect filenames, but cherry-pick and merge do not). But this
seems too varied to me.
And the first two look like bugs.
Any comments or suggestions?
Phil
^ permalink raw reply
* Re: [PATCH 4/4] Only re-encode certain parts in commit object, not the whole
From: Nguyen Thai Ngoc Duy @ 2012-02-22 2:01 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20120221182559.GB32668@sigill.intra.peff.net>
2012/2/22 Jeff King <peff@peff.net>:
> On Tue, Feb 21, 2012 at 09:24:52PM +0700, Nguyen Thai Ngoc Duy wrote:
>
>> Commit object has its own format, which happens to be in ascii, but
>> not really subject to re-encoding.
>>
>> There are only four areas that may be re-encoded: author line,
>> committer line, mergetag lines and commit body. Encoding of tags
>> embedded in mergetag lines is not decided by commit encoding, so leave
>> it out and consider it binary.
>
> Is this worth the effort? Yes, re-encoding the ASCII bits of the commit
> object is unnecessary. But do we actually handle encodings that are not
> ASCII supersets? IOW, I could see the point if this is making it
> possible to hold utf-16 names and messages in your commits (though why
> you would want to do so is beyond me...). But my understanding is that
> this is horribly broken anyway by other parts of the code. And even
> looking at your code below:
No, utf-16 and friends are out of question. 617/1168 supported
encodings in iconv translate chars 10,32-126 to something else, some
of them does not generate NUL. I suppose none of these are actually
used nowadays. Looking again, some don't even successfully translate
the given input. No, it's probably not worth the effort.
--
Duy
^ permalink raw reply
* Re: [PATCH] contrib: added git-diffall
From: Tim Henigan @ 2012-02-22 2:02 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vd397g8ic.fsf@alter.siamese.dyndns.org>
Thank you for taking the time to review this patch. I appreciate it.
On Tue, Feb 21, 2012 at 4:51 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Tim Henigan <tim.henigan@gmail.com> writes:
>> +#!/bin/bash -e
>
> Does this have to be bash-only (iow, infested with bash-isms beyond
> repair), or is you wrote this merely from inertia?
There is no specific reason it must be bash. I changed from
"#!/bin/sh" to "#!/bin/bash -e" due to a bug report from a user on
Ubuntu [1]. The user reported that:
"If you use /bin/sh on ubuntu you get the dash shell instead of bash shell.
This causes git_merge_tool_path to fail. The error isn't trapped,
so it exits
without displaying anything and without cleaning up."
Given that all the other scripts distributed with git use /bin/sh, I
will change this script to match.
> The following is only after a cursory scanning, so there may be other
> things that needs fixing, but anyway:
>
> - Don't use "which" in scripts. Its output is not machine parseable, and
> exit code is not reliable, across platforms. It is only meant for
> consumption by human who can read English (or natural language in the
> current locale).
I used "which" in two places. Both were added to support problems
with missing standard tools on certain platforms (missing mktemp on
msysgit and missing option from tar on Mac [2]). Is there some other
standard way to detect the platform or if certain utils are present?
>> + if test -z "$paths"
>> + then
>> + paths=$1
>> + else
>> + paths="$paths $1"
>> + fi
>
> Just a style tip; if you are going to let shell $IFS split this list
> anyway, it is customary to write the above as
>
> paths="$paths$1 "
Nice...I will change to use this format.
>> + git diff --name-only "$left"..."$right" -- $paths > "$tmp"/filelist
>
> git diff ... -- $paths >"$tmp/filelist"
... <snip>
> mkdir -p "$tmp/$left_dir" "$tmp/$right_dir"
Will change all instances.
>> + if test -n "$compare_staged"
>> + then
>> + ls_list=$(git ls-tree HEAD $name)
>> + if test -n "$ls_list"
>> + then
>> + mkdir -p "$tmp"/"$left_dir"/"$(dirname "$name")"
>> + git show HEAD:"$name" > "$tmp"/"$left_dir"/"$name"
>> + fi
>> + else
>> + mkdir -p "$tmp"/"$left_dir"/"$(dirname "$name")"
>> + git show :"$name" > "$tmp"/"$left_dir"/"$name"
>> + fi
>
> That's misleadingly indented. First I thought "in what case would we want
> to switch the LHS between HEAD:$path and :$path when doing diff --cached?"
> but the overindented four lines starting from the funny "fi" is about non
> cached case.
That is embarrassing. I will fix it.
<snip>
>> + find "$tmp/$right_dir" -type f|while read file; do
>> + cp "$file" "$git_top_dir/${file#$tmp/$right_dir/}"
>> + done
>
> Why is this loop written in such a dense way? Everything else (except for
> that misindented part) were almost to our CodingStyle and was fairly easy
> to read, though.
I missed this in my style cleanup. I will fix it.
>> + fi
>> +
>> + # Remove the tmp directory
>> + rm -rf "$tmp"
>> +}
>> +
>> +trap cleanup EXIT
>
> Does this even trigger? This is not Perl that parses and runs set-up code
> before executing everything else, so I suspect this last line amounts to
> the same thing as writing just
>
> cleanup
>
> without trap nor signal names.
The cleanup triggers on all the platforms I have tested (Ubuntu,
msysgit, Mac). I could change it, but for me it has "just worked".
> If you are to set up temporary files or directories that you want to clean
> up, a good discipline is to follow this order:
>
> - define variable(s) to hold the temporary locations, e.g.
> tmpdir=$(mktemp ...)
>
> - set the trap before starting to use these temporary locations, e.g.
> trap 'rm -rf "$tmpdir' 0 1 2 3 15
>
> - and then start populating tmpdir and do whatever you want to do.
I will review the changes needed for this before submitting v2 of my patch.
[1]: https://github.com/thenigan/git-diffall/pull/9
[2]: https://github.com/thenigan/git-diffall/pull/2#issuecomment-498472
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox