{"thread":{"id":"51353","subject":"specifying revision - how to enforce matching a tag/branch-name or revision only","startedAt":"2019-06-20T14:32:36Z","lastAt":"2019-07-01T00:31:09Z","messageCount":11,"participants":["Boettger, Heiko","Kyle Meyer","Junio C Hamano","Duy Nguyen","Nguyễn Thái Ngọc Duy","Jakub Narebski"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"377660","messageId":"8C0042D8869AEA4AA334B49AFBBCEF820243C01B6A@TUT-EX01-PV.KSTG.corp","threadId":"51353","inReplyTo":null,"subject":"specifying revision - how to enforce matching a tag/branch-name or revision only","fromName":"Boettger, Heiko","fromEmail":"heiko.boettger@karlstorz.com","sentAt":"2019-06-20T14:32:29Z","receivedAt":"2019-06-20T14:32:36Z","isPatch":false,"sender":{"key":"heiko.boettger@karlstorz.com","avatar":null},"body":"Hi,\n\nI discovered an interesting problem when using `git checkout` to which I couldn't find a good solution. We have an automatic system trying to checkout a branch only when it exists. To do so we check whether `git rev-parse` finds a commit for given branch name:\n\ngit rev-parse \"${BRANCH_NAME}\"  || git rev-parse \"refs/remotes/${UPSTREAM}/${BRANCH_NAME}\"\n\nUnfortunately somebody used the branch name \"add-gcc10\" and `git rev-parse` which didn't exist on one repository. However `git rev-parse`\nalso supports to parse the `git-describe` format which resulted in checkout a commit starting with \"cc10\".\n\nWe saw a similar problem with disambiguation of tag and branch-name. If there any possibility to enforce that `git checkout` and other commands such `git rev-parse` handle a revision as a specific type (branch, tag, commit-id). Is there something like\n`git checkout name@{branch}`  or `git checkout name@{tag}` similar to other rev-parse format such as  `branch@{upstream}`.\n\nBest Regards\n\nHeiko Böttger\n\n"},{"id":"377717","messageId":"87sgs3rhpo.fsf@kyleam.com","threadId":"51353","inReplyTo":"8C0042D8869AEA4AA334B49AFBBCEF820243C01B6A@TUT-EX01-PV.KSTG.corp","subject":"Re: specifying revision - how to enforce matching a tag/branch-name or revision only","fromName":"Kyle Meyer","fromEmail":"kyle@kyleam.com","sentAt":"2019-06-20T22:54:11Z","receivedAt":"2019-06-20T22:56:15Z","isPatch":false,"sender":{"key":"kyle@kyleam.com","avatar":"https://avatars.githubusercontent.com/u/1297788?v=4"},"body":"Hi,\n\n\"Boettger, Heiko\" <Heiko.Boettger@karlstorz.com> writes:\n\n> Hi,\n>\n> I discovered an interesting problem when using `git checkout` to which\n> I couldn't find a good solution. We have an automatic system trying to\n> checkout a branch only when it exists. To do so we check whether `git\n> rev-parse` finds a commit for given branch name:\n>\n> git rev-parse \"${BRANCH_NAME}\"  || git rev-parse \"refs/remotes/${UPSTREAM}/${BRANCH_NAME}\"\n>\n> Unfortunately somebody used the branch name \"add-gcc10\" and `git rev-parse` which didn't exist on one repository. However `git rev-parse`\n> also supports to parse the `git-describe` format which resulted in checkout a commit starting with \"cc10\".\n\nCan't you prepend \"refs/heads/\" to BRANCH_NAME to disambiguate?\n\n-- \nKyle\n"},{"id":"377749","messageId":"xmqq8stvklzd.fsf@gitster-ct.c.googlers.com","threadId":"51353","inReplyTo":"87sgs3rhpo.fsf@kyleam.com","subject":"Re: specifying revision - how to enforce matching a tag/branch-name or revision only","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-06-21T15:16:06Z","receivedAt":"2019-06-21T15:16:11Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Kyle Meyer <kyle@kyleam.com> writes:\n\n>> git rev-parse \"${BRANCH_NAME}\"  || git rev-parse \"refs/remotes/${UPSTREAM}/${BRANCH_NAME}\"\n>>\n>> Unfortunately somebody used the branch name \"add-gcc10\" and `git rev-parse` which didn't exist on one repository. However `git rev-parse`\n>> also supports to parse the `git-describe` format which resulted in checkout a commit starting with \"cc10\".\n>\n> Can't you prepend \"refs/heads/\" to BRANCH_NAME to disambiguate?\n\nYes, that is the kosher way for most commands.  \n\nIt gets a bit tricky for \"checkout <branch-or-committish>\" that\nchanges its behaviour (a local branch is checked out and the next\ncommit extends it, other committishes like tags and remote-tracking\nbranch tips are checked out on a detached HEAD), and has special\nrules for a \"${BRANCH_NAME}\" that is both the name of a local branch\nand something else.\n\nHopefully (Duy Cc'ed) \"git switch\" would improve the situation.\n"},{"id":"377974","messageId":"CACsJy8CBQhF2=nZ4tZj_h6Gr8PjK3amANjT38uhnurS0dehyiQ@mail.gmail.com","threadId":"51353","inReplyTo":"xmqq8stvklzd.fsf@gitster-ct.c.googlers.com","subject":"Re: specifying revision - how to enforce matching a tag/branch-name or revision only","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2019-06-25T11:43:16Z","receivedAt":"2019-06-25T11:43:45Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Fri, Jun 21, 2019 at 10:16 PM Junio C Hamano <gitster@pobox.com> wrote:\n>\n> Kyle Meyer <kyle@kyleam.com> writes:\n>\n> >> git rev-parse \"${BRANCH_NAME}\"  || git rev-parse \"refs/remotes/${UPSTREAM}/${BRANCH_NAME}\"\n> >>\n> >> Unfortunately somebody used the branch name \"add-gcc10\" and `git rev-parse` which didn't exist on one repository. However `git rev-parse`\n> >> also supports to parse the `git-describe` format which resulted in checkout a commit starting with \"cc10\".\n\nI wonder if something like refs/heads/foo-g<hash> could trip the\nparser and mistake it as a `git-describe` output. Staring at\nget_describe_name() alone the answer might be an unfortunate \"yes\".\nBut maybe something will kick in earlier and reject it.\n\n> >\n> > Can't you prepend \"refs/heads/\" to BRANCH_NAME to disambiguate?\n>\n> Yes, that is the kosher way for most commands.\n\nSome commands always prepend refs/heads/ to the <branch> argument you\ngive it if I remember correctly. Or I think I accidentally made\nrefs/heads/refs/heads/something once with some command (then hell\nensued). If true, prepending refs/heads/ is not really foolproof.\n\n> It gets a bit tricky for \"checkout <branch-or-committish>\" that\n> changes its behaviour (a local branch is checked out and the next\n> commit extends it, other committishes like tags and remote-tracking\n> branch tips are checked out on a detached HEAD), and has special\n> rules for a \"${BRANCH_NAME}\" that is both the name of a local branch\n> and something else.\n\nI think \"git checkout --no-guess --no-detach <branch>\" should only\naccept a branch (i.e. ref: refs/heads/<branch>). Dropping --no-detach\nshould allow any ref, which interprets refs/heads/foo as an absolute\nref, not as refs/heads/refs/heads/foo.\n\n> Hopefully (Duy Cc'ed) \"git switch\" would improve the situation.\n\n\"git switch\" has slightly saner defaults, but with --guess being\ndefault to be friendlier to interactive usage, it's still not that\nmuch friendlier to scripts, unfortunately.\n-- \nDuy\n"},{"id":"378001","messageId":"8C0042D8869AEA4AA334B49AFBBCEF82024806510C@TUT-EX01-PV.KSTG.corp","threadId":"51353","inReplyTo":"CACsJy8CBQhF2=nZ4tZj_h6Gr8PjK3amANjT38uhnurS0dehyiQ@mail.gmail.com","subject":"AW: specifying revision - how to enforce matching a tag/branch-name or revision only","fromName":"Boettger, Heiko","fromEmail":"heiko.boettger@karlstorz.com","sentAt":"2019-06-25T14:35:15Z","receivedAt":"2019-06-25T14:35:18Z","isPatch":false,"sender":{"key":"heiko.boettger@karlstorz.com","avatar":null},"body":"First of all many thanks for the responses.\n\nAnd yes refs/heads/foo-g<hash> will be interpreted by \"git checkout\" as \"git-describe\" output, if there is no such branch. My current solution for the moment is to extract the commit-sha1 from the output of \"git for-each-ref\" and fail if there is no exact match otherwise git checkout will anyway do what I want, because it finds the branch.\n\nRegards\nHeiko\n\n________________________________________\nVon: Duy Nguyen [pclouds@gmail.com]\nGesendet: Dienstag, 25. Juni 2019 13:43\nAn: Junio C Hamano\nCc: Kyle Meyer; Boettger, Heiko; git@vger.kernel.org\nBetreff: Re: specifying revision - how to enforce matching a tag/branch-name or revision only\n\nOn Fri, Jun 21, 2019 at 10:16 PM Junio C Hamano <gitster@pobox.com> wrote:\n>\n> Kyle Meyer <kyle@kyleam.com> writes:\n>\n> >> git rev-parse \"${BRANCH_NAME}\"  || git rev-parse \"refs/remotes/${UPSTREAM}/${BRANCH_NAME}\"\n> >>\n> >> Unfortunately somebody used the branch name \"add-gcc10\" and `git rev-parse` which didn't exist on one repository. However `git rev-parse`\n> >> also supports to parse the `git-describe` format which resulted in checkout a commit starting with \"cc10\".\n\nI wonder if something like refs/heads/foo-g<hash> could trip the\nparser and mistake it as a `git-describe` output. Staring at\nget_describe_name() alone the answer might be an unfortunate \"yes\".\nBut maybe something will kick in earlier and reject it.\n\n> >\n> > Can't you prepend \"refs/heads/\" to BRANCH_NAME to disambiguate?\n>\n> Yes, that is the kosher way for most commands.\n\nSome commands always prepend refs/heads/ to the <branch> argument you\ngive it if I remember correctly. Or I think I accidentally made\nrefs/heads/refs/heads/something once with some command (then hell\nensued). If true, prepending refs/heads/ is not really foolproof.\n\n> It gets a bit tricky for \"checkout <branch-or-committish>\" that\n> changes its behaviour (a local branch is checked out and the next\n> commit extends it, other committishes like tags and remote-tracking\n> branch tips are checked out on a detached HEAD), and has special\n> rules for a \"${BRANCH_NAME}\" that is both the name of a local branch\n> and something else.\n\nI think \"git checkout --no-guess --no-detach <branch>\" should only\naccept a branch (i.e. ref: refs/heads/<branch>). Dropping --no-detach\nshould allow any ref, which interprets refs/heads/foo as an absolute\nref, not as refs/heads/refs/heads/foo.\n\n> Hopefully (Duy Cc'ed) \"git switch\" would improve the situation.\n\n\"git switch\" has slightly saner defaults, but with --guess being\ndefault to be friendlier to interactive usage, it's still not that\nmuch friendlier to scripts, unfortunately.\n--\nDuy\n"},{"id":"378056","messageId":"20190626084139.30246-1-pclouds@gmail.com","threadId":"51353","inReplyTo":"8C0042D8869AEA4AA334B49AFBBCEF820243C01B6A@TUT-EX01-PV.KSTG.corp","subject":"[PATCH/RFC] get_oid: new extended SHA-1 syntax to control resolution process","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2019-06-26T08:41:39Z","receivedAt":"2019-06-26T08:41:49Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"[I feel like this is something we should do, but I'm not sure. Hence\nRFC. The patch is mostly to play with if you're curious. The die() thing\nthere is definitely wrong. And yeah all the syntax names are\nplaceholders.]\n\nget_oid() is flexible and accepts multiple SHA-1 sources. Sometimes this\nflexibility is actually unwanted, especially for scripts. Let's talk\nproblems:\n\n- Ambiguity aside, a script may want to check if branch A exists. Of\n  course \"git rev-parse A\" won't cut it. But even \"git rev-parse\n  refs/heads/A\" may fail: if you have refs/heads/refs/heads/A for\n  whatever reason and the real branch \"A\" does not exist, the rev-parse\n  rules allow to expand \"refs/heads/A\" to the long ref.\n\n- And then there's problem with using the wrong rule. 9309ba may look\n  like a short hash. But if such short hash does not match any object,\n  and there is refs/heads/9309ba, you'll have a little surprise.\n\n- The same for blahblah-g9309ba which could either be expanded to\n  refs/heads/blahblah-g9309ba, or interpreted as git-describe output.\n\n- Ambiguation will also cause problems, but I don't think we need to\n  get into that. Ambiguation may be addressed separately actually.\n\nThere could be existing mitigation (e.g. maybe you can resolve\nblahblah-g9309ba and see if it's a ref or not). But it feels like we\nwork around the problem than addressing it.\n\nThe problem is we try every possible way to resolve a rev. Let's have\nsome annotation to express that we only want to resolve a rev in a\ncertain way:\n\n- <hash>@{hash} only accepts a full hash or a short hash. If it's a\n  short hash, it cannot be ambiguous.\n\n- <ref>@{literal} only accepts full ref. No turning \"master\" into\n  \"refs/heads/master\".\n\n- <output>@{describe} interprets <output> as git-describe output\n  only, not an object name or a reference.\n\nThis gives scripts much better control over get_oid(), which\ntranslates to rev-parse and a bunch other commands.\n\nPS. The new syntax can stack with existing ones. E.g. you could write\nrefs/heads/master@{literal}@{yesterday} or <hash>@{hash}^{tree}.\nPerhaps I should allow these tags at the end too, so you can enforce a\nvariable like \"$REV\"@{literal} where $REV could be even HEAD~123\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n Documentation/revisions.txt | 10 ++++++++\n refs.c                      | 41 +++++++++++++++++++++++++++++--\n refs.h                      |  2 ++\n sha1-name.c                 | 48 +++++++++++++++++++++++++++++++++++--\n 4 files changed, 97 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/revisions.txt b/Documentation/revisions.txt\nindex 82c1e5754e..93eb278743 100644\n--- a/Documentation/revisions.txt\n+++ b/Documentation/revisions.txt\n@@ -86,6 +86,16 @@ some output processing may assume ref names in UTF-8.\n   immediately following a ref name and the ref must have an existing\n   log ('$GIT_DIR/logs/<refname>').\n \n+'<refname>@{literal}'::\n+  The ref is not expanded by Git. In other words, if '$GIT_DIR/<refname>'\n+  does not exist, the ref is not valid.\n+\n+'<sha1>@{hash}'::\n+  '<sha1>' must be an unambiguous (short or full) object name.\n+\n+'<describeOutput>@{describe}'::\n+  ...\n+\n '@{<n>}', e.g. '@\\{1\\}'::\n   You can use the '@' construct with an empty ref part to get at a\n   reflog entry of the current branch. For example, if you are on\ndiff --git a/refs.c b/refs.c\nindex b8a8430c96..2ee33257fd 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -634,6 +634,27 @@ int repo_dwim_ref(struct repository *r, const char *str, int len,\n \treturn refs_found;\n }\n \n+int repo_dwim_ref_strict(struct repository *r,\n+\t\t\t const char *str, int len,\n+\t\t\t struct object_id *oid,\n+\t\t\t char **ref)\n+{\n+\tchar *last_branch = substitute_branch_name(r, &str, &len);\n+\tstruct strbuf sb = STRBUF_INIT;\n+\tint flag;\n+\n+\tFREE_AND_NULL(last_branch);\n+\tstrbuf_add(&sb, str, len);\n+\t*ref = xstrdup_or_null(\n+\t\trefs_resolve_ref_unsafe(get_main_ref_store(r),\n+\t\t\t\t\tsb.buf,\n+\t\t\t\t\tRESOLVE_REF_READING,\n+\t\t\t\t\toid,\n+\t\t\t\t\t&flag));\n+\tstrbuf_release(&sb);\n+\treturn *ref != NULL;\n+}\n+\n int dwim_ref(const char *str, int len, struct object_id *oid, char **ref)\n {\n \treturn repo_dwim_ref(the_repository, str, len, oid, ref);\n@@ -673,8 +694,9 @@ int expand_ref(struct repository *repo, const char *str, int len,\n \treturn refs_found;\n }\n \n-int repo_dwim_log(struct repository *r, const char *str, int len,\n-\t\t  struct object_id *oid, char **log)\n+static int do_dwim_log(struct repository *r, const char *str, int len,\n+\t\t       struct object_id *oid, char **log,\n+\t\t       int ignore_rev_parse_rules)\n {\n \tstruct ref_store *refs = get_main_ref_store(r);\n \tchar *last_branch = substitute_branch_name(r, &str, &len);\n@@ -687,6 +709,9 @@ int repo_dwim_log(struct repository *r, const char *str, int len,\n \t\tstruct object_id hash;\n \t\tconst char *ref, *it;\n \n+\t\tif (ignore_rev_parse_rules && p != ref_rev_parse_rules)\n+\t\t\tbreak;\n+\n \t\tstrbuf_reset(&path);\n \t\tstrbuf_addf(&path, *p, len, str);\n \t\tref = refs_resolve_ref_unsafe(refs, path.buf,\n@@ -713,6 +738,18 @@ int repo_dwim_log(struct repository *r, const char *str, int len,\n \treturn logs_found;\n }\n \n+int repo_dwim_log(struct repository *r, const char *str, int len,\n+\t\t  struct object_id *oid, char **log)\n+{\n+\treturn do_dwim_log(r, str, len, oid, log, 0);\n+}\n+\n+int repo_dwim_log_strict(struct repository *r, const char *str, int len,\n+\t\t\t struct object_id *oid, char **log)\n+{\n+\treturn do_dwim_log(r, str, len, oid, log, 1);\n+}\n+\n int dwim_log(const char *str, int len, struct object_id *oid, char **log)\n {\n \treturn repo_dwim_log(the_repository, str, len, oid, log);\ndiff --git a/refs.h b/refs.h\nindex 730d05ad91..9395a2e708 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -151,6 +151,8 @@ void expand_ref_prefix(struct argv_array *prefixes, const char *prefix);\n int expand_ref(struct repository *r, const char *str, int len, struct object_id *oid, char **ref);\n int repo_dwim_ref(struct repository *r, const char *str, int len, struct object_id *oid, char **ref);\n int repo_dwim_log(struct repository *r, const char *str, int len, struct object_id *oid, char **ref);\n+int repo_dwim_ref_strict(struct repository *r, const char *str, int len, struct object_id *oid, char **ref);\n+int repo_dwim_log_strict(struct repository *r, const char *str, int len, struct object_id *oid, char **ref);\n int dwim_ref(const char *str, int len, struct object_id *oid, char **ref);\n int dwim_log(const char *str, int len, struct object_id *oid, char **ref);\n \ndiff --git a/sha1-name.c b/sha1-name.c\nindex 728e6f1f61..2d05414140 100644\n--- a/sha1-name.c\n+++ b/sha1-name.c\n@@ -789,6 +789,25 @@ static inline int push_mark(const char *string, int len)\n \treturn at_mark(string, len, suffix, ARRAY_SIZE(suffix));\n }\n \n+static inline int literal_mark(const char *string, int len)\n+{\n+\tconst char *suffix[] = { \"@{literal}\" };\n+\treturn at_mark(string, len, suffix, ARRAY_SIZE(suffix));\n+}\n+\n+static inline int find_mark(const char *mark, const char *string, int len, int *len_p)\n+{\n+\tint mark_len = strlen(mark);\n+\n+\tif (mark_len <= len &&\n+\t    !strncasecmp(string + len - mark_len, mark, mark_len)) {\n+\t\tif (len_p)\n+\t\t\t*len_p -= mark_len;\n+\t\treturn 1;\n+\t}\n+\treturn 0;\n+}\n+\n static enum get_oid_result get_oid_1(struct repository *r, const char *name, int len, struct object_id *oid, unsigned lookup_flags);\n static int interpret_nth_prior_checkout(struct repository *r, const char *name, int namelen, struct strbuf *buf);\n \n@@ -811,6 +830,18 @@ static int get_oid_basic(struct repository *r, const char *str, int len,\n \tint refs_found = 0;\n \tint at, reflog_len, nth_prior = 0;\n \n+\tif (find_mark(\"@{hash}\", str, len, &len)) {\n+\t\tstruct strbuf sb = STRBUF_INIT;\n+\n+\t\tstrbuf_add(&sb, str, len);\n+\t\tif (!get_oid_hex(sb.buf, oid) ||\n+\t\t    !get_short_oid(r, sb.buf, sb.len, oid, flags)) {\n+\t\t\tstrbuf_release(&sb);\n+\t\t\treturn 0;\n+\t\t}\n+\t\tdie(\"Not a hash. Yes I know this should be fatal\");\n+\t}\n+\n \tif (len == r->hash_algo->hexsz && !get_oid_hex(str, oid)) {\n \t\tif (warn_ambiguous_refs && warn_on_object_refname_ambiguity) {\n \t\t\trefs_found = repo_dwim_ref(r, str, len, &tmp_oid, &real_ref);\n@@ -837,7 +868,8 @@ static int get_oid_basic(struct repository *r, const char *str, int len,\n \t\t\t\t\tcontinue;\n \t\t\t\t}\n \t\t\t\tif (!upstream_mark(str + at, len - at) &&\n-\t\t\t\t    !push_mark(str + at, len - at)) {\n+\t\t\t\t    !push_mark(str + at, len - at) &&\n+\t\t\t\t    !literal_mark(str + at, len - at)) {\n \t\t\t\t\treflog_len = (len-1) - (at+2);\n \t\t\t\t\tlen = at;\n \t\t\t\t}\n@@ -862,7 +894,14 @@ static int get_oid_basic(struct repository *r, const char *str, int len,\n \t\t}\n \t}\n \n-\tif (!len && reflog_len)\n+\tif (find_mark(\"@{literal}\", str, len, &len)) {\n+\t\tif (reflog_len)\n+\t\t\trefs_found = repo_dwim_log_strict(r, str, len, oid, &real_ref);\n+\t\telse\n+\t\t\trefs_found = repo_dwim_ref_strict(r, str, len, oid, &real_ref);\n+\t\tif (!refs_found)\n+\t\t\tdie(\"bad ref\");\n+\t} else if (!len && reflog_len)\n \t\t/* allow \"@{...}\" to mean the current branch reflog */\n \t\trefs_found = repo_dwim_ref(r, \"HEAD\", 4, oid, &real_ref);\n \telse if (reflog_len)\n@@ -1180,6 +1219,11 @@ static enum get_oid_result get_oid_1(struct repository *r,\n \tif (!ret)\n \t\treturn FOUND;\n \n+\tif (find_mark(\"@{describe}\", name, len, &len)) {\n+\t\tret = get_describe_name(r, name, len, oid);\n+\t\treturn !ret ? FOUND : MISSING_OBJECT;\n+\t}\n+\n \tret = get_oid_basic(r, name, len, oid, lookup_flags);\n \tif (!ret)\n \t\treturn FOUND;\n-- \n2.22.0.rc0.322.g2b0371e29a\n\n"},{"id":"378076","messageId":"xmqqk1d8ch9g.fsf@gitster-ct.c.googlers.com","threadId":"51353","inReplyTo":"20190626084139.30246-1-pclouds@gmail.com","subject":"Re: [PATCH/RFC] get_oid: new extended SHA-1 syntax to control resolution process","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-06-26T18:51:07Z","receivedAt":"2019-06-26T18:51:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:\n\n> The problem is we try every possible way to resolve a rev. Let's have\n> some annotation to express that we only want to resolve a rev in a\n> certain way:\n>\n> - <hash>@{hash} only accepts a full hash or a short hash. If it's a\n>   short hash, it cannot be ambiguous.\n\nThis is to defeat something like refs/heads/deadbeef (the branch\nwhose name is \"deadbeef\") interfering an object whose name begins\nwith that hexadecimal substring.  Makes sense.\n\n> - <ref>@{literal} only accepts full ref. No turning \"master\" into\n>   \"refs/heads/master\".\n\nThis is already doable by saying \"refs/heads/master\", isn't it?  Any\nambiguous ones like a branch or a tag whose name is 'refs/heads/master'\n(i.e. refs/heads/refs/heads/master and refs/tags/refs/heads/master\nrespectively) won't interfere once you use the explicit prefix refs/\nlike so.  I'd rather not to see this one added.\n\n> - <output>@{describe} interprets <output> as git-describe output\n>   only, not an object name or a reference.\n\nAgain this is to defeat refs/heads/v2.22.0-221-gdeadbeef branch\ninterferring with the object \"deadbeef\" thusly described.  Makes\nsense.\n\nIn the above, my two \"Makes sense\" only refer to the capability the\nsyntax allows us to have, not the suggested syntax itself (which I\nam not convinced is a good use of @{<token>} yet).\n\nThanks.\n\n"},{"id":"378110","messageId":"CACsJy8AB+YVNi07G-nFkEhexyo2TG=qeNLT=JLksDVnDvs0gxg@mail.gmail.com","threadId":"51353","inReplyTo":"xmqqk1d8ch9g.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH/RFC] get_oid: new extended SHA-1 syntax to control resolution process","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2019-06-27T02:52:45Z","receivedAt":"2019-06-27T02:53:14Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Thu, Jun 27, 2019 at 1:51 AM Junio C Hamano <gitster@pobox.com> wrote:\n> > - <ref>@{literal} only accepts full ref. No turning \"master\" into\n> >   \"refs/heads/master\".\n>\n> This is already doable by saying \"refs/heads/master\", isn't it?  Any\n> ambiguous ones like a branch or a tag whose name is 'refs/heads/master'\n> (i.e. refs/heads/refs/heads/master and refs/tags/refs/heads/master\n> respectively) won't interfere once you use the explicit prefix refs/\n> like so.  I'd rather not to see this one added.\n\nIf refs/heads/master does _not_ exist, then ref/heads/master can\nresolve to refs/heads/refs/heads/master, which I think is unexpected.\n\"ref not found\" error or something should be returned instead.\n-- \nDuy\n"},{"id":"378169","messageId":"xmqqtvcbasb4.fsf@gitster-ct.c.googlers.com","threadId":"51353","inReplyTo":"CACsJy8AB+YVNi07G-nFkEhexyo2TG=qeNLT=JLksDVnDvs0gxg@mail.gmail.com","subject":"Re: [PATCH/RFC] get_oid: new extended SHA-1 syntax to control resolution process","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-06-27T16:47:43Z","receivedAt":"2019-06-27T16:47:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Duy Nguyen <pclouds@gmail.com> writes:\n\n> If refs/heads/master does _not_ exist, then ref/heads/master can\n> resolve to refs/heads/refs/heads/master, which I think is unexpected.\n> \"ref not found\" error or something should be returned instead.\n\nAh, that was what I missed.  Thanks.\n"},{"id":"378341","messageId":"86a7dz4e06.fsf@gmail.com","threadId":"51353","inReplyTo":"20190626084139.30246-1-pclouds@gmail.com","subject":"Re: [PATCH/RFC] get_oid: new extended SHA-1 syntax to control resolution process","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2019-06-30T09:30:01Z","receivedAt":"2019-06-30T09:30:09Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Bikeshed painting ahead.\n\nNguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:\n[...]\n> The problem is we try every possible way to resolve a rev. Let's have\n> some annotation to express that we only want to resolve a rev in a\n> certain way:\n>\n> - <hash>@{hash} only accepts a full hash or a short hash. If it's a\n>   short hash, it cannot be ambiguous.\n>\n> - <ref>@{literal} only accepts full ref. No turning \"master\" into\n>   \"refs/heads/master\".\n>\n> - <output>@{describe} interprets <output> as git-describe output\n>   only, not an object name or a reference.\n>\n> This gives scripts much better control over get_oid(), which\n> translates to rev-parse and a bunch other commands.\n>\n> PS. The new syntax can stack with existing ones. E.g. you could write\n> refs/heads/master@{literal}@{yesterday} or <hash>@{hash}^{tree}.\n> Perhaps I should allow these tags at the end too, so you can enforce a\n> variable like \"$REV\"@{literal} where $REV could be even HEAD~123\n\nI think it would be better to use <hash>^{hash} instead of\n<hash>@{hash}.\n\nThe <ref-ish>@{<something>} is used currently for information that is\noutside the DAG, like @{<date>}, @{<n>}, @{-<n>} uses information from\nreflog, and @{upstream} and @{push} uses information from the config.\n\nOn the other hand ^{<type>}, ^{/<search text>}, and the future\n^{<hashalgo>} all use DAG-only information.\n\nThough one could argue that refs information _is_ outside the DAG...\n\n\nP.S. We have 'git show-ref --verify' that requires exact match (no\nDWIM-mery), which can be used together with '--quiet' in a script.\nBut this doesn't allow for checking if an exact match, or a describe\noutput exists in repository.\n\n\nThanks for taking it up,\n--\nJakub Narębski\n"},{"id":"378354","messageId":"CACsJy8AQ=kiG4jH5WSujefoCQ2yCG4=dV7UUdVh-Gextoa_T7A@mail.gmail.com","threadId":"51353","inReplyTo":"86a7dz4e06.fsf@gmail.com","subject":"Re: [PATCH/RFC] get_oid: new extended SHA-1 syntax to control resolution process","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2019-07-01T00:30:40Z","receivedAt":"2019-07-01T00:31:09Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Sun, Jun 30, 2019 at 4:30 PM Jakub Narebski <jnareb@gmail.com> wrote:\n>\n> Bikeshed painting ahead.\n>\n> Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:\n> [...]\n> > The problem is we try every possible way to resolve a rev. Let's have\n> > some annotation to express that we only want to resolve a rev in a\n> > certain way:\n> >\n> > - <hash>@{hash} only accepts a full hash or a short hash. If it's a\n> >   short hash, it cannot be ambiguous.\n> >\n> > - <ref>@{literal} only accepts full ref. No turning \"master\" into\n> >   \"refs/heads/master\".\n> >\n> > - <output>@{describe} interprets <output> as git-describe output\n> >   only, not an object name or a reference.\n> >\n> > This gives scripts much better control over get_oid(), which\n> > translates to rev-parse and a bunch other commands.\n> >\n> > PS. The new syntax can stack with existing ones. E.g. you could write\n> > refs/heads/master@{literal}@{yesterday} or <hash>@{hash}^{tree}.\n> > Perhaps I should allow these tags at the end too, so you can enforce a\n> > variable like \"$REV\"@{literal} where $REV could be even HEAD~123\n>\n> I think it would be better to use <hash>^{hash} instead of\n> <hash>@{hash}.\n>\n> The <ref-ish>@{<something>} is used currently for information that is\n> outside the DAG, like @{<date>}, @{<n>}, @{-<n>} uses information from\n> reflog, and @{upstream} and @{push} uses information from the config.\n>\n> On the other hand ^{<type>}, ^{/<search text>}, and the future\n> ^{<hashalgo>} all use DAG-only information.\n\nI wasn't aware of ^{<hashalgo>}. hash-function-transition.txt is a bit\nlight on the exact behavior, but maybe we can just define it like\n<hash>@{hash} in this thread (then we don't even need to introduce\n@{hash})?\n\nThat is, if <hash>^{<hashalgo>} fails to find a matching object, the\nget_oid() result is \"not found\", there will be no further attempt to\ninterpret the given revision in a different way (e.g. try to see if\nthe same ref exists...). It also means <hash>^{<hashalgo>} will remain\nsupported even after the sha-512 transition period.\n\n@{describe} might go  the same way as @{hash} (that is, becoming\n^{describe}). I think it's just a slightly different way to write\nhashes.\n\n> Though one could argue that refs information _is_ outside the DAG...\n\nRefs info is outside DAG. But that's mostly for <ref>@{literal} and it\nallows something like <ref>@{literal}@{upstream}. But hash (or object\nname) is already used inside DAG (to link between commits, trees and\nblobs). Though one might still argue that's only true for full hash,\nnot short ones.\n-- \nDuy\n"}]}