git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: shejialuo <shejialuo@gmail.com>
To: Patrick Steinhardt <ps@pks.im>
Cc: git@vger.kernel.org, Sebastian Schuberth <sschuberth@gmail.com>
Subject: Re: [PATCH 0/5] Introduce configs for default repo format
Date: Thu, 15 Aug 2024 23:24:47 +0800	[thread overview]
Message-ID: <Zr4dvybR6j6Ifyya@ArchLinux> (raw)
In-Reply-To: <cover.1723708417.git.ps@pks.im>

On Thu, Aug 15, 2024 at 09:59:54AM +0200, Patrick Steinhardt wrote:
> Hi,
> 
> to set up the default object and ref storage formats, users have to set
> up some environment variables. This is somewhat unwieldy and not really
> in line with how a user typically expects to configure Git, namely by
> using the config system. It makes it harder than necessary to globally
> default to the different formats and requires the user to munge with
> files like `.profile` to persist that setting. Needless to say, this is
> a bit of an awkward user experience.
> 
> This patch series thus introduces two new configs to set the default
> object hash and ref storage format for newly created repositories. Like
> this, folks can simply use the global- or system-level config to adapt
> to their needs. This also has the advantage of giving them the ability
> to adapt the default formats via guarded includes, such that e.g. repos
> in some filesystem hierarchy use format A, whereas others use format B.
> 
> This comes from a discussion with Sebastian (Cc'd) at the Git User Group
> in Berlin yesterday.
> 
> Thanks!
> 
> Patrick
> 

It's a good idea to make the user could set up the default object and
ref storage formats by "git-config(1)". I have read all the patches,
from my perspective, there is no major problems.

But I wanna ask a question here about the following code snippet:

  if (repo_fmt->version >= 0 && hash !=  GIT_HASH_UNKNOWN && hash != repo_fmt->hash_algo)
          die(_("..."));
  else if (hash != GIT_HASH_UNKNOWN)
          repo_fmt->hash_algo = hash;
  else if (env) {
          ...
          repo_fmt->hash_algo = env_algo;
  } else if (cfg.hash != GIT_HASH_UNKNOWN) {
          repo_fmt->hash_algo = cfg.hash;
  }

It's obvious that the precedence of "hash" is the top. We need to make
sure when users execute "git init --object-format=sha256" command, this
explicit info should be considered at the top. However, in the current
design, the precedence of the environment variable is higher than the
"git-config(1)".

If the user uses the following command:

  $ export GIT_DEFAULT_HASH_ENVIRONMENT=sha1
  $ git -c init.defaultObjectFormat=sha256 repo

The repo would be initialized with the sha1 algorithm. I think we should
think carefully which precedence should be higher. I cannot give an
answer here. I am not familiar with the whole database and do not the
concern. But from my own perspective, I think the precedence of the
config should be higher than the environment variable. This is a new
feature, the people who would like to use it, they will never use
environment variable and we should ignore the functionality of the
environment variable. But for people who do not know this feature, they
will continue to use the environment variable and they will never be
influenced by the configs.

Thanks,
Jialuo

  parent reply	other threads:[~2024-08-15 15:24 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-08-15  7:59 [PATCH 0/5] Introduce configs for default repo format Patrick Steinhardt
2024-08-15  7:59 ` [PATCH 1/5] t0001: exercise initialization with ref formats more thoroughly Patrick Steinhardt
2024-08-15 20:35   ` Justin Tobler
2024-08-15  8:00 ` [PATCH 2/5] t0001: delete repositories when object format tests finish Patrick Steinhardt
2024-08-15  8:00 ` [PATCH 3/5] setup: merge configuration of repository formats Patrick Steinhardt
2024-08-15 21:37   ` Justin Tobler
2024-08-16  8:06     ` Patrick Steinhardt
2024-08-15  8:00 ` [PATCH 4/5] setup: make object format configurable via config Patrick Steinhardt
2024-08-15 22:17   ` Justin Tobler
2024-08-15  8:00 ` [PATCH 5/5] setup: make ref storage " Patrick Steinhardt
2024-08-15 22:29   ` Justin Tobler
2024-08-15 15:24 ` shejialuo [this message]
2024-08-15 21:16   ` [PATCH 0/5] Introduce configs for default repo format brian m. carlson
2024-08-15 21:52     ` Junio C Hamano
2024-08-16  8:07       ` Patrick Steinhardt
2024-08-15 21:22 ` brian m. carlson
2024-08-16  8:56 ` [PATCH v2 " Patrick Steinhardt
2024-08-16  8:56   ` [PATCH v2 1/5] t0001: exercise initialization with ref formats more thoroughly Patrick Steinhardt
2024-08-16  8:56   ` [PATCH v2 2/5] t0001: delete repositories when object format tests finish Patrick Steinhardt
2024-08-16  8:56   ` [PATCH v2 3/5] setup: merge configuration of repository formats Patrick Steinhardt
2024-08-16  8:57   ` [PATCH v2 4/5] setup: make object format configurable via config Patrick Steinhardt
2024-08-16 17:13     ` Junio C Hamano
2024-08-16  8:57   ` [PATCH v2 5/5] setup: make ref storage " Patrick Steinhardt
2024-08-16 14:46   ` [PATCH v2 0/5] Introduce configs for default repo format Justin Tobler

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=Zr4dvybR6j6Ifyya@ArchLinux \
    --to=shejialuo@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=ps@pks.im \
    --cc=sschuberth@gmail.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).