* Re: Alternates corruption issue
From: Jeff King @ 2012-01-31 21:47 UTC (permalink / raw)
To: Jonathan Nieder
Cc: Junio C Hamano, Richard Purdie, GIT Mailing-list, Hart, Darren,
Ashfield, Bruce
In-Reply-To: <20120131214047.GA13547@burratino>
On Tue, Jan 31, 2012 at 03:40:47PM -0600, Jonathan Nieder wrote:
> Jeff King wrote:
>
> > No, it does not match. While the sequence I outlined above makes the
> > most sense to me, it does not match what setup_git_directory does, which
> > prefers "foo/.git" to using "foo" as a bare repo. I think being
> > consistent between all of the lookup points makes sense. The patch took
> > the least-invasive approach and aligned clone and enter_repo with
> > setup_git_directory.
> >
> > However, we could also tweak setup_git_directory to prefer bare repos
> > over ".git" to keep things consistent. While it makes me feel good from
> > a theoretical standpoint (because the rules above seem simple and
> > intuitive to me), I'm not sure it's a good idea in practice.
>
> Wait, don't these two functions serve two completely different purposes?
Yes, but I would expect the lookup to be the same.
> One is the implementation of (A):
>
> cd foo
> git rev-parse --git-dir
>
> The other implements (B):
>
> git ls-remote foo
Right. But would you expect:
git ls-remote foo
to behave the same as:
cd foo
git ls-remote .
and would you furthermore expect that to behave the same as:
cd foo
git rev-parse --git-dir
?
Maybe I am crazy, but I see all of them as ways of specifying "I am
interested in repository foo", and any lookup magic should be the same
in all cases.
> If "foo" is actually a bare repository that moonlights as a worktree for
> a non-bare repository, then:
>
> 1) Whoever set up these directories is completely insane[*]. Maybe we
> should emit a warning.
Yes, I think we are dealing with an insane corner case.
> 2) As a naive user, I would expect (A) to give a different result
> from (B).
Why?
-Peff
^ permalink raw reply
* Re: Alternates corruption issue
From: Jonathan Nieder @ 2012-01-31 21:55 UTC (permalink / raw)
To: Jeff King
Cc: Junio C Hamano, Richard Purdie, GIT Mailing-list, Hart, Darren,
Ashfield, Bruce
In-Reply-To: <20120131214740.GA2465@sigill.intra.peff.net>
Jeff King wrote:
> Right. But would you expect:
>
> git ls-remote foo
>
> to behave the same as:
>
> cd foo
> git ls-remote .
>
> and would you furthermore expect that to behave the same as:
>
> cd foo
> git rev-parse --git-dir
>
> ?
Nah. I never run "git ls-remote .". ;-)
[...]
>> 2) As a naive user, I would expect (A) to give a different result
>> from (B).
>
> Why?
Normally git commands expect me to be in (possibly a deeply nested
subdirectory) of the worktree. The typical case is a non-bare
repository. I have been taught that it walks to the toplevel and
finds a ".git" directory.
By contrast, the path passed to git transport commands like "git fetch
otherhost:/foo/bar/baz.git" is a path to the git repository metadata.
I am not allowed to pass a path to a subdirectory, for example.
As a user, when would the distinction ever come up? One is a fuzzy
"which repository am I working in", and the other is a precise "point
me to the metadata directory, but I allow you to abbreviate a little
by leaving out the trailing .git".
^ permalink raw reply
* Re: Alternates corruption issue
From: Jeff King @ 2012-01-31 22:05 UTC (permalink / raw)
To: Jonathan Nieder
Cc: Junio C Hamano, Richard Purdie, GIT Mailing-list, Hart, Darren,
Ashfield, Bruce
In-Reply-To: <20120131215501.GF13252@burratino>
On Tue, Jan 31, 2012 at 03:55:01PM -0600, Jonathan Nieder wrote:
> >> 2) As a naive user, I would expect (A) to give a different result
> >> from (B).
> >
> > Why?
>
> Normally git commands expect me to be in (possibly a deeply nested
> subdirectory) of the worktree. The typical case is a non-bare
> repository. I have been taught that it walks to the toplevel and
> finds a ".git" directory.
>
> By contrast, the path passed to git transport commands like "git fetch
> otherhost:/foo/bar/baz.git" is a path to the git repository metadata.
> I am not allowed to pass a path to a subdirectory, for example.
True. But I consider that to make the walk-backwards-from-pwd case
simply a superset. That is, in (A) we are walking backwards and trying
to apply the lookup rule from (B) individually to each directory we
consider (though even that is not entirely true, as we don't look for
parallel "$PWD.git" directories in the walk).
I'll admit I don't care that much, though. This is extremely unlikely to
come up. The real issue is fixing the fact that we prefer "foo.git" to
"foo", even when the user told us "foo". I am content to leave the rest
of it as-is, which is what my original patch did.
-Peff
^ permalink raw reply
* Re: Alternates corruption issue
From: Jonathan Nieder @ 2012-01-31 22:22 UTC (permalink / raw)
To: Jeff King
Cc: Junio C Hamano, Richard Purdie, GIT Mailing-list, Hart, Darren,
Ashfield, Bruce
In-Reply-To: <20120131220510.GA3253@sigill.intra.peff.net>
Jeff King wrote:
> True. But I consider that to make the walk-backwards-from-pwd case
> simply a superset. That is, in (A) we are walking backwards and trying
> to apply the lookup rule from (B) individually to each directory we
> consider (though even that is not entirely true, as we don't look for
> parallel "$PWD.git" directories in the walk).
That parenthesis is important. Although in a nicer world maybe we
would want some symmetry like this, (A) and (B) really have nothing to
do with each other.
Forgetting the algorithms:
(A) means "find what repository we are in the worktree for, or,
barring that, what bare repository we are in".
(B) means "find which repository the user pointed to. To be extra
nice, we allow a '.git' extension to be left out, so the URL used
doesn't have to include the redundant information that this is a
git repository, and we even allow pointing to the toplevel of a
worktree instead of a repository, too."
Notice that the above description does not exactly match the actual
behavior of git. For example, if someone has a directory layout like
this:
repo-manipulator/
.git/
src/
testcases/
repo1.git/
repo2.git/
and git is run from repo1.git, according to the description above, the
naive user _wanted_ git commands to apply to the toplevel repository.
And in practice, I think that's often true, though changing the
behavior of git to match that would not be worth the downsides.
> I'll admit I don't care that much, though. This is extremely unlikely to
> come up.
I admit part of the reason I care is that just putting "" first would
probably taken care of the more important part of
<http://bugs.debian.org/399041>.
Anyway, thanks for explaining. Hopefully I can get to this soon and
factor out a common function for get_repo_path and enter_repo to call
so playing with the ordering becomes a little less scary. ;-)
Jonathan
^ permalink raw reply
* Re: Alternates corruption issue
From: Jeff King @ 2012-01-31 22:42 UTC (permalink / raw)
To: Jonathan Nieder
Cc: Junio C Hamano, Richard Purdie, GIT Mailing-list, Hart, Darren,
Ashfield, Bruce
In-Reply-To: <20120131222258.GG13252@burratino>
On Tue, Jan 31, 2012 at 04:22:58PM -0600, Jonathan Nieder wrote:
> Forgetting the algorithms:
>
> (A) means "find what repository we are in the worktree for, or,
> barring that, what bare repository we are in".
>
> (B) means "find which repository the user pointed to. To be extra
> nice, we allow a '.git' extension to be left out, so the URL used
> doesn't have to include the redundant information that this is a
> git repository, and we even allow pointing to the toplevel of a
> worktree instead of a repository, too."
I can see that as a plausible interpretation. But I think your "means"
is kind of vague. In other words, we are assigning a meaning that is
not actually documented anywhere, and which we are hoping matches user
expectations. So the correct behavior kind of depends on your mental
model. IOW, I find your mental model plausible, but it is not the one
that I had.
But like I said, I'm OK either way. I'll prepare this fix with the least
behavior change possible. Then doing your change on top should be a
trivial rearrangement of the lookup arrays (the "hard" part for your
proposed change is adding in the is_git_directory() calls[1], but I'll
do that as part of mine).
> I admit part of the reason I care is that just putting "" first would
> probably taken care of the more important part of
> <http://bugs.debian.org/399041>.
Would that fix it? If I understand it, the repo in question is bare with
a ".git" directory inside it. Your proposed change would make "git
ls-remote foo" work, but wouldn't "cd foo && git log" still be broken?
At the very least, it would show a completely different repo than
ls-remote.
-Peff
[1] Without the is_git_directory() check, the ordering you propose
breaks things very badly. Because if I point to a non-bare directory
"foo", we say "OK, foo exists" and never look at "foo/.git". So
instead, we have to say "OK, foo exists, but hey, it looks nothing
like a .git directory" and keep trying.
^ permalink raw reply
* Project structure of .NET-Projects using git submodule or something different
From: Harald Heigl @ 2012-01-31 22:41 UTC (permalink / raw)
To: git
Hi, I'm searching for an adequate Project-structure for our .NET-Projects,
but I think the problem isn't specific to .NET-Programs, it's more "how can
I nest submodules toghether".
Let's assume following Project structure (Dependencies and Subdependencies
are submodules and submodules of the submodules)
Project
Dependency 1
Dependency 2
Dependency 3
Dependency 4
Dependency 2
The problem is if I want to build them I need to build 2+3, then 1, 4 and 2
again and then the project. As you may see project 2 is a submodule of
dependency 1 and also of project. I don't feel comfortable with this setup.
What do you think?
I've also thought about symlinks (though I'm mainly on Windows ), but with
symlinks I'll lose the tight coupling of git submodules by SHA1-revision.
I could also add Dependency 1-4 to my main project, not loading submodule
2-3 within dependency 1, but then the Project-Files of .NET for dependency1
may change depending if I clone Dependency1 on its own or if I clone my
superproject.
Sometimes I have longer chains (project - submodule - subsubmodule - . ), so
it could easily be that I clone a superproject and 10 subprojects, even
though there are only 3-5 really different subprojects, rest would be
duplicates.
How would you handle this? Git submodules? Something different?
Thanks in advance,
Harald
^ permalink raw reply
* Re: [RFC/PATCH] add update to branch support for "floating submodules"
From: Phil Hord @ 2012-01-31 22:50 UTC (permalink / raw)
To: Jens Lehmann; +Cc: Junio C Hamano, Leif Gruenwoldt, git
In-Reply-To: <4F28554D.9090107@web.de>
On Tue, Jan 31, 2012 at 3:55 PM, Jens Lehmann <Jens.Lehmann@web.de> wrote:
> Am 30.01.2012 22:15, schrieb Phil Hord:
>> I lost my grip on this thread over the holidays...
>>
>> On Tue, Dec 13, 2011 at 4:09 PM, Jens Lehmann <Jens.Lehmann@web.de> wrote:
>>> Am 13.12.2011 15:17, schrieb Phil Hord:
>>>> git pull origin topic_foo && git submodule foreach 'git pull origin topic_foo'
>>>>
>>>> git submodule foreach 'git push origin topic_foo' && git push origin topic_foo
>>>
>>> This sounds to me like you would need the "--recurse-submodules" option
>>> implemented for "git pull" and "git push", no?
>>
>> Only if I have nested submodules, but yes, we do use --recurs* in our scripts.
>
> I'm confused, push doesn't know the "--recurse-submodules" option
> at all yet while pull only does a deep fetch when it is given, the
> submodule work trees are not updated to the merge result right now.
Sorry. I type faster than I think sometimes.
I meant that I do use something like this:
git submodule foreach --recursive 'git checkout master' && git
checkout master
And I expect to use something like this:
git submodule foreach --recursive 'git push origin topic_foo' &&
git push origin topic_foo
>>> And I miss to see how
>>> floating would help when the tips of some submodules are not ready to
>>> work with other submodules tips ...
>>
>> By project policy, for any branch, all submodules' tips of the
>> same-named branch should be interoperable. The CI server looks after
>> this, as much as he can.
>
> We do the same thing on our CI server, but it can only test some
> combinations (even though that tends to show most problems pretty
> early). But in the end every superproject is responsible to use a
> working set of submodule commits, and I would rather bet on a
> combination the CI server tested than on what happens to be on the
> current tips.
Yes, I see what you mean. In our case, what "happens to be on the
tips" should also have been vetted by our Gerrit+Jenkins code review
dance (for each branch * each superproject), and so it should always
be good. If it's not, we have the Jenkins-maintained gitlink history
we can use to bisect.
>> I think of branch names as sticky notes (extra-lightweight tags,
>> sometimes). We have linear history in many of our vendor submodules,
>> but multiple "branches" indicate where each superproject branch has
>> presumably finished integration.
>
> We also add a branch in submodules every time a superproject needs
> to move away from the submodules master (so the commits won't get
> lost by accident).
Good idea. Gerrit sees to this for us also, but we do share code on
ancillary git servers and we follow the same practice.
>>>> But not all my developers are git-gurus yet, and they sometimes mess
>>>> up their ad hoc scripts or miss important changes they forgot to push
>>>> in one submodule or another.
>>>
>>> Sure, even though current git should help you some by showing changes
>>> in the submodules.
>>
>> Real newbies may not even remember to use 'git status' strategically.
>
> Hmm, but then they will screw up things in the superproject too, no?
What I mean is that a developer may be completely focused on one
particular submodule (his domain). He does his work in this module,
and when it's ready he commits and pushes to the server. 'git status'
shows him that his directory is clean. But this is only because he
doesn't really know where the submodules top-directories are, so he
doesn't realize that he has changes in another submodule that he has
not committed. He has to know to run 'git status' from somewhere in
the superproject (ostensibly in the root directory of that
superproject). But he may forget since 'git status' already assured
him he was done.
Like this:
#-- Setup
mkdir super && cd super && git init
mkdir A && touch A/foo
git add .
git submodule add gerrit:iptv/iptv_scripts B
git commit -m "Initial commit"
#-- Work flow
cd A && echo changes > foo
cd ../B && echo changes > foo
git add . && git commit -m "Made some changes"
git status
# On branch master
# Your branch is ahead of 'origin/master' by 1 commit.
#
nothing to commit (working directory clean)
So git just told my new-to-git developer that his workdir is clean
because he happened to run it from super/B. But if he ran it from
super/A (or super) it would tell him otherwise.
I guess what would help here is something like the opposite of 'git
status' showing the status of descendant submodules; it would help if
it showed the status of sibling submodules and the superproject as
well.
>>>> Or worse, their pull or push fails and
>>>> they can't see the problem for all the noise. So they email it to me.
>>>
>>> We circumvent that by not pulling, but fetching and merging in the
>>> submodule first and after that in the superproject. You have much more
>>> control about what is going wrong where (and can have more
>>> git-experienced people help with - or even do - the merges).
>>
>> I do that, too, and I wish I didn't have to. I wish I could safely
>> and sanely recover from a conflicted "git pull --recurse-submodules"
>> pull from the superproject.
>
> That's what my recursive checkout work is aiming at. Me thinks after
> that we will also need some good ideas on how to present and help
> solving submodule merge conflicts.
>
>> That is, I wish doing so were as
>> straightforward as recovering from the same condition would be if all
>> my code were in one repository instead of in submodules.
>>
>> Which is the gist -- I wish submodules did not make git more
>> complicated than it already is.
>
> I think we can make working with submodule much easier than it is
> now, the next step being updating all submodule work trees as git
> updates the superproject's work tree. Even though I suspect that in
> the long run submodules will always be a bit more complicated than
> having everything in one repository, I'm confident that will be by
> far outweighed by the advantages they bring.
I'm really looking forward to it.
Thanks,
Phil
^ permalink raw reply
* Rebase regression in v1.7.9?
From: Felipe Contreras @ 2012-01-31 22:56 UTC (permalink / raw)
To: git
Hi,
Try the following steps:
% git checkout -b tmp v1.7.9
% git cherry-pick 6e1c9bb^2^
% git rebase -i --onto 6e1c9bb HEAD^
% git rebase --continue
The rebase will finish, but there will be a .git/CHERRY_PICK_HEAD file.
This doesn't happen if you run rebase without -i, or do rebase --skip instead.
IIRC correctly rebase --continue used to work fine.
Cheers.
--
Felipe Contreras
^ permalink raw reply
* Re: Alternates corruption issue
From: Jonathan Nieder @ 2012-01-31 22:59 UTC (permalink / raw)
To: Jeff King
Cc: Junio C Hamano, Richard Purdie, GIT Mailing-list, Hart, Darren,
Ashfield, Bruce
In-Reply-To: <20120131224240.GA3844@sigill.intra.peff.net>
Jeff King wrote:
> On Tue, Jan 31, 2012 at 04:22:58PM -0600, Jonathan Nieder wrote:
>> I admit part of the reason I care is that just putting "" first would
>> probably taken care of the more important part of
>> <http://bugs.debian.org/399041>.
>
> Would that fix it? If I understand it, the repo in question is bare with
> a ".git" directory inside it.
The layout was foo/.git/.git. The real repository was made with plain
"git init" or "git clone", and then "git svn init" or similar was run
from within .git, creating a .git subdirectory. (Something more must
have happened, actually, since the tree was weirdly sparse:
$ find .git/.git
.git/.git
.git/.git/svn
.git/.git/svn/git-svn
.git/.git/svn/git-svn/.rev_db
)
In the current scheme, to access such a broken repository remotely,
you have to use a URL ".../foo". URLs pointing to foo/.git (like gitk
once used) end up rewritten as .git/.git. Of course, your patch with
the is_git_directory() will fix this specific case. :)
Not a huge deal, especially since "git svn init" prints out where it
is writing these days.
^ permalink raw reply
* Re: [PATCH] pack-objects: convert to use parse_options()
From: Junio C Hamano @ 2012-01-31 23:29 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: git
In-Reply-To: <1328017702-14489-1-git-send-email-pclouds@gmail.com>
Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
> @@ -2305,183 +2300,140 @@ static void get_object_list(int ac, const char **av)
> loosen_unused_packed_objects(&revs);
> }
>
> +static int option_parse_index_version(const struct option *opt,
> + const char *arg, int unset)
> +{
> + char *c;
> + const char *val = arg;
> + pack_idx_opts.version = strtoul(val, &c, 10);
> + if (pack_idx_opts.version > 2)
> + die("unsupported index version %s", val);
Dying may have been the appropriate error path in the custom option
parser, but is it still so for a callback in the parse-options framework?
Or should this be a 'return error(_("..."), val)'?
> + if (*c == ',' && c[1])
> + pack_idx_opts.off32_limit = strtoul(c+1, &c, 0);
I do not think the original had this extra "&& c[1]" check and I didn't
see any explanation of this change in the log message, either.
> +static int option_parse_ulong(const struct option *opt,
> + const char *arg, int unset)
> +{
> + if (unset)
> + die("option %s does not accept negative form",
> + opt->long_name);
> +
> + if (!git_parse_ulong(arg, opt->value))
> + die("unable to parse value '%s' for option %s",
> + arg, opt->long_name);
> + return 0;
> +}
Likewise on "die()".
> +#define OPT_ULONG(s, l, v, h) \
> + { OPTION_CALLBACK, (s), (l), (v), "n", (h), \
> + PARSE_OPT_NONEG, option_parse_ulong }
> +
> int cmd_pack_objects(int argc, const char **argv, const char *prefix)
> {
> int use_internal_rev_list = 0;
> int thin = 0;
> int all_progress_implied = 0;
> - uint32_t i;
> - const char **rp_av;
> - int rp_ac_alloc = 64;
> - int rp_ac;
> + const char *rp_av[6];
Nice stackframe shrinkage.
> + int rp_ac = 0;
> + int rev_list_unpacked = 0, rev_list_all = 0, rev_list_reflog = 0;
> + struct option pack_objects_options[] = {
> + OPT_SET_INT('q', "quiet", &progress,
> + "do not show progress meter", 0),
> + OPT_SET_INT(0, "progress", &progress,
> + "show progress meter", 1),
> + OPT_SET_INT(0, "all-progress", &progress,
> + "show progress meter during object writing phase", 2),
Sounds as if the progress is not shown during the counting phase.
Nothing ", too" at the end could not fix, though.
> + OPT_BOOL(0, "all-progress-implied",
> + &all_progress_implied,
> + "similar to --all-progress when progress meter is shown"),
Hrm, interesting wording.
> + { OPTION_CALLBACK, 0, "index-version", NULL, "version",
> + "force generating pack index at a particular version",
> + 0, option_parse_index_version },
Sounds as if you can generate a pack index at HEAD~24, but that is not
what you meant. "version" is too loaded a word that needs qualification.
Perhaps "write the pack index file in the specified idx format version".
"index" is also a loaded word, and our documentation tries to use "idx"
when we talk about the pack index (we say "index" when we mean the
dircache).
> + OPT_INTEGER(0, "window", &window,
> + "limit pack window by objects"),
> + OPT_ULONG(0, "window-memory", &window_memory_limit,
> + "limit pack window by memory"),
> + OPT_INTEGER(0, "depth", &depth,
> + "limit pack window by maximum delta depth"),
These descriptions are somewhat interesting.
The "pack window" is limited by number of objects given by --window (and
by default this is set to 10), but if you give --window-memory, it could
further dynamically shrink it under memory pressure. If the logic were to
use this dynamic shrinkage without any limit on the number of objects in
flight when --window-memory and no --window is given, then you could say
"limit by memory" as if the "pack window" is counted in terms of number of
bytes in the window instead of number of objects and the description would
be accurate. But that is not quite the case as far as I know.
And --depth does not affect the size of the pack window at all. It rejects
an object with deep enough delta chain as a delta-base candidate to limit
the run-time performance penalty for the user of the resulting pack, but
does not stop the caller from trying the next object in the window.
"maximum length of delta chain allowed in the resulting pack", perhaps?
> + OPT_BOOL(0, "reuse-delta", &reuse_delta,
> + "reusing existing deltas"),
> + OPT_BOOL(0, "reuse-object", &reuse_object,
> + "reusing existing objects"),
s/reusing/reuse/???
> + OPT_BOOL(0, "delta-base-offset", &allow_ofs_delta,
> + "use OFS_DELTA objects"),
> + OPT_INTEGER(0, "threads", &delta_search_threads,
> + "use threads when searching for best delta matches"),
> + OPT_BOOL(0, "non-empty", &non_empty,
> + "only create if it would contain at least one object"),
create what? "do not create an empty pack output"?
> + OPT_BOOL(0, "revs", &use_internal_rev_list,
> + "read revision arguments from standard output"),
s/output/input/???
> + { OPTION_SET_INT, 0, "unpacked", &rev_list_unpacked, NULL,
> + "limit the objects to those that are not already packed",
> + PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, 1 },
s/already/yet/???
> + { OPTION_SET_INT, 0, "all", &rev_list_all, NULL,
> + "include all refs under $GIT_DIR/refs directory",
> + PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, 1 },
> + { OPTION_SET_INT, 0, "reflog", &rev_list_reflog, NULL,
> + "include objects referred by reflog entries",
> + PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, 1 },
The latter is more correct---packfile does not pack refs, it packs
objects.
Perhaps former should match: "include objects reachable from any
reference". I would also prefer to avoid "$GIT_DIR/refs _directory_"
to discourage people from running "ls -R .git/refs/".
> + OPT_BOOL(0, "stdout", &pack_to_stdout,
> + "output pack to stdout"),
> + OPT_BOOL(0, "include-tag", &include_tag,
> + "include unasked-for annotated tags"),
"include tag objects that refer to objects to be packed"?
> + OPT_BOOL(0, "keep-unreachable", &keep_unreachable,
> + "keep unreachable objects"),
What does this option mean these days (aka "where are they kept")?
IIRC, this was meant to be used in conjunction with the --unpacked= and
later with --kept-pack-only (see 03a9683d22) and then was later further
modified by 4d6acb70411.
I *think* this meant to include objects that are in packfiles that are not
marked with .keep even when they are not in the revision range to be
packed, so that we won't lose them during gc.
But I think --unpack-unreachable introduced by ca11b21 (let pack-objects
do the writing of unreachable objects as loose objects, 2008-05-14) that
writes these unreachable objects out of the pack to loose form made this
option unnecessary, and it remains unused ever since. It may not be a bad
idea to deprecate this option.
Obviously it is outside of the scope of this patch.
> + OPT_BOOL(0, "unpack-unreachable", &unpack_unreachable,
> + "unpack unreachable objects"),
> + OPT_BOOL(0, "thin", &thin,
> + "create thin packs"),
> + OPT_BOOL(0, "honor-pack-keep", &ignore_packed_keep,
> + "ignore packs that have companion .keep file"),
> + OPT_INTEGER(0, "compression", &pack_compression_level,
> + "pack compression level"),
> + OPT_SET_INT(0, "keep-true-parents", &grafts_replace_parents,
> + "do not hide commits by grafts", 0),
It is noteworthy that this is truly "do not hide".
A graft entry can also add extra commits, but this option will not prevent
the command from including them, so that the result of repacking will keep
the union of parents in the real history (i.e. in commit objects) and
parents in the pretend history (i.e. added by grafts).
> + OPT_END(),
> + };
>
> read_replace_refs = 0;
> ...
> - continue;
> - }
> - usage(pack_usage);
> + if (rev_list_unpacked) {
> + use_internal_rev_list = 1;
> + rp_av[rp_ac++] = "--unpacked";
> + }
> + if (rev_list_all) {
> + use_internal_rev_list = 1;
> + rp_av[rp_ac++] = "--all";
> + }
> + if (rev_list_reflog) {
> + use_internal_rev_list = 1;
> + rp_av[rp_ac++] = "--reflog";
> }
I'd prefer to have "unpacked" the last, to match the order of parameters
in which the typical caller, i.e. repack, calls us, but that is minor.
> @@ -2497,12 +2449,25 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
> * walker.
> */
>
> - if (!pack_to_stdout)
> - base_name = argv[i++];
> -
> - if (pack_to_stdout != !base_name)
> - usage(pack_usage);
> + if (!pack_to_stdout) {
> + if (!argc)
> + die("base name required if --stdout is not given");
> + base_name = argv[0];
> + argc--;
> + }
> + if (argc)
> + die("base name or --stdout are mutually exclusive");
Doesn't this change regress the most basic, i.e.
$ git pack-objects
usage: ...
Other than that, thanks for a pleasant and thought-provoking read.
^ permalink raw reply
* logging disjoint sets of commits in a single command
From: Bryan O'Sullivan @ 2012-02-01 0:15 UTC (permalink / raw)
To: git@vger.kernel.org
I'm trying to use "git log" to display only a handful of commits, where
the commits are not necessarily linearly related to each other.
^ permalink raw reply
* Re: logging disjoint sets of commits in a single command
From: Bryan O'Sullivan @ 2012-02-01 0:27 UTC (permalink / raw)
To: git@vger.kernel.org
In-Reply-To: <CB4DC432.72D%bryano@fb.com>
On 2012-01-31 16:15 , "Bryan O'Sullivan" <bryano@fb.com> wrote:
>I'm trying to use "git log" to display only a handful of commits, where
>the commits are not necessarily linearly related to each other.
And I beautifully fat-fingered the "send" key. Oops.
What I was *going* to say was that it looks like revision.c:limit_list is
(whether intentionally or not) getting in the way of this.
Here's a sample command line against a kernel tree:
git log 373af0c^..373af0c 590dfe2^..590dfe2
I want git to log those two specific commits, but in fact it looks like
limit_list is marking 590dfe2 as UNINTERESTING while processing 373af0c,
and so it gets pruned.
Is there some way around this, or would a patch to fix it be acceptable?
^ permalink raw reply
* Re: [PATCH/RFC v3] grep: Add the option '--exclude'
From: Junio C Hamano @ 2012-02-01 0:31 UTC (permalink / raw)
To: Albert Yale; +Cc: git, Nguyễn Thái Ngọc Duy
In-Reply-To: <1328021108-4662-1-git-send-email-surfingalbert@gmail.com>
Albert Yale <surfingalbert@gmail.com> writes:
> @@ -124,6 +125,12 @@ OPTIONS
> Use fixed strings for patterns (don't interpret pattern
> as a regex).
>
> +-x <pattern>::
> +--exclude <pattern>::
> + In addition to those found in .gitignore (per directory) and
> + $GIT_DIR/info/exclude, also consider these patterns to be in the
> + set of the ignore rules in effect.
That makes it sound as if "git grep" will not produce hits for a path that
was forcibly added despite it matches a pattern in .gitignore file, which
is not true at all.
> -n::
> --line-number::
> Prefix the line number to matching lines.
> diff --git a/builtin/grep.c b/builtin/grep.c
> index 9ce064a..e9e1ac1 100644
> --- a/builtin/grep.c
> +++ b/builtin/grep.c
> @@ -528,7 +528,8 @@ static int grep_cache(struct grep_opt *opt, const struct pathspec *pathspec, int
> struct cache_entry *ce = active_cache[nr];
> if (!S_ISREG(ce->ce_mode))
> continue;
> - if (!match_pathspec_depth(pathspec, ce->name, ce_namelen(ce), 0, NULL))
> + if (!match_pathspec_depth(pathspec, ce->name, ce_namelen(ce),
> + 0, NULL, 1))
> continue;
> /*
> * If CE_VALID is on, we assume worktree file and its cache entry
> @@ -566,6 +567,11 @@ static int grep_tree(struct grep_opt *opt, const struct pathspec *pathspec,
> while (tree_entry(tree, &entry)) {
> int te_len = tree_entry_len(&entry);
>
> + if (match_pathspec_depth(pathspec->exclude,
> + entry.path, strlen(entry.path),
> + 0, NULL, 0))
> + continue;
> +
Why???
IOW, why isn't this
if (!match_pathspec_depth(pathspec, ...))
continue;
My point of the two previous review messages was that it would be nice if
we can make it so that nobody outside match_pathspec_depth() should even
need to know existence of pathspec->exclude. Either that was ignored, or
perhaps you found a compelling reason why that is not a good idea, but if
so I'd like to know why.
I suspect that you do not need to add the extra 0 at the end (it makes the
caller even harder to read) if you go that route. The extra parameter
defeats the whole point of encapsulating the new "negative match" feature
behind match_pathspec_depth() implementation.
> + if (obj->type == OBJ_BLOB) {
> + const char *name_without_sha1 = strchr(name, ':') + 1;
> +
> + if (match_pathspec_depth(pathspec->exclude,
> + name_without_sha1,
> + strlen(name_without_sha1),
> + 0, NULL, 0))
> + return 0;
Likewise.
> diff --git a/cache.h b/cache.h
> index 9bd8c2d..683458a 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -533,9 +533,12 @@ struct pathspec {
> int len;
> unsigned int use_wildcard:1;
> } *items;
> +
> + struct pathspec *exclude; /* This is never NULL. */
> };
"This is never NULL" is blatantly wrong, as pathspec->exclude->exclude
will very likely be NULL.
I've been assuming that the resulting structure would be more like this:
struct pathspec_set {
int nr;
unsigned int has_wildcard:1;
unsigned int recursive:1;
struct pathspec_item {
const char *match;
int len;
unsigned int use_wildcard:1;
} *items;
};
struct pathspec {
const char **raw; /* get_pathspec() result, not freed by free_pathspec() */
int max_depth;
struct pathspec_set include;
struct pathspec_set exclude;
};
I am not including "raw" in the include/exclude because its use should be
deprecated away over time.
The various helpers used in match_pathspec_depth() that know _only_ about
matching and not about excluding will be changed to take a pointer to
"struct pathspec_set" (and possibly max_depth). The top-level callers
would not have to know any of these changes if we structure the API that
way, no?
I think something like that was more or less the structure we discussed
when Nguyễn brought up his negative pathspec work the last time.
^ permalink raw reply
* Re: logging disjoint sets of commits in a single command
From: Carlos Martín Nieto @ 2012-02-01 0:39 UTC (permalink / raw)
To: Bryan O'Sullivan; +Cc: git@vger.kernel.org
In-Reply-To: <CB4DC442.72F%bryano@fb.com>
[-- Attachment #1: Type: text/plain, Size: 1221 bytes --]
On Wed, 2012-02-01 at 00:27 +0000, Bryan O'Sullivan wrote:
> On 2012-01-31 16:15 , "Bryan O'Sullivan" <bryano@fb.com> wrote:
>
> >I'm trying to use "git log" to display only a handful of commits, where
> >the commits are not necessarily linearly related to each other.
>
> And I beautifully fat-fingered the "send" key. Oops.
>
> What I was *going* to say was that it looks like revision.c:limit_list is
> (whether intentionally or not) getting in the way of this.
>
> Here's a sample command line against a kernel tree:
>
> git log 373af0c^..373af0c 590dfe2^..590dfe2
>
> I want git to log those two specific commits, but in fact it looks like
> limit_list is marking 590dfe2 as UNINTERESTING while processing 373af0c,
> and so it gets pruned.
>
> Is there some way around this, or would a patch to fix it be acceptable?
From my reading of the manpage (and the way most git commands work) log
accepts one range of commits. They all get bunched up together.
You might find cat-file's --batch mode interesting.
git rev-list 373af0c^..373af0c | git cat-file --batch
git rev-list 590dfe2^..590dfe2 | git cat-file --batch
looks a lot like what you're looking for.
cmn
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]
^ permalink raw reply
* Re: logging disjoint sets of commits in a single command
From: Junio C Hamano @ 2012-02-01 0:48 UTC (permalink / raw)
To: Bryan O'Sullivan; +Cc: git@vger.kernel.org
In-Reply-To: <CB4DC442.72F%bryano@fb.com>
"Bryan O'Sullivan" <bryano@fb.com> writes:
> Here's a sample command line against a kernel tree:
>
> git log 373af0c^..373af0c 590dfe2^..590dfe2
This command line is _defined_ to be the same as this.
git log ^373af0c^ 373af0c ^590dfe2^ 590dfe2
Hence,
> Is there some way around this, or would a patch to fix it be acceptable?
the answer to the second question is "no, that is not a fix but is a
breakage for the *current* Git users".
The answer to the first question is that you may be able to do something
like this:
(
git rev-list 373af0c^..373af0c
git rev-list 590dfe2^..590dfe2
) |
sort -u |
xargs git show
Having said all that, for users of Git 2.0, giving richer meaning to the
explicit range notation to make your original command line work just like
the above scripted way would be more intuitive. While an unconditional
change to break the current users would totally be unacceptable, we would
want to see somebody come up with a clean migration path toward that goal
without hurting existing users in the longer term.
^ permalink raw reply
* Re: logging disjoint sets of commits in a single command
From: Jeff King @ 2012-02-01 0:53 UTC (permalink / raw)
To: Carlos Martín Nieto; +Cc: Bryan O'Sullivan, git@vger.kernel.org
In-Reply-To: <1328056769.31804.217.camel@centaur.lab.cmartin.tk>
On Wed, Feb 01, 2012 at 01:39:29AM +0100, Carlos Martín Nieto wrote:
> > Here's a sample command line against a kernel tree:
> >
> > git log 373af0c^..373af0c 590dfe2^..590dfe2
> >
> > I want git to log those two specific commits, but in fact it looks like
> > limit_list is marking 590dfe2 as UNINTERESTING while processing 373af0c,
> > and so it gets pruned.
> >
> > Is there some way around this, or would a patch to fix it be acceptable?
>
> From my reading of the manpage (and the way most git commands work) log
> accepts one range of commits. They all get bunched up together.
Right. That command is equivalent to:
373af0c 590dfe2 --not 373af0c^ 590dfe2^
So the limiting for one range you're interested in ends up marking part
of the other as uninteresting, and that's by design. This topic came up
recently, and I think the general consensus is that it would be cool to
be able to do totally independent ranges, but that would be backwards
incompatible with the current behavior.
In the general case, you can emulate this with:
{ git log 373af0c^..373af0c
git log 590dfe2^..590dfe2
} | $PAGER
which is of course slightly more annoying to type. If you're just
interested in _single_ commits, though, you can just give the commits
and turn off walking:
git log --no-walk 373af0c 590dfe2
> You might find cat-file's --batch mode interesting.
>
> git rev-list 373af0c^..373af0c | git cat-file --batch
> git rev-list 590dfe2^..590dfe2 | git cat-file --batch
>
> looks a lot like what you're looking for.
I think you could even drop the rev-lists in this case, since he just
wants a single commit. However, cat-file lacks the niceties of "log",
like fancy --pretty formatting and automatic diffing against parents.
-Peff
^ permalink raw reply
* Re: logging disjoint sets of commits in a single command
From: Bryan O'Sullivan @ 2012-02-01 1:02 UTC (permalink / raw)
To: Jeff King, Junio C Hamano; +Cc: git@vger.kernel.org
In-Reply-To: <20120201005332.GC30969@sigill.intra.peff.net>
On 2012-01-31, "Jeff King" <peff@peff.net> wrote:
>This topic came up
>recently, and I think the general consensus is that it would be cool to
>be able to do totally independent ranges, but that would be backwards
>incompatible with the current behavior.
That's totally sensible. I hadn't been able to tell from inspection
whether the behaviour was deliberate or not.
>which is of course slightly more annoying to type. If you're just
>interested in _single_ commits, though, you can just give the commits
>and turn off walking:
>
> git log --no-walk 373af0c 590dfe2
Oh, nice! I hadn't seen that option.
By the way, the reason I'm even interested in this in the first place is
that the performance of commands like "git blame" and "git log" on files
and subtrees has become a problem for us (> 10 seconds per invocation,
forecast to get much worse), and I wanted to see whether I could feed "git
log" a specific list of revisions, and if so, whether that could yield
good performance.
I have it in mind to build a secondary index (maintained externally) so
that I can supply these git commands with precise lists of revisions for
much faster response times.
Thanks, guys!
^ permalink raw reply
* Re: [PATCH] Correct singular form in diff summary line for human interaction
From: Nguyen Thai Ngoc Duy @ 2012-02-01 1:32 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jonathan Nieder, git, Ævar Arnfjörð,
Frederik Schwarzer, Brandon Casey, dickey
In-Reply-To: <7vvcnr92y0.fsf@alter.siamese.dyndns.org>
On Wed, Feb 1, 2012 at 12:50 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> If there is an environment variable to say "I don't want to see
>> variations on strings intended for humans", can it be spelled as
>> LC_ALL=C?
>
> ...
>
> If we were to touch this, I would prefer to do so unconditionally without
> "hrm, can we reliably guess this is meant for humans?" and release it
> unceremoniously, perhaps as part of the next release that will have a much
> bigger user-visible UI correction to 'merge'.
Unconditionally change is fine to me. There's another implication
that's not mentioned in the commit message, this change also allows
non-English translations. Any objections on i18n or we just keep this
line English only? Personally if scripts do not matter any more, I see
no reasons this line cannot be translated.
--
Duy
^ permalink raw reply
* Re: [PATCH] Correct singular form in diff summary line for human interaction
From: Thomas Dickey @ 2012-02-01 1:45 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy
Cc: Junio C Hamano, Jonathan Nieder, git,
Ævar Arnfjörð, Frederik Schwarzer, Brandon Casey,
dickey
In-Reply-To: <CACsJy8Dd4_Pnvzxww38EfZt8NgRow+qxCReohc45XoNpfJCbYg@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 530 bytes --]
On Wed, Feb 01, 2012 at 08:32:43AM +0700, Nguyen Thai Ngoc Duy wrote:
> On Wed, Feb 1, 2012 at 12:50 AM, Junio C Hamano <gitster@pobox.com> wrote:
> >> If there is an environment variable to say "I don't want to see
> >> variations on strings intended for humans", can it be spelled as
> >> LC_ALL=C?
I assume from Neider on the list that this is related to mawk, but I don't
have the email preceding this...
--
Thomas E. Dickey <dickey@invisible-island.net>
http://invisible-island.net
ftp://invisible-island.net
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* Re: [PATCH] Correct singular form in diff summary line for human interaction
From: Thomas Dickey @ 2012-02-01 1:56 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy
Cc: Junio C Hamano, Jonathan Nieder, git,
Ævar Arnfjörð, Frederik Schwarzer, Brandon Casey,
dickey
In-Reply-To: <CACsJy8Dd4_Pnvzxww38EfZt8NgRow+qxCReohc45XoNpfJCbYg@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1313 bytes --]
On Wed, Feb 01, 2012 at 08:32:43AM +0700, Nguyen Thai Ngoc Duy wrote:
> On Wed, Feb 1, 2012 at 12:50 AM, Junio C Hamano <gitster@pobox.com> wrote:
> >> If there is an environment variable to say "I don't want to see
> >> variations on strings intended for humans", can it be spelled as
> >> LC_ALL=C?
> >
> > ...
... diffstat (google helped find context)
> > If we were to touch this, I would prefer to do so unconditionally without
> > "hrm, can we reliably guess this is meant for humans?" and release it
> > unceremoniously, perhaps as part of the next release that will have a much
> > bigger user-visible UI correction to 'merge'.
>
> Unconditionally change is fine to me. There's another implication
> that's not mentioned in the commit message, this change also allows
> non-English translations. Any objections on i18n or we just keep this
> line English only? Personally if scripts do not matter any more, I see
> no reasons this line cannot be translated.
I seem to recall that gettext does support plurals...
However, going that route means that even innocuous things like the
parentheses "(+)" can be mangled by translators (guaranteed to break
scripts ;-)
--
Thomas E. Dickey <dickey@invisible-island.net>
http://invisible-island.net
ftp://invisible-island.net
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* Re: [PATCH] Correct singular form in diff summary line for human interaction
From: Nguyen Thai Ngoc Duy @ 2012-02-01 2:37 UTC (permalink / raw)
To: dickey
Cc: Junio C Hamano, Jonathan Nieder, git,
Ævar Arnfjörð, Frederik Schwarzer, Brandon Casey,
dickey
In-Reply-To: <20120201015606.GA24482@debian50-32.invisible-island.net>
On Wed, Feb 1, 2012 at 8:56 AM, Thomas Dickey <dickey@his.com> wrote:
>> > If we were to touch this, I would prefer to do so unconditionally without
>> > "hrm, can we reliably guess this is meant for humans?" and release it
>> > unceremoniously, perhaps as part of the next release that will have a much
>> > bigger user-visible UI correction to 'merge'.
>>
>> Unconditionally change is fine to me. There's another implication
>> that's not mentioned in the commit message, this change also allows
>> non-English translations. Any objections on i18n or we just keep this
>> line English only? Personally if scripts do not matter any more, I see
>> no reasons this line cannot be translated.
>
> I seem to recall that gettext does support plurals...
>
> However, going that route means that even innocuous things like the
> parentheses "(+)" can be mangled by translators (guaranteed to break
> scripts ;-)
We disregard scripts at this point already, I think. "+" and "-"
should not be translated. We could either leave them out of
translatable text, or put a note to translators saying not to
translate them.
--
Duy
^ permalink raw reply
* [BUG] gitk and "Ignore space change" option
From: Oleg Kostyuk @ 2012-02-01 2:42 UTC (permalink / raw)
To: git
Debian/testing, git version 1.7.8.3
There is no possibility to:
1) save value of "Ignore space change" option into config file (~/.gitk)
2) pass some option in cmd line (like --ignore-space-change or -w)
So, there is no other control to "Ignore space change", except via
configuration dialog. This is very inconvenient, and I think this
should be considered as bug.
As solution, any of (1) or (2) could be implemented, but if both -
then this will be fantastic :)
Thanks!
PS: could be useful -
http://stackoverflow.com/questions/8221877/gitk-setting-ignore-space-change-option-to-be-true-by-default
--
Sincerely yours,
Oleg Kostyuk (CUB-UANIC)
^ permalink raw reply
* Re: logging disjoint sets of commits in a single command
From: Jeff King @ 2012-02-01 3:03 UTC (permalink / raw)
To: Bryan O'Sullivan; +Cc: Junio C Hamano, git@vger.kernel.org
In-Reply-To: <CB4DCD5C.747%bryano@fb.com>
On Wed, Feb 01, 2012 at 01:02:57AM +0000, Bryan O'Sullivan wrote:
> By the way, the reason I'm even interested in this in the first place is
> that the performance of commands like "git blame" and "git log" on files
> and subtrees has become a problem for us (> 10 seconds per invocation,
> forecast to get much worse), and I wanted to see whether I could feed "git
> log" a specific list of revisions, and if so, whether that could yield
> good performance.
That sounds kind of slow. Is your repository really gigantic? Have you packed
everything? I'm just curious if there's some other way to make things
faster. Is the repository publicly available?
-Peff
^ permalink raw reply
* Re: [PATCH] Correct singular form in diff summary line for human interaction
From: Junio C Hamano @ 2012-02-01 3:04 UTC (permalink / raw)
To: dickey
Cc: Nguyen Thai Ngoc Duy, Jonathan Nieder, git,
Ævar Arnfjörð, Frederik Schwarzer, Brandon Casey,
dickey
In-Reply-To: <20120201015606.GA24482@debian50-32.invisible-island.net>
Thomas Dickey <dickey@his.com> writes:
> On Wed, Feb 01, 2012 at 08:32:43AM +0700, Nguyen Thai Ngoc Duy wrote:
>> On Wed, Feb 1, 2012 at 12:50 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> >> If there is an environment variable to say "I don't want to see
>> >> variations on strings intended for humans", can it be spelled as
>> >> LC_ALL=C?
>> >
>> > ...
>
> ... diffstat (google helped find context)
When we show diffstat from "git diff --stat" (or "git apply --stat"), we
currently do not do any singular/plural on the last line of the output
that summarizes the graph, ending up with:
1 files changed, 1 insertions(+), 0 deletions(-)
when there is a single line insertion to a file and nothing else.
My recollection is that our behaviour originally came from our desire to
be as close as what "diffstat" produces, but that does not seem to be the
case. I observed that the output from recent versions of "diffstat" is
much more human friendly. We get these, depending on the input, from
"diffstat" version 1.53:
1 file changed, 1 insertion(+)
1 file changed, 1 deletion(-)
0 files changed
2 files changed, 3 insertions(+), 1 deletion(-)
Namely, it does singular/plural correctly, and omits unnecessary "0
deletions(-)" and "0 insertions(+)".
I was wondering if you remember what the behaviour of older versions of
"diffstat" was, and if it was changed to be more human friendly over
time. It is very possible that I am misremembering this and "diffstat" has
always done the singular/plural correctly and omitted useless "0 lines".
Somehow it seems hard to get hold of older versions of "diffstat", and I
was hoping that I could get that information straight out of the current
maintainer.
Thanks for responding and sorry for the lack of context of the original
message.
^ permalink raw reply
* [PATCH] request-pull: explicitly ask tags/$name to be pulled
From: Junio C Hamano @ 2012-02-01 5:50 UTC (permalink / raw)
To: git; +Cc: Mark Brown
When asking for a tag to be pulled, disambiguate by leaving tags/ prefix
in front of the name of the tag. E.g.
... in the git repository at:
git://example.com/git/git.git/ tags/v1.2.3
for you to fetch changes up to 123456...
This way, older versions of "git pull" can be used to respond to such a
request more easily, as "git pull $URL v1.2.3" did not DWIM to fetch
v1.2.3 tag in older versions. Also this makes it clearer for humans that
the pull request is made for a tag and he should anticipate a signed one.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
* http://thread.gmane.org/gmane.linux.kernel/1245709/focus=1245909
triggered this
.../howto/using-signed-tag-in-pull-request.txt | 4 ++--
git-request-pull.sh | 2 +-
t/t5150-request-pull.sh | 6 +-----
3 files changed, 4 insertions(+), 8 deletions(-)
diff --git a/Documentation/howto/using-signed-tag-in-pull-request.txt b/Documentation/howto/using-signed-tag-in-pull-request.txt
index a1351c5..98c0033 100644
--- a/Documentation/howto/using-signed-tag-in-pull-request.txt
+++ b/Documentation/howto/using-signed-tag-in-pull-request.txt
@@ -109,7 +109,7 @@ The resulting msg.txt file begins like so:
are available in the git repository at:
- example.com:/git/froboz.git frotz-for-xyzzy
+ example.com:/git/froboz.git tags/frotz-for-xyzzy
for you to fetch changes up to 703f05ad5835c...:
@@ -141,7 +141,7 @@ After receiving such a pull request message, the integrator fetches and
integrates the tag named in the request, with:
------------
- $ git pull example.com:/git/froboz.git/ frotz-for-xyzzy
+ $ git pull example.com:/git/froboz.git/ tags/frotz-for-xyzzy
------------
This operation will always open an editor to allow the integrator to fine
diff --git a/git-request-pull.sh b/git-request-pull.sh
index 64960d6..e6438e2 100755
--- a/git-request-pull.sh
+++ b/git-request-pull.sh
@@ -63,7 +63,7 @@ die "fatal: No commits in common between $base and $head"
find_matching_ref='
sub abbr {
my $ref = shift;
- if ($ref =~ s|refs/heads/|| || $ref =~ s|refs/tags/||) {
+ if ($ref =~ s|^refs/heads/|| || $ref =~ s|^refs/tags/|tags/|) {
return $ref;
} else {
return $ref;
diff --git a/t/t5150-request-pull.sh b/t/t5150-request-pull.sh
index da25bc2..7c1dc64 100755
--- a/t/t5150-request-pull.sh
+++ b/t/t5150-request-pull.sh
@@ -179,11 +179,7 @@ test_expect_success 'request names an appropriate branch' '
read repository &&
read branch
} <digest &&
- {
- test "$branch" = full ||
- test "$branch" = master ||
- test "$branch" = for-upstream
- }
+ test "$branch" = tags/full
'
--
1.7.9.155.gf6ee6
^ permalink raw reply related
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