Re: [RFC/PATCH 0/3] Thinning the git toplevel directory

Jonathan Nieder [off-list ref] writes:

Thanks again for both of your help.  I've put up an updated series at

 git://repo.or.cz/git/jrn.git flatten-source

Huh?  Did you mean deepen or unflatten?

Changes since the series sent to the list:

 - put headers in libgit/ with the source files.  I don't
   know what I was thinking before.

Nicer; I liked Jeff's "lib/" even better, though.

 - renamed nonbuiltin/ to commands/.  Names like
   commands/add--interactive.perl even seem to make a kind of sense.

Yes. "nonbuiltin/" made me stop reading the patches ;-).

 - moved the http support mini-library to http/.

I don't understand the motivation behind this---wouldn't it belong to
"libgit/" and if not why not?

 - renamed git_remote_helpers to python/, though I'm not very happy
   about that.

I am not fond of naming a directory after a language _unless_ the contents
of the directory is _all_ about laying the foundation of something else
that happens to be implemented in that language.  Existing "perl/" is all
about providing the Git.pm module to be used by Perl scripts that live
outside the directory and is a very good example of naming the directory
after the language.  Does the contents of the new "python/" directory
satisfy that criteria?  If so, then it is fine.  If we plan to add more
remote-helpers and if we plan to allow these helpers written in other
languages, you might need to resurrect a separate "remote-helpers/"
directory, move some parts of the files you placed in "python/" that are
not "foundations for talking to git from Python" but are about "being a
remote-helper" to a subdirectory of it, so that new helper implementations
written in other languages can live next to that subdirectory.

For exactly the same reason "scripts/" is a name I am not entirely happy
about in your tree.  "test-programs/" is a very good name, but "scripts/"
only tells us that the contents _happen to be_ done as scripts, and does
not tell us _what they are for_; perhaps "build-helpers/" might be more
understandable.

Thanks.

Junio C Hamano wrote:

Jonathan Nieder [off-list ref] writes:

quoted

 git://repo.or.cz/git/jrn.git flatten-source

Huh?  Did you mean deepen or unflatten?

The branch name was unflatten-source.  Sorry about that.

I've pushed some more simple tweaks.

 - remove the ".sh" suffix from most build helpers
 - rename scripts/ to build-helpers/.  Much nicer.
 - rename libgit/ to lib/.  The shorter pathnames this allows are much
   nicer.
 - move compat/ under the lib/ umbrella
 - merge builtins/ with commands/.  It never was clear to me why
   making a command builtin should require changing its filename.
 - rename gitk-git/ to gitk/
 - rename t/ to tests/, to get a sense for what that feels like

Some remaining details:

quoted

 - moved the http support mini-library to http/.

I don't understand the motivation behind this---wouldn't it belong to
"libgit/" and if not why not?

They are not linked into the main git binary, to avoid a dependency on
libcurl and libexpat.  So it might (or might not) be useful to keep
them in a subdir to avoid tempting people to use those functions when
not appropriate.

lib/http/, with a README explaining their special status?

quoted

 - renamed git_remote_helpers to python/, though I'm not very happy
   about that.

I am not fond of naming a directory after a language _unless_ the contents
of the directory is _all_ about laying the foundation of something else
that happens to be implemented in that language.

"git remote-testgit" uses it for
 - basic utility functions (die, debug, warn)
 - accessing a git repository and listing its branches (GitRepo)
 - running git fast-export (GitExporter) and keeping a marks file
   between runs
 - running git fast-import (GitImporter) and keeping a marks file
   between runs
 - maintaining a mirror of a non-local git repo (NonLocalGit) to
   be able to run fast-export from it

I am guessing a longer term plan is for it to acquire subpackages with
analagous functionality accessing other version control systems.  It
would be tempting to do

 other-vcs/
	bazaar/
	git/
	mercurial/
	subversion/

(intermixing C and Python code) but that doesn't work because it does
not match the structure of the git_remote_helpers package.

Side note: I am not sure I like the git_remote_helpers name.  Wouldn't
a good goal be for these modules to be usable by other VCSes'
import/export scripts, too?

Thanks for the comments.

Heya,

On Sat, Feb 19, 2011 at 12:11, Jonathan Nieder [off-list ref] wrote:

Side note: I am not sure I like the git_remote_helpers name.  Wouldn't
a good goal be for these modules to be usable by other VCSes'
import/export scripts, too?

The reusable code (the fast-import library) is already living in it's
own library on pypi, I don't think there's much other code that is
reusable accross VCSes?

-- 
Cheers,

Sverre Rabbelier

Sverre Rabbelier wrote:

On Sat, Feb 19, 2011 at 12:11, Jonathan Nieder [off-list ref] wrote:

quoted

Side note: I am not sure I like the git_remote_helpers name.  Wouldn't
a good goal be for these modules to be usable by other VCSes'
import/export scripts, too?

The reusable code (the fast-import library) is already living in it's
own library on pypi, I don't think there's much other code that is
reusable accross VCSes?

Sorry for the thinko.  I see your point.

My thinking before was that if we have packages like

 git_remote_helpers.git
 git_remote_helpers.hg

to take care of such things as cloning remote repos in preparation fro
running fast-export from them, then other VCSes (e.g., bazaar) might
want to use these packages in their foreign VCS support, too.  But
that is probably too theoretical and it is always easy to expose what
others actually need when the time comes.

Thanks,
Jonathan

On Sat, Feb 19, 2011 at 05:11:03AM -0600, Jonathan Nieder wrote:

 - merge builtins/ with commands/.  It never was clear to me why
   making a command builtin should require changing its filename.

This may be a bit too radical, but maybe the Makefile should use this
structure to save maintenance effort. In other words, is there any
reason not to just have:

  BUILTIN_SOURCES = $(wildcard builtin/*.c)
  BUILTIN_OBJS = $(patsubst builtin/%.c, builtin/%.o, $(BUILTIN_SOURCES))

and similar for LIB_OBJS and LIB_H?

It is one less thing to need to do when writing new code, and one less
thing to have silly textual conflicts on. It probably doesn't matter
that much, though; we don't actually add new files or commands all that
often.

Speaking of Makefiles, one downside to all of this directory
segmentation is that you can't run "make" from the subdirectories. I
don't know how much we care. Certainly we could put a dummy Makefile
in each subdir that just goes to the toplevel and runs make.

-Peff