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.
> +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.
(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...
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.
> 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).
> +
> +#
> +# 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?
Because this pattern would appear a lot in superproject support,
it might be a good idea to give a new option, --subprojects, to
git-ls-files to limit its output to 160000 entries, but that is
a minor detail.
> + while read mode sha1 stage path
> + do
We would need to undo the shell-safety "quoted" output of paths
here. I suspect it would be much easier to code this in Perl or
Python, do the "grep -e" part above in the script, when we start
caring about unwrapping c-quoting of path (or "ls-files -z").
But that is a minor detail we could fix up later.
> + 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?
> +
> + 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...
> + 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.
> +
> + $(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
().
> ...
> +modules_update()
> +{
>...
> + if test "$subsha1" != "$sha1"
> + then
> + $(unset GIT_DIR && cd "$path" && git-fetch &&
> + git-checkout -q "$sha1") ||
> + die "Unable to checkout '$sha1' in submodule '$path'"
Likewise.
-
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
