Re: [PATCH 2/4] git-shell-commands: Add a command to list bare repos
- From
Greg Brockman <gdb@mit.edu>
- Date
- Jul 26, 2010, 23:28 UTC
- Message-ID
- <AANLkTi=WNQ1=dx3hssgs_8Vkj6gEmrn7Z+KKdELnq+Ad@mail.gmail.com>
- In-Reply-To
- <7vlj96m4mc.fsf@alter.siamese.dyndns.org>
Show 15 quoted lines
>> @@ -0,0 +1,5 @@
>> +#!/bin/sh
>> +set -eu
>> +
>> +print_if_bare_repo='[ "$(git --git-dir="$1" rev-parse --is-bare-repository)" = true ] && echo "${1#./}"'
>
> That's a very long line you have here. It might be better to do split the
> line perhaps like this for readability:
>
> print_if_bare_repo='
> if "$(git --git-dir="$1" rev-parse --is-bare-repository)" = true
> then
> printf "%s\n" "${1#./}"
> fi
> 'Sure, this seems reasonable. Done in the latest version of my patch series. I also added a 2>/dev/null to the 'find' to discard unnecessary error output.
Show 11 quoted lines
> It is unclear why it limits its listing only to bare repositories. "It's > my design decision" is a perfectly acceptable answer, but no matter what > the reasoning is, it needs to be documented as a part of "How to use this" > insn to the users. A separate file README in contrib/git-shell-commands > that reads like: > > Any bare repository whose name ends with ".git" under your home > directory is visible by "list" extended command (no other git > repositories are visible). > > would probably be a good start.
Yes, basically "it's my design decision" is the answer here. I now include a contrib/git-shell-commands/README, which contains the following text regarding 'list': """ list: Displays any bare repository whose name ends with ".git" under user's home directory. No other git repositories are visible, although they might be clonable through git-shell. 'list' is designed to minimize the number of calls to git that must be made in finding available repositories; if your setup has additional repositories that should be user-discoverable, you may wish to modify 'list' accordingly. """
>> +find -type d -name "*.git" -exec sh -c "$print_if_bare_repo" -- \{} \; -prune
>
> Also do you need "set -eu" at the beginning? I don't see it serving a
> useful purpose (other than being a development aid, that is).Yeah, it's was just a development aid. I've removed the "set -eu" line.