* Re: [RFC/PATCH 1/4] show: obey --textconv for blobs
From: Jeff King @ 2013-02-06 22:06 UTC (permalink / raw)
To: Michael J Gruber; +Cc: git, Junio C Hamano
In-Reply-To: <883f0163cb732932061a368ea9bc187c13e4ecca.1360162813.git.git@drmicha.warpmail.net>
On Wed, Feb 06, 2013 at 04:08:50PM +0100, Michael J Gruber wrote:
> diff --git a/builtin/log.c b/builtin/log.c
> index 8f0b2e8..f83870d 100644
> --- a/builtin/log.c
> +++ b/builtin/log.c
> @@ -402,10 +402,28 @@ static void show_tagger(char *buf, int len, struct rev_info *rev)
> strbuf_release(&out);
> }
>
> -static int show_blob_object(const unsigned char *sha1, struct rev_info *rev)
> +static int show_blob_object(const unsigned char *sha1, struct rev_info *rev, const char *obj_name)
Should this maybe just take the whole object_array_entry as a cleanup?
> {
> + unsigned char sha1c[20];
> + struct object_context obj_context;
> + char *buf;
> + unsigned long size;
> +
> fflush(stdout);
> - return stream_blob_to_fd(1, sha1, NULL, 0);
> + if (!DIFF_OPT_TST(&rev->diffopt, ALLOW_TEXTCONV))
> + return stream_blob_to_fd(1, sha1, NULL, 0);
> +
> + if (get_sha1_with_context(obj_name, 0, sha1c, &obj_context))
> + die("Not a valid object name %s", obj_name);
It seems a little hacky that we have to look up the sha1 again. What
should happen in the off chance that "hashcmp(sha1, sha1c) != 0" due to
a race with a simultaneous update of a ref?
Would it be better if object_array_entry replaced its "mode" member with
an object_context? The only downside I see is that we might waste a
significant amount of memory (each context has a PATH_MAX buffer in it).
-Peff
^ permalink raw reply
* [PATCH] connect.c: Tell *PLink to always use ssh protocol
From: Sven Strickroth @ 2013-02-06 21:58 UTC (permalink / raw)
To: git; +Cc: gitster, Jeff King
Default values for *plink can be set using PuTTY. If a user makes
telnet the default in PuTTY this breaks ssh clones in git.
Since git clones of the type user@host:path use ssh, tell *plink
to use ssh and override PuTTY defaults for the protocol to use.
Signed-off-by: Sven Strickroth <email@cs-ware.de>
---
connect.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/connect.c b/connect.c
index 49e56ba..d337b6f 100644
--- a/connect.c
+++ b/connect.c
@@ -625,6 +625,8 @@ struct child_process *git_connect(int fd[2], const char *url_orig,
if (!ssh) ssh = "ssh";
*arg++ = ssh;
+ if (putty)
+ *arg++ = "-ssh";
if (putty && !strcasestr(ssh, "tortoiseplink"))
*arg++ = "-batch";
if (port) {
--
Best regards,
Sven Strickroth
PGP key id F5A9D4C4 @ any key-server
^ permalink raw reply related
* Re: [PATCH] git-send-email: add ~/.authinfo parsing
From: Jeff King @ 2013-02-06 21:57 UTC (permalink / raw)
To: Ted Zlatanov
Cc: Matthieu Moy, Junio C Hamano, Michal Nazarewicz, git,
Krzysztof Mazur, Michal Nazarewicz
In-Reply-To: <87sj59mo2y.fsf@lifelogs.com>
On Wed, Feb 06, 2013 at 10:58:13AM -0500, Ted Zlatanov wrote:
> MM> I don't know about the netrc credential helper, but I guess that's
> MM> another layer. The git-remote-mediawiki code is the code to call the
> MM> credential C API, that in turn may (or may not) call a credential
> MM> helper.
>
> Yup. But what you call "read" and "write" are, to the credential
> helper, "write" and "read" but it's the same protocol :) So maybe the
> names should be changed to reflect that, e.g. "query" and "response."
Is that true? As a user of the credential system, git-remote-mediawiki
would want to "write" to git-credential, then "read" the response. As a
helper, git-credential-netrc would want to "read" the query then
"write" the response. The order is different, but the operations should
be the same in both cases.
The big difference is that mediawiki would want an additional function
to open a pipe to "git credential" and operate on that, whereas the
helper will be reading/writing stdio.
-Peff
^ permalink raw reply
* Re: [PATCH v3 0/8] Hiding refs
From: Michael Haggerty @ 2013-02-06 21:50 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Duy Nguyen, Jonathan Nieder, git, Jeff King, Shawn Pearce
In-Reply-To: <7v4nhpckwd.fsf@alter.siamese.dyndns.org>
On 02/06/2013 08:17 PM, Junio C Hamano wrote:
> Duy Nguyen <pclouds@gmail.com> writes:
>
>> On Tue, Feb 5, 2013 at 5:29 PM, Michael Haggerty <mhagger@alum.mit.edu> wrote:
>>> Hiderefs creates a "dark" corner of a remote git repo that can hold
>>> arbitrary content that is impossible for anybody to discover but
>>> nevertheless possible for anybody to download (if they know the name of
>>> a hidden reference). In earlier versions of the patch series I believe
>>> that it was possible to push to a hidden reference hierarchy, which made
>>> it possible to upload dark content. The new version appears (from the
>>> code) to prohibit adding references in a hidden hierarchy, which would
>>> close the main loophole that I was worried about. But the documentation
>>> and the unit tests only explicitly say that updates and deletes are
>>> prohibited; nothing is said about adding references (unless "update" is
>>> understood to include "add"). I think the true behavior should be
>>> clarified and tested.
>>>
>>> I was worried that somehow this "dark" content could be used for
>>> malicious purposes; for example, pushing compromised code then
>>> convincing somebody to download it by SHA1 with the implicit argument
>>> "it's safe since it comes directly from the project's official
>>> repository". If it is indeed impossible to populate the dark namespace
>>> remotely then I can't think of a way to exploit it.
>>
>> Or you can think hiderefs is the first step to addressing the
>> initial ref advertisment problem. The series says hidden refs are
>> to be fetched out of band, but that's not the only way.
>
> Let me help unconfuse this thread.
>
> I think the series as 8-patch series was poorly presented, and
> separating it into two will help understanding what they are about.
>
> The first three:
>
> upload-pack: share more code
> upload-pack: simplify request validation
> upload/receive-pack: allow hiding ref hierarchies
>
> is _the_ topic of the series. As far as I am concerned (I am not
> speaking for Gerrit users, but am speaking as the Git maintainer),
> the topic is solely about uncluttering. There may be refs that the
> server end may need to keep for its operation, but that remote users
> have _no_ business knowing about. Allowing the server to keep these
> refs in the repository, while not showing these refs over the wire,
> is the problem the series solves.
>
> In other words, it is not about "these are *usually* not wanted by
> clients, so do not show them by default". It is about "these are
> not to be shown, ever".
>
> OK?
Yes, the first three patches sound much more reasonable if this is the
goal. Do you know of users who want the feature defined by the first
three patches, or is it only a stepping stone towards an actually useful
feature? (I ask because I have trouble imagining a real-world scenario
where these alone would be useful.)
> Now, there may be some refs that are not *usually* wanted by clients
> but there may be cases where clients want to
>
> (1) learn about them via the same protocol; and/or
> (2) fetch them over the protocol.
>
> If you want to solve both of these two issues generally, the
> solution has to involve a separate protocol from the today's
> protocol. It would go like this:
[... omitted clear explanation of how delayed advertisement could be
implemented via a new protocol ...]
> But in the meantime, if there is a niche use case where a solution
> to only the second problem is sufficient (and Gerrit and GitHub pull
> requests could both be such use cases), the remainder of the series
> can help, without waiting the solution to solve "usually not wanted
> but may need to be learned" problem. That is the latter 4 patches
> (the very last one is a demonstration to illustrate why allowing a
> push to hidden ref hierarchy would not and should not work, and is
> not for application):
Given that some people *do* want to fetch all pull requests, is this a
feature that any hosting service would really turn on? True, the
majority of users would be spared clutter, but at the cost of completely
preventing other users from fetching all pull requests, mirroring the
repository, etc.
In other words, I wonder whether your two incremental steps are useful
at all, in the real world, without yet-to-be-implemented future changes.
If not, then it doesn't make sense to merge them without at least
imagining the final goal and gaining confidence that they are not false
starts.
I think that a more useful interim solution would be to make it easy to
have two URLs accessing a single git repository, with different levels
of reference visibility applied to each. This is something that
providers could turn on without sacrificing any existing functionality.
And it would solve all three problems: clutter, bandwidth, and provenance.
Your first three patches would allow two-tier access to be implemented,
for example by setting GIT_CONFIG or GIT_CONFIG_PARAMETERS or
command-line parameters differently for the processes serving the two
URLs, like:
git upload-pack ...
vs.
GIT_CONFIG=config-with-hidden-refs git upload-pack ...
or
git -c transfer.hiderefs=refs/pull upload-pack ...
But this is a bit awkward because the admin would either have to
maintain two config files, or maintain the hiderefs configuration in the
script starting upload-pack rather than in the configuration file.
Therefore, I suggest a slight change to how hiderefs are configured to
make two-tier URLs easier to configure, such as
# Define one or more views:
[view "uncluttered"]
hiderefs = refs/pull
# This would set the default view for all services:
[transfer]
view = uncluttered
# Peff also wanted the possibility to configure each service
# independently which could be done like this:
[receive]
view = uncluttered
[uploadpack]
view = full
I also tentatively suggest that we add a git-level option "--view" and
an environment variable GIT_VIEW (similar to "--namespace" and
GIT_NAMESPACE) to override the default setting:
GIT_VIEW=uncluttered git upload-pack ...
This way whoever starts the process only needs to choose a particular
view name; the actual definition would reside in the config file.
I think these changes would make it easier to support two-tier URLs and
would also leave the way open to use the "view" concept for other things
in the future.
I've said my piece now and am gratified that there has been more
discussion about your proposal, which was my main goal. Therefore FWIW
I turn my -1 into a -0 and leave it up to the people experiencing more
clutter-induced pain to decide how to proceed.
Michael
--
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/
^ permalink raw reply
* Re: will git provide `submodule remove` option ?
From: Jens Lehmann @ 2013-02-06 21:27 UTC (permalink / raw)
To: Lingcha X; +Cc: git@vger.kernel.org
In-Reply-To: <BAY176-W2328CE9FE72C92BCB27421B4000@phx.gbl>
Am 05.02.2013 11:32, schrieb Lingcha X:
> As we all know, git provides `submodule add , init, update, sync, sumary, foreach, status`, but where is `submodule remove`?
>
> will
> I not delete one of them sometime in the future? Although most people
> will not use submodule or one who uses it can remove submodule by hand, providing complete options may be a good idea.
Is assume either "git rm <submodule>" (available since 1.8.1) or the upcoming
"git submodule deinit" (currently in pu) will do what you want?
^ permalink raw reply
* [PATCH v4] submodule: add 'deinit' command
From: Jens Lehmann @ 2013-02-06 21:11 UTC (permalink / raw)
To: Git Mailing List
Cc: Junio C Hamano, Heiko Voigt, Michael J Gruber, Phil Hord,
Marc Branchaud, W. Trevor King
With "git submodule init" the user is able to tell git he cares about one
or more submodules and wants to have it populated on the next call to "git
submodule update". But currently there is no easy way he could tell git he
does not care about a submodule anymore and wants to get rid of his local
work tree (except he knows a lot about submodule internals and removes the
"submodule.$name.url" setting from .git/config together with the work tree
himself).
Help those users by providing a 'deinit' command. This removes the whole
submodule.<name> section from .git/config either for the given
submodule(s) or for all those which have been initialized if '.' is
given. Fail if the current work tree contains modifications unless
forced. Complain when for a submodule given on the command line the url
setting can't be found in .git/config, but nonetheless don't fail.
Add tests and link the man pages of "git submodule deinit" and "git rm"
to assist the user in deciding whether removing or unregistering the
submodule is the right thing to do for him.
Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
---
Changes since v3:
- Add deinit to the --force documentation of "git submodule"
- Never remove submodules containing a .git dir, even when forced
- Diagnostic output when "rm -rf" or "mkdir" fails
- More test cases
Documentation/git-rm.txt | 4 ++
Documentation/git-submodule.txt | 18 +++++++-
git-submodule.sh | 78 ++++++++++++++++++++++++++++++-
t/t7400-submodule-basic.sh | 100 ++++++++++++++++++++++++++++++++++++++++
4 files changed, 198 insertions(+), 2 deletions(-)
diff --git a/Documentation/git-rm.txt b/Documentation/git-rm.txt
index 92bac27..1d876c2 100644
--- a/Documentation/git-rm.txt
+++ b/Documentation/git-rm.txt
@@ -149,6 +149,10 @@ files that aren't ignored are present in the submodules work tree.
Ignored files are deemed expendable and won't stop a submodule's work
tree from being removed.
+If you only want to remove the local checkout of a submodule from your
+work tree without committing the removal,
+use linkgit:git-submodule[1] `deinit` instead.
+
EXAMPLES
--------
`git rm Documentation/\*.txt`::
diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index a0c9df8..bc06159 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -13,6 +13,7 @@ SYNOPSIS
[--reference <repository>] [--] <repository> [<path>]
'git submodule' [--quiet] status [--cached] [--recursive] [--] [<path>...]
'git submodule' [--quiet] init [--] [<path>...]
+'git submodule' [--quiet] deinit [-f|--force] [--] <path>...
'git submodule' [--quiet] update [--init] [--remote] [-N|--no-fetch] [--rebase]
[--reference <repository>] [--merge] [--recursive] [--] [<path>...]
'git submodule' [--quiet] summary [--cached|--files] [(-n|--summary-limit) <n>]
@@ -134,6 +135,19 @@ init::
the explicit 'init' step if you do not intend to customize
any submodule locations.
+deinit::
+ Unregister the given submodules, i.e. remove the whole
+ `submodule.$name` section from .git/config together with their work
+ tree. Further calls to `git submodule update`, `git submodule foreach`
+ and `git submodule sync` will skip any unregistered submodules until
+ they are initialized again, so use this command if you don't want to
+ have a local checkout of the submodule in your work tree anymore. If
+ you really want to remove a submodule from the repository and commit
+ that use linkgit:git-rm[1] instead.
++
+If `--force` is specified, the submodule's work tree will be removed even if
+it contains local modifications.
+
update::
Update the registered submodules, i.e. clone missing submodules and
checkout the commit specified in the index of the containing repository.
@@ -213,8 +227,10 @@ OPTIONS
-f::
--force::
- This option is only valid for add and update commands.
+ This option is only valid for add, deinit and update commands.
When running add, allow adding an otherwise ignored submodule path.
+ When running deinit the submodule work trees will be removed even if
+ they contain local changes.
When running update, throw away local changes in submodules when
switching to a different commit; and always run a checkout operation
in the submodule, even if the commit listed in the index of the
diff --git a/git-submodule.sh b/git-submodule.sh
index 004c034..f1b552f 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -8,6 +8,7 @@ dashless=$(basename "$0" | sed -e 's/-/ /')
USAGE="[--quiet] add [-b <branch>] [-f|--force] [--name <name>] [--reference <repository>] [--] <repository> [<path>]
or: $dashless [--quiet] status [--cached] [--recursive] [--] [<path>...]
or: $dashless [--quiet] init [--] [<path>...]
+ or: $dashless [--quiet] deinit [-f|--force] [--] <path>...
or: $dashless [--quiet] update [--init] [--remote] [-N|--no-fetch] [-f|--force] [--rebase] [--reference <repository>] [--merge] [--recursive] [--] [<path>...]
or: $dashless [--quiet] summary [--cached|--files] [--summary-limit <n>] [commit] [--] [<path>...]
or: $dashless [--quiet] foreach [--recursive] <command>
@@ -547,6 +548,81 @@ cmd_init()
}
#
+# Unregister submodules from .git/config and remove their work tree
+#
+# $@ = requested paths (use '.' to deinit all submodules)
+#
+cmd_deinit()
+{
+ # parse $args after "submodule ... init".
+ while test $# -ne 0
+ do
+ case "$1" in
+ -f|--force)
+ force=$1
+ ;;
+ -q|--quiet)
+ GIT_QUIET=1
+ ;;
+ --)
+ shift
+ break
+ ;;
+ -*)
+ usage
+ ;;
+ *)
+ break
+ ;;
+ esac
+ shift
+ done
+
+ if test $# = 0
+ then
+ die "$(eval_gettext "Use '.' if you really want to deinitialize all submodules")"
+ fi
+
+ module_list "$@" |
+ while read mode sha1 stage sm_path
+ do
+ die_if_unmatched "$mode"
+ name=$(module_name "$sm_path") || exit
+ url=$(git config submodule."$name".url)
+ if test -z "$url"
+ then
+ say "$(eval_gettext "No url found for submodule path '\$sm_path' in .git/config")"
+ continue
+ fi
+
+ # Remove the submodule work tree (unless the user already did it)
+ if test -d "$sm_path"
+ then
+ # Protect submodules containing a .git directory
+ if test -d "$sm_path/.git"
+ then
+ echo >&2 "$(eval_gettext "Submodule work tree $sm_path contains a .git directory")"
+ die "$(eval_gettext "(use 'rm -rf' if you really want to remove it including all of its history)")"
+ fi
+
+ if test -z "$force"
+ then
+ git rm -n "$sm_path" ||
+ die "$(eval_gettext "Submodule work tree $sm_path contains local modifications, use '-f' to discard them")"
+ fi
+ rm -rf "$sm_path" || say "$(eval_gettext "Could not remove submodule work tree '\$sm_path'")"
+ fi
+
+ mkdir "$sm_path" || say "$(eval_gettext "Could not create empty submodule directory '\$sm_path'")"
+
+ # Remove the whole section so we have a clean state when the
+ # user later decides to init this submodule again
+ git config --remove-section submodule."$name" &&
+ say "$(eval_gettext "Submodule '\$name' (\$url) unregistered for path '\$sm_path'")"
+ done
+}
+
+#
# Update each submodule path to correct revision, using clone and checkout as needed
#
# $@ = requested paths (default to all)
@@ -1157,7 +1233,7 @@ cmd_sync()
while test $# != 0 && test -z "$command"
do
case "$1" in
- add | foreach | init | update | status | summary | sync)
+ add | foreach | init | deinit | update | status | summary | sync)
command=$1
;;
-q|--quiet)
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index 2683cba..f54a40d 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -757,4 +757,104 @@ test_expect_success 'submodule add with an existing name fails unless forced' '
)
'
+test_expect_success 'set up a second submodule' '
+ git submodule add ./init2 example2 &&
+ git commit -m "submodle example2 added"
+'
+
+test_expect_success 'submodule deinit should remove the whole submodule section from .git/config' '
+ git config submodule.example.foo bar &&
+ git config submodule.example2.frotz nitfol &&
+ git submodule deinit init &&
+ test -z "$(git config submodule.example.url)" &&
+ test -z "$(git config submodule.example.foo)" &&
+ test -n "$(git config submodule.example2.url)" &&
+ test -n "$(git config submodule.example2.frotz)" &&
+ test -f example2/.git &&
+ rmdir init
+'
+
+test_expect_success 'submodule deinit . deinits all initialized submodules' '
+ git submodule update --init &&
+ git config submodule.example.foo bar &&
+ git config submodule.example2.frotz nitfol &&
+ test_must_fail git submodule deinit &&
+ git submodule deinit . &&
+ test -z "$(git config submodule.example.url)" &&
+ test -z "$(git config submodule.example.foo)" &&
+ test -z "$(git config submodule.example2.url)" &&
+ test -z "$(git config submodule.example2.frotz)" &&
+ rmdir init example2
+'
+
+test_expect_success 'submodule deinit deinits a submodule when its work tree is missing or empty' '
+ git submodule update --init &&
+ rm -rf init example2/* example2/.git &&
+ git submodule deinit init example2 &&
+ test -z "$(git config submodule.example.url)" &&
+ test -z "$(git config submodule.example2.url)" &&
+ rmdir init
+'
+
+test_expect_success 'submodule deinit fails when the submodule contains modifications unless forced' '
+ git submodule update --init &&
+ echo X >>init/s &&
+ test_must_fail git submodule deinit init &&
+ test -n "$(git config submodule.example.url)" &&
+ test -f example2/.git &&
+ git submodule deinit -f init &&
+ test -z "$(git config submodule.example.url)" &&
+ rmdir init
+'
+
+test_expect_success 'submodule deinit fails when the submodule contains untracked files unless forced' '
+ git submodule update --init &&
+ echo X >>init/untracked &&
+ test_must_fail git submodule deinit init &&
+ test -n "$(git config submodule.example.url)" &&
+ test -f example2/.git &&
+ git submodule deinit -f init &&
+ test -z "$(git config submodule.example.url)" &&
+ rmdir init
+'
+
+test_expect_success 'submodule deinit fails when the submodule HEAD does not match unless forced' '
+ git submodule update --init &&
+ (
+ cd init &&
+ git checkout HEAD^
+ ) &&
+ test_must_fail git submodule deinit init &&
+ test -n "$(git config submodule.example.url)" &&
+ test -f example2/.git &&
+ git submodule deinit -f init &&
+ test -z "$(git config submodule.example.url)" &&
+ rmdir init
+'
+
+test_expect_success 'submodule deinit complains but does not fail when used on an uninitialized submodule' '
+ git submodule update --init &&
+ git submodule deinit init >actual &&
+ test_i18ngrep "Submodule .example. (.*) unregistered for path .init" actual
+ git submodule deinit init >actual &&
+ test_i18ngrep "No url found for submodule path .init. in .git/config" actual &&
+ git submodule deinit . >actual &&
+ test_i18ngrep "Submodule .example2. (.*) unregistered for path .example2" actual
+ rmdir init example2
+'
+
+test_expect_success 'submodule deinit fails when submodule has a .git directory even when forced' '
+ git submodule update --init &&
+ (
+ cd init &&
+ rm .git &&
+ cp -R ../.git/modules/example .git &&
+ GIT_WORK_TREE=. git config --unset core.worktree
+ ) &&
+ test_must_fail git submodule deinit init &&
+ test_must_fail git submodule deinit -f init &&
+ test -d init/.git &&
+ test -n "$(git config submodule.example.url)"
+'
+
test_done
--
1.8.1.2.546.gea155c0
^ permalink raw reply related
* Re: [PATCH 0/4] Make git-send-email git-credential
From: Junio C Hamano @ 2013-02-06 20:54 UTC (permalink / raw)
To: Michal Nazarewicz; +Cc: Ted Zlatanov, Jeff King, Matthieu Moy, git
In-Reply-To: <xa1thalpp47z.fsf@mina86.com>
Michal Nazarewicz <mina86@mina86.com> writes:
> On second thought, give me a moment, ;) I've just discovered a bug
> preventing git-send-email from mailing a patchset.
I somehow found this highly amusing.
I wish all the bugs are like that: if your series is buggy, some
parts of the system prevents you from sending it to the list.
;-)
^ permalink raw reply
* Re: Why is ident_is_sufficient different on Windows?
From: Junio C Hamano @ 2013-02-06 20:52 UTC (permalink / raw)
To: Max Horn; +Cc: git
In-Reply-To: <7vip65b25c.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> I suspect somebody from the Windows camp saw a patch I posted
>> without the ifdef, noticed that there is a problem to expect
>> IDENT_NAME_GIVEN to be set on Windows for some reason, and resulted
>> in a reroll of the function in that shape.
>>
>> I didn't find anything in the list archive, though. So I am
>> stumped.
>
> The only thing I can think of is that on Unix we can guess name from
> GECOS, which could be considered sufficiently your name, while on
> Windows we probably do not get anything useful there.
http://thread.gmane.org/gmane.comp.version-control.git/137312/focus=137345
These days, we encourage setting user.name explicitly even on a
system on which it is likely that we will see a good GECOS value, so
removing the ifdef and always check with ALL may not hurt anybody.
I dunno.
^ permalink raw reply
* [PATCH 4/4] git-send-email: Use git credential to obtain password.
From: Michal Nazarewicz @ 2013-02-06 20:47 UTC (permalink / raw)
To: Junio C Hamano, Ted Zlatanov, Jeff King, Matthieu Moy; +Cc: git
In-Reply-To: <cover.1360183427.git.mina86@mina86.com>
From: Michal Nazarewicz <mina86@mina86.com>
If smtp_user is provided but smtp_pass is not, instead of prompting
for password, make git-send-email use git credential command
instead.
Signed-off-by: Michal Nazarewicz <mina86@mina86.com>
---
Documentation/git-send-email.txt | 4 +--
git-send-email.perl | 59 +++++++++++++++++++++++-----------------
2 files changed, 36 insertions(+), 27 deletions(-)
diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 44a1f7c..0cffef8 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -164,8 +164,8 @@ Sending
Furthermore, passwords need not be specified in configuration files
or on the command line. If a username has been specified (with
'--smtp-user' or a 'sendemail.smtpuser'), but no password has been
-specified (with '--smtp-pass' or 'sendemail.smtppass'), then the
-user is prompted for a password while the input is masked for privacy.
+specified (with '--smtp-pass' or 'sendemail.smtppass'), then
+a password is obtained using 'git-credential'.
--smtp-server=<host>::
If set, specifies the outgoing SMTP server to use (e.g.
diff --git a/git-send-email.perl b/git-send-email.perl
index be809e5..76bbfc3 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1045,6 +1045,39 @@ sub maildomain {
return maildomain_net() || maildomain_mta() || 'localhost.localdomain';
}
+# Returns 1 if authentication succeeded or was not necessary
+# (smtp_user was not specified), and 0 otherwise.
+
+sub smtp_auth_maybe {
+ if (!defined $smtp_authuser || $auth) {
+ return 1;
+ }
+
+ # Workaround AUTH PLAIN/LOGIN interaction defect
+ # with Authen::SASL::Cyrus
+ eval {
+ require Authen::SASL;
+ Authen::SASL->import(qw(Perl));
+ };
+
+ # TODO: Authentication may fail not because credentials were
+ # invalid but due to other reasons, in which we should not
+ # reject credentials.
+ $auth = Git::credential({
+ 'protocol' => 'smtp',
+ 'host' => join(':', $smtp_server, $smtp_server_port),
+ 'username' => $smtp_authuser,
+ # if there's no password, "git credential fill" will
+ # give us one, otherwise it'll just pass this one.
+ 'password' => $smtp_authpass
+ }, sub {
+ my $cred = shift;
+ return !!$smtp->auth($cred->{'username'}, $cred->{'password'});
+ });
+
+ return $auth;
+}
+
# Returns 1 if the message was sent, and 0 otherwise.
# In actuality, the whole program dies when there
# is an error sending a message.
@@ -1185,31 +1218,7 @@ X-Mailer: git-send-email $gitversion
defined $smtp_server_port ? " port=$smtp_server_port" : "";
}
- if (defined $smtp_authuser) {
- # Workaround AUTH PLAIN/LOGIN interaction defect
- # with Authen::SASL::Cyrus
- eval {
- require Authen::SASL;
- Authen::SASL->import(qw(Perl));
- };
-
- if (!defined $smtp_authpass) {
-
- system "stty -echo";
-
- do {
- print "Password: ";
- $_ = <STDIN>;
- print "\n";
- } while (!defined $_);
-
- chomp($smtp_authpass = $_);
-
- system "stty echo";
- }
-
- $auth ||= $smtp->auth( $smtp_authuser, $smtp_authpass ) or die $smtp->message;
- }
+ smtp_auth_maybe or die $smtp->message;
$smtp->mail( $raw_from ) or die $smtp->message;
$smtp->to( @recipients ) or die $smtp->message;
--
1.8.1.2.549.g4fa355e
^ permalink raw reply related
* Re: Why is ident_is_sufficient different on Windows?
From: Junio C Hamano @ 2013-02-06 20:47 UTC (permalink / raw)
To: Max Horn; +Cc: git
In-Reply-To: <7vmwvhb2fm.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> I suspect somebody from the Windows camp saw a patch I posted
> without the ifdef, noticed that there is a problem to expect
> IDENT_NAME_GIVEN to be set on Windows for some reason, and resulted
> in a reroll of the function in that shape.
>
> I didn't find anything in the list archive, though. So I am
> stumped.
The only thing I can think of is that on Unix we can guess name from
GECOS, which could be considered sufficiently your name, while on
Windows we probably do not get anything useful there.
^ permalink raw reply
* [PATCH 3/4] Git.pm: Add interface for git credential command.
From: Michal Nazarewicz @ 2013-02-06 20:47 UTC (permalink / raw)
To: Junio C Hamano, Ted Zlatanov, Jeff King, Matthieu Moy; +Cc: git
In-Reply-To: <cover.1360183427.git.mina86@mina86.com>
From: Michal Nazarewicz <mina86@mina86.com>
Add a credential() function which is an interface to the
git credential command.
Signed-off-by: Michal Nazarewicz <mina86@mina86.com>
---
perl/Git.pm | 112 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 111 insertions(+), 1 deletion(-)
diff --git a/perl/Git.pm b/perl/Git.pm
index 6a2d52d..5a18921 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -59,7 +59,8 @@ require Exporter;
command_bidi_pipe command_close_bidi_pipe
version exec_path html_path hash_object git_cmd_try
remote_refs prompt
- temp_acquire temp_release temp_reset temp_path);
+ temp_acquire temp_release temp_reset temp_path
+ credential);
=head1 DESCRIPTION
@@ -1000,6 +1001,115 @@ sub _close_cat_blob {
}
+sub _credential_read {
+ my %credential;
+ my ($reader, $op) = (@_);
+ while (<$reader>) {
+ chomp;
+ my ($key, $value) = /([^=]*)=(.*)/;
+ if (not defined $key) {
+ throw Error::Simple("unable to parse git credential $op response:\n$_\n");
+ }
+ $credential{$key} = $value;
+ }
+ return %credential;
+}
+
+sub _credential_write {
+ my ($credential, $writer) = @_;
+
+ for my $key (sort {
+ # url overwrites other fields, so it must come first
+ return -1 if $a eq 'url';
+ return 1 if $b eq 'url';
+ return $a cmp $b;
+ } keys %$credential) {
+ if (defined $credential->{$key} && length $credential->{$key}) {
+ print $writer $key, '=', $credential->{$key}, "\n";
+ }
+ }
+ print $writer "\n";
+}
+
+sub _credential_run {
+ my ($self, $credential, $op) = _maybe_self(@_);
+
+ my ($pid, $reader, $writer, $ctx) = command_bidi_pipe('credential', $op);
+
+ _credential_write $credential, $writer;
+ close $writer;
+
+ if ($op eq "fill") {
+ %$credential = _credential_read $reader, $op;
+ } elsif (<$reader>) {
+ throw Error::Simple("unexpected output from git credential $op response:\n$_\n");
+ }
+
+ command_close_bidi_pipe($pid, $reader, undef, $ctx);
+}
+
+=item credential( CREDENTIAL_HASH [, OPERATION ] )
+
+=item credential( CREDENTIAL_HASH, CODE )
+
+Executes C<git credential> for a given set of credentials and
+specified operation. In both form C<CREDENTIAL_HASH> needs to be
+a reference to a hash which stores credentials. Under certain
+conditions the hash can change.
+
+In the first form, C<OPERATION> can be C<'fill'> (or omitted),
+C<'approve'> or C<'reject'>, and function will execute corresponding
+C<git credential> sub-command. In case of C<'fill'> the values stored
+in C<CREDENTIAL_HASH> will be changed to the ones returned by the
+C<git credential> command. The usual usage would look something like:
+
+ my %cred = (
+ 'protocol' => 'https',
+ 'host' => 'example.com',
+ 'username' => 'bob'
+ );
+ Git::credential \%cred;
+ if (try_to_authenticate($cred{'username'}, $cred{'password'})) {
+ Git::credential \%cred, 'approve';
+ ... do more stuff ...
+ } else {
+ Git::credential \%cred, 'reject';
+ }
+
+In the second form, C<CODE> needs to be a reference to a subroutine.
+The function will execute C<git credential fill> to fill provided
+credential hash, than call C<CODE> with C<CREDENTIAL> as the sole
+argument, and finally depending on C<CODE>'s return value execute
+C<git credential approve> (if return value yields true) or C<git
+credential reject> (otherwise). The return value is the same as what
+C<CODE> returned. With this form, the usage might look as follows:
+
+ if (Git::credential {
+ 'protocol' => 'https',
+ 'host' => 'example.com',
+ 'username' => 'bob'
+ }, sub {
+ my $cred = shift;
+ return try_to_authenticate($cred->{'username'}, $cred->{'password'});
+ }) {
+ ... do more stuff ...
+ }
+
+=cut
+
+sub credential {
+ my ($self, $credential, $op_or_code) = (_maybe_self(@_), 'fill');
+
+ if ('CODE' eq ref $op_or_code) {
+ _credential_run $credential, 'fill';
+ my $ret = $op_or_code->($credential);
+ _credential_run $credential, $ret ? 'approve' : 'reject';
+ return $ret;
+ } else {
+ _credential_run $credential, $op_or_code;
+ }
+}
+
{ # %TEMP_* Lexical Context
my (%TEMP_FILEMAP, %TEMP_FILES);
--
1.8.1.2.549.g4fa355e
^ permalink raw reply related
* [PATCH 2/4] Git.pm: Allow pipes to be closed prior to calling command_close_bidi_pipe
From: Michal Nazarewicz @ 2013-02-06 20:47 UTC (permalink / raw)
To: Junio C Hamano, Ted Zlatanov, Jeff King, Matthieu Moy; +Cc: git
In-Reply-To: <cover.1360183427.git.mina86@mina86.com>
From: Michal Nazarewicz <mina86@mina86.com>
The command_close_bidi_pipe() function will insist on closing both
input and output pipes returned by command_bidi_pipe(). With this
change it is possible to close one of the pipes in advance and
pass undef as an argument.
This allows for something like:
my ($pid, $in, $out, $ctx) = command_bidi_pipe(...);
print $out "write data";
close $out;
# ... do stuff with $in
command_close_bidi_pipe($pid, $in, undef, $ctx);
Signed-off-by: Michal Nazarewicz <mina86@mina86.com>
---
perl/Git.pm | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/perl/Git.pm b/perl/Git.pm
index bbb753a..6a2d52d 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -432,7 +432,7 @@ sub command_close_bidi_pipe {
local $?;
my ($self, $pid, $in, $out, $ctx) = _maybe_self(@_);
foreach my $fh ($in, $out) {
- unless (close $fh) {
+ if (defined $fh && !close $fh) {
if ($!) {
carp "error closing pipe: $!";
} elsif ($? >> 8) {
--
1.8.1.2.549.g4fa355e
^ permalink raw reply related
* [PATCH 1/4] Git.pm: Allow command_close_bidi_pipe() to be called as method
From: Michal Nazarewicz @ 2013-02-06 20:47 UTC (permalink / raw)
To: Junio C Hamano, Ted Zlatanov, Jeff King, Matthieu Moy; +Cc: git
In-Reply-To: <cover.1360183427.git.mina86@mina86.com>
From: Michal Nazarewicz <mina86@mina86.com>
The documentation of command_close_bidi_pipe() claims that it can
be called as a method, but it does not check whether the first
argument is $self or not assuming the latter. Using _maybe_self()
fixes this.
Signed-off-by: Michal Nazarewicz <mina86@mina86.com>
---
perl/Git.pm | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/perl/Git.pm b/perl/Git.pm
index 931047c..bbb753a 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -430,7 +430,7 @@ have more complicated structure.
sub command_close_bidi_pipe {
local $?;
- my ($pid, $in, $out, $ctx) = @_;
+ my ($self, $pid, $in, $out, $ctx) = _maybe_self(@_);
foreach my $fh ($in, $out) {
unless (close $fh) {
if ($!) {
--
1.8.1.2.549.g4fa355e
^ permalink raw reply related
* [PATCH 0/4] Make git-send-email git-credential
From: Michal Nazarewicz @ 2013-02-06 20:47 UTC (permalink / raw)
To: Junio C Hamano, Ted Zlatanov, Jeff King, Matthieu Moy; +Cc: git
From: Michal Nazarewicz <mina86@mina86.com>
As discussed on the list, adding git-credential interface to Git.pm
(sort of copied from git-remote-mediawiki) and making git-send-email
use it.
I see git-remote-mediawiki does not have “use Git” so I did not touch
it. On top of that I'd have no way to tests the changes anyway.
Michal Nazarewicz (4):
Git.pm: Allow command_close_bidi_pipe() to be called as method
Git.pm: Allow pipes to be closed prior to calling
command_close_bidi_pipe
Git.pm: Add interface for git credential command.
git-send-email: Use git credential to obtain password.
Documentation/git-send-email.txt | 4 +-
git-send-email.perl | 59 +++++++++++---------
perl/Git.pm | 116 ++++++++++++++++++++++++++++++++++++++-
3 files changed, 149 insertions(+), 30 deletions(-)
--
1.8.1.2.549.g4fa355e
^ permalink raw reply
* Re: Why is ident_is_sufficient different on Windows?
From: Junio C Hamano @ 2013-02-06 20:41 UTC (permalink / raw)
To: Max Horn; +Cc: git
In-Reply-To: <991CBC1C-912C-4DD6-B911-93F6B41D895E@quendi.de>
Max Horn <max@quendi.de> writes:
> static int ident_is_sufficient(int user_ident_explicitly_given)
> {
> #ifndef WINDOWS
> return (user_ident_explicitly_given & IDENT_MAIL_GIVEN);
> #else
> return (user_ident_explicitly_given == IDENT_ALL_GIVEN);
> #endif
> }
>
>
> According to git blame, this was introduced here:
>
> commit 5aeb3a3a838b2cb03d250f3376cf9c41f4d4608e
> Author: Junio C Hamano <gitster@pobox.com>
> Date: Sun Jan 17 13:54:28 2010 -0800
>
> user_ident_sufficiently_given(): refactor the logic to be usable from elsewhere
>
>
> The commit message sounds as if this was only a refactoring, but
> the patch to me look as if it changes behaviour, too. Of course
> this could very well be false, say due to code elsewhere that
> already caused Windows to behave differently; I wouldn't know.
>
> Still, I wonder: Why does this difference exist?
Sorry but I do not recall why these ifdefs are there. The commit
did this to builtin-commit.c:
- if (user_ident_explicitly_given != IDENT_ALL_GIVEN)
+ if (!user_ident_sufficiently_given())
I would have written the function to always check with ALL_GIVEN
myself, and it is very likely that I was *not* the person who
noticed that the function needs to behave differently on Windows, as
I do not do Windows.
I suspect somebody from the Windows camp saw a patch I posted
without the ifdef, noticed that there is a problem to expect
IDENT_NAME_GIVEN to be set on Windows for some reason, and resulted
in a reroll of the function in that shape.
I didn't find anything in the list archive, though. So I am
stumped.
^ permalink raw reply
* Re: [PATCH 0/4] Make git-send-email git-credential
From: Michal Nazarewicz @ 2013-02-06 20:38 UTC (permalink / raw)
To: Junio C Hamano, Ted Zlatanov, Jeff King, Matthieu Moy; +Cc: git
In-Reply-To: <cover.1360172967.git.mina86@mina86.com>
[-- Attachment #1: Type: text/plain, Size: 1289 bytes --]
On Wed, Feb 06 2013, Michal Nazarewicz wrote:
> As discussed on the list, adding git-credential interface to Git.pm
> (sort of copied from git-remote-mediawiki) and making git-send-email
> use it.
>
> I see git-remote-mediawiki does not have “use Git” so I did not touch
> it. On top of that I'd have no way to tests the changes anyway.
>
> Michal Nazarewicz (4):
> Git.pm: Allow command_close_bidi_pipe() to be called as method
> Git.pm: Allow pipes to be closed prior to calling
> command_close_bidi_pipe
> Git.pm: Add interface for git credential command.
> git-send-email: Use git credential to obtain password.
>
> Documentation/git-send-email.txt | 4 +-
> git-send-email.perl | 60 +++++++++++---------
> perl/Git.pm | 116 ++++++++++++++++++++++++++++++++++++++-
> 3 files changed, 149 insertions(+), 31 deletions(-)
On second thought, give me a moment, ;) I've just discovered a bug
preventing git-send-email from mailing a patchset.
--
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o
..o | Computer Science, Michał “mina86” Nazarewicz (o o)
ooo +----<email/xmpp: mpn@google.com>--------------ooO--(_)--Ooo--
[-- Attachment #2.1: Type: text/plain, Size: 0 bytes --]
[-- Attachment #2.2: Type: application/pgp-signature, Size: 835 bytes --]
^ permalink raw reply
* [PATCH 0/4] Make git-send-email git-credential
From: Michal Nazarewicz @ 2013-02-06 20:34 UTC (permalink / raw)
To: Junio C Hamano, Ted Zlatanov, Jeff King, Matthieu Moy; +Cc: git
From: Michal Nazarewicz <mina86@mina86.com>
As discussed on the list, adding git-credential interface to Git.pm
(sort of copied from git-remote-mediawiki) and making git-send-email
use it.
I see git-remote-mediawiki does not have “use Git” so I did not touch
it. On top of that I'd have no way to tests the changes anyway.
Michal Nazarewicz (4):
Git.pm: Allow command_close_bidi_pipe() to be called as method
Git.pm: Allow pipes to be closed prior to calling
command_close_bidi_pipe
Git.pm: Add interface for git credential command.
git-send-email: Use git credential to obtain password.
Documentation/git-send-email.txt | 4 +-
git-send-email.perl | 60 +++++++++++---------
perl/Git.pm | 116 ++++++++++++++++++++++++++++++++++++++-
3 files changed, 149 insertions(+), 31 deletions(-)
--
1.8.1.2.550.g0d3a9c0.dirty
^ permalink raw reply
* Re: [RFC] test-lib.sh: No POSIXPERM for cygwin
From: Torsten Bögershausen @ 2013-02-06 20:16 UTC (permalink / raw)
To: kusmabite; +Cc: Torsten Bögershausen, ramsay, git, j6t
In-Reply-To: <CABPQNSYmGhaDG0Dzpp2C0ZwKoFwP5EokJW+TC8JT-Bw6rKa79A@mail.gmail.com>
Am 2013-02-06 10:34, schrieb Erik Faye-Lund:
> On Sun, Jan 27, 2013 at 3:57 PM, Torsten Bögershausen <tboegi@web.de> wrote:
>> t0070 and t1301 fail when running the test suite under cygwin.
>> Skip the failing tests by unsetting POSIXPERM.
>>
>
> But is this the real reason? I thought Cygwin implemented POSIX permissions...?
t0070:
'mktemp to unwritable directory prints filename'
mkdir cannotwrite &&
chmod -w cannotwrite &&
test_when_finished "chmod +w cannotwrite" &&
test_must_fail test-mktemp cannotwrite/testXXXXXX 2>err &&
grep "cannotwrite/test" err
When a directory under Linux/*nix has no write permission,
it is not allowed to create another directory (or file..) here.
This is not working under cygwin, a directory/file can be created
even if the parent directory has chmod 0.
-------------
tb@PC /cygdrive/c/temp
$ mkdir ttt
tb@PC /cygdrive/c/temp
$ chmod 0 ttt
tb@PC /cygdrive/c/temp
$ ls -ld ttt
d---------+ 1 tb None 0 Feb 6 20:33 ttt
tb@PC /cygdrive/c/temp
$ touch ttt/x
tb@PC /cygdrive/c/temp
$ ls -ld ttt
d---------+ 1 tb None 0 Feb 6 20:33 ttt
tb@PC /cygdrive/c/temp
$ ls -l ttt
total 0
-rw-r--r--+ 1 tb None 0 Feb 6 20:33 x
-------------------------------------------
If this is POSIX compliant? I'm not an expert here.
On the other hand:
This test case does not test git, but rather the file system,
so we can probaly remove it?
About 1301:
Some resereach needs to be done, to find out the connection between
umask, cygwin and the mount options.
On my system I have:
$mount
C: on /cygdrive/c type ntfs (binary,posix=0,user,noumount,auto)
/Torsten
^ permalink raw reply
* [PATCH] graph: output padding for merge subsequent parents
From: John Keeping @ 2013-02-06 19:57 UTC (permalink / raw)
To: Matthieu Moy; +Cc: Dale R. Worley, gitster, git
In-Reply-To: <vpqbobxwavv.fsf@grenoble-inp.fr>
On Wed, Feb 06, 2013 at 07:33:08PM +0100, Matthieu Moy wrote:
> John Keeping <john@keeping.me.uk> writes:
>
> > I would argue that the line should start with "| | ", since it really is
> > just a continuation of the same commit.
> >
> > | |
> > | | commit a393ed598e9fb11436f85bd58f1a38c82f2cadb7 (from 33e70e70c0173d634826b998bdc304f93c0966b8)
> > | | Merge: 2c1e6a3 33e70e7
> > | | Author: Matthieu Moy <Matthieu.Moy@imag.fr>
> > | | Date: Tue Feb 5 22:05:33 2013 +0100
>
> Yes.
>
> I had a look at the code, I guess the call to graph_show_commit() in
> show_log() (in log-tree.c) should have called graph_show_padding() but
> didn't in this case. Then I got lost in graph.c :-(.
I think this is the correct answer. But now I've found that "git log
--graph -c -p" doesn't indent the diff - that seems to be a separate
issue.
-- >8 --
When showing merges in git-log, the same commit is shown once for each
parent. Combined with "--graph" this results in graph_show_commit()
being called once for each parent without graph_update() being called.
Currently graph_show_commit() does not print anything on subsequent
invocations for the same commit (this was changed by commit 656197a -
"graph.c: infinite loop in git whatchanged --graph -m" from the previous
behaviour of looping infinitely).
Change this so that if the graph code believes it has already shown the
commit it prints a single padding line.
Signed-off-by: John Keeping <john@keeping.me.uk>
---
graph.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/graph.c b/graph.c
index 391a712..2a3fc5c 100644
--- a/graph.c
+++ b/graph.c
@@ -1227,6 +1227,16 @@ void graph_show_commit(struct git_graph *graph)
if (!graph)
return;
+ /*
+ * When showing a diff of a merge against each of its parents, we
+ * are called once for each parent without graph_update having been
+ * called. In this case, simply output a single padding line.
+ */
+ if (graph_is_commit_finished(graph)) {
+ graph_show_padding(graph);
+ shown_commit_line = 1;
+ }
+
while (!shown_commit_line && !graph_is_commit_finished(graph)) {
shown_commit_line = graph_next_line(graph, &msgbuf);
fwrite(msgbuf.buf, sizeof(char), msgbuf.len, stdout);
--
1.8.1.2
^ permalink raw reply related
* Re: [PATCH v3 0/8] Hiding refs
From: Jonathan Nieder @ 2013-02-06 19:55 UTC (permalink / raw)
To: Michael Haggerty; +Cc: Junio C Hamano, git, Jeff King, Shawn Pearce
In-Reply-To: <51122D9D.9040100@alum.mit.edu>
Michael Haggerty wrote:
> Scenario 1: Some providers junk up their users' repositories with
> content that is not created by the repository's owner and that the owner
> doesn't want to appear to vouch for (e.g., GitHub pull requests). These
> references might sometimes be useful to fetch, singly or in bulk.
>
> Scenario 2: Some systems junk up their users' repositories with
> additional references that are not interesting to most pullers (e.g.,
> Gerrit activity markers) though they don't add questionable content.
Actually Gerrit's refs/changes refs are pretty similar to Github's
refs/pull. Both are requests for code review.
[...]
> But now every time I do a "gitk --all" or "git log --decorate", the
> output is cluttered with all of his references (most of which are just
> old versions of references from the upstream repository that we both
> use). I would like to be able to hide his references most of the time
> but turn them back on when I need them.
>
> Scenario 5: Our upstream repository has gazillions of release tags under
> "refs/tags/releases/...", sometimes including customer-specific
> releases. In my daily life these are just clutter.
For both of these use cases, putting the refs somewhere other than
refs/heads, refs/tags, and refs/remotes should be enough to avoid
clutter.
I agree that a --decorate-glob along the lines of "git rev-parse"'s
--glob would be nice.
[...]
> * Some small improvements (e.g. allowing *multiple* views to be
> defined) would provide much more benefit for about the same effort,
> and would be a better base for building other features in the future
> (e.g., local views).
Would advertising GIT_CONFIG_PARAMETERS and giving examples for server
admins to set it in inetd et al to provide different kinds of access
to a same repository through different URLs work?
> Thanks for listening.
> Michael
>
> [1] Theoretically one could support multiple views of a single
> repository by using something like "GIT_CONFIG=view_1_config git
> upload-pack ..." or "git -c transfer.hiderefs=... git upload-pack ...",
> but this would be awkward.
Ah, I missed this comment before. What's awkward about that? I
think it's a clean way to make many aspects of how a repository is
presented (including hook actions) configurable.
Thanks for your help clarifying this feature. Hopefully some of the
discussion will filter into the documentation.
Jonathan
^ permalink raw reply
* [PATCH v2] Update CodingGuidelines for Perl 5
From: Ted Zlatanov @ 2013-02-06 19:49 UTC (permalink / raw)
To: Junio C Hamano; +Cc: demerphq, git, Jeff King
In-Reply-To: <7vwqulb5el.fsf@alter.siamese.dyndns.org>
Update the coding guidelines for Perl 5.
Signed-off-by: Ted Zlatanov <tzz@lifelogs.com>
---
Changes since PATCHv1:
- removed brace guidelines
- add "don't try to be clever" at beginning
Documentation/CodingGuidelines | 42 ++++++++++++++++++++++++++++++++++++++++
1 files changed, 42 insertions(+), 0 deletions(-)
diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
index 1d7de5f..166c141 100644
--- a/Documentation/CodingGuidelines
+++ b/Documentation/CodingGuidelines
@@ -18,6 +18,8 @@ code. For Git in general, three rough rules are:
judgement call, the decision based more on real world
constraints people face than what the paper standard says.
+For any programming language below, make your code readable and sensible, and
+don't try to be clever.
As for more concrete guidelines, just imitate the existing code
(this is a good guideline, no matter which project you are
@@ -179,6 +181,46 @@ For C programs:
- Use Git's gettext wrappers to make the user interface
translatable. See "Marking strings for translation" in po/README.
+For Perl 5 programs:
+
+ - Most of the C guidelines above apply.
+
+ - We try to support Perl 5.8 and later ("use Perl 5.008").
+
+ - use strict and use warnings are strongly preferred.
+
+ - Don't abuse statement modifiers--they are discouraged. But in general:
+
+ ... do something ...
+ do_this() unless (condition);
+ ... do something else ...
+
+ should be used instead of
+
+ ... do something ...
+ unless (condition) {
+ do_this();
+ }
+ ... do something else ...
+
+ *only* when when the condition is so rare that do_this() will be called
+ almost always.
+
+ - We try to avoid assignments inside if().
+
+ - Learn and use Git.pm if you need that functionality.
+
+ - For Emacs, it's useful to put the following in
+ GIT_CHECKOUT/.dir-locals.el, assuming you use cperl-mode:
+
+ ;; note the first part is useful for C editing, too
+ ((nil . ((indent-tabs-mode . t)
+ (tab-width . 8)
+ (fill-column . 80)))
+ (cperl-mode . ((cperl-indent-level . 8)
+ (cperl-extra-newline-before-brace . nil)
+ (cperl-merge-trailing-else . t))))
+
Writing Documentation:
Every user-visible change should be reflected in the documentation.
--
1.7.9.rc2
^ permalink raw reply related
* Re: [PATCH v3 0/8] Hiding refs
From: Jonathan Nieder @ 2013-02-06 19:45 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Duy Nguyen, Michael Haggerty, git, Jeff King, Shawn Pearce
In-Reply-To: <7v4nhpckwd.fsf@alter.siamese.dyndns.org>
Junio C Hamano wrote:
> Duy Nguyen <pclouds@gmail.com> writes:
>> On Tue, Feb 5, 2013 at 5:29 PM, Michael Haggerty <mhagger@alum.mit.edu> wrote:
>>> Hiderefs creates a "dark" corner of a remote git repo
[...]
>> Or you can think hiderefs is the first step to addressing the
>> initial ref advertisment problem. The series says hidden refs are
>> to be fetched out of band, but that's not the only way.
>
> Let me help unconfuse this thread.
>
> I think the series as 8-patch series was poorly presented, and
> separating it into two will help understanding what they are about.
>
> The first three:
>
> upload-pack: share more code
> upload-pack: simplify request validation
> upload/receive-pack: allow hiding ref hierarchies
>
> is _the_ topic of the series. As far as I am concerned (I am not
> speaking for Gerrit users, but am speaking as the Git maintainer),
> the topic is solely about uncluttering. There may be refs that the
> server end may need to keep for its operation, but that remote users
> have _no_ business knowing about.
An obvious question when looking at that alone is, is there ever
actually need for such private refs? If the refs are not meant to be
shared with users *at all*, why are they even refs?
An answer is "because refs force gc to keep the corresponding
objects". For example, the sysadmin may want to keep refs/archived/
refs for dead branches that should not be advertised or accessible to
the user any more. Seems sane, though not especially exciting.
What is more exciting to me is that it is a first step toward
addressing the complicated problem of offering access to more refs
than can be efficiently presented in the current ref advertisement. I
think that's a harder problem but something like this would be needed
in order to support existing clients without performance degredation.
And in the meantime, it helps with the refs/archived case.
Thanks for explaining.
Jonathan
^ permalink raw reply
* Re: What's cooking in git.git (Feb 2013, #03; Wed, 6)
From: Junio C Hamano @ 2013-02-06 19:39 UTC (permalink / raw)
To: Jens Lehmann; +Cc: git
In-Reply-To: <5112AE34.6080107@web.de>
Jens Lehmann <Jens.Lehmann@web.de> writes:
> Am 06.02.2013 19:29, schrieb Junio C Hamano:
>> * jl/submodule-deinit (2013-02-04) 1 commit
>> - submodule: add 'deinit' command
>>
>> There was no Porcelain way to say "I no longer am interested in
>> this submodule", once you express your interest in a submodule with
>> "submodule init". "submodule deinit" is the way to do so.
>>
>> Will merge to 'next'.
>
> Oops, I though you were waiting for a reroll. Currently I'm having the
> appended interdiff compared to your version. Changes are:
>
> - Add deinit to the --force documentation of "git submodule"
> - Never remove submodules containing a .git dir, even when forced
> - diagnostic output when "rm -rf" or "mkdir" fails
> - More test cases
>
> And I wanted to add three more test cases for modified submodules before
> sending v4. You could squash in the first two hunks into the commit you
> have in pu and I'll send a follow up patch with the extra tests soon or
> you could wait for me sending an updated patch. What do you think?
I haven't merged it down to 'next' yet. So please proceed as you
planned. Thanks for stopping me.
^ permalink raw reply
* Re: CodingGuidelines Perl amendment
From: Junio C Hamano @ 2013-02-06 19:37 UTC (permalink / raw)
To: Ted Zlatanov; +Cc: demerphq, git, Jeff King
In-Reply-To: <87bobxgtmw.fsf@lifelogs.com>
Ted Zlatanov <tzz@lifelogs.com> writes:
> "Make your code readable and sensible, and don't try to be clever."
>
> But this is good C and shell advice too,...
Sounds sensible.
^ permalink raw reply
* Re: What's cooking in git.git (Feb 2013, #03; Wed, 6)
From: Jens Lehmann @ 2013-02-06 19:25 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7v8v71cn3m.fsf@alter.siamese.dyndns.org>
Am 06.02.2013 19:29, schrieb Junio C Hamano:
> * jl/submodule-deinit (2013-02-04) 1 commit
> - submodule: add 'deinit' command
>
> There was no Porcelain way to say "I no longer am interested in
> this submodule", once you express your interest in a submodule with
> "submodule init". "submodule deinit" is the way to do so.
>
> Will merge to 'next'.
Oops, I though you were waiting for a reroll. Currently I'm having the
appended interdiff compared to your version. Changes are:
- Add deinit to the --force documentation of "git submodule"
- Never remove submodules containing a .git dir, even when forced
- diagnostic output when "rm -rf" or "mkdir" fails
- More test cases
And I wanted to add three more test cases for modified submodules before
sending v4. You could squash in the first two hunks into the commit you
have in pu and I'll send a follow up patch with the extra tests soon or
you could wait for me sending an updated patch. What do you think?
---------------8<------------------
diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index 7a149eb..45ee12b 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -227,8 +227,10 @@ OPTIONS
-f::
--force::
- This option is only valid for add and update commands.
+ This option is only valid for add, deinit and update commands.
When running add, allow adding an otherwise ignored submodule path.
+ When running deinit the submodule work trees will be removed even if
+ they contain local changes.
When running update, throw away local changes in submodules when
switching to a different commit; and always run a checkout operation
in the submodule, even if the commit listed in the index of the
diff --git a/git-submodule.sh b/git-submodule.sh
index f05b597..365c6de 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -595,14 +595,25 @@ cmd_deinit()
continue
fi
- # Remove the submodule work tree
- if test -z "$force"
+ # Remove the submodule work tree (unless the user already did it)
+ if test -d "$sm_path"
then
- git rm -n "$sm_path" ||
- die "$(eval_gettext "Submodule work tree $sm_path contains local modifications, use '-f' to discard them")"
+ # Protect submodules containing a .git directory
+ if test -d "$sm_path/.git"
+ then
+ echo >&2 "$(eval_gettext "Submodule work tree $sm_path contains a .git directory")"
+ die "$(eval_gettext "(use 'rm -rf' if you really want to remove it including all of its history)")"
+ fi
+
+ if test -z "$force"
+ then
+ git rm -n "$sm_path" ||
+ die "$(eval_gettext "Submodule work tree $sm_path contains local modifications, use '-f' to discard them")"
+ fi
+ rm -rf "$sm_path" || say "$(eval_gettext "Could not remove submodule work tree '\$sm_path'")"
fi
- rm -rf "$sm_path"
- mkdir "$sm_path"
+
+ mkdir "$sm_path" || say "$(eval_gettext "Could not create empty submodule directory '\$sm_path'")"
# Remove the whole section so we have a clean state when the
# user later decides to init this submodule again
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index 34d8274..0567f1a 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -757,20 +757,46 @@ test_expect_success 'submodule add with an existing name fails unless forced' '
)
'
+test_expect_success 'set up a second submodule' '
+ git submodule add ./init2 example2 &&
+ git commit -m "submodle example2 added"
+'
+
test_expect_success 'submodule deinit should remove the whole submodule section from .git/config' '
git config submodule.example.foo bar &&
+ git config submodule.example2.frotz nitfol &&
git submodule deinit init &&
test -z "$(git config submodule.example.url)" &&
- test -z "$(git config submodule.example.foo)"
+ test -z "$(git config submodule.example.foo)" &&
+ test -n "$(git config submodule.example2.url)" &&
+ test -n "$(git config submodule.example2.frotz)" &&
+ rmdir init
'
test_expect_success 'submodule deinit . deinits all initialized submodules' '
git submodule update --init &&
git config submodule.example.foo bar &&
+ git config submodule.example2.frotz nitfol &&
test_must_fail git submodule deinit &&
git submodule deinit . &&
test -z "$(git config submodule.example.url)" &&
- test -z "$(git config submodule.example.foo)"
+ test -z "$(git config submodule.example.foo)" &&
+ test -z "$(git config submodule.example2.url)" &&
+ test -z "$(git config submodule.example2.frotz)" &&
+ rmdir init example2
+'
+
+test_expect_success 'submodule deinit deinits a submodule when its work tree is missing or empty' '
+ git submodule update --init &&
+ rm -rf init example2/* example2/.git &&
+ git config submodule.example.foo bar &&
+ git config submodule.example2.frotz nitfol &&
+ git submodule deinit init example2 &&
+ test -z "$(git config submodule.example.url)" &&
+ test -z "$(git config submodule.example.foo)" &&
+ test -z "$(git config submodule.example2.url)" &&
+ test -z "$(git config submodule.example2.frotz)" &&
+ rmdir init
'
test_expect_success 'submodule deinit complains when explicitly used on an uninitialized submodule' '
@@ -778,7 +804,24 @@ test_expect_success 'submodule deinit complains when explicitly used on an unini
git submodule deinit init >actual &&
test_i18ngrep "Submodule .example. (.*) unregistered for path .init" actual
git submodule deinit init >actual &&
- test_i18ngrep "No url found for submodule path .init. in .git/config" actual
+ test_i18ngrep "No url found for submodule path .init. in .git/config" actual &&
+ git submodule deinit . >actual &&
+ test_i18ngrep "Submodule .example2. (.*) unregistered for path .example2" actual
+ rmdir init example2
+'
+
+test_expect_success 'submodule deinit fails when submodule has a .git directory even when forced' '
+ git submodule update --init &&
+ (
+ cd init &&
+ rm .git &&
+ cp -R ../.git/modules/example .git &&
+ GIT_WORK_TREE=. git config --unset core.worktree
+ ) &&
+ test_must_fail git submodule deinit init &&
+ test_must_fail git submodule deinit -f init &&
+ test -d init/.git &&
+ test -n "$(git config submodule.example.url)"
'
test_done
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox