Re: [PATCH] sha1_name.c: add an option to abort on ambiguous refs
Bringing this up again after I've seen another person accidentally create a refs/heads/origin/some-branch and get really confused because "git push" reports up-to-date but the remote branch is not updated. I don't know how he got into that situation, but I hope we should be able to catch it and suggest a way out. On Wed, Mar 23, 2016 at 2:30 PM, Nguyễn Thái Ngọc Duywrote: > There are cases when a warning on ambiguous refs may go unnoticed > (e.g. git-log filling up the whole screen). There are also cases when > people want to catch ambiguity early (e.g. it happens deep in some > script). In either case, aborting the program would accomplish it. > > Signed-off-by: Nguyễn Thái Ngọc Duy > --- > Documentation/config.txt | 3 ++- > config.c | 5 - > sha1_name.c | 10 -- > 3 files changed, 14 insertions(+), 4 deletions(-) > > diff --git a/Documentation/config.txt b/Documentation/config.txt > index 2cd6bdd..4172f59 100644 > --- a/Documentation/config.txt > +++ b/Documentation/config.txt > @@ -522,7 +522,8 @@ core.sharedRepository:: > > core.warnAmbiguousRefs:: > If true, Git will warn you if the ref name you passed it is ambiguous > - and might match multiple refs in the repository. True by default. > + and might match multiple refs in the repository. If set to "fatal", > + the program will abort on ambiguous refs. True by default. > > core.compression:: > An integer -1..9, indicating a default compression level. > diff --git a/config.c b/config.c > index 9ba40bc..79f1ea5 100644 > --- a/config.c > +++ b/config.c > @@ -738,7 +738,10 @@ static int git_default_core_config(const char *var, > const char *value) > } > > if (!strcmp(var, "core.warnambiguousrefs")) { > - warn_ambiguous_refs = git_config_bool(var, value); > + if (!strcasecmp(value, "fatal")) > + warn_ambiguous_refs = 2; > + else > + warn_ambiguous_refs = git_config_bool(var, value); > return 0; > } > > diff --git a/sha1_name.c b/sha1_name.c > index 3acf221..a0f0ab9 100644 > --- a/sha1_name.c > +++ b/sha1_name.c > @@ -480,6 +480,8 @@ static int get_sha1_basic(const char *str, int len, > unsigned char *sha1, > warning(warn_msg, len, str); > if (advice_object_name_warning) > fprintf(stderr, "%s\n", > _(object_name_msg)); > + if (warn_ambiguous_refs > 1) > + die(_("cannot continue with ambiguous > refs")); > } > free(real_ref); > } > @@ -537,8 +539,12 @@ static int get_sha1_basic(const char *str, int len, > unsigned char *sha1, > > if (warn_ambiguous_refs && !(flags & GET_SHA1_QUIETLY) && > (refs_found > 1 || > -!get_short_sha1(str, len, tmp_sha1, GET_SHA1_QUIETLY))) > - warning(warn_msg, len, str); > +!get_short_sha1(str, len, tmp_sha1, GET_SHA1_QUIETLY))) { > + if (warn_ambiguous_refs > 1) > + die(warn_msg, len, str); > + else > + warning(warn_msg, len, str); > + } > > if (reflog_len) { > int nth, i; > -- > 2.8.0.rc0.210.gd302cd2 > -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] sha1_name.c: add an option to abort on ambiguous refs
On Wed, Mar 23, 2016 at 10:45 PM, Johannes Schindelinwrote: > Hi Duy, > > On Wed, 23 Mar 2016, Nguyễn Thái Ngọc Duy wrote: > >> There are cases when a warning on ambiguous refs may go unnoticed >> (e.g. git-log filling up the whole screen). There are also cases when >> people want to catch ambiguity early (e.g. it happens deep in some >> script). In either case, aborting the program would accomplish it. > > Whenever I see a die() in code outside of builtin/*.c, I cringe. I do that > because it was *exactly* something like that that caused a serious > regression in the builtin am so that we had to resort back to spawning > separate processes *just so* that we could catch error conditions and > run certain code in that case. > > Maybe there would be a way to do what you want to do that does not fly in > the face of libification? Sorry I got nothing better. > Maybe some strbuf with an atexit() that > accumulates fatal errors that might be hidden and that are then written at > the end of the program (colorfully, if isatty(2))? That sounds hacky. If you don't want do die() deep inside then the callers have to handle it, but get_sha1() is spread everywhere. Plus I do not want to program to succeed when there is ambiguation because the ref git chooses may not be what I want. I want it to abort (and almost made a patch that die() unconditionally). -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] sha1_name.c: add an option to abort on ambiguous refs
Hi Duy, On Wed, 23 Mar 2016, Nguyễn Thái Ngọc Duy wrote: > There are cases when a warning on ambiguous refs may go unnoticed > (e.g. git-log filling up the whole screen). There are also cases when > people want to catch ambiguity early (e.g. it happens deep in some > script). In either case, aborting the program would accomplish it. Whenever I see a die() in code outside of builtin/*.c, I cringe. I do that because it was *exactly* something like that that caused a serious regression in the builtin am so that we had to resort back to spawning separate processes *just so* that we could catch error conditions and run certain code in that case. Maybe there would be a way to do what you want to do that does not fly in the face of libification? Maybe some strbuf with an atexit() that accumulates fatal errors that might be hidden and that are then written at the end of the program (colorfully, if isatty(2))? Ciao, Dscho
[PATCH] sha1_name.c: add an option to abort on ambiguous refs
There are cases when a warning on ambiguous refs may go unnoticed (e.g. git-log filling up the whole screen). There are also cases when people want to catch ambiguity early (e.g. it happens deep in some script). In either case, aborting the program would accomplish it. Signed-off-by: Nguyễn Thái Ngọc Duy--- Documentation/config.txt | 3 ++- config.c | 5 - sha1_name.c | 10 -- 3 files changed, 14 insertions(+), 4 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index 2cd6bdd..4172f59 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -522,7 +522,8 @@ core.sharedRepository:: core.warnAmbiguousRefs:: If true, Git will warn you if the ref name you passed it is ambiguous - and might match multiple refs in the repository. True by default. + and might match multiple refs in the repository. If set to "fatal", + the program will abort on ambiguous refs. True by default. core.compression:: An integer -1..9, indicating a default compression level. diff --git a/config.c b/config.c index 9ba40bc..79f1ea5 100644 --- a/config.c +++ b/config.c @@ -738,7 +738,10 @@ static int git_default_core_config(const char *var, const char *value) } if (!strcmp(var, "core.warnambiguousrefs")) { - warn_ambiguous_refs = git_config_bool(var, value); + if (!strcasecmp(value, "fatal")) + warn_ambiguous_refs = 2; + else + warn_ambiguous_refs = git_config_bool(var, value); return 0; } diff --git a/sha1_name.c b/sha1_name.c index 3acf221..a0f0ab9 100644 --- a/sha1_name.c +++ b/sha1_name.c @@ -480,6 +480,8 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1, warning(warn_msg, len, str); if (advice_object_name_warning) fprintf(stderr, "%s\n", _(object_name_msg)); + if (warn_ambiguous_refs > 1) + die(_("cannot continue with ambiguous refs")); } free(real_ref); } @@ -537,8 +539,12 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1, if (warn_ambiguous_refs && !(flags & GET_SHA1_QUIETLY) && (refs_found > 1 || -!get_short_sha1(str, len, tmp_sha1, GET_SHA1_QUIETLY))) - warning(warn_msg, len, str); +!get_short_sha1(str, len, tmp_sha1, GET_SHA1_QUIETLY))) { + if (warn_ambiguous_refs > 1) + die(warn_msg, len, str); + else + warning(warn_msg, len, str); + } if (reflog_len) { int nth, i; -- 2.8.0.rc0.210.gd302cd2 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html