Re: [PATCH 0/2] Fix initializing the_hash_algo

[brian m. carlson]

On Fri, Feb 23, 2018 at 04:56:38PM +0700, Nguyễn Thái Ngọc Duy wrote:
> I can certainly try! I start to remember all the hairy details in that
> setup code.
> 
> The first step may be something like this, which identifies all the
> "repo init" entry points. This is basically a revert of e26f7f19b6
> (repository: pre-initialize hash algo pointer - 2018-01-19) and doing
> things the proper way, hopefully.
> 
> This is on 'master', independent from Stefan's series. I have another
> patch on top of that series to remove the use of ignore_env in
> sha1_file.c (and things seem to work). Basically whenever you have to
> initialize the hash algorithm, there's a good chance you need to
> initialize object store as well. But I'll hold that off until
> Stefan's and this one are both merged.

I definitely think this series is an improvement over my previous patch.

My major concern is alarming users (or breaking scripts) with the
warning message.  I wonder if deferring the use of the warning until we
have multiple hash algorithms might be a better idea.  At that point,
the warning would become something people could act upon.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204


signature.asc
Description: PGP signature

[PATCH 0/2] Fix initializing the_hash_algo

[Nguyễn Thái Ngọc Duy]

On Thu, Feb 15, 2018 at 1:08 AM, Brandon Williams  wrote:
> At some point yes we would definitely want the setup code to fully
> initialize a repository object (in this case the_repository).  And I
> would even like to change the function signatures of all the builtin
> commands to take a repository object so that they don't implicitly rely
> on the_repository at all.
>
> When I introduced struct repository I seem to remember there being a
> couple things which were different about setup that made it difficult to
> simply call repo_init() on the_repository.  If you can fix whatever
> those issues with setup were (I can't remember all of them) then that
> would be great :)

I can certainly try! I start to remember all the hairy details in that
setup code.

The first step may be something like this, which identifies all the
"repo init" entry points. This is basically a revert of e26f7f19b6
(repository: pre-initialize hash algo pointer - 2018-01-19) and doing
things the proper way, hopefully.

This is on 'master', independent from Stefan's series. I have another
patch on top of that series to remove the use of ignore_env in
sha1_file.c (and things seem to work). Basically whenever you have to
initialize the hash algorithm, there's a good chance you need to
initialize object store as well. But I'll hold that off until
Stefan's and this one are both merged.

But yeah, it looks like we need some surgery in setup.c if we want
something as pretty as repo_submodule_init() but for the main repo.

Nguyễn Thái Ngọc Duy (2):
  setup.c: initialize the_repository correctly in all cases
  Revert "repository: pre-initialize hash algo pointer"

 builtin/index-pack.c | 5 +
 builtin/init-db.c| 3 ++-
 cache.h  | 3 ++-
 common-main.c| 4 
 diff-no-index.c  | 5 +
 path.c   | 2 +-
 repository.c | 2 +-
 setup.c  | 5 -
 t/helper/test-dump-split-index.c | 2 ++
 9 files changed, 26 insertions(+), 5 deletions(-)

-- 
2.16.1.435.g8f24da2e1a