* Re: [PATCH/RFC] ident: check /etc/mailname if email is unknown
From: Junio C Hamano @ 2011-10-03 5:30 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: git, Matt Kraai, Gerrit Pape, Ian Jackson, Linus Torvalds
In-Reply-To: <20111003045745.GA17604@elie>
Jonathan Nieder <jrnieder@gmail.com> writes:
> @@ -52,6 +52,8 @@ static void copy_gecos(const struct passwd *w, char *name, size_t sz)
>
> static void copy_email(const struct passwd *pw)
> {
> + FILE *mailname;
> +
> /*
> * Make up a fake email address
> * (name + '@' + hostname [+ '.' + domainname])
> @@ -61,6 +63,27 @@ static void copy_email(const struct passwd *pw)
> die("Your sysadmin must hate you!");
> memcpy(git_default_email, pw->pw_name, len);
> git_default_email[len++] = '@';
> +
> + /*
> + * The domain part comes from /etc/mailname if it is readable,
> + * or the current hostname and domain name otherwise.
> + */
> + mailname = fopen("/etc/mailname", "r");
> + if (!mailname) {
> + if (errno != ENOENT)
> + warning("cannot open /etc/mailname: %s",
> + strerror(errno));
> + } else if (fgets(git_default_email + len,
> + sizeof(git_default_email) - len, mailname)) {
> + /* success! */
> + fclose(mailname);
> + return;
> + } else {
> + if (ferror(mailname))
> + warning("cannot read /etc/mailname: %s",
> + strerror(errno));
> + fclose(mailname);
> + }
> gethostname(git_default_email + len, sizeof(git_default_email) - len);
> if (!strchr(git_default_email+len, '.')) {
> struct hostent *he = gethostbyname(git_default_email + len);
I do not think this would hurt, even though I see /etc/mailname on only
one of my boxes (i.e. Debian). For maintainability for the future,
however, I would prefer to see the above hunk separated into a helper
function to keep addition to copy_email() to the minimum, e.g.
memcpy(git_default_email, pw->pw_name, len);
git_default_email[len++] = '@';
+ if (add_mailname_host(git_default_email, len, sizeof(git_default_email)))
+ return; /* read from "/etc/mailname" (Debian) */
gethostname(git_default_email + len, sizeof(git_default_email) - len);
...
So that people who care about other distros can more easily add a single
implementation to a similar location without making copy_email() too long
to lose clarity. The fallback default logic that does gethostname() might
also want to become a separate helper function as well.
^ permalink raw reply
* [PATCH v2] ident: check /etc/mailname if email is unknown
From: Jonathan Nieder @ 2011-10-03 6:16 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Matt Kraai, Gerrit Pape, Ian Jackson, Linus Torvalds
In-Reply-To: <7v8vp2iqvc.fsf@alter.siamese.dyndns.org>
Before falling back to gethostname(), check /etc/mailname if
GIT_AUTHOR_EMAIL is not set in the environment or through config
files. Only fall back if /etc/mailname cannot be opened or read.
The /etc/mailname convention comes from Debian policy section 11.6
("mail transport, delivery and user agents"), though maybe it could be
useful sometimes on other machines, too. The lack of this support was
noticed by various people in different ways:
- Ian observed that git was choosing the address
'ian@anarres.relativity.greenend.org.uk' rather than
'ian@davenant.greenend.org.uk' as it should have done.
- Jonathan noticed that operations like "git commit" were needlessly
slow when using a resolver that was slow to handle reverse DNS
lookups.
Alas, after this patch, if /etc/mailname is set up and the [user] name
and email configuration aren't, the committer email will not provide a
charming reminder of which machine commits were made on any more. But
I think it's worth it.
Mechanics: the functionality of reading mailname goes in its own
function, so people who care about other distros can easily add an
implementation to a similar location without making copy_email() too
long and losing clarity. While at it, we split out the fallback
default logic that does gethostname(), too (rearranging it a little
and adding a check for errors from gethostname while at it).
Based on a patch by Gerrit Pape <pape@smarden.org>.
Requested-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Improved-by: Junio C Hamano <gitster@pobox.com>
---
Junio C Hamano wrote:
> I do not think this would hurt, even though I see /etc/mailname on only
> one of my boxes (i.e. Debian). For maintainability for the future,
> however, I would prefer to see the above hunk separated into a helper
> function to keep addition to copy_email() to the minimum, e.g.
Makes sense. How about this?
Documentation/git-commit-tree.txt | 8 ++++-
ident.c | 66 +++++++++++++++++++++++++++++-------
2 files changed, 60 insertions(+), 14 deletions(-)
diff --git a/Documentation/git-commit-tree.txt b/Documentation/git-commit-tree.txt
index 0fdb82ee..02133d5f 100644
--- a/Documentation/git-commit-tree.txt
+++ b/Documentation/git-commit-tree.txt
@@ -68,7 +68,9 @@ if set:
In case (some of) these environment variables are not set, the information
is taken from the configuration items user.name and user.email, or, if not
-present, system user name and fully qualified hostname.
+present, system user name and the hostname used for outgoing mail (taken
+from `/etc/mailname` and falling back to the fully qualified hostname when
+that file does not exist).
A commit comment is read from stdin. If a changelog
entry is not provided via "<" redirection, 'git commit-tree' will just wait
@@ -90,6 +92,10 @@ Discussion
include::i18n.txt[]
+FILES
+-----
+/etc/mailname
+
SEE ALSO
--------
linkgit:git-write-tree[1]
diff --git a/ident.c b/ident.c
index 35a6f264..edb43144 100644
--- a/ident.c
+++ b/ident.c
@@ -50,6 +50,54 @@ static void copy_gecos(const struct passwd *w, char *name, size_t sz)
}
+static int add_mailname_host(char *buf, size_t len)
+{
+ FILE *mailname;
+
+ mailname = fopen("/etc/mailname", "r");
+ if (!mailname) {
+ if (errno != ENOENT)
+ warning("cannot open /etc/mailname: %s",
+ strerror(errno));
+ return -1;
+ }
+ if (!fgets(buf, len, mailname)) {
+ if (ferror(mailname))
+ warning("cannot read /etc/mailname: %s",
+ strerror(errno));
+ fclose(mailname);
+ return -1;
+ }
+ /* success! */
+ fclose(mailname);
+ return 0;
+}
+
+static void add_domainname(char *buf, size_t len)
+{
+ struct hostent *he;
+ size_t namelen;
+ const char *domainname;
+
+ if (gethostname(buf, len)) {
+ warning("cannot get host name: %s", strerror(errno));
+ strlcpy(buf, "(none)", len);
+ return;
+ }
+ namelen = strlen(buf);
+ if (memchr(buf, '.', namelen))
+ return;
+
+ he = gethostbyname(buf);
+ buf[namelen++] = '.';
+ buf += namelen;
+ len -= namelen;
+ if (he && (domainname = strchr(he->h_name, '.')))
+ strlcpy(buf, domainname + 1, len);
+ else
+ strlcpy(buf, "(none)", len);
+}
+
static void copy_email(const struct passwd *pw)
{
/*
@@ -61,20 +109,12 @@ static void copy_email(const struct passwd *pw)
die("Your sysadmin must hate you!");
memcpy(git_default_email, pw->pw_name, len);
git_default_email[len++] = '@';
- gethostname(git_default_email + len, sizeof(git_default_email) - len);
- if (!strchr(git_default_email+len, '.')) {
- struct hostent *he = gethostbyname(git_default_email + len);
- char *domainname;
- len = strlen(git_default_email);
- git_default_email[len++] = '.';
- if (he && (domainname = strchr(he->h_name, '.')))
- strlcpy(git_default_email + len, domainname + 1,
- sizeof(git_default_email) - len);
- else
- strlcpy(git_default_email + len, "(none)",
- sizeof(git_default_email) - len);
- }
+ if (!add_mailname_host(git_default_email + len,
+ sizeof(git_default_email) - len))
+ return; /* read from "/etc/mailname" (Debian) */
+ add_domainname(git_default_email + len,
+ sizeof(git_default_email) - len);
}
static void setup_ident(void)
--
1.7.7.rc1
^ permalink raw reply related
* Re: [PATCH v2] gitk: Show patch for initial commit
From: Marcus Karlsson @ 2011-10-03 6:33 UTC (permalink / raw)
To: Zbigniew J??drzejewski-Szmek; +Cc: git, gitster, Paul Mackerras
In-Reply-To: <4E878016.703@in.waw.pl>
On Sat, Oct 01, 2011 at 11:03:18PM +0200, Zbigniew J??drzejewski-Szmek wrote:
> [cc: Paul Mackerras]
>
> Hi,
> I think that the historical explanation that Junio gave could
> be used as a basis for a commit message:
>
> In early days, all projects managed by git (except for git itself) had the
> product of a fairly mature development history in their first commit, and
> it was deemed unnecessary clutter to show additions of these thousands of
> paths as a patch.
>
> "git log" learned to show the patch for the initial commit without requiring
> --root command line option at 0f03ca9 (config option log.showroot to show
> the diff of root commits, 2006-11-23).
>
> Teach gitk to respect log.showroot.
Absolutely, that would be a much better commit message. I'll wait and
see if there are more comments and then resubmit.
> Also the gitk should be mentioned in the man-page for git-config log.showroot.
> The current description of this option seems suboptimal because it explains
> how it used to be, which is not really relevant:
> log.showroot
> If true, the initial commit will be shown as a big creation event. This is
> equivalent to a diff against an empty tree. Tools like git-log(1) or git-
> whatchanged(1), which normally hide the root commit will now show it. True by
> default.
> This could be changed to:
> If true (the default), the root commit will be shown as a big creation
> event --- a diff against an empty tree. This diff can be very large for
> a project which was imported into git after some development history.
> If log.showroot is false tools like git-log(1), git-whatchanged(1), or
> gitk(1) will not display the added files.
I agree, but that feels like something that could be made into a
separate patch. Or should I include that too?
Marcus
^ permalink raw reply
* [PATCH] Makefile: do not set setgid bit on directories on GNU/kFreeBSD
From: Jonathan Nieder @ 2011-10-03 6:41 UTC (permalink / raw)
To: Petr Salinger; +Cc: git
The g+s bit on directories to make group ownership inherited is a
SysVism --- BSD and most of its descendants do not need it since they
do the sane thing by default without g+s. In fact, on some
filesystems (but not all --- tmpfs works this way but UFS does not),
the kernel of FreeBSD does not even allow non-root users to set setgid
bit on directories and produces errors when one tries:
$ git init --shared dir
fatal: Could not make /tmp/dir/.git/refs writable by group
Since the setgid bit would only mean "do what you were going to do
already", it's better to avoid setting it. Accordingly, ever since
v1.5.5-rc0~59^2 (Do not use GUID on dir in git init --share=all on
FreeBSD, 2008-03-05), git on true FreeBSD has done exactly that. Set
DIR_HAS_BSD_GROUP_SEMANTICS in the makefile for GNU/kFreeBSD, too, so
machines that use glibc with the kernel of FreeBSD get the same fix.
This fixes t0001-init.sh and t1301-shared-repo.sh on GNU/kFreeBSD
when running tests with --root pointing to a directory that uses
tmpfs.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Sorry to have taken so long to send this one out. Anyway, it seems
to me like the right thing to do. Petr, what do you think?
Makefile | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/Makefile b/Makefile
index 8d6d4515..924749ed 100644
--- a/Makefile
+++ b/Makefile
@@ -820,6 +820,7 @@ ifeq ($(uname_S),GNU/kFreeBSD)
NO_STRLCPY = YesPlease
NO_MKSTEMPS = YesPlease
HAVE_PATHS_H = YesPlease
+ DIR_HAS_BSD_GROUP_SEMANTICS = YesPlease
endif
ifeq ($(uname_S),UnixWare)
CC = cc
--
1.7.7.rc1
^ permalink raw reply related
* Re: [PATCH v2] ident: check /etc/mailname if email is unknown
From: Johannes Sixt @ 2011-10-03 7:09 UTC (permalink / raw)
To: Jonathan Nieder
Cc: Junio C Hamano, git, Matt Kraai, Gerrit Pape, Ian Jackson,
Linus Torvalds
In-Reply-To: <20111003061633.GB17289@elie>
Am 10/3/2011 8:16, schrieb Jonathan Nieder:
> +static int add_mailname_host(char *buf, size_t len)
> +{
> + FILE *mailname;
> +
> + mailname = fopen("/etc/mailname", "r");
> + if (!mailname) {
> + if (errno != ENOENT)
> + warning("cannot open /etc/mailname: %s",
> + strerror(errno));
This warns on EACCES. Is that OK? (Just asking, I have no opinion.)
-- Hannes
^ permalink raw reply
* Re: Branches & directories
From: Hilco Wijbenga @ 2011-10-03 7:15 UTC (permalink / raw)
To: Jeff King
Cc: Robin Rosenberg, Kyle Moffett, Michael Witten, Junio C Hamano,
Evan Shelhamer, Git Mailing List
In-Reply-To: <20111003030723.GA24523@sigill.intra.peff.net>
On 2 October 2011 20:07, Jeff King <peff@peff.net> wrote:
<snip/>
> Or did you really mean your example literally, as in you run two
> checkouts back to back, without running anything in between, and the
> second checkout restores the state before the first one. In that case,
> yes, it would be correct to keep the old timestamps. But this is an
> optimization that can only apply in a few very specific cases. And
> moreoever, how can git know when it is OK to apply that optimization? It
> has no idea what commands you might have run since the last time we were
> at "master".
Yes, I meant it literally. And, no, Git could not possibly know so it
would have to be optional behaviour. But it's probably a lot of work
for (for most people) little gain.
^ permalink raw reply
* Re: [PATCH] Makefile: do not set setgid bit on directories on GNU/kFreeBSD
From: Jonathan Nieder @ 2011-10-03 7:19 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Petr Salinger
In-Reply-To: <20111003064120.GA24396@elie>
Jonathan Nieder wrote:
> Since the setgid bit would only mean "do what you were going to do
> already", it's better to avoid setting it. Accordingly, ever since
> v1.5.5-rc0~59^2 (Do not use GUID on dir in git init --share=all on
> FreeBSD, 2008-03-05), git on true FreeBSD has done exactly that. Set
> DIR_HAS_BSD_GROUP_SEMANTICS in the makefile for GNU/kFreeBSD, too, so
> machines that use glibc with the kernel of FreeBSD get the same fix.
[...]
> Sorry to have taken so long to send this one out. Anyway, it seems
> to me like the right thing to do. Petr, what do you think?
fwiw:
Acked-by: Petr Salinger <Petr.Salinger@seznam.cz>
Thanks for looking it over.
^ permalink raw reply
* Re: Branches & directories
From: Jeff King @ 2011-10-03 7:30 UTC (permalink / raw)
To: Hilco Wijbenga
Cc: Robin Rosenberg, Kyle Moffett, Michael Witten, Junio C Hamano,
Evan Shelhamer, Git Mailing List
In-Reply-To: <CAE1pOi2xmVHrVJcC85wvCv=anhn_kYizyUMpUVZF4EE33RoGmg@mail.gmail.com>
On Mon, Oct 03, 2011 at 12:15:33AM -0700, Hilco Wijbenga wrote:
> On 2 October 2011 20:07, Jeff King <peff@peff.net> wrote:
> <snip/>
> > Or did you really mean your example literally, as in you run two
> > checkouts back to back, without running anything in between, and the
> > second checkout restores the state before the first one. In that case,
> > yes, it would be correct to keep the old timestamps. But this is an
> > optimization that can only apply in a few very specific cases. And
> > moreoever, how can git know when it is OK to apply that optimization? It
> > has no idea what commands you might have run since the last time we were
> > at "master".
>
> Yes, I meant it literally. And, no, Git could not possibly know so it
> would have to be optional behaviour. But it's probably a lot of work
> for (for most people) little gain.
If you really want the human to trigger it, then you can do something
like this:
cat >git-checkout-timestamp <<\EOF
#!/bin/sh
old=`git rev-parse HEAD`
git checkout "$@" || exit 1
time=`git log -1 --format=%at`
git diff-tree --name-only -z "$old" HEAD |
perl -0ne "utime($time, $time, \$_)";
EOF
Drop that somewhere in your $PATH, and use it instead of regular
checkout. It restores the timestamps on any changed files, but not on
those that were not touched. So your:
git checkout branch
git checkout master
example would end up with timestamps set for "master" on the changed
files. Two caveats:
1. This can still break makefiles! For example, like this:
make foo.o ;# now foo.o is recent
vi foo.c ;# but foo.c is _more_ recent
git checkout branch ;# now it's even newer
git checkout-timestamp master ;# now we've restored it to some
;# old timestamp, and make will
;# think it's older than foo.o
2. In general, I'm not sure it makes any sense if there are local
worktree modifications to the files in question. But I didn't think
about it too hard. That ways madness lies.
-Peff
^ permalink raw reply
* Re: Branches & directories
From: Matthieu Moy @ 2011-10-03 7:32 UTC (permalink / raw)
To: Hilco Wijbenga
Cc: Jeff King, Robin Rosenberg, Kyle Moffett, Michael Witten,
Junio C Hamano, Evan Shelhamer, Git Mailing List
In-Reply-To: <CAE1pOi2xmVHrVJcC85wvCv=anhn_kYizyUMpUVZF4EE33RoGmg@mail.gmail.com>
Hilco Wijbenga <hilco.wijbenga@gmail.com> writes:
> Yes, I meant it literally. And, no, Git could not possibly know so it
> would have to be optional behaviour. But it's probably a lot of work
> for (for most people) little gain.
Not only little gain, but also important risk: users of this feature
would be likely to spend hours debugging something just because some
files weren't recompiled at the right time.
If you want to optimize the number of files compiled by "make", then
ccache is your friend. This one is safe.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply
* Re: Branches & directories
From: Jeff King @ 2011-10-03 7:34 UTC (permalink / raw)
To: Matthieu Moy
Cc: Hilco Wijbenga, Robin Rosenberg, Kyle Moffett, Michael Witten,
Junio C Hamano, Evan Shelhamer, Git Mailing List
In-Reply-To: <vpqaa9ijzt4.fsf@bauges.imag.fr>
On Mon, Oct 03, 2011 at 09:32:07AM +0200, Matthieu Moy wrote:
> Hilco Wijbenga <hilco.wijbenga@gmail.com> writes:
>
> > Yes, I meant it literally. And, no, Git could not possibly know so it
> > would have to be optional behaviour. But it's probably a lot of work
> > for (for most people) little gain.
>
> Not only little gain, but also important risk: users of this feature
> would be likely to spend hours debugging something just because some
> files weren't recompiled at the right time.
>
> If you want to optimize the number of files compiled by "make", then
> ccache is your friend. This one is safe.
Yes. Despite my previous message showing what _could_ be done, I do
think it's crazy. You should just use ccache.
Speaking of which; does anybody know of a git-aware ccache-like tool?
We already have a nice index of the sha1 of each file in the repository
(along with a stat cache showing us whether it's up-to-date or not).
Something like ccache could avoid even looking in the C files at all if
it relied on git's index.
I don't know how much speedup it would yield in practice, though.
-Peff
^ permalink raw reply
* Re: Branches & directories
From: Matthieu Moy @ 2011-10-03 7:41 UTC (permalink / raw)
To: Jeff King
Cc: Hilco Wijbenga, Robin Rosenberg, Kyle Moffett, Michael Witten,
Junio C Hamano, Evan Shelhamer, Git Mailing List
In-Reply-To: <20111003073456.GA10054@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> Speaking of which; does anybody know of a git-aware ccache-like tool?
> We already have a nice index of the sha1 of each file in the repository
> (along with a stat cache showing us whether it's up-to-date or not).
> Something like ccache could avoid even looking in the C files at all if
> it relied on git's index.
It would be a bit harder than that I think. IIRC, ccache hashes the
preprocessed file, hence it will notice if a .h file changed, even if
it's outside the project.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply
* Re: [PATCH] transport: do not allow to push over git:// protocol
From: Jeff King @ 2011-10-03 7:42 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: git
In-Reply-To: <1317432415-9459-1-git-send-email-pclouds@gmail.com>
On Sat, Oct 01, 2011 at 11:26:55AM +1000, Nguyen Thai Ngoc Duy wrote:
> This protocol has never been designed for pushing. Attempts to push
> over git:// usually result in
>
> fatal: The remote end hung up unexpectedly
>
> That message does not really point out the reason. With this patch, we get
>
> error: this protocol does not support pushing
> error: failed to push some refs to 'git://some-host.com/my/repo'
I thought pushing over git:// _is_ supported. It's just that most
servers don't have it turned on, for the obvious lack-of-authentication
reasons.
See 4b3b1e1 (git-push through git protocol, 2007-01-21), and the
discussion here:
http://thread.gmane.org/gmane.comp.version-control.git/37325
Your patch shuts it off at the client level, so even with it turned on
for the server, the client can never get to it.
I still think push-over-git:// is a bit insane, and especially now with
smart-http, you'd be crazy to run it. And in that sense, I wouldn't mind
seeing it deprecated. But just shutting it off without a deprecation
period seems unnecessarily harsh.
The real problem here seems to be that instead of communicating "no, we
don't support that", git-daemon just hangs up. It would be a much nicer
fix if we could change that. I'm not sure it's possible, though. There's
not much room in the beginning of the room to make that communication in
a way that's backwards compatible.
-Peff
^ permalink raw reply
* Re: Branches & directories
From: Jeff King @ 2011-10-03 7:44 UTC (permalink / raw)
To: Matthieu Moy
Cc: Hilco Wijbenga, Robin Rosenberg, Kyle Moffett, Michael Witten,
Junio C Hamano, Evan Shelhamer, Git Mailing List
In-Reply-To: <vpqmxdiikt2.fsf@bauges.imag.fr>
On Mon, Oct 03, 2011 at 09:41:29AM +0200, Matthieu Moy wrote:
> Jeff King <peff@peff.net> writes:
>
> > Speaking of which; does anybody know of a git-aware ccache-like tool?
> > We already have a nice index of the sha1 of each file in the repository
> > (along with a stat cache showing us whether it's up-to-date or not).
> > Something like ccache could avoid even looking in the C files at all if
> > it relied on git's index.
>
> It would be a bit harder than that I think. IIRC, ccache hashes the
> preprocessed file, hence it will notice if a .h file changed, even if
> it's outside the project.
Yeah, you'd have to maintain your own dependency tree, then. Which is
nasty (aside from the work involved), because I don't think you can
portably get the header dependencies out of the C compiler.
Oh well.
-Peff
^ permalink raw reply
* Re: [PATCH v2] ident: check /etc/mailname if email is unknown
From: Jonathan Nieder @ 2011-10-03 7:44 UTC (permalink / raw)
To: Johannes Sixt
Cc: Junio C Hamano, git, Matt Kraai, Gerrit Pape, Ian Jackson,
Linus Torvalds
In-Reply-To: <4E895FBD.8020904@viscovery.net>
Johannes Sixt wrote:
> This warns on EACCES. Is that OK? (Just asking, I have no opinion.)
Good catch. I was worried for a moment: could some command call
copy_email() in a loop, producing an annoyingly redundant stream of
warnings?
Luckily setup_ident() checks if git_default_email has been set to save
the trouble of computing it again. So the behavior is to warn exactly
once when using a command that uses an ident string, which is still
more often than ideal. It would be better to only warn if the
permissions are creating an actual problem, so the user can (1)
complain to the sysadmin and then (2) set an email address in
~/.gitconfig and move on.
We _could_ add parameters to setup_ident() to only warn if
/etc/mailname was going to be used to produce the committer ident (or
whichever ident is checked first). That would be confusing if
GIT_COMMITTER_EMAIL is set and GIT_AUTHOR_EMAIL is not.
In the long term it would be nice to find a way to warn when the
mailname we tried to retrieve was actually going to be used, but short
of that, the least confusing behavior is to just not warn at all.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
ident.c | 10 +---------
1 files changed, 1 insertions(+), 9 deletions(-)
diff --git a/ident.c b/ident.c
index edb43144..6f5c885d 100644
--- a/ident.c
+++ b/ident.c
@@ -55,16 +55,9 @@ static int add_mailname_host(char *buf, size_t len)
FILE *mailname;
mailname = fopen("/etc/mailname", "r");
- if (!mailname) {
- if (errno != ENOENT)
- warning("cannot open /etc/mailname: %s",
- strerror(errno));
+ if (!mailname)
return -1;
- }
if (!fgets(buf, len, mailname)) {
- if (ferror(mailname))
- warning("cannot read /etc/mailname: %s",
- strerror(errno));
fclose(mailname);
return -1;
}
@@ -80,7 +73,6 @@ static void add_domainname(char *buf, size_t len)
const char *domainname;
if (gethostname(buf, len)) {
- warning("cannot get host name: %s", strerror(errno));
strlcpy(buf, "(none)", len);
return;
}
--
1.7.7.rc1
^ permalink raw reply related
* Re: Branches & directories
From: Junio C Hamano @ 2011-10-03 7:48 UTC (permalink / raw)
To: Jeff King
Cc: Matthieu Moy, Hilco Wijbenga, Robin Rosenberg, Kyle Moffett,
Michael Witten, Evan Shelhamer, Git Mailing List
In-Reply-To: <20111003074412.GC9455@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> Yeah, you'd have to maintain your own dependency tree, then. Which is
> nasty (aside from the work involved), because I don't think you can
> portably get the header dependencies out of the C compiler.
Heh, but doesn't your Makefile know the header dependencies anyway?
^ permalink raw reply
* Re: Branches & directories
From: Jeff King @ 2011-10-03 7:51 UTC (permalink / raw)
To: Junio C Hamano
Cc: Matthieu Moy, Hilco Wijbenga, Robin Rosenberg, Kyle Moffett,
Michael Witten, Evan Shelhamer, Git Mailing List
In-Reply-To: <7v4nzqikhg.fsf@alter.siamese.dyndns.org>
On Mon, Oct 03, 2011 at 12:48:27AM -0700, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > Yeah, you'd have to maintain your own dependency tree, then. Which is
> > nasty (aside from the work involved), because I don't think you can
> > portably get the header dependencies out of the C compiler.
>
> Heh, but doesn't your Makefile know the header dependencies anyway?
Sort of. It's often wrong (e.g., we are way over-inclusive in the git
Makefile; one of the things I like about ccache is that even when our
Makefile is overly conservative, ccache is fast. Or at least faster than
actually recompiling).
And of course it doesn't generally involve headers outside of your
project, whereas ccache does recompile if those change.
-Peff
^ permalink raw reply
* Re: [PATCH] transport: do not allow to push over git:// protocol
From: Johannes Sixt @ 2011-10-03 8:44 UTC (permalink / raw)
To: Jeff King; +Cc: Nguyễn Thái Ngọc Duy, git
In-Reply-To: <20111003074250.GB9455@sigill.intra.peff.net>
Am 10/3/2011 9:42, schrieb Jeff King:
> I still think push-over-git:// is a bit insane, and especially now with
> smart-http, you'd be crazy to run it. And in that sense, I wouldn't mind
> seeing it deprecated.
You must be kidding ;) It is so much easier to type
git daemon --export-all --enable=receive-pack
for a one-shot, temporary git connection compared to setting up a
smart-http, ssh, or even a rsh server.
-- Hannes
^ permalink raw reply
* Symbolic references updated by fetch?
From: Mattias Jiderhamn @ 2011-10-03 8:47 UTC (permalink / raw)
To: git
In our current CVS setup, we have two separate builds in Jenkins CI; one for the
latest and greatest (head of "master" branch) and one for the last/current minor
release of the latest major release. The revisions to include in the minor
release build will be tagged with a tag we can call "next-minor-release" here.
Individual files are branched as needed and the "next-minor-release" tag is
moved onto the branch. The Continuous Integration job will fetch and build the
"next-minor-release" tag. As part of a major release, the "next-minor-release"
tag is moved to the head of the main branch again.
When moving to GIT, the natural thing will be a hotfix branch originating from
each major release. In order to have a fixed name for the current hotfix branch,
primarily for the CI but also simplifying for developers working with hotfixes,
it seems git symbolic-ref will do the trick. After the first major release we
can do something like this in our bare repository whereto developers push and
where from Jenkins pulls code to build:
git symbolic-ref refs/heads/current-hotfix-branch refs/heads/release-1-hotfixes
and after the next major release, we simply move the referece pointer, as such
git symbolic-ref refs/heads/current-hotfix-branch refs/heads/release-2-hotfixes
HOWEVER this seems to require that everyone fetching/pulling from that repo -
Jenkins included - delete their local "current-hotfix-branch" tracking branch,
and then refetch it.
Is there an easier way to solve the CI problem, eliminating the need to
explicitly deleting the tracking branch on all remote repos every time the
symbolic ref is moved i.e. at every major release?
How have others solved this? Do you simply reconfigure Jenkins every time...?
Thanks in advance.
^ permalink raw reply
* Re: [PATCH] transport: do not allow to push over git:// protocol
From: Nguyen Thai Ngoc Duy @ 2011-10-03 9:12 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Ilari Liusvaara, git
In-Reply-To: <20111001052910.GA6502@elie>
2011/10/1 Jonathan Nieder <jrnieder@gmail.com>:
> Ilari Liusvaara wrote:
>
>> What about sticking code to return an error to git daemon instead of this?
>
> The code has even been written:
> http://thread.gmane.org/gmane.comp.version-control.git/145456/focus=145573
>
> Testing and other improvements would be very welcome.
Tests aside, are there any problems with the patch? I don't see any
followup discussions. Personally I don't see much value in adding the
description though.
--
Duy
^ permalink raw reply
* Re: [PATCH] transport: do not allow to push over git:// protocol
From: Jeff King @ 2011-10-03 9:39 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Nguyễn Thái Ngọc Duy, git
In-Reply-To: <4E8975E7.2040804@viscovery.net>
On Mon, Oct 03, 2011 at 10:44:23AM +0200, Johannes Sixt wrote:
> Am 10/3/2011 9:42, schrieb Jeff King:
> > I still think push-over-git:// is a bit insane, and especially now with
> > smart-http, you'd be crazy to run it. And in that sense, I wouldn't mind
> > seeing it deprecated.
>
> You must be kidding ;) It is so much easier to type
>
> git daemon --export-all --enable=receive-pack
>
> for a one-shot, temporary git connection compared to setting up a
> smart-http, ssh, or even a rsh server.
Ah, yeah, I didn't think about one-shot invocations like that (I think
the original motivation was somebody actually running it all the time).
So yeah, that makes it even worse for the client to start refusing this
without even contacting the server. I forgot that we added the "ERR"
response way back in a807328 (connect.c: add a way for git-daemon to
pass an error back to client, 2008-11-01).
GitHub uses it to make nice messages:
$ git push origin
fatal: remote error:
You can't push to git://github.com/gitster/git.git
Use git@github.com:gitster/git.git
We should maybe do something like the patch below:
diff --git a/daemon.c b/daemon.c
index 4c8346d..c1fa55f 100644
--- a/daemon.c
+++ b/daemon.c
@@ -255,6 +255,7 @@ static int run_service(char *dir, struct daemon_service *service)
loginfo("Request %s for '%s'", service->name, dir);
if (!enabled && !service->overridable) {
+ packet_write(1, "ERR %s: service not enabled", service->name);
logerror("'%s': service not enabled.", service->name);
errno = EACCES;
return -1;
@@ -288,6 +289,8 @@ static int run_service(char *dir, struct daemon_service *service)
enabled = service_enabled;
}
if (!enabled) {
+ packet_write(1, "ERR %s: service not enabled for '%s'",
+ service->name, path);
logerror("'%s': service not enabled for '%s'",
service->name, path);
errno = EACCES;
but:
1. There is some information leakage there. In particular, one can
tell the difference now between "repo does not exist" and
"receive-pack is not turned on". Personally, I think the tradeoff
to have actual error messages is worth it. HTTP has had real error
codes for decades, and I don't think anybody is too up-in-arms that
I can probe which pages are 404, and which are 401.
2. It probably makes sense to have a more human-friendly error
message.
3. It may be worth adding error messages for lots of other conditions
(e.g., no such repo). Assuming we accept the information leakage
for (1).
-Peff
^ permalink raw reply related
* Re: [PATCH] transport: do not allow to push over git:// protocol
From: Nguyen Thai Ngoc Duy @ 2011-10-03 9:44 UTC (permalink / raw)
To: Jeff King; +Cc: Johannes Sixt, git, Jonathan Nieder
In-Reply-To: <20111003093912.GA16078@sigill.intra.peff.net>
2011/10/3 Jeff King <peff@peff.net>:
> So yeah, that makes it even worse for the client to start refusing this
> without even contacting the server. I forgot that we added the "ERR"
> response way back in a807328 (connect.c: add a way for git-daemon to
> pass an error back to client, 2008-11-01).
>
> GitHub uses it to make nice messages:
>
> $ git push origin
> fatal: remote error:
> You can't push to git://github.com/gitster/git.git
> Use git@github.com:gitster/git.git
>
> We should maybe do something like the patch below:
Jonathan also mentions another patch
http://article.gmane.org/gmane.comp.version-control.git/182536
> but:
>
> 1. There is some information leakage there. In particular, one can
> tell the difference now between "repo does not exist" and
> "receive-pack is not turned on". Personally, I think the tradeoff
> to have actual error messages is worth it. HTTP has had real error
> codes for decades, and I don't think anybody is too up-in-arms that
> I can probe which pages are 404, and which are 401.
To me, just "<service>: access denied" is enough. Not particularly
friendly but should be a good enough clue.
--
Duy
^ permalink raw reply
* Re: [PATCH] transport: do not allow to push over git:// protocol
From: Jeff King @ 2011-10-03 9:47 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy; +Cc: Johannes Sixt, git, Jonathan Nieder
In-Reply-To: <CACsJy8B7Z-fT+ED=4F-Ug-bhvCagSxr0X6vZqn5PGRfB7KnUTA@mail.gmail.com>
On Mon, Oct 03, 2011 at 08:44:22PM +1100, Nguyen Thai Ngoc Duy wrote:
> > GitHub uses it to make nice messages:
> >
> > $ git push origin
> > fatal: remote error:
> > You can't push to git://github.com/gitster/git.git
> > Use git@github.com:gitster/git.git
> >
> > We should maybe do something like the patch below:
>
> Jonathan also mentions another patch
>
> http://article.gmane.org/gmane.comp.version-control.git/182536
Yeah, I was just reading that. Sorry, I should have read the rest of the
thread more carefully. :)
> > 1. There is some information leakage there. In particular, one can
> > tell the difference now between "repo does not exist" and
> > "receive-pack is not turned on". Personally, I think the tradeoff
> > to have actual error messages is worth it. HTTP has had real error
> > codes for decades, and I don't think anybody is too up-in-arms that
> > I can probe which pages are 404, and which are 401.
>
> To me, just "<service>: access denied" is enough. Not particularly
> friendly but should be a good enough clue.
Yeah, maybe. Certainly it's better than "the remote end hung up
unexpectedly".
However, the leakage is still there. You would get "the remote hung up"
for no-such-repo, and "access denied" for this. Or were you just
proposing that _all_ errors give "access denied". Certainly it's better
than just hanging up, too, and there is no leakage there.
It might be nice to default to that, and let sites easily enable
friendlier messages, though.
-Peff
^ permalink raw reply
* Re: [PATCH] transport: do not allow to push over git:// protocol
From: Jakub Narebski @ 2011-10-03 9:49 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Jeff King, Nguyễn Thái Ngọc Duy, git
In-Reply-To: <4E8975E7.2040804@viscovery.net>
Johannes Sixt <j.sixt@viscovery.net> writes:
> Am 10/3/2011 9:42, schrieb Jeff King:
> > I still think push-over-git:// is a bit insane, and especially now with
> > smart-http, you'd be crazy to run it. And in that sense, I wouldn't mind
> > seeing it deprecated.
>
> You must be kidding ;) It is so much easier to type
>
> git daemon --export-all --enable=receive-pack
>
> for a one-shot, temporary git connection compared to setting up a
> smart-http, ssh, or even a rsh server.
I wonder if that is the case... but 48% responders of "Git User's
Survey 2011" (3424 out of 7100 responders who answered queston
"23) How do you publish/propagate your changes?") answered that they
use push via git protocol.
See https://www.survs.com/results/Q5CA9SKQ/P7DE07F0PL
--
Jakub Narębski
^ permalink raw reply
* Re: [PATCH] transport: do not allow to push over git:// protocol
From: Nguyen Thai Ngoc Duy @ 2011-10-03 9:52 UTC (permalink / raw)
To: Jeff King; +Cc: Johannes Sixt, git, Jonathan Nieder
In-Reply-To: <20111003094730.GA21610@sigill.intra.peff.net>
On Mon, Oct 3, 2011 at 8:47 PM, Jeff King <peff@peff.net> wrote:
>> To me, just "<service>: access denied" is enough. Not particularly
>> friendly but should be a good enough clue.
>
> Yeah, maybe. Certainly it's better than "the remote end hung up
> unexpectedly".
>
> However, the leakage is still there. You would get "the remote hung up"
> for no-such-repo, and "access denied" for this. Or were you just
> proposing that _all_ errors give "access denied". Certainly it's better
> than just hanging up, too, and there is no leakage there.
All of them. At least it's good to know my request has reached (and
rejected by) the server, not dropped on the floor by some random
firewall along the line.
> It might be nice to default to that, and let sites easily enable
> friendlier messages, though.
I'm thinking of passing "verbose" option back to server to get more
helpful messages, the option would be turned off by default. It's up
to admin to decide (would be actually helpful during deployment test,
for example). Or is it possible already?
--
Duy
^ permalink raw reply
* Re: [PATCHv3] git-web--browse: avoid the use of eval
From: Jeff King @ 2011-10-03 9:57 UTC (permalink / raw)
To: Chris Packham; +Cc: git, gitster, chriscool
In-Reply-To: <1317516257-24435-1-git-send-email-judge.packham@gmail.com>
On Sun, Oct 02, 2011 at 01:44:17PM +1300, Chris Packham wrote:
> Using eval causes problems when the URL contains an appropriately
> escaped ampersand (\&). Dropping eval from the built-in browser
> invocation avoids the problem.
>
> Helped-by: Jeff King <peff@peff.net> (test case)
> Signed-off-by: Chris Packham <judge.packham@gmail.com>
>
> ---
> The consensus from the last round of discussion [1] seemed to be to
> remove the eval from the built in browsers but quote custom browser
> commands appropriately.
>
> I've expanded the tests a little. A semi-colon had the same error as
> the ampersand. A hash was another common character that had meaning in
> a shell and in URL.
This looks good to me. I think we may want to squash in the two tests
below, too, which make sure we treat $browser_path and $browser_cmd
appropriately (the former is a filename, and the latter is a shell
snippet).
diff --git a/t/t9901-git-web--browse.sh b/t/t9901-git-web--browse.sh
index c6f48a9..7906e5d 100755
--- a/t/t9901-git-web--browse.sh
+++ b/t/t9901-git-web--browse.sh
@@ -34,4 +34,33 @@ test_expect_success \
test_cmp expect actual
'
+test_expect_success \
+ 'browser paths are properly quoted' '
+ echo fake: http://example.com/foo >expect &&
+ cat >"fake browser" <<-\EOF &&
+ #!/bin/sh
+ echo fake: "$@"
+ EOF
+ chmod +x "fake browser" &&
+ git config browser.w3m.path "`pwd`/fake browser" &&
+ git web--browse --browser=w3m \
+ http://example.com/foo >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success \
+ 'browser command allows arbitrary shell code' '
+ echo "arg: http://example.com/foo" >expect &&
+ git config browser.custom.cmd "
+ f() {
+ for i in \"\$@\"; do
+ echo arg: \$i
+ done
+ }
+ f" &&
+ git web--browse --browser=custom \
+ http://example.com/foo >actual &&
+ test_cmp expect actual
+'
+
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