Git development
 help / color / mirror / Atom feed
* Re: [PATCH v2 9/9] read_loose_refs(): read refs using resolve_ref_recursively()
From: Junio C Hamano @ 2017-02-13 23:18 UTC (permalink / raw)
  To: Michael Haggerty
  Cc: Nguyễn Thái Ngọc Duy, Stefan Beller,
	Johannes Schindelin, David Turner, Jeff King, git
In-Reply-To: <ff0b0df6-9aed-9417-d9d4-1234d53f05c3@alum.mit.edu>

Michael Haggerty <mhagger@alum.mit.edu> writes:

> I pushed the fixed commit to branch `submodule-hash` in my fork [1]. If
> you'd like me to send it to the mailing list again, please let me know.

I was tempted to ask you to send it again, because fetching,
comparing and then cherry-picking is a lot more work than just
replacing one, but just to make sure my intuition about the
work required is correct, I did it anyway and yes, fetching,
comparing, cherry-picking and then amending the sign-off in was a
lot more work ;-)

So no need to resend.  Thanks.


^ permalink raw reply

* Re: [PATCH v6] gc: ignore old gc.log files
From: Junio C Hamano @ 2017-02-13 23:21 UTC (permalink / raw)
  To: David Turner; +Cc: git, peff, pclouds
In-Reply-To: <20170210212822.14988-1-dturner@twosigma.com>

David Turner <dturner@twosigma.com> writes:

> +static unsigned long gc_log_expire_time;
> +static const char *gc_log_expire = "1.day.ago";

OK.

> @@ -113,6 +133,8 @@ static void gc_config(void)
>  	git_config_get_bool("gc.autodetach", &detach_auto);
>  	git_config_date_string("gc.pruneexpire", &prune_expire);
>  	git_config_date_string("gc.worktreepruneexpire", &prune_worktrees_expire);
> +	git_config_date_string("gc.logexpiry", &gc_log_expire);
> +

OK.

I think I had a stale one queued; will replace.

Thanks.

^ permalink raw reply

* [PATCH] completion: complete modified files for checkout with '--'
From: cornelius.weig @ 2017-02-13 23:33 UTC (permalink / raw)
  To: git; +Cc: Cornelius Weig, szeder.dev, j6t, bitte.keine.werbung.einwerfen

From: Cornelius Weig <cornelius.weig@tngtech.com>

The command line completion for git-checkout bails out when seeing '--'
as an isolated argument. For git-checkout this signifies the start of a
list of files which are to be checked out. Checkout of files makes only
sense for modified files, therefore completion can be a bit smarter:
Instead of bailing out, offer modified files for completion.

Signed-off-by: Cornelius Weig <cornelius.weig@tngtech.com>
---
 contrib/completion/git-completion.bash | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 6c6e1c7..d6523fd 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1059,7 +1059,10 @@ _git_bundle ()
 
 _git_checkout ()
 {
-	__git_has_doubledash && return
+	__git_has_doubledash && {
+		__git_complete_index_file "--modified"
+		return
+	}
 
 	case "$cur" in
 	--conflict=*)
-- 
2.10.2


^ permalink raw reply related

* Re: [PATCH 09/11] refs: move submodule code out of files-backend.c
From: Stefan Beller @ 2017-02-13 23:35 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy
  Cc: git@vger.kernel.org, Junio C Hamano, Michael Haggerty,
	Johannes Schindelin, David Turner
In-Reply-To: <20170213152011.12050-10-pclouds@gmail.com>

On Mon, Feb 13, 2017 at 7:20 AM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
> files-backend is now initialized with a $GIT_DIR. Converting a submodule
> path to where real submodule gitdir is located is done in get_ref_store().
>
> The new code in init_submodule_ref_store() is basically a copy of
> strbuf_git_path_submodule().
>
> This gives a slight performance improvement for submodules since we
> don't convert submodule path to gitdir at every backend call like
> before. We pay that once at ref-store creation.
>
> More cleanup in files_downcast() follows shortly. It's separate to keep
> noises from this patch.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  refs.c               | 48 +++++++++++++++++++++++++++++++++++++++++-------
>  refs/files-backend.c | 25 +++++++------------------
>  refs/refs-internal.h |  6 +++---
>  3 files changed, 51 insertions(+), 28 deletions(-)
>
> diff --git a/refs.c b/refs.c
> index 8ef7a52ba..9ac194945 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -9,6 +9,7 @@
>  #include "refs/refs-internal.h"
>  #include "object.h"
>  #include "tag.h"
> +#include "submodule-config.h"
>
>  /*
>   * List of all available backends
> @@ -1410,7 +1411,7 @@ static void register_ref_store(struct ref_store *refs, const char *submodule)
>   * Create, record, and return a ref_store instance for the specified
>   * submodule (or the main repository if submodule is NULL).
>   */
> -static struct ref_store *ref_store_init(const char *submodule)
> +static struct ref_store *ref_store_init(const char *submodule, const char *gitdir)
>  {
>         const char *be_name = "files";
>         struct ref_storage_be *be = find_ref_storage_backend(be_name);
> @@ -1419,7 +1420,7 @@ static struct ref_store *ref_store_init(const char *submodule)
>         if (!be)
>                 die("BUG: reference backend %s is unknown", be_name);
>
> -       refs = be->init(submodule);
> +       refs = be->init(gitdir);
>         register_ref_store(refs, submodule);
>         return refs;
>  }
> @@ -1445,15 +1446,48 @@ static struct ref_store *lookup_ref_store(const char *submodule)
>         return entry ? entry->refs : NULL;
>  }
>
> -static struct ref_store *init_submodule_ref_store(const char *submodule)
> +static struct ref_store *init_submodule_ref_store(const char *path)
>  {
>         struct strbuf submodule_sb = STRBUF_INIT;
> +       struct strbuf git_submodule_common_dir = STRBUF_INIT;
> +       struct strbuf git_submodule_dir = STRBUF_INIT;
> +       struct strbuf buf = STRBUF_INIT;
> +       const char *git_dir;
> +       const struct submodule *sub;
>         struct ref_store *refs = NULL;
>
> -       strbuf_addstr(&submodule_sb, submodule);
> -       if (is_nonbare_repository_dir(&submodule_sb))
> -               refs = ref_store_init(submodule);
> +       strbuf_addstr(&submodule_sb, path);
> +       if (!is_nonbare_repository_dir(&submodule_sb))
> +               goto done;
> +
> +       strbuf_addstr(&buf, path);
> +       strbuf_complete(&buf, '/');
> +       strbuf_addstr(&buf, ".git");
> +
> +       git_dir = read_gitfile(buf.buf);

if buf.buf is a (git) directory as opposed to a git file,
we error out in read_gitfile. Did you mean to use
read_gitfile_gently here or rather even resolve_gitdir_gently ?

> +       if (git_dir) {

when not using the _gently version git_dir is always
non NULL here (or we're dead)?

> +               strbuf_reset(&buf);
> +               strbuf_addstr(&buf, git_dir);
> +       }
> +       if (!is_git_directory(buf.buf)) {
> +               gitmodules_config();
> +               sub = submodule_from_path(null_sha1, path);
> +               if (!sub)
> +                       goto done;
> +               strbuf_reset(&buf);
> +               strbuf_git_path(&buf, "%s/%s", "modules", sub->name);

You can inline "modules" into the format string?

> +       }
> +       strbuf_addch(&buf, '/');
> +       strbuf_addbuf(&git_submodule_dir, &buf);
> +
> +       refs = ref_store_init(path, git_submodule_dir.buf);

strbuf_detach (git_submodule_dir) here, such that we keep
the string alive despite the release of the strbuf below?

so essentially this function
* takes a submodule path
* checks if there is a repo at the given path in the working tree
* resolves the gitfile if any
* if the gitfile could not resolve to a valid repo just make up the
  location to be $GIT_DIR/modules/<name>

sounds confusing to me. I need to reread it later.

>
> -       if (submodule) {
> -               refs->submodule = xstrdup_or_null(submodule);
> +       if (gitdir) {
> +               strbuf_addstr(&refs->gitdir, gitdir);
> +               get_common_dir_noenv(&refs->gitcommondir, gitdir);

Oh I see. we loose the _or_null here, so my remark on the previous patch
might be just unneeded work.

>         } else {
>                 strbuf_addstr(&refs->gitdir, get_git_dir());
>                 strbuf_addstr(&refs->gitcommondir, get_git_common_dir());
> @@ -1034,8 +1025,6 @@ static struct ref_store *files_ref_store_create(const char *submodule)
>  static void files_assert_main_repository(struct files_ref_store *refs,
>                                          const char *caller)
>  {
> -       if (refs->submodule)
> -               die("BUG: %s called for a submodule", caller);
>  }

In a followup we'd get rid of files_assert_main_repository
presumably?

Thanks,
Stefan

^ permalink raw reply

* Re: [PATCH 10/11] files-backend: remove submodule_allowed from files_downcast()
From: Stefan Beller @ 2017-02-13 23:44 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy
  Cc: git@vger.kernel.org, Junio C Hamano, Michael Haggerty,
	Johannes Schindelin, David Turner
In-Reply-To: <20170213152011.12050-11-pclouds@gmail.com>

On Mon, Feb 13, 2017 at 7:20 AM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
> Since submodule or not is irrelevant to files-backend after the last
> patch, remove the parameter from files_downcast() and kill
> files_assert_main_repository().
>
> PS. This implies that all ref operations are allowed for submodules. But
> we may need to look more closely to see if that's really true...
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---

Looks good,
Stefan

^ permalink raw reply

* Re: Continuous Testing of Git on Windows
From: Junio C Hamano @ 2017-02-13 23:46 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git-for-windows, git
In-Reply-To: <alpine.DEB.2.20.1702101241210.3496@virtualbox>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> That is why I taught the Git for Windows CI job that tests the four
> upstream Git integration branches to *also* bisect test breakages and then
> upload comments to the identified commit on GitHub

Good.  I do not think it is useful to try 'pu' as an aggregate and
expect it to always build and work [*1*], but your "bisect and
pinpoint" approach makes it useful to identify individual topic that
brings in a breakage.  I wouldn't be surprised if original submitter
and I were the only two people who actually compiled the patches on
a topic in isolation while a topic is in 'pu', and chances are that
these two people didn't try their builds on Windows.  A CI like this
one will help the coverage to stop premature topics from advancing
to 'pu' without getting any Windows exposure.

Thanks.


[Footnote]

*1* The reason why topics not in 'next' but in 'pu', especially the
    ones merged near the tip of 'pu', exist in 'pu' are because they
    are interesting enough and could be polished to become eligible
    for 'next' but known to be premature for 'next' yet.  They are
    there primarily to give human contributors an easier way to
    download them as a whole and help polish them.  And I have to be
    selective when I queue things on 'pu'; it is not like I have
    infinite amount of time to pick up any cruft that is sent to the
    list.

^ permalink raw reply

* Re: [PATCH] clean: use warning_errno() when appropriate
From: Junio C Hamano @ 2017-02-13 23:48 UTC (permalink / raw)
  To: Jeff King; +Cc: Nguyễn Thái Ngọc Duy, git
In-Reply-To: <20170213222210.dtfnxbbi77tsx4a6@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> So in that sense doing the errno dance as close to the caller who cares
> is the most _obvious_ thing, even if it isn't the shortest.

Yup.

> It would be nice if there was a way to annotate a function as
> errno-safe, and then transitively compute which other functions were
> errno-safe when they do not call any errno-unsafe function. I don't know
> if any static analyzers allow that kind of custom annotation, though
> (and also I wonder whether the cost/benefit of maintaining those
> annotations would be worth it).

Double yup.

^ permalink raw reply

* Re: [PATCH 11/11] refs: split and make get_*_ref_store() public API
From: Stefan Beller @ 2017-02-13 23:55 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy
  Cc: git@vger.kernel.org, Junio C Hamano, Michael Haggerty,
	Johannes Schindelin, David Turner
In-Reply-To: <20170213152011.12050-12-pclouds@gmail.com>

>
> +/*
> + * Return the ref_store instance for the specified submodule. For the
> + * main repository, use submodule==NULL; such a call cannot fail.

So now we have both a get_main as well as a get_submodule function,
but the submodule function can return the main as well?

I'd rather see this as a BUG; or asking another way:
What is the difference between get_submodule_ref_store(NULL)
and get_main_ref_store() ?

As you went through all call sites (by renaming the function), we'd
be able to tell that there is no caller with NULL, or is it?

Stefan

> For
> + * a submodule, the submodule must exist and be a nonbare repository,
> + * otherwise return NULL. If the requested reference store has not yet
> + * been initialized, initialize it first.
> + *
> + * For backwards compatibility, submodule=="" is treated the same as
> + * submodule==NULL.
> + */
> +struct ref_store *get_submodule_ref_store(const char *submodule);
> +struct ref_store *get_main_ref_store(void);

^ permalink raw reply

* [PATCH for NEXT] grep: do not unnecessarily query repo for "--"
From: Jonathan Tan @ 2017-02-14  0:11 UTC (permalink / raw)
  To: git; +Cc: Jonathan Tan, peff, gitster

When running a command of the form

  git grep --no-index pattern -- path

in the absence of a Git repository, an error message will be printed:

  fatal: BUG: setup_git_env called without repository

This is because "git grep" tries to interpret "--" as a rev. "git grep"
has always tried to first interpret "--" as a rev for at least a few
years, but this issue was upgraded from a pessimization to a bug in
commit 59332d1 ("Resurrect "git grep --no-index"", 2010-02-06), which
calls get_sha1 regardless of whether --no-index was specified. This bug
appeared to be benign until commit b1ef400 ("setup_git_env: avoid blind
fall-back to ".git"", 2016-10-20) when Git was taught to die in this
situation.  (This "git grep" bug appears to be one of the bugs that
commit b1ef400 is meant to flush out.)

Therefore, always interpret "--" as signaling the end of options,
instead of trying to interpret it as a rev first.

Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
---

There is probably a similar bug for commands of the form:

  git grep --no-index pattern foo

If there is a repo and "foo" is a rev, the "--no-index or --untracked
cannot be used with revs." error would occur. If there is a repo and
"foo" is not a rev, this command would proceed as usual. If there is no
repo, the "setup_git_env called without repository" error would occur.
(This is my understanding from reading the code - I haven't tested it
out.)

This patch does not fix this similar bug, but I decided to send it out
anyway because it still fixes a bug and unlocks the ability to
specify paths with "git grep --no-index".

 builtin/grep.c  |  9 +++++----
 t/t7810-grep.sh | 12 ++++++++++++
 2 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/builtin/grep.c b/builtin/grep.c
index 2c727ef49..1b68d1638 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -1154,6 +1154,11 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 		const char *arg = argv[i];
 		unsigned char sha1[20];
 		struct object_context oc;
+		if (!strcmp(arg, "--")) {
+			i++;
+			seen_dashdash = 1;
+			break;
+		}
 		/* Is it a rev? */
 		if (!get_sha1_with_context(arg, 0, sha1, &oc)) {
 			struct object *object = parse_object_or_die(sha1, arg);
@@ -1162,10 +1167,6 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 			add_object_array_with_path(object, arg, &list, oc.mode, oc.path);
 			continue;
 		}
-		if (!strcmp(arg, "--")) {
-			i++;
-			seen_dashdash = 1;
-		}
 		break;
 	}
 
diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
index 19f0108f8..29202f0e7 100755
--- a/t/t7810-grep.sh
+++ b/t/t7810-grep.sh
@@ -982,6 +982,18 @@ test_expect_success 'grep -e -- -- path' '
 	test_cmp expected actual
 '
 
+test_expect_success 'grep --no-index pattern -- path' '
+	rm -fr non &&
+	mkdir -p non/git &&
+	(
+		GIT_CEILING_DIRECTORIES="$(pwd)/non" &&
+		export GIT_CEILING_DIRECTORIES &&
+		cd non/git &&
+		echo hello >hello &&
+		git grep --no-index o -- .
+	)
+'
+
 cat >expected <<EOF
 hello.c:int main(int argc, const char **argv)
 hello.c:	printf("Hello world.\n");
-- 
2.11.0.483.g087da7b7c-goog


^ permalink raw reply related

* Re: [PATCH v3 0/5] stash: support pathspec argument
From: Jeff King @ 2017-02-14  0:27 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Thomas Gummerer, Matthieu Moy, git, Stephan Beyer, Marc Strapetz,
	Johannes Schindelin, Øyvind A . Holm, Jakub Narębski
In-Reply-To: <xmqqwpctabvw.fsf@gitster.mtv.corp.google.com>

On Mon, Feb 13, 2017 at 03:50:43PM -0800, Junio C Hamano wrote:

> Thomas Gummerer <t.gummerer@gmail.com> writes:
> 
> >> My "-p" suggestion suffers from a similar problem if you treat it as
> >> "you can omit the 'push' if you say "-p", rather than "if -p is the
> >> first option, it is a synonym for 'push -p'".
> >
> > I'm almost convinced of special casing "-p".  (Maybe I'm easy to
> > convince as well, because it would be convenient ;) ) However it's a
> > bit weird that now "git stash -p file" would work, but "git stash -m
> > message" wouldn't.
> 
> I am not sure why this matters.  The original "git stash <msg>" was
> just "Are you being extremely busy and cannot even afford to type
> 'save'?  Ok, let me assume you meant that!".  Now we are talking
> about picking and choosing hunks carefully going through interactive
> process, I really do not think there is any justification to infer
> 'push' when 'push' was omitted in "git stash push -p" the user wants
> to do.

Maybe it is just me and my muscle memory, but "git stash -p" is quite a
common command for me[1]. And I have typed "git stash -p foo" many times
and been annoyed that it didn't work. I was hoping to end that
annoyance.

I guess I could make an alias and retrain my fingers.

-Peff

[1] I almost never run "reset --hard", preferring instead to stash away
    changes just in case I would change my mind later and want them. And
    I quite often use "stash -p" because I like to double check what I
    am throwing away.

    I also use "stash -p" heavily when picking apart changes from the
    working tree.

^ permalink raw reply

* Re: [PATCH v3 0/5] stash: support pathspec argument
From: Jeff King @ 2017-02-14  0:37 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Thomas Gummerer, Matthieu Moy, git, Stephan Beyer, Marc Strapetz,
	Johannes Schindelin, Øyvind A . Holm, Jakub Narębski
In-Reply-To: <xmqqpoila9rt.fsf@gitster.mtv.corp.google.com>

On Mon, Feb 13, 2017 at 04:36:22PM -0800, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > Maybe it is just me and my muscle memory, but "git stash -p" is quite a
> > common command for me[1]. And I have typed "git stash -p foo" many times
> > and been annoyed that it didn't work. I was hoping to end that
> > annoyance.
> 
> OK, I never run "stash -p" and did not realize people already find
> it useful that it becomes "stash save -p".  Please ignore me, then.
> I agree that breaking the established use is not nice.

To be fair, I don't think anybody is proposing breaking the established
use of "stash -p". I was just hoping the new pathspec feature could
trickle down to it, as well. :)

-Peff

^ permalink raw reply

* Re: [RFC-PATCHv2] submodules: add a background story
From: Brandon Williams @ 2017-02-14  0:39 UTC (permalink / raw)
  To: Stefan Beller; +Cc: git
In-Reply-To: <20170209020855.23486-1-sbeller@google.com>

On 02/08, Stefan Beller wrote:
> +STATES
> +------
> +
> +When working with submodules, you can think of them as in a state machine.
> +So each submodule can be in a different state, the following indicators are used:
> +
> +* the existence of the setting of 'submodule.<name>.url' in the
> +  superprojects configuration
> +* the existence of the submodules working tree within the
> +  working tree of the superproject
> +* the existence of the submodules git directory within the superprojects
> +  git directory at $GIT_DIR/modules/<name> or within the submodules working
> +  tree
> +
> +      State      URL config        working tree     git dir
> +      -----------------------------------------------------
> +      uninitialized    no               no           no
> +      initialized     yes               no           no
> +      populated       yes              yes          yes
> +      depopulated     yes               no          yes
> +      deinitialized    no               no          yes
> +      uninteresting    no              yes          yes
> +
> +      invalid          no              yes           no
> +      invalid         yes              yes           no
> +      -----------------------------------------------------
> +
> +The first six states can be reached by normal git usage, the latter two are
> +only shown for completeness to show all possible eight states with 3 binary
> +indicators. The states in detail:
> +
> +uninitialized::
> +The uninitialized state is the default state if no
> +'--recurse-submodules' / '--recursive'. An empty directory will be put in
> +the working tree as a place holder, such that you are reminded of the
> +existence of the submodule.
> +---
> +To transition into the initialized state
> +you can use 'git submodule init', which copies the presets from the
> +.gitmodules file into the config.
> +
> +initialized::
> +Users transitioned from the uninitialized state to this state via
> +'git submodule init', which preset the URL configuration. As these URLs
> +may not be desired in certain scenarios, this state allows to change the
> +URLs.  For example in a corporate environment you may want to run
> +
> +    sed -i s/example.org/$internal-mirror/ .git/config
> ++

Maybe we can try to brainstorm and come up with some clearer terminology
while we are at it.  I was trying to think about the "initialized" state
and I may be the only one but it seems unclear what "initialized" means.
I mean I already have all the information about a submodule in the
.gitmodules file, isn't it already initialized then?   Maybe this state
would be better named "(in)active" as a module that is interesting to a
user is "active"?

-- 
Brandon Williams

^ permalink raw reply

* Re: [PATCH] completion: complete modified files for checkout with '--'
From: SZEDER Gábor @ 2017-02-14  0:50 UTC (permalink / raw)
  To: Cornelius Weig; +Cc: Git mailing list, j6t, Richard Wagner
In-Reply-To: <20170213233359.11149-1-cornelius.weig@tngtech.com>

On Tue, Feb 14, 2017 at 12:33 AM,  <cornelius.weig@tngtech.com> wrote:
> From: Cornelius Weig <cornelius.weig@tngtech.com>
>
> The command line completion for git-checkout bails out when seeing '--'
> as an isolated argument. For git-checkout this signifies the start of a
> list of files which are to be checked out. Checkout of files makes only
> sense for modified files,

No, there is e.g. 'git checkout that-branch this-path', too.


> therefore completion can be a bit smarter:
> Instead of bailing out, offer modified files for completion.
>
> Signed-off-by: Cornelius Weig <cornelius.weig@tngtech.com>
> ---
>  contrib/completion/git-completion.bash | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index 6c6e1c7..d6523fd 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -1059,7 +1059,10 @@ _git_bundle ()
>
>  _git_checkout ()
>  {
> -       __git_has_doubledash && return
> +       __git_has_doubledash && {
> +               __git_complete_index_file "--modified"
> +               return
> +       }
>
>         case "$cur" in
>         --conflict=*)
> --
> 2.10.2
>

^ permalink raw reply

* Re: [PATCH for NEXT] grep: do not unnecessarily query repo for "--"
From: Jeff King @ 2017-02-14  1:20 UTC (permalink / raw)
  To: Jonathan Tan; +Cc: git, gitster
In-Reply-To: <20170214001159.19079-1-jonathantanmy@google.com>

On Mon, Feb 13, 2017 at 04:11:59PM -0800, Jonathan Tan wrote:

> When running a command of the form
> 
>   git grep --no-index pattern -- path
> 
> in the absence of a Git repository, an error message will be printed:
> 
>   fatal: BUG: setup_git_env called without repository
> 
> This is because "git grep" tries to interpret "--" as a rev. "git grep"
> has always tried to first interpret "--" as a rev for at least a few
> years, but this issue was upgraded from a pessimization to a bug in
> commit 59332d1 ("Resurrect "git grep --no-index"", 2010-02-06), which
> calls get_sha1 regardless of whether --no-index was specified. This bug
> appeared to be benign until commit b1ef400 ("setup_git_env: avoid blind
> fall-back to ".git"", 2016-10-20) when Git was taught to die in this
> situation.  (This "git grep" bug appears to be one of the bugs that
> commit b1ef400 is meant to flush out.)
> 
> Therefore, always interpret "--" as signaling the end of options,
> instead of trying to interpret it as a rev first.

Nicely explained.

> There is probably a similar bug for commands of the form:
> 
>   git grep --no-index pattern foo
> 
> If there is a repo and "foo" is a rev, the "--no-index or --untracked
> cannot be used with revs." error would occur. If there is a repo and
> "foo" is not a rev, this command would proceed as usual. If there is no
> repo, the "setup_git_env called without repository" error would occur.
> (This is my understanding from reading the code - I haven't tested it
> out.)

Yes, it's easy to see that "git grep --no-index foo bar" outside of a
repo generates the same BUG. I suspect that "--no-index" should just
disable looking up revs entirely, even if we are actually in a
repository directory.

> This patch does not fix this similar bug, but I decided to send it out
> anyway because it still fixes a bug and unlocks the ability to
> specify paths with "git grep --no-index".

Yes, I think even if we fix the other bug, fixing this "--" thing is an
improvement.

> diff --git a/builtin/grep.c b/builtin/grep.c
> index 2c727ef49..1b68d1638 100644
> --- a/builtin/grep.c
> +++ b/builtin/grep.c
> @@ -1154,6 +1154,11 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
>  		const char *arg = argv[i];
>  		unsigned char sha1[20];
>  		struct object_context oc;
> +		if (!strcmp(arg, "--")) {
> +			i++;
> +			seen_dashdash = 1;
> +			break;
> +		}
>  		/* Is it a rev? */
>  		if (!get_sha1_with_context(arg, 0, sha1, &oc)) {
>  			struct object *object = parse_object_or_die(sha1, arg);

So I think this is a definite improvement, but I see a few leftover
oddities:

  - the end logic for this loop is now:

      if (arg is a rev) {
         ... handle rev ...
         continue;
      }
      break;

    It would probably be more obvious as:

      if (arg is not a rev)
          break;
      ... handle rev ...

  - the rev-handling code does:

      if (!seen_dashdash)
          verify_non_filename(prefix, arg);

    But I do not see how seen_dashdash could ever be untrue. We set it
    inside this loop, and break immediately when we see it. And indeed,
    running:

      echo content >master
      git grep content master --

    does not work. The "--" should tell us that "master" is a rev, but
    we don't know yet that we have a dashdash.

    I think we need a separate loop to find the "--" first, and _then_
    walk through the arguments, treating them as revs or paths as
    appropriate. This is how setup_revisions() does it.

    So this isn't a problem introduced by your patch, but it's
    intimately related.

> diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
> index 19f0108f8..29202f0e7 100755
> --- a/t/t7810-grep.sh
> +++ b/t/t7810-grep.sh
> @@ -982,6 +982,18 @@ test_expect_success 'grep -e -- -- path' '
>  	test_cmp expected actual
>  '
>  
> +test_expect_success 'grep --no-index pattern -- path' '
> +	rm -fr non &&
> +	mkdir -p non/git &&
> +	(
> +		GIT_CEILING_DIRECTORIES="$(pwd)/non" &&
> +		export GIT_CEILING_DIRECTORIES &&
> +		cd non/git &&
> +		echo hello >hello &&
> +		git grep --no-index o -- .
> +	)
> +'

Since de95302a4, you can do:

  nongit git grep --no-index -o -- .

Though if this is destined for maint, it might need to be done
separately this way and cleaned up later.

It might also be a good idea to confirm that the pathspec is actually
being respected in the --no-index case. Something like:

  echo hello >hello &&
  echo goodbye >goodbye &&
  echo hello:hello >expect &&
  git grep --no-index o -- hello >actual &&
  test_cmp expect actual

-Peff

^ permalink raw reply

* [PATCH v2 03/19] builtin/describe: convert to struct object_id
From: brian m. carlson @ 2017-02-14  2:31 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Michael Haggerty
In-Reply-To: <20170214023141.842922-1-sandals@crustytoothpaste.net>

Convert the functions in this file and struct commit_name  to struct
object_id.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 builtin/describe.c | 50 +++++++++++++++++++++++++-------------------------
 1 file changed, 25 insertions(+), 25 deletions(-)

diff --git a/builtin/describe.c b/builtin/describe.c
index 01490a157e..738e68f95b 100644
--- a/builtin/describe.c
+++ b/builtin/describe.c
@@ -39,11 +39,11 @@ static const char *diff_index_args[] = {
 
 struct commit_name {
 	struct hashmap_entry entry;
-	unsigned char peeled[20];
+	struct object_id peeled;
 	struct tag *tag;
 	unsigned prio:2; /* annotated tag = 2, tag = 1, head = 0 */
 	unsigned name_checked:1;
-	unsigned char sha1[20];
+	struct object_id oid;
 	char *path;
 };
 
@@ -54,17 +54,17 @@ static const char *prio_names[] = {
 static int commit_name_cmp(const struct commit_name *cn1,
 		const struct commit_name *cn2, const void *peeled)
 {
-	return hashcmp(cn1->peeled, peeled ? peeled : cn2->peeled);
+	return oidcmp(&cn1->peeled, peeled ? peeled : &cn2->peeled);
 }
 
-static inline struct commit_name *find_commit_name(const unsigned char *peeled)
+static inline struct commit_name *find_commit_name(const struct object_id *peeled)
 {
-	return hashmap_get_from_hash(&names, sha1hash(peeled), peeled);
+	return hashmap_get_from_hash(&names, sha1hash(peeled->hash), peeled->hash);
 }
 
 static int replace_name(struct commit_name *e,
 			       int prio,
-			       const unsigned char *sha1,
+			       const struct object_id *oid,
 			       struct tag **tag)
 {
 	if (!e || e->prio < prio)
@@ -77,13 +77,13 @@ static int replace_name(struct commit_name *e,
 		struct tag *t;
 
 		if (!e->tag) {
-			t = lookup_tag(e->sha1);
+			t = lookup_tag(e->oid.hash);
 			if (!t || parse_tag(t))
 				return 1;
 			e->tag = t;
 		}
 
-		t = lookup_tag(sha1);
+		t = lookup_tag(oid->hash);
 		if (!t || parse_tag(t))
 			return 0;
 		*tag = t;
@@ -96,24 +96,24 @@ static int replace_name(struct commit_name *e,
 }
 
 static void add_to_known_names(const char *path,
-			       const unsigned char *peeled,
+			       const struct object_id *peeled,
 			       int prio,
-			       const unsigned char *sha1)
+			       const struct object_id *oid)
 {
 	struct commit_name *e = find_commit_name(peeled);
 	struct tag *tag = NULL;
-	if (replace_name(e, prio, sha1, &tag)) {
+	if (replace_name(e, prio, oid, &tag)) {
 		if (!e) {
 			e = xmalloc(sizeof(struct commit_name));
-			hashcpy(e->peeled, peeled);
-			hashmap_entry_init(e, sha1hash(peeled));
+			oidcpy(&e->peeled, peeled);
+			hashmap_entry_init(e, sha1hash(peeled->hash));
 			hashmap_add(&names, e);
 			e->path = NULL;
 		}
 		e->tag = tag;
 		e->prio = prio;
 		e->name_checked = 0;
-		hashcpy(e->sha1, sha1);
+		oidcpy(&e->oid, oid);
 		free(e->path);
 		e->path = xstrdup(path);
 	}
@@ -154,7 +154,7 @@ static int get_name(const char *path, const struct object_id *oid, int flag, voi
 	else
 		prio = 0;
 
-	add_to_known_names(all ? path + 5 : path + 10, peeled.hash, prio, oid->hash);
+	add_to_known_names(all ? path + 5 : path + 10, &peeled, prio, oid);
 	return 0;
 }
 
@@ -212,7 +212,7 @@ static unsigned long finish_depth_computation(
 static void display_name(struct commit_name *n)
 {
 	if (n->prio == 2 && !n->tag) {
-		n->tag = lookup_tag(n->sha1);
+		n->tag = lookup_tag(n->oid.hash);
 		if (!n->tag || parse_tag(n->tag))
 			die(_("annotated tag %s not available"), n->path);
 	}
@@ -230,14 +230,14 @@ static void display_name(struct commit_name *n)
 		printf("%s", n->path);
 }
 
-static void show_suffix(int depth, const unsigned char *sha1)
+static void show_suffix(int depth, const struct object_id *oid)
 {
-	printf("-%d-g%s", depth, find_unique_abbrev(sha1, abbrev));
+	printf("-%d-g%s", depth, find_unique_abbrev(oid->hash, abbrev));
 }
 
 static void describe(const char *arg, int last_one)
 {
-	unsigned char sha1[20];
+	struct object_id oid;
 	struct commit *cmit, *gave_up_on = NULL;
 	struct commit_list *list;
 	struct commit_name *n;
@@ -246,20 +246,20 @@ static void describe(const char *arg, int last_one)
 	unsigned long seen_commits = 0;
 	unsigned int unannotated_cnt = 0;
 
-	if (get_sha1(arg, sha1))
+	if (get_oid(arg, &oid))
 		die(_("Not a valid object name %s"), arg);
-	cmit = lookup_commit_reference(sha1);
+	cmit = lookup_commit_reference(oid.hash);
 	if (!cmit)
 		die(_("%s is not a valid '%s' object"), arg, commit_type);
 
-	n = find_commit_name(cmit->object.oid.hash);
+	n = find_commit_name(&cmit->object.oid);
 	if (n && (tags || all || n->prio == 2)) {
 		/*
 		 * Exact match to an existing ref.
 		 */
 		display_name(n);
 		if (longformat)
-			show_suffix(0, n->tag ? n->tag->tagged->oid.hash : sha1);
+			show_suffix(0, n->tag ? &n->tag->tagged->oid : &oid);
 		if (dirty)
 			printf("%s", dirty);
 		printf("\n");
@@ -276,7 +276,7 @@ static void describe(const char *arg, int last_one)
 		struct commit *c;
 		struct commit_name *n = hashmap_iter_first(&names, &iter);
 		for (; n; n = hashmap_iter_next(&iter)) {
-			c = lookup_commit_reference_gently(n->peeled, 1);
+			c = lookup_commit_reference_gently(n->peeled.hash, 1);
 			if (c)
 				c->util = n;
 		}
@@ -380,7 +380,7 @@ static void describe(const char *arg, int last_one)
 
 	display_name(all_matches[0].name);
 	if (abbrev)
-		show_suffix(all_matches[0].depth, cmit->object.oid.hash);
+		show_suffix(all_matches[0].depth, &cmit->object.oid);
 	if (dirty)
 		printf("%s", dirty);
 	printf("\n");
-- 
2.11.0


^ permalink raw reply related

* [PATCH v2 16/19] sha1_file: introduce an nth_packed_object_oid function
From: brian m. carlson @ 2017-02-14  2:31 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Michael Haggerty
In-Reply-To: <20170214023141.842922-1-sandals@crustytoothpaste.net>

There are places in the code where we would like to provide a struct
object_id *, yet read the hash directly from the pack.  Provide an
nth_packed_object_oid function that is similar to the
nth_packed_object_sha1 function.

In order to avoid a potentially invalid cast, nth_packed_object_oid
provides a variable into which to store the value, which it returns on
success; on error, it returns NULL, as nth_packed_object_sha1 does.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 cache.h     |  6 ++++++
 sha1_file.c | 17 ++++++++++++++---
 2 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/cache.h b/cache.h
index 5dc89a058c..04b1d923f3 100644
--- a/cache.h
+++ b/cache.h
@@ -1607,6 +1607,12 @@ extern void check_pack_index_ptr(const struct packed_git *p, const void *ptr);
  * error.
  */
 extern const unsigned char *nth_packed_object_sha1(struct packed_git *, uint32_t n);
+/*
+ * Like nth_packed_object_oid, but write the data into the object specified by
+ * the the first argument.  Returns the first argument on success, and NULL on
+ * error.
+ */
+extern const struct object_id *nth_packed_object_oid(struct object_id *, struct packed_git *, uint32_t n);
 
 /*
  * Return the offset of the nth object within the specified packfile.
diff --git a/sha1_file.c b/sha1_file.c
index ec957db5e1..777b8e8eae 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -2628,6 +2628,17 @@ const unsigned char *nth_packed_object_sha1(struct packed_git *p,
 	}
 }
 
+const struct object_id *nth_packed_object_oid(struct object_id *oid,
+					      struct packed_git *p,
+					      uint32_t n)
+{
+	const unsigned char *hash = nth_packed_object_sha1(p, n);
+	if (!hash)
+		return NULL;
+	hashcpy(oid->hash, hash);
+	return oid;
+}
+
 void check_pack_index_ptr(const struct packed_git *p, const void *vptr)
 {
 	const unsigned char *ptr = vptr;
@@ -3788,13 +3799,13 @@ static int for_each_object_in_pack(struct packed_git *p, each_packed_object_fn c
 	int r = 0;
 
 	for (i = 0; i < p->num_objects; i++) {
-		const unsigned char *sha1 = nth_packed_object_sha1(p, i);
+		struct object_id oid;
 
-		if (!sha1)
+		if (!nth_packed_object_oid(&oid, p, i))
 			return error("unable to get sha1 of object %u in %s",
 				     i, p->pack_name);
 
-		r = cb(sha1, p, i, data);
+		r = cb(oid.hash, p, i, data);
 		if (r)
 			break;
 	}
-- 
2.11.0


^ permalink raw reply related

* [PATCH v2 11/19] builtin/replace: convert to struct object_id
From: brian m. carlson @ 2017-02-14  2:31 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Michael Haggerty
In-Reply-To: <20170214023141.842922-1-sandals@crustytoothpaste.net>

Convert various uses of unsigned char [20] to struct object_id.  Rename
replace_object_sha1 to rename_object_oid.  Finally, specify a constant
in terms of GIT_SHA1_HEXSZ.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 builtin/replace.c | 112 +++++++++++++++++++++++++++---------------------------
 1 file changed, 56 insertions(+), 56 deletions(-)

diff --git a/builtin/replace.c b/builtin/replace.c
index b58c714cb8..f7716a5472 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -88,78 +88,78 @@ static int list_replace_refs(const char *pattern, const char *format)
 }
 
 typedef int (*each_replace_name_fn)(const char *name, const char *ref,
-				    const unsigned char *sha1);
+				    const struct object_id *oid);
 
 static int for_each_replace_name(const char **argv, each_replace_name_fn fn)
 {
 	const char **p, *full_hex;
 	char ref[PATH_MAX];
 	int had_error = 0;
-	unsigned char sha1[20];
+	struct object_id oid;
 
 	for (p = argv; *p; p++) {
-		if (get_sha1(*p, sha1)) {
+		if (get_oid(*p, &oid)) {
 			error("Failed to resolve '%s' as a valid ref.", *p);
 			had_error = 1;
 			continue;
 		}
-		full_hex = sha1_to_hex(sha1);
+		full_hex = oid_to_hex(&oid);
 		snprintf(ref, sizeof(ref), "%s%s", git_replace_ref_base, full_hex);
 		/* read_ref() may reuse the buffer */
 		full_hex = ref + strlen(git_replace_ref_base);
-		if (read_ref(ref, sha1)) {
+		if (read_ref(ref, oid.hash)) {
 			error("replace ref '%s' not found.", full_hex);
 			had_error = 1;
 			continue;
 		}
-		if (fn(full_hex, ref, sha1))
+		if (fn(full_hex, ref, &oid))
 			had_error = 1;
 	}
 	return had_error;
 }
 
 static int delete_replace_ref(const char *name, const char *ref,
-			      const unsigned char *sha1)
+			      const struct object_id *oid)
 {
-	if (delete_ref(ref, sha1, 0))
+	if (delete_ref(ref, oid->hash, 0))
 		return 1;
 	printf("Deleted replace ref '%s'\n", name);
 	return 0;
 }
 
-static void check_ref_valid(unsigned char object[20],
-			    unsigned char prev[20],
+static void check_ref_valid(struct object_id *object,
+			    struct object_id *prev,
 			    char *ref,
 			    int ref_size,
 			    int force)
 {
 	if (snprintf(ref, ref_size,
 		     "%s%s", git_replace_ref_base,
-		     sha1_to_hex(object)) > ref_size - 1)
+		     oid_to_hex(object)) > ref_size - 1)
 		die("replace ref name too long: %.*s...", 50, ref);
 	if (check_refname_format(ref, 0))
 		die("'%s' is not a valid ref name.", ref);
 
-	if (read_ref(ref, prev))
-		hashclr(prev);
+	if (read_ref(ref, prev->hash))
+		oidclr(prev);
 	else if (!force)
 		die("replace ref '%s' already exists", ref);
 }
 
-static int replace_object_sha1(const char *object_ref,
-			       unsigned char object[20],
+static int replace_object_oid(const char *object_ref,
+			       struct object_id *object,
 			       const char *replace_ref,
-			       unsigned char repl[20],
+			       struct object_id *repl,
 			       int force)
 {
-	unsigned char prev[20];
+	struct object_id prev;
 	enum object_type obj_type, repl_type;
 	char ref[PATH_MAX];
 	struct ref_transaction *transaction;
 	struct strbuf err = STRBUF_INIT;
 
-	obj_type = sha1_object_info(object, NULL);
-	repl_type = sha1_object_info(repl, NULL);
+	obj_type = sha1_object_info(object->hash, NULL);
+	repl_type = sha1_object_info(repl->hash, NULL);
 	if (!force && obj_type != repl_type)
 		die("Objects must be of the same type.\n"
 		    "'%s' points to a replaced object of type '%s'\n"
@@ -167,11 +167,11 @@ static int replace_object_sha1(const char *object_ref,
 		    object_ref, typename(obj_type),
 		    replace_ref, typename(repl_type));
 
-	check_ref_valid(object, prev, ref, sizeof(ref), force);
+	check_ref_valid(object, &prev, ref, sizeof(ref), force);
 
 	transaction = ref_transaction_begin(&err);
 	if (!transaction ||
-	    ref_transaction_update(transaction, ref, repl, prev,
+	    ref_transaction_update(transaction, ref, repl->hash, prev.hash,
 				   0, NULL, &err) ||
 	    ref_transaction_commit(transaction, &err))
 		die("%s", err.buf);
@@ -182,14 +182,14 @@ static int replace_object_sha1(const char *object_ref,
 
 static int replace_object(const char *object_ref, const char *replace_ref, int force)
 {
-	unsigned char object[20], repl[20];
+	struct object_id object, repl;
 
-	if (get_sha1(object_ref, object))
+	if (get_oid(object_ref, &object))
 		die("Failed to resolve '%s' as a valid ref.", object_ref);
-	if (get_sha1(replace_ref, repl))
+	if (get_oid(replace_ref, &repl))
 		die("Failed to resolve '%s' as a valid ref.", replace_ref);
 
-	return replace_object_sha1(object_ref, object, replace_ref, repl, force);
+	return replace_object_oid(object_ref, &object, replace_ref, &repl, force);
 }
 
 /*
@@ -197,7 +197,7 @@ static int replace_object(const char *object_ref, const char *replace_ref, int f
  * If "raw" is true, then the object's raw contents are printed according to
  * "type". Otherwise, we pretty-print the contents for human editing.
  */
-static void export_object(const unsigned char *sha1, enum object_type type,
+static void export_object(const struct object_id *oid, enum object_type type,
 			  int raw, const char *filename)
 {
 	struct child_process cmd = CHILD_PROCESS_INIT;
@@ -213,7 +213,7 @@ static void export_object(const unsigned char *sha1, enum object_type type,
 		argv_array_push(&cmd.args, typename(type));
 	else
 		argv_array_push(&cmd.args, "-p");
-	argv_array_push(&cmd.args, sha1_to_hex(sha1));
+	argv_array_push(&cmd.args, oid_to_hex(oid));
 	cmd.git_cmd = 1;
 	cmd.out = fd;
 
@@ -226,7 +226,7 @@ static void export_object(const unsigned char *sha1, enum object_type type,
  * interpreting it as "type", and writing the result to the object database.
  * The sha1 of the written object is returned via sha1.
  */
-static void import_object(unsigned char *sha1, enum object_type type,
+static void import_object(struct object_id *oid, enum object_type type,
 			  int raw, const char *filename)
 {
 	int fd;
@@ -254,7 +254,7 @@ static void import_object(unsigned char *sha1, enum object_type type,
 
 		if (finish_command(&cmd))
 			die("mktree reported failure");
-		if (get_sha1_hex(result.buf, sha1) < 0)
+		if (get_oid_hex(result.buf, oid) < 0)
 			die("mktree did not return an object name");
 
 		strbuf_release(&result);
@@ -264,7 +264,7 @@ static void import_object(unsigned char *sha1, enum object_type type,
 
 		if (fstat(fd, &st) < 0)
 			die_errno("unable to fstat %s", filename);
-		if (index_fd(sha1, fd, &st, type, NULL, flags) < 0)
+		if (index_fd(oid->hash, fd, &st, type, NULL, flags) < 0)
 			die("unable to write object to database");
 		/* index_fd close()s fd for us */
 	}
@@ -279,29 +279,29 @@ static int edit_and_replace(const char *object_ref, int force, int raw)
 {
 	char *tmpfile = git_pathdup("REPLACE_EDITOBJ");
 	enum object_type type;
-	unsigned char old[20], new[20], prev[20];
+	struct object_id old, new, prev;
 	char ref[PATH_MAX];
 
-	if (get_sha1(object_ref, old) < 0)
+	if (get_oid(object_ref, &old) < 0)
 		die("Not a valid object name: '%s'", object_ref);
 
-	type = sha1_object_info(old, NULL);
+	type = sha1_object_info(old.hash, NULL);
 	if (type < 0)
-		die("unable to get object type for %s", sha1_to_hex(old));
+		die("unable to get object type for %s", oid_to_hex(&old));
 
-	check_ref_valid(old, prev, ref, sizeof(ref), force);
+	check_ref_valid(&old, &prev, ref, sizeof(ref), force);
 
-	export_object(old, type, raw, tmpfile);
+	export_object(&old, type, raw, tmpfile);
 	if (launch_editor(tmpfile, NULL, NULL) < 0)
 		die("editing object file failed");
-	import_object(new, type, raw, tmpfile);
+	import_object(&new, type, raw, tmpfile);
 
 	free(tmpfile);
 
-	if (!hashcmp(old, new))
-		return error("new object is the same as the old one: '%s'", sha1_to_hex(old));
+	if (!oidcmp(&old, &new))
+		return error("new object is the same as the old one: '%s'", oid_to_hex(&old));
 
-	return replace_object_sha1(object_ref, old, "replacement", new, force);
+	return replace_object_oid(object_ref, &old, "replacement", &new, force);
 }
 
 static void replace_parents(struct strbuf *buf, int argc, const char **argv)
@@ -312,7 +312,7 @@ static void replace_parents(struct strbuf *buf, int argc, const char **argv)
 
 	/* find existing parents */
 	parent_start = buf->buf;
-	parent_start += 46; /* "tree " + "hex sha1" + "\n" */
+	parent_start += GIT_SHA1_HEXSZ + 6; /* "tree " + "hex sha1" + "\n" */
 	parent_end = parent_start;
 
 	while (starts_with(parent_end, "parent "))
@@ -320,11 +320,11 @@ static void replace_parents(struct strbuf *buf, int argc, const char **argv)
 
 	/* prepare new parents */
 	for (i = 0; i < argc; i++) {
-		unsigned char sha1[20];
-		if (get_sha1(argv[i], sha1) < 0)
+		struct object_id oid;
+		if (get_oid(argv[i], &oid) < 0)
 			die(_("Not a valid object name: '%s'"), argv[i]);
-		lookup_commit_or_die(sha1, argv[i]);
-		strbuf_addf(&new_parents, "parent %s\n", sha1_to_hex(sha1));
+		lookup_commit_or_die(oid.hash, argv[i]);
+		strbuf_addf(&new_parents, "parent %s\n", oid_to_hex(&oid));
 	}
 
 	/* replace existing parents with new ones */
@@ -345,12 +345,12 @@ static void check_one_mergetag(struct commit *commit,
 {
 	struct check_mergetag_data *mergetag_data = (struct check_mergetag_data *)data;
 	const char *ref = mergetag_data->argv[0];
-	unsigned char tag_sha1[20];
+	struct object_id tag_oid;
 	struct tag *tag;
 	int i;
 
-	hash_sha1_file(extra->value, extra->len, typename(OBJ_TAG), tag_sha1);
-	tag = lookup_tag(tag_sha1);
+	hash_sha1_file(extra->value, extra->len, typename(OBJ_TAG), tag_oid.hash);
+	tag = lookup_tag(tag_oid.hash);
 	if (!tag)
 		die(_("bad mergetag in commit '%s'"), ref);
 	if (parse_tag_buffer(tag, extra->value, extra->len))
@@ -366,7 +366,7 @@ static void check_one_mergetag(struct commit *commit,
 	}
 
 	die(_("original commit '%s' contains mergetag '%s' that is discarded; "
-	      "use --edit instead of --graft"), ref, sha1_to_hex(tag_sha1));
+	      "use --edit instead of --graft"), ref, oid_to_hex(&tag_oid));
 }
 
 static void check_mergetags(struct commit *commit, int argc, const char **argv)
@@ -380,16 +380,16 @@ static void check_mergetags(struct commit *commit, int argc, const char **argv)
 
 static int create_graft(int argc, const char **argv, int force)
 {
-	unsigned char old[20], new[20];
+	struct object_id old, new;
 	const char *old_ref = argv[0];
 	struct commit *commit;
 	struct strbuf buf = STRBUF_INIT;
 	const char *buffer;
 	unsigned long size;
 
-	if (get_sha1(old_ref, old) < 0)
+	if (get_oid(old_ref, &old) < 0)
 		die(_("Not a valid object name: '%s'"), old_ref);
-	commit = lookup_commit_or_die(old, old_ref);
+	commit = lookup_commit_or_die(old.hash, old_ref);
 
 	buffer = get_commit_buffer(commit, &size);
 	strbuf_add(&buf, buffer, size);
@@ -404,15 +404,15 @@ static int create_graft(int argc, const char **argv, int force)
 
 	check_mergetags(commit, argc, argv);
 
-	if (write_sha1_file(buf.buf, buf.len, commit_type, new))
+	if (write_sha1_file(buf.buf, buf.len, commit_type, new.hash))
 		die(_("could not write replacement commit for: '%s'"), old_ref);
 
 	strbuf_release(&buf);
 
-	if (!hashcmp(old, new))
-		return error("new commit is the same as the old one: '%s'", sha1_to_hex(old));
+	if (!oidcmp(&old, &new))
+		return error("new commit is the same as the old one: '%s'", oid_to_hex(&old));
 
-	return replace_object_sha1(old_ref, old, "replacement", new, force);
+	return replace_object_oid(old_ref, &old, "replacement", &new, force);
 }
 
 int cmd_replace(int argc, const char **argv, const char *prefix)
-- 
2.11.0


^ permalink raw reply related

* [PATCH v2 10/19] Convert remaining callers of resolve_refdup to object_id
From: brian m. carlson @ 2017-02-14  2:31 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Michael Haggerty
In-Reply-To: <20170214023141.842922-1-sandals@crustytoothpaste.net>

There are a few leaf functions in various files that call
resolve_refdup.  Convert these functions to use struct object_id
internally to prepare for transitioning resolve_refdup itself.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 builtin/notes.c        | 18 +++++++++---------
 builtin/receive-pack.c |  4 ++--
 ref-filter.c           |  4 ++--
 reflog-walk.c          | 12 ++++++------
 transport.c            |  4 ++--
 wt-status.c            |  4 ++--
 6 files changed, 23 insertions(+), 23 deletions(-)

diff --git a/builtin/notes.c b/builtin/notes.c
index 5248a9bad8..8c569a49a0 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -693,7 +693,7 @@ static int merge_abort(struct notes_merge_options *o)
 static int merge_commit(struct notes_merge_options *o)
 {
 	struct strbuf msg = STRBUF_INIT;
-	unsigned char sha1[20], parent_sha1[20];
+	struct object_id oid, parent_oid;
 	struct notes_tree *t;
 	struct commit *partial;
 	struct pretty_print_context pretty_ctx;
@@ -705,27 +705,27 @@ static int merge_commit(struct notes_merge_options *o)
 	 * and target notes ref from .git/NOTES_MERGE_REF.
 	 */
 
-	if (get_sha1("NOTES_MERGE_PARTIAL", sha1))
+	if (get_oid("NOTES_MERGE_PARTIAL", &oid))
 		die(_("failed to read ref NOTES_MERGE_PARTIAL"));
-	else if (!(partial = lookup_commit_reference(sha1)))
+	else if (!(partial = lookup_commit_reference(oid.hash)))
 		die(_("could not find commit from NOTES_MERGE_PARTIAL."));
 	else if (parse_commit(partial))
 		die(_("could not parse commit from NOTES_MERGE_PARTIAL."));
 
 	if (partial->parents)
-		hashcpy(parent_sha1, partial->parents->item->object.oid.hash);
+		oidcpy(&parent_oid, &partial->parents->item->object.oid);
 	else
-		hashclr(parent_sha1);
+		oidclr(&parent_oid);
 
 	t = xcalloc(1, sizeof(struct notes_tree));
 	init_notes(t, "NOTES_MERGE_PARTIAL", combine_notes_overwrite, 0);
 
 	o->local_ref = local_ref_to_free =
-		resolve_refdup("NOTES_MERGE_REF", 0, sha1, NULL);
+		resolve_refdup("NOTES_MERGE_REF", 0, oid.hash, NULL);
 	if (!o->local_ref)
 		die(_("failed to resolve NOTES_MERGE_REF"));
 
-	if (notes_merge_commit(o, t, partial, sha1))
+	if (notes_merge_commit(o, t, partial, oid.hash))
 		die(_("failed to finalize notes merge"));
 
 	/* Reuse existing commit message in reflog message */
@@ -733,8 +733,8 @@ static int merge_commit(struct notes_merge_options *o)
 	format_commit_message(partial, "%s", &msg, &pretty_ctx);
 	strbuf_trim(&msg);
 	strbuf_insert(&msg, 0, "notes: ", 7);
-	update_ref(msg.buf, o->local_ref, sha1,
-		   is_null_sha1(parent_sha1) ? NULL : parent_sha1,
+	update_ref(msg.buf, o->local_ref, oid.hash,
+		   is_null_oid(&parent_oid) ? NULL : parent_oid.hash,
 		   0, UPDATE_REFS_DIE_ON_ERR);
 
 	free_notes(t);
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 1dbb8a0692..7966f4f4df 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -1414,7 +1414,7 @@ static void execute_commands(struct command *commands,
 {
 	struct check_connected_options opt = CHECK_CONNECTED_INIT;
 	struct command *cmd;
-	unsigned char sha1[20];
+	struct object_id oid;
 	struct iterate_data data;
 	struct async muxer;
 	int err_fd = 0;
@@ -1471,7 +1471,7 @@ static void execute_commands(struct command *commands,
 	check_aliased_updates(commands);
 
 	free(head_name_to_free);
-	head_name = head_name_to_free = resolve_refdup("HEAD", 0, sha1, NULL);
+	head_name = head_name_to_free = resolve_refdup("HEAD", 0, oid.hash, NULL);
 
 	if (use_atomic)
 		execute_commands_atomic(commands, si);
diff --git a/ref-filter.c b/ref-filter.c
index 3820b21cc7..f0de30e2ef 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -961,9 +961,9 @@ static void populate_value(struct ref_array_item *ref)
 	ref->value = xcalloc(used_atom_cnt, sizeof(struct atom_value));
 
 	if (need_symref && (ref->flag & REF_ISSYMREF) && !ref->symref) {
-		unsigned char unused1[20];
+		struct object_id unused1;
 		ref->symref = resolve_refdup(ref->refname, RESOLVE_REF_READING,
-					     unused1, NULL);
+					     unused1.hash, NULL);
 		if (!ref->symref)
 			ref->symref = "";
 	}
diff --git a/reflog-walk.c b/reflog-walk.c
index a246af2767..f98748e2ae 100644
--- a/reflog-walk.c
+++ b/reflog-walk.c
@@ -45,11 +45,11 @@ static struct complete_reflogs *read_complete_reflog(const char *ref)
 	reflogs->ref = xstrdup(ref);
 	for_each_reflog_ent(ref, read_one_reflog, reflogs);
 	if (reflogs->nr == 0) {
-		unsigned char sha1[20];
+		struct object_id oid;
 		const char *name;
 		void *name_to_free;
 		name = name_to_free = resolve_refdup(ref, RESOLVE_REF_READING,
-						     sha1, NULL);
+						     oid.hash, NULL);
 		if (name) {
 			for_each_reflog_ent(name, read_one_reflog, reflogs);
 			free(name_to_free);
@@ -172,18 +172,18 @@ int add_reflog_for_walk(struct reflog_walk_info *info,
 		reflogs = item->util;
 	else {
 		if (*branch == '\0') {
-			unsigned char sha1[20];
+			struct object_id oid;
 			free(branch);
-			branch = resolve_refdup("HEAD", 0, sha1, NULL);
+			branch = resolve_refdup("HEAD", 0, oid.hash, NULL);
 			if (!branch)
 				die ("No current branch");
 
 		}
 		reflogs = read_complete_reflog(branch);
 		if (!reflogs || reflogs->nr == 0) {
-			unsigned char sha1[20];
+			struct object_id oid;
 			char *b;
-			if (dwim_log(branch, strlen(branch), sha1, &b) == 1) {
+			if (dwim_log(branch, strlen(branch), oid.hash, &b) == 1) {
 				if (reflogs) {
 					free(reflogs->ref);
 					free(reflogs);
diff --git a/transport.c b/transport.c
index d72e089484..141af31e8e 100644
--- a/transport.c
+++ b/transport.c
@@ -467,11 +467,11 @@ void transport_print_push_status(const char *dest, struct ref *refs,
 {
 	struct ref *ref;
 	int n = 0;
-	unsigned char head_sha1[20];
+	struct object_id head_oid;
 	char *head;
 	int summary_width = transport_summary_width(refs);
 
-	head = resolve_refdup("HEAD", RESOLVE_REF_READING, head_sha1, NULL);
+	head = resolve_refdup("HEAD", RESOLVE_REF_READING, head_oid.hash, NULL);
 
 	if (verbose) {
 		for (ref = refs; ref; ref = ref->next)
diff --git a/wt-status.c b/wt-status.c
index d47012048f..0ec090a338 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -121,7 +121,7 @@ static void status_printf_more(struct wt_status *s, const char *color,
 
 void wt_status_prepare(struct wt_status *s)
 {
-	unsigned char sha1[20];
+	struct object_id oid;
 
 	memset(s, 0, sizeof(*s));
 	memcpy(s->color_palette, default_wt_status_colors,
@@ -129,7 +129,7 @@ void wt_status_prepare(struct wt_status *s)
 	s->show_untracked_files = SHOW_NORMAL_UNTRACKED_FILES;
 	s->use_color = -1;
 	s->relative_paths = 1;
-	s->branch = resolve_refdup("HEAD", 0, sha1, NULL);
+	s->branch = resolve_refdup("HEAD", 0, oid.hash, NULL);
 	s->reference = "HEAD";
 	s->fp = stdout;
 	s->index_file = get_index_file();
-- 
2.11.0


^ permalink raw reply related

* [PATCH v2 09/19] builtin/merge: convert to struct object_id
From: brian m. carlson @ 2017-02-14  2:31 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Michael Haggerty
In-Reply-To: <20170214023141.842922-1-sandals@crustytoothpaste.net>

Additionally convert several uses of the constant 40 into
GIT_SHA1_HEXSZ.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 builtin/merge.c | 134 ++++++++++++++++++++++++++++----------------------------
 1 file changed, 66 insertions(+), 68 deletions(-)

diff --git a/builtin/merge.c b/builtin/merge.c
index a96d4fb501..099cfab447 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -244,7 +244,7 @@ static void drop_save(void)
 	unlink(git_path_merge_mode());
 }
 
-static int save_state(unsigned char *stash)
+static int save_state(struct object_id *stash)
 {
 	int len;
 	struct child_process cp = CHILD_PROCESS_INIT;
@@ -265,7 +265,7 @@ static int save_state(unsigned char *stash)
 	else if (!len)		/* no changes */
 		return -1;
 	strbuf_setlen(&buffer, buffer.len-1);
-	if (get_sha1(buffer.buf, stash))
+	if (get_oid(buffer.buf, stash))
 		die(_("not a valid object: %s"), buffer.buf);
 	return 0;
 }
@@ -305,18 +305,18 @@ static void reset_hard(unsigned const char *sha1, int verbose)
 		die(_("read-tree failed"));
 }
 
-static void restore_state(const unsigned char *head,
-			  const unsigned char *stash)
+static void restore_state(const struct object_id *head,
+			  const struct object_id *stash)
 {
 	struct strbuf sb = STRBUF_INIT;
 	const char *args[] = { "stash", "apply", NULL, NULL };
 
-	if (is_null_sha1(stash))
+	if (is_null_oid(stash))
 		return;
 
-	reset_hard(head, 1);
+	reset_hard(head->hash, 1);
 
-	args[2] = sha1_to_hex(stash);
+	args[2] = oid_to_hex(stash);
 
 	/*
 	 * It is OK to ignore error here, for example when there was
@@ -376,10 +376,10 @@ static void squash_message(struct commit *commit, struct commit_list *remotehead
 
 static void finish(struct commit *head_commit,
 		   struct commit_list *remoteheads,
-		   const unsigned char *new_head, const char *msg)
+		   const struct object_id *new_head, const char *msg)
 {
 	struct strbuf reflog_message = STRBUF_INIT;
-	const unsigned char *head = head_commit->object.oid.hash;
+	const struct object_id *head = &head_commit->object.oid;
 
 	if (!msg)
 		strbuf_addstr(&reflog_message, getenv("GIT_REFLOG_ACTION"));
@@ -397,7 +397,7 @@ static void finish(struct commit *head_commit,
 		else {
 			const char *argv_gc_auto[] = { "gc", "--auto", NULL };
 			update_ref(reflog_message.buf, "HEAD",
-				new_head, head, 0,
+				new_head->hash, head->hash, 0,
 				UPDATE_REFS_DIE_ON_ERR);
 			/*
 			 * We ignore errors in 'gc --auto', since the
@@ -416,7 +416,7 @@ static void finish(struct commit *head_commit,
 			DIFF_FORMAT_SUMMARY | DIFF_FORMAT_DIFFSTAT;
 		opts.detect_rename = DIFF_DETECT_RENAME;
 		diff_setup_done(&opts);
-		diff_tree_sha1(head, new_head, "", &opts);
+		diff_tree_sha1(head->hash, new_head->hash, "", &opts);
 		diffcore_std(&opts);
 		diff_flush(&opts);
 	}
@@ -431,7 +431,7 @@ static void finish(struct commit *head_commit,
 static void merge_name(const char *remote, struct strbuf *msg)
 {
 	struct commit *remote_head;
-	unsigned char branch_head[20];
+	struct object_id branch_head;
 	struct strbuf buf = STRBUF_INIT;
 	struct strbuf bname = STRBUF_INIT;
 	const char *ptr;
@@ -441,25 +441,25 @@ static void merge_name(const char *remote, struct strbuf *msg)
 	strbuf_branchname(&bname, remote);
 	remote = bname.buf;
 
-	memset(branch_head, 0, sizeof(branch_head));
+	oidclr(&branch_head);
 	remote_head = get_merge_parent(remote);
 	if (!remote_head)
 		die(_("'%s' does not point to a commit"), remote);
 
-	if (dwim_ref(remote, strlen(remote), branch_head, &found_ref) > 0) {
+	if (dwim_ref(remote, strlen(remote), branch_head.hash, &found_ref) > 0) {
 		if (starts_with(found_ref, "refs/heads/")) {
 			strbuf_addf(msg, "%s\t\tbranch '%s' of .\n",
-				    sha1_to_hex(branch_head), remote);
+				    oid_to_hex(&branch_head), remote);
 			goto cleanup;
 		}
 		if (starts_with(found_ref, "refs/tags/")) {
 			strbuf_addf(msg, "%s\t\ttag '%s' of .\n",
-				    sha1_to_hex(branch_head), remote);
+				    oid_to_hex(&branch_head), remote);
 			goto cleanup;
 		}
 		if (starts_with(found_ref, "refs/remotes/")) {
 			strbuf_addf(msg, "%s\t\tremote-tracking branch '%s' of .\n",
-				    sha1_to_hex(branch_head), remote);
+				    oid_to_hex(&branch_head), remote);
 			goto cleanup;
 		}
 	}
@@ -590,8 +590,8 @@ static int git_merge_config(const char *k, const char *v, void *cb)
 	return git_diff_ui_config(k, v, cb);
 }
 
-static int read_tree_trivial(unsigned char *common, unsigned char *head,
-			     unsigned char *one)
+static int read_tree_trivial(struct object_id *common, struct object_id *head,
+			     struct object_id *one)
 {
 	int i, nr_trees = 0;
 	struct tree *trees[MAX_UNPACK_TREES];
@@ -606,13 +606,13 @@ static int read_tree_trivial(unsigned char *common, unsigned char *head,
 	opts.verbose_update = 1;
 	opts.trivial_merges_only = 1;
 	opts.merge = 1;
-	trees[nr_trees] = parse_tree_indirect(common);
+	trees[nr_trees] = parse_tree_indirect(common->hash);
 	if (!trees[nr_trees++])
 		return -1;
-	trees[nr_trees] = parse_tree_indirect(head);
+	trees[nr_trees] = parse_tree_indirect(head->hash);
 	if (!trees[nr_trees++])
 		return -1;
-	trees[nr_trees] = parse_tree_indirect(one);
+	trees[nr_trees] = parse_tree_indirect(one->hash);
 	if (!trees[nr_trees++])
 		return -1;
 	opts.fn = threeway_merge;
@@ -626,9 +626,9 @@ static int read_tree_trivial(unsigned char *common, unsigned char *head,
 	return 0;
 }
 
-static void write_tree_trivial(unsigned char *sha1)
+static void write_tree_trivial(struct object_id *oid)
 {
-	if (write_cache_as_tree(sha1, 0, NULL))
+	if (write_cache_as_tree(oid->hash, 0, NULL))
 		die(_("git write-tree failed to write a tree"));
 }
 
@@ -781,7 +781,7 @@ static void prepare_to_commit(struct commit_list *remoteheads)
 
 static int merge_trivial(struct commit *head, struct commit_list *remoteheads)
 {
-	unsigned char result_tree[20], result_commit[20];
+	struct object_id result_tree, result_commit;
 	struct commit_list *parents, **pptr = &parents;
 	static struct lock_file lock;
 
@@ -792,15 +792,15 @@ static int merge_trivial(struct commit *head, struct commit_list *remoteheads)
 		return error(_("Unable to write index."));
 	rollback_lock_file(&lock);
 
-	write_tree_trivial(result_tree);
+	write_tree_trivial(&result_tree);
 	printf(_("Wonderful.\n"));
 	pptr = commit_list_append(head, pptr);
 	pptr = commit_list_append(remoteheads->item, pptr);
 	prepare_to_commit(remoteheads);
-	if (commit_tree(merge_msg.buf, merge_msg.len, result_tree, parents,
-			result_commit, NULL, sign_commit))
+	if (commit_tree(merge_msg.buf, merge_msg.len, result_tree.hash, parents,
+			result_commit.hash, NULL, sign_commit))
 		die(_("failed to write commit object"));
-	finish(head, remoteheads, result_commit, "In-index merge");
+	finish(head, remoteheads, &result_commit, "In-index merge");
 	drop_save();
 	return 0;
 }
@@ -809,12 +809,12 @@ static int finish_automerge(struct commit *head,
 			    int head_subsumed,
 			    struct commit_list *common,
 			    struct commit_list *remoteheads,
-			    unsigned char *result_tree,
+			    struct object_id *result_tree,
 			    const char *wt_strategy)
 {
 	struct commit_list *parents = NULL;
 	struct strbuf buf = STRBUF_INIT;
-	unsigned char result_commit[20];
+	struct object_id result_commit;
 
 	free_commit_list(common);
 	parents = remoteheads;
@@ -822,11 +822,11 @@ static int finish_automerge(struct commit *head,
 		commit_list_insert(head, &parents);
 	strbuf_addch(&merge_msg, '\n');
 	prepare_to_commit(remoteheads);
-	if (commit_tree(merge_msg.buf, merge_msg.len, result_tree, parents,
-			result_commit, NULL, sign_commit))
+	if (commit_tree(merge_msg.buf, merge_msg.len, result_tree->hash, parents,
+			result_commit.hash, NULL, sign_commit))
 		die(_("failed to write commit object"));
 	strbuf_addf(&buf, "Merge made by the '%s' strategy.", wt_strategy);
-	finish(head, remoteheads, result_commit, buf.buf);
+	finish(head, remoteheads, &result_commit, buf.buf);
 	strbuf_release(&buf);
 	drop_save();
 	return 0;
@@ -854,18 +854,18 @@ static int suggest_conflicts(void)
 }
 
 static struct commit *is_old_style_invocation(int argc, const char **argv,
-					      const unsigned char *head)
+					      const struct object_id *head)
 {
 	struct commit *second_token = NULL;
 	if (argc > 2) {
-		unsigned char second_sha1[20];
+		struct object_id second_oid;
 
-		if (get_sha1(argv[1], second_sha1))
+		if (get_oid(argv[1], &second_oid))
 			return NULL;
-		second_token = lookup_commit_reference_gently(second_sha1, 0);
+		second_token = lookup_commit_reference_gently(second_oid.hash, 0);
 		if (!second_token)
 			die(_("'%s' is not a commit"), argv[1]);
-		if (hashcmp(second_token->object.oid.hash, head))
+		if (oidcmp(&second_token->object.oid, head))
 			return NULL;
 	}
 	return second_token;
@@ -1038,7 +1038,7 @@ static void handle_fetch_head(struct commit_list **remotes, struct strbuf *merge
 		die_errno(_("could not close '%s'"), filename);
 
 	for (pos = 0; pos < merge_names->len; pos = npos) {
-		unsigned char sha1[20];
+		struct object_id oid;
 		char *ptr;
 		struct commit *commit;
 
@@ -1048,16 +1048,16 @@ static void handle_fetch_head(struct commit_list **remotes, struct strbuf *merge
 		else
 			npos = merge_names->len;
 
-		if (npos - pos < 40 + 2 ||
-		    get_sha1_hex(merge_names->buf + pos, sha1))
+		if (npos - pos < GIT_SHA1_HEXSZ + 2 ||
+		    get_oid_hex(merge_names->buf + pos, &oid))
 			commit = NULL; /* bad */
-		else if (memcmp(merge_names->buf + pos + 40, "\t\t", 2))
+		else if (memcmp(merge_names->buf + pos + GIT_SHA1_HEXSZ, "\t\t", 2))
 			continue; /* not-for-merge */
 		else {
-			char saved = merge_names->buf[pos + 40];
-			merge_names->buf[pos + 40] = '\0';
+			char saved = merge_names->buf[pos + GIT_SHA1_HEXSZ];
+			merge_names->buf[pos + GIT_SHA1_HEXSZ] = '\0';
 			commit = get_merge_parent(merge_names->buf + pos);
-			merge_names->buf[pos + 40] = saved;
+			merge_names->buf[pos + GIT_SHA1_HEXSZ] = saved;
 		}
 		if (!commit) {
 			if (ptr)
@@ -1117,9 +1117,7 @@ static struct commit_list *collect_parents(struct commit *head_commit,
 
 int cmd_merge(int argc, const char **argv, const char *prefix)
 {
-	unsigned char result_tree[20];
-	unsigned char stash[20];
-	unsigned char head_sha1[20];
+	struct object_id result_tree, stash, head_oid;
 	struct commit *head_commit;
 	struct strbuf buf = STRBUF_INIT;
 	const char *head_arg;
@@ -1138,13 +1136,13 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 	 * Check if we are _not_ on a detached HEAD, i.e. if there is a
 	 * current branch.
 	 */
-	branch = branch_to_free = resolve_refdup("HEAD", 0, head_sha1, NULL);
+	branch = branch_to_free = resolve_refdup("HEAD", 0, head_oid.hash, NULL);
 	if (branch && starts_with(branch, "refs/heads/"))
 		branch += 11;
-	if (!branch || is_null_sha1(head_sha1))
+	if (!branch || is_null_oid(&head_oid))
 		head_commit = NULL;
 	else
-		head_commit = lookup_commit_or_die(head_sha1, "HEAD");
+		head_commit = lookup_commit_or_die(head_oid.hash, "HEAD");
 
 	init_diff_ui_defaults();
 	git_config(git_merge_config, NULL);
@@ -1242,7 +1240,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 		 * to forbid "git merge" into a branch yet to be born.
 		 * We do the same for "git pull".
 		 */
-		unsigned char *remote_head_sha1;
+		struct object_id *remote_head_oid;
 		if (squash)
 			die(_("Squash commit into empty head not supported yet"));
 		if (fast_forward == FF_NO)
@@ -1254,9 +1252,9 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 			die(_("%s - not something we can merge"), argv[0]);
 		if (remoteheads->next)
 			die(_("Can merge only exactly one commit into empty head"));
-		remote_head_sha1 = remoteheads->item->object.oid.hash;
-		read_empty(remote_head_sha1, 0);
-		update_ref("initial pull", "HEAD", remote_head_sha1,
+		remote_head_oid = &remoteheads->item->object.oid;
+		read_empty(remote_head_oid->hash, 0);
+		update_ref("initial pull", "HEAD", remote_head_oid->hash,
 			   NULL, 0, UPDATE_REFS_DIE_ON_ERR);
 		goto done;
 	}
@@ -1270,7 +1268,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 	 * additional safety measure to check for it.
 	 */
 	if (!have_message &&
-	    is_old_style_invocation(argc, argv, head_commit->object.oid.hash)) {
+	    is_old_style_invocation(argc, argv, &head_commit->object.oid)) {
 		warning("old-style 'git merge <msg> HEAD <commit>' is deprecated.");
 		strbuf_addstr(&merge_msg, argv[0]);
 		head_arg = argv[1];
@@ -1422,7 +1420,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 			goto done;
 		}
 
-		finish(head_commit, remoteheads, commit->object.oid.hash, msg.buf);
+		finish(head_commit, remoteheads, &commit->object.oid, msg.buf);
 		drop_save();
 		goto done;
 	} else if (!remoteheads->next && common->next)
@@ -1441,9 +1439,9 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 			/* See if it is really trivial. */
 			git_committer_info(IDENT_STRICT);
 			printf(_("Trying really trivial in-index merge...\n"));
-			if (!read_tree_trivial(common->item->object.oid.hash,
-					       head_commit->object.oid.hash,
-					       remoteheads->item->object.oid.hash)) {
+			if (!read_tree_trivial(&common->item->object.oid,
+					       &head_commit->object.oid,
+					       &remoteheads->item->object.oid)) {
 				ret = merge_trivial(head_commit, remoteheads);
 				goto done;
 			}
@@ -1495,14 +1493,14 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 	    /*
 	     * Stash away the local changes so that we can try more than one.
 	     */
-	    save_state(stash))
-		hashclr(stash);
+	    save_state(&stash))
+		oidclr(&stash);
 
 	for (i = 0; i < use_strategies_nr; i++) {
 		int ret;
 		if (i) {
 			printf(_("Rewinding the tree to pristine...\n"));
-			restore_state(head_commit->object.oid.hash, stash);
+			restore_state(&head_commit->object.oid, &stash);
 		}
 		if (use_strategies_nr != 1)
 			printf(_("Trying merge strategy %s...\n"),
@@ -1547,7 +1545,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 		}
 
 		/* Automerge succeeded. */
-		write_tree_trivial(result_tree);
+		write_tree_trivial(&result_tree);
 		automerge_was_ok = 1;
 		break;
 	}
@@ -1559,7 +1557,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 	if (automerge_was_ok) {
 		ret = finish_automerge(head_commit, head_subsumed,
 				       common, remoteheads,
-				       result_tree, wt_strategy);
+				       &result_tree, wt_strategy);
 		goto done;
 	}
 
@@ -1568,7 +1566,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 	 * it up.
 	 */
 	if (!best_strategy) {
-		restore_state(head_commit->object.oid.hash, stash);
+		restore_state(&head_commit->object.oid, &stash);
 		if (use_strategies_nr > 1)
 			fprintf(stderr,
 				_("No merge strategy handled the merge.\n"));
@@ -1581,7 +1579,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 		; /* We already have its result in the working tree. */
 	else {
 		printf(_("Rewinding the tree to pristine...\n"));
-		restore_state(head_commit->object.oid.hash, stash);
+		restore_state(&head_commit->object.oid, &stash);
 		printf(_("Using the %s to prepare resolving by hand.\n"),
 			best_strategy);
 		try_merge_strategy(best_strategy, common, remoteheads,
-- 
2.11.0


^ permalink raw reply related

* [PATCH v2 08/19] builtin/clone: convert to struct object_id
From: brian m. carlson @ 2017-02-14  2:31 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Michael Haggerty
In-Reply-To: <20170214023141.842922-1-sandals@crustytoothpaste.net>

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 builtin/clone.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/builtin/clone.c b/builtin/clone.c
index 3f63edbbf9..b4c929bb8a 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -681,7 +681,7 @@ static void update_head(const struct ref *our, const struct ref *remote,
 
 static int checkout(int submodule_progress)
 {
-	unsigned char sha1[20];
+	struct object_id oid;
 	char *head;
 	struct lock_file *lock_file;
 	struct unpack_trees_options opts;
@@ -692,7 +692,7 @@ static int checkout(int submodule_progress)
 	if (option_no_checkout)
 		return 0;
 
-	head = resolve_refdup("HEAD", RESOLVE_REF_READING, sha1, NULL);
+	head = resolve_refdup("HEAD", RESOLVE_REF_READING, oid.hash, NULL);
 	if (!head) {
 		warning(_("remote HEAD refers to nonexistent ref, "
 			  "unable to checkout.\n"));
@@ -700,7 +700,7 @@ static int checkout(int submodule_progress)
 	}
 	if (!strcmp(head, "HEAD")) {
 		if (advice_detached_head)
-			detach_advice(sha1_to_hex(sha1));
+			detach_advice(oid_to_hex(&oid));
 	} else {
 		if (!starts_with(head, "refs/heads/"))
 			die(_("HEAD not found below refs/heads!"));
@@ -721,7 +721,7 @@ static int checkout(int submodule_progress)
 	opts.src_index = &the_index;
 	opts.dst_index = &the_index;
 
-	tree = parse_tree_indirect(sha1);
+	tree = parse_tree_indirect(oid.hash);
 	parse_tree(tree);
 	init_tree_desc(&t, tree->buffer, tree->size);
 	if (unpack_trees(1, &t, &opts) < 0)
@@ -731,7 +731,7 @@ static int checkout(int submodule_progress)
 		die(_("unable to write new index file"));
 
 	err |= run_hook_le(NULL, "post-checkout", sha1_to_hex(null_sha1),
-			   sha1_to_hex(sha1), "1", NULL);
+			   oid_to_hex(&oid), "1", NULL);
 
 	if (!err && option_recursive) {
 		struct argv_array args = ARGV_ARRAY_INIT;
-- 
2.11.0


^ permalink raw reply related

* [PATCH v2 13/19] refs: convert each_reflog_ent_fn to struct object_id
From: brian m. carlson @ 2017-02-14  2:31 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Michael Haggerty
In-Reply-To: <20170214023141.842922-1-sandals@crustytoothpaste.net>

Make each_reflog_ent_fn take two struct object_id pointers instead of
two pointers to unsigned char.  Convert the various callbacks to use
struct object_id as well.  Also, rename fsck_handle_reflog_sha1 to
fsck_handle_reflog_oid.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 builtin/fsck.c       | 16 ++++++++--------
 builtin/merge-base.c |  6 +++---
 builtin/reflog.c     |  2 +-
 reflog-walk.c        |  6 +++---
 refs.c               | 24 ++++++++++++------------
 refs.h               |  2 +-
 refs/files-backend.c | 24 ++++++++++++------------
 revision.c           | 12 ++++++------
 sha1_name.c          |  2 +-
 wt-status.c          |  6 +++---
 10 files changed, 50 insertions(+), 50 deletions(-)

diff --git a/builtin/fsck.c b/builtin/fsck.c
index 1a5caccd0f..9b37606858 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -396,13 +396,13 @@ static int fsck_obj_buffer(const unsigned char *sha1, enum object_type type,
 
 static int default_refs;
 
-static void fsck_handle_reflog_sha1(const char *refname, unsigned char *sha1,
+static void fsck_handle_reflog_oid(const char *refname, struct object_id *oid,
 	unsigned long timestamp)
 {
 	struct object *obj;
 
-	if (!is_null_sha1(sha1)) {
-		obj = lookup_object(sha1);
+	if (!is_null_oid(oid)) {
+		obj = lookup_object(oid->hash);
 		if (obj && (obj->flags & HAS_OBJ)) {
 			if (timestamp && name_objects)
 				add_decoration(fsck_walk_options.object_names,
@@ -411,13 +411,13 @@ static void fsck_handle_reflog_sha1(const char *refname, unsigned char *sha1,
 			obj->used = 1;
 			mark_object_reachable(obj);
 		} else {
-			error("%s: invalid reflog entry %s", refname, sha1_to_hex(sha1));
+			error("%s: invalid reflog entry %s", refname, oid_to_hex(oid));
 			errors_found |= ERROR_REACHABLE;
 		}
 	}
 }
 
-static int fsck_handle_reflog_ent(unsigned char *osha1, unsigned char *nsha1,
+static int fsck_handle_reflog_ent(struct object_id *ooid, struct object_id *noid,
 		const char *email, unsigned long timestamp, int tz,
 		const char *message, void *cb_data)
 {
@@ -425,10 +425,10 @@ static int fsck_handle_reflog_ent(unsigned char *osha1, unsigned char *nsha1,
 
 	if (verbose)
 		fprintf(stderr, "Checking reflog %s->%s\n",
-			sha1_to_hex(osha1), sha1_to_hex(nsha1));
+			oid_to_hex(ooid), oid_to_hex(noid));
 
-	fsck_handle_reflog_sha1(refname, osha1, 0);
-	fsck_handle_reflog_sha1(refname, nsha1, timestamp);
+	fsck_handle_reflog_oid(refname, ooid, 0);
+	fsck_handle_reflog_oid(refname, noid, timestamp);
 	return 0;
 }
 
diff --git a/builtin/merge-base.c b/builtin/merge-base.c
index b572a37c26..db95bc29cf 100644
--- a/builtin/merge-base.c
+++ b/builtin/merge-base.c
@@ -131,7 +131,7 @@ static void add_one_commit(unsigned char *sha1, struct rev_collect *revs)
 	commit->object.flags |= TMP_MARK;
 }
 
-static int collect_one_reflog_ent(unsigned char *osha1, unsigned char *nsha1,
+static int collect_one_reflog_ent(struct object_id *ooid, struct object_id *noid,
 				  const char *ident, unsigned long timestamp,
 				  int tz, const char *message, void *cbdata)
 {
@@ -139,9 +139,9 @@ static int collect_one_reflog_ent(unsigned char *osha1, unsigned char *nsha1,
 
 	if (revs->initial) {
 		revs->initial = 0;
-		add_one_commit(osha1, revs);
+		add_one_commit(ooid->hash, revs);
 	}
-	add_one_commit(nsha1, revs);
+	add_one_commit(noid->hash, revs);
 	return 0;
 }
 
diff --git a/builtin/reflog.c b/builtin/reflog.c
index 7a7136e53e..7472775778 100644
--- a/builtin/reflog.c
+++ b/builtin/reflog.c
@@ -615,7 +615,7 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)
 	return status;
 }
 
-static int count_reflog_ent(unsigned char *osha1, unsigned char *nsha1,
+static int count_reflog_ent(struct object_id *ooid, struct object_id *noid,
 		const char *email, unsigned long timestamp, int tz,
 		const char *message, void *cb_data)
 {
diff --git a/reflog-walk.c b/reflog-walk.c
index fe5be41471..99679f5825 100644
--- a/reflog-walk.c
+++ b/reflog-walk.c
@@ -19,7 +19,7 @@ struct complete_reflogs {
 	int nr, alloc;
 };
 
-static int read_one_reflog(unsigned char *osha1, unsigned char *nsha1,
+static int read_one_reflog(struct object_id *ooid, struct object_id *noid,
 		const char *email, unsigned long timestamp, int tz,
 		const char *message, void *cb_data)
 {
@@ -28,8 +28,8 @@ static int read_one_reflog(unsigned char *osha1, unsigned char *nsha1,
 
 	ALLOC_GROW(array->items, array->nr + 1, array->alloc);
 	item = array->items + array->nr;
-	hashcpy(item->ooid.hash, osha1);
-	hashcpy(item->noid.hash, nsha1);
+	oidcpy(&item->ooid, ooid);
+	oidcpy(&item->noid, noid);
 	item->email = xstrdup(email);
 	item->timestamp = timestamp;
 	item->tz = tz;
diff --git a/refs.c b/refs.c
index cd36b64ed9..b900626d3f 100644
--- a/refs.c
+++ b/refs.c
@@ -674,7 +674,7 @@ struct read_ref_at_cb {
 	int *cutoff_cnt;
 };
 
-static int read_ref_at_ent(unsigned char *osha1, unsigned char *nsha1,
+static int read_ref_at_ent(struct object_id *ooid, struct object_id *noid,
 		const char *email, unsigned long timestamp, int tz,
 		const char *message, void *cb_data)
 {
@@ -698,30 +698,30 @@ static int read_ref_at_ent(unsigned char *osha1, unsigned char *nsha1,
 		 * hold the values for the previous record.
 		 */
 		if (!is_null_sha1(cb->osha1)) {
-			hashcpy(cb->sha1, nsha1);
-			if (hashcmp(cb->osha1, nsha1))
+			hashcpy(cb->sha1, noid->hash);
+			if (hashcmp(cb->osha1, noid->hash))
 				warning("Log for ref %s has gap after %s.",
 					cb->refname, show_date(cb->date, cb->tz, DATE_MODE(RFC2822)));
 		}
 		else if (cb->date == cb->at_time)
-			hashcpy(cb->sha1, nsha1);
-		else if (hashcmp(nsha1, cb->sha1))
+			hashcpy(cb->sha1, noid->hash);
+		else if (hashcmp(noid->hash, cb->sha1))
 			warning("Log for ref %s unexpectedly ended on %s.",
 				cb->refname, show_date(cb->date, cb->tz,
 						       DATE_MODE(RFC2822)));
-		hashcpy(cb->osha1, osha1);
-		hashcpy(cb->nsha1, nsha1);
+		hashcpy(cb->osha1, ooid->hash);
+		hashcpy(cb->nsha1, noid->hash);
 		cb->found_it = 1;
 		return 1;
 	}
-	hashcpy(cb->osha1, osha1);
-	hashcpy(cb->nsha1, nsha1);
+	hashcpy(cb->osha1, ooid->hash);
+	hashcpy(cb->nsha1, noid->hash);
 	if (cb->cnt > 0)
 		cb->cnt--;
 	return 0;
 }
 
-static int read_ref_at_ent_oldest(unsigned char *osha1, unsigned char *nsha1,
+static int read_ref_at_ent_oldest(struct object_id *ooid, struct object_id *noid,
 				  const char *email, unsigned long timestamp,
 				  int tz, const char *message, void *cb_data)
 {
@@ -735,9 +735,9 @@ static int read_ref_at_ent_oldest(unsigned char *osha1, unsigned char *nsha1,
 		*cb->cutoff_tz = tz;
 	if (cb->cutoff_cnt)
 		*cb->cutoff_cnt = cb->reccnt;
-	hashcpy(cb->sha1, osha1);
+	hashcpy(cb->sha1, ooid->hash);
 	if (is_null_sha1(cb->sha1))
-		hashcpy(cb->sha1, nsha1);
+		hashcpy(cb->sha1, noid->hash);
 	/* We just want the first entry */
 	return 1;
 }
diff --git a/refs.h b/refs.h
index 9fbff90e79..a075117a2a 100644
--- a/refs.h
+++ b/refs.h
@@ -292,7 +292,7 @@ int delete_reflog(const char *refname);
 
 /* iterate over reflog entries */
 typedef int each_reflog_ent_fn(
-		unsigned char *old_sha1, unsigned char *new_sha1,
+		struct object_id *old_oid, struct object_id *new_oid,
 		const char *committer, unsigned long timestamp,
 		int tz, const char *msg, void *cb_data);
 
diff --git a/refs/files-backend.c b/refs/files-backend.c
index c041d4ba21..d7a5fd2a7c 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -3113,15 +3113,15 @@ static int files_delete_reflog(struct ref_store *ref_store,
 
 static int show_one_reflog_ent(struct strbuf *sb, each_reflog_ent_fn fn, void *cb_data)
 {
-	unsigned char osha1[20], nsha1[20];
+	struct object_id ooid, noid;
 	char *email_end, *message;
 	unsigned long timestamp;
 	int tz;
 
 	/* old SP new SP name <email> SP time TAB msg LF */
 	if (sb->len < 83 || sb->buf[sb->len - 1] != '\n' ||
-	    get_sha1_hex(sb->buf, osha1) || sb->buf[40] != ' ' ||
-	    get_sha1_hex(sb->buf + 41, nsha1) || sb->buf[81] != ' ' ||
+	    get_oid_hex(sb->buf, &ooid) || sb->buf[40] != ' ' ||
+	    get_oid_hex(sb->buf + 41, &noid) || sb->buf[81] != ' ' ||
 	    !(email_end = strchr(sb->buf + 82, '>')) ||
 	    email_end[1] != ' ' ||
 	    !(timestamp = strtoul(email_end + 2, &message, 10)) ||
@@ -3136,7 +3136,7 @@ static int show_one_reflog_ent(struct strbuf *sb, each_reflog_ent_fn fn, void *c
 		message += 6;
 	else
 		message += 7;
-	return fn(osha1, nsha1, sb->buf + 82, timestamp, tz, message, cb_data);
+	return fn(&ooid, &noid, sb->buf + 82, timestamp, tz, message, cb_data);
 }
 
 static char *find_beginning_of_line(char *bob, char *scan)
@@ -3936,10 +3936,10 @@ struct expire_reflog_cb {
 	reflog_expiry_should_prune_fn *should_prune_fn;
 	void *policy_cb;
 	FILE *newlog;
-	unsigned char last_kept_sha1[20];
+	struct object_id last_kept_oid;
 };
 
-static int expire_reflog_ent(unsigned char *osha1, unsigned char *nsha1,
+static int expire_reflog_ent(struct object_id *ooid, struct object_id *noid,
 			     const char *email, unsigned long timestamp, int tz,
 			     const char *message, void *cb_data)
 {
@@ -3947,9 +3947,9 @@ static int expire_reflog_ent(unsigned char *osha1, unsigned char *nsha1,
 	struct expire_reflog_policy_cb *policy_cb = cb->policy_cb;
 
 	if (cb->flags & EXPIRE_REFLOGS_REWRITE)
-		osha1 = cb->last_kept_sha1;
+		ooid = &cb->last_kept_oid;
 
-	if ((*cb->should_prune_fn)(osha1, nsha1, email, timestamp, tz,
+	if ((*cb->should_prune_fn)(ooid->hash, noid->hash, email, timestamp, tz,
 				   message, policy_cb)) {
 		if (!cb->newlog)
 			printf("would prune %s", message);
@@ -3958,9 +3958,9 @@ static int expire_reflog_ent(unsigned char *osha1, unsigned char *nsha1,
 	} else {
 		if (cb->newlog) {
 			fprintf(cb->newlog, "%s %s %s %lu %+05d\t%s",
-				sha1_to_hex(osha1), sha1_to_hex(nsha1),
+				oid_to_hex(ooid), oid_to_hex(noid),
 				email, timestamp, tz, message);
-			hashcpy(cb->last_kept_sha1, nsha1);
+			oidcpy(&cb->last_kept_oid, noid);
 		}
 		if (cb->flags & EXPIRE_REFLOGS_VERBOSE)
 			printf("keep %s", message);
@@ -4047,14 +4047,14 @@ static int files_reflog_expire(struct ref_store *ref_store,
 		 */
 		int update = (flags & EXPIRE_REFLOGS_UPDATE_REF) &&
 			!(type & REF_ISSYMREF) &&
-			!is_null_sha1(cb.last_kept_sha1);
+			!is_null_oid(&cb.last_kept_oid);
 
 		if (close_lock_file(&reflog_lock)) {
 			status |= error("couldn't write %s: %s", log_file,
 					strerror(errno));
 		} else if (update &&
 			   (write_in_full(get_lock_file_fd(lock->lk),
-				sha1_to_hex(cb.last_kept_sha1), 40) != 40 ||
+				oid_to_hex(&cb.last_kept_oid), GIT_SHA1_HEXSZ) != GIT_SHA1_HEXSZ ||
 			    write_str_in_full(get_lock_file_fd(lock->lk), "\n") != 1 ||
 			    close_ref(lock) < 0)) {
 			status |= error("couldn't write %s",
diff --git a/revision.c b/revision.c
index b37dbec378..d9fe73318a 100644
--- a/revision.c
+++ b/revision.c
@@ -1196,11 +1196,11 @@ static void handle_refs(const char *submodule, struct rev_info *revs, unsigned f
 	for_each(submodule, handle_one_ref, &cb);
 }
 
-static void handle_one_reflog_commit(unsigned char *sha1, void *cb_data)
+static void handle_one_reflog_commit(struct object_id *oid, void *cb_data)
 {
 	struct all_refs_cb *cb = cb_data;
-	if (!is_null_sha1(sha1)) {
-		struct object *o = parse_object(sha1);
+	if (!is_null_oid(oid)) {
+		struct object *o = parse_object(oid->hash);
 		if (o) {
 			o->flags |= cb->all_flags;
 			/* ??? CMDLINEFLAGS ??? */
@@ -1214,12 +1214,12 @@ static void handle_one_reflog_commit(unsigned char *sha1, void *cb_data)
 	}
 }
 
-static int handle_one_reflog_ent(unsigned char *osha1, unsigned char *nsha1,
+static int handle_one_reflog_ent(struct object_id *ooid, struct object_id *noid,
 		const char *email, unsigned long timestamp, int tz,
 		const char *message, void *cb_data)
 {
-	handle_one_reflog_commit(osha1, cb_data);
-	handle_one_reflog_commit(nsha1, cb_data);
+	handle_one_reflog_commit(ooid, cb_data);
+	handle_one_reflog_commit(noid, cb_data);
 	return 0;
 }
 
diff --git a/sha1_name.c b/sha1_name.c
index 73a915ff1b..744e9f884a 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -1051,7 +1051,7 @@ struct grab_nth_branch_switch_cbdata {
 	struct strbuf buf;
 };
 
-static int grab_nth_branch_switch(unsigned char *osha1, unsigned char *nsha1,
+static int grab_nth_branch_switch(struct object_id *ooid, struct object_id *noid,
 				  const char *email, unsigned long timestamp, int tz,
 				  const char *message, void *cb_data)
 {
diff --git a/wt-status.c b/wt-status.c
index 0ec090a338..5fac8437b0 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1373,7 +1373,7 @@ struct grab_1st_switch_cbdata {
 	unsigned char nsha1[20];
 };
 
-static int grab_1st_switch(unsigned char *osha1, unsigned char *nsha1,
+static int grab_1st_switch(struct object_id *ooid, struct object_id *noid,
 			   const char *email, unsigned long timestamp, int tz,
 			   const char *message, void *cb_data)
 {
@@ -1387,13 +1387,13 @@ static int grab_1st_switch(unsigned char *osha1, unsigned char *nsha1,
 		return 0;
 	target += strlen(" to ");
 	strbuf_reset(&cb->buf);
-	hashcpy(cb->nsha1, nsha1);
+	hashcpy(cb->nsha1, noid->hash);
 	end = strchrnul(target, '\n');
 	strbuf_add(&cb->buf, target, end - target);
 	if (!strcmp(cb->buf.buf, "HEAD")) {
 		/* HEAD is relative. Resolve it to the right reflog entry. */
 		strbuf_reset(&cb->buf);
-		strbuf_add_unique_abbrev(&cb->buf, nsha1, DEFAULT_ABBREV);
+		strbuf_add_unique_abbrev(&cb->buf, noid->hash, DEFAULT_ABBREV);
 	}
 	return 1;
 }
-- 
2.11.0


^ permalink raw reply related

* [PATCH v2 06/19] builtin/grep: convert to struct object_id
From: brian m. carlson @ 2017-02-14  2:31 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Michael Haggerty
In-Reply-To: <20170214023141.842922-1-sandals@crustytoothpaste.net>

Convert several functions to use struct object_id, and rename them so
that they no longer refer to SHA-1.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 builtin/grep.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/builtin/grep.c b/builtin/grep.c
index 2c727ef499..0393b0fdc4 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -294,17 +294,17 @@ static int grep_cmd_config(const char *var, const char *value, void *cb)
 	return st;
 }
 
-static void *lock_and_read_sha1_file(const unsigned char *sha1, enum object_type *type, unsigned long *size)
+static void *lock_and_read_oid_file(const struct object_id *oid, enum object_type *type, unsigned long *size)
 {
 	void *data;
 
 	grep_read_lock();
-	data = read_sha1_file(sha1, type, size);
+	data = read_sha1_file(oid->hash, type, size);
 	grep_read_unlock();
 	return data;
 }
 
-static int grep_sha1(struct grep_opt *opt, const unsigned char *sha1,
+static int grep_oid(struct grep_opt *opt, const struct object_id *oid,
 		     const char *filename, int tree_name_len,
 		     const char *path)
 {
@@ -323,7 +323,7 @@ static int grep_sha1(struct grep_opt *opt, const unsigned char *sha1,
 
 #ifndef NO_PTHREADS
 	if (num_threads) {
-		add_work(opt, GREP_SOURCE_SHA1, pathbuf.buf, path, sha1);
+		add_work(opt, GREP_SOURCE_SHA1, pathbuf.buf, path, oid);
 		strbuf_release(&pathbuf);
 		return 0;
 	} else
@@ -332,7 +332,7 @@ static int grep_sha1(struct grep_opt *opt, const unsigned char *sha1,
 		struct grep_source gs;
 		int hit;
 
-		grep_source_init(&gs, GREP_SOURCE_SHA1, pathbuf.buf, path, sha1);
+		grep_source_init(&gs, GREP_SOURCE_SHA1, pathbuf.buf, path, oid);
 		strbuf_release(&pathbuf);
 		hit = grep_source(opt, &gs);
 
@@ -690,7 +690,7 @@ static int grep_cache(struct grep_opt *opt, const struct pathspec *pathspec,
 			    ce_skip_worktree(ce)) {
 				if (ce_stage(ce) || ce_intent_to_add(ce))
 					continue;
-				hit |= grep_sha1(opt, ce->oid.hash, ce->name,
+				hit |= grep_oid(opt, &ce->oid, ce->name,
 						 0, ce->name);
 			} else {
 				hit |= grep_file(opt, ce->name);
@@ -750,7 +750,7 @@ static int grep_tree(struct grep_opt *opt, const struct pathspec *pathspec,
 		strbuf_add(base, entry.path, te_len);
 
 		if (S_ISREG(entry.mode)) {
-			hit |= grep_sha1(opt, entry.oid->hash, base->buf, tn_len,
+			hit |= grep_oid(opt, entry.oid, base->buf, tn_len,
 					 check_attr ? base->buf + tn_len : NULL);
 		} else if (S_ISDIR(entry.mode)) {
 			enum object_type type;
@@ -758,7 +758,7 @@ static int grep_tree(struct grep_opt *opt, const struct pathspec *pathspec,
 			void *data;
 			unsigned long size;
 
-			data = lock_and_read_sha1_file(entry.oid->hash, &type, &size);
+			data = lock_and_read_oid_file(entry.oid, &type, &size);
 			if (!data)
 				die(_("unable to read tree (%s)"),
 				    oid_to_hex(entry.oid));
@@ -787,7 +787,7 @@ static int grep_object(struct grep_opt *opt, const struct pathspec *pathspec,
 		       struct object *obj, const char *name, const char *path)
 {
 	if (obj->type == OBJ_BLOB)
-		return grep_sha1(opt, obj->oid.hash, name, 0, path);
+		return grep_oid(opt, &obj->oid, name, 0, path);
 	if (obj->type == OBJ_COMMIT || obj->type == OBJ_TREE) {
 		struct tree_desc tree;
 		void *data;
@@ -1152,11 +1152,11 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 	/* Check revs and then paths */
 	for (i = 0; i < argc; i++) {
 		const char *arg = argv[i];
-		unsigned char sha1[20];
+		struct object_id oid;
 		struct object_context oc;
 		/* Is it a rev? */
-		if (!get_sha1_with_context(arg, 0, sha1, &oc)) {
-			struct object *object = parse_object_or_die(sha1, arg);
+		if (!get_sha1_with_context(arg, 0, oid.hash, &oc)) {
+			struct object *object = parse_object_or_die(oid.hash, arg);
 			if (!seen_dashdash)
 				verify_non_filename(prefix, arg);
 			add_object_array_with_path(object, arg, &list, oc.mode, oc.path);
-- 
2.11.0


^ permalink raw reply related

* [PATCH v2 15/19] refs: simplify parsing of reflog entries
From: brian m. carlson @ 2017-02-14  2:31 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Michael Haggerty
In-Reply-To: <20170214023141.842922-1-sandals@crustytoothpaste.net>

The current code for reflog entries uses a lot of hard-coded constants,
making it hard to read and modify.  Use parse_oid_hex and two temporary
variables to simplify the code and reduce the use of magic constants.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 refs/files-backend.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/refs/files-backend.c b/refs/files-backend.c
index d7a5fd2a7c..09227a3f63 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -3117,12 +3117,14 @@ static int show_one_reflog_ent(struct strbuf *sb, each_reflog_ent_fn fn, void *c
 	char *email_end, *message;
 	unsigned long timestamp;
 	int tz;
+	const char *p = sb->buf;
+	const int minlen = 2 * GIT_SHA1_HEXSZ + 3;
 
 	/* old SP new SP name <email> SP time TAB msg LF */
-	if (sb->len < 83 || sb->buf[sb->len - 1] != '\n' ||
-	    get_oid_hex(sb->buf, &ooid) || sb->buf[40] != ' ' ||
-	    get_oid_hex(sb->buf + 41, &noid) || sb->buf[81] != ' ' ||
-	    !(email_end = strchr(sb->buf + 82, '>')) ||
+	if (sb->len < minlen || sb->buf[sb->len - 1] != '\n' ||
+	    parse_oid_hex(p, &ooid, &p) || *p++ != ' ' ||
+	    parse_oid_hex(p, &noid, &p) || *p++ != ' ' ||
+	    !(email_end = strchr(p, '>')) ||
 	    email_end[1] != ' ' ||
 	    !(timestamp = strtoul(email_end + 2, &message, 10)) ||
 	    !message || message[0] != ' ' ||
@@ -3136,7 +3138,7 @@ static int show_one_reflog_ent(struct strbuf *sb, each_reflog_ent_fn fn, void *c
 		message += 6;
 	else
 		message += 7;
-	return fn(&ooid, &noid, sb->buf + 82, timestamp, tz, message, cb_data);
+	return fn(&ooid, &noid, sb->buf + minlen - 1, timestamp, tz, message, cb_data);
 }
 
 static char *find_beginning_of_line(char *bob, char *scan)
-- 
2.11.0


^ permalink raw reply related

* [PATCH v2 12/19] reflog-walk: convert struct reflog_info to struct object_id
From: brian m. carlson @ 2017-02-14  2:31 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Michael Haggerty
In-Reply-To: <20170214023141.842922-1-sandals@crustytoothpaste.net>

Convert struct reflog_info to use struct object_id by changing the
structure definition and applying the following semantic patch:

@@
struct reflog_info E1;
@@
- E1.osha1
+ E1.ooid.hash

@@
struct reflog_info *E1;
@@
- E1->osha1
+ E1->ooid.hash

@@
struct reflog_info E1;
@@
- E1.nsha1
+ E1.noid.hash

@@
struct reflog_info *E1;
@@
- E1->nsha1
+ E1->noid.hash

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 reflog-walk.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/reflog-walk.c b/reflog-walk.c
index f98748e2ae..fe5be41471 100644
--- a/reflog-walk.c
+++ b/reflog-walk.c
@@ -10,7 +10,7 @@ struct complete_reflogs {
 	char *ref;
 	const char *short_ref;
 	struct reflog_info {
-		unsigned char osha1[20], nsha1[20];
+		struct object_id ooid, noid;
 		char *email;
 		unsigned long timestamp;
 		int tz;
@@ -28,8 +28,8 @@ static int read_one_reflog(unsigned char *osha1, unsigned char *nsha1,
 
 	ALLOC_GROW(array->items, array->nr + 1, array->alloc);
 	item = array->items + array->nr;
-	hashcpy(item->osha1, osha1);
-	hashcpy(item->nsha1, nsha1);
+	hashcpy(item->ooid.hash, osha1);
+	hashcpy(item->noid.hash, nsha1);
 	item->email = xstrdup(email);
 	item->timestamp = timestamp;
 	item->tz = tz;
@@ -238,13 +238,13 @@ void fake_reflog_parent(struct reflog_walk_info *info, struct commit *commit)
 	do {
 		reflog = &commit_reflog->reflogs->items[commit_reflog->recno];
 		commit_reflog->recno--;
-		logobj = parse_object(reflog->osha1);
+		logobj = parse_object(reflog->ooid.hash);
 	} while (commit_reflog->recno && (logobj && logobj->type != OBJ_COMMIT));
 
-	if (!logobj && commit_reflog->recno >= 0 && is_null_sha1(reflog->osha1)) {
+	if (!logobj && commit_reflog->recno >= 0 && is_null_sha1(reflog->ooid.hash)) {
 		/* a root commit, but there are still more entries to show */
 		reflog = &commit_reflog->reflogs->items[commit_reflog->recno];
-		logobj = parse_object(reflog->nsha1);
+		logobj = parse_object(reflog->noid.hash);
 	}
 
 	if (!logobj || logobj->type != OBJ_COMMIT) {
-- 
2.11.0


^ permalink raw reply related

* [PATCH v2 14/19] hex: introduce parse_oid_hex
From: brian m. carlson @ 2017-02-14  2:31 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Michael Haggerty
In-Reply-To: <20170214023141.842922-1-sandals@crustytoothpaste.net>

Introduce a function, parse_oid_hex, which parses a hexadecimal object
ID and if successful, sets a pointer to just beyond the last character.
This allows for simpler, more robust parsing without needing to
hard-code integer values throughout the codebase.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 cache.h | 8 ++++++++
 hex.c   | 8 ++++++++
 2 files changed, 16 insertions(+)

diff --git a/cache.h b/cache.h
index 61fc86e6d7..5dc89a058c 100644
--- a/cache.h
+++ b/cache.h
@@ -1319,6 +1319,14 @@ extern char *oid_to_hex_r(char *out, const struct object_id *oid);
 extern char *sha1_to_hex(const unsigned char *sha1);	/* static buffer result! */
 extern char *oid_to_hex(const struct object_id *oid);	/* same static buffer as sha1_to_hex */
 
+/*
+ * Parse a hexadecimal object ID starting from hex, updating the pointer
+ * specified by p when parsing stops.  The resulting object ID is stored in oid.
+ * Returns 0 on success.  Parsing will stop on the first NUL or other invalid
+ * character.
+ */
+extern int parse_oid_hex(const char *hex, struct object_id *oid, const char **p);
+
 extern int interpret_branch_name(const char *str, int len, struct strbuf *);
 extern int get_oid_mb(const char *str, struct object_id *oid);
 
diff --git a/hex.c b/hex.c
index 845b01a874..54252b1445 100644
--- a/hex.c
+++ b/hex.c
@@ -53,6 +53,14 @@ int get_oid_hex(const char *hex, struct object_id *oid)
 	return get_sha1_hex(hex, oid->hash);
 }
 
+int parse_oid_hex(const char *hex, struct object_id *oid, const char **p)
+{
+	int ret = get_oid_hex(hex, oid);
+	if (!ret)
+		*p = hex + GIT_SHA1_HEXSZ;
+	return ret;
+}
+
 char *sha1_to_hex_r(char *buffer, const unsigned char *sha1)
 {
 	static const char hex[] = "0123456789abcdef";
-- 
2.11.0


^ 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