Re: [PATCH v3 2/3] sha1_name: get_sha1_with_context learns to follow symlinks
- From
Junio C Hamano <gitster@pobox.com>
- Date
- May 10, 2015, 17:47 UTC
- Message-ID
- <xmqq8ucwgw8o.fsf@gitster.dls.corp.google.com>
- In-Reply-To
- <1431203769-11855-2-git-send-email-dturner@twopensource.com>
dturner@twopensource.com writes:
Show 15 quoted lines
> -#define GET_SHA1_QUIETLY 01 > -#define GET_SHA1_COMMIT 02 > -#define GET_SHA1_COMMITTISH 04 > -#define GET_SHA1_TREE 010 > -#define GET_SHA1_TREEISH 020 > -#define GET_SHA1_BLOB 040 > -#define GET_SHA1_ONLY_TO_DIE 04000 > +#define GET_SHA1_QUIETLY 01 > +#define GET_SHA1_COMMIT 02 > +#define GET_SHA1_COMMITTISH 04 > +#define GET_SHA1_TREE 010 > +#define GET_SHA1_TREEISH 020 > +#define GET_SHA1_BLOB 040 > +#define GET_SHA1_FOLLOW_SYMLINKS 0100 > +#define GET_SHA1_ONLY_TO_DIE 04000
There is nothing wrong per-se, but this effort for aligning looks amusing ;-) Perhaps the next person who wants to mimick this pattern will order their constants in such a way that gives a shoter code to a symbol with a longer name?
Show 27 quoted lines
> diff --git a/sha1_name.c b/sha1_name.c
> index 6d10f05..325f666 100644
> --- a/sha1_name.c
> +++ b/sha1_name.c
> @@ -1434,15 +1434,22 @@ static int get_sha1_with_context_1(const char *name,
> new_filename = resolve_relative_path(filename);
> if (new_filename)
> filename = new_filename;
> - ret = get_tree_entry(tree_sha1, filename, sha1, &oc->mode);
> - if (ret && only_to_die) {
> - diagnose_invalid_sha1_path(prefix, filename,
> - tree_sha1,
> - name, len);
> + if (flags & GET_SHA1_FOLLOW_SYMLINKS) {
> + ret = get_tree_entry_follow_symlinks(tree_sha1,
> + filename, sha1, &oc->symlink_path,
> + &oc->mode);
> + } else {
> + ret = get_tree_entry(tree_sha1, filename,
> + sha1, &oc->mode);
> + if (ret && only_to_die) {
> + diagnose_invalid_sha1_path(prefix,
> + filename,
> + tree_sha1,
> + name, len);
> + }
> }> - hashcpy(oc->tree, tree_sha1); > strlcpy(oc->path, filename, sizeof(oc->path)); > - > + hashcpy(oc->tree, tree_sha1);
Did the order and additional blank line matter here? I think you'd prefer to keep these two lines as before...
Show 11 quoted lines
> free(new_filename);
> return ret;
> } else {
> @@ -1469,5 +1476,7 @@ void maybe_die_on_misspelt_object_name(const char *name, const char *prefix)
>
> int get_sha1_with_context(const char *str, unsigned flags, unsigned char *sha1, struct object_context *orc)
> {
> + if (flags & GET_SHA1_FOLLOW_SYMLINKS && flags & GET_SHA1_ONLY_TO_DIE)
> + die("BUG: incompatible flags for get_sha1_with_context");
> return get_sha1_with_context_1(str, flags, NULL, sha1, orc);
> }Other than that, the patch looks good.
Thanks.