[23/24] t5505,t5516: create .git/branches/ when needed

Commit Message

Johannes Schindelin via GitGitGadget July 18, 2019, 1:19 p.m. UTC

From: Johannes Schindelin <[e-mail]>

It is a real old anachronism from the Cogito days to have a
.git/branches/ directory. And to have tests that ensure that Cogito
users can migrate away from using that directory.

But so be it, let's continue testing it.

Let's make sure, however, that git init does not need to create that
directory.

This bug was noticed when testing with templates that had been
pre-committed, skipping the empty branches/ directory of course because
Git does not track empty directories.

Signed-off-by: Johannes Schindelin <[e-mail]>
---
 t/t5505-remote.sh     | 2 ++
 t/t5516-fetch-push.sh | 4 ++++
 2 files changed, 6 insertions(+)

Comments

Junio C Hamano July 18, 2019, 8:34 p.m. UTC | #1

"Johannes Schindelin via GitGitGadget" <[e-mail]>
writes:

> From: Johannes Schindelin <[e-mail]>
>
> It is a real old anachronism from the Cogito days to have a
> .git/branches/ directory. And to have tests that ensure that Cogito
> users can migrate away from using that directory.
>
> But so be it, let's continue testing it.
>
> Let's make sure, however, that git init does not need to create that
> directory.
>
> This bug was noticed when testing with templates that had been
> pre-committed, skipping the empty branches/ directory of course because
> Git does not track empty directories.

Many tests assume that the .git/info/ directory exists, and the only
reason why they can is because we have templates/info--exclude; the
situation around .git/branches/ is exactly the same.

Deprecating .git/branches/ directory and dropping the parts of tests
that wants to make sure the support still works (t5516 is not about
migration but about working with .git/branches configuration) would
be a different matter.  Until that happens, let's not do this patch;
otherwise it would force us to sprinkle "mkdir -p .git/info/" all
over the place for the same rationale.  A directory in .git/ created
by hardcoded mkdir in "git init" (like .git/refs/) and created by
copying templates by "git init" (like .git/info/ and .git/branches)
are both "created by 'git init'".  Special casing the latter is just
silly.

Johannes Schindelin July 19, 2019, 2:52 p.m. UTC | #2

Hi Junio,

On Thu, 18 Jul 2019, Junio C Hamano wrote:

> "Johannes Schindelin via GitGitGadget" <[e-mail]>
> writes:
>
> > From: Johannes Schindelin <[e-mail]>
> >
> > It is a real old anachronism from the Cogito days to have a
> > .git/branches/ directory. And to have tests that ensure that Cogito
> > users can migrate away from using that directory.
> >
> > But so be it, let's continue testing it.
> >
> > Let's make sure, however, that git init does not need to create that
> > directory.
> >
> > This bug was noticed when testing with templates that had been
> > pre-committed, skipping the empty branches/ directory of course because
> > Git does not track empty directories.
>
> Many tests assume that the .git/info/ directory exists, and the only
> reason why they can is because we have templates/info--exclude; the
> situation around .git/branches/ is exactly the same.

No, the situation with `info/` is that it includes a file called
`exclude`, which means that the `templates/` directory can be "tracked".
This is important because we want to commit all those generated files
that cannot be generated inside Visual Studio without Git for Windows'
SDK (which weighs in with several hundred megabytes to download, a
gigabyte on disk).

The `branches/` subdirectory, in contrast, is totally useless for at
least 99.999% of the users, and hence it is understandable that it does
not even contain a single file. Which means that it is *not* "tracked".

So when checking out, e.g. the `vs/master` branch at
https://github.com/git-for-windows/git, which is auto-generated using
this here patch series from Git for Windows' `master`, to allow building
in Visual Studio without having to download the full Git for Windows
SDK, there will be the `info/` directory (by virtue of containing a
trackable file) exists, and the `branches/` subdirectory won't exist.

As a consequence, the test scripts indicated in the commit message,
which _can_ be run in a regular Git for Windows (no Git for Windows SDK
required), will fail without this patch.

> Deprecating .git/branches/ directory and dropping the parts of tests
> that wants to make sure the support still works (t5516 is not about
> migration but about working with .git/branches configuration) would
> be a different matter.

Indeed.

You probably forgot that I already proposed that, long time ago:
https://public-inbox.org/git/[e-mail]/

I haven't forgotten, because you shot it down unceremoniously:

	The last time I thought about trying this several years ago, I found
	that people who need to grab things from many places still do use
	.git/branches/ and their use case is hard to migrate to .git/config,
	primarily because the former is "one per file" and it is easy to
	add/remove/tweak without affecting others.  Ask akpm@ if he still
	prefers to use .git/branches/ for example.

Of course, with such a kind of argument, there is no way how I could
possibly prove that it is no longer advisable to have `.git/branches/`.

"Someone, somewhere _probably_ finds this still useful, so we won't
remove it."

> Until that happens, let's not do this patch;

Well, that's another of these type of arguments.

Without this patch, and without removing support for `.git/branches/`,
you force the Visual Studio build to _not_ pass the test suite. It's as
simple as that.

So basically, by this statement you decided that there will not be a
fully-functional way to build Git in Visual Studio and to pass the test
suite with that.

Which is a shame.

> otherwise it would force us to sprinkle "mkdir -p .git/info/" all
> over the place for the same rationale.

You may have assumed that I did not carefully verify that the test suite
passes with these patches. But I did. And I would not have submitted
this patch series if such a patch was necessary, at least not without
said patch.

> A directory in .git/ created by hardcoded mkdir in "git init" (like
> .git/refs/) and created by copying templates by "git init" (like
> .git/info/ and .git/branches) are both "created by 'git init'".
> Special casing the latter is just silly.

The thing that _really_ is silly is that we have that directory in
Git's templates, still.

Git itself does not populate it. Git does not need it, ever. Git works
Just Fine without it, and it is really by design that it does not
require the presence of that subdirectory.

Only those two test cases insist, for some reason that escapes me, on
the presence of those subdirectories.

As I said, Git does not populate that subdirectory. It even lacks all
facilities to populate it. You have to write the files inside it
yourself, you have to figure out the syntax of the files in that
directory, and the only place where we document this is buried deeply in
`Documentation/gitrepository-layout.txt`.

It is a bit silly, too, that we say there, in that very documentation
for that very feature, that this feature is "slightly deprecated".

Either it is deprecated, or it isn't.

Further, it is a bit silly that this "slight deprecation" has been there
since a1d4aa74241 (Add repository-layout document., 2005-09-01).

In other words, that feature was "slightly deprecated" already a mere
five months after Git was made public. Yet we still shlep it around,
fourteen (!) years later.

In yet other words, the support for `.git/branches` has been "slightly
deprecated" more than 33x longer than it hasn't been.

I just wish that my patches to remove the support for `.git/branches`
would have gone somewhere useful instead of into this ugly impasse.

Ciao,
Dscho

Junio C Hamano July 19, 2019, 3:07 p.m. UTC | #3

Johannes Schindelin <[e-mail]> writes:

> Without this patch, and without removing support for `.git/branches/`,
> you force the Visual Studio build to _not_ pass the test suite. It's as
> simple as that.

I see it as deficiency of the build procedure.  What makes it so
different, compared to "make" we type on our boxes, where we do get
the .git/branches/ directory anyway, even without having any file in
it?  The patch in question is to punt solving that and instead break
the test suite.

Junio C Hamano July 19, 2019, 3:36 p.m. UTC | #4

Johannes Schindelin <[e-mail]> writes:

> You probably forgot that I already proposed that, long time ago:
> https://public-inbox.org/git/[e-mail]/

No, I haven't.  It was actually meant as an invitation to you to
help us come up with a reasonable deprecation path.

I'd be super happy if we did not have to support 3 sources and
instead just 2 sources of the remote information, but as I said
number of times in the previous attempts' discussion, I think it is
trickier than any of our past migration (like "push.default") to
remove support for .git/branches, as I see no reasonably convenient
alternative/workaround for those who do take advantage of the fact
that .git/branches/ is "one liner, single file per remote source",
which would make it convenient when you need to interact with dozens
of sublevel integrators and the population of them changes
regularly.  "Run 'git remote add' with these options to limit its
scope to a single branch" is easy to say but cumbersome to execute.

The best case scenario I came up with is to start giving a message
when we read *and* *use* information from .git/branches/ hierarchy
(i.e. when we know we found a potential user who will get affected
by removal of .git/branches support), asking to tell us not to go
forward with the removal if and only if an alternative we leave for
them gives them unacceptably high cost (i.e. "we cannot afford to
migrate our workflow not to use .git/branches/ because ..."), and
after a few releases we do not hear anything from anybody.  The
second best would be to see responses that can serve as concrete
examples to build our easy-to-use alternative after removing the
support for .git/branches/.  The alternative might end up not
removing the support, but that is OK---we are in far better position
than we currently are either way.  The reason why we still have the
support is mainly because we know there were users who took active
advantage of that facility (again, not Cogito) several years ago,
and we do not know if they moved on to update their workflows to use
the config-based settings.  After the above (or something similar)
happens, we will know that nobody needs it and we can remove it with
confidence.

Junio C Hamano July 19, 2019, 4:30 p.m. UTC | #5

Junio C Hamano <[e-mail]> writes:

> Johannes Schindelin <[e-mail]> writes:
>
>> You probably forgot that I already proposed that, long time ago:
>> https://public-inbox.org/git/[e-mail]/
>
> No, I haven't.  It was actually meant as an invitation to you to
> help us come up with a reasonable deprecation path.

By the way, while the "deprecation plan" still has my attention ;-),
there is another thing I've been wanting to see happen *without*
burning me like the last time it was brought up [*1*], which is also
hard to come up with a reasonable deprecation path.

A newer port like Git for Windows, where I suspect that most of the
users are not even aware of the "git-foo" form, can probably get
away by not shipping the libexec/git/git-foo without getting its
users complaining (and as you repeatedly said, nobody on Windows
write shell scripts, so lack of support for old scripts writtin in
the days back when git-foo was a norm is perfectly fine there).  But
I am not sure about my tree where audiences are beyond Windows, and
I certainly do not want to get burned again myself.  

Somebody else volunteering to take both blame (and flame) and credit
would be most welcomed ;-).


[Reference]

*1* https://public-inbox.org/git/[e-mail]/

Patch

diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh
index 883b32efa0..1132964044 100755
--- a/t/t5505-remote.sh
+++ b/t/t5505-remote.sh
@@ -824,6 +824,7 @@  test_expect_success 'migrate a remote from named file in $GIT_DIR/branches' '
 	(
 		cd six &&
 		git remote rm origin &&
+		mkdir -p .git/branches &&
 		echo "$origin_url" >.git/branches/origin &&
 		git remote rename origin origin &&
 		test_path_is_missing .git/branches/origin &&
@@ -838,6 +839,7 @@  test_expect_success 'migrate a remote from named file in $GIT_DIR/branches (2)'
 	(
 		cd seven &&
 		git remote rm origin &&
+		mkdir -p .git/branches &&
 		echo "quux#foom" > .git/branches/origin &&
 		git remote rename origin origin &&
 		test_path_is_missing .git/branches/origin &&
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index c81ca360ac..47c2959a90 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -866,6 +866,7 @@  test_expect_success 'fetch with branches' '
 	mk_empty testrepo &&
 	git branch second $the_first_commit &&
 	git checkout second &&
+	mkdir -p testrepo/.git/branches &&
 	echo ".." > testrepo/.git/branches/branch1 &&
 	(
 		cd testrepo &&
@@ -879,6 +880,7 @@  test_expect_success 'fetch with branches' '
 
 test_expect_success 'fetch with branches containing #' '
 	mk_empty testrepo &&
+	mkdir -p testrepo/.git/branches &&
 	echo "..#second" > testrepo/.git/branches/branch2 &&
 	(
 		cd testrepo &&
@@ -893,6 +895,7 @@  test_expect_success 'fetch with branches containing #' '
 test_expect_success 'push with branches' '
 	mk_empty testrepo &&
 	git checkout second &&
+	mkdir -p .git/branches &&
 	echo "testrepo" > .git/branches/branch1 &&
 	git push branch1 &&
 	(
@@ -905,6 +908,7 @@  test_expect_success 'push with branches' '
 
 test_expect_success 'push with branches containing #' '
 	mk_empty testrepo &&
+	mkdir -p .git/branches &&
 	echo "testrepo#branch3" > .git/branches/branch2 &&
 	git push branch2 &&
 	(