* Re: [PATCH] diff and apply: fix singular/plural grammar nit.
From: Jakub Narebski @ 2011-11-27 14:47 UTC (permalink / raw)
To: David Ripton; +Cc: git
In-Reply-To: <4ED23EB5.1030208@ripton.net>
David Ripton <dripton@ripton.net> writes:
> Remove the trailing 's' from "files", "insertions", and "deletions"
> when there is only one of the item.
>
> Signed-off-by: David Ripton <dripton@ripton.net>
> ---
[...]
> - printf(" %d files changed, %d insertions(+), %d
> deletions(-)\n", files, adds, dels);
Whitespace damaged. Please turn off word wrapping (limiting line
width) when sending patches.
> + printf(" %d file%s changed, %d insertion%s(+), %d deletion%s(-)\n",
> + files, (files == 1 ? "" : "s"),
> + adds, (adds == 1 ? "" : "s"),
> + dels, (dels == 1 ? "" : "s"));
> }
First, I think this is an API / plumbing and should not be changed.
But I might be mistaken about that.
Second, it is a perfect example ho to *not* handle plural form in the
presence of internationalization (i18n) efforts. See e.g.
http://www.gnu.org/s/hello/manual/gettext/Plural-forms.html
--
Jakub Narębski
^ permalink raw reply
* [PATCH] diff and apply: fix singular/plural grammar nit.
From: David Ripton @ 2011-11-27 13:44 UTC (permalink / raw)
To: git, gitster
Remove the trailing 's' from "files", "insertions", and "deletions"
when there is only one of the item.
Signed-off-by: David Ripton <dripton@ripton.net>
---
builtin/apply.c | 5 ++++-
diff.c | 13 +++++++++----
2 files changed, 13 insertions(+), 5 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index 84a8a0b..47bbc23 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -3244,7 +3244,10 @@ static void stat_patch_list(struct patch *patch)
show_stats(patch);
}
- printf(" %d files changed, %d insertions(+), %d deletions(-)\n",
files, adds, dels);
+ printf(" %d file%s changed, %d insertion%s(+), %d deletion%s(-)\n",
+ files, (files == 1 ? "" : "s"),
+ adds, (adds == 1 ? "" : "s"),
+ dels, (dels == 1 ? "" : "s"));
}
static void numstat_patch_list(struct patch *patch)
diff --git a/diff.c b/diff.c
index 374ecf3..531dcb1 100644
--- a/diff.c
+++ b/diff.c
@@ -1467,8 +1467,10 @@ static void show_stats(struct diffstat_t *data,
struct diff_options *options)
}
fprintf(options->file, "%s", line_prefix);
fprintf(options->file,
- " %d files changed, %d insertions(+), %d deletions(-)\n",
- total_files, adds, dels);
+ " %d file%s changed, %d insertion%s(+), %d deletion%s(-)\n",
+ total_files, (total_files == 1 ? "" : "s"),
+ adds, (adds == 1 ? "" : "s"),
+ dels, (dels == 1 ? "" : "s"));
}
static void show_shortstats(struct diffstat_t *data, struct
diff_options *options)
@@ -1498,8 +1500,11 @@ static void show_shortstats(struct diffstat_t
*data, struct diff_options *option
options->output_prefix_data);
fprintf(options->file, "%s", msg->buf);
}
- fprintf(options->file, " %d files changed, %d insertions(+), %d
deletions(-)\n",
- total_files, adds, dels);
+ fprintf(options->file,
+ " %d file%s changed, %d insertion%s(+), %d deletion%s(-)\n",
+ total_files, (total_files == 1 ? "" : "s"),
+ adds, (adds == 1 ? "" : "s"),
+ dels, (dels == 1 ? "" : "s"));
}
static void show_numstat(struct diffstat_t *data, struct diff_options
*options)
--
1.7.8.rc3
--
David Ripton dripton@ripton.net
^ permalink raw reply related
* Re: what are the chances of a 'pre-upload' hook?
From: Sitaram Chamarty @ 2011-11-27 13:16 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, Git
In-Reply-To: <7v8vn2vt8p.fsf@alter.siamese.dyndns.org>
On Sun, Nov 27, 2011 at 2:26 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Jeff King <peff@peff.net> writes:
>
>> Just because it is passed through the environment does not mean you need
>> to have it set all the time. There is nothing wrong with:
>>
>> GIT_ALLOW_UNTRUSTED_HOOKS=true git fetch ~bob/repo.git
>>
>> We can even spell it:
>>
>> git --allow-untrusted-hooks fetch ~bob/repo.git
>>
>> but it should probably still end up as an environment variable to make
>> it through to the remote side (you could also tack it on to the
>> upload-pack command line; that wouldn't make it across git:// or http://
>> connections, but those are irrelevant here anyway).
>
> I actually like the idea of allowing pre-upload-pack hook on git:// and
> possibly http:// only. git-daemon can tell the upload-pack that it is OK
> to run the hook, and the hook can do the things that only the daemon can
> do, never touching what the original requestor would but the repository
> owner would not have an access to. Normal file:// and ssh:// transfer
> should not be needed for GitHub's stats or Sitaram's proxy usage, and we
> should be able to unconditionally disable the hook when transfer is going
> over these protocols, no?
>
> One scenario I do not want to see is this. Suppose there is a company that
> runs more than one project, and one of the projects is so secret that only
> engineers working on the project have access to its repository, while all
> people, including the project-A engineers, in the company have access to
> other repositories. Further suppose that people involved in some or many
> of the general repositories want to do something before a fetch from them
> happens. They will use the pre-upload-hook to implement it if we make it
> available, and their new-hire training course will tell their engineers to
> set the GIT_ALLOW_UNTRUSTED_HOOKS. Perhaps the /etc/skel/profile the
> company gives the new-hires already defines it.
>
> And somebody who controls one of the general repositories installs a
> pre-upload-hook that borrows credential of a project-A engineer who
> happens to fetch from it to access repositories of the project-A.
>
> Imagine how the blame game that will ensue goes after materials from
> project-A is exposed. Of course, the owner of the rogue hook will get the
> first blame, but I do not think Git will come unscathed out of it. At
> least we will be blamed for adding an inherently unsafe mechanism.
>
I'm sorry I started this discussion. I worked around it, though it's
a bit kludgy, so maybe time to drop the debate.
--
Sitaram
^ permalink raw reply
* Re: Switch from svn to git and modify repo completely
From: Matthias Fechner @ 2011-11-27 10:56 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: git, Alexey Shumkin
In-Reply-To: <20111121070247.GA16708@elie.hsd1.il.comcast.net>
Am 21.11.2011 08:02, schrieb Jonathan Nieder:
> The section "CHECKLIST FOR SHRINKING A REPOSITORY" from the
> git-filter-branch(1) manual page has some hints. In particular, "git
> clone --no-hardlinks" still _copies_ all objects --- you probably
> would want "git clone file://$(pwd)/repo-orig" to make sure the
> ordinary transfer negotiation kicks in.
>
> It's very important that the documentation not be misleading, so if
> you can point to places where the wording can be less confusing, that
> would be very welcome.
it would be fantastic if exactly this information is noted in the
manual. For me it is not really clear what is the difference between a
git clone with and without --no-hardlinks.
I was now able to convert my svn repo to a git repo and split the new
git repo in several different ones, including the complete history.
Here a short summary for all how need to do the same.
At first convert the svn into a git reposity:
git svn clone file:///path/to/svn -A authors -s gitrepo.git
To create the authors file, check the man page.
To get a list of all files:
git show --pretty="format:" --name-only startrev..endrev | sort | uniq
Maybe you want to remove some branches from the repo because svn users
used it in a completely wrong way:
git branch -rd badbranch
Now clone the repo and remove files from it you do not want anymore:
git clone gitrepo.git tofilter.git
cd tofilter.git
git filter-branch -f --tree-filter 'rm -Rf file1 file2.bla dir1 \
projekt1/dir2' --prune-empty -- --all
git gc
cd ..
Repeat this step for all combination you need.
Now create a bare repository and enable some options to use it as a
central repo:
git clone --bare tofilter.git tofilter_bare.git
cd tofilter_bare.git
git config core.sharedRepository 1
git config receive.denyNonFastForwards true
git gc
cd ..
Now you can copy the repo to the target server/directory and clone it
from there.
Bye
Matthias
--
"Programming today is a race between software engineers striving to
build bigger and better idiot-proof programs, and the universe trying to
produce bigger and better idiots. So far, the universe is winning." --
Rich Cook
^ permalink raw reply
* [PATCH 2/2] checkout,merge: disallow overwriting ignored files with --no-overwrite-ignore
From: Nguyễn Thái Ngọc Duy @ 2011-11-27 10:15 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Johannes Sixt, Taylor Hedberg, Bertrand BENOIT,
Nguyễn Thái Ngọc Duy
In-Reply-To: <1322388933-6284-1-git-send-email-pclouds@gmail.com>
Ignored files usually are generated files (e.g. .o files) and can be
safely discarded. However sometimes users may have important files in
working directory, but still want a clean "git status", so they mark
them as ignored files. But in this case, these files should not be
overwritten without asking first.
Enable this use case with --no-overwrite-ignore, where git only sees
tracked and untracked files, no ignored files. Those who mix
discardable ignored files with important ones may have to sort it out
themselves.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
builtin/checkout.c | 11 ++++++++---
builtin/merge.c | 12 ++++++++----
2 files changed, 16 insertions(+), 7 deletions(-)
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 51840b9..5f9474d 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -34,6 +34,7 @@ struct checkout_opts {
int force_detach;
int writeout_stage;
int writeout_error;
+ int overwrite_ignore;
/* not set by parse_options */
int branch_exists;
@@ -409,9 +410,11 @@ static int merge_working_tree(struct checkout_opts *opts,
topts.gently = opts->merge && old->commit;
topts.verbose_update = !opts->quiet;
topts.fn = twoway_merge;
- topts.dir = xcalloc(1, sizeof(*topts.dir));
- topts.dir->flags |= DIR_SHOW_IGNORED;
- setup_standard_excludes(topts.dir);
+ if (opts->overwrite_ignore) {
+ topts.dir = xcalloc(1, sizeof(*topts.dir));
+ topts.dir->flags |= DIR_SHOW_IGNORED;
+ setup_standard_excludes(topts.dir);
+ }
tree = parse_tree_indirect(old->commit ?
old->commit->object.sha1 :
EMPTY_TREE_SHA1_BIN);
@@ -926,6 +929,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
3),
OPT__FORCE(&opts.force, "force checkout (throw away local modifications)"),
OPT_BOOLEAN('m', "merge", &opts.merge, "perform a 3-way merge with the new branch"),
+ OPT_BOOLEAN(0, "overwrite-ignore", &opts.overwrite_ignore, "update ignored files (default)"),
OPT_STRING(0, "conflict", &conflict_style, "style",
"conflict style (merge or diff3)"),
OPT_BOOLEAN('p', "patch", &patch_mode, "select hunks interactively"),
@@ -937,6 +941,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
memset(&opts, 0, sizeof(opts));
memset(&new, 0, sizeof(new));
+ opts.overwrite_ignore = 1;
gitmodules_config();
git_config(git_checkout_config, &opts);
diff --git a/builtin/merge.c b/builtin/merge.c
index 1387376..07102c4 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -48,6 +48,7 @@ static int show_diffstat = 1, shortlog_len, squash;
static int option_commit = 1, allow_fast_forward = 1;
static int fast_forward_only, option_edit;
static int allow_trivial = 1, have_message;
+static int overwrite_ignore = 1;
static struct strbuf merge_msg;
static struct commit_list *remoteheads;
static struct strategy **use_strategies;
@@ -207,6 +208,7 @@ static struct option builtin_merge_options[] = {
OPT_BOOLEAN(0, "abort", &abort_current_merge,
"abort the current in-progress merge"),
OPT_SET_INT(0, "progress", &show_progress, "force progress reporting", 1),
+ OPT_BOOLEAN(0, "overwrite-ignore", &overwrite_ignore, "update ignored files (default)"),
OPT_END()
};
@@ -773,10 +775,12 @@ int checkout_fast_forward(const unsigned char *head, const unsigned char *remote
memset(&trees, 0, sizeof(trees));
memset(&opts, 0, sizeof(opts));
memset(&t, 0, sizeof(t));
- memset(&dir, 0, sizeof(dir));
- dir.flags |= DIR_SHOW_IGNORED;
- setup_standard_excludes(&dir);
- opts.dir = &dir;
+ if (overwrite_ignore) {
+ memset(&dir, 0, sizeof(dir));
+ dir.flags |= DIR_SHOW_IGNORED;
+ setup_standard_excludes(&dir);
+ opts.dir = &dir;
+ }
opts.head_idx = 1;
opts.src_index = &the_index;
--
1.7.4.74.g639db
^ permalink raw reply related
* [PATCH 1/2] checkout,merge: loosen overwriting untracked file check based on info/exclude
From: Nguyễn Thái Ngọc Duy @ 2011-11-27 10:15 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Johannes Sixt, Taylor Hedberg, Bertrand BENOIT,
Nguyễn Thái Ngọc Duy
In-Reply-To: <CACsJy8AXLBNSPmeEJjD1QX2zF1ic+S9kb_+4=EBO9xd07xhAYQ@mail.gmail.com>
Back in 1127148 (Loosen "working file will be lost" check in
Porcelain-ish - 2006-12-04), git-checkout.sh learned to quietly
overwrite ignored files. Howver the code only took .gitignore files
into account.
Standard ignored files include all specified in .gitignore files in
working directory _and_ $GIT_DIR/info/exclude. This patch makes sure
ignored files in info/exclude can also be overwritten automatically in
the spirit of the original patch.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
builtin/checkout.c | 2 +-
builtin/merge.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 2a80772..51840b9 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -411,7 +411,7 @@ static int merge_working_tree(struct checkout_opts *opts,
topts.fn = twoway_merge;
topts.dir = xcalloc(1, sizeof(*topts.dir));
topts.dir->flags |= DIR_SHOW_IGNORED;
- topts.dir->exclude_per_dir = ".gitignore";
+ setup_standard_excludes(topts.dir);
tree = parse_tree_indirect(old->commit ?
old->commit->object.sha1 :
EMPTY_TREE_SHA1_BIN);
diff --git a/builtin/merge.c b/builtin/merge.c
index 2870a6a..1387376 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -775,7 +775,7 @@ int checkout_fast_forward(const unsigned char *head, const unsigned char *remote
memset(&t, 0, sizeof(t));
memset(&dir, 0, sizeof(dir));
dir.flags |= DIR_SHOW_IGNORED;
- dir.exclude_per_dir = ".gitignore";
+ setup_standard_excludes(&dir);
opts.dir = &dir;
opts.head_idx = 1;
--
1.7.4.74.g639db
^ permalink raw reply related
* Re: [PATCH] Add test that checkout does not overwrite entries in .git/info/exclude
From: Nguyen Thai Ngoc Duy @ 2011-11-27 10:13 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Sixt, Taylor Hedberg, Bertrand BENOIT, git
In-Reply-To: <7vk46th5bz.fsf@alter.siamese.dyndns.org>
On Mon, Nov 21, 2011 at 10:18 PM, Junio C Hamano <gitster@pobox.com> wrote:
> In the medium term, I think one reasonable way forward solving the "TODO
> that used to be tracked but now untracked and ignored" issue is to
> introduce "info/exclude-override" that comes between command line and
> in-tree patterns. The info/exclude file is designed as the fallback
> definition to be used when all other sources are too lax, and comes near
> the precedence stack; the "TODO" situation however calls for an override
> that is stronger than the in-tree patterns.
Short term, should we allow an option to disregard ignored status
(i.e. all files are precious)? Something like [2/2]
[1/2] checkout,merge: loosen overwriting untracked file check based on
info/exclude
[2/2] checkout,merge: disallow overwriting ignored files with
--no-overwrite-ignore
--
Duy
^ permalink raw reply
* Re: [PATCH 0/6] echo usernames as they are typed
From: Erik Faye-Lund @ 2011-11-27 9:17 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20111127082744.GA32068@sigill.intra.peff.net>
On Sun, Nov 27, 2011 at 9:27 AM, Jeff King <peff@peff.net> wrote:
> On Thu, Nov 24, 2011 at 05:58:01AM -0500, Jeff King wrote:
>
>> Here's a revised version of the http-auth / credential-helper series.
>
> And here's something I've been meaning to do on top: actually echo
> characters at the username prompt. We can't do this portably, but we can
> at least stub out a compatibility layer and let each system do something
> sensible.
>
> [1/6]: move git_getpass to its own source file
> [2/6]: refactor git_getpass into generic prompt function
> [3/6]: stub out getpass_echo function
> [4/6]: prompt: add PROMPT_ECHO flag
> [5/6]: credential: use git_prompt instead of git_getpass
> [6/6]: compat/getpass: add a /dev/tty implementation
>
> -Peff
Interesting, I've been working on something pretty similar: getting
rid of getpass usage all together:
https://github.com/kusma/git/tree/work/askpass
My reason to write a getpass replacement was to avoid capping input to
PASS_MAX, which can be as low as 8 characters (and AFAIK is just that
on Solaris)...
^ permalink raw reply
* Re: [PATCH 0/6] echo usernames as they are typed
From: Junio C Hamano @ 2011-11-27 8:56 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20111127082744.GA32068@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Thu, Nov 24, 2011 at 05:58:01AM -0500, Jeff King wrote:
>
>> Here's a revised version of the http-auth / credential-helper series.
>
> And here's something I've been meaning to do on top: actually echo
> characters at the username prompt. We can't do this portably, but we can
> at least stub out a compatibility layer and let each system do something
> sensible.
Yay ;-)
^ permalink raw reply
* Re: what are the chances of a 'pre-upload' hook?
From: Junio C Hamano @ 2011-11-27 8:56 UTC (permalink / raw)
To: Jeff King; +Cc: Git, Sitaram Chamarty
In-Reply-To: <20111127000603.GA7687@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> Just because it is passed through the environment does not mean you need
> to have it set all the time. There is nothing wrong with:
>
> GIT_ALLOW_UNTRUSTED_HOOKS=true git fetch ~bob/repo.git
>
> We can even spell it:
>
> git --allow-untrusted-hooks fetch ~bob/repo.git
>
> but it should probably still end up as an environment variable to make
> it through to the remote side (you could also tack it on to the
> upload-pack command line; that wouldn't make it across git:// or http://
> connections, but those are irrelevant here anyway).
I actually like the idea of allowing pre-upload-pack hook on git:// and
possibly http:// only. git-daemon can tell the upload-pack that it is OK
to run the hook, and the hook can do the things that only the daemon can
do, never touching what the original requestor would but the repository
owner would not have an access to. Normal file:// and ssh:// transfer
should not be needed for GitHub's stats or Sitaram's proxy usage, and we
should be able to unconditionally disable the hook when transfer is going
over these protocols, no?
One scenario I do not want to see is this. Suppose there is a company that
runs more than one project, and one of the projects is so secret that only
engineers working on the project have access to its repository, while all
people, including the project-A engineers, in the company have access to
other repositories. Further suppose that people involved in some or many
of the general repositories want to do something before a fetch from them
happens. They will use the pre-upload-hook to implement it if we make it
available, and their new-hire training course will tell their engineers to
set the GIT_ALLOW_UNTRUSTED_HOOKS. Perhaps the /etc/skel/profile the
company gives the new-hires already defines it.
And somebody who controls one of the general repositories installs a
pre-upload-hook that borrows credential of a project-A engineer who
happens to fetch from it to access repositories of the project-A.
Imagine how the blame game that will ensue goes after materials from
project-A is exposed. Of course, the owner of the rogue hook will get the
first blame, but I do not think Git will come unscathed out of it. At
least we will be blamed for adding an inherently unsafe mechanism.
^ permalink raw reply
* [PATCH 6/6] compat/getpass: add a /dev/tty implementation
From: Jeff King @ 2011-11-27 8:31 UTC (permalink / raw)
To: git
In-Reply-To: <20111127082744.GA32068@sigill.intra.peff.net>
This is more or less what regular getpass() does, but
without turning off character echoing. You have to set
HAVE_DEV_TTY to enable it.
For now, only Linux enables this by default. People on other
/dev/tty-enabled systems can submit patches to turn it on
once they have tested it.
Signed-off-by: Jeff King <peff@peff.net>
---
Makefile | 8 ++++++++
compat/getpass.c | 35 +++++++++++++++++++++++++++++++++++
2 files changed, 43 insertions(+), 0 deletions(-)
diff --git a/Makefile b/Makefile
index d133e2b..a2afe31 100644
--- a/Makefile
+++ b/Makefile
@@ -227,6 +227,9 @@ all::
#
# Define NO_REGEX if you have no or inferior regex support in your C library.
#
+# Define HAVE_DEV_TTY if your system can open /dev/tty to interact with the
+# user.
+#
# Define GETTEXT_POISON if you are debugging the choice of strings marked
# for translation. In a GETTEXT_POISON build, you can turn all strings marked
# for translation into gibberish by setting the GIT_GETTEXT_POISON variable
@@ -835,6 +838,7 @@ ifeq ($(uname_S),Linux)
NO_STRLCPY = YesPlease
NO_MKSTEMPS = YesPlease
HAVE_PATHS_H = YesPlease
+ HAVE_DEV_TTY = YesPlease
endif
ifeq ($(uname_S),GNU/kFreeBSD)
NO_STRLCPY = YesPlease
@@ -1641,6 +1645,10 @@ ifdef HAVE_PATHS_H
BASIC_CFLAGS += -DHAVE_PATHS_H
endif
+ifdef HAVE_DEV_TTY
+ BASIC_CFLAGS += -DHAVE_DEV_TTY
+endif
+
ifdef DIR_HAS_BSD_GROUP_SEMANTICS
COMPAT_CFLAGS += -DDIR_HAS_BSD_GROUP_SEMANTICS
endif
diff --git a/compat/getpass.c b/compat/getpass.c
index 8ae82f0..b5bd1dd 100644
--- a/compat/getpass.c
+++ b/compat/getpass.c
@@ -1,6 +1,41 @@
#include "../git-compat-util.h"
+#ifdef HAVE_DEV_TTY
+
+char *getpass_echo(const char *prompt)
+{
+ static char buf[1024];
+ FILE *fh;
+ int i;
+
+ fh = fopen("/dev/tty", "w+");
+ if (!fh)
+ return NULL;
+
+ fputs(prompt, fh);
+ fflush(fh);
+ for (i = 0; i < sizeof(buf) - 1; i++) {
+ int ch = getc(fh);
+ if (ch == EOF || ch == '\n')
+ break;
+ buf[i] = ch;
+ }
+ buf[i] = '\0';
+
+ if (ferror(fh)) {
+ fclose(fh);
+ return NULL;
+ }
+
+ fclose(fh);
+ return buf;
+}
+
+#else
+
char *getpass_echo(const char *prompt)
{
return getpass(prompt);
}
+
+#endif
--
1.7.7.4.7.g24824
^ permalink raw reply related
* [PATCH 5/6] credential: use git_prompt instead of git_getpass
From: Jeff King @ 2011-11-27 8:31 UTC (permalink / raw)
To: git
In-Reply-To: <20111127082744.GA32068@sigill.intra.peff.net>
When we request a username and password from the user on the
terminal, we can't use stdin/stdout, because they may be
connected to pipes to other git processes.
Instead, we use getpass() (via git_getpass), which will
generally open /dev/tty and read and write from that. The
only problem is that getpass is meant to take passwords, and
therefore will not echo characters from the username, which
is annoying.
Now that git_prompt understand the "echo" flag, we can use
that to let the user see their username as they type it.
Signed-off-by: Jeff King <peff@peff.net>
---
credential.c | 15 +++++++--------
1 files changed, 7 insertions(+), 8 deletions(-)
diff --git a/credential.c b/credential.c
index d918ba5..9ad580d 100644
--- a/credential.c
+++ b/credential.c
@@ -109,7 +109,8 @@ static void credential_describe(struct credential *c, struct strbuf *out)
strbuf_addf(out, "/%s", c->path);
}
-static char *credential_ask_one(const char *what, struct credential *c)
+static char *credential_ask_one(const char *what, struct credential *c,
+ int flags)
{
struct strbuf desc = STRBUF_INIT;
struct strbuf prompt = STRBUF_INIT;
@@ -121,11 +122,7 @@ static void credential_describe(struct credential *c, struct strbuf *out)
else
strbuf_addf(&prompt, "%s: ", what);
- /* FIXME: for usernames, we should do something less magical that
- * actually echoes the characters. However, we need to read from
- * /dev/tty and not stdio, which is not portable (but getpass will do
- * it for us). http.c uses the same workaround. */
- r = git_getpass(prompt.buf);
+ r = git_prompt(prompt.buf, what, flags);
strbuf_release(&desc);
strbuf_release(&prompt);
@@ -135,9 +132,11 @@ static void credential_describe(struct credential *c, struct strbuf *out)
static void credential_getpass(struct credential *c)
{
if (!c->username)
- c->username = credential_ask_one("Username", c);
+ c->username = credential_ask_one("Username", c,
+ PROMPT_ASKPASS|PROMPT_ECHO);
if (!c->password)
- c->password = credential_ask_one("Password", c);
+ c->password = credential_ask_one("Password", c,
+ PROMPT_ASKPASS);
}
int credential_read(struct credential *c, FILE *fp)
--
1.7.7.4.7.g24824
^ permalink raw reply related
* [PATCH 4/6] prompt: add PROMPT_ECHO flag
From: Jeff King @ 2011-11-27 8:30 UTC (permalink / raw)
To: git
In-Reply-To: <20111127082744.GA32068@sigill.intra.peff.net>
This will use getpass_echo when set.
Signed-off-by: Jeff King <peff@peff.net>
---
prompt.c | 5 ++++-
prompt.h | 1 +
2 files changed, 5 insertions(+), 1 deletions(-)
diff --git a/prompt.c b/prompt.c
index 7c8f9aa..c46227f 100644
--- a/prompt.c
+++ b/prompt.c
@@ -2,6 +2,7 @@
#include "run-command.h"
#include "strbuf.h"
#include "prompt.h"
+#include "compat/getpass.h"
static char *do_askpass(const char *cmd, const char *prompt, const char *name)
{
@@ -48,7 +49,9 @@
return do_askpass(askpass, prompt, name);
}
- return getpass(prompt);
+ return flags & PROMPT_ECHO ?
+ getpass_echo(prompt) :
+ getpass(prompt);
}
char *git_getpass(const char *prompt)
diff --git a/prompt.h b/prompt.h
index 18868c2..7201cae 100644
--- a/prompt.h
+++ b/prompt.h
@@ -2,6 +2,7 @@
#define PROMPT_H
#define PROMPT_ASKPASS (1<<0)
+#define PROMPT_ECHO (1<<1)
char *git_prompt(const char *prompt, const char *name, int flags);
char *git_getpass(const char *prompt);
--
1.7.7.4.7.g24824
^ permalink raw reply related
* [PATCH 3/6] stub out getpass_echo function
From: Jeff King @ 2011-11-27 8:30 UTC (permalink / raw)
To: git
In-Reply-To: <20111127082744.GA32068@sigill.intra.peff.net>
We can't implement getpass_echo portably, but we can at
least put in the infrastructure so that builds can provide a
system-specific way of accomplishing this.
Right now we just fall back on calling getpass (which
doesn't echo, but is available almost everywhere).
Signed-off-by: Jeff King <peff@peff.net>
---
Makefile | 2 ++
compat/getpass.c | 6 ++++++
compat/getpass.h | 6 ++++++
3 files changed, 14 insertions(+), 0 deletions(-)
create mode 100644 compat/getpass.c
create mode 100644 compat/getpass.h
diff --git a/Makefile b/Makefile
index 14c6480..d133e2b 100644
--- a/Makefile
+++ b/Makefile
@@ -521,6 +521,7 @@ LIB_H += color.h
LIB_H += commit.h
LIB_H += compat/bswap.h
LIB_H += compat/cygwin.h
+LIB_H += compat/getpass.h
LIB_H += compat/mingw.h
LIB_H += compat/obstack.h
LIB_H += compat/win32/pthread.h
@@ -609,6 +610,7 @@ LIB_OBJS += cache-tree.o
LIB_OBJS += color.o
LIB_OBJS += combine-diff.o
LIB_OBJS += commit.o
+LIB_OBJS += compat/getpass.o
LIB_OBJS += compat/obstack.o
LIB_OBJS += config.o
LIB_OBJS += connect.o
diff --git a/compat/getpass.c b/compat/getpass.c
new file mode 100644
index 0000000..8ae82f0
--- /dev/null
+++ b/compat/getpass.c
@@ -0,0 +1,6 @@
+#include "../git-compat-util.h"
+
+char *getpass_echo(const char *prompt)
+{
+ return getpass(prompt);
+}
diff --git a/compat/getpass.h b/compat/getpass.h
new file mode 100644
index 0000000..5f1986b
--- /dev/null
+++ b/compat/getpass.h
@@ -0,0 +1,6 @@
+#ifndef GETPASS_H
+#define GETPASS_H
+
+char *getpass_echo(const char *prompt);
+
+#endif /* GETPASS_H */
--
1.7.7.4.7.g24824
^ permalink raw reply related
* [PATCH 2/6] refactor git_getpass into generic prompt function
From: Jeff King @ 2011-11-27 8:29 UTC (permalink / raw)
To: git
In-Reply-To: <20111127082744.GA32068@sigill.intra.peff.net>
This will allow callers to specify more options (e.g.,
leaving echo on). The original git_getpass becomes a slim
wrapper around the new function.
Signed-off-by: Jeff King <peff@peff.net>
---
Making askpass optional isn't necessary for this series, but splitting
it actually makes the code a little cleaner, imho, and some callers
might eventually want to turn it off.
prompt.c | 41 +++++++++++++++++++++++++----------------
prompt.h | 3 +++
2 files changed, 28 insertions(+), 16 deletions(-)
diff --git a/prompt.c b/prompt.c
index 42a1c9f..7c8f9aa 100644
--- a/prompt.c
+++ b/prompt.c
@@ -3,26 +3,13 @@
#include "strbuf.h"
#include "prompt.h"
-char *git_getpass(const char *prompt)
+static char *do_askpass(const char *cmd, const char *prompt, const char *name)
{
- const char *askpass;
struct child_process pass;
const char *args[3];
static struct strbuf buffer = STRBUF_INIT;
- askpass = getenv("GIT_ASKPASS");
- if (!askpass)
- askpass = askpass_program;
- if (!askpass)
- askpass = getenv("SSH_ASKPASS");
- if (!askpass || !(*askpass)) {
- char *result = getpass(prompt);
- if (!result)
- die_errno("Could not read password");
- return result;
- }
-
- args[0] = askpass;
+ args[0] = cmd;
args[1] = prompt;
args[2] = NULL;
@@ -35,7 +22,7 @@
strbuf_reset(&buffer);
if (strbuf_read(&buffer, pass.out, 20) < 0)
- die("failed to read password from %s\n", askpass);
+ die("failed to read %s from %s\n", name, cmd);
close(pass.out);
@@ -46,3 +33,25 @@
return buffer.buf;
}
+
+char *git_prompt(const char *prompt, const char *name, int flags)
+{
+ if (flags & PROMPT_ASKPASS) {
+ const char *askpass;
+
+ askpass = getenv("GIT_ASKPASS");
+ if (!askpass)
+ askpass = askpass_program;
+ if (!askpass)
+ askpass = getenv("SSH_ASKPASS");
+ if (askpass && *askpass)
+ return do_askpass(askpass, prompt, name);
+ }
+
+ return getpass(prompt);
+}
+
+char *git_getpass(const char *prompt)
+{
+ return git_prompt(prompt, "password", PROMPT_ASKPASS);
+}
diff --git a/prompt.h b/prompt.h
index 0fd7bd9..18868c2 100644
--- a/prompt.h
+++ b/prompt.h
@@ -1,6 +1,9 @@
#ifndef PROMPT_H
#define PROMPT_H
+#define PROMPT_ASKPASS (1<<0)
+
+char *git_prompt(const char *prompt, const char *name, int flags);
char *git_getpass(const char *prompt);
#endif /* PROMPT_H */
--
1.7.7.4.7.g24824
^ permalink raw reply related
* [PATCH 1/6] move git_getpass to its own source file
From: Jeff King @ 2011-11-27 8:28 UTC (permalink / raw)
To: git
In-Reply-To: <20111127082744.GA32068@sigill.intra.peff.net>
This is currently in connect.c, but really has nothing to
do with the git protocol itself. Let's make a new source
file all about prompting the user, which will make it
cleaner to refactor.
Signed-off-by: Jeff King <peff@peff.net>
---
Makefile | 2 ++
cache.h | 1 -
connect.c | 44 --------------------------------------------
credential.c | 1 +
imap-send.c | 1 +
prompt.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
prompt.h | 6 ++++++
7 files changed, 58 insertions(+), 45 deletions(-)
create mode 100644 prompt.c
create mode 100644 prompt.h
diff --git a/Makefile b/Makefile
index 2537128..14c6480 100644
--- a/Makefile
+++ b/Makefile
@@ -563,6 +563,7 @@ LIB_H += parse-options.h
LIB_H += patch-ids.h
LIB_H += pkt-line.h
LIB_H += progress.h
+LIB_H += prompt.h
LIB_H += quote.h
LIB_H += reflog-walk.h
LIB_H += refs.h
@@ -669,6 +670,7 @@ LIB_OBJS += pkt-line.o
LIB_OBJS += preload-index.o
LIB_OBJS += pretty.o
LIB_OBJS += progress.o
+LIB_OBJS += prompt.o
LIB_OBJS += quote.o
LIB_OBJS += reachable.o
LIB_OBJS += read-cache.o
diff --git a/cache.h b/cache.h
index 2e6ad36..f320c98 100644
--- a/cache.h
+++ b/cache.h
@@ -1024,7 +1024,6 @@ struct ref {
extern struct ref *find_ref_by_name(const struct ref *list, const char *name);
#define CONNECT_VERBOSE (1u << 0)
-extern char *git_getpass(const char *prompt);
extern struct child_process *git_connect(int fd[2], const char *url, const char *prog, int flags);
extern int finish_connect(struct child_process *conn);
extern int git_connection_is_socket(struct child_process *conn);
diff --git a/connect.c b/connect.c
index 51990fa..519e527 100644
--- a/connect.c
+++ b/connect.c
@@ -619,47 +619,3 @@ int finish_connect(struct child_process *conn)
free(conn);
return code;
}
-
-char *git_getpass(const char *prompt)
-{
- const char *askpass;
- struct child_process pass;
- const char *args[3];
- static struct strbuf buffer = STRBUF_INIT;
-
- askpass = getenv("GIT_ASKPASS");
- if (!askpass)
- askpass = askpass_program;
- if (!askpass)
- askpass = getenv("SSH_ASKPASS");
- if (!askpass || !(*askpass)) {
- char *result = getpass(prompt);
- if (!result)
- die_errno("Could not read password");
- return result;
- }
-
- args[0] = askpass;
- args[1] = prompt;
- args[2] = NULL;
-
- memset(&pass, 0, sizeof(pass));
- pass.argv = args;
- pass.out = -1;
-
- if (start_command(&pass))
- exit(1);
-
- strbuf_reset(&buffer);
- if (strbuf_read(&buffer, pass.out, 20) < 0)
- die("failed to read password from %s\n", askpass);
-
- close(pass.out);
-
- if (finish_command(&pass))
- exit(1);
-
- strbuf_setlen(&buffer, strcspn(buffer.buf, "\r\n"));
-
- return buffer.buf;
-}
diff --git a/credential.c b/credential.c
index 36ff0ec..d918ba5 100644
--- a/credential.c
+++ b/credential.c
@@ -3,6 +3,7 @@
#include "string-list.h"
#include "run-command.h"
#include "url.h"
+#include "prompt.h"
void credential_init(struct credential *c)
{
diff --git a/imap-send.c b/imap-send.c
index e1ad1a4..8fc2e2d 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -25,6 +25,7 @@
#include "cache.h"
#include "exec_cmd.h"
#include "run-command.h"
+#include "prompt.h"
#ifdef NO_OPENSSL
typedef void *SSL;
#else
diff --git a/prompt.c b/prompt.c
new file mode 100644
index 0000000..42a1c9f
--- /dev/null
+++ b/prompt.c
@@ -0,0 +1,48 @@
+#include "cache.h"
+#include "run-command.h"
+#include "strbuf.h"
+#include "prompt.h"
+
+char *git_getpass(const char *prompt)
+{
+ const char *askpass;
+ struct child_process pass;
+ const char *args[3];
+ static struct strbuf buffer = STRBUF_INIT;
+
+ askpass = getenv("GIT_ASKPASS");
+ if (!askpass)
+ askpass = askpass_program;
+ if (!askpass)
+ askpass = getenv("SSH_ASKPASS");
+ if (!askpass || !(*askpass)) {
+ char *result = getpass(prompt);
+ if (!result)
+ die_errno("Could not read password");
+ return result;
+ }
+
+ args[0] = askpass;
+ args[1] = prompt;
+ args[2] = NULL;
+
+ memset(&pass, 0, sizeof(pass));
+ pass.argv = args;
+ pass.out = -1;
+
+ if (start_command(&pass))
+ exit(1);
+
+ strbuf_reset(&buffer);
+ if (strbuf_read(&buffer, pass.out, 20) < 0)
+ die("failed to read password from %s\n", askpass);
+
+ close(pass.out);
+
+ if (finish_command(&pass))
+ exit(1);
+
+ strbuf_setlen(&buffer, strcspn(buffer.buf, "\r\n"));
+
+ return buffer.buf;
+}
diff --git a/prompt.h b/prompt.h
new file mode 100644
index 0000000..0fd7bd9
--- /dev/null
+++ b/prompt.h
@@ -0,0 +1,6 @@
+#ifndef PROMPT_H
+#define PROMPT_H
+
+char *git_getpass(const char *prompt);
+
+#endif /* PROMPT_H */
--
1.7.7.4.7.g24824
^ permalink raw reply related
* [PATCH 0/6] echo usernames as they are typed
From: Jeff King @ 2011-11-27 8:27 UTC (permalink / raw)
To: git
In-Reply-To: <20111124105801.GA6168@sigill.intra.peff.net>
On Thu, Nov 24, 2011 at 05:58:01AM -0500, Jeff King wrote:
> Here's a revised version of the http-auth / credential-helper series.
And here's something I've been meaning to do on top: actually echo
characters at the username prompt. We can't do this portably, but we can
at least stub out a compatibility layer and let each system do something
sensible.
[1/6]: move git_getpass to its own source file
[2/6]: refactor git_getpass into generic prompt function
[3/6]: stub out getpass_echo function
[4/6]: prompt: add PROMPT_ECHO flag
[5/6]: credential: use git_prompt instead of git_getpass
[6/6]: compat/getpass: add a /dev/tty implementation
-Peff
^ permalink raw reply
* Re: [PATCH] Remove redundant assignments
From: Junio C Hamano @ 2011-11-27 8:26 UTC (permalink / raw)
To: Jan Stępień; +Cc: git
In-Reply-To: <1322381142-20645-1-git-send-email-jstepien@users.sourceforge.net>
Jan Stępień <jstepien@users.sourceforge.net> writes:
> There were two redundant variable assignments in transport.c and
> wt-status.c. Removing them hasn't introduced any compiler warnings or
> regressions.
It may not have added any warnings for _you_.
These "type a = a;" initializations (they are not even assignments and
would not result in any code in the output for sane compilers) are idioms
to squelch "unused" warnings from versions of compiler whose data flow
analysis is less than perfect by telling them that the author of the
section of the code knows what s/he is doing.
At least, that is why these pseudo initializations were added in these
codepaths. I personally feel that they should be made into useless but
real assignments (which may result in a useless assignment emited in the
resulting object code) rather than removing like your patch did which
unfortunately forces people with imperfect compilers to live with the
warnings, if one finds them ugly.
> Signed-off-by: Jan Stępień <jstepien@users.sourceforge.net>
> ---
> transport.c | 2 +-
> wt-status.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/transport.c b/transport.c
> index 51814b5..5143718 100644
> --- a/transport.c
> +++ b/transport.c
> @@ -105,7 +105,7 @@ static void insert_packed_refs(const char *packed_refs, struct ref **list)
> return;
>
> for (;;) {
> - int cmp = cmp, len;
> + int cmp, len;
>
> if (!fgets(buffer, sizeof(buffer), f)) {
> fclose(f);
> diff --git a/wt-status.c b/wt-status.c
> index 70fdb76..35f61f4 100644
> --- a/wt-status.c
> +++ b/wt-status.c
> @@ -229,7 +229,7 @@ static void wt_status_print_change_data(struct wt_status *s,
> {
> struct wt_status_change_data *d = it->util;
> const char *c = color(change_type, s);
> - int status = status;
> + int status;
> char *one_name;
> char *two_name;
> const char *one, *two;
^ permalink raw reply
* [PATCH] Remove redundant assignments
From: Jan Stępień @ 2011-11-27 8:05 UTC (permalink / raw)
To: git; +Cc: Jan Stępień
There were two redundant variable assignments in transport.c and
wt-status.c. Removing them hasn't introduced any compiler warnings or
regressions.
Signed-off-by: Jan Stępień <jstepien@users.sourceforge.net>
---
transport.c | 2 +-
wt-status.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/transport.c b/transport.c
index 51814b5..5143718 100644
--- a/transport.c
+++ b/transport.c
@@ -105,7 +105,7 @@ static void insert_packed_refs(const char *packed_refs, struct ref **list)
return;
for (;;) {
- int cmp = cmp, len;
+ int cmp, len;
if (!fgets(buffer, sizeof(buffer), f)) {
fclose(f);
diff --git a/wt-status.c b/wt-status.c
index 70fdb76..35f61f4 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -229,7 +229,7 @@ static void wt_status_print_change_data(struct wt_status *s,
{
struct wt_status_change_data *d = it->util;
const char *c = color(change_type, s);
- int status = status;
+ int status;
char *one_name;
char *two_name;
const char *one, *two;
--
1.7.7.4
^ permalink raw reply related
* Re: what are the chances of a 'pre-upload' hook?
From: Junio C Hamano @ 2011-11-27 7:51 UTC (permalink / raw)
To: Jeff King; +Cc: Sitaram Chamarty, Git Mailing List
In-Reply-To: <20111126233133.GA31129@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> It depends on what you want your hook to do. I thought Sitaram's
> use-case was putting something like "git fetch upstream" into a hook
> that runs before upload-pack, to create a transparent proxy. That has
> nothing to do with the accessing user.
>
> At GitHub, we run a pre-upload-pack hook to keep statistics. That has
> nothing to do with the accessing user. We have to patch git to provide
> the hook.
I personally have a huge problem with shipping an inherently unsafe
mechanism that is disabled by default even if it is marked with big red
letters saying that it is unsafe and should not be enabled casually. It
makes it up to the system administrator to decide what is casual and what
is not, but when end users are get harmed by a clueless administrator's
decision, the repercussion will come back to the Git software, not to the
clueless administrator who enabled an unsafe mechanism in an environment
where it was not designed to support.
It may merely be a matter of perception. After all, the pre-upload-pack
hook I did in a8563ec (upload-pack: add a trigger for post-upload-pack
hook, 2009-08-26) in response to GitHub's stat request was so trivial that
even clueless admins could trojan it back into the current upload-pack
source to use in their own site, without letting their users know that
their fetch request may be opening their account to a random hook
script. So in that sense, it might not be worth my effort to fight for Git
users' safety to find a better solution and instead go with the big red
letter approach which is easy. I know perfect is an enemy of good, but
when I do not think an easy Quick-and-Dirty solution is even less than
good, I need to point out that not having less than bad is better than
having it.
If the mechanism to notify the external machinery (i.e. counting accesses,
learning the true destination of a new fetch request and have the fetcher
wait while the real fetch goes to the origin site) were not via a hook
that runs as the fetcher but were via something else that runs as the
owner of that external service (i.e. counting accesses, maintaining the
proxy object pool), I wouldn't have trouble with the proposal.
For example, upload-pack could write the details of the original request
to a named pipe to ask the "service" process listening on the other end,
and wait for its response.
^ permalink raw reply
* gitweb: in-page errors don't work with mod_perl
From: Jürgen Kreileder @ 2011-11-27 4:05 UTC (permalink / raw)
To: git
Hi
when gitweb.perl (208a1cc3d3) is run with mod_perl2 (2.0.5-2ubuntu1 on
Ubuntu 11.10) custom error pages don't work: Any page with status !=
200 shows the plain Apache error document instead of a gitweb's error
page.
Jürgen
--
http://blog.blackdown.de/
http://www.flickr.com/photos/jkreileder/
^ permalink raw reply
* gitweb: 404 links on some blob pages
From: Jürgen Kreileder @ 2011-11-27 4:03 UTC (permalink / raw)
To: git
Hi,
some blob pages have broken links:
For example, on
https://git.blackdown.de/?p=contactalbum.git;a=blob;f=Classes/WindowController.m;h=b84d1882cb6c3a2d2058cbdd56b2280b48f1690a;hb=b84d1882cb6c3a2d2058cbdd56b2280b48f1690a
the blob_plain link for WindowController.m leads to '404 - Cannot find file':
https://git.blackdown.de/?p=contactalbum.git;a=blob_plain;f=Classes/WindowController.m;hb=b84d1882cb6c3a2d2058cbdd56b2280b48f1690a
And the tree link for "[contactalbum.git]" leads to '404 - Reading tree failed':
https://git.blackdown.de/?p=contactalbum.git;a=tree;hb=b84d1882cb6c3a2d2058cbdd56b2280b48f1690a
Here's a page from git.kernel.org which shows exactly the same problem:
http://git.kernel.org/?p=linux/kernel/git/next/linux-next-history.git;a=blob;f=drivers/Makefile;h=91077ac6b1564a21449a155cde1b84d6678d6e13;hb=91077ac6b1564a21449a155cde1b84d6678d6e13
Jürgen
--
http://blog.blackdown.de/
http://www.flickr.com/photos/jkreileder/
^ permalink raw reply
* gitweb: broken links with pathinfo
From: Jürgen Kreileder @ 2011-11-27 4:02 UTC (permalink / raw)
To: git
Hi,
if pathinfo (gitweb.perl 208a1cc3d3) is enabled, some pages lead to
404 errors. For example, on
https://git.blackdown.de/contactalbum.git/blobdiff/c7fdc45614bd127581852ce429c4483b1d21c0d4..82f7c8babf6095224bf5d8ab04f6eea4b1a555ee:/Classes/WindowController.h
a/Classes/WindowController.h ->
https://git.blackdown.de/contactalbum.git/blob/3c27189e8adc690f421334a9cad7ad719c066eb4:/Classes/WindowController.h
b/Classes/WindowController.h ->
https://git.blackdown.de/contactalbum.git/blob/f285b2728d2c7782806f0b7f6d696b226a88f03c:/Classes/WindowController.h
Both links give '404 - Cannot find file'
With pathinfo disabled, I get the following working URLs intead:
a/Classes/WindowController.h ->
https://git.blackdown.de/?p=contactalbum.git;a=blob;f=Classes/WindowController.h;h=3c27189e8adc690f421334a9cad7ad719c066eb4;hb=3c27189e8adc690f421334a9cad7ad719c066eb4
b/Classes/WindowController.h ->
https://git.blackdown.de/?p=contactalbum.git;a=blob;f=Classes/WindowController.h;h=f285b2728d2c7782806f0b7f6d696b226a88f03c;hb=f285b2728d2c7782806f0b7f6d696b226a88f03c
Jürgen
--
http://blog.blackdown.de/
http://www.flickr.com/photos/jkreileder/
^ permalink raw reply
* Re: what are the chances of a 'pre-upload' hook?
From: Jeff King @ 2011-11-27 0:06 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git, Sitaram Chamarty
In-Reply-To: <CAPc5daUodry_=6pZxA=QOpuRUj9C2ed9Gzp6E1_G93iGfOOvOA@mail.gmail.com>
On Sat, Nov 26, 2011 at 03:57:40PM -0800, Junio C Hamano wrote:
> Did I say anything about saNe?. I was talking about saFe.
Fine. But that doesn't change my point: the purpose of such a feature is
to tell git "do _not_ be safe; I have decided already for you whether it
is OK to do this".
> > By turning it on, you
> > are saying "it's OK to run arbitrary code from the repo as the current
> > user".
>
> The problem I have with it is that you are saying much more than that.
> ... as the current user ANYWHERE on the machine.
Just because it is passed through the environment does not mean you need
to have it set all the time. There is nothing wrong with:
GIT_ALLOW_UNTRUSTED_HOOKS=true git fetch ~bob/repo.git
We can even spell it:
git --allow-untrusted-hooks fetch ~bob/repo.git
but it should probably still end up as an environment variable to make
it through to the remote side (you could also tack it on to the
upload-pack command line; that wouldn't make it across git:// or http://
connections, but those are irrelevant here anyway).
-Peff
^ permalink raw reply
* Re: what are the chances of a 'pre-upload' hook?
From: Jeff King @ 2011-11-26 23:51 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Sitaram Chamarty, Git
In-Reply-To: <CAPc5daXY_4aimugj8Z4BFE8YvBSM1K+evPU69rLGH5ETo6PO=Q@mail.gmail.com>
On Sat, Nov 26, 2011 at 03:47:09PM -0800, Junio C Hamano wrote:
> > My point is to make it available, give it safe
> > semantics by default, and let people who are running daemon-like service
> > (i.e., where the admin controls the daemon and arbitrary users can't
> > write into the hooks directory) use it by setting an environment
> > variable, rather than patching git.
>
> I think we re on the same page on that point, and this thread is to find
> such a safe default and safe semantics when enabled.
>
> Unfortunately neither your "trusted" switch nor check the gid of repository
> is that safe thing (sane default part is easy; do not allow it by default).
Sorry, why is the trusted switch not a sane thing? By turning it on, you
are saying "it's OK to run arbitrary code from the repo as the current
user". It's _exactly_ what some people are going to want to do[1],
regardless of any other heuristics.
Sure, maybe it's giving people rope to hang themselves with, but I don't
see a problem with that as long as the issues are clearly laid out in
the documentation.
-Peff
[1] An alternate and even more flexible form is to not just say "it's OK
to run hooks", but to say "run this particular hook as a
pre-upload-hook" without any regard for what's in $GIT_DIR/hooks. It is
a superset of the other form, because of course the hook you tell it
to run can be "sh $GIT_DIR/hooks/pre-upload-pack".
^ permalink raw reply
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