git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jonathan Tan <jonathantanmy@google.com>
To: git@vger.kernel.org
Cc: Jonathan Tan <jonathantanmy@google.com>, gitster@pobox.com
Subject: [PATCH v3 0/3] Fix for git checkout @{u} (non-local) then git status
Date: Tue,  1 Sep 2020 15:28:06 -0700	[thread overview]
Message-ID: <cover.1598998706.git.jonathantanmy@google.com> (raw)
In-Reply-To: <20200513004058.34456-1-jonathantanmy@google.com>

Junio writes [1]:

> I even wonder why dwim_ref() is not defined like so in a header:
> 
> 	#define dwim_ref(s, l, o, r) \
> 		repo_dwim_ref(the_repository, (s), (l), (o), (r))
> 
> Which would force a change like the one we are discussing to keep
> them in parallel and keep the promise that only difference between
> the two is that dwim_ref() works with the_repository and the other
> can take an arbitrary repository.  Perhaps that can be a preliminary
> clean-up patch before these two patches ;-)

OK done - that's patch 2 here. (I used "static inline" instead of a
preprocessor #define because I prefer not to use the preprocessor whenever
possible, but switching to #define is fine too.)

> Reducing the size of the diff is a good justification only when the
> end result is the same.  If it burdens future developers more, that
> is "I write less at the expense of forcing others to write more",
> which is not quite the same thing.

OK - that makes sense.

[1] https://lore.kernel.org/git/xmqqzh6ag1ih.fsf@gitster.c.googlers.com/

Jonathan Tan (3):
  sha1-name: replace unsigned int with option struct
  refs: move dwim_ref() to header file
  wt-status: tolerate dangling marks

 archive.c             |  4 ++--
 branch.c              |  2 +-
 builtin/checkout.c    |  4 ++--
 builtin/fast-export.c |  2 +-
 builtin/log.c         |  2 +-
 builtin/merge.c       |  2 +-
 builtin/reset.c       |  2 +-
 builtin/rev-parse.c   |  2 +-
 builtin/show-branch.c |  2 +-
 builtin/stash.c       |  2 +-
 bundle.c              |  2 +-
 cache.h               | 27 ++++++++++++++++++--------
 commit.c              |  2 +-
 refs.c                | 20 +++++++++----------
 refs.h                | 12 ++++++++++--
 remote.c              |  2 +-
 revision.c            |  3 ++-
 sha1-name.c           | 45 ++++++++++++++++++++++++++++---------------
 t/t7508-status.sh     | 12 ++++++++++++
 wt-status.c           |  2 +-
 20 files changed, 98 insertions(+), 53 deletions(-)

Range-diff against v2:
-:  ---------- > 1:  6b3e3077e6 refs: move dwim_ref() to header file
1:  59b91a206d ! 2:  8f489d9633 wt-status: tolerate dangling marks
    @@ Commit message
     
         Therefore, when calculating the status of a worktree, tolerate dangling
         marks. This is done by adding an additional parameter to
    -    repo_dwim_ref().
    +    dwim_ref() and repo_dwim_ref().
     
         Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
     
    + ## archive.c ##
    +@@ archive.c: static void parse_treeish_arg(const char **argv,
    + 		const char *colon = strchrnul(name, ':');
    + 		int refnamelen = colon - name;
    + 
    +-		if (!dwim_ref(name, refnamelen, &oid, &ref))
    ++		if (!dwim_ref(name, refnamelen, &oid, &ref, 0))
    + 			die(_("no such ref: %.*s"), refnamelen, name);
    + 	} else {
    +-		dwim_ref(name, strlen(name), &oid, &ref);
    ++		dwim_ref(name, strlen(name), &oid, &ref, 0);
    + 	}
    + 
    + 	if (get_oid(name, &oid))
    +
    + ## branch.c ##
    +@@ branch.c: void create_branch(struct repository *r,
    + 		die(_("Not a valid object name: '%s'."), start_name);
    + 	}
    + 
    +-	switch (dwim_ref(start_name, strlen(start_name), &oid, &real_ref)) {
    ++	switch (dwim_ref(start_name, strlen(start_name), &oid, &real_ref, 0)) {
    + 	case 0:
    + 		/* Not branching from any existing branch */
    + 		if (explicit_tracking)
    +
    + ## builtin/checkout.c ##
    +@@ builtin/checkout.c: static void setup_branch_path(struct branch_info *branch)
    + 	 * If this is a ref, resolve it; otherwise, look up the OID for our
    + 	 * expression.  Failure here is okay.
    + 	 */
    +-	if (!dwim_ref(branch->name, strlen(branch->name), &branch->oid, &branch->refname))
    ++	if (!dwim_ref(branch->name, strlen(branch->name), &branch->oid, &branch->refname, 0))
    + 		repo_get_oid_committish(the_repository, branch->name, &branch->oid);
    + 
    + 	strbuf_branchname(&buf, branch->name, INTERPRET_BRANCH_LOCAL);
    +@@ builtin/checkout.c: static void die_expecting_a_branch(const struct branch_info *branch_info)
    + 	struct object_id oid;
    + 	char *to_free;
    + 
    +-	if (dwim_ref(branch_info->name, strlen(branch_info->name), &oid, &to_free) == 1) {
    ++	if (dwim_ref(branch_info->name, strlen(branch_info->name), &oid, &to_free, 0) == 1) {
    + 		const char *ref = to_free;
    + 
    + 		if (skip_prefix(ref, "refs/tags/", &ref))
    +
    + ## builtin/fast-export.c ##
    +@@ builtin/fast-export.c: static void get_tags_and_duplicates(struct rev_cmdline_info *info)
    + 		if (e->flags & UNINTERESTING)
    + 			continue;
    + 
    +-		if (dwim_ref(e->name, strlen(e->name), &oid, &full_name) != 1)
    ++		if (dwim_ref(e->name, strlen(e->name), &oid, &full_name, 0) != 1)
    + 			continue;
    + 
    + 		if (refspecs.nr) {
    +
    + ## builtin/log.c ##
    +@@ builtin/log.c: static char *find_branch_name(struct rev_info *rev)
    + 		return NULL;
    + 	ref = rev->cmdline.rev[positive].name;
    + 	tip_oid = &rev->cmdline.rev[positive].item->oid;
    +-	if (dwim_ref(ref, strlen(ref), &branch_oid, &full_ref) &&
    ++	if (dwim_ref(ref, strlen(ref), &branch_oid, &full_ref, 0) &&
    + 	    skip_prefix(full_ref, "refs/heads/", &v) &&
    + 	    oideq(tip_oid, &branch_oid))
    + 		branch = xstrdup(v);
    +
    + ## builtin/merge.c ##
    +@@ builtin/merge.c: static void merge_name(const char *remote, struct strbuf *msg)
    + 	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, &found_ref, 0) > 0) {
    + 		if (starts_with(found_ref, "refs/heads/")) {
    + 			strbuf_addf(msg, "%s\t\tbranch '%s' of .\n",
    + 				    oid_to_hex(&branch_head), remote);
    +
    + ## builtin/reset.c ##
    +@@ builtin/reset.c: int cmd_reset(int argc, const char **argv, const char *prefix)
    + 			char *ref = NULL;
    + 			int err;
    + 
    +-			dwim_ref(rev, strlen(rev), &dummy, &ref);
    ++			dwim_ref(rev, strlen(rev), &dummy, &ref, 0);
    + 			if (ref && !starts_with(ref, "refs/"))
    + 				ref = NULL;
    + 
    +
    + ## builtin/rev-parse.c ##
    +@@ builtin/rev-parse.c: static void show_rev(int type, const struct object_id *oid, const char *name)
    + 			struct object_id discard;
    + 			char *full;
    + 
    +-			switch (dwim_ref(name, strlen(name), &discard, &full)) {
    ++			switch (dwim_ref(name, strlen(name), &discard, &full, 0)) {
    + 			case 0:
    + 				/*
    + 				 * Not found -- not a ref.  We could
    +
    + ## builtin/show-branch.c ##
    +@@ builtin/show-branch.c: int cmd_show_branch(int ac, const char **av, const char *prefix)
    + 			die(Q_("only %d entry can be shown at one time.",
    + 			       "only %d entries can be shown at one time.",
    + 			       MAX_REVS), MAX_REVS);
    +-		if (!dwim_ref(*av, strlen(*av), &oid, &ref))
    ++		if (!dwim_ref(*av, strlen(*av), &oid, &ref, 0))
    + 			die(_("no such ref %s"), *av);
    + 
    + 		/* Has the base been specified? */
    +
    + ## builtin/stash.c ##
    +@@ builtin/stash.c: static int get_stash_info(struct stash_info *info, int argc, const char **argv)
    + 	end_of_rev = strchrnul(revision, '@');
    + 	strbuf_add(&symbolic, revision, end_of_rev - revision);
    + 
    +-	ret = dwim_ref(symbolic.buf, symbolic.len, &dummy, &expanded_ref);
    ++	ret = dwim_ref(symbolic.buf, symbolic.len, &dummy, &expanded_ref, 0);
    + 	strbuf_release(&symbolic);
    + 	switch (ret) {
    + 	case 0: /* Not found, but valid ref */
    +
    + ## bundle.c ##
    +@@ bundle.c: static int write_bundle_refs(int bundle_fd, struct rev_info *revs)
    + 
    + 		if (e->item->flags & UNINTERESTING)
    + 			continue;
    +-		if (dwim_ref(e->name, strlen(e->name), &oid, &ref) != 1)
    ++		if (dwim_ref(e->name, strlen(e->name), &oid, &ref, 0) != 1)
    + 			goto skip_write_ref;
    + 		if (read_ref_full(e->name, RESOLVE_REF_READING, &oid, &flag))
    + 			flag = 0;
    +
      ## cache.h ##
     @@ cache.h: struct interpret_branch_name_options {
      	 * allowed, even ones to refs outside of those namespaces.
    @@ cache.h: struct interpret_branch_name_options {
      int repo_interpret_branch_name(struct repository *r,
      			       const char *str, int len,
     
    + ## commit.c ##
    +@@ commit.c: struct commit *get_fork_point(const char *refname, struct commit *commit)
    + 	struct commit *ret = NULL;
    + 	char *full_refname;
    + 
    +-	switch (dwim_ref(refname, strlen(refname), &oid, &full_refname)) {
    ++	switch (dwim_ref(refname, strlen(refname), &oid, &full_refname, 0)) {
    + 	case 0:
    + 		die("No such ref: '%s'", refname);
    + 	case 1:
    +
      ## refs.c ##
     @@ refs.c: const char *git_default_branch_name(void)
       * to name a branch.
    @@ refs.c: static char *substitute_branch_name(struct repository *r,
      	int   refs_found  = expand_ref(r, str, len, oid, ref);
      	free(last_branch);
      	return refs_found;
    -@@ refs.c: int repo_dwim_ref(struct repository *r, const char *str, int len,
    - 
    - int dwim_ref(const char *str, int len, struct object_id *oid, char **ref)
    - {
    --	return repo_dwim_ref(the_repository, str, len, oid, ref);
    -+	return repo_dwim_ref(the_repository, str, len, oid, ref, 0);
    - }
    - 
    - int expand_ref(struct repository *repo, const char *str, int len,
     @@ refs.c: int repo_dwim_log(struct repository *r, const char *str, int len,
      		  struct object_id *oid, char **log)
      {
    @@ refs.h: struct strvec;
     +int repo_dwim_ref(struct repository *r, const char *str, int len,
     +		  struct object_id *oid, char **ref, int nonfatal_dangling_mark);
      int repo_dwim_log(struct repository *r, const char *str, int len, struct object_id *oid, char **ref);
    - int dwim_ref(const char *str, int len, struct object_id *oid, char **ref);
    + static inline int dwim_ref(const char *str, int len, struct object_id *oid,
    +-			   char **ref)
    ++			   char **ref, int nonfatal_dangling_mark)
    + {
    +-	return repo_dwim_ref(the_repository, str, len, oid, ref);
    ++	return repo_dwim_ref(the_repository, str, len, oid, ref,
    ++			     nonfatal_dangling_mark);
    + }
      int dwim_log(const char *str, int len, struct object_id *oid, char **ref);
    + 
    +
    + ## remote.c ##
    +@@ remote.c: static void set_merge(struct branch *ret)
    + 		    strcmp(ret->remote_name, "."))
    + 			continue;
    + 		if (dwim_ref(ret->merge_name[i], strlen(ret->merge_name[i]),
    +-			     &oid, &ref) == 1)
    ++			     &oid, &ref, 0) == 1)
    + 			ret->merge[i]->dst = ref;
    + 		else
    + 			ret->merge[i]->dst = xstrdup(ret->merge_name[i]);
     
      ## sha1-name.c ##
     @@ sha1-name.c: static int get_oid_basic(struct repository *r, const char *str, int len,
    @@ wt-status.c: static void wt_status_get_detached_from(struct repository *r,
      	}
      
     -	if (dwim_ref(cb.buf.buf, cb.buf.len, &oid, &ref) == 1 &&
    -+	if (repo_dwim_ref(the_repository, cb.buf.buf, cb.buf.len, &oid, &ref, 1) == 1 &&
    ++	if (dwim_ref(cb.buf.buf, cb.buf.len, &oid, &ref, 1) == 1 &&
      	    /* sha1 is a commit? match without further lookup */
      	    (oideq(&cb.noid, &oid) ||
      	     /* perhaps sha1 is a tag, try to dereference to a commit */
-- 
2.28.0.402.g5ffc5be6b7-goog


  parent reply	other threads:[~2020-09-01 22:28 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-13  0:40 [PATCH] wt-status: expand, not dwim, a "detached from" ref Jonathan Tan
2020-05-13  5:33 ` Junio C Hamano
2020-05-13 14:59   ` Junio C Hamano
2020-05-18 22:24     ` Jonathan Tan
2020-08-27  1:47 ` Jonathan Nieder
2020-08-27  2:10   ` Junio C Hamano
2020-08-29  1:02 ` [PATCH v2 0/2] Fix for git checkout @{u} (non-local) then git status Jonathan Tan
2020-08-29  1:02   ` [PATCH v2 1/2] sha1-name: replace unsigned int with option struct Jonathan Tan
2020-08-29 18:44     ` Junio C Hamano
2020-08-29  1:02   ` [PATCH v2 2/2] wt-status: tolerate dangling marks Jonathan Tan
2020-08-29 18:55     ` Junio C Hamano
2020-08-31 17:17       ` Jonathan Tan
2020-08-31 17:37         ` Junio C Hamano
2020-09-01 22:28 ` Jonathan Tan [this message]
2020-09-01 22:28   ` [PATCH v3 1/3] sha1-name: replace unsigned int with option struct Jonathan Tan
2020-09-01 22:28   ` [PATCH v3 2/3] refs: move dwim_ref() to header file Jonathan Tan
2020-09-01 22:28   ` [PATCH v3 3/3] wt-status: tolerate dangling marks Jonathan Tan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=cover.1598998706.git.jonathantanmy@google.com \
    --to=jonathantanmy@google.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).