* Re: [PATCH 2/2] Documentation: filter-branch env-filter example
From: Junio C Hamano @ 2013-02-21 20:49 UTC (permalink / raw)
To: Tadeusz Andrzej Kadłubowski; +Cc: git
In-Reply-To: <5126824A.2060903@hell.org.pl>
Tadeusz Andrzej Kadłubowski <yess@hell.org.pl> writes:
> filter-branch --env-filter example that shows how to change the email
> address in all commits before publishing a project.
>
> Signed-off-by: Tadeusz Andrzej Kadłubowski <yess@hell.org.pl>
> ---
Assuming that the result formats well both as html and manpage, the
added example looks good to me. I somehow suspect that there is a
patch-mangling going on, though.
> Documentation/git-filter-branch.txt | 21 +++++++++++++++++++++
> 1 file changed, 21 insertions(+)
>
> diff --git a/Documentation/git-filter-branch.txt b/Documentation/git-filter-branch.txt
> index e50ee2f..660bd32 100644
> --- a/Documentation/git-filter-branch.txt
> +++ b/Documentation/git-filter-branch.txt
> @@ -329,6 +329,27 @@ git filter-branch --msg-filter '
> ' HEAD~10..HEAD
> --------------------------------------------------------
> +The `--env-filter` option can be used to modify committer and/or author
> +identity. For example, if you found out that your commits have the wrong
> +identity due to a misconfigured user.email, you can make a correction,
> +before publishing the project, like this:
> +
> +
> +--------------------------------------------------------
> +git filter-branch --env-filter '
> + if test "$GIT_AUTHOR_EMAIL" = "root@localhost"
> + then
> + GIT_AUTHOR_EMAIL=john@example.com
> + export GIT_AUTHOR_EMAIL
> + fi
> + if test "$GIT_COMMITTER_EMAIL" = "root@localhost"
> + then
> + GIT_COMMITTER_EMAIL=john@example.com
> + export GIT_COMMITTER_EMAIL
> + fi
> +' -- --all
> +--------------------------------------------------------
> +
> To restrict rewriting to only part of the history, specify a revision
> range in addition to the new branch name. The new branch name will
> point to the top-most revision that a 'git rev-list' of this range
^ permalink raw reply
* Re: [PATCH v4.1 09/12] sequencer.c: teach append_signoff to avoid adding a duplicate newline
From: Junio C Hamano @ 2013-02-21 21:27 UTC (permalink / raw)
To: Brandon Casey; +Cc: git, pclouds, jrnieder, Brandon Casey
In-Reply-To: <CA+sFfMcNWvPKuQpNWnacegbfgN0ZP=zfuDPDRkXs1G2FMrb+iA@mail.gmail.com>
Brandon Casey <drafnel@gmail.com> writes:
> Yes. The fix described by John Keeping restores the above behavior
> for 'commit -s'. Or the fix I described which inserts two preceding
> newlines so it looks like this:
>
> ----------------------------------------------------------------
>
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ----------------------------------------------------------------
>
> So then the cursor would be placed on the first line and a space would
> separate it from the sob which is arguably a better indication to the
> user that a blank line should separate the commit message body from
> the sob.
That sounds like an improvement to me.
> But, this does not fix the same problem for 'cherry-pick --edit -s'
> when used to cherry-pick a commit without a sob. ...
> Using 'cherry-pick --edit -s' to cherry-pick a commit with an empty
> commit message is going to be a pretty rare corner case....
We actively discourage an empty commit message by requiring users to
say "commit --allow-empty-message". I think it is in line with the
philosophy for a Porcelain command "git cherry-pick -s" to punish
users by making them work harder to use a commit with an empty
message ;-).
^ permalink raw reply
* [PATCH] Fix in Git.pm cat_blob crashes on large files
From: Joshua Clayton @ 2013-02-21 22:13 UTC (permalink / raw)
To: git
Greetings.
This is my first patch here. Hopefully I get the stylistic & political
details right... :)
Patch applies against maint and master
(If I understand the mechanics, in theory a negative offset should work,
if the values lined up just right, but would be very wrong, overwriting the
lower contents of the file)
Developer's Certificate of Origin 1.1
By making a contribution to this project, I certify that:
(a) The contribution was created in whole or in part by me and I
have the right to submit it under the open source license
indicated in the file; or
(b) The contribution is based upon previous work that, to the best
of my knowledge, is covered under an appropriate open source
license and I have the right under that license to submit that
work with modifications, whether created in whole or in part
by me, under the same open source license (unless I am
permitted to submit under a different license), as indicated
in the file; or
(c) The contribution was provided directly to me by some other
person who certified (a), (b) or (c) and I have not modified
it.
(d) I understand and agree that this project and the contribution
are public and that a record of the contribution (including all
personal information I submit with it, including my sign-off) is
maintained indefinitely and may be redistributed consistent with
this project or the open source license(s) involved.
Affects git svn clone/fetch
Original code loaded entire file contents into a variable
before writing to disk. If the offset within the variable passed
2 GiB, it becrame negative, resulting in a crash.
On a 32 bit system, or a system with low memory it may crash before
reaching 2 GiB due to memory exhaustion.
Fix writes in smaller 64K increments. Tested to work on git svn fetch
Signed-off-by: Joshua Clayton <stillcompiling@gmail.com>
---
perl/Git.pm | 19 ++++++++++++-------
1 file changed, 12 insertions(+), 7 deletions(-)
diff --git a/perl/Git.pm b/perl/Git.pm
index 931047c..e55840f 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -942,6 +942,8 @@ sub cat_blob {
my $size = $1;
my $blob;
+ my $blobSize = 0;
+ my $flushSize = 1024*64;
my $bytesRead = 0;
while (1) {
@@ -949,13 +951,21 @@ sub cat_blob {
last unless $bytesLeft;
my $bytesToRead = $bytesLeft < 1024 ? $bytesLeft : 1024;
- my $read = read($in, $blob, $bytesToRead, $bytesRead);
+ my $read = read($in, $blob, $bytesToRead, $blobSize);
unless (defined($read)) {
$self->_close_cat_blob();
throw Error::Simple("in pipe went bad");
}
-
$bytesRead += $read;
+ $blobSize += $read;
+ if (($blobSize >= $flushSize) || ($bytesLeft <= 1024)) {
+ unless (print $fh $blob) {
+ $self->_close_cat_blob();
+ throw Error::Simple("couldn't write to passed in filehandle");
+ }
+ $blob = "";
+ $blobSize = 0;
+ }
}
# Skip past the trailing newline.
@@ -970,11 +980,6 @@ sub cat_blob {
throw Error::Simple("didn't find newline after blob");
}
- unless (print $fh $blob) {
- $self->_close_cat_blob();
- throw Error::Simple("couldn't write to passed in filehandle");
- }
-
return $size;
}
--
1.7.10.4
^ permalink raw reply related
* Re: [PATCH] Fix in Git.pm cat_blob crashes on large files
From: Jeff King @ 2013-02-21 22:43 UTC (permalink / raw)
To: Joshua Clayton; +Cc: git
In-Reply-To: <CAMB+bfKYLjmDavcLaO7scBPfTLmzqAmH+k9uBj0WJ+dzj9vuyA@mail.gmail.com>
On Thu, Feb 21, 2013 at 02:13:32PM -0800, Joshua Clayton wrote:
> Greetings.
> This is my first patch here. Hopefully I get the stylistic & political
> details right... :)
> Patch applies against maint and master
I have some comments. :)
The body of your email should contain the commit message (i.e., whatever
people reading "git log" a year from now would see). Cover letter bits
like this should go after the "---". That way "git am" knows which part
is which.
> Developer's Certificate of Origin 1.1
You don't need to include the DCO. Your "Signed-off-by" is an indication
that you agree to it.
> Affects git svn clone/fetch
> Original code loaded entire file contents into a variable
> before writing to disk. If the offset within the variable passed
> 2 GiB, it becrame negative, resulting in a crash.
Interesting. I didn't think perl had signed wrap-around issues like
this, as its numeric variables are not strictly integers. But I don't
have a 32-bit machine to test on (and numbers larger than 2G obviously
work on 64-bit machines). At any rate, though:
> On a 32 bit system, or a system with low memory it may crash before
> reaching 2 GiB due to memory exhaustion.
Yeah, it is stupid to read the whole thing into memory if we are just
going to dump it to another filehandle.
> @@ -949,13 +951,21 @@ sub cat_blob {
> last unless $bytesLeft;
>
> my $bytesToRead = $bytesLeft < 1024 ? $bytesLeft : 1024;
> - my $read = read($in, $blob, $bytesToRead, $bytesRead);
> + my $read = read($in, $blob, $bytesToRead, $blobSize);
> unless (defined($read)) {
> $self->_close_cat_blob();
> throw Error::Simple("in pipe went bad");
> }
Hmph. The existing code already reads in 1024-byte chunks. For no
reason, as far as I can tell, since we are just loading the blob buffer
incrementally into memory, only to then flush it all out at once.
Why do you read at the $blobSize offset? If we are just reading in
chunks, we be able to just keep writing to the start of our small
buffer, as we flush each chunk out before trying to read more.
IOW, shouldn't the final code look like this:
my $bytesLeft = $size;
while ($bytesLeft > 0) {
my $buf;
my $bytesToRead = $bytesLeft < 1024 ? $bytesLeft : 1024;
my $read = read($in, $buf, $bytesToRead);
unless (defined($read)) {
$self->_close_cat_blob();
throw Error::Simple("unable to read cat-blob pipe");
}
unless (print $fh $buf) {
$self->_close_cat_blob();
throw Error::Simple("unable to write blob output");
}
$bytesLeft -= $read;
}
By having the read and flush size be the same, it's much simpler.
Your change (and my proposed code) do mean that an error during the read
operation will result in a truncated output file, rather than an empty
one. I think that is OK, though. That can happen anyway in the original
due to a failure in the "print" step. Any caller who wants to be careful
that they leave only a full file in place must either:
1. Check the return value of cat_blob and verify that the result has
$size bytes, and otherwise delete it.
2. Write to a temporary file, then once success has been returned from
cat_blob, rename the result into place.
Neither of which is affected by this change.
-Peff
^ permalink raw reply
* Re: [RFC] Provide a mechanism to turn off symlink resolution in ceiling paths
From: Junio C Hamano @ 2013-02-21 22:53 UTC (permalink / raw)
To: Michael Haggerty
Cc: git, Anders Kaseorg, David Aguilar, Jiang Xin, Lea Wiemann,
David Reiss, Johannes Sixt, Lars R. Damerow, Jeff King,
Marc Jordan
In-Reply-To: <1361351364-15479-1-git-send-email-mhagger@alum.mit.edu>
Michael Haggerty <mhagger@alum.mit.edu> writes:
> Unfortunately I am swamped with other work right now so I don't have
> time to test the code and might not be able to respond promptly to
> feedback.
A note like the above is a good way to give a cue to others so that
we can work together to pick up, tie the loose ends and move us
closer to the goal, and is very much appreciated.
I think the patch makes sense; I expanded on the part that has
Anders's report in the log message and added a trivial test.
Testing and eyeballing by others would help very much. We'd
obviously need our sign-off as well ;-)
Thanks.
-- >8 --
From: Michael Haggerty <mhagger@alum.mit.edu>
Date: Wed, 20 Feb 2013 10:09:24 +0100
Subject: [PATCH] Provide a mechanism to turn off symlink resolution in ceiling paths
Commit 1b77d83cab 'setup_git_directory_gently_1(): resolve symlinks
in ceiling paths' changed the setup code to resolve symlinks in the
entries in GIT_CEILING_DIRECTORIES. Because those entries are
compared textually to the symlink-resolved current directory, an
entry in GIT_CEILING_DIRECTORIES that contained a symlink would have
no effect. It was known that this could cause performance problems
if the symlink resolution *itself* touched slow filesystems, but it
was thought that such use cases would be unlikely. The intention of
the earlier change was to deal with a case when the user has this:
GIT_CEILING_DIRECTORIES=/home/gitster
but in reality, /home/gitster is a symbolic link to somewhere else,
e.g. /net/machine/home4/gitster. A textual comparison between the
specified value /home/gitster and the location getcwd(3) returns
would not help us, but readlink("/home/gitster") would still be
fast.
After this change was released, Anders Kaseorg <andersk@mit.edu>
reported:
> [...] my computer has been acting so slow when I’m not connected to
> the network. I put various network filesystem paths in
> $GIT_CEILING_DIRECTORIES, such as
> /afs/athena.mit.edu/user/a/n/andersk (to avoid hitting its parents
> /afs/athena.mit.edu, /afs/athena.mit.edu/user/a, and
> /afs/athena.mit.edu/user/a/n which all live in different AFS
> volumes). Now when I’m not connected to the network, every
> invocation of Git, including the __git_ps1 in my shell prompt, waits
> for AFS to timeout.
To allow users to work this around, give them a mechanism to turn
off symlink resolution in GIT_CEILING_DIRECTORIES entries. All the
entries that follow an empty entry will not be checked for symbolic
links and used literally in comparison. E.g. with these:
GIT_CEILING_DIRECTORIES=:/foo/bar:/xyzzy or
GIT_CEILING_DIRECTORIES=/foo/bar::/xyzzy
we will not readlink("/xyzzy"), and with the former, we will not
readlink("/foo/bar"), either.
---
Documentation/git.txt | 19 +++++++++++++------
setup.c | 32 ++++++++++++++++++++++----------
t/t1504-ceiling-dirs.sh | 17 +++++++++++++++++
3 files changed, 52 insertions(+), 16 deletions(-)
diff --git a/Documentation/git.txt b/Documentation/git.txt
index 6710cb0..5c03616 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -653,12 +653,19 @@ git so take care if using Cogito etc.
The '--namespace' command-line option also sets this value.
'GIT_CEILING_DIRECTORIES'::
- This should be a colon-separated list of absolute paths.
- If set, it is a list of directories that git should not chdir
- up into while looking for a repository directory.
- It will not exclude the current working directory or
- a GIT_DIR set on the command line or in the environment.
- (Useful for excluding slow-loading network directories.)
+ This should be a colon-separated list of absolute paths. If
+ set, it is a list of directories that git should not chdir up
+ into while looking for a repository directory (useful for
+ excluding slow-loading network directories). It will not
+ exclude the current working directory or a GIT_DIR set on the
+ command line or in the environment. Normally, Git has to read
+ the entries in this list are read to resolve any symlinks that
+ might be present in order to compare them with the current
+ directory. However, if even this access is slow, you
+ can add an empty entry to the list to tell Git that the
+ subsequent entries are not symlinks and needn't be resolved;
+ e.g.,
+ 'GIT_CEILING_DIRECTORIES=/maybe/symlink::/very/slow/non/symlink'.
'GIT_DISCOVERY_ACROSS_FILESYSTEM'::
When run in a directory that does not have ".git" repository
diff --git a/setup.c b/setup.c
index f108c4b..1b12017 100644
--- a/setup.c
+++ b/setup.c
@@ -624,22 +624,32 @@ static dev_t get_device_or_die(const char *path, const char *prefix, int prefix_
/*
* A "string_list_each_func_t" function that canonicalizes an entry
* from GIT_CEILING_DIRECTORIES using real_path_if_valid(), or
- * discards it if unusable.
+ * discards it if unusable. The presence of an empty entry in
+ * GIT_CEILING_DIRECTORIES turns off canonicalization for all
+ * subsequent entries.
*/
static int canonicalize_ceiling_entry(struct string_list_item *item,
- void *unused)
+ void *cb_data)
{
+ int *empty_entry_found = cb_data;
char *ceil = item->string;
- const char *real_path;
- if (!*ceil || !is_absolute_path(ceil))
+ if (!*ceil) {
+ *empty_entry_found = 1;
return 0;
- real_path = real_path_if_valid(ceil);
- if (!real_path)
+ } else if (!is_absolute_path(ceil)) {
return 0;
- free(item->string);
- item->string = xstrdup(real_path);
- return 1;
+ } else if (*empty_entry_found) {
+ /* Keep entry but do not canonicalize it */
+ return 1;
+ } else {
+ const char *real_path = real_path_if_valid(ceil);
+ if (!real_path)
+ return 0;
+ free(item->string);
+ item->string = xstrdup(real_path);
+ return 1;
+ }
}
/*
@@ -679,9 +689,11 @@ static const char *setup_git_directory_gently_1(int *nongit_ok)
return setup_explicit_git_dir(gitdirenv, cwd, len, nongit_ok);
if (env_ceiling_dirs) {
+ int empty_entry_found = 0;
+
string_list_split(&ceiling_dirs, env_ceiling_dirs, PATH_SEP, -1);
filter_string_list(&ceiling_dirs, 0,
- canonicalize_ceiling_entry, NULL);
+ canonicalize_ceiling_entry, &empty_entry_found);
ceil_offset = longest_ancestor_length(cwd, &ceiling_dirs);
string_list_clear(&ceiling_dirs, 0);
}
diff --git a/t/t1504-ceiling-dirs.sh b/t/t1504-ceiling-dirs.sh
index cce87a5..3d51615 100755
--- a/t/t1504-ceiling-dirs.sh
+++ b/t/t1504-ceiling-dirs.sh
@@ -44,6 +44,10 @@ test_prefix ceil_at_sub ""
GIT_CEILING_DIRECTORIES="$TRASH_ROOT/sub/"
test_prefix ceil_at_sub_slash ""
+if test_have_prereq SYMLINKS
+then
+ ln -s sub top
+fi
mkdir -p sub/dir || exit 1
cd sub/dir || exit 1
@@ -68,6 +72,19 @@ test_fail subdir_ceil_at_sub
GIT_CEILING_DIRECTORIES="$TRASH_ROOT/sub/"
test_fail subdir_ceil_at_sub_slash
+if test_have_prereq SYMLINKS
+then
+ GIT_CEILING_DIRECTORIES="$TRASH_ROOT/top"
+ test_fail subdir_ceil_at_top
+ GIT_CEILING_DIRECTORIES="$TRASH_ROOT/top/"
+ test_fail subdir_ceil_at_top_slash
+
+ GIT_CEILING_DIRECTORIES=":$TRASH_ROOT/top"
+ test_prefix subdir_ceil_at_top_no_resolve "sub/dir/"
+ GIT_CEILING_DIRECTORIES=":$TRASH_ROOT/top/"
+ test_prefix subdir_ceil_at_top_slash_no_resolve "sub/dir/"
+fi
+
GIT_CEILING_DIRECTORIES="$TRASH_ROOT/sub/dir"
test_prefix subdir_ceil_at_subdir "sub/dir/"
--
1.8.2.rc0.152.g52dbac4
^ permalink raw reply related
* Re: [PATCH 2/2] format-patch: --inline-single
From: Jeff King @ 2013-02-21 23:13 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git list, Adam Spiers
In-Reply-To: <7v4nh5v2fl.fsf_-_@alter.siamese.dyndns.org>
On Thu, Feb 21, 2013 at 12:26:22PM -0800, Junio C Hamano wrote:
> Some people may find it convenient to append a simple patch at the
> bottom of a discussion e-mail separated by a "scissors" mark, ready
> to be applied with "git am -c". Introduce "--inline-single" option
> to format-patch to do so. A typical usage example might be to start
> 'F'ollow-up to a discussion, write your message, conclude with "a
> patch to do so may look like this.", and
>
> \C-u M-! git format-patch --inline-single -1 HEAD <ENTER>
>
> if you are an Emacs user. Users of other MUA's may want to consult
> their manuals to find equivalent command to append output from an
> external command to the message being composed.
Interesting. I usually just do this by hand, but this could save a few
keystrokes in my workflow.
> +static int is_current_user(const struct pretty_print_context *pp,
> + const char *email, size_t emaillen,
> + const char *name, size_t namelen)
> +{
> + const char *me = git_committer_info(0);
> + const char *myname, *mymail;
> + size_t mynamelen, mymaillen;
> + struct ident_split ident;
> +
> + if (split_ident_line(&ident, me, strlen(me)))
> + return 0; /* play safe, as we do not know */
> + mymail = ident.mail_begin;
> + mymaillen = ident.mail_end - ident.mail_begin;
> + myname = ident.name_begin;
> + mynamelen = ident.name_end - ident.name_begin;
> + if (pp->mailmap)
> + map_user(pp->mailmap, &mymail, &mymaillen, &myname, &mynamelen);
> + return (mymaillen == emaillen &&
> + mynamelen == namelen &&
> + !memcmp(mymail, email, emaillen) &&
> + !memcmp(myname, name, namelen));
> +}
Nice, I'm glad you handled this case properly. I've wondered if we
should have an option to do a similar test when writing out the "real"
message format. I.e., to put the extra "From" line in the body of the
message when !is_current_user(). Traditionally we have just said "that
is the responsibility of the MUA you use", and let send-email handle it.
But it means people who do not use send-email have to reimplement the
feature themselves.
> @@ -421,6 +443,9 @@ void pp_user_info(const struct pretty_print_context *pp,
> if (pp->mailmap)
> map_user(pp->mailmap, &mailbuf, &maillen, &namebuf, &namelen);
>
> + if (pp->inline_single && is_current_user(pp, mailbuf, maillen, namebuf, namelen))
> + return;
> +
> strbuf_init(&mail, 0);
> strbuf_init(&name, 0);
This makes sense to suppress the user line when it is not necessary. But
we should probably always be suppressing the Date line, as it is almost
always useless.
I also wonder if we should suppress the subject-prefix in such a case,
as it is not adding anything (it is not the subject of the email, so it
does not need to grab attention there, and it will not make it into the
final commit). On the other hand, having tried it, the "Subject:" looks
a little lonely without it. Perhaps the [PATCH] is still necessary to
grab attention after the scissors line. I dunno.
Patch for both below if you want to pick up either suggestion.
diff --git a/log-tree.c b/log-tree.c
index 15c9749..8994354 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -348,7 +348,8 @@ void log_write_email_headers(struct rev_info *opt, struct commit *commit,
digits_in_number(opt->total),
opt->nr, opt->total);
subject = buffer;
- } else if (opt->total == 0 && opt->subject_prefix && *opt->subject_prefix) {
+ } else if (opt->total == 0 && !opt->inline_single &&
+ opt->subject_prefix && *opt->subject_prefix) {
static char buffer[256];
snprintf(buffer, sizeof(buffer),
"Subject: [%s] ",
diff --git a/pretty.c b/pretty.c
index 363b3d9..1a7352c 100644
--- a/pretty.c
+++ b/pretty.c
@@ -490,7 +490,8 @@ void pp_user_info(const struct pretty_print_context *pp,
strbuf_addf(sb, "Date: %s\n", show_date(time, tz, pp->date_mode));
break;
case CMIT_FMT_EMAIL:
- strbuf_addf(sb, "Date: %s\n", show_date(time, tz, DATE_RFC2822));
+ if (!pp->inline_single)
+ strbuf_addf(sb, "Date: %s\n", show_date(time, tz, DATE_RFC2822));
break;
case CMIT_FMT_FULLER:
strbuf_addf(sb, "%sDate: %s\n", what, show_date(time, tz, pp->date_mode));
^ permalink raw reply related
* Re: [PATCH] Fix in Git.pm cat_blob crashes on large files
From: Joshua Clayton @ 2013-02-21 23:18 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20130221224319.GA19021@sigill.intra.peff.net>
On Thu, Feb 21, 2013 at 2:43 PM, Jeff King <peff@peff.net> wrote:
> On Thu, Feb 21, 2013 at 02:13:32PM -0800, Joshua Clayton wrote:
>
>> Greetings.
>> This is my first patch here. Hopefully I get the stylistic & political
>> details right... :)
>> Patch applies against maint and master
>
> I have some comments. :)
>
> The body of your email should contain the commit message (i.e., whatever
> people reading "git log" a year from now would see). Cover letter bits
> like this should go after the "---". That way "git am" knows which part
> is which.
>
>> Developer's Certificate of Origin 1.1
>
> You don't need to include the DCO. Your "Signed-off-by" is an indication
> that you agree to it.
>
>> Affects git svn clone/fetch
>> Original code loaded entire file contents into a variable
>> before writing to disk. If the offset within the variable passed
>> 2 GiB, it becrame negative, resulting in a crash.
>
> Interesting. I didn't think perl had signed wrap-around issues like
> this, as its numeric variables are not strictly integers. But I don't
> have a 32-bit machine to test on (and numbers larger than 2G obviously
> work on 64-bit machines). At any rate, though:
>
>> On a 32 bit system, or a system with low memory it may crash before
>> reaching 2 GiB due to memory exhaustion.
>
> Yeah, it is stupid to read the whole thing into memory if we are just
> going to dump it to another filehandle.
>
>> @@ -949,13 +951,21 @@ sub cat_blob {
>> last unless $bytesLeft;
>>
>> my $bytesToRead = $bytesLeft < 1024 ? $bytesLeft : 1024;
>> - my $read = read($in, $blob, $bytesToRead, $bytesRead);
>> + my $read = read($in, $blob, $bytesToRead, $blobSize);
>> unless (defined($read)) {
>> $self->_close_cat_blob();
>> throw Error::Simple("in pipe went bad");
>> }
>
> Hmph. The existing code already reads in 1024-byte chunks. For no
> reason, as far as I can tell, since we are just loading the blob buffer
> incrementally into memory, only to then flush it all out at once.
>
> Why do you read at the $blobSize offset? If we are just reading in
> chunks, we be able to just keep writing to the start of our small
> buffer, as we flush each chunk out before trying to read more.
>
> IOW, shouldn't the final code look like this:
>
> my $bytesLeft = $size;
> while ($bytesLeft > 0) {
> my $buf;
> my $bytesToRead = $bytesLeft < 1024 ? $bytesLeft : 1024;
> my $read = read($in, $buf, $bytesToRead);
> unless (defined($read)) {
> $self->_close_cat_blob();
> throw Error::Simple("unable to read cat-blob pipe");
> }
> unless (print $fh $buf) {
> $self->_close_cat_blob();
> throw Error::Simple("unable to write blob output");
> }
>
> $bytesLeft -= $read;
> }
>
> By having the read and flush size be the same, it's much simpler.
My original bugfix did just read 1024, and write 1024. That works fine
and, yes, is simpler.
I changed it to be more similar to the original code in case there
were performance reasons for doing it that way.
That was the only reason I could think of for the design, and adding
the $flushSize variable means that
some motivated person could easily optimize it.
So far I have been too lazy to profile the two versions....
I guess I'll try a trivial git svn init; git svn fetch and check back in.
Its running now.
>
> Your change (and my proposed code) do mean that an error during the read
> operation will result in a truncated output file, rather than an empty
> one. I think that is OK, though. That can happen anyway in the original
> due to a failure in the "print" step. Any caller who wants to be careful
> that they leave only a full file in place must either:
>
> 1. Check the return value of cat_blob and verify that the result has
> $size bytes, and otherwise delete it.
>
> 2. Write to a temporary file, then once success has been returned from
> cat_blob, rename the result into place.
>
> Neither of which is affected by this change.
>
> -Peff
In git svn fetch (which is how I discovered it) the file being passed
to cat_blob is a temporary file, which is checksummed before putting
it into place.
^ permalink raw reply
* What's cooking in git.git (Feb 2013, #09; Thu, 21)
From: Junio C Hamano @ 2013-02-21 23:20 UTC (permalink / raw)
To: git
Here are the topics that have been cooking. Commits prefixed with
'-' are only in 'pu' (proposed updates) while commits prefixed with
'+' are in 'next'.
On the 'maint' front, a maintenance release 1.8.1.4 is out. The
same fixes are also included in the 'master' and upwards.
The tip of the 'master' is a bit past 1.8.2-rc0; new topics that are
not listed in this report are likely to be too late for the upcoming
release.
You can find the changes described here in the integration branches of the
repositories listed at
http://git-blame.blogspot.com/p/git-public-repositories.html
--------------------------------------------------
[New Topics]
* jk/pkt-line-cleanup (2013-02-20) 19 commits
- remote-curl: always parse incoming refs
- remote-curl: move ref-parsing code up in file
- remote-curl: pass buffer straight to get_remote_heads
- teach get_remote_heads to read from a memory buffer
- pkt-line: share buffer/descriptor reading implementation
- pkt-line: provide a LARGE_PACKET_MAX static buffer
- pkt-line: move LARGE_PACKET_MAX definition from sideband
- pkt-line: teach packet_read_line to chomp newlines
- pkt-line: provide a generic reading function with options
- pkt-line: drop safe_write function
- pkt-line: move a misplaced comment
- write_or_die: raise SIGPIPE when we get EPIPE
- upload-archive: use argv_array to store client arguments
- upload-archive: do not copy repo name
- send-pack: prefer prefixcmp over memcmp in receive_status
- fetch-pack: fix out-of-bounds buffer offset in get_ack
- upload-pack: remove packet debugging harness
- upload-pack: do not add duplicate objects to shallow list
- upload-pack: use get_sha1_hex to parse "shallow" lines
Cleans up pkt-line API, implementation and its callers to make
them more robust.
Will merge to and cook in 'next'.
* ob/imap-send-ssl-verify (2013-02-20) 1 commit
- imap-send: support Server Name Indication (RFC4366)
Correctly connect to SSL/TLS sites that serve multiple hostnames on
a single IP by including Server Name Indication in the client-hello.
Will merge to and cook in 'next'.
* jn/less-reconfigure (2013-02-20) 1 commit
(merged to 'next' on 2013-02-21 at 811e0c0)
+ Makefile: avoid infinite loop on configure.ac change
A change made on v1.8.1.x maintenance track had a nasty regression
to break the build when autoconf is used.
Will fast-track to 'master' and 'maint' (regression fix).
* jc/format-patch (2013-02-21) 2 commits
- format-patch: --inline-single
- format-patch: rename "no_inline" field
A new option to send a single patch to the standard output to be
appended at the bottom of a message. I personally have no need for
this, but it was easy enough to cobble together.
* mh/maint-ceil-absolute (2013-02-21) 1 commit
- Provide a mechanism to turn off symlink resolution in ceiling paths
An earlier workaround designed to help people who list logical
directories that will not match what getcwd(3) returns in the
GIT_CEILING_DIRECTORIES had an adverse effect when it is slow to
stat and readlink a directory component of an element listed on it.
Waiting for review. Needs sign-off.
* tk/doc-filter-branch (2013-02-21) 3 commits
- [SQUASH???] reword desc on filter-branch's use of env
- Documentation: filter-branch env-filter example
- git-filter-branch.txt: clarify ident variables usage
--------------------------------------------------
[Stalled]
* mb/gitweb-highlight-link-target (2012-12-20) 1 commit
- Highlight the link target line in Gitweb using CSS
Expecting a reroll.
$gmane/211935
* jc/add-delete-default (2012-08-13) 1 commit
- git add: notice removal of tracked paths by default
"git add dir/" updated modified files and added new files, but does
not notice removed files, which may be "Huh?" to some users. They
can of course use "git add -A dir/", but why should they?
Resurrected from graveyard, as I thought it was a worthwhile thing
to do in the longer term.
There seems to be some interest. Let's see if it results in a solid
execution of a sensible transition plan towards Git 2.0.
* mb/remote-default-nn-origin (2012-07-11) 6 commits
- Teach get_default_remote to respect remote.default.
- Test that plain "git fetch" uses remote.default when on a detached HEAD.
- Teach clone to set remote.default.
- Teach "git remote" about remote.default.
- Teach remote.c about the remote.default configuration setting.
- Rename remote.c's default_remote_name static variables.
When the user does not specify what remote to interact with, we
often attempt to use 'origin'. This can now be customized via a
configuration variable.
Expecting a reroll.
$gmane/210151
"The first remote becomes the default" bit is better done as a
separate step.
--------------------------------------------------
[Cooking]
* as/check-ignore (2013-02-19) 2 commits
(merged to 'next' on 2013-02-21 at 27927a2)
+ name-hash: allow hashing an empty string
+ t0008: document test_expect_success_multi
"git check-ignore ." segfaulted, as a function it calls deep in its
callchain took a string in the <ptr, length> form but did not stop
when given an empty string.
Will fast-track to 'master' (this is a new feature in the upcoming release).
* bc/commit-complete-lines-given-via-m-option (2013-02-19) 4 commits
(merged to 'next' on 2013-02-19 at cf622b7)
+ Documentation/git-commit.txt: rework the --cleanup section
+ git-commit: only append a newline to -m mesg if necessary
+ t7502: demonstrate breakage with a commit message with trailing newlines
+ t/t7502: compare entire commit message with what was expected
'git commit -m "$str"' when $str was already terminated with a LF
now avoids adding an extra LF to the message.
Will cook in 'next'.
* ct/autoconf-htmldir (2013-02-19) 1 commit
(merged to 'next' on 2013-02-21 at 44f127d)
+ Bugfix: undefined htmldir in config.mak.autogen
An earlier change to config.mak.autogen broke a build driven by the
./configure script when --htmldir is not specified on the command
line of ./configure.
Will fast-track to 'master' (regression fix).
* wk/user-manual (2013-02-19) 3 commits
(merged to 'next' on 2013-02-19 at dbc0eb2)
+ user-manual: Flesh out uncommitted changes and submodule updates
+ user-manual: Use request-pull to generate "please pull" text
+ user-manual: Reorganize the reroll sections, adding 'git rebase -i'
Further updates to the user manual.
Will merge to 'master'.
* wk/man-deny-current-branch-is-default-these-days (2013-02-18) 1 commit
(merged to 'next' on 2013-02-21 at e67b15b)
+ user-manual: typofix (ofthe->of the)
Will fast-track to 'master'.
* da/difftool-fixes (2013-02-20) 3 commits
- t7800: modernize tests
- t7800: update copyright notice
- difftool: silence uninitialized variable warning
Minor maintenance updates to difftool, and updates to its tests.
* nd/read-directory-recursive-optim (2013-02-17) 1 commit
(merged to 'next' on 2013-02-17 at 36ba9f4)
+ read_directory: avoid invoking exclude machinery on tracked files
"git status" has been optimized by taking advantage of the fact
that paths that are already known to the index do not have to be
checked against the .gitignore mechanism under some conditions.
Will cook in 'next'.
* mg/gpg-interface-using-status (2013-02-14) 5 commits
- pretty: make %GK output the signing key for signed commits
- pretty: parse the gpg status lines rather than the output
- gpg_interface: allow to request status return
- log-tree: rely upon the check in the gpg_interface
- gpg-interface: check good signature in a reliable way
Call "gpg" using the right API when validating the signature on
tags.
* jn/shell-disable-interactive (2013-02-11) 2 commits
- shell: pay attention to exit status from 'help' command
- shell doc: emphasize purpose and security model
Expecting a reroll.
$gmane/216229
* jc/fetch-raw-sha1 (2013-02-07) 4 commits
(merged to 'next' on 2013-02-14 at ffa3c65)
+ fetch: fetch objects by their exact SHA-1 object names
+ upload-pack: optionally allow fetching from the tips of hidden refs
+ fetch: use struct ref to represent refs to be fetched
+ parse_fetch_refspec(): clarify the codeflow a bit
Allows requests to fetch objects at any tip of refs (including
hidden ones). It seems that there may be use cases even outside
Gerrit (e.g. $gmane/215701).
Will cook in 'next'.
* mn/send-email-works-with-credential (2013-02-12) 6 commits
- git-send-email: use git credential to obtain password
- Git.pm: add interface for git credential command
- Git.pm: allow pipes to be closed prior to calling command_close_bidi_pipe
- Git.pm: refactor command_close_bidi_pipe to use _cmd_close
- Git.pm: fix example in command_close_bidi_pipe documentation
- Git.pm: allow command_close_bidi_pipe to be called as method
Hooks the credential system to send-email.
Rerolled.
Waiting for a review.
* nd/branch-show-rebase-bisect-state (2013-02-08) 1 commit
- branch: show rebase/bisect info when possible instead of "(no branch)"
Expecting a reroll.
$gmane/215771
* nd/count-garbage (2013-02-15) 4 commits
(merged to 'next' on 2013-02-17 at b2af923)
+ count-objects: report how much disk space taken by garbage files
+ count-objects: report garbage files in pack directory too
+ sha1_file: reorder code in prepare_packed_git_one()
+ git-count-objects.txt: describe each line in -v output
Will cook in 'next'.
* tz/credential-authinfo (2013-02-05) 1 commit
- Add contrib/credentials/netrc with GPG support
A new read-only credential helper (in contrib/) to interact with
the .netrc/.authinfo files. Hopefully mn/send-email-authinfo topic
can rebuild on top of something like this.
Expecting a reroll.
$gmane/215556
* jl/submodule-deinit (2013-02-17) 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.
* jc/remove-export-from-config-mak-in (2013-02-12) 2 commits
(merged to 'next' on 2013-02-12 at eb8af04)
+ Makefile: do not export mandir/htmldir/infodir
(merged to 'next' on 2013-02-07 at 33f7d4f)
+ config.mak.in: remove unused definitions
config.mak.in template had an "export" line to cause a few
common makefile variables to be exported; if they need to be
expoted for autoconf/configure users, they should also be exported
for people who write config.mak the same way. Move the "export" to
the main Makefile. Also, stop exporting mandir that used to be
exported (only) when config.mak.autogen was used. It would have
broken installation of manpages (but not other documentation
formats).
Will cook in 'next'.
* jc/remove-treesame-parent-in-simplify-merges (2013-01-17) 1 commit
(merged to 'next' on 2013-01-30 at b639b47)
+ simplify-merges: drop merge from irrelevant side branch
The --simplify-merges logic did not cull irrelevant parents from a
merge that is otherwise not interesting with respect to the paths
we are following.
This touches a fairly core part of the revision traversal
infrastructure; even though I think this change is correct, please
report immediately if you find any unintended side effect.
Will cook in 'next'.
* jc/push-2.0-default-to-simple (2013-01-16) 14 commits
(merged to 'next' on 2013-01-16 at 23f5df2)
+ t5570: do not assume the "matching" push is the default
+ t5551: do not assume the "matching" push is the default
+ t5550: do not assume the "matching" push is the default
(merged to 'next' on 2013-01-09 at 74c3498)
+ doc: push.default is no longer "matching"
+ push: switch default from "matching" to "simple"
+ t9401: do not assume the "matching" push is the default
+ t9400: do not assume the "matching" push is the default
+ t7406: do not assume the "matching" push is the default
+ t5531: do not assume the "matching" push is the default
+ t5519: do not assume the "matching" push is the default
+ t5517: do not assume the "matching" push is the default
+ t5516: do not assume the "matching" push is the default
+ t5505: do not assume the "matching" push is the default
+ t5404: do not assume the "matching" push is the default
Will cook in 'next' until Git 2.0.
* bc/append-signed-off-by (2013-02-12) 12 commits
- Unify appending signoff in format-patch, commit and sequencer
- format-patch: update append_signoff prototype
- t4014: more tests about appending s-o-b lines
- sequencer.c: teach append_signoff to avoid adding a duplicate newline
- sequencer.c: teach append_signoff how to detect duplicate s-o-b
- sequencer.c: always separate "(cherry picked from" from commit body
- sequencer.c: require a conforming footer to be preceded by a blank line
- sequencer.c: recognize "(cherry picked from ..." as part of s-o-b footer
- t/t3511: add some tests of 'cherry-pick -s' functionality
- t/test-lib-functions.sh: allow to specify the tag name to test_commit
- commit, cherry-pick -s: remove broken support for multiline rfc2822 fields
- sequencer.c: rework search for start of footer to improve clarity
Waiting for further reviews.
$gmane/216327 may need to be addressed.
--------------------------------------------------
[Discarded]
* jc/maint-name-rev (2012-09-17) 7 commits
- describe --contains: use "name-rev --algorithm=weight"
- name-rev --algorithm=weight: tests and documentation
- name-rev --algorithm=weight: cache the computed weight in notes
- name-rev --algorithm=weight: trivial optimization
- name-rev: --algorithm option
- name_rev: clarify the logic to assign a new tip-name to a commit
- name-rev: lose unnecessary typedef
"git name-rev" names the given revision based on a ref that can be
reached in the smallest number of steps from the rev, but that is
not useful when the caller wants to know which tag is the oldest one
that contains the rev. This teaches a new mode to the command that
uses the oldest ref among those which contain the rev.
I am not sure if this is worth it; for one thing, even with the help
from notes-cache, it seems to make the "describe --contains" even
slower. Also the command will be unusably slow for a user who does
not have a write access (hence unable to create or update the
notes-cache).
Stalled mostly due to lack of responses.
* jk/smart-http-robustify (2013-02-17) 3 commits
. remote-curl: sanity check ref advertisement from server
. remote-curl: verify smart-http metadata lines
. pkt-line: teach packet_get_line a no-op mode
Parse the HTTP exchange that implements the native Git protocol as
a series of stateless RPC more carefully to diagnose protocol
breakage better.
Superseded by jk/pkt-line-cleanup
* jc/xprm-generation (2012-09-14) 1 commit
- test-generation: compute generation numbers and clock skews
A toy to analyze how bad the clock skews are in histories of real
world projects.
Stalled mostly due to lack of responses.
* jk/lua-hackery (2012-10-07) 6 commits
- pretty: fix up one-off format_commit_message calls
- Minimum compilation fixup
- Makefile: make "lua" a bit more configurable
- add a "lua" pretty format
- add basic lua infrastructure
- pretty: make some commit-parsing helpers more public
Interesting exercise. When we do this for real, we probably would want
to wrap a commit to make it more like an "object" with methods like
"parents", etc.
* rc/maint-complete-git-p4 (2012-09-24) 1 commit
- Teach git-completion about git p4
Comment from Pete will need to be addressed ($gmane/206172).
^ permalink raw reply
* Re: [PATCH] Fix in Git.pm cat_blob crashes on large files
From: Jeff King @ 2013-02-21 23:24 UTC (permalink / raw)
To: Joshua Clayton; +Cc: git
In-Reply-To: <CAMB+bf+whVFD03neCh-gBORXOBoNjgaCbfP_mh8HgDy6UqGFZA@mail.gmail.com>
On Thu, Feb 21, 2013 at 03:18:40PM -0800, Joshua Clayton wrote:
> > By having the read and flush size be the same, it's much simpler.
>
> My original bugfix did just read 1024, and write 1024. That works fine
> and, yes, is simpler.
> I changed it to be more similar to the original code in case there
> were performance reasons for doing it that way.
> That was the only reason I could think of for the design, and adding
> the $flushSize variable means that
> some motivated person could easily optimize it.
>
> So far I have been too lazy to profile the two versions....
> I guess I'll try a trivial git svn init; git svn fetch and check back in.
> Its running now.
I doubt it will make much of a difference; we are already writing to a
perl filehandle, so it will be buffered there (which I assume is 4K, but
I haven't checked). And your version retains the 1024-byte read. I do
think 1024 is quite low for this sort of descriptor-to-descriptor
copying. I'd be tempted to just bump that 1024 to 64K.
> In git svn fetch (which is how I discovered it) the file being passed
> to cat_blob is a temporary file, which is checksummed before putting
> it into place.
Great. There may be other callers outside of our tree, of course, but I
think it's pretty clear that the responsibility is on the caller to make
sure the function succeeded. We are changing what gets put on the output
stream for various error conditions, but ultimately that is an
implementation detail that the caller should not be depending on.
-Peff
^ permalink raw reply
* Re: git svn problem, probably a regression
From: RIceball LEE @ 2013-02-20 22:13 UTC (permalink / raw)
To: git
In-Reply-To: <loom.20121109T143454-821@post.gmane.org>
Joseph Crowell <joseph.w.crowell <at> gmail.com> writes:
>
> > > Use of uninitialized value $u in substitution (s///) at
> /usr/local/Cellar/git/1.8.0/lib/Git/SVN.pm
> > line 106.
> > > Use of uninitialized value $u in concatenation (.) or string at
> > /usr/local/Cellar/git/1.8.0/lib/Git/SVN.pm line 106.
> > > refs/remotes/svn/asset-manager-redesign: 'svn+ssh://<IP address>' not found
> in ''
>
> >
> > If you have time and can reproduce the bug at will, it would be very
> > helpful to use "git-bisect" to pinpoint the exact commit that causes the
> > breakage.
> >
> > -Peff
> >
>
> I just encountered the exact same issue while converting an svn repo to git on
> Windows through Git Bash. Same error.
>
>
me too:
git svn fetch
Found possible branch point:
https://amanda.svn.sourceforge.net/svnroot/amanda/amanda/branches/amanda-260 =>
https://amanda.svn.sourceforge.net/svnroot/amanda/amanda/tags/amanda260p1, 1022
Use of uninitialized value $u in substitution (s///) at
/usr/local/Cellar/git/1.8.1.3/lib/Git/SVN.pm
line 106.
Use of uninitialized value $u in concatenation (.) or string at
/usr/local/Cellar/git/1.8.1.3/lib/Git/SVN.pm line 106.
refs/remotes/svn/amanda-260: 'https://amanda.svn.sourceforge.net/svnroot/amanda'
not found
in ''
^ permalink raw reply
* Re: [PATCH v3 4/4] t7800: "defaults" is no longer a builtin tool name
From: David Aguilar @ 2013-02-21 23:31 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jonathan Nieder
In-Reply-To: <7vzjyyw9bi.fsf@alter.siamese.dyndns.org>
On Wed, Feb 20, 2013 at 9:00 PM, Junio C Hamano <gitster@pobox.com> wrote:
> David Aguilar <davvid@gmail.com> writes:
>
>> diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh
>> index fb00273..21fbba9 100755
>> --- a/t/t7800-difftool.sh
>> +++ b/t/t7800-difftool.sh
>> @@ -60,9 +60,9 @@ test_expect_success PERL 'custom commands' '
>> '
>>
>> test_expect_success PERL 'custom tool commands override built-ins' '
>> - test_config difftool.defaults.cmd "cat \"\$REMOTE\"" &&
>> + test_config difftool.vimdiff "cat \"\$REMOTE\"" &&
>> echo master >expect &&
>> - git difftool --tool defaults --no-prompt branch >actual &&
>> + git difftool --tool vimdiff --no-prompt branch >actual &&
>> test_cmp expect actual
>> '
>
> Eek.
>
> $ sh t7800-difftool.sh -i
> ok 1 - setup
> ok 2 - custom commands
> not ok 3 - custom tool commands override built-ins
> #
> # test_config difftool.vimdiff "cat \"\$REMOTE\"" &&
> # echo master >expect &&
> # git difftool --tool vimdiff --no-prompt branch >actual &&
> # test_cmp expect actual
> #
>
> Running the same test with "-v" seems to get stuck with this
> forever:
>
> expecting success:
> test_config difftool.vimdiff "cat \"\$REMOTE\"" &&
> echo master >expect &&
> git difftool --tool vimdiff --no-prompt branch >actual &&
> test_cmp expect actual
>
> Vim: Warning: Output is not to a terminal
> Vim: Warning: Input is not from a terminal
>
Oh boy. I botched the rebase. I have a replacement. I thought I
re-ran the tests! I'll resend it.
--
David
^ permalink raw reply
* [PATCH v4 4/4] t7800: "defaults" is no longer a builtin tool name
From: David Aguilar @ 2013-02-21 23:32 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jonathan Nieder
073678b8e6324a155fa99f40eee0637941a70a34 reworked the
mergetools/ directory so that every file corresponds to a
difftool-supported tool. When this happened the "defaults"
file went away as it was no longer needed by mergetool--lib.
t7800 tests that configured commands can override builtins,
but this test was not adjusted when the "defaults" file was
removed because the test continued to pass.
Adjust the test to use the everlasting "vimdiff" tool name
instead of "defaults" so that it correctly tests against a tool
that is known by mergetool--lib.
Signed-off-by: David Aguilar <davvid@gmail.com>
---
Replacement patch to fix my botched rebase.
t/t7800-difftool.sh | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh
index fb00273..3aab6e1 100755
--- a/t/t7800-difftool.sh
+++ b/t/t7800-difftool.sh
@@ -60,9 +60,9 @@ test_expect_success PERL 'custom commands' '
'
test_expect_success PERL 'custom tool commands override built-ins' '
- test_config difftool.defaults.cmd "cat \"\$REMOTE\"" &&
+ test_config difftool.vimdiff.cmd "cat \"\$REMOTE\"" &&
echo master >expect &&
- git difftool --tool defaults --no-prompt branch >actual &&
+ git difftool --tool vimdiff --no-prompt branch >actual &&
test_cmp expect actual
'
--
1.8.2.rc0.20.gf548dd7
^ permalink raw reply related
* Re: [PATCH 2/2] format-patch: --inline-single
From: Junio C Hamano @ 2013-02-21 23:41 UTC (permalink / raw)
To: Jeff King; +Cc: git list, Adam Spiers
In-Reply-To: <20130221231328.GA19808@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
>> @@ -421,6 +443,9 @@ void pp_user_info(const struct pretty_print_context *pp,
>> if (pp->mailmap)
>> map_user(pp->mailmap, &mailbuf, &maillen, &namebuf, &namelen);
>>
>> + if (pp->inline_single && is_current_user(pp, mailbuf, maillen, namebuf, namelen))
>> + return;
>> +
>> strbuf_init(&mail, 0);
>> strbuf_init(&name, 0);
>
> This makes sense to suppress the user line when it is not necessary. But
> we should probably always be suppressing the Date line, as it is almost
> always useless.
When I (figuratively) am sending my patch in a discussion, saying
"You could do it this way", on the other hand, I agree that the date
is uninteresting.
I however think I would prefer to keep the Date: line when I am
relaying somebody else's work during a discussion. It is more like
"Yeah, Peff already did that with this commit; here it is for
reference". The fact that I have _your_ patch makes it more "done",
than the case I send out my own patch.
Besides, removing an extra line in the MUA editor is far easier than
having to type what the tool "helpfully" omitted, guided by an "it
is almost always useless" that is not backed by the user preference.
I'd rather err on the side of giving extra than omitting too much.
> I also wonder if we should suppress the subject-prefix in such a case,
> as it is not adding anything (it is not the subject of the email, so it
> does not need to grab attention there, and it will not make it into the
> final commit).
If the user does not want to waste too much space in the message,
not passing the --subject-prefix=foo from the command line, or
editing it out in the editor buffer if for some reason the user ran
the command with the option, are both easy things to do. I do not
think extra lines to excise subject prefix is not worth it, and who
knows what the user's preferences are.
But there is something more important.
We should make sure that we disable MIMEy stuff (i.e. MIME-Version,
C-T-E: 8bit/quoted-printable, Content-type, etc.) when producing the
output to be appended to the body, which should be just a straight
8-bit text. I do not think the posted patch tries to do anything to
that effect.
^ permalink raw reply
* Re: [PATCH 2/2] format-patch: --inline-single
From: Junio C Hamano @ 2013-02-21 23:47 UTC (permalink / raw)
To: Jeff King; +Cc: git list, Adam Spiers
In-Reply-To: <7v7gm1teuf.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Jeff King <peff@peff.net> writes:
>
>>> @@ -421,6 +443,9 @@ void pp_user_info(const struct pretty_print_context *pp,
>>> if (pp->mailmap)
>>> map_user(pp->mailmap, &mailbuf, &maillen, &namebuf, &namelen);
>>>
>>> + if (pp->inline_single && is_current_user(pp, mailbuf, maillen, namebuf, namelen))
>>> + return;
>>> +
>>> strbuf_init(&mail, 0);
>>> strbuf_init(&name, 0);
>>
>> This makes sense to suppress the user line when it is not necessary. But
>> we should probably always be suppressing the Date line, as it is almost
>> always useless.
>
> When I (figuratively) am sending my patch in a discussion, saying
> "You could do it this way", on the other hand, I agree that the date
> is uninteresting.
Just in case somebody is wondering, please s/, on the other hand//;
above. I swapped the paragraphs after I wrote them X-<.
^ permalink raw reply
* Re: [PATCH v4 4/4] t7800: "defaults" is no longer a builtin tool name
From: Junio C Hamano @ 2013-02-21 23:49 UTC (permalink / raw)
To: David Aguilar; +Cc: git, Jonathan Nieder
In-Reply-To: <1361489572-25403-1-git-send-email-davvid@gmail.com>
Thanks.
^ permalink raw reply
* -B option of git log
From: Eckhard Maass @ 2013-02-22 0:44 UTC (permalink / raw)
To: git
Hello,
As far as I understand the documentation, -B of git-log should help
correct rename detection. But it does not seem to work for me.
Let me get a setup:
$> git init .
Initialized empty Git repository in /tmp/t2/.git/
$> echo 'Lorem ipsum doler sed. Lorem ipsum doler sed. Lorem ipsum doler
sed. Lorem ipsum doler sed.' > a
$> git add a
$> git commit -m 'Init.'
[master (root-commit) b78205c] Init.
1 file changed, 1 insertion(+)
create mode 100644 a
$> mv a b
$> echo 'new' > a
$> git add -A .
$> git commit -m '2nd.'
[master a30ca49] 2nd.
2 files changed, 2 insertions(+), 1 deletion(-)
create mode 100644 b
Now, I get the following:
$> git log --oneline -B20%/80% -M20% --name-status
a30ca49 2nd.
M a
A b
b78205c Init.
A a
But I would expect that git-log shows me a rename from a to b and a new a.
What is my misunderstanding? (And I tried it also with files with a
couple of lines.)
SEcki
^ permalink raw reply
* Re: -B option of git log
From: Jeff King @ 2013-02-22 0:57 UTC (permalink / raw)
To: Eckhard Maass; +Cc: git
In-Reply-To: <5126BF50.6020500@gmx.net>
On Fri, Feb 22, 2013 at 01:44:00AM +0100, Eckhard Maass wrote:
> Let me get a setup:
>
> $> git init .
> Initialized empty Git repository in /tmp/t2/.git/
> $> echo 'Lorem ipsum doler sed. Lorem ipsum doler sed. Lorem ipsum doler
> sed. Lorem ipsum doler sed.' > a
> $> git add a
> $> git commit -m 'Init.'
> [master (root-commit) b78205c] Init.
> 1 file changed, 1 insertion(+)
> create mode 100644 a
> $> mv a b
> $> echo 'new' > a
> $> git add -A .
> $> git commit -m '2nd.'
> [master a30ca49] 2nd.
> 2 files changed, 2 insertions(+), 1 deletion(-)
> create mode 100644 b
>
> Now, I get the following:
>
> $> git log --oneline -B20%/80% -M20% --name-status
> a30ca49 2nd.
> M a
> A b
> b78205c Init.
> A a
>
> But I would expect that git-log shows me a rename from a to b and a new a.
I think the problem is that your test file is so tiny that it falls
afoul of git's MINIMUM_BREAK_SIZE heuristic of 400 bytes (which prevents
false positives on tiny files). Try replacing your "Lorem ipsum" echo
with something like "seq 1 150", and I think you will find the result
you are expecting.
-Peff
^ permalink raw reply
* [PATCH] submodule update: when using recursion, show full path
From: Will Entriken @ 2013-02-22 4:25 UTC (permalink / raw)
To: git; +Cc: Phil Hord, Jens Lehmann, Stefan Zager
>From d3fe2c76e6fa53e4cfa6f81600685c21bdadd4e3 Mon Sep 17 00:00:00 2001
From: William Entriken <github.com@phor.net>
Date: Thu, 21 Feb 2013 23:10:07 -0500
Subject: [PATCH] submodule update: when using recursion, show full path
Previously when using update with recursion, only the path for the
inner-most module was printed. Now the path is printed from GIT_DIR.
This now matches the behavior of submodule foreach
---
First patch. Several tests failed, but they were failing before I
started. This is against maint, I would consider this a (low priority)
bug.
How does it look? Please let me know next steps.
Signed-off-by: William Entriken <github.com@phor.net>
git-submodule.sh | 31 ++++++++++++++++++-------------
1 file changed, 18 insertions(+), 13 deletions(-)
diff --git a/git-submodule.sh b/git-submodule.sh
index 2365149..f2c53c9 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -588,7 +588,7 @@ cmd_update()
die_if_unmatched "$mode"
if test "$stage" = U
then
- echo >&2 "Skipping unmerged submodule $sm_path"
+ echo >&2 "Skipping unmerged submodule $prefix$sm_path"
continue
fi
name=$(module_name "$sm_path") || exit
@@ -602,7 +602,7 @@ cmd_update()
if test "$update_module" = "none"
then
- echo "Skipping submodule '$sm_path'"
+ echo "Skipping submodule '$prefix$sm_path'"
continue
fi
@@ -611,7 +611,7 @@ cmd_update()
# Only mention uninitialized submodules when its
# path have been specified
test "$#" != "0" &&
- say "$(eval_gettext "Submodule path '\$sm_path' not initialized
+ say "$(eval_gettext "Submodule path '\$prefix\$sm_path' not initialized
Maybe you want to use 'update --init'?")"
continue
fi
@@ -624,7 +624,7 @@ Maybe you want to use 'update --init'?")"
else
subsha1=$(clear_local_git_env; cd "$sm_path" &&
git rev-parse --verify HEAD) ||
- die "$(eval_gettext "Unable to find current revision in submodule
path '\$sm_path'")"
+ die "$(eval_gettext "Unable to find current revision in submodule
path '\$prefix\$sm_path'")"
fi
if test "$subsha1" != "$sha1" -o -n "$force"
@@ -643,7 +643,7 @@ Maybe you want to use 'update --init'?")"
(clear_local_git_env; cd "$sm_path" &&
( (rev=$(git rev-list -n 1 $sha1 --not --all 2>/dev/null) &&
test -z "$rev") || git-fetch)) ||
- die "$(eval_gettext "Unable to fetch in submodule path '\$sm_path'")"
+ die "$(eval_gettext "Unable to fetch in submodule path '\$prefix\$sm_path'")"
fi
# Is this something we just cloned?
@@ -657,20 +657,20 @@ Maybe you want to use 'update --init'?")"
case "$update_module" in
rebase)
command="git rebase"
- die_msg="$(eval_gettext "Unable to rebase '\$sha1' in submodule path
'\$sm_path'")"
- say_msg="$(eval_gettext "Submodule path '\$sm_path': rebased into '\$sha1'")"
+ die_msg="$(eval_gettext "Unable to rebase '\$sha1' in submodule path
'\$prefix\$sm_path'")"
+ say_msg="$(eval_gettext "Submodule path '\$prefix\$sm_path': rebased
into '\$sha1'")"
must_die_on_failure=yes
;;
merge)
command="git merge"
- die_msg="$(eval_gettext "Unable to merge '\$sha1' in submodule path
'\$sm_path'")"
- say_msg="$(eval_gettext "Submodule path '\$sm_path': merged in '\$sha1'")"
+ die_msg="$(eval_gettext "Unable to merge '\$sha1' in submodule path
'\$prefix\$sm_path'")"
+ say_msg="$(eval_gettext "Submodule path '\$prefix\$sm_path': merged
in '\$sha1'")"
must_die_on_failure=yes
;;
*)
command="git checkout $subforce -q"
- die_msg="$(eval_gettext "Unable to checkout '\$sha1' in submodule
path '\$sm_path'")"
- say_msg="$(eval_gettext "Submodule path '\$sm_path': checked out '\$sha1'")"
+ die_msg="$(eval_gettext "Unable to checkout '\$sha1' in submodule
path '\$prefix\$sm_path'")"
+ say_msg="$(eval_gettext "Submodule path '\$prefix\$sm_path': checked
out '\$sha1'")"
;;
esac
@@ -688,11 +688,16 @@ Maybe you want to use 'update --init'?")"
if test -n "$recursive"
then
- (clear_local_git_env; cd "$sm_path" && eval cmd_update "$orig_flags")
+ (
+ prefix="$prefix$sm_path/"
+ clear_local_git_env
+ cd "$sm_path" &&
+ eval cmd_update "$orig_flags"
+ )
res=$?
if test $res -gt 0
then
- die_msg="$(eval_gettext "Failed to recurse into submodule path '\$sm_path'")"
+ die_msg="$(eval_gettext "Failed to recurse into submodule path
'\$prefix\$sm_path'")"
if test $res -eq 1
then
err="${err};$die_msg"
--
1.7.11.3
^ permalink raw reply related
* [PATCH] add: allow users to silence Git 2.0 warnings about "add -u"
From: David Aguilar @ 2013-02-22 6:07 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
When "git add -u" is invoked from a subdirectory it prints a
loud warning message about an upcoming Git 2.0 behavior change.
Some users do not care to be warned. Accomodate them.
The "add.silence-pathless-warnings" configuration variable can
now be used to silence this warning.
Signed-off-by: David Aguilar <davvid@gmail.com>
---
I found the warning a informative but also a little annoying.
I can imagine others might as well.
I would also like to change the warning message to mention what the Git 2.0
behavior will be (which it does not mention), but I realize that the string
has already been translated. That can be a follow-on patch if this is seen as
a worthwhile change, but might not be worth the trouble since it's a problem
which will go away in 2.0.
Documentation/config.txt | 7 +++++++
builtin/add.c | 8 +++++++-
t/t2200-add-update.sh | 11 +++++++++++
3 files changed, 25 insertions(+), 1 deletion(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 3bb53da..b6ed859 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -648,6 +648,13 @@ core.abbrev::
for abbreviated object names to stay unique for sufficiently long
time.
+add.silence-pathless-warnings::
+ Tells 'git add' to silence warnings when 'git add -u' is used in
+ a subdirectory without specifying a path. Git 2.0 updates the
+ whole tree. Git 1.x updates the current directory only, and warns
+ about the upcoming change unless this variable is set to true.
+ False by default, and ignored by Git 2.0.
+
add.ignore-errors::
add.ignoreErrors::
Tells 'git add' to continue adding files when some files cannot be
diff --git a/builtin/add.c b/builtin/add.c
index 0dd014e..01b9cac 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -272,6 +272,7 @@ N_("The following paths are ignored by one of your .gitignore files:\n");
static int verbose = 0, show_only = 0, ignored_too = 0, refresh_only = 0;
static int ignore_add_errors, addremove, intent_to_add, ignore_missing = 0;
+static int silence_pathless_warnings;
static struct option builtin_add_options[] = {
OPT__DRY_RUN(&show_only, N_("dry run")),
@@ -296,6 +297,8 @@ static int add_config(const char *var, const char *value, void *cb)
!strcmp(var, "add.ignore-errors")) {
ignore_add_errors = git_config_bool(var, value);
return 0;
+ } else if (!strcmp(var, "add.silence-pathless-warnings")) {
+ silence_pathless_warnings = git_config_bool(var, value);
}
return git_default_config(var, value, cb);
}
@@ -321,7 +324,8 @@ static int add_files(struct dir_struct *dir, int flags)
return exit_status;
}
-static void warn_pathless_add(const char *option_name, const char *short_name) {
+static void warn_pathless_add(const char *option_name, const char *short_name)
+{
/*
* To be consistent with "git add -p" and most Git
* commands, we should default to being tree-wide, but
@@ -332,6 +336,8 @@ static void warn_pathless_add(const char *option_name, const char *short_name) {
* turned into a die(...), and eventually we may
* reallow the command with a new behavior.
*/
+ if (silence_pathless_warnings)
+ return;
warning(_("The behavior of 'git add %s (or %s)' with no path argument from a\n"
"subdirectory of the tree will change in Git 2.0 and should not be used anymore.\n"
"To add content for the whole tree, run:\n"
diff --git a/t/t2200-add-update.sh b/t/t2200-add-update.sh
index 4cdebda..779dbe7 100755
--- a/t/t2200-add-update.sh
+++ b/t/t2200-add-update.sh
@@ -171,4 +171,15 @@ test_expect_success '"add -u non-existent" should fail' '
! (git ls-files | grep "non-existent")
'
+test_expect_success 'add.silence-pathless-warnings configuration variable' '
+ : >expect &&
+ test_config add.silence-pathless-warnings true &&
+ (
+ cd dir1 &&
+ echo more >>sub2 &&
+ git add -u
+ ) >actual 2>&1 &&
+ test_cmp expect actual
+'
+
test_done
--
1.8.2.rc0.22.gb3600c3.dirty
^ permalink raw reply related
* Re: [PATCH] add: allow users to silence Git 2.0 warnings about "add -u"
From: Junio C Hamano @ 2013-02-22 6:23 UTC (permalink / raw)
To: David Aguilar; +Cc: git, Matthieu Moy
In-Reply-To: <1361513224-34550-1-git-send-email-davvid@gmail.com>
David Aguilar <davvid@gmail.com> writes:
> When "git add -u" is invoked from a subdirectory it prints a
> loud warning message about an upcoming Git 2.0 behavior change.
> Some users do not care to be warned. Accomodate them.
I do not think this is what we discussed to do.
It was very much deliberate to make the way to "squelch the warning"
not a "set once and *forget*", aka configuration variable, but a
simple-to-type extra command line argument i.e. "git add -u .", that
you will *always* type to train your fingers to explicitly say what
you mean, so that the default switch will not matter to existing
users.
^ permalink raw reply
* Re: [PATCH] add: allow users to silence Git 2.0 warnings about "add -u"
From: David Aguilar @ 2013-02-22 7:12 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Matthieu Moy
In-Reply-To: <7vtxp4sw8e.fsf@alter.siamese.dyndns.org>
On Thu, Feb 21, 2013 at 10:23 PM, Junio C Hamano <gitster@pobox.com> wrote:
> David Aguilar <davvid@gmail.com> writes:
>
>> When "git add -u" is invoked from a subdirectory it prints a
>> loud warning message about an upcoming Git 2.0 behavior change.
>> Some users do not care to be warned. Accomodate them.
>
> I do not think this is what we discussed to do.
>
> It was very much deliberate to make the way to "squelch the warning"
> not a "set once and *forget*", aka configuration variable, but a
> simple-to-type extra command line argument i.e. "git add -u .", that
> you will *always* type to train your fingers to explicitly say what
> you mean, so that the default switch will not matter to existing
> users.
In my use case:
- The user is always in a subdir; the behavior change
is immaterial to them.
- The user does not care about these details,
and is not harmed by using the short and sweet command.
Please enlighten me.
Are we really getting rid of it and replacing it with ":/"?
That syntax looks like a meh face.. just sayin'
I was actually surprised when "add -u" didn't do the whole tree
and am happy that 2.0 will make it do the right thing...
(and perhaps I am deluded, and am not aware of what 2.0 will
do when not given pathspecs.. is it really going to die()?
that's so mean! ;)
Sorry if I am missing most of the context.
I was reading this in builtin/add.c:
/*
* To be consistent with "git add -p" and most Git
* commands, we should default to being tree-wide, but
* this is not the original behavior and can't be
* changed until users trained themselves not to type
* "git add -u" or "git add -A". For now, we warn and
* keep the old behavior. Later, this warning can be
* turned into a die(...), and eventually we may
* reallow the command with a new behavior.
*/
...and I was being too optimistic about, "and eventually".
I misread that and thought it meant that (eventually) 2.0
would default to the full tree and fix the consistency.
I didn't think that meant 2.0 would die and "git add -u"
will not be a valid syntax anymore.
Why punish these users? They are going to have :/ face.
Unlike push.default, whose warning can be silenced with configuration,
git 1.x does not have a way to silence this warning without retraining
existing users.
In other words I will have to answer an email about it one day, and I'm lazy ;-)
Another example...
$ git grep 'stuff' :/
would it be too much to teach it to do:
$ git grep -u 'stuff'
(some users are really simple...)
but in 2.0 that -u would be a no-op because "grep" will be full tree, no?
Would having that as an option and configuration be a way to allow 1.x
users to transition themselves to a 2.0 world?
I need to read the old discussions.
Can someone tell me the magic google search syntax they use to dig them up?
Would a better way be a method to make "git add -u" behave like 2.0 today?
I'm thinking of Python's "from __future__ import better_behavior" as a analog.
If full-tree is a better default then that should be the default.
Surely that's better than die(), no?
Apologies in advance as I have not read the discussions (yet).
--
David
^ permalink raw reply
* Re: [RFC] Provide a mechanism to turn off symlink resolution in ceiling paths
From: Michael Haggerty @ 2013-02-22 7:23 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Anders Kaseorg, David Aguilar, Jiang Xin, Lea Wiemann,
David Reiss, Johannes Sixt, Lars R. Damerow, Jeff King,
Marc Jordan
In-Reply-To: <7vk3q1th1l.fsf@alter.siamese.dyndns.org>
On 02/21/2013 11:53 PM, Junio C Hamano wrote:
> Michael Haggerty <mhagger@alum.mit.edu> writes:
>
>> Unfortunately I am swamped with other work right now so I don't have
>> time to test the code and might not be able to respond promptly to
>> feedback.
>
> A note like the above is a good way to give a cue to others so that
> we can work together to pick up, tie the loose ends and move us
> closer to the goal, and is very much appreciated.
>
> I think the patch makes sense; I expanded on the part that has
> Anders's report in the log message and added a trivial test.
>
> Testing and eyeballing by others would help very much. We'd
> obviously need our sign-off as well ;-)
Thanks for following up on this. Your tests look OK by eyeball and they
run successfully here whether the testing --root is under a symlink or
not. I did notice some minor niggles in the text (including one in my
original submission); see below.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Michael
> -- >8 --
> From: Michael Haggerty <mhagger@alum.mit.edu>
> Date: Wed, 20 Feb 2013 10:09:24 +0100
> Subject: [PATCH] Provide a mechanism to turn off symlink resolution in ceiling paths
>
> Commit 1b77d83cab 'setup_git_directory_gently_1(): resolve symlinks
> in ceiling paths' changed the setup code to resolve symlinks in the
> entries in GIT_CEILING_DIRECTORIES. Because those entries are
> compared textually to the symlink-resolved current directory, an
> entry in GIT_CEILING_DIRECTORIES that contained a symlink would have
> no effect. It was known that this could cause performance problems
> if the symlink resolution *itself* touched slow filesystems, but it
> was thought that such use cases would be unlikely. The intention of
> the earlier change was to deal with a case when the user has this:
>
> GIT_CEILING_DIRECTORIES=/home/gitster
>
> but in reality, /home/gitster is a symbolic link to somewhere else,
> e.g. /net/machine/home4/gitster. A textual comparison between the
> specified value /home/gitster and the location getcwd(3) returns
> would not help us, but readlink("/home/gitster") would still be
> fast.
>
> After this change was released, Anders Kaseorg <andersk@mit.edu>
> reported:
>
>> [...] my computer has been acting so slow when I’m not connected to
>> the network. I put various network filesystem paths in
>> $GIT_CEILING_DIRECTORIES, such as
>> /afs/athena.mit.edu/user/a/n/andersk (to avoid hitting its parents
>> /afs/athena.mit.edu, /afs/athena.mit.edu/user/a, and
>> /afs/athena.mit.edu/user/a/n which all live in different AFS
>> volumes). Now when I’m not connected to the network, every
>> invocation of Git, including the __git_ps1 in my shell prompt, waits
>> for AFS to timeout.
>
> To allow users to work this around, give them a mechanism to turn
s/this around/around this problem/
> off symlink resolution in GIT_CEILING_DIRECTORIES entries. All the
> entries that follow an empty entry will not be checked for symbolic
> links and used literally in comparison. E.g. with these:
Make it clear that "not" doesn't apply to both sides of the "and", since
the operator precedence in English is undocumented:
s/and/but rather will be/
>
> GIT_CEILING_DIRECTORIES=:/foo/bar:/xyzzy or
> GIT_CEILING_DIRECTORIES=/foo/bar::/xyzzy
>
> we will not readlink("/xyzzy"), and with the former, we will not
> readlink("/foo/bar"), either.
> ---
> Documentation/git.txt | 19 +++++++++++++------
> setup.c | 32 ++++++++++++++++++++++----------
> t/t1504-ceiling-dirs.sh | 17 +++++++++++++++++
> 3 files changed, 52 insertions(+), 16 deletions(-)
>
> diff --git a/Documentation/git.txt b/Documentation/git.txt
> index 6710cb0..5c03616 100644
> --- a/Documentation/git.txt
> +++ b/Documentation/git.txt
> @@ -653,12 +653,19 @@ git so take care if using Cogito etc.
> The '--namespace' command-line option also sets this value.
>
> 'GIT_CEILING_DIRECTORIES'::
> - This should be a colon-separated list of absolute paths.
> - If set, it is a list of directories that git should not chdir
> - up into while looking for a repository directory.
> - It will not exclude the current working directory or
> - a GIT_DIR set on the command line or in the environment.
> - (Useful for excluding slow-loading network directories.)
> + This should be a colon-separated list of absolute paths. If
> + set, it is a list of directories that git should not chdir up
> + into while looking for a repository directory (useful for
> + excluding slow-loading network directories). It will not
> + exclude the current working directory or a GIT_DIR set on the
> + command line or in the environment. Normally, Git has to read
> + the entries in this list are read to resolve any symlinks that
"read" is duplicated:
s/are read to//
> + might be present in order to compare them with the current
> + directory. However, if even this access is slow, you
> + can add an empty entry to the list to tell Git that the
> + subsequent entries are not symlinks and needn't be resolved;
> + e.g.,
> + 'GIT_CEILING_DIRECTORIES=/maybe/symlink::/very/slow/non/symlink'.
>
> 'GIT_DISCOVERY_ACROSS_FILESYSTEM'::
> When run in a directory that does not have ".git" repository
> diff --git a/setup.c b/setup.c
> index f108c4b..1b12017 100644
> --- a/setup.c
> +++ b/setup.c
> @@ -624,22 +624,32 @@ static dev_t get_device_or_die(const char *path, const char *prefix, int prefix_
> /*
> * A "string_list_each_func_t" function that canonicalizes an entry
> * from GIT_CEILING_DIRECTORIES using real_path_if_valid(), or
> - * discards it if unusable.
> + * discards it if unusable. The presence of an empty entry in
> + * GIT_CEILING_DIRECTORIES turns off canonicalization for all
> + * subsequent entries.
> */
> static int canonicalize_ceiling_entry(struct string_list_item *item,
> - void *unused)
> + void *cb_data)
> {
> + int *empty_entry_found = cb_data;
> char *ceil = item->string;
> - const char *real_path;
>
> - if (!*ceil || !is_absolute_path(ceil))
> + if (!*ceil) {
> + *empty_entry_found = 1;
> return 0;
> - real_path = real_path_if_valid(ceil);
> - if (!real_path)
> + } else if (!is_absolute_path(ceil)) {
> return 0;
> - free(item->string);
> - item->string = xstrdup(real_path);
> - return 1;
> + } else if (*empty_entry_found) {
> + /* Keep entry but do not canonicalize it */
> + return 1;
> + } else {
> + const char *real_path = real_path_if_valid(ceil);
> + if (!real_path)
> + return 0;
> + free(item->string);
> + item->string = xstrdup(real_path);
> + return 1;
> + }
> }
>
> /*
> @@ -679,9 +689,11 @@ static const char *setup_git_directory_gently_1(int *nongit_ok)
> return setup_explicit_git_dir(gitdirenv, cwd, len, nongit_ok);
>
> if (env_ceiling_dirs) {
> + int empty_entry_found = 0;
> +
> string_list_split(&ceiling_dirs, env_ceiling_dirs, PATH_SEP, -1);
> filter_string_list(&ceiling_dirs, 0,
> - canonicalize_ceiling_entry, NULL);
> + canonicalize_ceiling_entry, &empty_entry_found);
> ceil_offset = longest_ancestor_length(cwd, &ceiling_dirs);
> string_list_clear(&ceiling_dirs, 0);
> }
> diff --git a/t/t1504-ceiling-dirs.sh b/t/t1504-ceiling-dirs.sh
> index cce87a5..3d51615 100755
> --- a/t/t1504-ceiling-dirs.sh
> +++ b/t/t1504-ceiling-dirs.sh
> @@ -44,6 +44,10 @@ test_prefix ceil_at_sub ""
> GIT_CEILING_DIRECTORIES="$TRASH_ROOT/sub/"
> test_prefix ceil_at_sub_slash ""
>
> +if test_have_prereq SYMLINKS
> +then
> + ln -s sub top
> +fi
>
> mkdir -p sub/dir || exit 1
> cd sub/dir || exit 1
> @@ -68,6 +72,19 @@ test_fail subdir_ceil_at_sub
> GIT_CEILING_DIRECTORIES="$TRASH_ROOT/sub/"
> test_fail subdir_ceil_at_sub_slash
>
> +if test_have_prereq SYMLINKS
> +then
> + GIT_CEILING_DIRECTORIES="$TRASH_ROOT/top"
> + test_fail subdir_ceil_at_top
> + GIT_CEILING_DIRECTORIES="$TRASH_ROOT/top/"
> + test_fail subdir_ceil_at_top_slash
> +
> + GIT_CEILING_DIRECTORIES=":$TRASH_ROOT/top"
> + test_prefix subdir_ceil_at_top_no_resolve "sub/dir/"
> + GIT_CEILING_DIRECTORIES=":$TRASH_ROOT/top/"
> + test_prefix subdir_ceil_at_top_slash_no_resolve "sub/dir/"
> +fi
> +
> GIT_CEILING_DIRECTORIES="$TRASH_ROOT/sub/dir"
> test_prefix subdir_ceil_at_subdir "sub/dir/"
>
>
--
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/
^ permalink raw reply
* Re: What's cooking in git.git (Feb 2013, #05; Tue, 12)
From: Miles Bader @ 2013-02-22 8:32 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Andrew Ardill, git@vger.kernel.org
In-Reply-To: <7vvc9uwkmm.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> * Introduce "git add --ignore-removal" option in the release after
> the current cycle (a new feature is too late for this cycle):
Too late in the cycle even if the option is simply ignored ... ?
[To extend the range of git versions where it's not an error]
-miles
--
Kilt, n. A costume sometimes worn by Scotchmen [sic] in America and Americans
in Scotland.
^ permalink raw reply
* [PATCH] git-commit: populate the edit buffer with 2 blank lines before s-o-b
From: Brandon Casey @ 2013-02-22 9:25 UTC (permalink / raw)
To: gitster; +Cc: pclouds, jrnieder, john, git, Brandon Casey
In-Reply-To: <7vobfdtl1n.fsf@alter.siamese.dyndns.org>
Before commit 33f2f9ab, 'commit -s' would populate the edit buffer with
a blank line before the Signed-off-by line. This provided a nice
hint to the user that something should be filled in. Let's restore that
behavior, but now let's ensure that the Signed-off-by line is preceded
by two blank lines to hint that something should be filled in, and that
a blank line should separate it from the Signed-off-by line.
Plus, add a test for this behavior.
Reported-by: John Keeping <john@keeping.me.uk>
Signed-off-by: Brandon Casey <drafnel@gmail.com>
---
Ok. Here's a patch on top of 959a2623 bc/append-signed-off-by. It
implements the "2 blank lines preceding sob" behavior.
-Brandon
sequencer.c | 5 +++--
t/t7502-commit.sh | 12 ++++++++++++
2 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index 53ee49a..2dac106 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -1127,9 +1127,10 @@ void append_signoff(struct strbuf *msgbuf, int ignore_footer, unsigned flag)
const char *append_newlines = NULL;
size_t len = msgbuf->len - ignore_footer;
- if (len && msgbuf->buf[len - 1] != '\n')
+ /* ensure a blank line precedes our signoff */
+ if (!len || msgbuf->buf[len - 1] != '\n')
append_newlines = "\n\n";
- else if (len > 1 && msgbuf->buf[len - 2] != '\n')
+ else if (len == 1 || msgbuf->buf[len - 2] != '\n')
append_newlines = "\n";
if (append_newlines)
diff --git a/t/t7502-commit.sh b/t/t7502-commit.sh
index deb187e..a53a1e0 100755
--- a/t/t7502-commit.sh
+++ b/t/t7502-commit.sh
@@ -349,6 +349,18 @@ test_expect_success 'A single-liner subject with a token plus colon is not a foo
'
+test_expect_success 'commit -s places sob on third line after two empty lines' '
+ git commit -s --allow-empty --allow-empty-message &&
+ cat <<-EOF >expect &&
+
+
+ Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>
+
+ EOF
+ egrep -v '^#' .git/COMMIT_EDITMSG >actual &&
+ test_cmp expect actual
+'
+
write_script .git/FAKE_EDITOR <<\EOF
mv "$1" "$1.orig"
(
--
1.8.0.1.253.gfcb57d5.dirty
^ permalink raw reply related
* Re: [PATCH] add: allow users to silence Git 2.0 warnings about "add -u"
From: Matthieu Moy @ 2013-02-22 9:32 UTC (permalink / raw)
To: David Aguilar; +Cc: Junio C Hamano, git
In-Reply-To: <CAJDDKr4dCJ3p9QBGr09kW4_0BsVJcpE7s83=eNxKE15pMznWCw@mail.gmail.com>
David Aguilar <davvid@gmail.com> writes:
> Please enlighten me.
> Are we really getting rid of it and replacing it with ":/"?
> That syntax looks like a meh face.. just sayin'
The current behavior is indeed replaced by "git add -u .", not ":/".
> Unlike push.default, whose warning can be silenced with configuration,
> git 1.x does not have a way to silence this warning without retraining
> existing users.
Yes, but push.default is really different: there is a config variable,
and we want the behavior to be configurable. In the case of "git add",
I don't think adding a configuration option would be the right thing.
That would mean typing "git add -u" on an account which isn't yours will
be unpredictable *forever*.
OTOH, "git add -u :/" and "git add -u ." will behave predictibly on any
version of Git that accepts them, past present or future (:/ was added
in 1.7.6, a year and a half ago).
> Another example...
>
> $ git grep 'stuff' :/
>
> would it be too much to teach it to do:
>
> $ git grep -u 'stuff'
"git grep" is out of the scope of this change. Yes, it is inconsistant
with the rest of Git, but doesn't seem to surprise users as much as "git
add -u" (for which the inconsistancy appears within the "add" command).
I don't understand what you mean by "git grep -u". "git add -u" is a
shortcut for "git add --update", and "git grep --update" wouldn't make
sense to me. Do you mean we should add a "--full-tree" to "git grep"?
That seems really overkill to me since we already have the :/ pathspec.
> but in 2.0 that -u would be a no-op because "grep" will be full tree, no?
No it won't.
> I need to read the old discussions.
> Can someone tell me the magic google search syntax they use to dig them up?
See the discussion here:
http://thread.gmane.org/gmane.comp.version-control.git/213988/focus=214106
(recursively, there's a pointer to an older discussion)
> Would a better way be a method to make "git add -u" behave like 2.0 today?
As I said, I think adding a configuration option that would remain after
2.0 would do more harm than good. But after thinking about it, I'm not
against an option like a boolean add.use2dot0Behavior that would:
* Right now, adopt the future behavior and kill the warning
* From 2.0, kill the warning without changing the bevavior
* When we stop warning, disapear.
This, or the add.silence-pathless-warnings (which BTW should be spelled
add.silencePathlessWarnings) would not harm as long as they are not
advertized in the warning. What we don't want is dumb users reading half
the message and apply the quickest receipe they find to kill the warning
without thinking about the consequences.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ 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