* Re: [PATCH 1/2] t5704 (bundle): rewrite for larger coverage
From: Jonathan Nieder @ 2011-12-15 21:16 UTC (permalink / raw)
To: Ramkumar Ramachandra; +Cc: Git List, Junio C Hamano, Joey Hess
In-Reply-To: <1323967528-10537-2-git-send-email-artagnon@gmail.com>
Hi,
Ramkumar Ramachandra wrote:
> Rewrite
Always a scary word. Very rarely justified, especially when the
original and rewritten versions of something are not going to be able
to coexist for a period while the bugs are ironed out. It doesn't
leave me optimistic.
> the git-bundle testsuite to exercise more of its
> functionality.
Luckily, this goal suggests that I am going to see some new tests
added, without the existing coverage being removed or mangled, so
maybe I can ignore the fears awakened by the word "Rewrite". Let's
see...
[...]
> --- a/t/t5704-bundle.sh
> +++ b/t/t5704-bundle.sh
> @@ -1,56 +1,99 @@
> #!/bin/sh
>
> -test_description='some bundle related tests'
> +test_description='Test git-bundle'
Why?
> . ./test-lib.sh
>
> test_expect_success 'setup' '
> + git config core.logAllRefUpdates false &&
Why?
> + test_commit initial &&
> + git checkout -b branch &&
> + test_commit second &&
> + test_commit third &&
> + git checkout master &&
> + test_commit fourth file
> +'
No explicit tags in the setup this time. Now all commits are referred
to by tags, which worsens the coverage, since if some future change
caused commits not referred to by a tag to be dropped, it would be
missed. Paraphrasing
>file &&
git add file &&
test_tick &&
git commit -m initial &&
git tag -m initial initial
to
test_commit initial file
when not preparing to make some other change in the same place and if
the original was not too confusing feels like gratuitous churn.
[...]
> +test_expect_success 'create succeeds' '
> + git bundle create bundle second third &&
> + cat >expect <<-\EOF &&
> + OBJID refs/tags/third
> + OBJID refs/tags/second
> + EOF
> + {
> + git ls-remote bundle |
> + sed "s/$_x40/OBJID/g"
> + } >actual &&
> + test_cmp expect actual
> +'
A new test. What assertion is it testing? Why censor out the
object names when comparing the expected object names to the
actual ones, instead of computing the appropriate object names
for the expected result? Is this new test useful, or does it
cover ground already tested in t5510-fetch.sh?
> +test_expect_success 'verify succeeds' '
> + git bundle create bundle second third &&
> + git bundle verify bundle
> '
A test for "git bundle verify" is a welcome addition.
> +test_expect_success 'list-heads succeeds' '
> + git bundle create bundle second third &&
> + cat >expect <<-\EOF &&
> + OBJID refs/tags/second
> + OBJID refs/tags/third
> + EOF
> + {
> + git bundle list-heads bundle |
> + sed "s/$_x40/OBJID/g"
> + } >actual &&
> + test_cmp expect actual
> +'
> +
Based on 'git grep -e "git bundle list-heads" -- t', there don't seem
to be any existing tests for "git bundle list-heads" except for
t5510-fetch.sh, but I'm not sure what this adds on top of that one.
> -test_expect_success 'tags can be excluded by rev-list options' '
> -
> - git bundle create bundle --all --since=7.Apr.2005.15:16:00.-0700 &&
> - git ls-remote bundle > output &&
> - ! grep tag output
Dropped?
> +test_expect_success 'create dies on invalid bundle filename' '
> + mkdir adir &&
> + test_expect_code 128 git bundle create adir --all
> +'
How is "invalid bundle filename" a clearer explanation than "bundle
file cannot be created"?
>
> +test_expect_success 'disallow stray command-line options' '
> + test_must_fail git bundle create --junk bundle second third
> '
Ok. Might also be useful to check that no "--junk" or "bundle" file
is created.
> +test_expect_failure 'disallow stray command-line arguments' '
> + git bundle create bundle second third &&
> + test_must_fail git bundle verify bundle junk
> +'
In this case, "stray command-line arguments" actually means "extra
arguments to 'verify'", I guess?
What happens if I run "git bundle verify *.bundle" in a directory
with multiple bundles? What should happen?
> +test_expect_success 'create accepts rev-list options to narrow refs' '
> + git bundle create bundle --all -- file &&
I don't understand what "options to narrow refs" means. Does that
mean options like --remotes=origin which yield refs from some subset
of the ref namespace, unlike --all?
[...]
> +test_expect_success 'unbundle succeeds' '
A test for "git bundle unbundle" is a welcome addition.
[...]
> test_expect_failure 'bundle --stdin' '
> -
> echo master | git bundle create stdin-bundle.bdl --stdin &&
> git ls-remote stdin-bundle.bdl >output &&
> grep master output
> -
> '
Seems like a gratuitous change to mix into a patch that introduces
functional changes.
I found this hard to review, since it doesn't seem very focussed ---
it mixes style cleanups, removal of code, and introduction of new
code. I'd be way happier to see a new patch that just adds new tests
to the script without potentially breaking anything on the way. Then
if the style cleanups still seem important to you, they can be
reviewed as a separate patch.
Hoping that clarifies a little,
Jonathan
^ permalink raw reply
* Re: [PATCH] gitk: fix the display of files when filtered by path
From: Pat Thoyts @ 2011-12-15 21:18 UTC (permalink / raw)
To: Martin von Zweigbergk
Cc: msysGit, Pat Thoyts, Johannes Schindelin, Git, David Aguilar,
Junio C Hamano, Paul Mackerras
[-- Attachment #1: Type: text/plain, Size: 2034 bytes --]
On Dec 15, 2011 8:01 PM, "Martin von Zweigbergk" <
martin.von.zweigbergk@gmail.com> wrote:
>
> Hi,
>
> On Thu, Dec 15, 2011 at 1:24 AM, Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
> > Hi,
> >
> > On Wed, 14 Dec 2011, David Aguilar wrote:
> >
> >> On Tue, Dec 13, 2011 at 8:50 AM, Pat Thoyts
> >> <patthoyts@users.sourceforge.net> wrote:
> >> > Launching 'gitk -- .' or 'gitk -- ..\t' restricts the display to
files
> >> > under the given directory but the file list is left empty. This is
because
> >> > the path_filter function fails to match the filenames which are
relative
> >> > to the working tree to the filter which is filessytem relative.
> >> > This solves the problem by making both names fully qualified
filesystem
> >> > paths before performing the comparison.
>
> How is this related to my patches from April? See
> http://thread.gmane.org/gmane.comp.version-control.git/170853. It's
> clearly not the same problem, but will the patches conflict? Will some
> of mine be unnecessary?
>
> > Thanks for reminding me that I did not yet apply and push. Did so now.
>
> What do you mean by this? Push to where?
> git://git.kernel.org/pub/scm/gitk/gitk.git is still down.
This is about msysgit as I also posted this there.
> Paul and Junio, the patches I sent in April are still not in git.git,
> are they? Can we use another repo until the kernel.org one is up? More
> than eight months to get a patch (or eight) merged is way too long,
> IMO.
>
> Martin
I'm not sure how this might relate to your patches. I've got a version
merged on top of the last version of the gitk report that I have which
includes those and it seems fine. As stated I did this work against
git-core as the gitk repository continues to be unavailable. However as I
have a pretty current snapshot I have pushed this to github to provide
some visibility of things I know are not present within git-core. See
http://github.com/patthoyts/gitk.git
On Dec 15, 2011 8:01 PM, "Martin von Zweigbergk" <
martin.von.zweigbergk@gmail.com> wrote:
[-- Attachment #2: Type: text/html, Size: 2962 bytes --]
^ permalink raw reply
* Re: [PATCH 10/10] revert: report fine-grained error messages from insn parser
From: Junio C Hamano @ 2011-12-15 21:23 UTC (permalink / raw)
To: Ramkumar Ramachandra; +Cc: Git List, Jonathan Nieder
In-Reply-To: <1323881677-11117-11-git-send-email-artagnon@gmail.com>
Ramkumar Ramachandra <artagnon@gmail.com> writes:
> Three kinds of errors can arise from parsing '.git/sequencer/todo':
> 1. Unrecognized action
> 2. Missing space after valid action prefix
I think these two are the same and shouldn't result in different error
messages, i.e. the first one in this sequence is still an "Unrecognized
action" error and does not have anything to do with "pick" action.
pickle rr/cherry-pick~4
pick rr/cherry-pick~3
^ permalink raw reply
* Re: [PATCH 2/2] bundle: rewrite builtin to use parse-options
From: Jonathan Nieder @ 2011-12-15 21:29 UTC (permalink / raw)
To: Ramkumar Ramachandra; +Cc: Git List, Junio C Hamano, Joey Hess
In-Reply-To: <1323967528-10537-3-git-send-email-artagnon@gmail.com>
Ramkumar Ramachandra wrote:
> The git-bundle builtin currently parses command-line options by hand;
> this is fragile, and reports cryptic errors on failure. Use the
> parse-options library to do the parsing instead.
I don't understand how this is fragile. I haven't actually run into
error messages from "git bundle" I found to be cryptic, but if they
are, they surely can be improved locally. Could you give an example
or something?
> Encouraged-by: Jonathan Nieder <jrnieder@gmail.com>
No, not encouraged.
But parseoptification does have some nice benefits, so let's see how
the patch looks...
[...]
> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
> ---
> builtin/bundle.c | 91 +++++++++++++++++++++++++++++++----------------------
> t/t5704-bundle.sh | 2 +-
> 2 files changed, 54 insertions(+), 39 deletions(-)
>
> diff --git a/builtin/bundle.c b/builtin/bundle.c
> index 92a8a60..13ed770 100644
> --- a/builtin/bundle.c
> +++ b/builtin/bundle.c
[...]
> @@ -9,57 +10,71 @@
[...]
> int cmd_bundle(int argc, const char **argv, const char *prefix)
> {
> - struct bundle_header header;
> - const char *cmd, *bundle_file;
> + int prefix_length;
> int bundle_fd = -1;
> - char buffer[PATH_MAX];
> - if (argc < 3)
> - usage(builtin_bundle_usage);
> + const char *subcommand, *bundle_file;
> + struct bundle_header header;
> + struct option options[] = { OPT_END() };
> +
> + argc = parse_options(argc, argv, prefix, options,
> + builtin_bundle_usage, PARSE_OPT_STOP_AT_NON_OPTION);
No, just no. Using parse-options with an empty option table is
complete overkill for handling the "-h" option. Without a lot more
justification, this doesn't make it seem more sane or readable at all.
Stopping here. I wouldn't mind seeing "git bundle" being
parseoptified, but not if the result looks like this.
I _do_ think that a systematic option-parsing library that handles
subcommands would be something possible and probably useful for git.
Its input might include a table with subcommand names, an option table
for each, and a function to call when that subcommand is used:
struct parseopt_subcommand subcmds[] = {
{ "list", no_options, notes_list },
{ "add", add_options, notes_add },
{ "copy", copy_options, notes_copy },
{ "append", append_options, notes_append },
{ "edit", no_options, notes_edit },
{ "show", no_options, notes_show },
...
};
Then "git notes -h" might be able to automatically generate a table of
synopses, and "gite notes --help-all" might print help for all
subcommands. Something like that.
Hope that helps,
Jonathan
^ permalink raw reply
* Re: [PATCH 0/2] Improve git-bundle builtin
From: Junio C Hamano @ 2011-12-15 21:30 UTC (permalink / raw)
To: Ramkumar Ramachandra; +Cc: Git List, Jonathan Nieder, Joey Hess
In-Reply-To: <1323967528-10537-1-git-send-email-artagnon@gmail.com>
Ramkumar Ramachandra <artagnon@gmail.com> writes:
> A couple of thoughts:
>
> 1. There's a SP between the OBJID and the ref name in list-heads as
> opposed to the TAB used by other git commands such as ls-remote,
> diff-tree. Will fixing it break someone's parser somewhere?
If somebody will get broken when you change something, that change is not
"fixing" but just "changing". Why do you even want to "fix" it?
> 2. Is it worth fixing the "--stdin" tests? What is the usecase?
Having to pack too many refs that would not fit on a command line length
limit, which is not an unusual requirement.
^ permalink raw reply
* Re: [msysGit] [PATCH] gitk: fix the display of files when filtered by path
From: Pat Thoyts @ 2011-12-15 21:33 UTC (permalink / raw)
To: Martin von Zweigbergk
Cc: Johannes Schindelin, Paul Mackerras, Junio C Hamano,
David Aguilar, Git, msysGit
In-Reply-To: <CAOeW2eHD7Xutf+pHDyMOo=uZC9PSFZi+aMq1Rx80iTKPFApr8A@mail.gmail.com>
Martin von Zweigbergk <martin.von.zweigbergk@gmail.com> writes:
[resending as my earlier post got bounced from vger]
>Hi,
>
>On Thu, Dec 15, 2011 at 1:24 AM, Johannes Schindelin
><Johannes.Schindelin@gmx.de> wrote:
>> Hi,
>>
>> On Wed, 14 Dec 2011, David Aguilar wrote:
>>
>>> On Tue, Dec 13, 2011 at 8:50 AM, Pat Thoyts
>>> <patthoyts@users.sourceforge.net> wrote:
>>> > Launching 'gitk -- .' or 'gitk -- ..\t' restricts the display to files
>>> > under the given directory but the file list is left empty. This is because
>>> > the path_filter function fails to match the filenames which are relative
>>> > to the working tree to the filter which is filessytem relative.
>>> > This solves the problem by making both names fully qualified filesystem
>>> > paths before performing the comparison.
>
>How is this related to my patches from April? See
>http://thread.gmane.org/gmane.comp.version-control.git/170853. It's
>clearly not the same problem, but will the patches conflict? Will some
>of mine be unnecessary?
>
>> Thanks for reminding me that I did not yet apply and push. Did so now.
>
>What do you mean by this? Push to where?
>git://git.kernel.org/pub/scm/gitk/gitk.git is still down.
>
This is for msysGit.
>Paul and Junio, the patches I sent in April are still not in git.git,
>are they? Can we use another repo until the kernel.org one is up? More
>than eight months to get a patch (or eight) merged is way too long,
>IMO.
I'm not sure how this might relate to your patches. I've got a version
merged on top of the last version of the gitk report that I have which
includes those and it seems fine. As stated I did this work against
git-core as the gitk repository continues to be unavailable. However as
I have a pretty current snapshot I have pushed this to github to provide
some visibility of things I know are not present within git-core. See
http://github.com/patthoyts/gitk.git
--
Pat Thoyts http://www.patthoyts.tk/
PGP fingerprint 2C 6E 98 07 2C 59 C8 97 10 CE 11 E6 04 E0 B9 DD
^ permalink raw reply
* Re: Revisiting metadata storage
From: Hilco Wijbenga @ 2011-12-15 21:40 UTC (permalink / raw)
To: Richard Hartmann; +Cc: Ronan Keryell, Git List
In-Reply-To: <CAD77+gR=SjU0Ne9jort91pdHDA=RjkTJUJmnqKBipqoGUmoL_A@mail.gmail.com>
On 14 December 2011 09:59, Richard Hartmann
<richih.mailinglist@gmail.com> wrote:
> On Tue, Dec 6, 2011 at 23:45, Ronan Keryell
> <Ronan.Keryell@hpc-project.com> wrote:
>
>> At least I'm interested and began to dig into it but I do not have a lot
>> of time to work on it...
>
> If we can agree on Perl, I can try to help. I don't think I speak
> enough Python to be of use with that.
>
> Other people who have an interest in this: Please pipe up so we can
> hammer out a rough consensus & roadmap.
I'd love to have better support for metadata (specifically
timestamps). I don't care whether it's Perl, Python, Bash, or C. I
don't think I'll be much help coding but I'd like to try.
In all honesty though, I plan to rewrite our build to use file digests
instead of timestamps. Right now every rebase means a full (and almost
completely unnecessary) rebuild. Luckily I'm using the wonderful
git-new-workdir so there is no pain when switching branches.
Once the rewrite is complete (in one or two months) Git's relentless
timestamp changes should no longer affect us as much anymore. I would
still like to get a better grip on metadata though. Git should be able
to not touch files that have not changed. But whether that's feasible
or even in scope of what you have in mind... :-)
Cheers,
Hilco
^ permalink raw reply
* Re: [PATCH] Add '-P' as a synonym for '--no-pager' in the git command
From: Junio C Hamano @ 2011-12-15 21:41 UTC (permalink / raw)
To: Joe Ratterman; +Cc: git
In-Reply-To: <1323982541-18995-1-git-send-email-jratt0@gmail.com>
Sorry, but why was I listed on the To: line?
I do not recall there was a list concensus that this is a must-have or
even good-to-have, and I am not personally interested in this. I hesitate
to accept a patch that lets a feature without clear advantage squat on a
short-and-sweet single-letter option, generally speaking.
^ permalink raw reply
* Re: [PATCH 3/4] fetch-pack: match refs exactly
From: Junio C Hamano @ 2011-12-15 21:46 UTC (permalink / raw)
To: Jeff King; +Cc: git, Kevin Sawicki
In-Reply-To: <20111213004808.GC3699@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> This is obviously the one that can break existing fetch-pack users. I
> doubt they exist. If they do, there are a few alternatives:
>
> 1. Come up with some more sane rules for path_match (e.g., try full
> strings first, use full-string matching for things starting with
> "refs/", etc).
>
> 2. Leave the matching in-place for git-fetch-pack, but use exact
> matching for internal users that will always provide qualified refs
> (i.e., "git fetch").
I think the latter is the sane thing to do _if_ this becomes an issue, and
as you mention, it is in line with what "git fetch" wrapper already
does.
Given that fetch-pack was meant to be driven by "git fetch" wrapper that
turns the command line and other refspecs into full refnames on the remote
end before calling it, and also as you mentionied that it is clearly
documented as "relative to $GIT_DIR", I do not think we should support the
tail-match semantics at all in the first place.
^ permalink raw reply
* git help prune accuracy?
From: Martin Fick @ 2011-12-15 21:32 UTC (permalink / raw)
To: Git List
I admit that I am guessing here, but I was wondering if this
paragraph from git help prune was a incorrect, it says:
This runs git fsck --unreachable using all the refs
available in refs/, optionally with additional set of
objects specified on the command line, and prunes all
unpacked objects unreachable from any of these head
objects from the object database. In addition, it prunes
the unpacked objects that are also found in packs by
running git prune-packed.
The last sentence seems like it should maybe have the
following fix:
s/it prunes the unpacked/it prunes the unreferenced/
-Martin
--
Employee of Qualcomm Innovation Center, Inc. which is a
member of Code Aurora Forum
^ permalink raw reply
* Re: git help prune accuracy?
From: Martin Fick @ 2011-12-15 21:53 UTC (permalink / raw)
To: Git List
In-Reply-To: <201112151432.09252.mfick@codeaurora.org>
On Thursday, December 15, 2011 02:32:09 pm Martin Fick
wrote:
> I admit that I am guessing here, but I was wondering if
> this paragraph from git help prune was a incorrect, it
> says:
>
> This runs git fsck --unreachable using all the refs
> available in refs/, optionally with additional set of
> objects specified on the command line, and prunes all
> unpacked objects unreachable from any of these head
> objects from the object database. In addition, it
> prunes the unpacked objects that are also found in packs
> by running git prune-packed.
>
> The last sentence seems like it should maybe have the
> following fix:
>
> s/it prunes the unpacked/it prunes the unreferenced/
Ack, I meant:
s/it prunes the unpacked/it prunes the unreachable/
-Martin
--
Employee of Qualcomm Innovation Center, Inc. which is a
member of Code Aurora Forum
^ permalink raw reply
* Re: [PATCHv2 5/9] add generic terminal prompt function
From: Pete Wyckoff @ 2011-12-15 21:59 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, git
In-Reply-To: <20111215133939.GA2241@sigill.intra.peff.net>
peff@peff.net wrote on Thu, 15 Dec 2011 08:39 -0500:
> I agree it might be a little more obvious to put it there (I think what
> happened is my initial revision did not look at "echo" ever again, and
> then that conditional was added later when I realized that the "!echo"
> case needed us to print the newline manually).
>
> > And why no sigchain_pop() for the signal handler?
>
> Because I used sigchain_push_common, which has no pop_common analog. But
> it's OK, because calling restore_term sets term_fd to -1, making further
> calls a no-op. So leaving the handler in place is fine.
>
> Another option would be to add sigchain_pop_common, which pops the
> same signals from push_common.
Thanks for the detailed explanation. It was indeed the lack of
symmetry that set me off. I missed the term_fd check for both.
-- Pete
^ permalink raw reply
* Re: [PATCH] Gitweb: Avoid warnings when a repo does not have a valid HEAD
From: Junio C Hamano @ 2011-12-15 22:02 UTC (permalink / raw)
To: Joe Ratterman; +Cc: git, jnareb
In-Reply-To: <1323982737-19065-1-git-send-email-jratt0@gmail.com>
Joe Ratterman <jratt0@gmail.com> writes:
> It is possible that the HEAD reference does not point to an existing
> branch. When viewing such a repository in gitweb, a message like this
> one was sent to the error log:
>
> gitweb.cgi: Use of uninitialized value in string eq at /usr/src/git/gitweb/gitweb.cgi line 5115.
>
> Signed-off-by: Joe Ratterman <jratt0@gmail.com>
> ---
> gitweb/gitweb.perl | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 4f0c3bd..5af06d6 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -5440,7 +5440,7 @@ sub git_heads_body {
> for (my $i = $from; $i <= $to; $i++) {
> my $entry = $headlist->[$i];
> my %ref = %$entry;
> - my $curr = $ref{'id'} eq $head;
> + my $curr = $head ? ($ref{'id'} eq $head) : 0;
Makes one wonder if $head could be '0', but I presume this is about the
case where (!defined $head) holds true. Also makes one wonder if a similar
issue exists on the $ref{"id"} side. I suspect that won't be true unless
you have a very screwed-up repository, but in that case a repository with
a HEAD that points at an unborn branch _and_ have other refs that do point
at existing commit is already screwed-up, so if we want to be extremely
pedantic then perhaps ...
my $curr = ((defined $head && exists $ref{"id"} && defined $ref{"id"})
? ($ref{"id"} eq $head)
: 0);
^ permalink raw reply
* Re: [msysGit] [PATCH] gitk: fix the display of files when filtered by path
From: Pat Thoyts @ 2011-12-15 22:06 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Martin von Zweigbergk, Paul Mackerras, git
In-Reply-To: <7v1us5obqa.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
>Martin von Zweigbergk <martin.von.zweigbergk@gmail.com> writes:
>
>> Paul and Junio, the patches I sent in April are still not in git.git,
>> are they? Can we use another repo until the kernel.org one is up? More
>> than eight months to get a patch (or eight) merged is way too long,
>> IMO.
>
>I tend to agree.
>
>I have this slight suspicion that Paul would appreciate if somebody who
>cares about gitk who is capable and willing steps forward and takes over
>the maintainership of gitk, as he is busy in his other projects.
I can do this one along with git-gui if this is the case.
--
Pat Thoyts http://www.patthoyts.tk/
PGP fingerprint 2C 6E 98 07 2C 59 C8 97 10 CE 11 E6 04 E0 B9 DD
^ permalink raw reply
* Re: git help prune accuracy?
From: Junio C Hamano @ 2011-12-15 22:18 UTC (permalink / raw)
To: Martin Fick; +Cc: Git List
In-Reply-To: <201112151453.52157.mfick@codeaurora.org>
Martin Fick <mfick@codeaurora.org> writes:
>> objects from the object database. In addition, it
>> prunes the unpacked objects that are also found in packs
>> by running git prune-packed.
>>
>> The last sentence seems like it should maybe have the
>> following fix:
>>
>> s/it prunes the unpacked/it prunes the unreferenced/
>
> Ack, I meant:
>
> s/it prunes the unpacked/it prunes the unreachable/
"In addition" part is about objects that exist in loose format that are
also found in packs and has nothing to do with reachability.
Running "git pack-objects" to collect loose objects into a new pack will
not remove these loose objects that are copied into that new pack. Because
we try to access objects from an already open packfile before trying a
loose object file, removing these now-redundant loose ones after they are
packed makes sense. And that is what "git prune-packed" does.
^ permalink raw reply
* Re: [PATCH] Gitweb: Avoid warnings when a repo does not have a valid HEAD
From: Junio C Hamano @ 2011-12-15 22:28 UTC (permalink / raw)
To: Joe Ratterman; +Cc: git, jnareb
In-Reply-To: <7v62hhmr2s.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Joe Ratterman <jratt0@gmail.com> writes:
>
>> It is possible that the HEAD reference does not point to an existing
>> branch. When viewing such a repository in gitweb, a message like this
>> one was sent to the error log:
>>
>> gitweb.cgi: Use of uninitialized value in string eq at /usr/src/git/gitweb/gitweb.cgi line 5115.
>>
> ..., but in that case a repository with
> a HEAD that points at an unborn branch _and_ have other refs that do point
> at existing commit is already screwed-up, so if we want to be extremely
> pedantic then perhaps ...
>
> my $curr = ((defined $head && exists $ref{"id"} && defined $ref{"id"})
> ? ($ref{"id"} eq $head)
> : 0);
Just in case, I was not suggesting to update the patch to look like the
above by "if we want to be extremely pedantic".
After all, that error message in the log may be a good thing that notifies
the site administrators about a suspicious repository so that it can be
fixed (even though it was not a designed "feature" but something that
happens to work).
^ permalink raw reply
* Re: Branch names with slashes
From: Michael Haggerty @ 2011-12-15 22:41 UTC (permalink / raw)
To: Neal Kreitzinger; +Cc: Leonardo Kim, git
In-Reply-To: <4EEA5B55.1040405@gmail.com>
On 12/15/2011 09:40 PM, Neal Kreitzinger wrote:
> On 12/14/2011 4:17 AM, Leonardo Kim wrote:
>> Branch names can contain slashes, so we can use 'development/foo' as
>> a branch name. If I choose 'development' as a branch name, it doesn't
>> work. There is a directory named development at '.git/refs/heads'
>> directory. So we cannot create a file named development for
>> 'refs/heads/development'.
>>
>> An error message may occurs like below. Unfortunately, It is not of
>> help to me. 'error: 'refs/heads/development/foo' exists; cannot
>> create 'refs/heads/development'.
>>
>> I think that dealing with a file system and an error message above is
>> not sufficient for a novice like me. I hope that it should be
>> improved.
>>
> FYI, We also use slashes in our branchnames to leverage some of the
> benefits of a path-like namespace like pattern matching and the logical
> expression of hierarchies using descriptive compound names. (We use git
> 1.7.1 on linux.) Here's something to keep in mind: You have to plan
> (think ahead) for your naming conventions so that the namespaces will
> maintain uniqueness at a peer level over time that cannot be confused as
> subdirs under .git/refs/heads. For example:
>
> branchnames:
> /mysystem/generic
> /mysystem/generic/project1
>
> will not work because /mysystem/generic appears to be a parent dir to
> /mysystem/generic/project1 under .git/refs/heads. The solution is:
>
> /mysystem/generic/base
> /mysystem/generic/project1
>
> these branches can coexist because they are unique without one appearing
> to be a parent dir of the other. IOW, their namespaces are peers in
> their entirety. To carry the example a little further,
>
> /mysystem/generic/project1/part2
> will not work because once again it appears to be a subdir of an
> existing branchname (ref). However,
> /mysystem/generic/project1-part2
> will work.
>
> I think the reason for this is that if you look at .git/refs/heads you
> will see that these slash delimited branch names are actually stored as
> subdirs in the filesystem sense. Therefore, git will get confused and
> error out as it tries to find branchnames that are not entirely unique
> by their full path namespace under .git/refs/heads because a branch
> namespace that is a prefix (in the filesystem sense) of another branch
> name would occupy the same path under .git/refs/heads without being
> distinguishable as unique, and vice versa.
Everything that you say is correct. And it is known, at least to a few
git implementers (i.e., not a bug but a conscious design decision). For
example, the function is_refname_available() is used explicitly to
prevent refnames that conflict in the way that you describe *even if*
the refnames in question are packed and therefore not 1:1 with
filesystem paths. This is all a limitation of the fact that references
*can* be represented by files and therefore they inherit the filesystem
constraint that a file and subdirectory within a single parent directory
cannot have the same name.
I don't believe that it would be possible to relax this limitations
without at least a little breakage of backwards compatibility.
What is the bottom line? Feel free to submit patches to improve the
documentation or error reporting. But I doubt that the underlying
limitation will be removed.
Michael
--
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/
^ permalink raw reply
* Re: How to commit incomplete changes?
From: Neal Kreitzinger @ 2011-12-15 22:51 UTC (permalink / raw)
To: Hallvard B Furuseth; +Cc: git
In-Reply-To: <4cfc9cf0515b1bc751f6aa0de4f55e2a@ulrik.uio.no>
On 12/14/2011 5:24 PM, Hallvard B Furuseth wrote:
> Do people have any feelings or conventions for how and when to publish
> a series of commits where the first one(s) break something and the next
> ones clear it up? I've found some discussion, but with vague results.
>
> I'm about to commit some small edits which go together with bigger
> generated changes. It seems both more readable and more cherry-pick-
> friendly to me to keep these in separate commits.
>
> What I've found is I can use a line in the commit message like
> "Incomplete change, requires next commit (update foo/ dir)."
> and, if there is any point, do a no-ff merge past the breakage.
>
A main purpose for the squash and fixup options is (as Randall Schwartz
put it in his git video http://www.youtube.com/watch?v=8dhZ9BXQgc4) "To
make it look like you did it all perfectly without making any mistakes"
(or a reasonable facsimile thereof). You insights on the cherry-picking
of fixes is interesting, but makes no sense in the context of
unpublished work. Why would you need to cherry-pick fixes to mistakes
that have not yet been propagated (published)? If the cherry-picks of
fixes are for your other already merged local branches then just save
the pre-squash/fixup version of the branch to another branch, (ie, git
branch mybranch-b4-fixup) and cherry-pick from that unsquashed copy to
patch up your other unpublished branches. Keep in mind that cherry-pick
is not alway the best way to apply fixes. A merge or rebase to get the
fix is the sign of a better workflow in many cases, TBOMK. On the other
hand, if the bugs have been published then you have no choice but to
commit the fix separately because you can't rearrage/edit published
history. Keep in mind that ideally commits should be logical. You can
use the rearrage feature of interactive rebase to squash fixes into the
feature commit they go to. IOW, I don't think squashing everything into
a giant commit just to consolidate bugfixes into a single commit makes
sense if that would mean losing the distinct separation between
differing feature commits.
I assume by 'generated changes' you mean the automerge in git that is a
wonderful default for vast systems like the linux kernel in which code
is unlikely to overlap logically, but very dangerous in legacy
application systems where changes to the same file can create logical
bugs despite not being on the 'exact same line of code'. You are
supposed to review all your merged files after a merge regardless.
However, we don't trust ourselves that much in our shop so we force
conflicts on same-file edits by making "user-date stamp" updates on
"line 1" (depends on language-dependent comment line rules) in our
pre-commit hook. That way we are forced to manually review the merge of
same-file edits "by hand" thus avoiding "generated results". Of course,
unique-file edits can still break things and thus a merge review is
still in order.
Hope this helps. I'm not a git workflow expert, but my comments are
based on experience. I too am still looking for better ways to manage
workflow while leveraging the flexibity and agility of git for
concurrent development.
v/r,
neal
^ permalink raw reply
* Re: [BUG] in rev-parse
From: Michael Haggerty @ 2011-12-15 22:53 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, nathan.panike, git
In-Reply-To: <20111215070521.GB1327@sigill.intra.peff.net>
On 12/15/2011 08:05 AM, Jeff King wrote:
> On Wed, Dec 14, 2011 at 07:20:41PM -0800, Junio C Hamano wrote:
>
>> Jeff King <peff@peff.net> writes:
>>
>>> On the other hand, it has been like this since it was introduced in
>>> 2006, and I wonder if scripts rely on the --verify side effect.
>>
>> It would have been nicer if it did not to imply --verify at all; a long
>> hexdigit that do not name an existing object at all will be shortened to
>> its prefix that still do not collide with an abbreviated object name of an
>> existing object, and even in such a case, the command should not error out
>> only because it was fed a non-existing object (of course, if "--verify" is
>> given at the same time, its "one input that names existing object only"
>> rule should also kick in).
>
> Dropping the implied verify is easy (see below). But handling
> non-existant sha1s is a much more complicated change, as the regular
> abbreviation machinery assumes that they exist. E.g., with the patch
> below:
>
> $ good=73c6b3575bc638b7096ec913bd91193707e2265d
> $ bad=${good#d}e
> $ git rev-parse --short $good
> 73c6b35
> $ git rev-parse --short $bad
> [no output]
>
> Anyway, I'm not sure it's worth changing at this point. It's part of the
> plumbing API that has been that way forever, it's kind of a rare thing
> to ask for, and I've already shown a workaround using rev-list.
I believe that the OP was more inconvenienced that "git rev-parse
--short" chokes on multiple objects than by the fact that it insists
that the objects exist. (And shortening the SHA1s of non-existent
objects doesn't sound very useful anyway.) So I think that a useful
compromise would be for "git rev-parse --short" to accept multiple args
but continue to insist that each of the args is a valid object.
If that is considered too big a break with backwards compatibility, one
could add a --no-verify option that turns off the verification behavior
of --short. But IMHO this problem is not important enough to justify
adding an extra option.
Michael
--
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/
^ permalink raw reply
* Re: git help prune accuracy?
From: Martin Fick @ 2011-12-15 23:01 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git List
In-Reply-To: <7v1us5mqc4.fsf@alter.siamese.dyndns.org>
On Thursday, December 15, 2011 03:18:19 pm Junio C Hamano
wrote:
> Martin Fick <mfick@codeaurora.org> writes:
> >> objects from the object database. In addition, it
> >>
> >> prunes the unpacked objects that are also found in
> >> packs by running git prune-packed.
> >>
> >> The last sentence seems like it should maybe have the
> >> following fix:
> >>
> >> s/it prunes the unpacked/it prunes the unreferenced/
> >
> > Ack, I meant:
> >
> > s/it prunes the unpacked/it prunes the unreachable/
>
> "In addition" part is about objects that exist in loose
> format that are also found in packs and has nothing to
> do with reachability.
>
> Running "git pack-objects" to collect loose objects into
> a new pack will not remove these loose objects that are
> copied into that new pack. Because we try to access
> objects from an already open packfile before trying a
> loose object file, removing these now-redundant loose
> ones after they are packed makes sense. And that is what
> "git prune-packed" does.
Thanks Junio, that makes a lot of sense. I don't know why I
was not getting that from the description even though that
was exactly what I was looking for. Maybe it was because of
the intro/summary line? Now that I think I understand what
it is doing, it seems like this command is more about
"pruning loose objects" (whether unreachable or already
packed) than it is about "pruning unreachable objects"
(which could be loose or packed)? The summary line reads:
git-prune - Prune all unreachable objects from the object
database
Maybe I am not familiar enough with git terminology, but
does "object database" imply loose objects only? Because
the word "all" in that summary makes it sound like it will
prune all unreachable objects (loose and packed).
I don't quite have an alternative suggestion for a better
summary, the best I could do (but don't like) is:
git-prune - Prune loose objects (unreachable or packed)
-Martin
--
Employee of Qualcomm Innovation Center, Inc. which is a
member of Code Aurora Forum
^ permalink raw reply
* Re: [msysGit] [PATCH] gitk: fix the display of files when filtered by path
From: Paul Mackerras @ 2011-12-15 23:01 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Martin von Zweigbergk, git
In-Reply-To: <7v1us5obqa.fsf@alter.siamese.dyndns.org>
On Thu, Dec 15, 2011 at 11:50:53AM -0800, Junio C Hamano wrote:
> Martin von Zweigbergk <martin.von.zweigbergk@gmail.com> writes:
>
> > Paul and Junio, the patches I sent in April are still not in git.git,
> > are they? Can we use another repo until the kernel.org one is up? More
> > than eight months to get a patch (or eight) merged is way too long,
> > IMO.
>
> I tend to agree.
>
> I have this slight suspicion that Paul would appreciate if somebody who
> cares about gitk who is capable and willing steps forward and takes over
> the maintainership of gitk, as he is busy in his other projects.
Indeed. For now I have put up a repository on ozlabs.org:
git://ozlabs.org/~paulus/gitk.git
but if someone wants to take on the gitk maintainership, please let me
know.
Paul.
^ permalink raw reply
* Re: [msysGit] [PATCH] gitk: fix the display of files when filtered by path
From: Paul Mackerras @ 2011-12-15 22:59 UTC (permalink / raw)
To: Martin von Zweigbergk
Cc: Pat Thoyts, Johannes Schindelin, Junio C Hamano, David Aguilar,
Git, msysGit
In-Reply-To: <CAOeW2eHD7Xutf+pHDyMOo=uZC9PSFZi+aMq1Rx80iTKPFApr8A@mail.gmail.com>
On Thu, Dec 15, 2011 at 11:42:38AM -0800, Martin von Zweigbergk wrote:
> git://git.kernel.org/pub/scm/gitk/gitk.git is still down.
I have just created a repository on ozlabs.org for gitk, since I don't
have kernel.org access at this point. The repository is:
git://ozlabs.org/~paulus/gitk.git
> Paul and Junio, the patches I sent in April are still not in git.git,
> are they? Can we use another repo until the kernel.org one is up? More
> than eight months to get a patch (or eight) merged is way too long,
> IMO.
Your patches are in the master branch. I applied them back in July
but then kernel.org went down.
Paul.
^ permalink raw reply
* Re: [PATCH 10/10] revert: report fine-grained error messages from insn parser
From: Ramkumar Ramachandra @ 2011-12-15 23:18 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git List, Jonathan Nieder
In-Reply-To: <7vsjklmsvt.fsf@alter.siamese.dyndns.org>
Hi Junio,
Junio C Hamano wrote:
> Ramkumar Ramachandra <artagnon@gmail.com> writes:
>
>> Three kinds of errors can arise from parsing '.git/sequencer/todo':
>> 1. Unrecognized action
>> 2. Missing space after valid action prefix
>
> I think these two are the same and shouldn't result in different error
> messages, i.e. the first one in this sequence is still an "Unrecognized
> action" error and does not have anything to do with "pick" action.
>
> pickle rr/cherry-pick~4
> pick rr/cherry-pick~3
Good point. I'll change the parser to check for "pick " || "pick\t"
and "revert " || "revert\t" explicitly in the re-roll.
Thanks.
-- Ram
^ permalink raw reply
* Re: git help prune accuracy?
From: Junio C Hamano @ 2011-12-16 0:04 UTC (permalink / raw)
To: Martin Fick; +Cc: Git List
In-Reply-To: <201112151601.52968.mfick@codeaurora.org>
Martin Fick <mfick@codeaurora.org> writes:
> ... The summary line reads:
>
> git-prune - Prune all unreachable objects from the object
> database
Yea, prune itself has always been primarily about getting rid of
unreachable objects. I suspect (I didn't check) that we did not even have
a call to prune-packed in its original implementation and it was later
added as "we are doing the pruning anyway, why not do this as well while
at it".
Yeah, I just checked. Before 51890a6 (Call prune-packed from "git prune"
as well., 2005-08-19), we didn't. And 2396ec8 (Add "git-prune-packed"
that removes objects that exist in a pack., 2005-07-03) explains it rather
nicely:
Add "git-prune-packed" that removes objects that exist in a pack.
This, together with "git repack" can be used to clean up unpacked
git archives.
> I don't quite have an alternative suggestion for a better
> summary, the best I could do (but don't like) is:
>
> git-prune - Prune loose objects (unreachable or packed)
For a one-liner description, "Remove unnecessary or redundant loose
objects" without parentheses may be better. Explaining "git prune" as
"this prunes" does not add as much information as restating it using a
different and more common verb.
The body text can clarify what we mean by "unnecessary" and "redundant".
A loose object that is old may be unreachable from any of the refs,
i.e. unused, and hence unnecessary. Or the same object as a loose one may
be found in a pack, which would make the loose one redundant.
^ permalink raw reply
* Re: Branch names with slashes
From: Junio C Hamano @ 2011-12-16 0:10 UTC (permalink / raw)
To: Michael Haggerty; +Cc: Neal Kreitzinger, Leonardo Kim, git
In-Reply-To: <4EEA777C.6040607@alum.mit.edu>
Michael Haggerty <mhagger@alum.mit.edu> writes:
> I don't believe that it would be possible to relax this limitations
> without at least a little breakage of backwards compatibility.
I do not think it is worth it, but you _could_ update your git to use a
file whose name is "refs/heads/.foo/bar" as the representation of foo/bar
branch and refs.c API, especially the ones close to resolve_ref(), should
be the only ones that need to know about this. While walking the filesystem
in refs/ hierarchy, any directory you see whose name do not begin with dot
you would treat it as a legacy one and perhaps rename it to match the new
convention on the fly.
> What is the bottom line? Feel free to submit patches to improve the
> documentation or error reporting. But I doubt that the underlying
> limitation will be removed.
I do not think so either. It is not even a limitation but is a basic
nature of "hierarchical names".
^ 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