Add git_repository_init_ext for power initters #844

merged 6 commits into from Aug 23, 2012


None yet

5 participants

libgit2 member

The new git_repository_init_ext adds support for many of the things that you can do with git init and sets up structures that will make it easier to extend further in the future. It is documented in include/git2/repository.h reasonably well, I think.

My motivation for adding this was to make it easier for the submodule code to create a repository where the repo dir was <parent>/.git/modules/<child> and the working directory was <parent>/<child>. Using the new function, you can explicitly set the repo dir and working dir, and init is smart enough to automatically make the gitlink between them.

Additionally, this gives more fine grained control over many of the other things that can be tweaked in git init.

This PR is not ready to be merged. The main issue is that most of the new features don't have tests written for them. Also, there are a couple of features that are implied by the new extended flags that are not implemented yet (such as support for copying template files out of an external directory). I have tested the parts of the new functionality that are used by submodules, but that code is really orthogonal to this (and I haven't included it in this PR).

One thing that surprised me a little while writing this was that the behavior of git_repository_init is to create the full path to the repository if it does not exist. I also implemented two less aggressive directory creation modes (i.e. just create the ".git" directory and just create ".git" and its immediate parent directory). Don't know if the new modes matter - I mostly wrote them because I didn't realize that creating the full path was the existing behavior.


This pull request fails (merged 702419fe into 0aeae70).

libgit2 member

Oh, I see travis is failing because of the setgid bits. I knew that was going to be an issue when I wrote it...

libgit2 member

The SGID implementation doesn't work as is anyhow, because I need to explicitly chmod to get the SGID bit set on directories (can't just pass it in to mkdir). I'll update tomorrow, but feel free to look over the API for other feedback.

libgit2 member

Ballin'. 💜 the header with the template init data.

libgit2 member

@scunz Thanks for noticing! The documentation is incorrect. git_repository_open_ext won't cross file system boundaries unless GIT_REPOSITORY_OPEN_CROSS_FS is set. I'll fix it...

libgit2 member

@scunz Thanks for pointing out the missing symlink check.

Reading over core git, the comment ( says that it checks the work tree (as you point out), but reading the code and running the the debugger, it looks to me like it actually checks the repo directory.

Should libgit2 implement the documented behavior or the actual behavior? I feel like the test ought to be in the working directory, so I guess I'll match that.


I've not yet heard of a user that uses symbolic links on file system level inside the .git-dir - so, i think checking there doesn't make sense.

libgit2 member

@scunz The git-new-workdir script in git/contrib makes symlinks inside the .git directory and we've had several people mention it in issues. Of course, that script is independent of core.symlinks behavior, so I agree with you that checking there doesn't make sense.

libgit2 member

By the way, when defining the option flags for git_repository_init_ext(), I intentionally tried to avoid flags whose sole purpose is to set a config value. Initially I had some like that, but then it seemed like a slippery slope to having dozens of flags and instead I think the library user should just call the git_config APIs to adjust the configuration if they need to.

libgit2 member

This looks gorgeous. Any updates on the sgid issues?

libgit2 member

Yep, I've got it mostly worked out. I've added support for copying repo templates (if, for example, core git has already installed them), and I'm writing tests right now.

I've "solved" the sgid stuff by saying that the sgid bits will be defined to zero on platforms that don't support set-gid directories, and then in the code there are just one or two places where I back out of chmod'ing a directory if S_ISGID is zero.

Trying to update the PR in the next hour or so...


This pull request fails (merged 3bcebd13 into 0aeae70).

libgit2 member

I'm not surprised to see travis fail here. I wanted to push up these changes because most of the functionality is working, but I didn't do any cross platform testing yet and I'm not surprised that the set-GID stuff (and probably the symlink stuff, too) is broken. I'll dig into that next...

libgit2 member

Well, that should fix at least some of the builds - I needed to call umask in the clar tests to test some of the new directory chmod'ing stuff, and I reverted the value in a clar cleanup callback referencing a stack-allocated copy of the old value. Of course, the stack had gone away by the time the cleanup function ran, so I was basically setting the umask to a totally fucked up value and then destroying the rest of the tests.


This pull request passes (merged 39ca1685 into 0aeae70).

@arrbee arrbee referenced this pull request Aug 2, 2012

Submodule extensions #852


This pull request passes (merged f7e9317a into d96c386).

arrbee added some commits Jul 26, 2012
@arrbee arrbee Add git_repository_init_ext for power initters
The extended version of repository init adds support for many
of the things that you can do with `git init` and sets up
structures that will make it easier to extend further in the
@arrbee arrbee Add template dir and set gid to repo init
This extends git_repository_init_ext further with support for
initializing the repository from an external template directory
and with support for the "create shared" type flags that make a
set GID repository directory.

This also adds tests for much of the new functionality to the
existing `repo/init.c` test suite.

Also, this adds a bunch of new utility functions including a
very general purpose `git_futils_mkdir` (with the ability to
make paths and to chmod the paths post-creation) and a file
tree copying function `git_futils_cp_r`.  Also, this includes
some new path functions that were useful to keep the code
@arrbee arrbee fix missing validation and type cast warning 0e26202
@arrbee arrbee Don't reference stack vars in cleanup callback
If you use the clar cleanup callback function, you can't pass a
reference pointer to a stack allocated variable because when the
cleanup function runs, the stack won't exist anymore.
@arrbee arrbee Some cleanup suggested during review
This cleans up a number of items suggested during code review
with @vmg, including:

* renaming "outside repo" config API to `git_config_open_default`
* killing the `git_config_open_global` API
* removing the `git_` prefix from the static functions in fileops
* removing some unnecessary functionality from the "cp" command
@arrbee arrbee Fix warnings and merge issues on Win64 e9ca852

This pull request passes (merged e9ca852 into 5fdc41e).

libgit2 member

Hey @vmg! I've incorporated the changes we discussed, namely:

  • Renamed the function to open system and global configs to git_config_open_default
  • Made that function replace the existing git_config_open_global (i.e. killed the old one)
  • Made the various static functions inside fileops.c not start with git_futils_... so they won't be confused with public APIs

Since we talked, I also:

  • Rebased to be current with the latest development
  • Tested builds for Win32 and Win64, and fixed up a bunch of Win64 warnings
  • Fixing Win64 ended up including a rewrite of git_repository_message since it seemed to be leaking memory in addition to needing some 64-bit > 32-bit casts
  • Improved some comments in some public header files

Anyhow, I'm feeling like this is pretty merge-able at this point. Once we get that done, I'll rebase #852 so it only includes the submodule related changes and then it should become a much smaller diff and be easier to review.

libgit2 member


@vmg vmg merged commit c920e16 into libgit2:development Aug 23, 2012

1 check passed

Details default The Travis build passed

If giterr_clear() is called, shouldn't error be set to 0 as well? Otherwise, the following code leads to a return of error (with no giterr()) because the following for loop checks for !error. This pattern is repeated in various other instances in this commit.


If this is supposed to test the template support, shouldn't GIT_REPOSITORY_INIT_EXTERNAL_TEMPLATE be added here as well?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment