On 5/26/07, Junio C Hamano <junkio@xxxxxxx> wrote:
Lars Hjemli <hjemli@xxxxxxxxx> writes:
> With this entry in .gitmodules (and a commit reference in the index entry for
> the path "git"), the command 'git submodule init' will clone the repository
> at kernel.org into the directory "git".
>
> Signed-off-by: Lars Hjemli <hjemli@xxxxxxxxx>
> ---
>
> On 5/26/07, Lars Hjemli <hjemli@xxxxxxxxx> wrote:
>> I'll redo the patch, removing the branch-specific things, and try to
>> shut up :)
Hey, don't shut up. Starting small and covering corner cases
incrementally is really the right approach.
Ok, my meds have kicked in and I'm ready for more ;-)
> +status::
> + Show the status of the submodules. This will print the sha1 of the
> + currently checked out commit for each submodule, along with the
> + submodule path and the output of gitlink:git-describe[1] for the
> + sha1. Each sha1 will be prefixed with '-' if the submodule is not
> + initialized and '+' if the currently checked out submodule commit
> + does not match the sha1 found in the index of the containing
> + repository. This command is the default command for git-submodule.
(markup) probably you would want `` there for typewriter face.
Ok, will fix
(wording) didn't we have "the name of the hash function is
SHA-1" patch earlier? I'd personally prefer calling them
"object names", though...
Some quick grepping wasn't helpfull:
~/src/git/Documentation$ git grep sha1 | wc -l
98
~/src/git/Documentation$ git grep SHA1 | wc -l
80
~/src/git/Documentation$ git grep SHA-1 | wc -l
22
Would you prefer SHA-1?
Other than that, the command description is very nicely done,
which means the design of the command set hence the semantics is
cleanly done. Good job.
Thanks!
> diff --git a/git-submodule.sh b/git-submodule.sh
> new file mode 100755
> index 0000000..247b1ee
> --- /dev/null
> +++ b/git-submodule.sh
> @@ -0,0 +1,172 @@
> ...
> +#
> +# print stuff on stdout unless -q was specified
> +#
> +say()
> +{
> + if test -z "$quiet"
> + then
> + echo -e "$@"
> + fi
> +}
We tend to avoid "echo -e" (not POSIX). I do not see any string
you feed to this function that you would _want_ backslash
escaped sequences (actually I would suspect you would not want
them).
I do use \t between submodule path and the result of git-describe, but
it's not really needed. I'll drop it.
> +
> +#
> +# Run clone + checkout on missing submodules
> +#
> +# $@ = requested paths (default to all)
> +#
> +modules_init()
> +{
> + git ls-files --stage -- $@ | grep -e '^160000 ' |
Did you mean "$@", i.e. inside double-quotes?
That looks right, yes
> + test -d "$path/.git" && continue
> +
> + if test -d "$path"
> + then
> + rmdir "$path" 2>/dev/null ||
> + die "Directory '$path' exist, but not as a submodule"
> + fi
Could the currently checked-out $path be a symlink to another
directory, and what does the code do in such a case?
This I will have to test, but I _suspect_ it would fail on "test -e
path" _unless_ the "test -d $path/.git" kicks in.
> +
> + test -e "$path" &&
> + die "A file already exist at path '$path'"
"test -e" is a relatively new invention and I tended to stay
away from it, but it should be safe to use these days...
Would you prefer separate -f and -l instead? Hmm, that would match up
nicely with your concern about symlinks ;-)
> + url=$(GIT_CONFIG=.gitmodules git-config module."$path".url)
> + test -z "$url" &&
> + die "No url found for submodule '$path' in .gitmodules"
> +
> + git-clone "$url" "$path" ||
> + die "Clone of submodule '$path' failed"
"git-clone -n" please, as you will checkout something different
in the next step anyway.
Nice, I hadn't noticed -n, will fix
> +
> + $(unset GIT_DIR && cd "$path" && git-checkout -q "$sha1") ||
> + die "Checkout of submodule '$path' failed"
Lose $() around this, as it is not producing a string which is
the name of the command to run. You do want a subshell here
because of chdir, so instead of losing $(), replace them with
().
Heh, there's always something new to learn on this list, thanks.
--
larsh
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
