Git development
 help / color / mirror / Atom feed
* Re: [RFC 2/2] exclude-promisor-objects: declare when option is allowed
From: Jeff King @ 2018-12-01 19:44 UTC (permalink / raw)
  To: Matthew DeVore
  Cc: Matthew DeVore, git, gitster, pclouds, jonathantanmy, jeffhost
In-Reply-To: <19c82fb0-e0d6-0b15-06ab-cfba4d699d94@comcast.net>

On Fri, Nov 30, 2018 at 05:32:47PM -0800, Matthew DeVore wrote:

> > Speaking of which, would this flag work better as a field in
> > setup_revision_opt, which is passed to setup_revisions()? The intent
> > seem to be to influence how we parse command-line arguments, and that's
> > where other similar flags are (e.g., assume_dashdash).
> 
> Good idea. This would solve the problem of mistakenly believing the flag
> matters when it doesn't, since it is in the struct which is used as the
> arguments of the exact function that cares about it. Here's a new patch -
> I'm tweaking e-mail client settings so hopefully this makes it to the list
> without mangling - if not I'll resend it with `git-send-email` later.
> 
> From 941c89fe1e226ed4d210ce35d0d906526b8277ed Mon Sep 17 00:00:00 2001
> From: Matthew DeVore <matvore@google.com>
> Date: Fri, 30 Nov 2018 16:43:32 -0800
> Subject: [PATCH] revisions.c: put promisor option in specialized struct
> 
> Put the allow_exclude_promisor_objects flag in setup_revision_opt. When
> it was in rev_info, it was unclear when it was used, since rev_info is
> passed to functions that don't use the flag. This resulted in
> unnecessary setting of the flag in prune.c, so fix that as well.

Thanks, this looks pretty reasonable overall. Two comments:

>  	repo_init_revisions(the_repository, &revs, NULL);
>  	save_commit_buffer = 0;
> -	revs.allow_exclude_promisor_objects_opt = 1;
> -	setup_revisions(ac, av, &revs, NULL);
> +
> +	memset(&s_r_opt, 0, sizeof(s_r_opt));
> +	s_r_opt.allow_exclude_promisor_objects = 1;
> +	setup_revisions(ac, av, &revs, &s_r_opt);

I wonder if a static initializer for setup_revision_opt is worth it. It
would remove the need for this memset. Probably not a big deal either
way, though.

>  static int handle_revision_opt(struct rev_info *revs, int argc, const char
> **argv,
> -			       int *unkc, const char **unkv)
> +			       int *unkc, const char **unkv,
> +			       int allow_exclude_promisor_objects)

Why not pass in the whole setup_revision_opt struct? We don't need
anything else from it yet, but it seems like the point of that struct is
to pass around preferences like this.

-Peff

^ permalink raw reply

* Re: [PATCH 8/9] sha1-file: use loose object cache for quick existence check
From: Jeff King @ 2018-12-01 19:49 UTC (permalink / raw)
  To: René Scharfe
  Cc: Geert Jansen, Ævar Arnfjörð Bjarmason,
	Junio C Hamano, git@vger.kernel.org, Takuto Ikuta
In-Reply-To: <ec25f85c-4b8c-0b83-addb-074957de1e1c@web.de>

On Tue, Nov 27, 2018 at 09:48:57PM +0100, René Scharfe wrote:

> > +static int quick_has_loose(struct repository *r,
> > +			   const unsigned char *sha1)
> > +{
> > +	int subdir_nr = sha1[0];
> > +	struct object_id oid;
> > +	struct object_directory *odb;
> > +
> > +	hashcpy(oid.hash, sha1);
> > +
> > +	prepare_alt_odb(r);
> > +	for (odb = r->objects->odb; odb; odb = odb->next) {
> > +		odb_load_loose_cache(odb, subdir_nr);
> 
> Is this thread-safe?  What happens if e.g. one index-pack thread resizes
> the array while another one sorts it?

No, but neither is any of the object_info / has_object_file path, which
may use static function-local buffers, or (before my series) alt scratch
bufs, or even call reprepare_packed_git().

In the long run, I think the solution is probably going to be pushing
some mutexes into the right places, and putting one around the cache
fill is an obvious place.

> Loading the cache explicitly up-front would avoid that, and improves
> performance a bit in my (very limited) tests on an SSD.  Demo patch for
> next at the bottom.  How does it do against your test cases?

It's going to do badly on corner cases where we don't need to load every
object subdirectory, and one or more of them are big. I.e., if I look up
"1234abcd", the current code only needs to fault in $GIT_DIR/objects/12.
Pre-loading means we'd hit them all. Even without a lot of objects, on
NFS that's 256 latencies instead of 1.

-Peff

^ permalink raw reply

* Re: [PATCH] t5562: skip if NO_CURL is enabled
From: Jeff King @ 2018-12-01 19:50 UTC (permalink / raw)
  To: SZEDER Gábor; +Cc: Carlo Arenas, max, git
In-Reply-To: <20181128132708.GE30222@szeder.dev>

On Wed, Nov 28, 2018 at 02:27:08PM +0100, SZEDER Gábor wrote:

> > Curiously, the act.err file also has 54 NUL bytes before the "fatal:"
> > message.
> 
> I think those NUL bytes come from the file system.
> 
> The contents of 'act.err' from the previous test ('fetch gzipped
> empty') is usually:
> 
>   fatal: request ended in the middle of the gzip stream
>   fatal: the remote end hung up unexpectedly
> 
> Notice that the length of the first line is 54 bytes (including the
> trailing newline).  So I suspect that the following is happening:
> 
>   - http-backend in the previous test writes the first line,
>   - that test finishes and this one starts,
>   - this test truncates 'act.err',
>   - and then the still-running http-backend from the previous test
>     finally writes the second line.
> 
> So at this point 'act.err' is empty, but the offset of the fd of the
> redirection still open from the previous test is at 54, so the file
> system fills those bytes with NULs.

Right, good thinking. Thanks for the explanation!

-Peff

^ permalink raw reply

* Re: [PATCH] t5562: skip if NO_CURL is enabled
From: Jeff King @ 2018-12-01 19:53 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason; +Cc: Carlo Arenas, max, git
In-Reply-To: <878t1dz1wi.fsf@evledraar.gmail.com>

On Wed, Nov 28, 2018 at 03:56:29PM +0100, Ævar Arnfjörð Bjarmason wrote:

> 
> On Thu, Nov 22 2018, Jeff King wrote:
> 
> > On Thu, Nov 22, 2018 at 02:17:01AM -0800, Carlo Arenas wrote:
> >> PS. upstreaming the PERL_PATH fix is likely to be good to do soonish
> >> as I presume at least all BSD might be affected, let me know if you
> >> would rather me do that instead as I suspect we might be deadlocked
> >> otherwise ;)
> >
> > Yeah, the $PERL_PATH thing is totally orthogonal, and should graduate
> > separately.
> 
> On the subject of orthagonal things: This test fails on AIX with /bin/sh
> (but not /bin/bash) due to some interaction of ssize_b100dots and the
> build_option function. On that system:
> 
>     $ ./git version --build-options
>     git version 2.20.0-rc1
>     cpu: 00FA74164C00
>     no commit associated with this build
>     sizeof-long: 4
>     sizeof-size_t: 4
> 
> But it somehow ends up in the 'die' condition in that case statement. I
> dug around briefly but couldn't find the cause, probably some limitation
> in the shell constructs it supports. Just leaving a note about this...

That's weird. The functions involved are pretty vanilla. I'd suspect
something funny with the sed invocation:

  build_option () {
        git version --build-options |
        sed -ne "s/^$1: //p"
  }

but that's the one thing that shouldn't be dependent on the shell in
use.

Can you manually replicate the shell commands to see where it goes
wrong?

-Peff

^ permalink raw reply

* Re: [PATCH] rebase -i: introduce the 'test' command
From: Jeff King @ 2018-12-01 20:02 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Paul Morelle, Git Users
In-Reply-To: <nycvar.QRO.7.76.6.1811281935310.41@tvgsbejvaqbjf.bet>

On Thu, Nov 29, 2018 at 09:32:48AM +0100, Johannes Schindelin wrote:

> > > Would it not make more sense to add a command-line option (and a config
> > > setting) to re-schedule failed `exec` commands? Like so:
> > 
> > Your proposition would do in most cases, however it is not possible to
> > make a distinction between reschedulable and non-reschedulable commands.
> 
> True. But I don't think that's so terrible.
> 
> What I think is something to avoid is two commands that do something very,
> very similar, but with two very, very different names.
> 
> In reality, I think that it would even make sense to change the default to
> reschedule failed `exec` commands. Which is why I suggested to also add a
> config option.

I sometimes add "x false" to the top of the todo list to stop and create
new commits before the first one. That would be awkward if I could never
get past that line. However, I think elsewhere a "pause" line has been
discussed, which would serve the same purpose.

I wonder how often this kind of "yes, I know it fails, but keep going
anyway" situation would come up. And what the interface is like for
getting past it. E.g., what if you fixed a bunch of stuff but your tests
still fail? You may not want to abandon the changes you've made, but you
need to "rebase --continue" to move forward. I encounter this often when
the correct fix is actually in an earlier commit than the one that
yields the test failure. You can't rewind an interactive rebase, so I
complete and restart it, adding an "e"dit at the earlier commit.

How would I move past the test that fails to continue? I guess "git
rebase --edit-todo" and then manually remove it (and any other remaining
test lines)?

That's not too bad, but I wonder if people would find it more awkward
than the current way (which is to just "rebase --continue" until you get
to the end).

I dunno. I am not sure if I am for or against changing the default, so
these are just some musings. :)

-Peff

^ permalink raw reply

* [RFC] git clean --local
From: Cameron Boehmer @ 2018-12-01 22:51 UTC (permalink / raw)
  To: git

-x and -X are great, but they remove files that are ignored via my
~/.gitignore that I'd rather keep (personal toolchain dotfiles). If
others also would like to see this addressed and we settle on a
specific solution, I'd be happy to submit a patch. Some ideas:

1) add a new flag
-l, --local
    Do not consult git config --global core.excludesFile in
determining what files git ignores. This is useful in conjunction with
-x/-X to preserve user files while removing build artifacts.

2) change the behavior of -x/-X
While it would be inconsistent with git's behavior elsewhere to NOT
consult the global excludesFile, the intent behind -x/-X seems to have
everything to do with the contents of current project's .gitignores,
and nothing to do with the global excludes. However, even if this is
palatable, it's not backwards compatible, and I'm not sure this meets
the threshold of significance for breaking changes.

Of course, I'm open to suggestions.

Thanks to all for their contributions,
Cameron

^ permalink raw reply

* Re: [RFC] git clean --local
From: Junio C Hamano @ 2018-12-02  0:04 UTC (permalink / raw)
  To: Cameron Boehmer; +Cc: git
In-Reply-To: <CAM+q9MeVS1e11vzu+-RP-i5NhSsnRz=x21q3gcGy8L62yceiMw@mail.gmail.com>

Cameron Boehmer <cameron.boehmer@gmail.com> writes:

> 1) add a new flag
> -l, --local
>     Do not consult git config --global core.excludesFile in
> determining what files git ignores. This is useful in conjunction with
> -x/-X to preserve user files while removing build artifacts.

This does not belong to the "clean" command (who says the need to
ignore the global configuration is limited to "clean" and why?), so
"git clean --local" is simply not acceptable.

But it might be useful as an option that affects any "git" command,
e.g. "git --local-config-only clean".  I dunno.

> 2) change the behavior of -x/-X

This won't happen without a long deprecation period.

If you haven't seen and read them, check the recent list archive for
the past few weeks, with keywords "trashable", "precious", etc.

^ permalink raw reply

* Re: [PATCH] rebase -i: introduce the 'test' command
From: Eric Sunshine @ 2018-12-02  2:28 UTC (permalink / raw)
  To: Jeff King; +Cc: Johannes Schindelin, paul.morelle, Git List
In-Reply-To: <20181201200209.GC29120@sigill.intra.peff.net>

On Sat, Dec 1, 2018 at 3:02 PM Jeff King <peff@peff.net> wrote:
> On Thu, Nov 29, 2018 at 09:32:48AM +0100, Johannes Schindelin wrote:
> > In reality, I think that it would even make sense to change the default to
> > reschedule failed `exec` commands. Which is why I suggested to also add a
> > config option.
>
> I sometimes add "x false" to the top of the todo list to stop and create
> new commits before the first one. That would be awkward if I could never
> get past that line. However, I think elsewhere a "pause" line has been
> discussed, which would serve the same purpose.

Are you thinking of the "break" command (not "pause") which Dscho
already added[1]?

[1]: 71f82465b1 (rebase -i: introduce the 'break' command, 2018-10-12)

^ permalink raw reply

* [PATCH] t5004: avoid using tar for empty packages
From: Carlo Marcelo Arenas Belón @ 2018-12-02  2:33 UTC (permalink / raw)
  To: git; +Cc: peff, l.s.r

ea2d20d4c2 ("t5004: avoid using tar for checking emptiness of archive",
2013-05-09), introduced a fake empty tar archive to allow for portable
tests of emptiness without having to invoke tar

4318094047 ("archive: don't add empty directories to archives", 2017-09-13)
changed the expected result for its tests from one containing an empty
directory to a plain empty archive but the portable test wasn't updated
resulting on them failing again in (at least) NetBSD and OpenBSD

Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
---
 t/t5004-archive-corner-cases.sh | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/t/t5004-archive-corner-cases.sh b/t/t5004-archive-corner-cases.sh
index ced44355ca..271eb5a1fd 100755
--- a/t/t5004-archive-corner-cases.sh
+++ b/t/t5004-archive-corner-cases.sh
@@ -3,8 +3,12 @@
 test_description='test corner cases of git-archive'
 . ./test-lib.sh
 
-test_expect_success 'create commit with empty tree' '
-	git commit --allow-empty -m foo
+# the 10knuls.tar file is used to test for an empty git generated tar
+# without having to invoke tar because an otherwise valid empty GNU tar
+# will be considered broken by {Open,Net}BSD tar
+test_expect_success 'create commit with empty tree and fake empty tar' '
+	git commit --allow-empty -m foo &&
+	perl -e "print \"\\0\" x 10240" >10knuls.tar
 '
 
 # Make a dir and clean it up afterwards
@@ -47,7 +51,6 @@ test_expect_success HEADER_ONLY_TAR_OK 'tar archive of commit with empty tree' '
 
 test_expect_success 'tar archive of empty tree is empty' '
 	git archive --format=tar HEAD: >empty.tar &&
-	perl -e "print \"\\0\" x 10240" >10knuls.tar &&
 	test_cmp_bin 10knuls.tar empty.tar
 '
 
@@ -106,16 +109,12 @@ test_expect_success 'create a commit with an empty subtree' '
 
 test_expect_success 'archive empty subtree with no pathspec' '
 	git archive --format=tar $root_tree >subtree-all.tar &&
-	make_dir extract &&
-	"$TAR" xf subtree-all.tar -C extract &&
-	check_dir extract
+	test_cmp_bin 10knuls.tar subtree-all.tar
 '
 
 test_expect_success 'archive empty subtree by direct pathspec' '
 	git archive --format=tar $root_tree -- sub >subtree-path.tar &&
-	make_dir extract &&
-	"$TAR" xf subtree-path.tar -C extract &&
-	check_dir extract
+	test_cmp_bin 10knuls.tar subtree-path.tar
 '
 
 ZIPINFO=zipinfo
-- 
2.20.0.rc2


^ permalink raw reply related

* [PATCH] t5004: avoid using tar for empty packages
From: Carlo Marcelo Arenas Belón @ 2018-12-02  2:40 UTC (permalink / raw)
  To: git

ea2d20d4c2 ("t5004: avoid using tar for checking emptiness of archive",
2013-05-09), introduced a fake empty tar archive to allow for portable
tests of emptiness without having to invoke tar

4318094047 ("archive: don't add empty directories to archives", 2017-09-13)
changed the expected result for its tests from one containing an empty
directory to a plain empty archive but the portable test wasn't updated
resulting on them failing again in (at least) NetBSD and OpenBSD

Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
---
 t/t5004-archive-corner-cases.sh | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/t/t5004-archive-corner-cases.sh b/t/t5004-archive-corner-cases.sh
index ced44355ca..271eb5a1fd 100755
--- a/t/t5004-archive-corner-cases.sh
+++ b/t/t5004-archive-corner-cases.sh
@@ -3,8 +3,12 @@
 test_description='test corner cases of git-archive'
 . ./test-lib.sh
 
-test_expect_success 'create commit with empty tree' '
-	git commit --allow-empty -m foo
+# the 10knuls.tar file is used to test for an empty git generated tar
+# without having to invoke tar because an otherwise valid empty GNU tar
+# will be considered broken by {Open,Net}BSD tar
+test_expect_success 'create commit with empty tree and fake empty tar' '
+	git commit --allow-empty -m foo &&
+	perl -e "print \"\\0\" x 10240" >10knuls.tar
 '
 
 # Make a dir and clean it up afterwards
@@ -47,7 +51,6 @@ test_expect_success HEADER_ONLY_TAR_OK 'tar archive of commit with empty tree' '
 
 test_expect_success 'tar archive of empty tree is empty' '
 	git archive --format=tar HEAD: >empty.tar &&
-	perl -e "print \"\\0\" x 10240" >10knuls.tar &&
 	test_cmp_bin 10knuls.tar empty.tar
 '
 
@@ -106,16 +109,12 @@ test_expect_success 'create a commit with an empty subtree' '
 
 test_expect_success 'archive empty subtree with no pathspec' '
 	git archive --format=tar $root_tree >subtree-all.tar &&
-	make_dir extract &&
-	"$TAR" xf subtree-all.tar -C extract &&
-	check_dir extract
+	test_cmp_bin 10knuls.tar subtree-all.tar
 '
 
 test_expect_success 'archive empty subtree by direct pathspec' '
 	git archive --format=tar $root_tree -- sub >subtree-path.tar &&
-	make_dir extract &&
-	"$TAR" xf subtree-path.tar -C extract &&
-	check_dir extract
+	test_cmp_bin 10knuls.tar subtree-path.tar
 '
 
 ZIPINFO=zipinfo
-- 
2.20.0.rc2


^ permalink raw reply related

* [PATCH 1/2] config.mak.uname: OpenBSD uses BSD semantics with fread for directories
From: Carlo Marcelo Arenas Belón @ 2018-12-02  2:43 UTC (permalink / raw)
  To: git

this "fixes" test 23 (proper error on directory "files") from t1308

MirBSD likely also affected but this was only tested with OpenBSD and
therefore this specific change only affects that platform

the optional 'configure' sets this automatically (tested with 6.1 to 6.4)
but considering this is a legacy feature it is likely that it affected
all old versions and is probably what most users had been using as a
workaround

Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
---
 config.mak.uname | 1 +
 1 file changed, 1 insertion(+)

diff --git a/config.mak.uname b/config.mak.uname
index 3ee7da0e23..378ca0a582 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -233,6 +233,7 @@ ifeq ($(uname_S),OpenBSD)
 	HAVE_BSD_SYSCTL = YesPlease
 	HAVE_BSD_KERN_PROC_SYSCTL = YesPlease
 	PROCFS_EXECUTABLE_PATH = /proc/curproc/file
+	FREAD_READS_DIRECTORIES = UnfortunatelyYes
 endif
 ifeq ($(uname_S),MirBSD)
 	NO_STRCASESTR = YesPlease
-- 
2.20.0.rc2


^ permalink raw reply related

* Re: [PATCH 1/2] config.mak.uname: OpenBSD uses BSD semantics with fread for directories
From: Carlo Arenas @ 2018-12-02  2:51 UTC (permalink / raw)
  To: git
In-Reply-To: <20181202024320.65160-1-carenas@gmail.com>

FWIW this patch doesn't have any other siblings and subject should had
been just [PATCH]; apologize for the confusion and the spam (including
that other duplicated email, and most likely this one)

Carlo

^ permalink raw reply

* [L10N] Kickoff for Git 2.20.0 round 3
From: Jiang Xin @ 2018-12-02  3:17 UTC (permalink / raw)
  To: Alexander Shopov, Jordi Mas, Ralf Thielow, Christopher Díaz,
	Jean-Noël Avila, Marco Paolone, Gwan-gyeong Mun,
	Vasco Almeida, Dimitriy Ryazantcev, Peter Krefting,
	Trần Ngọc Quân, Jiang Xin
  Cc: Git List

Hi,

Git v2.20.0-rc2 has been released, and there are 5 new messages need to
be translated. So let's start new round of l10n for Git 2.20.0. See commit:

    l10n: git.pot: v2.20.0 round 3 (5 new, 3 removed)

    Generate po/git.pot from v2.20.0-rc2 for git v2.20.0 l10n round 3.

    Signed-off-by: Jiang Xin <worldhello.net@gmail.com>

You can get it from the usual place:

    https://github.com/git-l10n/git-po/

As how to update your XX.po and help to translate Git, please see
"Updating a XX.po file" and other sections in “po/README" file.

--
Jiang Xin

^ permalink raw reply

* Re: [PATCH] rebase -i: introduce the 'test' command
From: Jeff King @ 2018-12-02  3:31 UTC (permalink / raw)
  To: Eric Sunshine; +Cc: Johannes Schindelin, paul.morelle, Git List
In-Reply-To: <CAPig+cT6yo+wBHhkaAFK1REisOdtZsJNWEhoShcspzQsFeiWxQ@mail.gmail.com>

On Sat, Dec 01, 2018 at 09:28:47PM -0500, Eric Sunshine wrote:

> On Sat, Dec 1, 2018 at 3:02 PM Jeff King <peff@peff.net> wrote:
> > On Thu, Nov 29, 2018 at 09:32:48AM +0100, Johannes Schindelin wrote:
> > > In reality, I think that it would even make sense to change the default to
> > > reschedule failed `exec` commands. Which is why I suggested to also add a
> > > config option.
> >
> > I sometimes add "x false" to the top of the todo list to stop and create
> > new commits before the first one. That would be awkward if I could never
> > get past that line. However, I think elsewhere a "pause" line has been
> > discussed, which would serve the same purpose.
> 
> Are you thinking of the "break" command (not "pause") which Dscho
> already added[1]?
> 
> [1]: 71f82465b1 (rebase -i: introduce the 'break' command, 2018-10-12)

Yes, thanks (as you can see, I haven't actually used it yet ;) ).

-Peff

^ permalink raw reply

* Re: [RFC] git clean --local
From: Junio C Hamano @ 2018-12-02  4:43 UTC (permalink / raw)
  To: Cameron Boehmer; +Cc: git
In-Reply-To: <xmqqsgzgiyk2.fsf@gitster-ct.c.googlers.com>

Junio C Hamano <gitster@pobox.com> writes:

> Cameron Boehmer <cameron.boehmer@gmail.com> writes:
>
>> 1) add a new flag
>> -l, --local
>>     Do not consult git config --global core.excludesFile in
>> determining what files git ignores. This is useful in conjunction with
>> -x/-X to preserve user files while removing build artifacts.
> ...
> But it might be useful as an option that affects any "git" command,
> e.g. "git --local-config-only clean".  I dunno.

If you only want to say "there is no global excludes file", perhaps

    $ git -c core.excludesFile=/dev/null clean -x

may be sufficient, so for that particular use case, there is no need
to introduce a new command, I'd think.

In the longer term, however, I think we would want to introduce a
distinction among ignored files---we only support "ignored and
expendable" class, but not "ignored but precious" class.  With the
latter class introduced, it would make sense for "git clean -x/-X"
to notice that a path is ignored but precious and keep it.  If a
dir/foo is ignored, dir/bar is tracked in commit A that is currently
checked out, and there is no dir/ directory in commit B, checking
out commit B would remove dir/foo (because the last tracked file in
the directory goes away and all remaining files in the directory
would be ignored but expendable).  But if we introduced a new
"ignored but precious" class and made dir/foo a member of such a
class, then you will be prevented from checkout out B until you do
something about dir/foo that is now "precious".





^ permalink raw reply

* Confusing inconsistent option syntax
From: Robert White @ 2018-12-02 10:07 UTC (permalink / raw)
  To: git

`git log --pretty short` gives the error message "ambiguous argument
'short'". To get the expected result, you need to use `git log
--pretty=short`. However, `git log --since yesterday` and `git log
--since=yesterday` both work as expected.

When is an = needed? What is the reason for these inconsistencies?

---
Robert White

^ permalink raw reply

* Re: Confusing inconsistent option syntax
From: Duy Nguyen @ 2018-12-02 10:18 UTC (permalink / raw)
  To: rjwhite2453; +Cc: Git Mailing List
In-Reply-To: <20181202100747.GA5019@laptop>

On Sun, Dec 2, 2018 at 11:13 AM Robert White <rjwhite2453@gmail.com> wrote:
>
> `git log --pretty short` gives the error message "ambiguous argument
> 'short'". To get the expected result, you need to use `git log
> --pretty=short`. However, `git log --since yesterday` and `git log
> --since=yesterday` both work as expected.
>
> When is an = needed? What is the reason for these inconsistencies?

--pretty can take no arguments. --pretty alone is the same as
--pretty=medium. --since always needs an argument.
-- 
Duy

^ permalink raw reply

* Re: [PATCH 8/9] sha1-file: use loose object cache for quick existence check
From: René Scharfe @ 2018-12-02 10:52 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason, Jeff King
  Cc: Geert Jansen, Junio C Hamano, git@vger.kernel.org, Takuto Ikuta
In-Reply-To: <878t1x2t3e.fsf@evledraar.gmail.com>

Am 13.11.2018 um 11:02 schrieb Ævar Arnfjörð Bjarmason:
> 
> On Mon, Nov 12 2018, Ævar Arnfjörð Bjarmason wrote:
> 
>> On Mon, Nov 12 2018, Ævar Arnfjörð Bjarmason wrote:
>>
>>> I get:
>>>
>>>     Test                                             origin/master     peff/jk/loose-cache      avar/check-collisions-config
>>>     ------------------------------------------------------------------------------------------------------------------------
>>>     0008.2: index-pack with 256*1 loose objects      4.31(0.55+0.18)   0.41(0.40+0.02) -90.5%   0.23(0.36+0.01) -94.7%
>>>     0008.3: index-pack with 256*10 loose objects     4.37(0.45+0.21)   0.45(0.40+0.02) -89.7%   0.25(0.38+0.01) -94.3%
>>>     0008.4: index-pack with 256*100 loose objects    4.47(0.53+0.23)   0.67(0.63+0.02) -85.0%   0.24(0.38+0.01) -94.6%
>>>     0008.5: index-pack with 256*250 loose objects    5.01(0.67+0.30)   1.04(0.98+0.06) -79.2%   0.24(0.37+0.01) -95.2%
>>>     0008.6: index-pack with 256*500 loose objects    5.11(0.57+0.21)   1.81(1.70+0.09) -64.6%   0.25(0.38+0.01) -95.1%
>>>     0008.7: index-pack with 256*750 loose objects    5.12(0.60+0.22)   2.54(2.38+0.14) -50.4%   0.24(0.38+0.01) -95.3%
>>>     0008.8: index-pack with 256*1000 loose objects   4.52(0.52+0.21)   3.36(3.17+0.17) -25.7%   0.23(0.36+0.01) -94.9%
>>>
>>> I then hacked it to test against git.git, but skipped origin/master for
>>> that one because it takes *ages*. So just mine v.s. yours:
>>>
>>>     $ GIT_PERF_REPEAT_COUNT=3 GIT_PERF_MAKE_OPTS='-j56 CFLAGS="-O3"' ./run peff/jk/loose-cache avar/check-collisions-config p0008-index-pack.sh
>>>     [...]
>>>     Test                                             peff/jk/loose-cache   avar/check-collisions-config
>>>     ---------------------------------------------------------------------------------------------------
>>>     0008.2: index-pack with 256*1 loose objects      12.57(28.72+0.61)     12.68(29.36+0.62) +0.9%
>>>     0008.3: index-pack with 256*10 loose objects     12.77(28.75+0.61)     12.50(28.88+0.56) -2.1%
>>>     0008.4: index-pack with 256*100 loose objects    13.20(29.49+0.66)     12.38(28.58+0.60) -6.2%
>>>     0008.5: index-pack with 256*250 loose objects    14.10(30.59+0.64)     12.54(28.22+0.57) -11.1%
>>>     0008.6: index-pack with 256*500 loose objects    14.48(31.06+0.74)     12.43(28.59+0.60) -14.2%
>>>     0008.7: index-pack with 256*750 loose objects    15.31(31.91+0.74)     12.67(29.23+0.64) -17.2%
>>>     0008.8: index-pack with 256*1000 loose objects   16.34(32.84+0.76)     13.11(30.19+0.68) -19.8%
>>>
>>> So not much of a practical difference perhaps. But then again this isn't
>>> a very realistic test case of anything. Rarely are you going to push a
>>> history of something the size of git.git into a repo with this many
>>> loose objects.
>>>
>>> Using sha1collisiondetection.git is I think the most realistic scenario,
>>> i.e. you'll often end up fetching/pushing something roughly the size of
>>> its entire history on a big repo, and with it:
>>>
>>>     Test                                             peff/jk/loose-cache   avar/check-collisions-config
>>>     ---------------------------------------------------------------------------------------------------
>>>     0008.2: index-pack with 256*1 loose objects      0.16(0.04+0.01)       0.05(0.03+0.00) -68.8%
>>>     0008.3: index-pack with 256*10 loose objects     0.19(0.04+0.02)       0.05(0.02+0.00) -73.7%
>>>     0008.4: index-pack with 256*100 loose objects    0.32(0.17+0.02)       0.04(0.02+0.00) -87.5%
>>>     0008.5: index-pack with 256*250 loose objects    0.57(0.41+0.03)       0.04(0.02+0.00) -93.0%
>>>     0008.6: index-pack with 256*500 loose objects    1.02(0.83+0.06)       0.04(0.03+0.00) -96.1%
>>>     0008.7: index-pack with 256*750 loose objects    1.47(1.24+0.10)       0.04(0.02+0.00) -97.3%
>>>     0008.8: index-pack with 256*1000 loose objects   1.94(1.70+0.10)       0.04(0.02+0.00) -97.9%
>>>
>>> As noted in previous threads I have an in-house monorepo where (due to
>>> expiry policies) loose objects hover around the 256*250 mark.
>>>
>>> The script, which is hacky as hell and takes shortcuts not to re-create
>>> the huge fake loose object collection every time (takes ages). Perhaps
>>> you're interested in incorporating some version of this into a v2. To be
>>> useful it should take some target path as an env variable.
>>
>> I forgot perhaps the most useful metric. Testing against origin/master
>> too on the sha1collisiondetection.git repo, which as noted above I think
>> is a good stand-in for making a medium sized push to a big repo. This
>> shows when the loose cache becomes counterproductive:
>>
>>     Test                                             origin/master     peff/jk/loose-cache       avar/check-collisions-config
>>     -------------------------------------------------------------------------------------------------------------------------
>>     0008.2: index-pack with 256*1 loose objects      0.42(0.04+0.03)   0.17(0.04+0.00) -59.5%    0.04(0.03+0.00) -90.5%
>>     0008.3: index-pack with 256*10 loose objects     0.49(0.04+0.03)   0.19(0.04+0.01) -61.2%    0.04(0.02+0.00) -91.8%
>>     0008.4: index-pack with 256*100 loose objects    0.49(0.04+0.04)   0.33(0.18+0.01) -32.7%    0.05(0.02+0.00) -89.8%
>>     0008.5: index-pack with 256*250 loose objects    0.54(0.03+0.04)   0.59(0.43+0.02) +9.3%     0.04(0.02+0.01) -92.6%
>>     0008.6: index-pack with 256*500 loose objects    0.49(0.04+0.03)   1.04(0.83+0.07) +112.2%   0.04(0.02+0.00) -91.8%
>>     0008.7: index-pack with 256*750 loose objects    0.56(0.04+0.05)   1.50(1.28+0.08) +167.9%   0.04(0.02+0.00) -92.9%
>>     0008.8: index-pack with 256*1000 loose objects   0.54(0.05+0.03)   1.95(1.68+0.13) +261.1%   0.04(0.02+0.00) -92.6%
>>
>> I still think it's best to take this patch series since it's unlikely
>> we're making anything worse in practice, the >50k objects case is a
>> really high number, which I don't think is worth worrying about.
>>
>> But I am somewhat paranoid about the potential performance
>> regression. I.e. this is me testing against a really expensive and
>> relatively well performing NetApp NFS device where the ping stats are:
>>
>>     rtt min/avg/max/mdev = 0.155/0.396/1.387/0.349 ms
>>
>> So I suspect this might get a lot worse for setups which don't enjoy the
>> same performance or network locality.
> 
> I tried this with the same filer mounted from another DC with ~10x the
> RTT:
> 
>     rtt min/avg/max/mdev = 11.553/11.618/11.739/0.121 ms
> 
> But otherwise the same setup (same machine type/specs mounting it). It
> had the opposite results of what I was expecting:
> 
>     Test                                             origin/master     peff/jk/loose-cache      avar/check-collisions-config
>     ------------------------------------------------------------------------------------------------------------------------
>     0008.2: index-pack with 256*1 loose objects      7.78(0.04+0.03)   2.75(0.03+0.01) -64.7%   0.40(0.02+0.00) -94.9%
>     0008.3: index-pack with 256*10 loose objects     7.75(0.04+0.04)   2.77(0.05+0.01) -64.3%   0.40(0.02+0.00) -94.8%
>     0008.4: index-pack with 256*100 loose objects    7.75(0.05+0.02)   2.91(0.18+0.01) -62.5%   0.40(0.02+0.00) -94.8%
>     0008.5: index-pack with 256*250 loose objects    7.73(0.04+0.04)   3.19(0.43+0.02) -58.7%   0.40(0.02+0.00) -94.8%
>     0008.6: index-pack with 256*500 loose objects    7.73(0.04+0.04)   3.64(0.83+0.05) -52.9%   0.40(0.02+0.00) -94.8%
>     0008.7: index-pack with 256*750 loose objects    7.73(0.04+0.02)   4.14(1.29+0.07) -46.4%   0.40(0.02+0.00) -94.8%
>     0008.8: index-pack with 256*1000 loose objects   7.73(0.04+0.03)   4.55(1.72+0.09) -41.1%   0.40(0.02+0.01) -94.8%
> 
> I.e. there the cliff of where the cache becomes counterproductive comes
> much later, not earlier. The sha1collisiondetection.git repo has 418
> objects.
> 
> So is it cheaper to fill a huge cache than look up those 418? I don't
> know, haven't dug. But so far what this suggests is that we're helping
> slow FSs to the detriment of faster ones.
> 
> So here's the same test not against NFS, but the local ext4 fs (CO7;
> Linux 3.10) for sha1collisiondetection.git:
> 
>     Test                                             origin/master     peff/jk/loose-cache        avar/check-collisions-config
>     --------------------------------------------------------------------------------------------------------------------------
>     0008.2: index-pack with 256*1 loose objects      0.02(0.02+0.00)   0.02(0.02+0.01) +0.0%      0.02(0.02+0.00) +0.0%
>     0008.3: index-pack with 256*10 loose objects     0.02(0.02+0.00)   0.03(0.03+0.00) +50.0%     0.02(0.02+0.00) +0.0%
>     0008.4: index-pack with 256*100 loose objects    0.02(0.02+0.00)   0.17(0.16+0.01) +750.0%    0.02(0.02+0.00) +0.0%
>     0008.5: index-pack with 256*250 loose objects    0.02(0.02+0.00)   0.43(0.40+0.03) +2050.0%   0.02(0.02+0.00) +0.0%
>     0008.6: index-pack with 256*500 loose objects    0.02(0.02+0.00)   0.88(0.80+0.09) +4300.0%   0.02(0.02+0.00) +0.0%
>     0008.7: index-pack with 256*750 loose objects    0.02(0.02+0.00)   1.35(1.27+0.09) +6650.0%   0.02(0.02+0.00) +0.0%
>     0008.8: index-pack with 256*1000 loose objects   0.02(0.02+0.00)   1.83(1.70+0.14) +9050.0%   0.02(0.02+0.00) +0.0%
> 
> And for mu.git, a ~20k object repo:
> 
>     Test                                             origin/master     peff/jk/loose-cache       avar/check-collisions-config
>     -------------------------------------------------------------------------------------------------------------------------
>     0008.2: index-pack with 256*1 loose objects      0.59(0.91+0.06)   0.58(0.93+0.03) -1.7%     0.57(0.89+0.04) -3.4%
>     0008.3: index-pack with 256*10 loose objects     0.59(0.91+0.07)   0.59(0.92+0.03) +0.0%     0.57(0.89+0.03) -3.4%
>     0008.4: index-pack with 256*100 loose objects    0.59(0.91+0.05)   0.81(1.13+0.04) +37.3%    0.58(0.91+0.04) -1.7%
>     0008.5: index-pack with 256*250 loose objects    0.59(0.91+0.05)   1.23(1.51+0.08) +108.5%   0.58(0.91+0.04) -1.7%
>     0008.6: index-pack with 256*500 loose objects    0.59(0.90+0.06)   1.96(2.20+0.12) +232.2%   0.58(0.91+0.04) -1.7%
>     0008.7: index-pack with 256*750 loose objects    0.59(0.92+0.05)   2.72(2.92+0.17) +361.0%   0.58(0.90+0.04) -1.7%
>     0008.8: index-pack with 256*1000 loose objects   0.59(0.90+0.06)   3.50(3.67+0.21) +493.2%   0.57(0.90+0.04) -3.4%

OK, here's another theory: The cache scales badly with increasing
numbers of loose objects because it sorts the array 256 times as it is
filled.  Loading it fully and sorting once would help, as would using
one array per subdirectory.

We can simulate the oid_array operations with test-sha1-array.  It has
no explicit sort command, but we can use for_each_unique for that; we
just have to add 127.5 extra calls (that don't sort) to get the same
amount of output in the two latter cases, to be able to compare just
the sort time:

  for command in '
      foreach (0..255) {
        $subdir = sprintf("%02x", $_);
        foreach (1..$ARGV[0]) {
          printf("append %s%038d\n", $subdir, $_);
        }
        # intermediate sort
        print "for_each_unique\n";
      }
    ' '
      foreach (0..255) {
        $subdir = sprintf("%02x", $_);
        foreach (1..$ARGV[0]) {
          printf("append %s%038d\n", $subdir, $_);
        }
      }
      # sort once at the end
      print "for_each_unique\n";
      # ... and generate roughly the same amount of output
      foreach (0..127) {
        print "for_each_unique\n";
      }
    ' '
      foreach (0..255) {
        $subdir = sprintf("%02x", $_);
        foreach (1..$ARGV[0]) {
          printf("append %s%038d\n", $subdir, $_);
        }
        # sort each subdirectory separately
        print "for_each_unique\n";
	# ... and generate roughly the same amount of output
        foreach (0..127) {
          print "for_each_unique\n";
        }
        print "clear\n";
      }
    '
  do
    time perl -e "$command" 1000 | t/helper/test-tool sha1-array | wc -l
  done

My results:

  32896000

  real    0m6.521s
  user    0m5.269s
  sys     0m2.234s
  33024000

  real    0m3.464s
  user    0m2.178s
  sys     0m2.251s
  33024000

  real    0m3.353s
  user    0m2.179s
  sys     0m1.939s

So this adds up to a significant amount of time spent on sorting.  Here's
a patch, on top of next:

-- >8 --
Subject: [PATCH] object-store: use one oid_array per subdirectory for loose cache

The loose objects cache is filled one subdirectory at a time as needed.
It is stored in an oid_array, which has to be resorted after each add
operation.  So when querying a wide range of objects the array needs to
be resorted up to 256 times -- once for each subdirectory.  This is not
efficient.

Use one oid_array for each subdirectory.  This ensures that entries have
to only be sorted once.

It speeds up cache operations in a repository with ca. 100 loose
objects per subdirectory (it's used for collision checks for the
placeholders %h, %t and %p):

$ git count-objects
25805 objects, 302452 kilobytes

$ (cd t/perf && ./run HEAD^ HEAD ./p4205-log-pretty-formats.sh)
[...]
Test                        HEAD^             HEAD
--------------------------------------------------------------------
4205.1: log with %H         0.56(0.52+0.04)   0.57(0.54+0.02) +1.8%
4205.2: log with %h         0.92(0.86+0.06)   0.66(0.62+0.04) -28.3%
4205.3: log with %T         0.56(0.52+0.04)   0.57(0.55+0.01) +1.8%
4205.4: log with %t         0.92(0.88+0.04)   0.67(0.62+0.05) -27.2%
4205.5: log with %P         0.57(0.54+0.02)   0.57(0.54+0.03) +0.0%
4205.6: log with %p         0.92(0.86+0.05)   0.64(0.60+0.04) -30.4%
4205.7: log with %h-%h-%h   1.02(0.98+0.04)   0.72(0.69+0.03) -29.4%

Reported-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Rene Scharfe <l.s.r@web.de>
---
This patch goes on top of next.  p4205 is better suited to show the
cost of sorting in the loose objects cache than the slooow index-pack.
This change does fix the index-pack regression on ext4 for me as well,
though.  Not sure it warrants adding a loose objects test to p5302.

 object-store.h | 2 +-
 object.c       | 5 ++++-
 packfile.c     | 4 +++-
 sha1-file.c    | 5 +++--
 sha1-name.c    | 8 +++++---
 5 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/object-store.h b/object-store.h
index 8dceed0f31..ee67a50980 100644
--- a/object-store.h
+++ b/object-store.h
@@ -20,7 +20,7 @@ struct object_directory {
 	 * Be sure to call odb_load_loose_cache() before using.
 	 */
 	char loose_objects_subdir_seen[256];
-	struct oid_array loose_objects_cache;
+	struct oid_array loose_objects_cache[256];
 
 	/*
 	 * Path to the alternative object store. If this is a relative path,
diff --git a/object.c b/object.c
index c29a97a7e9..965493ba76 100644
--- a/object.c
+++ b/object.c
@@ -484,8 +484,11 @@ struct raw_object_store *raw_object_store_new(void)
 
 static void free_object_directory(struct object_directory *odb)
 {
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(odb->loose_objects_cache); i++)
+		oid_array_clear(&odb->loose_objects_cache[i]);
 	free(odb->path);
-	oid_array_clear(&odb->loose_objects_cache);
 	free(odb);
 }
 
diff --git a/packfile.c b/packfile.c
index 56496bb425..3ef6a241b7 100644
--- a/packfile.c
+++ b/packfile.c
@@ -995,7 +995,9 @@ void reprepare_packed_git(struct repository *r)
 	struct object_directory *odb;
 
 	for (odb = r->objects->odb; odb; odb = odb->next) {
-		oid_array_clear(&odb->loose_objects_cache);
+		int i;
+		for (i = 0; i < ARRAY_SIZE(odb->loose_objects_cache); i++)
+			oid_array_clear(&odb->loose_objects_cache[i]);
 		memset(&odb->loose_objects_subdir_seen, 0,
 		       sizeof(odb->loose_objects_subdir_seen));
 	}
diff --git a/sha1-file.c b/sha1-file.c
index 05f63dfd4e..d2f5e65865 100644
--- a/sha1-file.c
+++ b/sha1-file.c
@@ -933,7 +933,8 @@ static int quick_has_loose(struct repository *r,
 	prepare_alt_odb(r);
 	for (odb = r->objects->odb; odb; odb = odb->next) {
 		odb_load_loose_cache(odb, subdir_nr);
-		if (oid_array_lookup(&odb->loose_objects_cache, &oid) >= 0)
+		if (oid_array_lookup(&odb->loose_objects_cache[subdir_nr],
+				     &oid) >= 0)
 			return 1;
 	}
 	return 0;
@@ -2173,7 +2174,7 @@ void odb_load_loose_cache(struct object_directory *odb, int subdir_nr)
 	for_each_file_in_obj_subdir(subdir_nr, &buf,
 				    append_loose_object,
 				    NULL, NULL,
-				    &odb->loose_objects_cache);
+				    &odb->loose_objects_cache[subdir_nr]);
 	odb->loose_objects_subdir_seen[subdir_nr] = 1;
 	strbuf_release(&buf);
 }
diff --git a/sha1-name.c b/sha1-name.c
index b24502811b..fdb22147b2 100644
--- a/sha1-name.c
+++ b/sha1-name.c
@@ -94,14 +94,16 @@ static void find_short_object_filename(struct disambiguate_state *ds)
 	     odb && !ds->ambiguous;
 	     odb = odb->next) {
 		int pos;
+		struct oid_array *loose_subdir_objects;
 
 		odb_load_loose_cache(odb, subdir_nr);
-		pos = oid_array_lookup(&odb->loose_objects_cache, &ds->bin_pfx);
+		loose_subdir_objects = &odb->loose_objects_cache[subdir_nr];
+		pos = oid_array_lookup(loose_subdir_objects, &ds->bin_pfx);
 		if (pos < 0)
 			pos = -1 - pos;
-		while (!ds->ambiguous && pos < odb->loose_objects_cache.nr) {
+		while (!ds->ambiguous && pos < loose_subdir_objects->nr) {
 			const struct object_id *oid;
-			oid = odb->loose_objects_cache.oid + pos;
+			oid = loose_subdir_objects->oid + pos;
 			if (!match_sha(ds->len, ds->bin_pfx.hash, oid->hash))
 				break;
 			update_candidates(ds, oid);
-- 
2.19.2

^ permalink raw reply related

* easy way to demonstrate length of colliding SHA-1 prefixes?
From: Robert P. J. Day @ 2018-12-02 11:50 UTC (permalink / raw)
  To: Git Mailing list


  as part of an upcoming git class i'm delivering, i thought it would
be amusing to demonstrate the maximum length of colliding SHA-1
prefixes in a repository (in my case, i use the linux kernel git repo
for most of my examples).

  is there a way to display the objects in the object database that
clash in the longest object name SHA-1 prefix; i mean, short of
manually listing all object names, running that through cut and sort
and uniq and ... you get the idea.

  is there a cute way to do that? thanks.

rday

^ permalink raw reply

* Re: easy way to demonstrate length of colliding SHA-1 prefixes?
From: Ævar Arnfjörð Bjarmason @ 2018-12-02 13:23 UTC (permalink / raw)
  To: Robert P. J. Day; +Cc: Git Mailing list
In-Reply-To: <alpine.LFD.2.21.1812020647440.32023@localhost.localdomain>


On Sun, Dec 02 2018, Robert P. J. Day wrote:

>   as part of an upcoming git class i'm delivering, i thought it would
> be amusing to demonstrate the maximum length of colliding SHA-1
> prefixes in a repository (in my case, i use the linux kernel git repo
> for most of my examples).
>
>   is there a way to display the objects in the object database that
> clash in the longest object name SHA-1 prefix; i mean, short of
> manually listing all object names, running that through cut and sort
> and uniq and ... you get the idea.
>
>   is there a cute way to do that? thanks.

You'll always need to list them all. It's inherently an operation where
for each SHA-1 you need to search for other ones with that prefix up to
a given length.

Perhaps you've missed that you can use --abbrev=N for this, and just
grep for things that are loger than that N, e.g. for linux.git:

    git log --oneline --abbrev=10 --pretty=format:%h |
    grep -E -v '^.{10}$' |
    perl -pe 's/^(.{10}).*/$1/'

This will list the 4 objects that need more than 10 characters to be
shown unambiguously. If you then "git cat-file -t" them you'll get the
disambiguation help.

^ permalink raw reply

* Re: [RFC] git clean --local
From: Ævar Arnfjörð Bjarmason @ 2018-12-02 13:25 UTC (permalink / raw)
  To: Cameron Boehmer; +Cc: git
In-Reply-To: <CAM+q9MeVS1e11vzu+-RP-i5NhSsnRz=x21q3gcGy8L62yceiMw@mail.gmail.com>


On Sat, Dec 01 2018, Cameron Boehmer wrote:

> 1) add a new flag
> -l, --local
>     Do not consult git config --global core.excludesFile in
> determining what files git ignores. This is useful in conjunction with
> -x/-X to preserve user files while removing build artifacts.

Or perhaps a general flag to ignore configuration would be useful for
such cases, see
https://public-inbox.org/git/87zhtqvm66.fsf@evledraar.gmail.com/

^ permalink raw reply

* Re: [PATCH v3 01/42] parse-options: support --git-completion-helper
From: Ævar Arnfjörð Bjarmason @ 2018-12-02 13:41 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy
  Cc: git, Junio C Hamano, Eric Sunshine, SZEDER Gábor
In-Reply-To: <20180209110221.27224-2-pclouds@gmail.com>


On Fri, Feb 09 2018, Nguyễn Thái Ngọc Duy wrote:

> +static int show_gitcomp(struct parse_opt_ctx_t *ctx,
> +			const struct option *opts)
> +{

Says it returns 'static int'...

> [...]
> +	exit(0);

Then just exits...

> +		/* lone --git-completion-helper is asked by git-completion.bash */
> +		if (ctx->total == 1 && !strcmp(arg + 1, "-git-completion-helper"))
> +			return show_gitcomp(ctx, options);

This return value is never used.

Whine from SunCC (not fatal, also this was in v2.17.0 so no need to fix
before 2.20, just saw this now):

    "parse-options.c", line 520: warning: Function has no return
    statement : show_gitcomp

^ permalink raw reply

* Re: easy way to demonstrate length of colliding SHA-1 prefixes?
From: Robert P. J. Day @ 2018-12-02 16:24 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason; +Cc: Git Mailing list
In-Reply-To: <87y398uknn.fsf@evledraar.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 1835 bytes --]

On Sun, 2 Dec 2018, Ævar Arnfjörð Bjarmason wrote:

> On Sun, Dec 02 2018, Robert P. J. Day wrote:
>
> >   as part of an upcoming git class i'm delivering, i thought it
> > would be amusing to demonstrate the maximum length of colliding
> > SHA-1 prefixes in a repository (in my case, i use the linux kernel
> > git repo for most of my examples).
> >
> >   is there a way to display the objects in the object database
> > that clash in the longest object name SHA-1 prefix; i mean, short
> > of manually listing all object names, running that through cut and
> > sort and uniq and ... you get the idea.
> >
> >   is there a cute way to do that? thanks.
>
> You'll always need to list them all. It's inherently an operation
> where for each SHA-1 you need to search for other ones with that
> prefix up to a given length.

  i assumed as much, just wasn't sure about the esoteric dark corners
of git i've never gotten to yet.

> Perhaps you've missed that you can use --abbrev=N for this, and just
> grep for things that are loger than that N, e.g. for linux.git:
>
>     git log --oneline --abbrev=10 --pretty=format:%h |
>     grep -E -v '^.{10}$' |
>     perl -pe 's/^(.{10}).*/$1/'
>
> This will list the 4 objects that need more than 10 characters to be
> shown unambiguously. If you then "git cat-file -t" them you'll get
> the disambiguation help.

  that's pretty close to what i came up with, thanks.

rday

-- 

========================================================================
Robert P. J. Day                                 Ottawa, Ontario, CANADA
                  http://crashcourse.ca/dokuwiki

Twitter:                                       http://twitter.com/rpjday
LinkedIn:                               http://ca.linkedin.com/in/rpjday
========================================================================

^ permalink raw reply

* "git add -p" versus "git add -i", followed by "p"
From: Robert P. J. Day @ 2018-12-02 16:30 UTC (permalink / raw)
  To: Git Mailing list


  testing adding by patch for the very first time (i've just never
needed this), and reading the "progit" book and reading the man page,
and the impression i'm getting is that running "git add -p" (going
straight to patch mode) is supposed to be equivalent to running "git
add -i", then typing "p" to switch to patch mode.

  that is most emphatically not what i'm seeing. if i run "git add
-p", then i get to what i expect -- the patch subsystem:

  $ git add -p
  diff --git a/README.asc b/README.asc
  index fa40bad..840e85b 100644
  --- a/README.asc
  +++ b/README.asc
  @@ -1,3 +1,9 @@
  +change 1
  +
  +
  +
  +
  +
   = Pro Git, Second Edition

   Welcome to the second edition of the Pro Git book.
  Stage this hunk [y,n,q,a,d,j,J,g,/,e,?]?

but if i start with "git add -i", there seems to be no way to get to
patch mode -- certainly "p" doesn't do it. am i stupidly missing
something trivial? is the explanation misleading or inncomplete?

rday

-- 

========================================================================
Robert P. J. Day                                 Ottawa, Ontario, CANADA
                  http://crashcourse.ca/dokuwiki

Twitter:                                       http://twitter.com/rpjday
LinkedIn:                               http://ca.linkedin.com/in/rpjday
========================================================================

^ permalink raw reply

* Re: "git add -p" versus "git add -i", followed by "p"
From: SZEDER Gábor @ 2018-12-02 16:56 UTC (permalink / raw)
  To: Robert P. J. Day; +Cc: Git Mailing list
In-Reply-To: <alpine.LFD.2.21.1812021124350.5509@localhost.localdomain>

On Sun, Dec 02, 2018 at 11:30:19AM -0500, Robert P. J. Day wrote:
> 
>   testing adding by patch for the very first time (i've just never
> needed this), and reading the "progit" book and reading the man page,
> and the impression i'm getting is that running "git add -p" (going
> straight to patch mode) is supposed to be equivalent to running "git
> add -i", then typing "p" to switch to patch mode.
> 
>   that is most emphatically not what i'm seeing. if i run "git add
> -p", then i get to what i expect -- the patch subsystem:
> 
>   $ git add -p
>   diff --git a/README.asc b/README.asc
>   index fa40bad..840e85b 100644
>   --- a/README.asc
>   +++ b/README.asc
>   @@ -1,3 +1,9 @@
>   +change 1
>   +
>   +
>   +
>   +
>   +
>    = Pro Git, Second Edition
> 
>    Welcome to the second edition of the Pro Git book.
>   Stage this hunk [y,n,q,a,d,j,J,g,/,e,?]?
> 
> but if i start with "git add -i", there seems to be no way to get to
> patch mode -- certainly "p" doesn't do it. am i stupidly missing
> something trivial? is the explanation misleading or inncomplete?

Worksforme™:

  $ echo "New content" >>README.md 
  $ echo "New content" >>t/README
  $ echo "New content" >>contrib//README
  $ git add -i
             staged     unstaged path
    1:    unchanged        +1/-0 README.md
    2:    unchanged        +1/-0 contrib/README
    3:    unchanged        +1/-0 t/README
  
  *** Commands ***
    1: status       2: update       3: revert       4: add untracked
    5: patch        6: diff         7: quit         8: help
  What now> p
             staged     unstaged path
    1:    unchanged        +1/-0 README.md
    2:    unchanged        +1/-0 contrib/README
    3:    unchanged        +1/-0 t/README
  Patch update>> 1
             staged     unstaged path
  * 1:    unchanged        +1/-0 README.md
    2:    unchanged        +1/-0 contrib/README
    3:    unchanged        +1/-0 t/README
  Patch update>> 2
             staged     unstaged path
  * 1:    unchanged        +1/-0 README.md
  * 2:    unchanged        +1/-0 contrib/README
    3:    unchanged        +1/-0 t/README
  Patch update>> 

Here I hit enter.  Did you?

  diff --git a/README.md b/README.md
  index f920a42fad..63dee5cfc3 100644
  --- a/README.md
  +++ b/README.md
  @@ -62,3 +62,4 @@ and the name as (depending on your mood):
   [Documentation/giteveryday.txt]: Documentation/giteveryday.txt
   [Documentation/gitcvs-migration.txt]:
  Documentation/gitcvs-migration.txt
   [Documentation/SubmittingPatches]: Documentation/SubmittingPatches
  +New content
  Stage this hunk [y,n,q,a,d,e,?]? y
  
  diff --git a/contrib/README b/contrib/README
  index 05f291c1f1..2b152dfcff 100644
  --- a/contrib/README
  +++ b/contrib/README
  @@ -41,3 +41,4 @@ submit a patch to create a subdirectory of contrib/
  and put your
   stuff there.
   
   -jc
  +New content
  Stage this hunk [y,n,q,a,d,e,?]? n
  
  *** Commands ***
    1: status       2: update       3: revert       4: add untracked
    5: patch        6: diff         7: quit         8: help
  What now> q
  Bye.
  $ git diff --cached 
  diff --git a/README.md b/README.md
  index f920a42fad..63dee5cfc3 100644
  --- a/README.md
  +++ b/README.md
  @@ -62,3 +62,4 @@ and the name as (depending on your mood):
   [Documentation/giteveryday.txt]: Documentation/giteveryday.txt
   [Documentation/gitcvs-migration.txt]: Documentation/gitcvs-migration.txt
   [Documentation/SubmittingPatches]: Documentation/SubmittingPatches
  +New content
  $


Arguably the documentation could make it clear that the user can
choose multiple files at once, e.g.:

diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt
index c9623854bf..061f9cbb0d 100644
--- a/Documentation/git-add.txt
+++ b/Documentation/git-add.txt
@@ -317,9 +317,9 @@ add untracked::
 
 patch::
 
-  This lets you choose one path out of a 'status' like selection.
-  After choosing the path, it presents the diff between the index
-  and the working tree file and asks you if you want to stage
+  This lets you choose one or more paths out of a 'status' like selection.
+  After choosing the path(s), it presents the diff between the index
+  and the working tree file(s) and asks you if you want to stage
   the change of each hunk.  You can select one of the following
   options and type return:
 
And perhaps we could have a dedicated menu entry for "I'm done with
selecting paths"?  Dunno; I'm a 'git add -p' user myself.


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox