Re: [PATCH v3 2/4] path: optimize common dir checking

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

 



Michael Haggerty <mhagger@xxxxxxxxxxxx> writes:

> Let's take a step back.
>
> We have always had a ton of code that uses `git_path()` and friends to
> convert abstract things into filesystem paths. Let's take the
> reference-handling code as an example:
> ...
> This seems crazy to me. It is the *reference* code that should know
> whether a particular reference should be stored under `$GIT_DIR` or
> `$GIT_COMMON_DIR`, or indeed whether it should be stored in a database.

It is more like:

 1. The system as a whole should decide if HEAD and refs/heads/
    should be per workspace or shared across a repository (and we say
    the former should be per workspace, the latter should be shared).

 2. The reference code should decide which ref-backend is used to
    store refs.

 3. And any ref-backend should follow the decision made by the
    system as a whole in 1.

I'd imagine that David's ref-backend code inherited from Ronnie
would still accept the string "refs/heads/master" from the rest of
the system (i.e. callers that call into the ref API) to mean "the
ref that represents the 'master' branch", and uses that as the key
to decide "ok, that is shared across workspaces" to honor the
system-wide decision made in 1.  The outside callers wouldn't pass
the result of calling git_path("refs/heads/master") into the ref
API, which may expand to "$somewhere_else/refs/heads/master" when
run in a secondary workspace to point at the common location.

I'd also imagine that the workspace API would give ways for the
implementation of the reference API to ask these questions:

 - which workspace am I operating for?  where is the "common" thing?
   how would I identify this workspace among the ones that share the
   same "common" thing?

 - is this ref (or ref-like thing) supposed to be in common or per
   workspace?

I agree with you that there needs an intermediate level, between
what Duy and David's git_path() does (i.e. which gives the final
result of deciding where in the filesystem the thing should go) and
your two stupid functions would do (i.e. knowing which kind the
thing is, give you the final location in the filesystem).  That is,
to let the caller know if the thing is supposed to be shared or per
workspace in the first place.

With that intermediate level function, a database-based ref-backend
could make ('common', ref/heads/master) as the key for the current
value of the master branch and (workspace-name, HEAD) as the key for
the current value of the HEAD for a given workspace.

> We should have two *stupid* functions, `git_workspace_path()` and
> `git_common_path()`, and have the *callers* decide which one to call.

So I think we should have *three* functions:

 - git_workspace_name(void) returns some name that uniquely
   identifies the current workspace among the workspaces linked to
   the same repository.

 - is_common_thing(const char *) takes a path (that is relative to
   $GIT_DIR where the thing would have lived at in a pre-workspace
   world), and tells if it is a common thing or a per-workspace
   thing.

 - git_path() can stay the external interface and can be thought of:

	git_path(const char *path)
        {
		if (is_common_thing(path))
			return git_common_path(path);
		return git_workspace_path(git_workspace_name(), path);
	}

   if you think in terms of your two helpers.

But I do not think that git_common_path() and git_workspace_path()
need to be called from any other place in the system, and that is
the reason why I did not say we should have four functions (or five,
counting git_path() itself).
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[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]