Git development
 help / color / mirror / Atom feed
* Re: [PATCH] mingw: allow hooks to be .exe files
From: Johannes Schindelin @ 2017-01-26 13:45 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqq37g6akfx.fsf@gitster.mtv.corp.google.com>

Hi Junio,

On Wed, 25 Jan 2017, Junio C Hamano wrote:

> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> 
> > This change is necessary to allow the files in .git/hooks/ to optionally
> > have the file extension `.exe` on Windows, as the file names are
> > hardcoded otherwise.
> 
> Hmph.  There is no longer ".com"?

No, no longer .com. You have to jump through hoops in this century to
build .com files.

> I briefly wondered if hooks/post-receive.{py,rb,...} would be good
> things to support, but I do not think we want to go there, primarily
> because we do not want to deal with "what happens when there are many?"

The answer is correct, the reasoning not. The reason why .exe is special:
it simply won't execute unless it has the .exe file extension. That is not
true for .py, .rb, etc

Ciao,
Johannes

^ permalink raw reply

* Re: SubmittingPatches: drop temporal reference for PGP signing
From: Cornelius Weig @ 2017-01-26 13:30 UTC (permalink / raw)
  To: Stefan Beller, Philip Oakley
  Cc: Johannes Sixt, bitte.keine.werbung.einwerfen, git@vger.kernel.org,
	Junio C Hamano, thomas.braun, John Keeping
In-Reply-To: <CAGZ79kaRdtKD7DNJRWXsyg07GbTM4OsKUmHHcFczEMJA1YK2KA@mail.gmail.com>


> 
> Yeah I agree. My patch was not the best shot by far.
> 

How about something along these lines? Does the forward reference
break the main line of thought too severly?

diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
index 08352de..c2b0cbe 100644
--- a/Documentation/SubmittingPatches
+++ b/Documentation/SubmittingPatches
@@ -216,12 +216,12 @@ that it will be postponed.
 Exception:  If your mailer is mangling patches then someone may ask
 you to re-send them using MIME, that is OK.
 
-Do not PGP sign your patch, at least for now.  Most likely, your
-maintainer or other people on the list would not have your PGP
-key and would not bother obtaining it anyway.  Your patch is not
-judged by who you are; a good patch from an unknown origin has a
-far better chance of being accepted than a patch from a known,
-respected origin that is done poorly or does incorrect things.
+Do not PGP sign your patch, but do sign-off your work as explained in (5).
+Most likely, your maintainer or other people on the list would not have your
+PGP key and would not bother obtaining it anyway. Your patch is not judged by
+who you are; a good patch from an unknown origin has a far better chance of
+being accepted than a patch from a known, respected origin that is done poorly
+or does incorrect things.
 
 If you really really really really want to do a PGP signed
 patch, format it as "multipart/signed", not a text/plain message
@@ -246,7 +246,7 @@ patch.
      *2* The mailing list: git@vger.kernel.org
 
 
-(5) Sign your work
+(5) Certify your work by signing off
 
 To improve tracking of who did what, we've borrowed the
 "sign-off" procedure from the Linux kernel project on patches

^ permalink raw reply related

* Re: [PATCH 2/2] ref-filter: add function to parse atoms from a nul-terminated string
From: SZEDER Gábor @ 2017-01-26 13:15 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, SZEDER Gábor
In-Reply-To: <20161207160923.7028-3-szeder.dev@gmail.com>

On Wed, Dec 7, 2016 at 5:09 PM, SZEDER Gábor <szeder.dev@gmail.com> wrote:
> ref-filter's parse_ref_filter_atom() function parses an atom between
> the start and end pointers it gets as arguments.  This is fine for two
> of its callers, which process '%(atom)' format specifiers and the end
> pointer comes directly from strchr() looking for the closing ')'.
> However, it's not quite so straightforward for its other two callers,
> which process sort specifiers given as plain nul-terminated strings.
> Especially not for ref_default_sorting(), which has the default
> hard-coded as a string literal, but can't use it directly, because a
> pointer to the end of that string literal is needed as well.
> The next patch will add yet another caller using a string literal.
>
> Add a helper function around parse_ref_filter_atom() to parse an atom
> up to the terminating nul, and call this helper in those two callers.
>
> Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
> ---
>  ref-filter.c | 8 ++------
>  ref-filter.h | 4 ++++
>  2 files changed, 6 insertions(+), 6 deletions(-)

Ping?

It looks like that this little two piece cleanup series fell on the floor.



> diff --git a/ref-filter.c b/ref-filter.c
> index dfadf577c..3c6fd4ba7 100644
> --- a/ref-filter.c
> +++ b/ref-filter.c
> @@ -1658,19 +1658,16 @@ void show_ref_array_item(struct ref_array_item *info, const char *format, int qu
>  /*  If no sorting option is given, use refname to sort as default */
>  struct ref_sorting *ref_default_sorting(void)
>  {
> -       static const char cstr_name[] = "refname";
> -
>         struct ref_sorting *sorting = xcalloc(1, sizeof(*sorting));
>
>         sorting->next = NULL;
> -       sorting->atom = parse_ref_filter_atom(cstr_name, cstr_name + strlen(cstr_name));
> +       sorting->atom = parse_ref_filter_atom_from_string("refname");
>         return sorting;
>  }
>
>  void parse_sorting_string(const char *arg, struct ref_sorting **sorting_tail)
>  {
>         struct ref_sorting *s;
> -       int len;
>
>         s = xcalloc(1, sizeof(*s));
>         s->next = *sorting_tail;
> @@ -1683,8 +1680,7 @@ void parse_sorting_string(const char *arg, struct ref_sorting **sorting_tail)
>         if (skip_prefix(arg, "version:", &arg) ||
>             skip_prefix(arg, "v:", &arg))
>                 s->version = 1;
> -       len = strlen(arg);
> -       s->atom = parse_ref_filter_atom(arg, arg+len);
> +       s->atom = parse_ref_filter_atom_from_string(arg);
>  }
>
>  int parse_opt_ref_sorting(const struct option *opt, const char *arg, int unset)
> diff --git a/ref-filter.h b/ref-filter.h
> index 49466a17d..1250537cf 100644
> --- a/ref-filter.h
> +++ b/ref-filter.h
> @@ -94,6 +94,10 @@ int filter_refs(struct ref_array *array, struct ref_filter *filter, unsigned int
>  void ref_array_clear(struct ref_array *array);
>  /*  Parse format string and sort specifiers */
>  int parse_ref_filter_atom(const char *atom, const char *ep);
> +static inline int parse_ref_filter_atom_from_string(const char *atom)
> +{
> +       return parse_ref_filter_atom(atom, atom+strlen(atom));
> +}
>  /*  Used to verify if the given format is correct and to parse out the used atoms */
>  int verify_ref_format(const char *format);
>  /*  Sort the given ref_array as per the ref_sorting provided */
> --
> 2.11.0.78.g5a2d011
>

^ permalink raw reply

* [PATCH v2] mingw: allow hooks to be .exe files
From: Johannes Schindelin @ 2017-01-26 12:21 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Jeff King
In-Reply-To: <dc388b2df3baee83972f007cd77a57410553a411.1485362812.git.johannes.schindelin@gmx.de>


This change is necessary to allow the files in .git/hooks/ to optionally
have the file extension `.exe` on Windows, as the file names are
hardcoded otherwise.

Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
---
Published-As: https://github.com/dscho/git/releases/tag/exe-as-hook-v2
Fetch-It-Via: git fetch https://github.com/dscho/git exe-as-hook-v2
Interdiff vs v1:

 diff --git a/run-command.c b/run-command.c
 index 45229ef052..5227f78aea 100644
 --- a/run-command.c
 +++ b/run-command.c
 @@ -873,7 +873,7 @@ const char *find_hook(const char *name)
  	strbuf_git_path(&path, "hooks/%s", name);
  	if (access(path.buf, X_OK) < 0) {
  #ifdef STRIP_EXTENSION
 -		strbuf_addstr(&path, ".exe");
 +		strbuf_addstr(&path, STRIP_EXTENSION);
  		if (access(path.buf, X_OK) >= 0)
  			return path.buf;
  #endif


 run-command.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/run-command.c b/run-command.c
index 73bfba7ef9..5227f78aea 100644
--- a/run-command.c
+++ b/run-command.c
@@ -871,8 +871,14 @@ const char *find_hook(const char *name)
 
 	strbuf_reset(&path);
 	strbuf_git_path(&path, "hooks/%s", name);
-	if (access(path.buf, X_OK) < 0)
+	if (access(path.buf, X_OK) < 0) {
+#ifdef STRIP_EXTENSION
+		strbuf_addstr(&path, STRIP_EXTENSION);
+		if (access(path.buf, X_OK) >= 0)
+			return path.buf;
+#endif
 		return NULL;
+	}
 	return path.buf;
 }
 

base-commit: 4e59582ff70d299f5a88449891e78d15b4b3fabe
-- 
2.11.1.windows.prerelease.2.9.g3014b57

^ permalink raw reply related

* Re: [PATCH] mingw: allow hooks to be .exe files
From: Johannes Schindelin @ 2017-01-26 12:21 UTC (permalink / raw)
  To: Jeff King; +Cc: git, Junio C Hamano
In-Reply-To: <20170125211529.jq4halip4ndbff5k@sigill.intra.peff.net>

Hi Peff,

On Wed, 25 Jan 2017, Jeff King wrote:

> On Wed, Jan 25, 2017 at 05:58:42PM +0100, Johannes Schindelin wrote:
> 
> > -	if (access(path.buf, X_OK) < 0)
> > +	if (access(path.buf, X_OK) < 0) {
> > +#ifdef STRIP_EXTENSION
> > +		strbuf_addstr(&path, ".exe");
> 
> I think STRIP_EXTENSION is a string.  Should this line be:
> 
>   strbuf_addstr(&path, STRIP_EXTENSION);

Yep.

v2 coming,
Johannes

^ permalink raw reply

* Re: [PATCH] Retire the `relink` command
From: Johannes Schindelin @ 2017-01-26 12:17 UTC (permalink / raw)
  To: Eric Wong; +Cc: git, Junio C Hamano
In-Reply-To: <20170125232051.GA25810@whir>

Hi Eric,

On Wed, 25 Jan 2017, Eric Wong wrote:

> Johannes Schindelin <johannes.schindelin@gmx.de> wrote:
> > Back in the olden days, when all objects were loose and rubber boots
> > were made out of wood, it made sense to try to share (immutable)
> > objects between repositories.
> > 
> > Ever since the arrival of pack files, it is but an anachronism.
> > 
> > Let's move the script to the contrib/examples/ directory and no longer
> > offer it.
> 
> On the other hand, we have no idea if there are still people
> using it for whatever reason...
> 
> I suggest we have a deprecation period where:

I would be fine with a deprecation phase, but that decision is solely on
Junio's shoulders.

Ciao,
Johannes

^ permalink raw reply

* Re: [PATCH 2/2] fsck: lazily load types under --connectivity-only
From: Johannes Schindelin @ 2017-01-26 12:07 UTC (permalink / raw)
  To: Jeff King; +Cc: git
In-Reply-To: <20170126041206.5qfv7r7czbwfqvaa@sigill.intra.peff.net>

Hi Peff,

On Wed, 25 Jan 2017, Jeff King wrote:

>  builtin/fsck.c | 58 +++++++---------------------------------------------------
>  fsck.c         |  4 ++++
>  2 files changed, 11 insertions(+), 51 deletions(-)

Patch looks good to my eyes.

Ciao,
Johannes

^ permalink raw reply

* Re: [PATCH] connect: handle putty/plink also in GIT_SSH_COMMAND
From: Johannes Schindelin @ 2017-01-26 12:01 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Segev Finer
In-Reply-To: <xmqqinp2939j.fsf@gitster.mtv.corp.google.com>

Hi Junio,

On Wed, 25 Jan 2017, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > Now, with the patch in question (without the follow-up, which I would
> > like to ask you to ignore, just like you did so far), Git would not
> > figure out that your script calls PuTTY eventually. The work-around?
> > Easy:
> >
> > 	DUMMY=/plink.exe /path/to/junio-is-a-superstar.sh
> 
> Think about how you would explain that to an end-user in our document?
> You'll need to explain how exactly the auto-detection works, so that the
> user can "exploit" the loophole to do that.  And what maintenance burden
> does it add when auto-detection is updated?

Fine, you do not like it. Saying so (instead of asking me questions) would
have been helpful.

> I think I know you well enough that you know well enough that it is too
> ugly to live, and I suspect that the above is a tongue-in-cheek "arguing
> for the sake of argument" and would not need a serious response, but
> just in case...

It was not tongue-in-cheek, I was being serious.

> Yes.  Here is what comes on an obvious clean-up patch (which will be
> sent as a follow-up to this message).

I'd much rather prefer
https://github.com/git-for-windows/git/pull/1030 than your patch.

Ciao,
Johannes

^ permalink raw reply

* Re: [PATCH] Documentation: implement linkgit macro for Asciidoctor
From: Johannes Schindelin @ 2017-01-26 11:43 UTC (permalink / raw)
  To: brian m. carlson; +Cc: git, Jeff King, Øyvind A. Holm
In-Reply-To: <20170126001344.445534-1-sandals@crustytoothpaste.net>

Hi Brian,

On Thu, 26 Jan 2017, brian m. carlson wrote:

> AsciiDoc uses a configuration file to implement macros like linkgit,
> while Asciidoctor uses Ruby extensions.  Implement a Ruby extension that
> implements the linkgit macro for Asciidoctor in the same way that
> asciidoc.conf does for AsciiDoc.  Adjust the Makefile to use it by
> default.

I like it.

Thank you,
Johannes

^ permalink raw reply

* Re: Fixing the warning about warning(""); was: Re: [PATCH] difftool.c: mark a file-local symbol with static
From: Johannes Schindelin @ 2017-01-26 11:37 UTC (permalink / raw)
  To: Johannes Sixt
  Cc: Jeff King, Junio C Hamano, David Aguilar, Ramsay Jones,
	GIT Mailing-list
In-Reply-To: <546179e0-1d6e-86f7-00cf-e13218b76de1@kdbg.org>

Hi Hannes,

On Thu, 26 Jan 2017, Johannes Sixt wrote:

> Am 25.01.2017 um 23:01 schrieb Jeff King:
> > +#pragma GCC diagnostic ignored "-Wformat-zero-length"
> 
> Last time I used #pragma GCC in a cross-platform project, it triggered
> an "unknown pragma" warning for MSVC.

It is starting to become a little funny how many ways we can discuss the
resolution of the GCC compiler warning.

And it starts to show: we try to solve the thing in so many ways, just to
avoid the obviously most-trivial patch to change warning(""); to
warning("%s", "") (the change to warning(" "); would change behavior, but
I would be fine with that, too).

I am not really interested in any of these complicated workarounds. If you
gentle people decide they are better in Git's source code, go ahead. I do
not have to like what you are doing, I just have to work with it.

> (It was the C++ compiler, I don't know if the C compiler would also
> warn.) It would have to be spelled like this:
> 
> #pragma warning(disable: 4068)   /* MSVC: unknown pragma */
> #pragma GCC diagnostic ignored "-Wformat-zero-length"
> 
> Dscho mentioned that he's compiling with MSVC. It would do him a favor.

I am compiling with MSVC, and the idea is to tap into that large number of
Windows developers who Git traditionally has had a really bad time
attracting. From that perspective, I would say it would not only do me a
favor, but anybody who builds Git for Windows using Visual Studio.

But we also have to consider whether it would do anybody a "dis-favor".
#pragma statements are by definition highly dependent on the compiler. I
have no idea whether there are developers out there building Git with
C compilers other than GCC, clang or MSVC (as I did back in the days on
IRIX and HP/UX), but there is quite the potential for problems here [*1*].

To keep Git's source code truly portable, the #pragma would have to be
guarded by a GCC-specific #ifdef ... #endif.

Ciao,
Dscho

Footnote *1*: This is just another instance where a discussion on the Git
mailing list reminds me of
http://thedailywtf.com/articles/The_Complicator_0x27_s_Gloves, as it tries
to avoid an obvious solution by trying to come up with a different
solution that in turn requires additional solutions to additional problems
caused by the alternative solution.

^ permalink raw reply

* Re: sparse checkout - weird behavior
From: Paul Hammant @ 2017-01-26 11:20 UTC (permalink / raw)
  To: Jeff King; +Cc: git
In-Reply-To: <20170126045936.wyleenuwunhrvbn2@sigill.intra.peff.net>

Well I feel a bit silly. Thanks for responding.

On Wed, Jan 25, 2017 at 11:59 PM, Jeff King <peff@peff.net> wrote:
> On Wed, Jan 25, 2017 at 10:21:19PM -0500, Paul Hammant wrote:
>
>> Related bug (maybe the same). Reproduction:
>>
>>   $ git clone git@github.com:jekyll/jekyll.git --no-checkout
>>   Cloning into 'jekyll'...
>>   remote: Counting objects: 41331, done.
>>   remote: Compressing objects: 100% (5/5), done.
>>   remote: Total 41331 (delta 0), reused 0 (delta 0), pack-reused 41326
>>   Receiving objects: 100% (41331/41331), 11.91 MiB | 9.15 MiB/s, done.
>>   Resolving deltas: 100% (26530/26530), done.
>>   $ cd jekyll
>>   $ git config core.sparsecheckout true
>>   $ echo 'CONDUCT.markdown' > .git/info/sparse-checkout
>>   $ echo 'Gemfile' >> .git/info/sparse-checkout
>>   $ echo 'Rakefile' >> .git/info/sparse-checkout
>>   $ echo 'appveyor.yml' >> .git/info/sparse-checkout
>>   $ git checkout --
>>   Your branch is up-to-date with 'origin/master'.
>>   $ ls
>>   CONDUCT.markdown Gemfile Rakefile appveyor.yml lib
>>
>> I was not expecting to see 'lib' in the resulting file list
>
> Yep, I think this is the same problem. Inside lib, you get only
> "lib/theme_template/Gemfile", because it matches your unanchored
> pattern. Using "/Gemfile" in the sparse-checkout file fixes it.
>
> -Peff

^ permalink raw reply

* Re: Fixing the warning about warning(""); was: Re: [PATCH] difftool.c: mark a file-local symbol with static
From: Johannes Schindelin @ 2017-01-26 11:16 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, David Aguilar, Ramsay Jones, GIT Mailing-list
In-Reply-To: <20170125183542.pe5qolexqqx6jhsi@sigill.intra.peff.net>

Hi Peff,

On Wed, 25 Jan 2017, Jeff King wrote:

> On Wed, Jan 25, 2017 at 11:36:50AM +0100, Johannes Schindelin wrote:
> 
> > > Gross, but at least it's self documenting. :)
> > > 
> > > I guess a less horrible version of that is:
> > > 
> > >   static inline warning_blank_line(void)
> > >   {
> > > 	warning("%s", "");
> > >   }
> > > 
> > > We'd potentially need a matching one for error(), but at last it avoids
> > > macro trickery.
> > 
> > I fail to see how this function, or this definition, makes the code better
> > than simply calling `warning("%s", "");` and be done with it.
> 
> The only advantage is that it is self-documenting, so somebody does not
> come through later and convert ("%s", "") back to ("").

We could switch the DEVELOPER option on by default, when gcc or clang is
used at least. Otherwise the DEVELOPER option (which I like very much)
would not be able to live up to its full potential.

Another thing we should consider: paying more attention to Continuous
Integration. At the moment, it happens quite frequently that `pu` builds
and passes the test suite fine on Linux, but neither on Windows nor on
MacOSX and it takes days to get the regressions fixed.

I vote for this patch:

> -- >8 --
> Subject: [PATCH] difftool: hack around -Wzero-length-format warning
> 
> Building with "gcc -Wall" will complain that the format in:
> 
>   warning("")
> 
> is empty. Which is true, but the warning is over-eager. We
> are calling the function for its side effect of printing
> "warning:", even with an empty string.
> 
> Our DEVELOPER Makefile knob disables the warning, but not
> everybody uses it. Let's silence the warning in the code so
> that nobody reports it or tries to "fix" it.
> 
> Signed-off-by: Jeff King <peff@peff.net>
> ---
>  builtin/difftool.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/builtin/difftool.c b/builtin/difftool.c
> index 42ad9e804..b5e85ab07 100644
> --- a/builtin/difftool.c
> +++ b/builtin/difftool.c
> @@ -567,7 +567,7 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix,
>  				warning(_("both files modified: '%s' and '%s'."),
>  					wtdir.buf, rdir.buf);
>  				warning(_("working tree file has been left."));
> -				warning("");
> +				warning("%s", "");
>  				err = 1;
>  			} else if (unlink(wtdir.buf) ||
>  				   copy_file(wtdir.buf, rdir.buf, st.st_mode))
> -- 
> 2.11.0.840.gd37c5973a

Ciao,
Dscho

^ permalink raw reply

* Re: What's cooking in git.git (Jan 2017, #04; Mon, 23)
From: Lars Schneider @ 2017-01-26  9:48 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, Johannes Schindelin, git
In-Reply-To: <D9F0976B-9F78-44BE-B9DD-CAB6506FA3A9@gmail.com>


> On 26 Jan 2017, at 10:14, Lars Schneider <larsxschneider@gmail.com> wrote:
> 
> 
>> On 25 Jan 2017, at 23:51, Junio C Hamano <gitster@pobox.com> wrote:
>> 
>> Jeff King <peff@peff.net> writes:
>> 
>>> I guess the way to dig would be to add a test that looks at the output
>>> of "type mv" or something, push it to a Travis-hooked branch, and then
>>> wait for the output
>> 
>> Sounds tempting ;-)
> 
> Well, I tried that:
> 
> mv is /bin/mv
> 
> ... and "/bin/mv" is exactly the version that I have on my machine.
> 
> The difference between Travis and my machine is that I changed the 
> default shell to ZSH with a few plugins [1]. If I run the test with 
> plain BASH on my Mac then I can reproduce the test failure. Therefore,
> we might want to adjust the commit message if anyone else can reproduce
> the problem on a Mac. 
> 
> I can even reproduce the failure if I run the test with plain ZSH. 
> However, I can't find a plugin that defines an alias for "mv". Puzzled...
> 
> - Lars
> 
> [1] https://github.com/robbyrussell/oh-my-zsh

Oh. I must have made a mistake on my very first test run. I can reproduce
the failure with ZSH and my plugins... looks like it's a Mac OS problem
and no TravisCI only problem after all. 

Sorry for the noise/confusion,
Lars

^ permalink raw reply

* Re: [PATCH 4/5] revision.c: refactor ref selection handler after --exclude
From: Duy Nguyen @ 2017-01-26  9:28 UTC (permalink / raw)
  To: Jeff King; +Cc: Git Mailing List, Jacob Keller
In-Reply-To: <20170125205718.ksqstdnazmgbkehy@sigill.intra.peff.net>

On Thu, Jan 26, 2017 at 3:57 AM, Jeff King <peff@peff.net> wrote:
> I don't think it means either. It means to include remotes in the
> selected revisions, but excluding the entries mentioned by --exclude.
>
> IOW:
>
>   --exclude=foo --remotes
>         include all remotes except refs/remotes/foo
>
>   --exclude=foo --unrelated --remotes
>         same
>
>   --exclude=foo --decorate-reflog --remotes
>         decorate reflogs of all remotes except "foo". Do _not_ use them
>         as traversal tips.
>
>   --decorate-reflog --exclude=foo --remotes
>         same
>
> IOW, the ref-selector options build up until a group option is given,
> which acts on the built-up options (over that group) and then resets the
> built-up options. Doing "--unrelated" as above is orthogonal (though I
> think in practice nobody would do that, because it's hard to read).

This is because it makes sense to combine --exclude and
--decorate-reflog. But what about a new --something that conflicts
with either --exclude or --decorate-reflog? Should we simply catch
such combinations and error out (which may be a bit more complicated
than this patch, or maybe not)?
-- 
Duy

^ permalink raw reply

* Re: [PATCH 2/5] revision.c: group ref selection options together
From: Duy Nguyen @ 2017-01-26  9:18 UTC (permalink / raw)
  To: Jeff King; +Cc: Git Mailing List, Jacob Keller
In-Reply-To: <20170125205037.cg3aebh5rsx4zb2l@sigill.intra.peff.net>

On Thu, Jan 26, 2017 at 3:50 AM, Jeff King <peff@peff.net> wrote:
> On Wed, Jan 25, 2017 at 07:50:51PM +0700, Nguyễn Thái Ngọc Duy wrote:
>
>> These options have on thing in common: when specified right after
>> --exclude, they will de-select refs instead of selecting them by
>> default.
>>
>> This change makes it possible to introduce new options that use these
>> options in the same way as --exclude. Such an option would just
>> implement something like handle_refs_pseudo_opt().
>>
>> parse_ref_selector_option() is taken out of handle_refs_pseudo_opt() so
>> that similar functions like handle_refs_pseudo_opt() are forced to
>> handle all ref selector options, not skipping some by mistake, which may
>> revert the option back to default behavior (rev selection).
>>
>> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
>> ---
>>  revision.c | 134 +++++++++++++++++++++++++++++++++++++++++++++----------------
>>  1 file changed, 100 insertions(+), 34 deletions(-)
>
> Hmm. I see what you're trying to do here, and abstract the repeated
> bits. But I'm not sure the line-count reflects a real simplification.
> Everything ends up converted to an enum, and then that enum just expands
> to similar C code.

It's not simplification, but hopefully for better maintainability. This

if (strcmp(arg, "--remotes")) {
   if (preceded_by_exclide())
      does_something();
   else if (preceded_by_decorate())
      does_another()
} else if (strcmp(arg, "--branches")) {
   if (preceded_by_exclide())
      does_something();
   else if (preceded_by_decorate())
      does_another()
}

starts to look ugly especially when the third "preceded_by_" comes
into picture. Putting all "does_something" in one group and
"does_another" in another, I think, gives us a better view how ref
selection is handled for a specific operation like --exclude or
--decorate-ref.

> I kind of expected that clear_ref_exclusion() would just become a more
> abstract clear_ref_selection(). For now it would clear exclusions, and
> then later learn to clear the decoration flags.

It may go that way, depending on how we handle these options for
decorate-reflog. The current load_ref_decorations() is not really
suited for fine-grained ref selection yet.
--
Duy

^ permalink raw reply

* Re: What's cooking in git.git (Jan 2017, #04; Mon, 23)
From: Lars Schneider @ 2017-01-26  9:14 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, Johannes Schindelin, git
In-Reply-To: <xmqq7f5i92jk.fsf@gitster.mtv.corp.google.com>


> On 25 Jan 2017, at 23:51, Junio C Hamano <gitster@pobox.com> wrote:
> 
> Jeff King <peff@peff.net> writes:
> 
>> I guess the way to dig would be to add a test that looks at the output
>> of "type mv" or something, push it to a Travis-hooked branch, and then
>> wait for the output
> 
> Sounds tempting ;-)

Well, I tried that:

mv is /bin/mv

... and "/bin/mv" is exactly the version that I have on my machine.

The difference between Travis and my machine is that I changed the 
default shell to ZSH with a few plugins [1]. If I run the test with 
plain BASH on my Mac then I can reproduce the test failure. Therefore,
we might want to adjust the commit message if anyone else can reproduce
the problem on a Mac. 

I can even reproduce the failure if I run the test with plain ZSH. 
However, I can't find a plugin that defines an alias for "mv". Puzzled...

- Lars

[1] https://github.com/robbyrussell/oh-my-zsh

^ permalink raw reply

* Re: [PATCH 2/5] revision.c: group ref selection options together
From: Duy Nguyen @ 2017-01-26  9:12 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git Mailing List, Jeff King, Jacob Keller
In-Reply-To: <xmqqo9yualqn.fsf@gitster.mtv.corp.google.com>

On Thu, Jan 26, 2017 at 4:11 AM, Junio C Hamano <gitster@pobox.com> wrote:
>  * I am expecting that the new one yet to be introduced will not
>    share the huge "switch (selector)" part, but does its own things
>    in a separate function with a similar structure.  The only thing
>    common between these two functions would be the structure
>    (i.e. it has a big "switch(selector)" that does different things
>    depending on REF_SELECT_*) and a call to clear_* function.

Yep. The "new one" is demonstrated in 5/5.

>    If we were to add a new kind of REF_SELECT_* (say
>    REF_SELECT_NOTES just for the sake of being concrete), what
>    changes will be needed to the code if the addition of "use reflog
>    from this class of refs for decoration" feature was done with or
>    without this step?  I have a suspicion that the change will be
>    simpler without this step.

The switch/case is to deal with new REF_SELECT_* (at least it's how I
imagine it). What I was worried about was, when a user adds
--select-notes, they may not be aware that it's in the same
all/branches/tags/remotes group that's supposed to work with
--decorate-reflog as well, and as a result "--decorate-reflog
--select-notes" is the same as "--select-notes".

With the switch/case, when you add a new enum item, at the least the
compiler should warn about unhandled cases. And we can have a new
"case REF_SELECT_NOTES:" for both --exclude and --decorate-reflog.
Without the switch/case, I guess it's still possible to do something
like

if (!strcmp(arg, "--select-notes")) {
    if (preceded_by_exclude())
        does_one_thing();
    else if (preceded_by_decorate_reflog())
       does_another_thing();
}

It's probably easier to maintain though, if all
decorate-reflog-related things are grouped together, rather than
spread out per option like the above.
-- 
Duy

^ permalink raw reply

* Re: [PATCH] Documentation: implement linkgit macro for Asciidoctor
From: Eric Wong @ 2017-01-26  7:43 UTC (permalink / raw)
  To: Jeff King; +Cc: brian m. carlson, git, Johannes Schindelin, Øyvind A. Holm
In-Reply-To: <20170126034655.fwzow2mgkjj5dpek@sigill.intra.peff.net>

Jeff King <peff@peff.net> wrote:
> On Thu, Jan 26, 2017 at 12:13:44AM +0000, brian m. carlson wrote:
> > +
> > +      def process(parent, target, attrs)
> > +        if parent.document.basebackend? 'html'
> > +          prefix = parent.document.attr('git-relative-html-prefix')
> > +          %(<a href="#{prefix}#{target}.html">#{target}(#{attrs[1]})</a>\n)
> > +        elsif parent.document.basebackend? 'docbook'
> > +          %(<citerefentry>
> > +<refentrytitle>#{target}</refentrytitle><manvolnum>#{attrs[1]}</manvolnum>
> > +</citerefentry>
> > +)

<snip>

> The multi-line string is kind of ugly because of the indentation.
> Apparently Ruby has here-docs that will eat leading whitespace, but the
> syntax was not introduce until Ruby 2.3, which is probably more recent
> than we should count on.

You can use '\' to continue long lines with any Ruby version:

    "<citerefentry>" \
      "<refentrytitle>#{target}</refentrytitle>" \
      "<manvolnum>#{attrs[1]}</manvolnum>" \
    "</citerefentry>"

The above happens during the parse phase, so there's no garbage
or method call overhead compared to the more-frequently seen '+'
or '<<' method calls to combine strings.

> I think you could write:
> 
>           %(<citerefentry>
>             <refentrytitle>#{target}</refentrytitle><manvolnum>#{attrs[1]}</manvolnum>
>             </citerefentry>
> 	  ).gsub(/^\s*/, "")
> 
> I don't know if that's too clever or not.

Ick...

> But either way, I like this better than introducing an extra dependency.

Agreed.

^ permalink raw reply

* Re: Fixing the warning about warning(""); was: Re: [PATCH] difftool.c: mark a file-local symbol with static
From: Johannes Sixt @ 2017-01-26  6:39 UTC (permalink / raw)
  To: Jeff King
  Cc: Junio C Hamano, Johannes Schindelin, David Aguilar, Ramsay Jones,
	GIT Mailing-list
In-Reply-To: <20170125220101.et67ebkumsqosaku@sigill.intra.peff.net>

Am 25.01.2017 um 23:01 schrieb Jeff King:
> +#pragma GCC diagnostic ignored "-Wformat-zero-length"

Last time I used #pragma GCC in a cross-platform project, it triggered 
an "unknown pragma" warning for MSVC. (It was the C++ compiler, I don't 
know if the C compiler would also warn.) It would have to be spelled 
like this:

#pragma warning(disable: 4068)   /* MSVC: unknown pragma */
#pragma GCC diagnostic ignored "-Wformat-zero-length"

Dscho mentioned that he's compiling with MSVC. It would do him a favor.

-- Hannes


^ permalink raw reply

* Re: [PATCH] gpg-interface: Add some output from gpg when it errors out.
From: Jeff King @ 2017-01-26  5:21 UTC (permalink / raw)
  To: Mike Hommey; +Cc: Junio C Hamano, git
In-Reply-To: <20170125235410.byxwmo7o7zdszzot@glandium.org>

On Thu, Jan 26, 2017 at 08:54:10AM +0900, Mike Hommey wrote:

> > Implementation-wise, I'd be happier if we do not add any new
> > callsites of strbuf_split(), which is a horrible interface.  But
> > that is a minor detail.
> 
> What would you suggest otherwise?

Try string_list_split() (or its in_place() variant, since it is probably
OK to hack up the string for your use case). Like this:

diff --git a/gpg-interface.c b/gpg-interface.c
index 2768bb307..051bb7d3e 100644
--- a/gpg-interface.c
+++ b/gpg-interface.c
@@ -158,14 +158,16 @@ static int pipe_gpg_command(struct child_process *cmd,
 	/* Print out any line that doesn't start with [GNUPG:] if the gpg
 	 * command failed. */
 	if (ret) {
-		struct strbuf **err_lines = strbuf_split(err, '\n');
-		for (struct strbuf **line = err_lines; *line; line++) {
-			if (memcmp((*line)->buf, "[GNUPG:]", 8)) {
-				strbuf_rtrim(*line);
-				fprintf(stderr, "%s\n", (*line)->buf);
-			}
+		struct string_list lines = STRING_LIST_INIT_NODUP;
+		int i;
+
+		string_list_split_in_place(&lines, err->buf, '\n', 0);
+		for (i = 0; i < lines.nr; i++) {
+			const char *line = lines.items[i].string;
+			if (!starts_with(line, "[GNUPG:]"))
+				fprintf(stderr, "%s\n", line);
 		}
-		strbuf_list_free(err_lines);
+		string_list_clear(&lines, 0);
 	}
 	return ret;
 }

Note that I also replaced the memcmp with starts_with(). That avoids the
magic number "8". I also suspect your original can read off the end of
the buffer when the line is shorter than 8 characters (e.g., if memcmp
does 64-bit loads).

-Peff

^ permalink raw reply related

* Re: Force Confirmation for Dropping Changed Lines
From: Jacob Keller @ 2017-01-26  5:05 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Hilco Wijbenga, Git Users
In-Reply-To: <xmqqd1fa7dqf.fsf@gitster.mtv.corp.google.com>

On Wed, Jan 25, 2017 at 6:32 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Where did you get that "unset" from?  If that is this paragraph in
> Documentation/gitattributes.txt:
>

Ok so that whole section of documentation is very confusing to me.
Maybe it could be improved for more readability. I'll see if I can't
help clear up some of my confusion.

Thanks,
Jake

^ permalink raw reply

* Re: sparse checkout - weird behavior
From: Jeff King @ 2017-01-26  4:59 UTC (permalink / raw)
  To: Paul Hammant; +Cc: git
In-Reply-To: <CA+298Ujx2wH2WnoYiOaWKoneBrF_E5VUXXSMqecGgNLYS0Wemg@mail.gmail.com>

On Wed, Jan 25, 2017 at 10:21:19PM -0500, Paul Hammant wrote:

> Related bug (maybe the same). Reproduction:
> 
>   $ git clone git@github.com:jekyll/jekyll.git --no-checkout
>   Cloning into 'jekyll'...
>   remote: Counting objects: 41331, done.
>   remote: Compressing objects: 100% (5/5), done.
>   remote: Total 41331 (delta 0), reused 0 (delta 0), pack-reused 41326
>   Receiving objects: 100% (41331/41331), 11.91 MiB | 9.15 MiB/s, done.
>   Resolving deltas: 100% (26530/26530), done.
>   $ cd jekyll
>   $ git config core.sparsecheckout true
>   $ echo 'CONDUCT.markdown' > .git/info/sparse-checkout
>   $ echo 'Gemfile' >> .git/info/sparse-checkout
>   $ echo 'Rakefile' >> .git/info/sparse-checkout
>   $ echo 'appveyor.yml' >> .git/info/sparse-checkout
>   $ git checkout --
>   Your branch is up-to-date with 'origin/master'.
>   $ ls
>   CONDUCT.markdown Gemfile Rakefile appveyor.yml lib
> 
> I was not expecting to see 'lib' in the resulting file list

Yep, I think this is the same problem. Inside lib, you get only
"lib/theme_template/Gemfile", because it matches your unanchored
pattern. Using "/Gemfile" in the sparse-checkout file fixes it.

-Peff

^ permalink raw reply

* Re: sparse checkout - weird behavior
From: Jeff King @ 2017-01-26  4:57 UTC (permalink / raw)
  To: Paul Hammant; +Cc: git
In-Reply-To: <CA+298Uh=bfCJq3hmVdGUsinAgKFQd86em_J_8fwB9jQR5PZVgQ@mail.gmail.com>

On Wed, Jan 25, 2017 at 09:59:38PM -0500, Paul Hammant wrote:

> Here's a simple reproducible bug - something unexpected in sparse-checkout mode:
> 
>   $ git clone git@github.com:jekyll/jekyll.git --no-checkout
>   Cloning into 'jekyll'...
>   remote: Counting objects: 41331, done.
>   remote: Compressing objects: 100% (5/5), done.
>   remote: Total 41331 (delta 0), reused 0 (delta 0), pack-reused 41326
>   Receiving objects: 100% (41331/41331), 11.91 MiB | 7.98 MiB/s, done.
>   Resolving deltas: 100% (26530/26530), done.
>   $ cd jekyll
>   $ ls
>   $ git config core.sparsecheckout true
>   $ echo 'docs*' > .git/info/sparse-checkout
>   $ git read-tree -mu HEAD
>   $ ls
>   docs rake
> 
> I didn't expect to see 'rake' amongst the results.

If you look inside the rake/ directory, you should see that only
"docs.rake" was checked out.

The sparse-checkout file uses the same parser as .git/info/exclude. One
important aspect of that file is that entries are _not_ left-anchored
unless they start with "/". So you asked Git to include files named
"docs*" anywhere in the tree.

You probably wanted just:

  echo /docs >.git/info/sparse-checkout

-Peff

^ permalink raw reply

* [PATCH 2/2] fsck: lazily load types under --connectivity-only
From: Jeff King @ 2017-01-26  4:12 UTC (permalink / raw)
  To: git
In-Reply-To: <20170126041025.hqg3znwew7jxgxxg@sigill.intra.peff.net>

The recent fixes to "fsck --connectivity-only" load all of
the objects with their correct types. This keeps the
connectivity-only code path close to the regular one, but it
also introduces some unnecessary inefficiency. While getting
the type of an object is cheap compared to actually opening
and parsing the object (as the non-connectivity-only case
would do), it's still not free.

For reachable non-blob objects, we end up having to parse
them later anyway (to see what they point to), making our
type lookup here redundant.

For unreachable objects, we might never hit them at all in
the reachability traversal, making the lookup completely
wasted. And in some cases, we might have quite a few
unreachable objects (e.g., when alternates are used for
shared object storage between repositories, it's normal for
there to be objects reachable from other repositories but
not the one running fsck).

The comment in mark_object_for_connectivity() claims two
benefits to getting the type up front:

  1. We need to know the types during fsck_walk(). (And not
     explicitly mentioned, but we also need them when
     printing the types of broken or dangling commits).

     We can address this by lazy-loading the types as
     necessary. Most objects never need this lazy-load at
     all, because they fall into one of these categories:

       a. Reachable from our tips, and are coerced into the
	  correct type as we traverse (e.g., a parent link
	  will call lookup_commit(), which converts OBJ_NONE
	  to OBJ_COMMIT).

       b. Unreachable, but not at the tip of a chunk of
          unreachable history. We only mention the tips as
	  "dangling", so an unreachable commit which links
	  to hundreds of other objects needs only report the
	  type of the tip commit.

  2. It serves as a cross-check that the coercion in (1a) is
     correct (i.e., we'll complain about a parent link that
     points to a blob). But we get most of this for free
     already, because right after coercing, we'll parse any
     non-blob objects. So we'd notice then if we expected a
     commit and got a blob.

     The one exception is when we expect a blob, in which
     case we never actually read the object contents.

     So this is a slight weakening, but given that the whole
     point of --connectivity-only is to sacrifice some data
     integrity checks for speed, this seems like an
     acceptable tradeoff.

Here are before and after timings for an extreme case with
~5M reachable objects and another ~12M unreachable (it's the
torvalds/linux repository on GitHub, connected to shared
storage for all of the other kernel forks):

  [before]
  $ time git fsck --no-dangling --connectivity-only
  real	3m4.323s
  user	1m25.121s
  sys	1m38.710s

  [after]
  $ time git fsck --no-dangling --connectivity-only
  real	0m51.497s
  user	0m49.575s
  sys	0m1.776s

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin/fsck.c | 58 +++++++---------------------------------------------------
 fsck.c         |  4 ++++
 2 files changed, 11 insertions(+), 51 deletions(-)

diff --git a/builtin/fsck.c b/builtin/fsck.c
index 3d5ced2d3..140357b6f 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -60,6 +60,12 @@ static const char *printable_type(struct object *obj)
 {
 	const char *ret;
 
+	if (obj->type == OBJ_NONE) {
+		enum object_type type = sha1_object_info(obj->oid.hash, NULL);
+		if (type > 0)
+			object_as_type(obj, type, 0);
+	}
+
 	ret = typename(obj->type);
 	if (!ret)
 		ret = "unknown";
@@ -595,57 +601,7 @@ static int fsck_cache_tree(struct cache_tree *it)
 
 static void mark_object_for_connectivity(const unsigned char *sha1)
 {
-	struct object *obj = lookup_object(sha1);
-
-	/*
-	 * Setting the object type here isn't strictly necessary for a
-	 * connectivity check. In most cases, our walk will expect a certain
-	 * type (e.g., a tree referencing a blob) and will use lookup_blob() to
-	 * assign the type. But doing it here has two advantages:
-	 *
-	 *   1. When the fsck_walk code looks at objects that _don't_ come from
-	 *      links (e.g., the tip of a ref), it may complain about the
-	 *      "unknown object type".
-	 *
-	 *   2. This serves as a nice cross-check that the graph links are
-	 *      sane. So --connectivity-only does not check that the bits of
-	 *      blobs are not corrupted, but it _does_ check that 100644 tree
-	 *      entries point to blobs, and so forth.
-	 *
-	 * Unfortunately we can't just use parse_object() here, because the
-	 * whole point of --connectivity-only is to avoid reading the object
-	 * data more than necessary.
-	 */
-	if (!obj || obj->type == OBJ_NONE) {
-		enum object_type type = sha1_object_info(sha1, NULL);
-		switch (type) {
-		case OBJ_BAD:
-			error("%s: unable to read object type",
-			      sha1_to_hex(sha1));
-			break;
-		case OBJ_COMMIT:
-			obj = (struct object *)lookup_commit(sha1);
-			break;
-		case OBJ_TREE:
-			obj = (struct object *)lookup_tree(sha1);
-			break;
-		case OBJ_BLOB:
-			obj = (struct object *)lookup_blob(sha1);
-			break;
-		case OBJ_TAG:
-			obj = (struct object *)lookup_tag(sha1);
-			break;
-		default:
-			error("%s: unknown object type %d",
-			      sha1_to_hex(sha1), type);
-		}
-
-		if (!obj || obj->type == OBJ_NONE) {
-			errors_found |= ERROR_OBJECT;
-			return;
-		}
-	}
-
+	struct object *obj = lookup_unknown_object(sha1);
 	obj->flags |= HAS_OBJ;
 }
 
diff --git a/fsck.c b/fsck.c
index 4a3069e20..939792752 100644
--- a/fsck.c
+++ b/fsck.c
@@ -458,6 +458,10 @@ int fsck_walk(struct object *obj, void *data, struct fsck_options *options)
 {
 	if (!obj)
 		return -1;
+
+	if (obj->type == OBJ_NONE)
+		parse_object(obj->oid.hash);
+
 	switch (obj->type) {
 	case OBJ_BLOB:
 		return 0;
-- 
2.11.0.840.gd37c5973a

^ permalink raw reply related

* [PATCH 1/2] fsck: move typename() printing to its own function
From: Jeff King @ 2017-01-26  4:11 UTC (permalink / raw)
  To: git
In-Reply-To: <20170126041025.hqg3znwew7jxgxxg@sigill.intra.peff.net>

When an object has a problem, we mention its type. But we do
so by feeding the result of typename() directly to
fprintf(). This is potentially dangerous because typename()
can return NULL for some type values (like OBJ_NONE).

It's doubtful that this can be triggered in practice with
the current code, so this is probably not fixing a bug. But
it future-proofs us against modifications that make things
like OBJ_NONE more likely (and gives future patches a
central point to handle them).

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin/fsck.c | 29 ++++++++++++++++++++---------
 1 file changed, 20 insertions(+), 9 deletions(-)

diff --git a/builtin/fsck.c b/builtin/fsck.c
index 57f529b41..3d5ced2d3 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -56,6 +56,17 @@ static const char *describe_object(struct object *obj)
 	return buf.buf;
 }
 
+static const char *printable_type(struct object *obj)
+{
+	const char *ret;
+
+	ret = typename(obj->type);
+	if (!ret)
+		ret = "unknown";
+
+	return ret;
+}
+
 static int fsck_config(const char *var, const char *value, void *cb)
 {
 	if (strcmp(var, "fsck.skiplist") == 0) {
@@ -83,7 +94,7 @@ static void objreport(struct object *obj, const char *msg_type,
 			const char *err)
 {
 	fprintf(stderr, "%s in %s %s: %s\n",
-		msg_type, typename(obj->type), describe_object(obj), err);
+		msg_type, printable_type(obj), describe_object(obj), err);
 }
 
 static int objerror(struct object *obj, const char *err)
@@ -114,7 +125,7 @@ static int mark_object(struct object *obj, int type, void *data, struct fsck_opt
 	if (!obj) {
 		/* ... these references to parent->fld are safe here */
 		printf("broken link from %7s %s\n",
-			   typename(parent->type), describe_object(parent));
+			   printable_type(parent), describe_object(parent));
 		printf("broken link from %7s %s\n",
 			   (type == OBJ_ANY ? "unknown" : typename(type)), "unknown");
 		errors_found |= ERROR_REACHABLE;
@@ -131,9 +142,9 @@ static int mark_object(struct object *obj, int type, void *data, struct fsck_opt
 	if (!(obj->flags & HAS_OBJ)) {
 		if (parent && !has_object_file(&obj->oid)) {
 			printf("broken link from %7s %s\n",
-				 typename(parent->type), describe_object(parent));
+				 printable_type(parent), describe_object(parent));
 			printf("              to %7s %s\n",
-				 typename(obj->type), describe_object(obj));
+				 printable_type(obj), describe_object(obj));
 			errors_found |= ERROR_REACHABLE;
 		}
 		return 1;
@@ -205,7 +216,7 @@ static void check_reachable_object(struct object *obj)
 	if (!(obj->flags & HAS_OBJ)) {
 		if (has_sha1_pack(obj->oid.hash))
 			return; /* it is in pack - forget about it */
-		printf("missing %s %s\n", typename(obj->type),
+		printf("missing %s %s\n", printable_type(obj),
 			describe_object(obj));
 		errors_found |= ERROR_REACHABLE;
 		return;
@@ -231,7 +242,7 @@ static void check_unreachable_object(struct object *obj)
 	 * since this is something that is prunable.
 	 */
 	if (show_unreachable) {
-		printf("unreachable %s %s\n", typename(obj->type),
+		printf("unreachable %s %s\n", printable_type(obj),
 			describe_object(obj));
 		return;
 	}
@@ -250,7 +261,7 @@ static void check_unreachable_object(struct object *obj)
 	 */
 	if (!obj->used) {
 		if (show_dangling)
-			printf("dangling %s %s\n", typename(obj->type),
+			printf("dangling %s %s\n", printable_type(obj),
 			       describe_object(obj));
 		if (write_lost_and_found) {
 			char *filename = git_pathdup("lost-found/%s/%s",
@@ -324,7 +335,7 @@ static int fsck_obj(struct object *obj)
 
 	if (verbose)
 		fprintf(stderr, "Checking %s %s\n",
-			typename(obj->type), describe_object(obj));
+			printable_type(obj), describe_object(obj));
 
 	if (fsck_walk(obj, NULL, &fsck_obj_options))
 		objerror(obj, "broken links");
@@ -350,7 +361,7 @@ static int fsck_obj(struct object *obj)
 		struct tag *tag = (struct tag *) obj;
 
 		if (show_tags && tag->tagged) {
-			printf("tagged %s %s", typename(tag->tagged->type),
+			printf("tagged %s %s", printable_type(tag->tagged),
 				describe_object(tag->tagged));
 			printf(" (%s) in %s\n", tag->tag,
 				describe_object(&tag->object));
-- 
2.11.0.840.gd37c5973a


^ 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