#981 git support for dunecontrol

Metadata

Property	Value
Reported by	Christoph Grüninger (gruenich@iws.uni-stuttgart.de)
Reported at	Nov 21, 2011 12:17
Type	Feature Request
Version	Git (pre2.4) [autotools]
Operating System	Unspecified / All
Last edited by	Sven Marnach (sven@pantoffel-wg.de)
Last edited at	Dec 5, 2011 12:32
Closed by	Sven Marnach (sven@pantoffel-wg.de)
Closed at	Dec 5, 2011 12:32
Closed in version	Unknown
Resolution	Implemented
Comment	Implemented in SVN revision 6536.

Description

Together with Andreas Lauser, we prepared a patch for dunecontrol to support git as a version control system. This is quite handy because some modules like dune-multidomain are kept in git repositories. The patch is local and considers the use of git-svn.

Attachments

• git-dunecontrol.patch

I remember vaguely that we used to have that. It got removed because it never worked well. Does anybody remember the details?

Maybe #479 (closed) or #429 (closed). But I would like to have git support. We tried to avoid messing around with git svn, thats we wrote it that way.

Everybody wants to have git support :-) If your code succeeds where Sven's failed, then it is very welcome.

#429 (closed) was a trivial fix. #479 (closed) was (by Sven):

====================================================================== The support for git-svn in dunecontrol only works for one special use of git and breaks things in many other use cases. These cases include one-shot checkouts using git-svn, not being on the master branch and having modified files that are different in the current branch, being in the middle of an interactive rebase and many more.

Moreover, local changes in .git/info/exclude are overwritten (without any backup copy!) and replaced by some arbitrary default.

I think the git-svn support should be dropped. It would be a lot of work to fix it, and hardly anyone would use it. In my local checkout, I removed the support a long time ago because it screwed up "dunecontrol update" over and over again.

These concerns were for a different implementation, but lets see how valid they are with the proposed patch.

1. One-shot checkouts: A one-shot checkout, I believe, is where you convert a subversion-repository to a git repository, and subsequently make the git-repository authorative, either as a fork or by replacing the subversion repository. The problem here is that a one-shot checkout differs only in purpose from any other git-svn repository. On dunecontrol update it would try to download changes from svn. In the case of a fork, it would then try to rebase the "local changes" onto the "upstream changes", which (failing or not) is not what is desired. This is undesired, but in git this change to the repository can be undone. In other cases, the download would either download nothing, or would fail because the upstream svn repository no longer exists. In any case, no great harm is done, but having modules in such repositories would prevent the use of "dunecontrol update" completely.

2. Being on a different branch than master: If the branch is a svn branch as well, the patch should work flawlessly. If it isn't the branch is rebased onto the current head of the svn branch (or trunk) it was branched of from. This may or may not be what you want.

3. Modified files: You're not supposed to rebase with modified and uncommited files. If you do, git will error out. This is actually reasonable behaviour, so I see no problem here.

4. Middle of an interactive rebase: This is another repository state where you shouldn't try to update. I'm not actually sure what git will do if you try, but I suppose it will error out because you're attemping a rebase while another rebase is going on.

5. Local changes in .git/info/exclude: The current patch doesn't try to deal with excludes/ignores, so this isn't a problem.

To remedy the remaining problems, we can introduce a git config option, which can be set per-repository, like

[ dune ] update = false # or git, or svn

This can be used to either disable updating on "dunecontrol update", or to force the plain git or git-svn update methods (instead of autodetecting them). This way you can force one-shot checkout to have no upstream or to even have a git upstream. If you create a branch that you don't want to update from svn, you can (temporarily) mark your repository not to be updated.

We could even introduce a per-branch option (dune-update) so people don't have to edit their git-options each time they switch branches. But I suppose that would be the another iteration.

IMHO it would be a little problematic to enforce a default behaviour for git. While dunecontrol should clearly do 'svn up' it is in general not clear if you want 'git pull' or 'git rebase'.

Why not use git submodules to manage several independent git repositories?

Andreas Lauser (andreas.lauser@iws.uni-stuttgart.de) commented on the issue:

Hi Everyone,

First, let me be clear that, although I'm primarily responsible for the patch, I don't use dunecontrol's update feature and thus I do not really care about whether it gets merged or not. So, without further ado, here are my remarks:

0. git got quite a bit more careful about local changes since Sven wrote #479 (closed). For example it used to silently revert all local changes when switching branches or it allowed to pull in the middle of a rebase. no more. In addition, there is always 'git reflog' to the rescue if something gets seriously screwed up.
1. On the question of rebase-vs-pull: IMHO that only affects modules which natively use git. for the core modules, you will probably always want to use 'git svn rebase', as it is the only way to work with SVN repositories that I'm aware of. (We can also trivially extend the patch so that it aborts the rebase if there are any conflicts, but in this case dunecontrol would be more careful when using git than when using SVN.) Further, I would mandate for 'git pull' behavior for native git repositories. But let's cross that bridge when we come to it.
2. Finally, I slowly get the impression that the semantics of 'duncontrol update' are too imprecise and should be improved. maybe it should only update modules which do not have any local changes, regardless of the SCM involved.

I wasn't aware of the bunch of use cases with git-svn. Maybe it might be a better idea to exclude it from dunecontrol. The attached patch checks for git-svn and does nothing when it is found. If git without git-svn is found, "git pull" is called. Is this a better approach?

Sven Marnach (sven@pantoffel-wg.de) commented on the issue:

A few remarks from my part:

1. The old git-svn support wasn't written by me, as suggested above. :)

2. The old code was only meant to support the use case that a central SVN repository is checked out using git-svn. Unfortunately it broke native git repositories in several ways. So the old code wasn't git support -- it was git-svn support making the use of native git quite uncomfortable.

3. As far as I'm aware, git never silently reverted all local changes when switching branches -- at least this wasn't what I was referring to in #FS479. And the reflog wouldn't help me to recover an overwritten .git/info/exclude.

4. I consider an automatic "git pull" for native git repositories harmful. I, for one, usually do not want to merge. Maybe it is an option to just "git fetch" to update the remote branches and let the user decide whether to merge, rebase, cherry-pick or whatever, but this would make the semnatics of "duncontrol update" very strange because it did essentially different things for different VCSs.

There doesn't seem to be a satisfactory "one size fits all" solution, so let the user decide:

I propose to introduce a per-module "dune-update-hook" script. If such a hook is found in the module directory, it is execduted and no further action taken. If not, and dunecontrol detects svn or cvs, the current behaviour is retained. The proposed patch for git could then just be shipped as an example "dune-update-hook" for git, and everyone can easily customise it to one's needs.

Andreas Lauser (andreas.lauser@iws.uni-stuttgart.de) commented on the issue:

@sven: I think I lost a day's work on point 3 once or twice, but it may be that I screwed it differently. The update hook is a good idea, but to me it seems to be too complicated (if you write a hook, you may also write a bash script which does the job of 'duncontrol update' completely.)

I still don't care about the update feature, but here is a patch (for dune 2.1) which bails out if the current head is not the head of a remote branch or if there are uncommitted changes in the working copy. (strictly speaking, the latter check is not really necessary as git refuses to rebase/pull in that case.)

Attachments

• 0001-add-git-support-for-dunecontrol-update.patch

Sven Marnach (sven@pantoffel-wg.de) commented on the issue:

@Andreas: Writing your own script to do "dunecontrol update" is much more complex than just writing a hook. You would have to replicate the whole module detection functionality of dunecontrol. ("dunecontrol print" doesn't help you -- it only prints the module names, and there doesn't seem to be a way to deduce paths from module names.)

I still think the hook is the easiest and most general solution. It would allow to easily exclude modules from "dunecontrol update" ("touch update-hook", or however the hook would be called). Simply copying the example git update hook from dune-common would enable the proposed git support. We wouldn't need the git config option suggested by Jö.

Alas, I don't actually care much about the feature either. I do use "dunecontrol update" and I want it to update my svn repositories and not to break anything else.

Regarding the proposed patch: It seems to be quite careful about whether to call git at all, yet it would break at least one of my repositories. This repository has both, svn and git remotes, with the remote git repository being the "main" repository. If this repository is up to date with the git remote repository, the proposed patch would try to rebase the svn remote on top of it, which is clearly undesirable. If this patch gets applied, I'd like some option to disable it.

Another problem with the patch is that it tries to parse refs itself, bypassing the API to access them. This fails in multiple ways; most notably it completely ignores packed-refs.

I would go for a much simpler approach: Always call "git fetch", silencing any errors. If .git/svn exists, also call "git svn fetch", again silencing errors. These commands only update the remote tracking branches, no risk of breaking anything here. Finally, call "git merge --ff-only". This will merge only if it is a fast forward, and will work for both, native git and git-svn, since in the case of a fast forward, a rebase and a merge are the same. It also implicitly checks for local changes, eliminating any sophisticated tests.

Andreas Lauser (andreas.lauser@iws.uni-stuttgart.de) commented on the issue:

@Sven: I rather thought it's something like this (that's how I do updates BTW):

for TMP in dune-* custom-module; do git svn rebase # or whatever done

the module detection code is not necessary, because you know in advance which modules are around. if you want to only update the core modules, maybe a new duncontrol command "update-core-modules" should be introduced.

hm, thanks for suggesting --ff-only. does this work with "git-svn dcommit"?

Sven Marnach (sven@pantoffel-wg.de) commented on the issue:

@Andreas: Regarding "git merge --ff-only": In the case that merging another branch into the current branch results in a fast forward, "git merge other" and "git rebase other" both do the same thing -- they fast-forward the current branch. Since "git svn rebase" is just "git svn fetch" followed by "git rebase", there won't be any difference to "git svn rebase". In conclusion: Yes, this works with "git svn dcommit" -- of course assuming that the branch is correctly configured in a way that "git merge" knows which branch to merge.

Andreas Lauser (andreas.lauser@iws.uni-stuttgart.de) commented on the issue:

Thanks for pointing this out. Do you see any problems with the patch attached?

Attachments

• 0001-add-git-support-for-dunecontrol-update.patch

Sven Marnach (sven@pantoffel-wg.de) commented on the issue:

I'm sorry, my last comments were partially wrong. I assumed git-svn would set FETCH_HEAD after doing "git svn fetch". Well, it doesn't. And there is no way to find out which svn branch "git svn rebase" would rebase upon. "git svn" determines this branch by parsing the whole output of "git log", but there is know way to make git-svn reveal this information, which means that the whole idea doesn't work.

I attached a new version of the patch which first tests whether the current HEAD points to a commit that came from svn, and if it does, it calls "git svn rebase". Otherwise, it calls "git remote update" to update all remotes, and finally tries to fast-forward using "git merge --ff-only".

Another advantage over the previous patch is that it doesn't error out if there are neither git nor svn remotes. (I think dunecontrol would stop on any command that has non-zero exit code since it runs with "set -e".)

I've tested this version with a few of my repositories, and it seems to work fine. I didn't test really rigorously, but I'm pretty confident the patch won't lead to data loss.

Note that the new version of the patch is against the current trunk.

Attachments

• git-support-for-dunecontrol.patch

Does anyone object to sven's patch? Can it be committed, please? I tested it and worked for me. But I am only using SVN and git but no git-svn.

Status changed to closed