Page MenuHomePhabricator

Remap all submodules to tin

Authored by thcipriani on Oct 6 2015, 12:18 AM.
Referenced Files
Unknown Object (File)
Mon, Aug 1, 7:22 PM
Unknown Object (File)
Jun 24 2022, 3:33 AM
Unknown Object (File)
Jun 24 2022, 3:27 AM
Unknown Object (File)
Jun 19 2022, 7:04 AM
Unknown Object (File)
Apr 7 2017, 12:55 PM
Unknown Object (File)
Mar 13 2017, 12:52 PM
Unknown Object (File)
Feb 17 2017, 1:40 AM
Unknown Object (File)
Feb 17 2017, 1:40 AM


Group Reviewers
rMWTS20a383d71327: Remap all submodules to tin
rMSCA20a383d71327: Remap all submodules to tin
Patch without arc
git checkout -b D9 && curl -L | git apply

Change-Id: I905ffe975b6c37f63da7347508676b6825f90554

Test Plan


Change-Id: Ie5d3d8f3fa68d6a0190319cec7069604c671290a

Diff Detail

rMSCA Scap
Lint Not Applicable
Tests Not Applicable

Event Timeline

thcipriani retitled this revision from to Remap all submodules to tin.
thcipriani updated this object.
thcipriani edited the test plan for this revision. (Show Details)
thcipriani added reviewers: mmodell, dduvall, demon.

This seems to say that all submodules have to be named '/repo/modules/something' which isn't very flexible. Am I missing something?


The path here is actually: [server's gitdir]/modules/[submodule-path], so, for example, to point to the restbase submodule the remote url is: http://tin/deployment/restbase/.git/modules/restbase

This is the submodule's gitdir path as of git 1.7.8 (tin is on 1.7.9):

In testing I found that, while a local submodule clone will obey a submodule .git file's gitdir: [path], a remote http submodule clone just complains that [submodule/path]/.git/ is not a directory. As such you have to update the remote's submodule path to point to the location of a submodule's actual git directory at [parent gitdir]/modules/[submodule-path]

Break apart submodule update and git checkout

The tasks.git_checkout command was previously handling submodule
checkout as well. Now that we're remapping submodules, the additional
functionality warrants its own function.

mmodell edited edge metadata.
mmodell added inline comments.

I think we should switch to using get_logger() and stop using logger arguments, given how problematic the @decorator function signature stuff turned out to be. Is there an advantage to using the logger kwarg?

This is not a blocker though, I'll accept this as is.

This revision is now accepted and ready to land.Oct 7 2015, 1:00 PM

Can we make this optional? There might be situations where it makes more sense to pull from gerrit (or eventually phabricator) instead of tin.

thcipriani edited edge metadata.

Make submodule mapping to tin optional

Also, reduce the use of the "logger" keyword argument


One possible solution to avoid such repetition in tasks.git_* (logger, user, location etc.) and actually a lot of other tasks functions would be to introduce a basic Command or Proc class/module that encapsulates the command string, executing user, child logger, etc. There was actually a lot about checks.CheckJob and checks.execute, namely the event-loop schedule/run/poll/wait execution pattern, that I thought could live in such a module as well.

Anyway, I'm of the same mind: it's something to think about for future refactoring but not a blocker of current work.


We just want to process one level of submodules but won't this will process all .gitmodules files recursively?


Just throwing out a straw man here but since you're strip()-ing below, you might want to just split on '='. That would catch edge cases where someone might have manually modified the .gitmodules files and used no spaces (which I think is still technically valid as far as git is concerned).


I like the idea of a command / process class, and in fact I've implemented (badly) something like that in iscap. The reason for the abstraction in iscap is that I want to support executing regular shell subprocesses as well as python cli.Application instances, where the python commands can potentially run in-process instead of via the shell. The same abstraction would be really nice for remote commands which could have a local instance to represent the remote execution, making the code a lot more consistent and seamless (the remote execution would be completely abstracted away from the high-level logic)

Make submodule remap non-recursive, Add comments

git_remap_submodules now only looks for the top-level .gitmodules file
Added comments about how non-bare submodules are mapped for http remotes.

dduvall edited edge metadata.

Looks good!

This revision was automatically updated to reflect the committed changes.