Re: [PATCH 2/2] Revert "repository: pre-initialize hash algo pointer"

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Feb 23, 2018 at 04:56:40PM +0700, Nguyễn Thái Ngọc Duy wrote:
> diff --git a/builtin/index-pack.c b/builtin/index-pack.c
> index 7e3e1a461c..8ee935504e 100644
> --- a/builtin/index-pack.c
> +++ b/builtin/index-pack.c
> @@ -1673,6 +1673,11 @@ int cmd_index_pack(int argc, const char **argv, const char *prefix)
>  	if (prefix && chdir(prefix))
>  		die(_("Cannot come back to cwd"));
>  
> +	if (!the_hash_algo) {
> +		warning(_("Running without a repository, assuming SHA-1 hash"));
> +		repo_set_hash_algo(the_repository, GIT_HASH_SHA1);
> +	}

Is this warning going to be visible to users in the normal course of
operation?  If so, people are probably going to find this bothersome or
alarming.

>  	for (i = 1; i < argc; i++) {
>  		const char *arg = argv[i];
>  
> diff --git a/common-main.c b/common-main.c
> index 6a689007e7..12aec36794 100644
> --- a/common-main.c
> +++ b/common-main.c
> @@ -1,6 +1,7 @@
>  #include "cache.h"
>  #include "exec_cmd.h"
>  #include "attr.h"
> +#include "repository.h"
>  
>  /*
>   * Many parts of Git have subprograms communicate via pipe, expect the
> @@ -40,5 +41,8 @@ int main(int argc, const char **argv)
>  
>  	restore_sigpipe_to_default();
>  
> +	if (getenv("GIT_HASH_FIXUP"))
> +		repo_set_hash_algo(the_repository, GIT_HASH_SHA1);

I'm lukewarm on adding this environment variable, but considering our
history here, we had probably better.  We can always remove it after a
few releases.

>  	return cmd_main(argc, argv);
>  }
> diff --git a/diff-no-index.c b/diff-no-index.c
> index 0ed5f0f496..f038f665bc 100644
> --- a/diff-no-index.c
> +++ b/diff-no-index.c
> @@ -241,6 +241,11 @@ void diff_no_index(struct rev_info *revs,
>  	struct strbuf replacement = STRBUF_INIT;
>  	const char *prefix = revs->prefix;
>  
> +	if (!the_hash_algo) {
> +		warning(_("Running without a repository, assuming SHA-1 hash"));
> +		repo_set_hash_algo(the_repository, GIT_HASH_SHA1);
> +	}

Again, same concern.  I can imagine scripts that will blow up loudly if
git diff --no-index spews things to standard error.

I'm not opposed to making this more visible, but I wonder if maybe it
should only be visible to developers or such.  The only way I can think
of doing is that is with an advice options, but maybe there's a better
way.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux