Reply
I have something very confidential and private to discuss with you
Re: git svn clone fails
Jörg Schaible wrote: > Is there really no other place for a bug report? This will simply vanish in > the list's noise ... These messages do get seen and read. (And I would not have seen this message if it were posted anywhere else). But I don't have much time or motivation to work on git svn since it's mostly made itself obsolete for me. --preserve-empty-dirs probably hasn't seen much real-world use and (IIRC) not something that could always be 100% reliable. Regardless of options, git svn does log empty directories, so there's also an obscure, probably equally-unused "git svn mkdirs" command which processes the unhandled.log files inside $GIT_DIR to recreate empty directories. You could give that a try. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-svn: parse authors file more leniently
Michael J Gruber wrote: > Instead, make git svn uses the perl regex > > /^(.+?|\(no author\))\s*=\s*(.+?)\s*<(.*)>\s*$/ > > for parsing the authors file so that the same (slightly more lenient) > regex is used in both cases. > > Reported-by: Till Schäfer > Signed-off-by: Michael J Gruber Thanks. Signed-off-by: Eric Wong And pushed to master of git://bogomips.org/git-svn (commit f7c6de0ea1bd5722a1181c6279676c6831b38a34) By the way, I also had some other patches sitting around for you. Did you ever have time to revisit them? (I haven't) t/lib-httpd: load mod_unixd t/lib-git-svn: check same httpd module dirs as lib-httpd -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-svn: make batch mode optional for git-cat-file
Victor Leschuk wrote: > The thing is that git-cat-file keeps growing during work when running > in "batch" mode. See the figure attached: it is for cloning a rather > small repo (1 hour to clone about ~14000 revisions). However the clone > of a large repo (~28 revisions) took about 2 weeks and > git-cat-file has outgrown the parent perl process several times > (git-cat-file - ~3-4Gb, perl - 400Mb). Ugh, that sucks. Even the 400Mb size of Perl annoys me greatly and I'd work on fixing it if I had more time. But I'm completely against adding this parameter to git-svn. git-svn is not the only "cat-file --batch" user, so this option is only hiding problems. The best choice is to figure out why cat-file is wasting memory. Disclaimer: I'm no expert on parts of git written in C, but perhaps the alloc.c interface is why memory keeps growing. > What was done: > * I have run it under valgrind and mtrace and haven't found any memory leaks > * Found the source of most number of memory reallocations > (batch_object_write() function (strbuf_expand -> realloc)) - tried to make > the streambuf object static and avoid reallocs - didn't help > * Tried preloading other allocators than standard glibc - no significant > difference A few more questions: * What is the largest file that existed in that repo? * Did you try "MALLOC_MMAP_THRESHOLD_" with glibc? Perhaps setting that to 131072 will help, that'll force releasing larger chunks than that; but it might be moot if alloc.c is getting in the way. If alloc.c is the culprit, I would consider to transparently restart "cat-file --batch" once it grows to a certain size or after a certain number of requests are made to it. We can probably do this inside "git cat-file" itself without changing any callers by calling execve. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-svn: make batch mode optional for git-cat-file
Eric Wong wrote: > Victor Leschuk wrote: > > The thing is that git-cat-file keeps growing during work when running > > in "batch" mode. See the figure attached: it is for cloning a rather > > small repo (1 hour to clone about ~14000 revisions). However the clone > > of a large repo (~28 revisions) took about 2 weeks and > > git-cat-file has outgrown the parent perl process several times > > (git-cat-file - ~3-4Gb, perl - 400Mb). How much of that is anonymous memory, though? (pmap $PID_OF_GIT_CAT_FILE) Running the following on the Linux kernel tree I had lying around: (for i in $(seq 100 200); do git ls-files | sed -e "s/^/HEAD~$i:/"; done)|\ git cat-file --batch >/dev/null Reveals about 510M RSS in top, but pmap says less than 20M of that is anonymous. So the rest are mmap-ed packfiles; that RSS gets transparently released back to the kernel under memory pressure. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-svn: make batch mode optional for git-cat-file
Victor Leschuk wrote:
> Hello Eric, thanks for looking into it.
>
> >> git-cat-file has outgrown the parent perl process several times
> >> (git-cat-file - ~3-4Gb, perl - 400Mb).
>
> > Ugh, that sucks.
> > Even the 400Mb size of Perl annoys me greatly and I'd work
> > on fixing it if I had more time.
>
> I was going to look at this problem also, but first I'd like to improve the
> situation with cat-file as on large repos it is larger problem. By the way,
> what direction would you suggest to begin with?
See below :)
> > > git-cat-file has outgrown the parent perl process several times
> > > (git-cat-file - ~3-4Gb, perl - 400Mb).
>
> > How much of that is anonymous memory, though?
>
> Haven't measured on this particular repo: didn't redo the 2 week
> experiment =) However I checked on a smaller test repo and anon memory
> is about 12M out of 40M total. Most of memory is really taken by
> mmaped *.pack and *idx files.
If it's mmap-ed files, that physical memory is only used on-demand
and can be dropped at any time because it's backed by disk.
In other words, I would not worry about any file-backed mmap at all
(unless you're on 32-bit, but I think git has workarounds for that)
Do you still have that giant repo around?
Are the combined size of the pack + idx files are at least 3-4 GB?
This should cat all the blobs in history without re-running git-svn:
git log --all --raw -r --no-abbrev | \
awk '/^:/ {print $3; print $4}' | git cat-file --batch
git log actually keeps growing, but the cat-file process shouldn't
use anonymous memory much if you inspect it with pmap.
> Actually I accidentally found out that if I export GIT_MALLOC_LIMIT
> variable set to several megabytes it has the following effect:
> * git-svn.perl launches git-gc
> * git-gc can't allocate enough memory and thus doesn't create any pack files
> * git-cat-file works only with pure blob object, not packs, and it's
> memory usage doesn't grow larger than 4-5M
>
> It gave me a thought that maybe we could get rid of "git gc" calls
> after each commit in perl code and just perform one large gc operation
> at the end. It will cost disk space during clone but save us memory.
> What do you think?
You can set gc.auto to zero in your $GIT_CONFIG to disable gc.
The "git gc" calls were added because unpacked repos were growing
too large and caused problems for other people.
Perhaps play with some other pack* options documented in
Documentation/config to limit maximum pack size/depth.
Is this a 32-bit or 64-bit system?
> As for your suggestion regarding periodic restart of batch process
> inside git-cat-file, I think we could give it a try, I can prepare a
> patch and run some tests.
I am not sure if we need it for git-svn.
In another project, the only reason I've found to restart
"cat-file --batch" is in case the repo got repacked and old packs
got unlinked, cat-file would hold a reference onto the old file
and suck up space. It might be better if "cat-file --batch" learned
to detect unlinked files and then munmap + close them.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] prune: keep files created after process start
Avoid pruning files which were written after the prune process
starts, as it possible to concurrently create new objects while
"git prune" is running.
Tested on git.git by starting "git prune" in one terminal,
creating a random loose object via "git hash-object --stdin -w"
in a different terminal, and ensuring the loose object remains
after "git prune" completes.
Signed-off-by: Eric Wong
---
I'm somewhat surprised this check didn't already exist;
but maybe nobody else runs prune manually, anymore.
builtin/prune.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/builtin/prune.c b/builtin/prune.c
index 8f4f052..d4cd054 100644
--- a/builtin/prune.c
+++ b/builtin/prune.c
@@ -14,6 +14,7 @@ static const char * const prune_usage[] = {
static int show_only;
static int verbose;
static unsigned long expire;
+static time_t start;
static int show_progress = -1;
static int prune_tmp_file(const char *fullpath)
@@ -21,7 +22,7 @@ static int prune_tmp_file(const char *fullpath)
struct stat st;
if (lstat(fullpath, &st))
return error("Could not stat '%s'", fullpath);
- if (st.st_mtime > expire)
+ if (st.st_mtime > expire || st.st_ctime >= start)
return 0;
if (show_only || verbose)
printf("Removing stale temporary file %s\n", fullpath);
@@ -47,7 +48,7 @@ static int prune_object(const unsigned char *sha1, const char
*fullpath,
error("Could not stat '%s'", fullpath);
return 0;
}
- if (st.st_mtime > expire)
+ if (st.st_mtime > expire || st.st_ctime >= start)
return 0;
if (show_only || verbose) {
enum object_type type = sha1_object_info(sha1, NULL);
@@ -111,6 +112,7 @@ int cmd_prune(int argc, const char **argv, const char
*prefix)
};
char *s;
+ start = time(NULL);
expire = ULONG_MAX;
save_commit_buffer = 0;
check_replace_refs = 0;
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] prune: keep files created after process start
Junio C Hamano wrote: > Eric Wong writes: > > I'm somewhat surprised this check didn't already exist; > > but maybe nobody else runs prune manually, anymore. > > The only time an end user would run "git prune" in their > repositories with working trees these days is "git repack" followed > by "git prune", I would guess. Right, I wanted to drop some sensitive data with that. > You generally cannot compare a timestamp you read from the > filesystem and the timestamp you obtain from time(2) when network > filesystems are involved, so I am not sure the implementation is > quite right, though. Yes, but I'm not aware of a good way to deal with this; I would expect machines on the same network would have synchronized times. Perhaps having a small slack time (one second?) could mitigate some problems with machines being slightly off: start = time(NULL) - 1; And then warning if it encountered files within the slack period and asking the user to rerun "prune" if needed. But I'm not sure it's worth it. Any other suggestions? Thanks. Having prune prevent object creation entirely while running seems unacceptable. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] prune: keep files created after process start
Andreas Schwab wrote:
> Eric Wong writes:
>
> > @@ -21,7 +22,7 @@ static int prune_tmp_file(const char *fullpath)
> > struct stat st;
> > if (lstat(fullpath, &st))
> > return error("Could not stat '%s'", fullpath);
> > - if (st.st_mtime > expire)
> > + if (st.st_mtime > expire || st.st_ctime >= start)
>
> That will also mean objects created (or their inode changed) up to a
> second before the start of prune will not be removed. For example,
> objects ejected out of a pack by a previous repack may be affected.
True, but I prefer to err on the side of keeping data rather than
removing it. But keeping it can also be a liability (as it was
in my case :) So, perhaps warn users instead:
diff --git a/builtin/prune.c b/builtin/prune.c
index d4cd054..c1642d1 100644
--- a/builtin/prune.c
+++ b/builtin/prune.c
@@ -16,14 +16,22 @@ static int verbose;
static unsigned long expire;
static time_t start;
static int show_progress = -1;
+static unsigned long ctime_matches;
static int prune_tmp_file(const char *fullpath)
{
struct stat st;
if (lstat(fullpath, &st))
return error("Could not stat '%s'", fullpath);
- if (st.st_mtime > expire || st.st_ctime >= start)
+ if (st.st_mtime > expire || st.st_ctime > start)
return 0;
+
+ if (st.st_ctime == start) {
+ ctime_matches++;
+ warning("Keeping %s since it changed as we started", fullpath);
+ return 0;
+ }
+
if (show_only || verbose)
printf("Removing stale temporary file %s\n", fullpath);
if (!show_only)
@@ -48,8 +56,13 @@ static int prune_object(const unsigned char *sha1, const
char *fullpath,
error("Could not stat '%s'", fullpath);
return 0;
}
- if (st.st_mtime > expire || st.st_ctime >= start)
+ if (st.st_mtime > expire || st.st_ctime > start)
return 0;
+ if (st.st_ctime == start) {
+ ctime_matches++;
+ warning("Keeping %s since it changed as we started", fullpath);
+ return 0;
+ }
if (show_only || verbose) {
enum object_type type = sha1_object_info(sha1, NULL);
printf("%s %s\n", sha1_to_hex(sha1),
@@ -155,5 +168,12 @@ int cmd_prune(int argc, const char **argv, const char
*prefix)
if (is_repository_shallow())
prune_shallow(show_only);
+ if (ctime_matches) {
+ warning("%lu files kept since they changed as prune started",
+ ctime_matches);
+ warning("rerun prune after %s",
+ show_date(start, 0, DATE_MODE(NORMAL)));
+ }
+
return 0;
}
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: may be bug in svn fetch no-follow-parent
+Cc: a bunch of folks who may know better how mergeinfo works in git-svn
Александр Овчинников wrote:
> Why git svn fetch try to handle mergeinfo changes when
> no-follow-parent is enabled?
It probably should not... --no-follow-parent isn't a common
config, though. Can you try the patch below?
> Git try to follow parents regardless of this option value.
> If branch created without this option then git will follow
> parent succesfully
> If branch created with this option then git try to follow and
> fail with "cannot find common ancestor" error
> If branch does not exists (ignored) then git try to follow and
> fail with "couldn't find revmap" error. It is very long
> operation
Do you have an example repo you could share with us?
Thanks.
I still don't think I've encountered a repo which uses
mergeinfo myself.
Hopefully the following patch works for you:
-8<
Subject: [PATCH] git-svn: skip mergeinfo with --no-follow-parent
For repositories without parent following enabled, computing
mergeinfo can be expensive and pointless.
Note: Only tested on existing test cases.
---
perl/Git/SVN.pm | 25 -
1 file changed, 16 insertions(+), 9 deletions(-)
diff --git a/perl/Git/SVN.pm b/perl/Git/SVN.pm
index d94d01c..bee1e7d 100644
--- a/perl/Git/SVN.pm
+++ b/perl/Git/SVN.pm
@@ -1905,15 +1905,22 @@ sub make_log_entry {
my @parents = @$parents;
my $props = $ed->{dir_prop}{$self->path};
- if ( $props->{"svk:merge"} ) {
- $self->find_extra_svk_parents($props->{"svk:merge"}, \@parents);
- }
- if ( $props->{"svn:mergeinfo"} ) {
- my $mi_changes = $self->mergeinfo_changes
- ($parent_path, $parent_rev,
-$self->path, $rev,
-$props->{"svn:mergeinfo"});
- $self->find_extra_svn_parents($mi_changes, \@parents);
+ if ($self->follow_parent) {
+ my $tickets = $props->{"svk:merge"};
+ if ($tickets) {
+ $self->find_extra_svk_parents($tickets, \@parents);
+ }
+
+ my $mergeinfo_prop = $props->{"svn:mergeinfo"};
+ if ($mergeinfo_prop) {
+ my $mi_changes = $self->mergeinfo_changes(
+ $parent_path,
+ $parent_rev,
+ $self->path,
+ $rev,
+ $mergeinfo_prop);
+ $self->find_extra_svn_parents($mi_changes, \@parents);
+ }
}
open my $un, '>>', "$self->{dir}/unhandled.log" or croak $!;
--
EW
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
http-backend fatal error message on shallow clone
I noticed "fatal: The remote end hung up unexpectedly" in server logs from shallow clones. Totally reproducible in the test cases, too. The following change shows it: diff --git a/t/t5561-http-backend.sh b/t/t5561-http-backend.sh index 90e0d6f..cfa55ce 100755 --- a/t/t5561-http-backend.sh +++ b/t/t5561-http-backend.sh @@ -132,5 +132,11 @@ test_expect_success 'server request log matches test results' ' test_cmp exp act ' +test_expect_success 'shallow clone' ' + config http.uploadpack true && + git clone --depth=1 "$HTTPD_URL/smart/repo.git" shallow && + tail "$HTTPD_ROOT_PATH"/error.log | grep fatal +' + stop_httpd test_done And the last test ends like this: expecting success: config http.uploadpack true && git clone --depth=1 "$HTTPD_URL/smart/repo.git" shallow && tail "$HTTPD_ROOT_PATH"/error.log | grep fatal Cloning into 'shallow'... [Tue Jun 21 11:07:41.391269 2016] [cgi:error] [pid 21589] [client 127.0.0.1:37518] AH01215: fatal: The remote end hung up unexpectedly ok 15 - shallow clone It doesn't show above, but I think http-backend exits with a non-zero status, too, which might cause some CGI implementations to complain or break. Not sure if it's just a corner case that wasn't tested or something else, but the clone itself seems successful... -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] doc: git-htmldocs.googlecode.com is no more
Jonathan Nieder wrote: > I'd rather use an up-to-date https link for the rendered > documentation, but I wasn't able to find one. According to the 'todo' > branch, prebuilt documentation is pushed to > > 1. https://kernel.googlesource.com/pub/scm/git/git-htmldocs > 2. git://repo.or.cz/git-htmldocs > 3. somewhere on git.sourceforge.jp and git.sourceforge.net? > 4. https://github.com/gitster/git-htmldocs > 5. https://github.com/git/htmldocs Just wondering, who updates https://kernel.org/pub/software/scm/git/docs/ and why hasn't it been updated in a while? (currently it says Last updated 2015-06-06 at the bottom) I normally link people there since I would rather not advertise for a commercial service. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] doc: git-htmldocs.googlecode.com is no more
Junio C Hamano wrote: > On Wed, Jun 22, 2016 at 12:00 PM, Eric Wong wrote: > > > > Just wondering, who updates > > https://kernel.org/pub/software/scm/git/docs/ > > and why hasn't it been updated in a while? > > (currently it says Last updated 2015-06-06 at the bottom) > > Nobody. It is too cumbersome to use their upload tool to update many > files (it is geared towards updating a handful of tarballs at a time). Alright, I've setup https://git-htmldocs.bogomips.org/ for my own usage, at least. It should check for updates twice an hour(*), and plain HTTP is also available in case Let's Encrypt goes away. Can't hurt to have more mirrors: --8<- #!/bin/sh set -e DST=/path/to/server-docroot/git-htmldocs # my mirror of git://git.kernel.org/pub/scm/git/git-htmldocs.git: GIT_DIR=/path/to/mirrors/git-htmldocs.git export GIT_DIR # rsync from a temporary dir for atomicity so nobody fetches # a partially written file tmp="$(mktemp -t -d htmldocs.XXX)" git archive --format=tar HEAD | tar x -C "$tmp" chmod 755 "$tmp" rsync -a "$tmp/" "$DST/" rm -rf "$tmp" # for servers which support pre-gzipped files (e.g. gzip_static in nginx) find "$DST" -type f -name '*.html' -o -name '*.txt' | while read file do gz="$file.gz" if ! test -e "$gz" || test "$gz" -ot "$file" then gztmp="$gz.$$.tmp" gzip -9 <"$file" >"$gztmp" touch -r "$file" "$gztmp" mv "$gztmp" "$gz" fi done --- (*) On a side note, It would be nice to something like IMAP IDLE for real-time updates of mirrors. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: may be bug in svn fetch no-follow-parent
Александр Овчинников wrote:
> Unfortunately this is not open source repository. I agree that it is
> pointless try to handle mergeinfo (because it always fails).
> Cases when it is expensive:
> 1. delete and restore mergeinfo property
> 2. merge trunk to very old branch
> 3. create, delete, create branch with --no-follow-parent. All records in
> mergeinfo will be hadled like new.
>
> I already patched like this and this is helpfull, works fine and fast.
Thanks for the info. Patch + pull request below for Junio.
> I can share only mergeinfo property
Oops, looks like your zip attachment got flagged as spam for
my mailbox and swallowed by vger.kernel.org :x
-8<
Subject: [PATCH] git-svn: skip mergeinfo handling with --no-follow-parent
For repositories without parent following enabled, finding
git parents through svn:mergeinfo or svk::parents can be
expensive and pointless.
Reported-by: Александр Овчинников
http://mid.gmane.org/4094761466408...@web24o.yandex.ru
Signed-off-by: Eric Wong
---
The following changes since commit ab7797dbe95fff38d9265869ea367020046db118:
Start the post-2.9 cycle (2016-06-20 11:06:49 -0700)
are available in the git repository at:
git://bogomips.org/git-svn.git svn-nfp-mergeinfo
for you to fetch changes up to 6d523a3ab76cfa4ed9ae0ed9da7af43efcff3f07:
git-svn: skip mergeinfo handling with --no-follow-parent (2016-06-22
22:48:54 +)
----
Eric Wong (1):
git-svn: skip mergeinfo handling with --no-follow-parent
perl/Git/SVN.pm | 25 -
1 file changed, 16 insertions(+), 9 deletions(-)
diff --git a/perl/Git/SVN.pm b/perl/Git/SVN.pm
index d94d01c..bee1e7d 100644
--- a/perl/Git/SVN.pm
+++ b/perl/Git/SVN.pm
@@ -1905,15 +1905,22 @@ sub make_log_entry {
my @parents = @$parents;
my $props = $ed->{dir_prop}{$self->path};
- if ( $props->{"svk:merge"} ) {
- $self->find_extra_svk_parents($props->{"svk:merge"}, \@parents);
- }
- if ( $props->{"svn:mergeinfo"} ) {
- my $mi_changes = $self->mergeinfo_changes
- ($parent_path, $parent_rev,
-$self->path, $rev,
-$props->{"svn:mergeinfo"});
- $self->find_extra_svn_parents($mi_changes, \@parents);
+ if ($self->follow_parent) {
+ my $tickets = $props->{"svk:merge"};
+ if ($tickets) {
+ $self->find_extra_svk_parents($tickets, \@parents);
+ }
+
+ my $mergeinfo_prop = $props->{"svn:mergeinfo"};
+ if ($mergeinfo_prop) {
+ my $mi_changes = $self->mergeinfo_changes(
+ $parent_path,
+ $parent_rev,
+ $self->path,
+ $rev,
+ $mergeinfo_prop);
+ $self->find_extra_svn_parents($mi_changes, \@parents);
+ }
}
open my $un, '>>', "$self->{dir}/unhandled.log" or croak $!;
--
EW
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git-svn aborts with "Use of uninitialized value $u" when a non-svn-backed branch is present in remote
Jacob Godserv wrote:
> On Tue, Sep 22, 2015 at 2:48 PM, Jacob Godserv wrote:
> > I found a specific case in which git-svn improperly aborts:
> >
> > 1. I created a git-svn repository, named "git-svn repo", by cloned an
> > svn repository via the git-svn tool.
> > 2. I created a normal git repository, named "configuration repo". I
> > renamed the master branch to "configuration". The initial commit adds
> > a README and some utility scripts.
> > 3. I created a bare repository, named "master repo".
> > 4. I pushed from the git-svn repo to the master repo.
> > 5. I pushed from the configuration repo to the master repo.
> >
> > The idea is the configuration branch, which is detached from any
> > git-svn history, can contain some useful tools, defaults, etc., that I
> > can share with teammates who want to use git on this svn project. It's
> > an odd use of git, but it has been working well.
> >
> > However, a vanilla distribution of Git for Windows 2.5.2 produces the
> > following error when running any git-svn command, such as "git svn
> > info", on the cloned master repo:
> >
> > Use of uninitialized value $u in substitution (s///) at
> > /mingw64/share/perl5/site_perl/Git/SVN.pm line 105.
> > Use of uninitialized value $u in concatenation (.) or string at
> > /mingw64/share/perl5/site_perl/Git/SVN.pm line 105.
> > refs/remotes/origin/configuration: 'svn+ssh://10.0.1.1/repos/projectA'
> > not found in ''
> >
> > In the mentioned SVN.pm file, after the line:
> >
> > my $u = (::cmt_metadata("$refname"))[0];
> >
> > I added the following four lines:
> >
> > if (not defined $u) {
> > warn "W: $refname does not exist in
> > SVN; skipping";
> > next;
> > }
Christian (Cc-ed) also noticed the problem a few weeks ago
and took a more drastic approach by having git-svn die
instead of warning:
http://mid.gmane.org/1462604323-18545-1-git-send-email-chrisc...@tuxfamily.org
which landed as commit 523a33ca17c76bee007d7394fb3930266c577c02
in git.git: https://bogomips.org/mirrors/git.git/patch?id=523a33ca17c7
Is dying here too drastic and maybe warn is preferable?
> > git-svn appears to operate correctly with this patch. This is my first
> > time ever editing a perl script, so I apologize if I murdered an
> > adorable animal just now.
> >
> > I'm sending this in so more knowledgeable git-svn developers can
> > comment on this and fix this in the official distribution of git,
> > assuming there is a bug here to fix.
> >
> > --
> > Jacob
>
> This e-mail has gone ignored several months. Is the maintainer of
> git-svn on this mailing list? Should I submit this issue elsewhere?
Sorry, I wasn't paying attention to the list at that time.
It is customary to Cc: authors of the code in question
(that also decentralizes our workflow in case vger is down),
and also acceptable to send reminders after a week or two
in case we're overloaded with other work.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git-svn aborts with "Use of uninitialized value $u" when a non-svn-backed branch is present in remote
Please don't drop Cc:, re-adding git@vger and Christian Jacob Godserv wrote: > > Christian (Cc-ed) also noticed the problem a few weeks ago > > and took a more drastic approach by having git-svn die > > instead of warning: > > http://mid.gmane.org/1462604323-18545-1-git-send-email-chrisc...@tuxfamily.org > > which landed as commit 523a33ca17c76bee007d7394fb3930266c577c02 > > in git.git: https://bogomips.org/mirrors/git.git/patch?id=523a33ca17c7 > > > > Is dying here too drastic and maybe warn is preferable? > > In my opinion this is too drastic. It keeps me from storing > git-specific data on a git-svn mirror. I tend to agree, but will wait to see what Christian thinks. > Here's my setup: > - My git-svn mirror uses git-svn to create a git repo that mirrors > svn history. This repository is then pushed to a clean bare > repository. So far so good. Only svn-sourced branches exist. > - The git-svn mirror script also saves a copy of the git-svn > configuration used to generate the git mirror repository in an > "orphaned" branch called something like 'git-svn-conf'. This is > completely separate from the svn history, and exists only for my > git-svn purposes. > - On the "client" side, another script I wrote knows how to parse the > git-svn configuration in that 'git-svn-conf' branch to properly > reconfigure git-svn on the local machine, so I can use 'git svn' > themselves to commit, etc., and still generate the same hashes so > there's no forked history during the next mirror fetch. > > Long story short: I have branches which aren't in SVN history for > automated git-svn purposes. > > It appears that simply skipping the branch in that loop fixes the > issue. However, I don't know how the metadata is stored and what > exactly that loop does, so I may be creating hidden side effects I > have been lucky enough to not trigger yet. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC] gc: correct gc.autoPackLimit documentation
I'm not sure if this is the best approach, or if changing too_many_packs can be done without causing problems for hosts of big repos. ---8<- Subject: [PATCH] gc: correct gc.autoPackLimit documentation I want to ensure there is only one pack in my repo to take advantage of pack bitmaps. Based on my reading of the documentation, I configured gc.autoPackLimit=1 which led to "gc --auto" constantly trying to repack on every invocation. Update the documentation to reflect what is probably a long-standing off-by-one bug in builtin/gc.c::too_many_packs: - return gc_auto_pack_limit <= cnt; + return gc_auto_pack_limit < cnt; However, changing gc itself at this time may cause problems for people who are already using gc.autoPackLimit=2 and expect bitmaps to work for them. Signed-off-by: Eric Wong --- Documentation/config.txt | 2 +- Documentation/git-gc.txt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index 2e1b2e4..b0de3f1 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -1345,7 +1345,7 @@ gc.auto:: default value is 6700. Setting this to 0 disables it. gc.autoPackLimit:: - When there are more than this many packs that are not + When there at least this many packs that are not marked with `*.keep` file in the repository, `git gc --auto` consolidates them into one larger pack. The default value is 50. Setting this to 0 disables it. diff --git a/Documentation/git-gc.txt b/Documentation/git-gc.txt index fa15104..658612d 100644 --- a/Documentation/git-gc.txt +++ b/Documentation/git-gc.txt @@ -54,7 +54,7 @@ all loose objects are combined into a single pack using `git repack -d -l`. Setting the value of `gc.auto` to 0 disables automatic packing of loose objects. + -If the number of packs exceeds the value of `gc.autoPackLimit`, +If the number of packs matches or exceeds the value of `gc.autoPackLimit`, then existing packs (except those marked with a `.keep` file) are consolidated into a single pack by using the `-A` option of 'git repack'. Setting `gc.autoPackLimit` to 0 disables -- EW -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] gc: correct gc.autoPackLimit documentation
Jeff King wrote: > On Sat, Jun 25, 2016 at 01:14:50AM +, Eric Wong wrote: > > > I'm not sure if this is the best approach, or if changing > > too_many_packs can be done without causing problems for > > hosts of big repos. > > > > ---8<- > > Subject: [PATCH] gc: correct gc.autoPackLimit documentation > > > > I want to ensure there is only one pack in my repo to take > > advantage of pack bitmaps. Based on my reading of the > > documentation, I configured gc.autoPackLimit=1 which led to > > "gc --auto" constantly trying to repack on every invocation. > > I'm not sure if you might be misinterpreting earlier advice on bitmaps > here. At the time of packing, bitmaps need for all of the objects to go > to a single pack (they cannot handle a case where one object in the pack > can reach another object that is not in the pack). But that is easily > done with "git repack -adb". > > After that packing, you can add new packs that do not have bitmaps, and > the bitmaps will gracefully degrade. E.g., imagine master was at tip X > when you repacked with bitmaps, and now somebody has pushed to make it > tip Y. Somebody then clones, asking for Y. The bitmap code will start > at Y and walk backwards. When it hits X, it stops walking as it can fill > in the rest of the reachability from there. Ah, thanks, makes sense. I was misinterpreting earlier advice on bitmaps. > That's neither here nor there for the off-by-one in gc or its > documentation, of course, but just FYI. I'm now inclined to fix the problem in gc and leave the documentation as-is (unless it cause other problems...) -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] gc: fix off-by-one error with gc.autoPackLimit
This matches the documentation and allows gc.autoPackLimit=1 to maintain a single pack without attempting a repack on every "git gc --auto" invocation. Signed-off-by: Eric Wong --- builtin/gc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builtin/gc.c b/builtin/gc.c index c583aad..332bcf7 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -177,7 +177,7 @@ static int too_many_packs(void) */ cnt++; } - return gc_auto_pack_limit <= cnt; + return gc_auto_pack_limit < cnt; } static void add_repack_all_option(void) -- EW -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v12 04/20] index-helper: new daemon for caching index and related stuff
David Turner wrote: > On 06/25/2016 10:33 AM, Duy Nguyen wrote: > >>+ /* > >>+* Our connection to the client is blocking since a client > >>+* can always be killed by SIGINT or similar. > >>+*/ > >>+ set_socket_blocking_flag(client_fd, 0); > > > >Out of curiosity, do we really need this? I thought default behavior > >was always blocking (and checked linux kernel, it seemed to agree with > >me). Maybe for extra safety because other OSes may default to > >something else? > > Yes -- see this bug report for details: > https://bugs.python.org/issue7995 I realize it's an issue with BSDs, but it still seems unnecessary, here: 1) the packet_read => get_packet_data => read_in_full => xread call chain already poll()s on EAGAIN/EWOULDBLOCK. write_in_full => xwrite busy loops on EAGAIN/EWOULDBLOCK. xwrite should probably poll, too; but I guess EAGAIN is uncommon with small writes. 2) you create the listen fd you call accept on and never set non-blocking on it. It might be an issue one day if we use socket activation and inherit a socket, but the retries mentioned in 1) should cover that case. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/2] wrapper: xread/xwrite fixes for non-blocking FDs
1/2 fixes a bug introduced in commit 1079c4be0b720
("xread: poll on non blocking fds") where the "continue"
got dropped.
I noticed the 1/2 bug while working on 2/2 and intentionally
triggering EAGAIN on a custom HTTP server to test 100% CPU
usage. I originally blindly copied the branch from xread
into xwrite without the "continue" and was greeted with a
failed clone.
Eric Wong (2):
xread: retry after poll on EAGAIN/EWOULDBLOCK
xwrite: poll on non-blocking FDs
wrapper.c | 23 +--
1 file changed, 21 insertions(+), 2 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] xwrite: poll on non-blocking FDs
write(2) can hit the same EAGAIN/EWOULDBLOCK errors as read(2),
so busy-looping on a non-blocking FD is a waste of resources.
Currently, I do not know of a way for this happen:
* the NonBlocking directive in systemd does not apply to stdin,
stdout, or stderr.
* xinetd provides no way to set the non-blocking flag at all
But theoretically, it's possible a careless C10K HTTP server
could use pipe2(..., O_NONBLOCK) to setup a pipe for
git-http-backend with only the intent to use non-blocking reads;
but accidentally leave non-blocking set on the write end passed
as stdout to git-upload-pack.
Followup-to: 1079c4be0b720 ("xread: poll on non blocking fds")
Signed-off-by: Eric Wong
---
wrapper.c | 22 --
1 file changed, 20 insertions(+), 2 deletions(-)
diff --git a/wrapper.c b/wrapper.c
index f1155d0..d973f86 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -274,8 +274,26 @@ ssize_t xwrite(int fd, const void *buf, size_t len)
len = MAX_IO_SIZE;
while (1) {
nr = write(fd, buf, len);
- if ((nr < 0) && (errno == EAGAIN || errno == EINTR))
- continue;
+ if (nr < 0) {
+ if (errno == EINTR)
+ continue;
+ if (errno == EAGAIN || errno == EWOULDBLOCK) {
+ struct pollfd pfd;
+ pfd.events = POLLOUT;
+ pfd.fd = fd;
+ /*
+* it is OK if this poll() failed; we
+* want to leave this infinite loop
+* only when write() returns with
+* success, or an expected failure,
+* which would be checked by the next
+* call to write(2).
+*/
+ poll(&pfd, 1, -1);
+ continue;
+ }
+ }
+
return nr;
}
}
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] xread: retry after poll on EAGAIN/EWOULDBLOCK
We should continue to loop after EAGAIN/EWOULDBLOCK as the
intent of xread is to read as much as possible until an
EOF or real error occurs.
Fixes: 1079c4be0b720 ("xread: poll on non blocking fds")
Signed-off-by: Eric Wong
---
wrapper.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/wrapper.c b/wrapper.c
index 5dc4e15..f1155d0 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -255,6 +255,7 @@ ssize_t xread(int fd, void *buf, size_t len)
* call to read(2).
*/
poll(&pfd, 1, -1);
+ continue;
}
}
return nr;
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v12 04/20] index-helper: new daemon for caching index and related stuff
David Turner wrote: > On 06/26/2016 04:53 AM, Eric Wong wrote: > >David Turner wrote: > >>On 06/25/2016 10:33 AM, Duy Nguyen wrote: > >>>>+ /* > >>>>+* Our connection to the client is blocking since a client > >>>>+* can always be killed by SIGINT or similar. > >>>>+*/ > >>>>+ set_socket_blocking_flag(client_fd, 0); > >>> > >>>Out of curiosity, do we really need this? I thought default behavior > >>>was always blocking (and checked linux kernel, it seemed to agree with > >>>me). Maybe for extra safety because other OSes may default to > >>>something else? > >> > >>Yes -- see this bug report for details: > >>https://bugs.python.org/issue7995 > > > >I realize it's an issue with BSDs, but it still seems > >unnecessary, here: > > > >1) the packet_read => get_packet_data => read_in_full => xread > >call chain already poll()s on EAGAIN/EWOULDBLOCK. > >write_in_full => xwrite busy loops on EAGAIN/EWOULDBLOCK. > >xwrite should probably poll, too; but I guess EAGAIN is > >uncommon with small writes. > > That is a CPU-burning busy loop on a non-blocking socket. Indeed, fixes proposed in (xread was also broken(!)): http://mid.gmane.org/20160626232112.721-...@80x24.org http://mid.gmane.org/20160626232112.721-...@80x24.org http://mid.gmane.org/20160626232112.721-...@80x24.org -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv2 1/2] xread: retry after poll on EAGAIN/EWOULDBLOCK
Jeff King wrote:
> On Sun, Jun 26, 2016 at 11:21:11PM +, Eric Wong wrote:
>
> > We should continue to loop after EAGAIN/EWOULDBLOCK as the
> > intent of xread is to read as much as possible until an
> > EOF or real error occurs.
>
> BTW, a minor nit here. xread() does _not_ read as much as possible until
> EOF. It tries until it gets a real error or at least one byte.
>
> I know you inherited this mistaken text from 1079c4be0b, but we should
> probably not repeat it.
Good catch, here's v2 of PATCH 1/2 reworded:
--8<--
Subject: [PATCH] xread: retry after poll on EAGAIN/EWOULDBLOCK
We should continue to loop after EAGAIN/EWOULDBLOCK as the
intent of xread is to try until there is available data,
EOF, or an unrecoverable error.
Fixes: 1079c4be0b720 ("xread: poll on non blocking fds")
Signed-off-by: Eric Wong
---
wrapper.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/wrapper.c b/wrapper.c
index 5dc4e15..f1155d0 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -255,6 +255,7 @@ ssize_t xread(int fd, void *buf, size_t len)
* call to read(2).
*/
poll(&pfd, 1, -1);
+ continue;
}
}
return nr;
--
EW
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] xread: retry after poll on EAGAIN/EWOULDBLOCK
Stefan Beller wrote:
> On Mon, Jun 27, 2016 at 7:36 AM, Jeff King wrote:
> > It's also true that our error rate will never be 0%. So some bugs will
> > always slip through, some review comments will be forgotten, etc. Eric
> > did find and fix the bug just now, so the "many eyes" theory did work
> > here eventually.
>
> Eric, thanks for catching and fixing the bug!
No problem :) I only noticed it because I was scanning emails
randomly and Duy and David's index-helper thread turned up.
> Quite a while ago, when I started doing code reviews professionally, I
> wondered
> if the code review procedure can be semi-automated, as automation helps
> keeping
> the error rate low. By that I mean having a check list which I can
> check off each point
Maybe a test case or even a small unit test would've helped.
I didn't notice the problem in xread until:
1) I copied the code into xwrite
2) s/POLLIN/POLLOUT/;
3) forced EAGAIN using a patched, home-baked HTTP server
The biggish comment before the poll() obscured the missing
"continue" for me. I read xread() before and did not notice
the missing "continue".
Maybe the following optional patch on top of this series
improves readability:
--8<
Subject: [PATCH 3/2] hoist out io_wait function for xread and xwrite
At least for me, this improves the readability of xread and
xwrite; hopefully allowing missing "continue" statements to
be spotted more easily.
Signed-off-by: Eric Wong
---
wrapper.c | 40
1 file changed, 16 insertions(+), 24 deletions(-)
diff --git a/wrapper.c b/wrapper.c
index d973f86..04bb952 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -227,6 +227,20 @@ int xopen(const char *path, int oflag, ...)
}
}
+static void io_wait(int fd, short poll_events)
+{
+ struct pollfd pfd;
+
+ pfd.fd = fd;
+ pfd.events = poll_events;
+
+ /*
+* no need to check for errors, here;
+* a subsequent read/write will detect unrecoverable errors
+*/
+ poll(&pfd, 1, -1);
+}
+
/*
* xread() is the same a read(), but it automatically restarts read()
* operations with a recoverable error (EAGAIN and EINTR). xread()
@@ -243,18 +257,7 @@ ssize_t xread(int fd, void *buf, size_t len)
if (errno == EINTR)
continue;
if (errno == EAGAIN || errno == EWOULDBLOCK) {
- struct pollfd pfd;
- pfd.events = POLLIN;
- pfd.fd = fd;
- /*
-* it is OK if this poll() failed; we
-* want to leave this infinite loop
-* only when read() returns with
-* success, or an expected failure,
-* which would be checked by the next
-* call to read(2).
-*/
- poll(&pfd, 1, -1);
+ io_wait(fd, POLLIN);
continue;
}
}
@@ -278,18 +281,7 @@ ssize_t xwrite(int fd, const void *buf, size_t len)
if (errno == EINTR)
continue;
if (errno == EAGAIN || errno == EWOULDBLOCK) {
- struct pollfd pfd;
- pfd.events = POLLOUT;
- pfd.fd = fd;
- /*
-* it is OK if this poll() failed; we
-* want to leave this infinite loop
-* only when write() returns with
-* success, or an expected failure,
-* which would be checked by the next
-* call to write(2).
-*/
- poll(&pfd, 1, -1);
+ io_wait(fd, POLLOUT);
continue;
}
}
--
EW
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] xread: retry after poll on EAGAIN/EWOULDBLOCK
Jeff King wrote:
> On Mon, Jun 27, 2016 at 05:49:48PM -0400, Jeff King wrote:
>
> > So in general I would say that handing non-blocking descriptors to git
> > is not safe.
Indeed. This also makes me wonder if our output to stdout/stderr
suffer from the same theoretical problems if given non-blocking
outputs; I suspect they do.
>> I think it's possible to loop on getdelim() when we see
>> EAGAIN, but I'm not sure if it's worth it.
> The patch for that is below, for the curious. It works with even this:
>
> {
> for i in H E A D; do
> sleep 1
> printf $i
> done
> printf "\n"
> } | nonblock git cat-file --batch-check
>
> Note that I folded the "did we see EAGAIN" check into my sub-function
> (which is the equivalent of your io_wait). You might want to do that in
> the xread() code path, too, as it makes the resulting code there a very
> nice:
>
> if (errno == EINTR)
> continue;
> if (handle_nonblock(fd, POLLIN))
> continue;
Yes :)
> +int handle_nonblock(FILE *fp, short poll_events)
> +{
> + if (ferror(fp) && (errno == EAGAIN || errno == EWOULDBLOCK)) {
> + clearerr(fp);
> + return 1;
> + }
> + return 0;
> +}
> +
> +static int getline_stdio_loop(struct strbuf *sb, FILE *fp, int term)
> +{
> + int ch;
> + do {
> + errno = 0;
> + flockfile(fp);
> + while ((ch = getc_unlocked(fp)) != EOF) {
> + }
> + funlockfile(fp);
> + } while (handle_nonblock(fp, POLLIN));
I haven't used stdio in a while and I'm glad :)
Error handling with ferror + clearerr + errno checking is
difficult and error-prone.
Linus said this back in 2006, too:
http://mid.gmane.org/pine.lnx.4.64.0609141023130.4...@g5.osdl.org
So I wonder if we're better off relying entirely on xread/xwrite
+ strbuf for all our buffering.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git svn clone segmentation faul issue
ioannis.kap...@rbs.com wrote: > Fortunately, a patch has already been submitted to subversion > with (github) revision > a074af86c8764404b28ce99d0bedcb668a321408 (at > https://github.com/apache/subversion/commit/a074af86c8764404b28ce99d0bedcb668a321408 > ) on the trunk to handle this and a couple of other similar > cases. But by the looks of it has not been picked up yet in > the latest subversion 1.9.4 release or the 1.9.x branch, > perhaps because this patch was identified in sanity checks > rather than coming out from a perceivable production issue? Thank you for documenting this. Curious, does this affect older SVN versions or only 1.9.x? I don't know Perl internals well or SWIG at all; so reports like these are very much appreciated. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH v2 00/10] Add initial experimental external ODB support
Christian Couder wrote:
> Design discussion about performance
> ~~~
>
> Yeah, it is not efficient to fork/exec a command to just read or write
> one object to or from the external ODB. Batch calls and/or using a
> daemon and/or RPC should be used instead to be able to store regular
> objects in an external ODB. But for now the external ODB would be all
> about really big files, where the cost of a fork+exec should not
> matter much. If we later want to extend usage of external ODBs, yeah
> we will probably need to design other mechanisms.
I would also investigate switching run_command to use vfork+exec
or posix_spawn for performance (keeping in mind vfork
caveats documented at https://ewontfix.com/7/ )
posix_spawn in future glibc (probably 2.24) will use CLONE_VFORK
in all cases under Linux, and posix_spawn may help with
portability, too. I think the only thing we can't support
with posix_spawn which run_command supports is chdir;
all the redirections/closing FDs should be fine.
With only 10MB malloc-ed, the following shows vfork performance
being noticeably faster than plain fork:
/* gcc -o vfork-test -Wall -O2 vfork-test.c */
#include
#include
#include
#include
#include
int main(int argc, char *argv[], char *envp[])
{
int i;
int do_vfork = argc > 1 && !strcmp(argv[1], "vfork");
char * const cmd[] = { "/bin/true", 0 };
size_t n = 1024 * 1024 * 10;
char *mem = malloc(n);
memset(mem, 'a', n); /* make sure it's really allocated */
for (i = 0; i < 1; i++) {
pid_t pid = do_vfork ? vfork() : fork();
if (pid == 0) {
execve(cmd[0], cmd, envp);
write(2, "exec error\n", 11);
_exit(1);
}
waitpid(pid, 0, 0);
}
return 0;
}
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/9] merge-recursive: avoid returning a wholesale struct
Johannes Schindelin wrote: > P.S.: If it is not too much of a problem, may I ask you to simply delete > remainders of my patches when replying and not commenting on them? I just > deleted 226 lines after verifying that you really did not respond to any > part of it in the unhelpful Alpine client (which I still use because it > *still* fits much better with my workflow than anything else, by a lot). > Again, not a big deal if it would make your life more painful. I find the over-quoting annoying, too. Fwiw, my muttrc has: bind pager , skip-quoted Which allows me to hit the comma key to skip over quoted text. Not sure if Alpine or other clients have something similar. But I'd rather not waste the keystroke/bandwidth/storage. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 3/4] push: accept push options
Stefan Beller wrote:
> @@ -513,6 +531,17 @@ int send_pack(struct send_pack_args *args,
> strbuf_release(&req_buf);
> strbuf_release(&cap_buf);
>
> + if (use_push_options) {
> + struct string_list_item *item;
> + struct strbuf sb = STRBUF_INIT;
> +
> + for_each_string_list_item(item, args->push_options)
> + packet_buf_write(&sb, "%s", item->string);
> + write_or_die(out, sb.buf, sb.len);
write_or_die looks mis-indented.
> + packet_flush(out);
> + strbuf_release(&sb);
> + }
> +
> if (use_sideband && cmds_sent) {
> memset(&demux, 0, sizeof(demux));
> demux.proc = sideband_demux;
> @@ -640,6 +641,7 @@ struct transport *transport_get(struct remote *remote,
> const char *url)
> struct transport *ret = xcalloc(1, sizeof(*ret));
>
> ret->progress = isatty(2);
> + ret->push_options = NULL;
Seems unnecessary to set NULL, here, xcalloc is right above.
> if (!remote)
> die("No remote provided to transport_get()");
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] git-svn: warn instead of dying when commit data is missing
Christian Couder wrote:
> On Fri, Jun 24, 2016 at 10:06 PM, Eric Wong wrote:
> > Jacob Godserv wrote:
> >> > Christian (Cc-ed) also noticed the problem a few weeks ago
> >> > and took a more drastic approach by having git-svn die
> >> > instead of warning:
> >> > http://mid.gmane.org/1462604323-18545-1-git-send-email-chrisc...@tuxfamily.org
> >> > which landed as commit 523a33ca17c76bee007d7394fb3930266c577c02
> >> > in git.git: https://bogomips.org/mirrors/git.git/patch?id=523a33ca17c7
> >> >
> >> > Is dying here too drastic and maybe warn is preferable?
> >>
> >> In my opinion this is too drastic. It keeps me from storing
> >> git-specific data on a git-svn mirror.
> >
> > I tend to agree, but will wait to see what Christian thinks.
>
> Yeah a warning is probably enough.
OK, patch below.
> Another possibility would be to default to an error that tells people
> about a configuration variable that could let them decide depending on
> their workflow if this should be an error, a warning or just be
> ignored.
I think that's too much. I'm not a fan of having too many
configuration variables to throw on users.
--8<--
Subject: [PATCH] git-svn: warn instead of dying when commit data is missing
It is possible to have refs globbed by git-svn which stores data
purely in git; gently skip those instead of dying and assuming
user error.
ref:
http://mid.gmane.org/cali1mtdtnf_gtzyptbfb7n51wwxsfy7zm8hsgwxr3thczzb...@mail.gmail.com
Suggested-by: Jacob Godserv
Cc: Christian Couder
Signed-off-by: Eric Wong
---
perl/Git/SVN.pm | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/perl/Git/SVN.pm b/perl/Git/SVN.pm
index bee1e7d..018beb8 100644
--- a/perl/Git/SVN.pm
+++ b/perl/Git/SVN.pm
@@ -97,8 +97,12 @@ sub resolve_local_globs {
"existing: $existing\n",
" globbed: $refname\n";
}
- my $u = (::cmt_metadata("$refname"))[0] or die
- "$refname: no associated commit metadata\n";
+ my $u = (::cmt_metadata("$refname"))[0];
+ if (!defined($u)) {
+ warn
+"W: $refname: no associated commit metadata from SVN, skipping\n";
+ next;
+ }
$u =~ s!^\Q$url\E(/|$)!! or die
"$refname: '$url' not found in '$u'\n";
if ($pathname ne $u) {
--
EW
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-svn: clone: Fail on missing url argument
Christopher Layne wrote:
> * cmd_clone should detect a missing $url arg before using it otherwise
> an uninitialized value error is emitted in even the simplest case of
> 'git svn clone' without arguments.
Thanks, this patch looks obviously correct.
I've eliminated the '* ' and space prefix from the version I've
applied since it's not the convention around here.
> Signed-off-by: Christopher Layne
Signed-off-by: Eric Wong
And pushed to "master" of git://bogomips.org/git-svn
(I'll request for Junio to pull within a few days while
other changes pile up).
> sub cmd_clone {
> my ($url, $path) = @_;
> - if (!defined $path &&
> + if (!$url) {
> + die "SVN repository location required ",
> + "as a command-line argument\n";
"as a command-line argument" seems like an unnecessary phrase,
but I see we use it elsewhere; so it's fine here.
I might be tempted to queue up a separate patch
to eliminate this extra statement from the rest of git-svn,
though. Not sure if others feel the same way.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] http-walker: remove unused parameter from fetch_object
This parameter has not been used since commit 1d389ab65dc6
("Add support for parallel HTTP transfers") back in 2005
Signed-off-by: Eric Wong
---
I might followup in a few days/weeks with further updates in
this area, but at least should be trivially correct :)
http-walker.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/http-walker.c b/http-walker.c
index 2c721f0..9f28523 100644
--- a/http-walker.c
+++ b/http-walker.c
@@ -447,7 +447,7 @@ static void abort_object_request(struct object_request
*obj_req)
release_object_request(obj_req);
}
-static int fetch_object(struct walker *walker, struct alt_base *repo, unsigned
char *sha1)
+static int fetch_object(struct walker *walker, unsigned char *sha1)
{
char *hex = sha1_to_hex(sha1);
int ret = 0;
@@ -518,7 +518,7 @@ static int fetch(struct walker *walker, unsigned char *sha1)
struct walker_data *data = walker->data;
struct alt_base *altbase = data->alt;
- if (!fetch_object(walker, altbase, sha1))
+ if (!fetch_object(walker, sha1))
return 0;
while (altbase) {
if (!http_fetch_pack(walker, altbase, sha1))
--
EW
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Jul 2016, #03; Fri, 8)
Junio C Hamano wrote: > * ew/svn-bad-ref (2016-07-06) 1 commit > - git-svn: warn instead of dying when commit data is missing > I'm likely to discard this, favouring a direct pull request from > the subsystem maintainer directly going to 'master'. Pushed with your sign-off added, along with Christopher's patch. I was hoping to work on the Apache 2.4 test suite fixes Michael started working on last year this week, maybe next (or very soon) since most of my machines are finally upgraded to Debian Jessie. The following changes since commit cf4c2cfe52be5bd973a4838f73a35d3959ce2f43: Second batch of topics for 2.10 (2016-06-27 10:07:08 -0700) are available in the git repository at: git://bogomips.org/git-svn.git master for you to fetch changes up to 2af7da9f8fb68337030630d88c19db512189babc: git-svn: warn instead of dying when commit data is missing (2016-07-09 22:53:54 +) Christopher Layne (1): git-svn: clone: Fail on missing url argument Eric Wong (1): git-svn: warn instead of dying when commit data is missing git-svn.perl| 5 - perl/Git/SVN.pm | 8 ++-- 2 files changed, 10 insertions(+), 3 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Over-/underquoting, was Re: [PATCH] revert: clarify seemingly bogus OPT_END repetition
Michael J Gruber wrote: > 2016-07-06 22:15 GMT+02:00 Jacob Keller : > > On Wed, Jul 6, 2016 at 6:16 AM, Johannes Schindelin > > wrote: > ... > >>> It is not unheard of that a MUA can collapse and expand properly quoted > >>> parts on request... > >> > >> Sure. Show me some kick-ass, scriptable mail client and I will have a > >> look. I like mutt since it lets me pipe stuff to arbitrary commands, "T" works in the pager for toggling quoted text. > > Yep, I haven't found a mail client that really works for me either :( > > Every other day I want to get rid of Thunderbird... always on the > verge of going (back) to mutt, though I do need some calendar > integration and such. It's possible to use both with IMAP (offlineimap + local dovecot works well on slow connections). I rely on at+atd to email myself and also maintain a text file over IMAP. > Until the day when the scriptable mua we wish for comes upon us, may I > recommend the "QuoteCollapse" extension to Thunderbird that transforms > those huge quotes into neat short headlines with a twist(y). The "T" (toggle-quoted) command in mutt hides quoted entirely, which makes it a bit confusing, at times; but toggling again unhides it. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Jul 2016, #03; Fri, 8)
Junio C Hamano wrote:
> * sb/submodule-parallel-fetch (2016-06-27) 2 commits
> (merged to 'next' on 2016-07-06 at de5fd35)
> + xwrite: poll on non-blocking FDs
> + xread: retry after poll on EAGAIN/EWOULDBLOCK
>
> Fix a recently introduced codepaths that are involved in parallel
> submodule operations, which gave up on reading too early, and
> could have wasted CPU while attempting to write under a corner case
> condition.
>
> Will merge to 'master'.
Any thoughts on my cleanup 3/2 patch for this series?
("hoist out io_wait function for xread and xwrite")
https://public-inbox.org/git/20160627201311.ga7...@dcvr.yhbt.net/raw
Jeff liked it:
https://public-inbox.org/git/20160627214947.ga17...@sigill.intra.peff.net/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[ANNOUNCE] more archives of this list
Very much a work-in-progress, but NNTP and HTTP/HTTPS sorta work based on stuff that is on gmane and stuff I'm accumulating by being a subscriber. The first two Tor hidden service onions are actually on better hardware than the non-hidden public-inbox.org one: nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git nntp://news.public-inbox.org/inbox.comp.version-control.git http://czquwvybam4bgbro.onion/git http://hjrcffqmbrq6wope.onion/git http://ou63pmih66umazou.onion/git https://public-inbox.org/git/ HTTP URLs are clonable, but I've generated the following fast-export dump: https://public-inbox.org/.temp/git.vger.kernel.org-6c38c917e55c.gz (362M) git init --bare mirror.git curl $FAST_EXPORT_GZ_URL | git --git-dir=mirror.git fast-import git --git-dir=mirror.git remote add --mirror=fetch origin $URL I recommend bare repos for importing, since the trees consist of 2/38 SHA-1 paths of Message-IDs and there's nearly 300K messages. In contrast, bundles and packs delta poorly and only get down around 750-800M with aggressive packing (And I haven't done that in a while.) Code is AGPL-3.0+: git clone https://public-inbox.org/ Additional mirrors or forks (perhaps different UIs) are very welcome, as I expect none of my servers or network connections to be reliable. I have the "public-inbox-watch" command running in screen watching my Maildirs, it uses a config file which is parseable/writable using git-config: ==> ~/.public-inbox/config <== [publicinboxlearn] ; spam gets moved here for auto-removal: watchspam = maildir:/path/to/maildirs/.INBOX.spam [publicinboxwatch] ; optional, adds some additional spam checking spamcheck = spamc [publicinbox "git"] ; git repo for this list mainrepo = /path/to/mirror.git ; this removes the list footer signature: filter = PublicInbox::Filter::Vger ; this is where my git-related mail goes (some of it is from Debian) watch = maildir:/path/to/maildirs/.INBOX.git ; only match messages with the correct List-Id header: watchheader = List-Id: ; next 4 lines are only necessary for HTTP and NNTP servers address = git@vger.kernel.org url = http://ou63pmih66umazou.onion/git newsgroup = inbox.comp.version-control.git infourl = http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Jul 2016, #03; Fri, 8)
Jeff King wrote:
> On Sat, Jul 09, 2016 at 11:45:18PM +, Eric Wong wrote:
> > Junio C Hamano wrote:
> > > * sb/submodule-parallel-fetch (2016-06-27) 2 commits
> > > (merged to 'next' on 2016-07-06 at de5fd35)
> > > + xwrite: poll on non-blocking FDs
> > > + xread: retry after poll on EAGAIN/EWOULDBLOCK
> > Any thoughts on my cleanup 3/2 patch for this series?
> > ("hoist out io_wait function for xread and xwrite")
> > https://public-inbox.org/git/20160627201311.ga7...@dcvr.yhbt.net/raw
> >
> > Jeff liked it:
> > https://public-inbox.org/git/20160627214947.ga17...@sigill.intra.peff.net/
>
> I did (and do) like it. I think it may have simply gotten lost in the
> mass of "should we handle EAGAIN from getdelim()" patches I sent (to
> which I concluded "probably not").
>
> There was one minor improvement I suggested[1] (and which you seemed to
> like), which is to push the errno check into the function. That wasn't
> expressed in patch form, though, so I've included below a version of
> your patch with my suggestion squashed in.
Yes, I'm fine with either, but I'm slightly thrown off by
a function relying on errno being set by the caller, even if it
is errno. So maybe localizing it is better (see below)
> I didn't include the test I mentioned in [2]. I think the potential for
> portability and raciness problems make it probably not worth the
> trouble.
Agreed.
> ---
> Since both conditionals just call "continue", you could actually fold
> them into a single if() in each caller, but I think it's easier to
> follow as two separate ones.
>
> You could actually fold the t
Copy-paste error?
> +static int handle_nonblock(int fd, short poll_events)
> +{
> + struct pollfd pfd;
> +
> + if (errno != EAGAIN && errno != EWOULDBLOCK)
> + return 0;
> +
I prefer localizing errno and having the caller pass it
explicitly:
static int handle_nonblock(int fd, short poll_events, int err)
{
struct pollfd pfd;
if (err != EAGAIN && err != EWOULDBLOCK)
return 0;
And the callers would pass errno:
if (handle_nonblock(fd, POLLIN, errno))
continue;
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [ANNOUNCE] more archives of this list
Eric Wong wrote: > https://public-inbox.org/.temp/git.vger.kernel.org-6c38c917e55c.gz > (362M) > > git init --bare mirror.git > curl $FAST_EXPORT_GZ_URL | git --git-dir=mirror.git fast-import Oops, that is missing zcat: curl $FAST_EXPORT_GZ_URL | zcat | git --git-dir=mirror.git fast-import > git --git-dir=mirror.git remote add --mirror=fetch origin $URL And I forgot to set a branch for fast-export and just exported a ref, so importers will need to create master explicitly: git update-ref refs/heads/master 6c38c917e55c -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/2] hoist out handle_nonblock function for xread and xwrite
At least for me, this improves the readability of xread and
xwrite; hopefully allowing missing "continue" statements to
be spotted more easily.
Signed-off-by: Eric Wong
Signed-off-by: Jeff King
---
Jeff King wrote:
> On Sun, Jul 10, 2016 at 03:47:36AM +0000, Eric Wong wrote:
> > Yes, I'm fine with either, but I'm slightly thrown off by
> > a function relying on errno being set by the caller, even if it
> > is errno. So maybe localizing it is better (see below)
>
> Yeah, I had a similar reservation, but didn't want to clutter the
> interface. However, just passing errno isn't too bad (as you showed
> below), and is much less magical.
>
> Do you want to squash that and re-send the whole patch to make Junio's
> life easier?
Done, also updated the subject to match the new function name.
I wasn't attached to the "io_wait" name, either, as it could
be confused with "I/O wait" commonly associated with disk I/O
and not often with socket/pipe I/O that poll gets used for.
wrapper.c | 48
1 file changed, 20 insertions(+), 28 deletions(-)
diff --git a/wrapper.c b/wrapper.c
index 0b920f1..78f6431 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -227,6 +227,24 @@ int xopen(const char *path, int oflag, ...)
}
}
+static int handle_nonblock(int fd, short poll_events, int err)
+{
+ struct pollfd pfd;
+
+ if (err != EAGAIN && err != EWOULDBLOCK)
+ return 0;
+
+ pfd.fd = fd;
+ pfd.events = poll_events;
+
+ /*
+* no need to check for errors, here;
+* a subsequent read/write will detect unrecoverable errors
+*/
+ poll(&pfd, 1, -1);
+ return 1;
+}
+
/*
* xread() is the same a read(), but it automatically restarts read()
* operations with a recoverable error (EAGAIN and EINTR). xread()
@@ -242,21 +260,8 @@ ssize_t xread(int fd, void *buf, size_t len)
if (nr < 0) {
if (errno == EINTR)
continue;
- if (errno == EAGAIN || errno == EWOULDBLOCK) {
- struct pollfd pfd;
- pfd.events = POLLIN;
- pfd.fd = fd;
- /*
-* it is OK if this poll() failed; we
-* want to leave this infinite loop
-* only when read() returns with
-* success, or an expected failure,
-* which would be checked by the next
-* call to read(2).
-*/
- poll(&pfd, 1, -1);
+ if (handle_nonblock(fd, POLLIN, errno))
continue;
- }
}
return nr;
}
@@ -277,21 +282,8 @@ ssize_t xwrite(int fd, const void *buf, size_t len)
if (nr < 0) {
if (errno == EINTR)
continue;
- if (errno == EAGAIN || errno == EWOULDBLOCK) {
- struct pollfd pfd;
- pfd.events = POLLOUT;
- pfd.fd = fd;
- /*
-* it is OK if this poll() failed; we
-* want to leave this infinite loop
-* only when write() returns with
-* success, or an expected failure,
-* which would be checked by the next
-* call to write(2).
-*/
- poll(&pfd, 1, -1);
+ if (handle_nonblock(fd, POLLOUT, errno))
continue;
- }
}
return nr;
--
EW
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] config.mak.uname: define NEEDS_LIBRT under Linux, for now
Junio C Hamano wrote: > Ronald Wampler writes: > > > I am not sure if this the correction solution. Another option I > > considered was to wrap the EXTLIBS += -lrt is an ifndef NO_RT and only > > defining NO_RT for Mac OS X in config.mak.uname. > > That alternative would make the resulting code noisier/uglier with > nested ifdef, I would imagine, but it would be of less impact to the > existing users. But my gut feeling is that the patch you sent is > probably a better solution for the longer term. That change broke my Debian wheezy LTS system, which isn't too out-of-date. I think having excessive linkage on newer systems is preferable to breaking the out-of-the-box experience. I don't know about other platforms, but I think the following will help users on older GNU/Linux systems for the next few years: ---8<-- Subject: [PATCH] config.mak.uname: define NEEDS_LIBRT under Linux, for now My Debian wheezy LTS system is still on glibc 2.13; and LTS distros may use older glibc, still, so lets not unnecessarily break things out-of-the-box. We seem to assume Linux is using glibc in our Makefiles anyways, so I don't think this will introduce new breakage for users of alternative libc implementations. Signed-off-by: Eric Wong --- config.mak.uname | 2 ++ 1 file changed, 2 insertions(+) diff --git a/config.mak.uname b/config.mak.uname index a88f139..22958a8 100644 --- a/config.mak.uname +++ b/config.mak.uname @@ -36,6 +36,8 @@ ifeq ($(uname_S),Linux) HAVE_DEV_TTY = YesPlease HAVE_CLOCK_GETTIME = YesPlease HAVE_CLOCK_MONOTONIC = YesPlease + # -lrt is needed for clock_gettime on glibc <= 2.16 + NEEDS_LIBRT = YesPlease HAVE_GETDELIM = YesPlease SANE_TEXT_GREP=-a endif -- EW -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/3] http: avoid disconnecting on 404s for loose objects
404s are common when fetching loose objects on static HTTP
servers, and reestablishing a connection for every single
404 adds additional latency.
Signed-off-by: Eric Wong
---
http-walker.c | 9 +
http.c| 16 ++--
2 files changed, 23 insertions(+), 2 deletions(-)
diff --git a/http-walker.c b/http-walker.c
index 9f28523..48f2df4 100644
--- a/http-walker.c
+++ b/http-walker.c
@@ -488,6 +488,15 @@ static int fetch_object(struct walker *walker, unsigned
char *sha1)
req->localfile = -1;
}
+ /*
+* we turned off CURLOPT_FAILONERROR to avoid losing a
+* persistent connection and got CURLE_OK.
+*/
+ if (req->http_code == 404 && req->curl_result == CURLE_OK &&
+ (starts_with(req->url, "http://";) ||
+starts_with(req->url, "https://";)))
+ req->curl_result = CURLE_HTTP_RETURNED_ERROR;
+
if (obj_req->state == ABORTED) {
ret = error("Request for %s aborted", hex);
} else if (req->curl_result != CURLE_OK &&
diff --git a/http.c b/http.c
index d8b2bec..e81dd13 100644
--- a/http.c
+++ b/http.c
@@ -1975,8 +1975,19 @@ static size_t fwrite_sha1_file(char *ptr, size_t
eltsize, size_t nmemb,
unsigned char expn[4096];
size_t size = eltsize * nmemb;
int posn = 0;
- struct http_object_request *freq =
- (struct http_object_request *)data;
+ struct http_object_request *freq = data;
+ struct active_request_slot *slot = freq->slot;
+
+ if (slot) {
+ CURLcode c = curl_easy_getinfo(slot->curl, CURLINFO_HTTP_CODE,
+ &slot->http_code);
+ if (c != CURLE_OK)
+ die("BUG: curl_easy_getinfo for HTTP code failed: %s",
+ curl_easy_strerror(c));
+ if (slot->http_code >= 400)
+ return size;
+ }
+
do {
ssize_t retval = xwrite(freq->localfile,
(char *) ptr + posn, size - posn);
@@ -2097,6 +2108,7 @@ struct http_object_request *new_http_object_request(const
char *base_url,
freq->slot = get_active_slot();
curl_easy_setopt(freq->slot->curl, CURLOPT_FILE, freq);
+ curl_easy_setopt(freq->slot->curl, CURLOPT_FAILONERROR, 0);
curl_easy_setopt(freq->slot->curl, CURLOPT_WRITEFUNCTION,
fwrite_sha1_file);
curl_easy_setopt(freq->slot->curl, CURLOPT_ERRORBUFFER, freq->errorstr);
curl_easy_setopt(freq->slot->curl, CURLOPT_URL, freq->url);
--
EW
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/3] http-walker: reduce O(n) ops with doubly-linked list
Using the a Linux-kernel-derived doubly-linked list
implementation from the Userspace RCU library allows us to
enqueue and delete items from the object request queue in
constant time.
This change reduces enqueue times in the prefetch() function
where object request queue could grow to several thousand
objects.
I left out the list_for_each_entry* family macros from list.h
which relied on the __typeof__ operator as we support platforms
without it. Thus, list_entry (aka "container_of") needs to be
called explicitly inside macro-wrapped for loops.
The downside is this costs us an additional pointer per object
request, but this is offset by reduced overhead on queue
operations leading to improved performance and shorter queue
depths.
Signed-off-by: Eric Wong
---
http-walker.c | 42 ++-
list.h| 164 ++
2 files changed, 179 insertions(+), 27 deletions(-)
create mode 100644 list.h
diff --git a/http-walker.c b/http-walker.c
index 48f2df4..0b24255 100644
--- a/http-walker.c
+++ b/http-walker.c
@@ -2,6 +2,7 @@
#include "commit.h"
#include "walker.h"
#include "http.h"
+#include "list.h"
struct alt_base {
char *base;
@@ -23,7 +24,7 @@ struct object_request {
struct alt_base *repo;
enum object_request_state state;
struct http_object_request *req;
- struct object_request *next;
+ struct list_head node;
};
struct alternates_request {
@@ -41,7 +42,7 @@ struct walker_data {
struct alt_base *alt;
};
-static struct object_request *object_queue_head;
+static LIST_HEAD(object_queue_head);
static void fetch_alternates(struct walker *walker, const char *base);
@@ -110,19 +111,10 @@ static void process_object_response(void *callback_data)
static void release_object_request(struct object_request *obj_req)
{
- struct object_request *entry = object_queue_head;
-
if (obj_req->req !=NULL && obj_req->req->localfile != -1)
error("fd leakage in release: %d", obj_req->req->localfile);
- if (obj_req == object_queue_head) {
- object_queue_head = obj_req->next;
- } else {
- while (entry->next != NULL && entry->next != obj_req)
- entry = entry->next;
- if (entry->next == obj_req)
- entry->next = entry->next->next;
- }
+ list_del(&obj_req->node);
free(obj_req);
}
@@ -130,8 +122,10 @@ static void release_object_request(struct object_request
*obj_req)
static int fill_active_slot(struct walker *walker)
{
struct object_request *obj_req;
+ struct list_head *pos, *tmp, *head = &object_queue_head;
- for (obj_req = object_queue_head; obj_req; obj_req = obj_req->next) {
+ list_for_each_safe(pos, tmp, head) {
+ obj_req = list_entry(pos, struct object_request, node);
if (obj_req->state == WAITING) {
if (has_sha1_file(obj_req->sha1))
obj_req->state = COMPLETE;
@@ -148,7 +142,6 @@ static int fill_active_slot(struct walker *walker)
static void prefetch(struct walker *walker, unsigned char *sha1)
{
struct object_request *newreq;
- struct object_request *tail;
struct walker_data *data = walker->data;
newreq = xmalloc(sizeof(*newreq));
@@ -157,18 +150,9 @@ static void prefetch(struct walker *walker, unsigned char
*sha1)
newreq->repo = data->alt;
newreq->state = WAITING;
newreq->req = NULL;
- newreq->next = NULL;
http_is_verbose = walker->get_verbosely;
-
- if (object_queue_head == NULL) {
- object_queue_head = newreq;
- } else {
- tail = object_queue_head;
- while (tail->next != NULL)
- tail = tail->next;
- tail->next = newreq;
- }
+ list_add_tail(&newreq->node, &object_queue_head);
#ifdef USE_CURL_MULTI
fill_active_slots();
@@ -451,11 +435,15 @@ static int fetch_object(struct walker *walker, unsigned
char *sha1)
{
char *hex = sha1_to_hex(sha1);
int ret = 0;
- struct object_request *obj_req = object_queue_head;
+ struct object_request *obj_req = NULL;
struct http_object_request *req;
+ struct list_head *pos, *head = &object_queue_head;
- while (obj_req != NULL && hashcmp(obj_req->sha1, sha1))
- obj_req = obj_req->next;
+ list_for_each(pos, head) {
+ obj_req = list_entry(pos, struct object_request, node);
+ if (!hashcmp(obj_req->sha1, sha1))
+ break;
+ }
if (obj_req == NULL)
return error("Couldn't fi
[PATCH 1/3] http-walker: remove unused parameter from fetch_object
This parameter has not been used since commit 1d389ab65dc6
("Add support for parallel HTTP transfers") back in 2005
Signed-off-by: Eric Wong
---
http-walker.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/http-walker.c b/http-walker.c
index 2c721f0..9f28523 100644
--- a/http-walker.c
+++ b/http-walker.c
@@ -447,7 +447,7 @@ static void abort_object_request(struct object_request
*obj_req)
release_object_request(obj_req);
}
-static int fetch_object(struct walker *walker, struct alt_base *repo, unsigned
char *sha1)
+static int fetch_object(struct walker *walker, unsigned char *sha1)
{
char *hex = sha1_to_hex(sha1);
int ret = 0;
@@ -518,7 +518,7 @@ static int fetch(struct walker *walker, unsigned char *sha1)
struct walker_data *data = walker->data;
struct alt_base *altbase = data->alt;
- if (!fetch_object(walker, altbase, sha1))
+ if (!fetch_object(walker, sha1))
return 0;
while (altbase) {
if (!http_fetch_pack(walker, altbase, sha1))
--
EW
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC 0/3] dumb HTTP transport speedups
TL;DR: dumb HTTP clone from a certain badly-packed repo goes from ~2 hours to ~30 min memory usage drops from 2G to 360M I hadn't packed the public repo at https://public-inbox.org/git for a few weeks. As an admin of a small server limited memory and CPU resources but fairly good bandwidth, I prefer clients use dumb HTTP for initial clones. Unfortunately, I noticed my dinky netbook runs out-of-memory when using GIT_SMART_HTTP=0 to clone this giant repo; and a machine with more memory still takes over two hours depending on network conditions (and uses around 2GB RSS!). Anyways, https://public-inbox.org/git is better packed, now; but I've kept https://80x24.org/git-i-forgot-to-pack available with over 7K loose objects to illustrate the problem: (this is dumb HTTP-only) git clone --mirror https://80x24.org/git-i-forgot-to-pack The primary problem is fixed by PATCH 3/3 in this series, and I can now clone the above in around 30 minutes and "only" seems to use around 360M memory. I'll leave git-i-forgot-to-pack up for a few months/year so others can test and hammer away at it. The following changes since commit 5c589a73de4394ad125a4effac227b3aec856fa1: Third batch of topics for 2.10 (2016-07-06 13:42:58 -0700) are available in the git repository at: git://bogomips.org/git-svn.git dumb-speedups for you to fetch changes up to b9d5aca4b8e6c9f7fb5ee4e0ce33bb42c4ea2992: http-walker: reduce O(n) ops with doubly-linked list (2016-07-11 20:25:51 +) ---- Eric Wong (3): http-walker: remove unused parameter from fetch_object http: avoid disconnecting on 404s for loose objects http-walker: reduce O(n) ops with doubly-linked list http-walker.c | 55 ++-- http.c| 16 +- list.h| 164 ++ 3 files changed, 204 insertions(+), 31 deletions(-) create mode 100644 list.h -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[REJECT 4/3] http-walker: use hashmap to reduce list scan
For the sake of documentation, I worked on this patch but I
don't there was a measurable improvement (hard to tell with
variable network conditions) and it increased memory usage to
around 380M.
I wanted to reduce the list scanning in fill_active_slot() by
deleting during iteration, but I'm not sure it helps since the
loop in that is nowhere near as bad as the prefetch() insertion
loop fixed in 3/3.
list_for_each in fetch_object() also tends hit the first object
in the list when iterating, so there's no improvement I see
with this patch.
-8<--
Subject: [PATCH] http-walker: use hashmap to reduce list scan
We can reduce list walking in fill_active_slot by deleting items
as we walk through the object request queue of pending objects.
However, we still need to maintain a mapping of live objects
for fetch_object, so introduce the use of a hashmap to keep
track of all live object requests in O(1) average time.
Signed-off-by: Eric Wong
---
http-walker.c | 35 +++
1 file changed, 27 insertions(+), 8 deletions(-)
diff --git a/http-walker.c b/http-walker.c
index 0b24255..8d27707 100644
--- a/http-walker.c
+++ b/http-walker.c
@@ -3,6 +3,7 @@
#include "walker.h"
#include "http.h"
#include "list.h"
+#include "hashmap.h"
struct alt_base {
char *base;
@@ -19,6 +20,7 @@ enum object_request_state {
};
struct object_request {
+ struct hashmap_entry ent;
struct walker *walker;
unsigned char sha1[20];
struct alt_base *repo;
@@ -43,6 +45,7 @@ struct walker_data {
};
static LIST_HEAD(object_queue_head);
+static struct hashmap *object_requests;
static void fetch_alternates(struct walker *walker, const char *base);
@@ -114,7 +117,11 @@ static void release_object_request(struct object_request
*obj_req)
if (obj_req->req !=NULL && obj_req->req->localfile != -1)
error("fd leakage in release: %d", obj_req->req->localfile);
- list_del(&obj_req->node);
+ /* XXX seems unnecessary with list_del in fill_active_slot */
+ if (!list_empty(&obj_req->node))
+ list_del(&obj_req->node);
+
+ hashmap_remove(object_requests, obj_req, obj_req->sha1);
free(obj_req);
}
@@ -127,6 +134,8 @@ static int fill_active_slot(struct walker *walker)
list_for_each_safe(pos, tmp, head) {
obj_req = list_entry(pos, struct object_request, node);
if (obj_req->state == WAITING) {
+ /* _init so future list_del is idempotent */
+ list_del_init(pos);
if (has_sha1_file(obj_req->sha1))
obj_req->state = COMPLETE;
else {
@@ -145,6 +154,8 @@ static void prefetch(struct walker *walker, unsigned char
*sha1)
struct walker_data *data = walker->data;
newreq = xmalloc(sizeof(*newreq));
+ hashmap_entry_init(&newreq->ent, sha1hash(sha1));
+ hashmap_add(object_requests, &newreq->ent);
newreq->walker = walker;
hashcpy(newreq->sha1, sha1);
newreq->repo = data->alt;
@@ -435,15 +446,12 @@ static int fetch_object(struct walker *walker, unsigned
char *sha1)
{
char *hex = sha1_to_hex(sha1);
int ret = 0;
- struct object_request *obj_req = NULL;
+ struct object_request *obj_req;
+ struct hashmap_entry key;
struct http_object_request *req;
- struct list_head *pos, *head = &object_queue_head;
- list_for_each(pos, head) {
- obj_req = list_entry(pos, struct object_request, node);
- if (!hashcmp(obj_req->sha1, sha1))
- break;
- }
+ hashmap_entry_init(&key, sha1hash(sha1));
+ obj_req = hashmap_get(object_requests, &key, sha1);
if (obj_req == NULL)
return error("Couldn't find request for %s in the queue", hex);
@@ -553,6 +561,12 @@ static void cleanup(struct walker *walker)
}
}
+static int obj_req_cmp(const struct object_request *e1,
+ const struct object_request *e2, const unsigned char *sha1)
+{
+ return hashcmp(e1->sha1, sha1 ? sha1 : e2->sha1);
+}
+
struct walker *get_http_walker(const char *url)
{
char *s;
@@ -580,5 +594,10 @@ struct walker *get_http_walker(const char *url)
add_fill_function(walker, (int (*)(void *)) fill_active_slot);
#endif
+ if (!object_requests) {
+ object_requests = xmalloc(sizeof(*object_requests));
+ hashmap_init(object_requests, (hashmap_cmp_fn)obj_req_cmp, 0);
+ }
+
return walker;
}
--
EW
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [ANNOUNCE] Git v2.9.1
Jeff King wrote: > Otherwise, we'll have to skip the test, perhaps with something like the > patch below. I suspect the problem is actually the size of "unsigned > long", not time_t, as we use that internally for a bunch of time > computation. We should probably be using int64_t for time calculations; "unsigned long" is 32-bits on 32-bit x86 systems. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Plugin mechanism(s) for Git?
Konstantin Khomoutov wrote: > Ben Peart wrote: > > > Lars Schneider wrote: > > > > > > We could dynamically load libraries but this would force us to > > > freeze the ABI as mentioned by Duy: > > > http://article.gmane.org/gmane.comp.version-control.git/298463 > > > > > > > I wouldn’t be too quick to dismiss dynamically loaded libraries as > > there are some distinct advantages over the other patterns especially > > performance and simplicity. I realize it requires us to version the > > ABI but there are established patterns to manage this. It also isn’t > > that much different than us having to freeze or version the protocol > > for communicating with a remote-helper. (re-adding dropped CCs) The critical difference is protocols can be tested and debugged in a language/tool-independent way. > Using dynamically loaded libraries precludes or greatly complicates > creation of plugins written in languages different from C (or C++ FWIW). Agreed, and folks working on language bindings are often not experts in or comfortable working in C (I speak for myself, at least). It's probably not a good use of git core developers time to learn to deal with bindings and quirks for language XYZ, either (as language XYZ likely breaks compatibility more than C, POSIX or the git data model). The SVN Perl XS bindings we use for git-svn have introduced numerous incompatibilities and bugs over the years we've had to deal with. I doubt Perl bindings are a high priority for SVN developers; and even checking their API headers reveals a huge number of deprecated functions in their native C API. I'm not sure if it's a lack of foresight on their part or what, but I'm happy git's data model and command-line interface has barely had to deprecate or break compatibility in over a decade. Protocols like "cat-file --batch" and fast-import using pipes is great and I hope they're portable enough. I would welcome additions to avoid process spawning costs for things like update-ref/rev-parse/etc... -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Jul 2016, #05; Wed, 13)
Lars Schneider wrote: > > On 13 Jul 2016, at 18:56, Junio C Hamano wrote: > > * ew/http-walker (2016-07-12) 3 commits > > - http-walker: reduce O(n) ops with doubly-linked list > > - http: avoid disconnecting on 404s for loose objects > > - http-walker: remove unused parameter from fetch_object > > > > Optimize dumb http transport on the client side. > > It looks like as if this topic breaks the OS X build because > it defines LIST_HEAD. LIST_HEAD is already defined in > /usr/include/sys/queue.h. Oops, I suppose GIT_LIST_HEAD is an acceptable name? (looks a bit like a refname, though...). Or maybe CDS_LIST_HEAD (since I originally took it from the cds namespace under urcu) I also wonder where we use sys/queue.h, since I use LIST_HEAD from ccan/list/list.h in a different project without conflicts... > See: https://travis-ci.org/git/git/jobs/145121761 Is there a way to get plain-text logs? No JavaScript, here. Thanks. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] list: avoid incompatibility with *BSD sys/queue.h
Eric Wong wrote:
> Lars Schneider wrote:
> > It looks like as if this topic breaks the OS X build because
> > it defines LIST_HEAD. LIST_HEAD is already defined in
> > /usr/include/sys/queue.h.
>
> Oops, I suppose GIT_LIST_HEAD is an acceptable name?
> (looks a bit like a refname, though...).
>
> Or maybe CDS_LIST_HEAD (since I originally took it from the cds
> namespace under urcu)
Naming things is hard; I think it's better to just undef an
existing LIST_HEAD, instead, since it's unlikely we'd ever use
sys/queue.h from *BSD. (sys/queue.h is branchier, and IMHO
sys/queue.h macros are uglier than list_entry (container_of))
> I also wonder where we use sys/queue.h, since I use
> LIST_HEAD from ccan/list/list.h in a different project
> without conflicts...
Still wondering... Checking sys/mman.h in an old FreeBSD source
tree I had lying around reveals "#include " is
guarded by "#if defined(_KERNEL)", so it mman.h wouldn't pull
it in for userspace builds...
-8<--
Subject: [PATCH] list: avoid incompatibility with *BSD sys/queue.h
Somehow, the OS X build pulls in sys/queue.h and causes
conflicts with the LIST_HEAD macro, here.
ref: http://mid.gmane.org/fb76544f-16f7-45ca-9649-fd62ee44b...@gmail.com
Reported-by: Lars Schneider
Signed-off-by: Eric Wong
---
list.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/list.h b/list.h
index f65edce..a226a87 100644
--- a/list.h
+++ b/list.h
@@ -36,6 +36,8 @@ struct list_head {
struct list_head *next, *prev;
};
+/* avoid conflicts with BSD-only sys/queue.h */
+#undef LIST_HEAD
/* Define a variable with the head and tail of the list. */
#define LIST_HEAD(name) \
struct list_head name = { &(name), &(name) }
--
EW
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] list: avoid incompatibility with *BSD sys/queue.h
Eric Sunshine wrote: > On Sat, Jul 16, 2016 at 8:25 PM, Eric Wong wrote: > > Eric Wong wrote: > >> I also wonder where we use sys/queue.h, since I use > >> LIST_HEAD from ccan/list/list.h in a different project > >> without conflicts... > > > > Still wondering... Checking sys/mman.h in an old FreeBSD source > > tree I had lying around reveals "#include " is > > guarded by "#if defined(_KERNEL)", so it mman.h wouldn't pull > > it in for userspace builds... > > It's pulled in like this: > > git-compat-util.h -> > sys/sysctl.h -> > sys/ucred.h -> > sys/queue.h Ah, thanks (and I've been sidetracked into using FreeBSD again :>) > Very reminiscent of [1]. > > [1]: > http://git.661346.n2.nabble.com/PATCH-ewah-bitmap-silence-warning-about-MASK-macro-redefinition-td7632287.html Alright. In this case, I think "LIST_HEAD" is sufficiently descriptive compared to more-generically named "MASK" or "BLOCK" that it's harmless to "#undef LIST_HEAD" here without worrying about conflicts or breaking outside code. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git-svn dies all the time
Alexandre Bique wrote:
> Hi,
>
> I did by mistake report the bug here:
>
> https://github.com/git/git-scm.com/issues/807#issuecomment-233200404
>
> In short this is a backtrace of git svn rebase crashing in perl, and
> likely because of an out of memory issue. It crashed on my laptop but
> not on my desktop (4G of RAM vs 16G of RAM).
This might be the same bug in SVN Ioannis (cc-ed) told us about
a few weeks ago:
https://public-inbox.org/git/0bca1e695085c645b9cd4a27dd59f6fa39aad...@gbwgceuhubd0101.rbsres07.net/T/
Curious, which version of SVN bindings are you using?
("git svn --version" should tell you)
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] daemon: ignore ENOTSOCK from setsockopt
In inetd mode, we are not guaranteed stdin or stdout is a
socket; callers could filter the data through a pipe
or be testing with regular files.
This prevents t5802 from polluting syslog.
Signed-off-by: Eric Wong
---
daemon.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/daemon.c b/daemon.c
index 46dddac..a844951 100644
--- a/daemon.c
+++ b/daemon.c
@@ -673,9 +673,11 @@ static void set_keep_alive(int sockfd)
{
int ka = 1;
- if (setsockopt(sockfd, SOL_SOCKET, SO_KEEPALIVE, &ka, sizeof(ka)) < 0)
- logerror("unable to set SO_KEEPALIVE on socket: %s",
- strerror(errno));
+ if (setsockopt(sockfd, SOL_SOCKET, SO_KEEPALIVE, &ka, sizeof(ka)) < 0) {
+ if (errno != ENOTSOCK)
+ logerror("unable to set SO_KEEPALIVE on socket: %s",
+ strerror(errno));
+ }
}
static int execute(void)
--
EW
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] test-lib: stricter unzip(1) check
On FreeBSD 10.3 (but presumably any FreeBSD 8+), /usr/bin/unzip
exists, but is insufficient for t5003 due to its non-standard
handling of the -a option[1]. This version of unzip exits
with "1" when given the "-v" flag.
However, the common Info-ZIP version may be installed at
/usr/local/bin/unzip (via "pkg install unzip") to pass t5003.
This Info-ZIP version exits with "0" when given "-v",
so limit the prereq to only versions which return 0 on "-v".
[1]
https://www.freebsd.org/cgi/man.cgi?query=unzip&sektion=1&manpath=FreeBSD+10.3-RELEASE
Signed-off-by: Eric Wong
---
t/test-lib.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 11201e9..938f788 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -1103,7 +1103,7 @@ test_lazy_prereq SANITY '
GIT_UNZIP=${GIT_UNZIP:-unzip}
test_lazy_prereq UNZIP '
"$GIT_UNZIP" -v
- test $? -ne 127
+ test $? -eq 0
'
run_with_limited_cmdline () {
--
EW
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] pager: disable color when pager is "more"
Johannes Schindelin wrote:
> On Sun, 17 Jul 2016, n...@dad.org wrote:
> > 'git diff' outputs escape characters which clutter my terminal. Yes, I
> > can sed them out, but then why are they there?
>
> Those are most likely the ANSI sequences to add color. Can you call Git
> with the --no-color option and see whether the escape characters go away?
Norm: do you have PAGER=more set by any chance?
Perhaps changing it to "less" will allow you to preserve colors.
I saw a similar or identical problem during my vacation in
FreeBSD-land. Perhaps the out-of-the-box experience can be
improved:
-8<-
Subject: [PATCH] pager: disable color when pager is "more"
more(1) traditionally cannot handle colors.
On FreeBSD 10.3, a new user ~/.profile explicitly sets
PAGER=more, but does not configure it to display colors, leading
to a bad out-of-the-box experience with escape sequences being
seen by the user.
In the FreeBSD 10.3 case, /usr/bin/more is actually a hardlink
to /usr/bin/less and capable of handling colors. While we could
set MORE=FRX, this breaks other more(1) implementations,
including the one provided by util-linux on common GNU/Linux
systems.
So take the safe route and assume anybody still using more(1)
today can live with monochrome output, but acknowledge 'R'
in the MORE environment variable if it was set by the user.
Signed-off-by: Eric Wong
---
environment.c | 2 +-
pager.c | 11 +++
2 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/environment.c b/environment.c
index ca72464..cfb56fd 100644
--- a/environment.c
+++ b/environment.c
@@ -41,7 +41,7 @@ size_t packed_git_limit = DEFAULT_PACKED_GIT_LIMIT;
size_t delta_base_cache_limit = 96 * 1024 * 1024;
unsigned long big_file_threshold = 512 * 1024 * 1024;
const char *pager_program;
-int pager_use_color = 1;
+int pager_use_color = -1;
const char *editor_program;
const char *askpass_program;
const char *excludes_file;
diff --git a/pager.c b/pager.c
index 4bc0481..3110df4 100644
--- a/pager.c
+++ b/pager.c
@@ -80,6 +80,17 @@ void setup_pager(void)
if (!pager)
return;
+ if (pager_use_color < 0 && !strcmp(pager, "more")) {
+ const char *more = getenv("MORE");
+
+ /*
+* MORE=R does not work everywhere, so we cannot set it,
+* but we can respect it if set.
+*/
+ if (!more || !strchr(more, 'R'))
+ pager_use_color = 0;
+ }
+
/*
* force computing the width of the terminal before we redirect
* the standard output to the pager.
--
EW
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] configure.ac: stronger test for pthread linkage
We need to test linkage of pthread_create and pthread_join,
as pthread_mutex_* and pthread_key_* functions do not need
extra linkage under FreeBSD 10.3, leading to a false-positive
of the empty case.
Signed-off-by: Eric Wong
---
configure.ac | 5 +
1 file changed, 5 insertions(+)
diff --git a/configure.ac b/configure.ac
index c279025..aa9c91d 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1108,14 +1108,19 @@ GIT_CONF_SUBST([HAVE_BSD_SYSCTL])
AC_DEFUN([PTHREADTEST_SRC], [
AC_LANG_PROGRAM([[
#include
+static void *noop(void *ignore) { return ignore; }
]], [[
pthread_mutex_t test_mutex;
pthread_key_t test_key;
+ pthread_t th;
int retcode = 0;
+ void *ret = (void *)0;
retcode |= pthread_key_create(&test_key, (void *)0);
retcode |= pthread_mutex_init(&test_mutex,(void *)0);
retcode |= pthread_mutex_lock(&test_mutex);
retcode |= pthread_mutex_unlock(&test_mutex);
+ retcode |= pthread_create(&th, ret, noop, ret);
+ retcode |= pthread_join(th, &ret);
return retcode;
]])])
--
EW
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] test-lib: stricter unzip(1) check
Junio C Hamano wrote: > +test_lazy_prereq UNZIP_AUTOTEXT ' > + ( > + mkdir unzip-autotext && > + cd unzip-autotext > + "$GIT_UNZIP" -a "$TEST_DIRECTORY"/t5003/infozip-text.zip && > + test -f text > + ) /usr/bin/unzip actually takes -a on FreeBSD, just not in the same way the Info-ZIP version does, so I suspect "test -f" here is not enough. I would test this, but I can't apply it: > diff --git a/t/t5003/infozip-text.zip b/t/t5003/infozip-text.zip > new file mode 100644 > index 000..a019acb > Binary files /dev/null and b/t/t5003/infozip-text.zip differ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] test-lib: stricter unzip(1) check
Junio C Hamano wrote: > Eric Wong writes: > > Junio C Hamano wrote: > >> +test_lazy_prereq UNZIP_AUTOTEXT ' > >> + ( > >> + mkdir unzip-autotext && > >> + cd unzip-autotext > >> + "$GIT_UNZIP" -a "$TEST_DIRECTORY"/t5003/infozip-text.zip && > >> + test -f text > >> + ) > > > > /usr/bin/unzip actually takes -a on FreeBSD, just not in the > > same way the Info-ZIP version does, so I suspect "test -f" > > here is not enough. > > Hmph. So it only and always does "CRLF -> LF", while Info-ZIP > version does something like autocrlf? No, it does CRLF -> LF based on the absence of non-ASCII characters and ignoring metadata set in the zipfile. The unzip manpage states: Normally, the -a option should only affect files which are marked as text files in the zipfile's central directory. Since the archive(3) library reads zipfiles sequentially, and does not use the central directory, that information is not available to the unzip utility. Instead, the unzip utility will assume that a file is a text file if no non-ASCII characters are present within the first block of data decompressed for that file. If non-ASCII characters appear in subsequent blocks of data, a warning will be issued. https://www.freebsd.org/cgi/man.cgi?query=unzip&sektion=1&manpath=FreeBSD+10.3-RELEASE > Heh. It was created like so: > > $ printf 'text\r\n' >text && zip -ll infozip-text.zip text > $ zipinfo infozip-text.zip text > -rw-r- 3.0 unx5 tx stor 16-Jul-18 13:12 text > Thanks, but I think the test file is too small. I tried setting up a test to store the text file as binary in the zip to check for inadvertant CRLF conversions: printf 'text\r\n' >binary && zip -B infozip-binary.zip binary But zip -B/--binary only works on VM/CMS and MVS... So I'm inclined to go with Dscho's patch. (apologies for messing up René's name in my previous email; on my new FreeBSD setup: mutt displays it fine, as does vim when taking it from .mailmap, but something gets lost when mutt populates the file for vim. Perhaps some Debian patch didn't make it upstream...) -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
t7063 failure on FreeBSD 10.3 i386/amd64
Not sure what's going on, below is the relevant output when run with -i -v --tee (the rest succeeds without -i): ok 26 - untracked cache correct after status expecting success: avoid_racy && : >../trace && GIT_TRACE_UNTRACKED_STATS="$TRASH_DIRECTORY/trace" \ git status --porcelain >../status.actual && cat >../status.expect <../trace.expect
Re: t7063 failure on FreeBSD 10.3 i386/amd64
Oops, forgot to Cc some folks who worked on this :x Filesystem is ufs and it fails regardless of whether soft-updates is enabled or not. Eric Wong wrote: > Not sure what's going on, below is the relevant output when > run with -i -v --tee (the rest succeeds without -i): > > ok 26 - untracked cache correct after status > > expecting success: > avoid_racy && > : >../trace && > GIT_TRACE_UNTRACKED_STATS="$TRASH_DIRECTORY/trace" \ > git status --porcelain >../status.actual && > cat >../status.expect < M done/two > ?? .gitignore > ?? done/five > ?? dtwo/ > EOF > test_cmp ../status.expect ../status.actual && > cat >../trace.expect < node creation: 0 > gitignore invalidation: 0 > directory invalidation: 0 > opendir: 0 > EOF > test_cmp ../trace.expect ../trace > > --- ../trace.expect 2016-07-18 22:23:28.679886000 + > +++ ../trace 2016-07-18 22:23:28.677135000 + > @@ -1,4 +1,4 @@ > node creation: 0 > gitignore invalidation: 0 > -directory invalidation: 0 > -opendir: 0 > +directory invalidation: 1 > +opendir: 1 > not ok 27 - test sparse status again with untracked cache -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] pager: disable color when pager is "more"
Junio C Hamano wrote: > Jeff King writes: > > On Mon, Jul 18, 2016 at 09:19:07AM +0000, Eric Wong wrote: > >> Johannes Schindelin wrote: > >> > On Sun, 17 Jul 2016, n...@dad.org wrote: > >> > > 'git diff' outputs escape characters which clutter my terminal. Yes, I > >> > > can sed them out, but then why are they there? > >> > > >> > Those are most likely the ANSI sequences to add color. Can you call Git > >> > with the --no-color option and see whether the escape characters go away? > >> > >> Norm: do you have PAGER=more set by any chance? > >> Perhaps changing it to "less" will allow you to preserve colors. > >> > >> I saw a similar or identical problem during my vacation in > >> FreeBSD-land. Perhaps the out-of-the-box experience can be > >> improved: > >> > >> -8<- > >> Subject: [PATCH] pager: disable color when pager is "more" > > > > This is the tip of a smaller iceberg. See > > > > http://public-inbox.org/git/52D87A79.6060600%40rawbw.com/t/#u > > > > for more discussion, and some patches that fix more cases (like "LESS" > > without "R", or "more" that _does_ understand "R"). I think it was > > discarded as being a little too intimate with the details of pagers, but > > it does suck that the out-of-the-box experience on FreeBSD is not good. > > Maybe we should revisit it. Yes; I'd prefer not to get too intimate with the details of pagers, either, and I think we should err on the side of monochrome for systems we do not know much about. > Yup, the three-patch series at > > > http://public-inbox.org/git/20140117041430.GB19551%40sigill.intra.peff.net/ I am not a fan of adding #ifdefs for platform-specific things; so I prefer starting with my original patch to disable colors for "more". (or, even disable colors for everything which is not "less" or "lv") > would be a safe starting point that is low-impact. I think what > ended up being discarded was a more elaborate side topic that > started from exploring the possibility of checking if LESS has 'R' > in it to see if it is possible to help people with LESS that does > not allow coloring explicitly exported. Heh... (see below) > I do not think the approach in the same thread suggested by Kyle > > > http://public-inbox.org/git/62DB6DEF-8B39-4481-BA06-245BF45233E5%40gmail.com/ > > is too bad, either. I like Kyle's suggestion, and I think that can be a good transition from your original patch to move pager configuration into the build: https://public-inbox.org/git/xmqq61piw4yf@gitster.dls.corp.google.com/ I've updated just that and pushed just that to the "pager-build" topic of git://bogomips.org/git-svn So I'd prefer we drop the later automatic header generation changes that got squashed into later iterations. Unfortunately, it looks like that all got lost in Jeff's 13-patch "makefile refactoring" topic starting at: https://public-inbox.org/git/20140205174823.ga15...@sigill.intra.peff.net/ Yeah, we tend to get sidetracked :x -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] git-svn: document svn.authorsProg in config
This has always been supported since we read config variables based on the command-line option parser. Document it explicitly since users usually want to maintain the same program across invocations. Signed-off-by: Eric Wong --- Pushed to master of bogomips.org/git-svn.git, but I might have more small git-svn changes coming in over the week as I work on them.. Documentation/git-svn.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Documentation/git-svn.txt b/Documentation/git-svn.txt index 7e17cad..5f9e65b 100644 --- a/Documentation/git-svn.txt +++ b/Documentation/git-svn.txt @@ -625,6 +625,9 @@ config key: svn.authorsfile with the committer name as the first argument. The program is expected to return a single line of the form "Name ", which will be treated as if included in the authors file. ++ +[verse] +config key: svn.authorsProg -q:: --quiet:: -- EW -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC] git-svn: allow --version to work anywhere
Checking the version of the installed SVN libraries should not
require a git repository at all. This matches the behavior of
"git --version".
Add a test for "git svn help" for the same behavior while we're
at it, too.
Signed-off-by: Eric Wong
---
I'm hoping "cd /" in the test will always succeed;
but I suppose non-*nix systems might fail, here.
And maybe a BOFH did "chmod 700 /":(
Anyways this is sitting in master of git://bogomips.org/git-svn.git
git-svn.perl | 4 ++--
t/t9100-git-svn-basic.sh | 8
2 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/git-svn.perl b/git-svn.perl
index f609e54..4d41d22 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -339,7 +339,7 @@ if ($cmd && $cmd =~ /(?:clone|init|multi-init)$/) {
die "failed to open $ENV{GIT_DIR}: $!\n";
$ENV{GIT_DIR} = $1 if <$fh> =~ /^gitdir: (.+)$/;
}
-} else {
+} elsif ($cmd) {
my ($git_dir, $cdup);
git_cmd_try {
$git_dir = command_oneline([qw/rev-parse --git-dir/]);
@@ -356,7 +356,7 @@ if ($cmd && $cmd =~ /(?:clone|init|multi-init)$/) {
my %opts = %{$cmd{$cmd}->[2]} if (defined $cmd);
-read_git_config(\%opts);
+read_git_config(\%opts) if $ENV{GIT_DIR};
if ($cmd && ($cmd eq 'log' || $cmd eq 'blame')) {
Getopt::Long::Configure('pass_through');
}
diff --git a/t/t9100-git-svn-basic.sh b/t/t9100-git-svn-basic.sh
index 28082b1..10408d0 100755
--- a/t/t9100-git-svn-basic.sh
+++ b/t/t9100-git-svn-basic.sh
@@ -19,6 +19,14 @@ case "$GIT_SVN_LC_ALL" in
;;
esac
+test_expect_success 'git svn --version works anywhere' '
+ ( cd / || exit 0; git svn --version )
+'
+
+test_expect_success 'git svn help works anywhere' '
+ ( cd / || exit 0; git svn help )
+'
+
test_expect_success \
'initialize git svn' '
mkdir import &&
--
EW
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] config.mak.uname: set PERL_PATH for FreeBSD 5.0+
Perl has not been part of the base system since FreeBSD 5.0: https://www.freebsd.org/releases/5.0R/relnotes-i386.html Signed-off-by: Eric Wong --- Does anybody still run git on FreeBSD 4.x or earlier? 4.11 was released a few months before git in 2005: https://www.freebsd.org/releases/ config.mak.uname | 5 + 1 file changed, 5 insertions(+) diff --git a/config.mak.uname b/config.mak.uname index a88f139..6c29545 100644 --- a/config.mak.uname +++ b/config.mak.uname @@ -202,6 +202,11 @@ ifeq ($(uname_S),FreeBSD) NO_UINTMAX_T = YesPlease NO_STRTOUMAX = YesPlease endif + R_MAJOR := $(shell expr "$(uname_R)" : '\([0-9]*\)\.') + + ifeq ($(shell test "$(R_MAJOR)" -ge 5 && echo 1),1) + PERL_PATH = /usr/local/bin/perl + endif PYTHON_PATH = /usr/local/bin/python HAVE_PATHS_H = YesPlease GMTIME_UNRELIABLE_ERRORS = UnfortunatelyYes -- EW -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: t7063 failure on FreeBSD 10.3 i386/amd64
Duy Nguyen wrote: > On Tue, Jul 19, 2016 at 12:54 AM, Eric Wong wrote: > > Oops, forgot to Cc some folks who worked on this :x > > > > Filesystem is ufs and it fails regardless of whether > > soft-updates is enabled or not. > > Nothing stands out to my eyes, so I'm going to install freebsd this > weekend. I hope ufs does not have any nasty surprise for me. Stay > tuned. Thanks, this problem might be ufs-specific, tmpfs is fine. Tested tmpfs with: kldload tmpfs mkdir /tmp/tmpfs mount -t tmpfs tmpfs /tmp/tmpfs (Documenting all this since much of this is new to me) I noticed FreeBSD now provides ready-to-run VM images along with normal installation stuff, including qcow2 ones for QEMU users, so that saves some time. http://ftp.freebsd.org/pub/FreeBSD/releases/VM-IMAGES/10.3-RELEASE/amd64/Latest/FreeBSD-10.3-RELEASE-amd64.qcow2.xz Notes: * "-net user" because I'm lazy (ICMP ping won't work out-of-the-box) * kvm can be substituted for qemu-system-$ARCH for the KVM-less or users lacking write access to /dev/kvm * hostfwd=... allows me to ssh into port 22215 from my Linux host to hit port 22 in the guest * "dhclient vtnet0 && pkg install git gettext gmake python libiconv" should be enough to get started kvm -smp 8 -m 2048 \ -drive if=virtio,file=FreeBSD-10.3-RELEASE-amd64.qcow2 \ -net nic,model=virtio \ -net user,hostfwd=tcp:127.0.0.1:22215-:22 \ -display curses -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] config.mak.uname: set PERL_PATH for FreeBSD 5.0+
Johannes Schindelin wrote: > Hi Eric, > > On Wed, 20 Jul 2016, Eric Wong wrote: > > > diff --git a/config.mak.uname b/config.mak.uname > > index a88f139..6c29545 100644 > > --- a/config.mak.uname > > +++ b/config.mak.uname > > @@ -202,6 +202,11 @@ ifeq ($(uname_S),FreeBSD) > > NO_UINTMAX_T = YesPlease > > NO_STRTOUMAX = YesPlease > > endif > > + R_MAJOR := $(shell expr "$(uname_R)" : '\([0-9]*\)\.') > > + > > + ifeq ($(shell test "$(R_MAJOR)" -ge 5 && echo 1),1) > > + PERL_PATH = /usr/local/bin/perl > > + endif > > In keeping with other uname_R usage, should this not read > > # Since FreeBSD 5.0, Perl is part of the core > ifneq ($(shell expr "$(uname_R)" : '[1-4]\.'),2) > PERL_PATH = /usr/local/bin/perl > endif > > instead? That's fine; however I don't use `expr` often, so it required a little more time to realize the '2' means 2 characters were matched. Also, my use of a numeric comparison may be more future-proof in case FreeBSD decides to have /usr/bin/perl again. I also wonder why we don't use `which` to search for somewhat standard path components, instead. Something like: PERL_PATH = $(shell PATH=/bin:/usr/bin:/usr/local/bin which perl) -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] test-lib: on FreeBSD, look for unzip(1) in /usr/local/bin/
Johannes Schindelin wrote: > The common work-around is to install Info-Zip on FreeBSD, into > /usr/local/bin/. > > Signed-off-by: Johannes Schindelin Thanks, t5003 now works out-of-the-box. Tested with Info-ZIP unzip installed and uninstalled. Tested-by: Eric Wong -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH ew/daemon-socket-keepalive] Windows: add missing definition of ENOTSOCK
Johannes Sixt wrote: > The previous commit introduced the first use of ENOTSOCK. This macro is > not available on Windows. Define it as WSAENOTSOCK because that is the > corresponding error value reported by the Windows versions of socket > functions. Thanks. I thought compat/poll/poll.c already used ENOTSOCK but I was mislead by the #ifdefs :x -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] git-svn: allow --version to work anywhere
Junio C Hamano wrote:
> Eric Wong writes:
> > I'm hoping "cd /" in the test will always succeed;
> > but I suppose non-*nix systems might fail, here.
>
> How about digging a few levels of directory hierarchy, exporting
> GIT_CEILING_DIRECTORIES so that we won't find any repository and
> going there to run these tests?
Ah, thanks; I missed that.
Repushed my master in case it's a convenient time to pull.
The following changes since commit 29493589e97a2de0c4c1c314f61ccafaee3b5caf:
archive-tar: huge offset and future timestamps would not work on 32-bit
(2016-07-15 10:51:55 -0700)
are available in the git repository at:
git://bogomips.org/git-svn.git master
for you to fetch changes up to c0071ae5dc1c610ab3791ece7ccf7d4772fde151:
git-svn: allow --version to work anywhere (2016-07-22 20:38:11 +)
----
Eric Wong (2):
git-svn: document svn.authorsProg in config
git-svn: allow --version to work anywhere
Documentation/git-svn.txt | 3 +++
git-svn.perl | 4 ++--
t/t9100-git-svn-basic.sh | 19 +++
3 files changed, 24 insertions(+), 2 deletions(-)
-8<-
Subject: [PATCH] git-svn: allow --version to work anywhere
Checking the version of the installed SVN libraries should not
require a git repository at all. This matches the behavior of
"git --version".
Add a test for "git svn help" for the same behavior while we're
at it, too.
Signed-off-by: Eric Wong
---
git-svn.perl | 4 ++--
t/t9100-git-svn-basic.sh | 19 +++
2 files changed, 21 insertions(+), 2 deletions(-)
diff --git a/git-svn.perl b/git-svn.perl
index f609e54..4d41d22 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -339,7 +339,7 @@ if ($cmd && $cmd =~ /(?:clone|init|multi-init)$/) {
die "failed to open $ENV{GIT_DIR}: $!\n";
$ENV{GIT_DIR} = $1 if <$fh> =~ /^gitdir: (.+)$/;
}
-} else {
+} elsif ($cmd) {
my ($git_dir, $cdup);
git_cmd_try {
$git_dir = command_oneline([qw/rev-parse --git-dir/]);
@@ -356,7 +356,7 @@ if ($cmd && $cmd =~ /(?:clone|init|multi-init)$/) {
my %opts = %{$cmd{$cmd}->[2]} if (defined $cmd);
-read_git_config(\%opts);
+read_git_config(\%opts) if $ENV{GIT_DIR};
if ($cmd && ($cmd eq 'log' || $cmd eq 'blame')) {
Getopt::Long::Configure('pass_through');
}
diff --git a/t/t9100-git-svn-basic.sh b/t/t9100-git-svn-basic.sh
index 28082b1..ab6593b 100755
--- a/t/t9100-git-svn-basic.sh
+++ b/t/t9100-git-svn-basic.sh
@@ -19,6 +19,25 @@ case "$GIT_SVN_LC_ALL" in
;;
esac
+deepdir=nothing-above
+ceiling=$PWD
+
+test_expect_success 'git svn --version works anywhere' '
+ mkdir -p "$deepdir" && (
+ export GIT_CEILING_DIRECTORIES="$ceiling" &&
+ cd "$deepdir" &&
+ git svn --version
+ )
+'
+
+test_expect_success 'git svn help works anywhere' '
+ mkdir -p "$deepdir" && (
+ export GIT_CEILING_DIRECTORIES="$ceiling" &&
+ cd "$deepdir" &&
+ git svn help
+ )
+'
+
test_expect_success \
'initialize git svn' '
mkdir import &&
--
EW
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] format-patch: escape "From " lines recognized by mailsplit
Users have mistakenly copied "From " lines into commit messages
in the past, and will certainly make the same mistakes in the
future. Since not everyone uses mboxrd, yet, we should at least
prevent miss-split mails by always escaping "From " lines based
on the check used by mailsplit.
mailsplit will not perform unescaping by default, yet, as it
could cause further invocations of format-patch from old
versions of git to generate bad output. Propagating the mboxo
escaping is preferable to miss-split patches. Unescaping may
still be performed via "--mboxrd".
ref: https://public-inbox.org/git/20160606230248.ga15...@dcvr.yhbt.net/T/#u
Signed-off-by: Eric Wong
---
builtin/mailsplit.c | 32 +---
mailinfo.c | 31 +++
mailinfo.h | 1 +
pretty.c| 16 +---
t/t4014-format-patch.sh | 14 ++
5 files changed, 60 insertions(+), 34 deletions(-)
diff --git a/builtin/mailsplit.c b/builtin/mailsplit.c
index 3068168..bb8f9c9 100644
--- a/builtin/mailsplit.c
+++ b/builtin/mailsplit.c
@@ -8,41 +8,11 @@
#include "builtin.h"
#include "string-list.h"
#include "strbuf.h"
+#include "mailinfo.h"
static const char git_mailsplit_usage[] =
"git mailsplit [-d] [-f] [-b] [--keep-cr] -o
[(|)...]";
-static int is_from_line(const char *line, int len)
-{
- const char *colon;
-
- if (len < 20 || memcmp("From ", line, 5))
- return 0;
-
- colon = line + len - 2;
- line += 5;
- for (;;) {
- if (colon < line)
- return 0;
- if (*--colon == ':')
- break;
- }
-
- if (!isdigit(colon[-4]) ||
- !isdigit(colon[-2]) ||
- !isdigit(colon[-1]) ||
- !isdigit(colon[ 1]) ||
- !isdigit(colon[ 2]))
- return 0;
-
- /* year */
- if (strtol(colon+3, NULL, 10) <= 90)
- return 0;
-
- /* Ok, close enough */
- return 1;
-}
-
static struct strbuf buf = STRBUF_INIT;
static int keep_cr;
static int mboxrd;
diff --git a/mailinfo.c b/mailinfo.c
index 9f19ca1..0ebd953 100644
--- a/mailinfo.c
+++ b/mailinfo.c
@@ -1035,3 +1035,34 @@ void clear_mailinfo(struct mailinfo *mi)
strbuf_release(&mi->log_message);
}
+
+int is_from_line(const char *line, int len)
+{
+ const char *colon;
+
+ if (len < 20 || memcmp("From ", line, 5))
+ return 0;
+
+ colon = line + len - 2;
+ line += 5;
+ for (;;) {
+ if (colon < line)
+ return 0;
+ if (*--colon == ':')
+ break;
+ }
+
+ if (!isdigit(colon[-4]) ||
+ !isdigit(colon[-2]) ||
+ !isdigit(colon[-1]) ||
+ !isdigit(colon[ 1]) ||
+ !isdigit(colon[ 2]))
+ return 0;
+
+ /* year */
+ if (strtol(colon+3, NULL, 10) <= 90)
+ return 0;
+
+ /* Ok, close enough */
+ return 1;
+}
diff --git a/mailinfo.h b/mailinfo.h
index 93776a7..c1430a0 100644
--- a/mailinfo.h
+++ b/mailinfo.h
@@ -37,5 +37,6 @@ struct mailinfo {
extern void setup_mailinfo(struct mailinfo *);
extern int mailinfo(struct mailinfo *, const char *msg, const char *patch);
extern void clear_mailinfo(struct mailinfo *);
+int is_from_line(const char *line, int len);
#endif /* MAILINFO_H */
diff --git a/pretty.c b/pretty.c
index 9fa42c2..898c0a3 100644
--- a/pretty.c
+++ b/pretty.c
@@ -10,6 +10,7 @@
#include "color.h"
#include "reflog-walk.h"
#include "gpg-interface.h"
+#include "mailinfo.h"
static char *user_format;
static struct cmt_fmt_map {
@@ -1745,9 +1746,18 @@ void pp_remainder(struct pretty_print_context *pp,
strbuf_add_tabexpand(sb, pp->expand_tabs_in_log,
line, linelen);
else {
- if (pp->fmt == CMIT_FMT_MBOXRD &&
- is_mboxrd_from(line, linelen))
- strbuf_addch(sb, '>');
+ switch (pp->fmt) {
+ case CMIT_FMT_EMAIL:
+ if (is_from_line(line, linelen))
+ strbuf_addch(sb, '>');
+ break;
+ case CMIT_FMT_MBOXRD:
+ if (is_mboxrd_from(line, linelen))
+ strbuf_addch(sb, '>');
+ break;
+ default:
+ break;
+ }
strbuf_add(sb, line, linel
[PATCH 0/2] fix git-svn HTTP tests under Apache 2.4
This resurrects the series started by Michael in April 2015 at: https://public-inbox.org/git/cover.1428505184.git@drmicha.warpmail.net/T/ But only keeps only his first patch. PATCH 2/2 reuses the work Clemens introduced in t/lib-httpd.sh back in 2008 to enable mod_dav_svn. The following changes since commit 08bb3500a2a718c3c78b0547c68601cafa7a8fd9: Sixth batch of topics for 2.10 (2016-07-19 13:26:16 -0700) are available in the git repository at: git://bogomips.org/git-svn.git svn-httpd for you to fetch changes up to cbc1a1fbd2911b851467862b7163c303a14b17ab: git svn: migrate tests to use lib-httpd (2016-07-23 03:31:21 +) Eric Wong (1): git svn: migrate tests to use lib-httpd Michael J Gruber (1): t/t91*: do not say how to avoid the tests t/lib-git-svn.sh | 91 +-- t/lib-httpd.sh| 8 ++- t/lib-httpd/apache.conf | 4 +- t/t9100-git-svn-basic.sh | 2 - t/t9115-git-svn-dcommit-funky-renames.sh | 7 ++- t/t9118-git-svn-funky-branch-names.sh | 2 +- t/t9120-git-svn-clone-with-percent-escapes.sh | 2 +- t/t9142-git-svn-shallow-clone.sh | 2 +- t/t9158-git-svn-mergeinfo.sh | 2 - t/t9160-git-svn-preserve-empty-dirs.sh| 1 - 10 files changed, 30 insertions(+), 91 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] git svn: migrate tests to use lib-httpd
This allows us to use common test infrastructure and parallelize
the tests. For now, GIT_SVN_TEST_HTTPD=true needs to be set to
enable the SVN HTTP tests because we reuse the same test cases
for both file:// and http:// SVN repositories. SVN_HTTPD_PORT
is no longer honored.
Tested under Apache 2.2 and 2.4 on Debian 7.x (wheezy) and
8.x (jessie), respectively.
Cc: Clemens Buchacher
Cc: Michael J Gruber
Signed-off-by: Eric Wong
---
t/lib-git-svn.sh | 91 +--
t/lib-httpd.sh| 8 ++-
t/lib-httpd/apache.conf | 4 +-
t/t9115-git-svn-dcommit-funky-renames.sh | 7 ++-
t/t9118-git-svn-funky-branch-names.sh | 2 +-
t/t9120-git-svn-clone-with-percent-escapes.sh | 2 +-
t/t9142-git-svn-shallow-clone.sh | 2 +-
7 files changed, 30 insertions(+), 86 deletions(-)
diff --git a/t/lib-git-svn.sh b/t/lib-git-svn.sh
index fb88232..688313e 100644
--- a/t/lib-git-svn.sh
+++ b/t/lib-git-svn.sh
@@ -65,81 +65,22 @@ svn_cmd () {
svn "$orig_svncmd" --config-dir "$svnconf" "$@"
}
-prepare_httpd () {
- for d in \
- "$SVN_HTTPD_PATH" \
- /usr/sbin/apache2 \
- /usr/sbin/httpd \
- ; do
- if test -f "$d"
- then
- SVN_HTTPD_PATH="$d"
- break
- fi
- done
- if test -z "$SVN_HTTPD_PATH"
- then
- echo >&2 '*** error: Apache not found'
- return 1
- fi
- for d in \
- "$SVN_HTTPD_MODULE_PATH" \
- /usr/lib/apache2/modules \
- /usr/libexec/apache2 \
- ; do
- if test -d "$d"
- then
- SVN_HTTPD_MODULE_PATH="$d"
- break
- fi
- done
- if test -z "$SVN_HTTPD_MODULE_PATH"
- then
- echo >&2 '*** error: Apache module dir not found'
- return 1
- fi
- if test ! -f "$SVN_HTTPD_MODULE_PATH/mod_dav_svn.so"
- then
- echo >&2 '*** error: Apache module "mod_dav_svn" not found'
- return 1
- fi
-
- repo_base_path="${1-svn}"
- mkdir "$GIT_DIR"/logs
-
- cat > "$GIT_DIR/httpd.conf" <
- DAV svn
- SVNPath "$rawsvnrepo"
-
-EOF
-}
-
-start_httpd () {
- if test -z "$SVN_HTTPD_PORT"
- then
- echo >&2 'SVN_HTTPD_PORT is not defined!'
- return
- fi
-
- prepare_httpd "$1" || return 1
-
- "$SVN_HTTPD_PATH" -f "$GIT_DIR"/httpd.conf -k start
- svnrepo="http://127.0.0.1:$SVN_HTTPD_PORT/$repo_base_path";
-}
-
-stop_httpd () {
- test -z "$SVN_HTTPD_PORT" && return
- test ! -f "$GIT_DIR/httpd.conf" && return
- "$SVN_HTTPD_PATH" -f "$GIT_DIR"/httpd.conf -k stop
+maybe_start_httpd () {
+ loc=${1-svn}
+
+ test_tristate GIT_SVN_TEST_HTTPD
+ case $GIT_SVN_TEST_HTTPD in
+ true)
+ . "$TEST_DIRECTORY"/lib-httpd.sh
+ LIB_HTTPD_SVN="$loc"
+ start_httpd
+ ;;
+ *)
+ stop_httpd () {
+ : noop
+ }
+ ;;
+ esac
}
convert_to_rev_db () {
diff --git a/t/lib-httpd.sh b/t/lib-httpd.sh
index ac2cbee..435a374 100644
--- a/t/lib-httpd.sh
+++ b/t/lib-httpd.sh
@@ -24,7 +24,7 @@
#LIB_HTTPD_MODULE_PATH web server modules path
#LIB_HTTPD_PORT listening port
#LIB_HTTPD_DAV enable DAV
-#LIB_HTTPD_SVN enable SVN
+#LIB_HTTPD_SVN enable SVN at given location (e.g. "svn")
#LIB_HTTPD_SSL enable SSL
#
# Copyright (c) 2008 Clemens Buchacher
@@ -162,8 +162,10 @@ prepare_httpd() {
if test -n "$LIB_HTTPD_SVN"
then
HTTPD_PARA="$HTTPD_PARA -DSVN"
- rawsvnrepo="$HTTPD_ROOT_PATH/svnrepo"
- svnrepo="http://127.0.0.1:$LIB_HTTPD_PORT/svn";
+ LIB_HTTPD_SVNPATH="$rawsvnrepo"
+ svnrepo="http://127.0.0.1:$LIB_HTTPD_PORT/";
+ svnrepo="$svnrepo$LIB_HTTPD_SVN"
+ export LIB_HTTPD_SVN LIB_HTTPD_SVNPATH
fi
fi
}
diff --git a/t/lib-httpd/apache.conf b/t/lib-httpd/apache.conf
index 018a83a..c3e6313 100644
--- a/t/lib-httpd/apache.conf
+++ b/t
[PATCH 1/2] t/t91*: do not say how to avoid the tests
From: Michael J Gruber
Some of the tests "say" how to stop the svn tests from running, some do
not.
The test suite is directed at people reading t/README where we keep all
information about running the test suite (partly, with options etc.).
Remove said "say" occurences.
Signed-off-by: Michael J Gruber
Signed-off-by: Eric Wong
---
t/t9100-git-svn-basic.sh | 2 --
t/t9158-git-svn-mergeinfo.sh | 2 --
t/t9160-git-svn-preserve-empty-dirs.sh | 1 -
3 files changed, 5 deletions(-)
diff --git a/t/t9100-git-svn-basic.sh b/t/t9100-git-svn-basic.sh
index 28082b1..c23b11f 100755
--- a/t/t9100-git-svn-basic.sh
+++ b/t/t9100-git-svn-basic.sh
@@ -8,8 +8,6 @@ GIT_SVN_LC_ALL=${LC_ALL:-$LANG}
. ./lib-git-svn.sh
-say 'define NO_SVN_TESTS to skip git svn tests'
-
case "$GIT_SVN_LC_ALL" in
*.UTF-8)
test_set_prereq UTF8
diff --git a/t/t9158-git-svn-mergeinfo.sh b/t/t9158-git-svn-mergeinfo.sh
index 13f78f2..a875b45 100755
--- a/t/t9158-git-svn-mergeinfo.sh
+++ b/t/t9158-git-svn-mergeinfo.sh
@@ -7,8 +7,6 @@ test_description='git svn mergeinfo propagation'
. ./lib-git-svn.sh
-say 'define NO_SVN_TESTS to skip git svn tests'
-
test_expect_success 'initialize source svn repo' '
svn_cmd mkdir -m x "$svnrepo"/trunk &&
svn_cmd co "$svnrepo"/trunk "$SVN_TREE" &&
diff --git a/t/t9160-git-svn-preserve-empty-dirs.sh
b/t/t9160-git-svn-preserve-empty-dirs.sh
index b4a4434..0ede3cf 100755
--- a/t/t9160-git-svn-preserve-empty-dirs.sh
+++ b/t/t9160-git-svn-preserve-empty-dirs.sh
@@ -11,7 +11,6 @@ local Git repository with placeholder files.'
. ./lib-git-svn.sh
-say 'define NO_SVN_TESTS to skip git svn tests'
GIT_REPO=git-svn-repo
test_expect_success 'initialize source svn repo containing empty dirs' '
--
EW
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1 3/3] convert: add filter..useProtocol option
Jakub Narębski wrote: > W dniu 2016-07-22 o 17:49, larsxschnei...@gmail.com pisze: > > +use strict; > > +use warnings; > > +use autodie; > > autodie? "set -e" for Perl (man autodie) It's been a part of Perl for ages, but I've never used it myself, either; I suppose it's fine for tests... > > +$| = 1; # autoflush STDOUT > > Perhaps *STDOUT->autoflush(1), if I remember my Perl correctly? > Should this matter? Why it is needed? It's better to always disable automatic output buffering when writing to pipes or sockets for IPC. Otherwise output may be buffered indefinitely because the buffering mechanism doesn't know a reader is stalled. Same problem with using stdio.h functions for IPC in C. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1 3/3] convert: add filter..useProtocol option
larsxschnei...@gmail.com wrote:
> Please note that the protocol filters do not support stream processing
> with this implemenatation because the filter needs to know the length of
> the result in advance. A protocol version 2 could address this in a
> future patch.
Would it be prudent to reuse pkt-line for this?
> +static void stop_protocol_filter(struct cmd2process *entry) {
> + if (!entry)
> + return;
> + sigchain_push(SIGPIPE, SIG_IGN);
> + close(entry->process.in);
> + close(entry->process.out);
> + sigchain_pop(SIGPIPE);
> + finish_command(&entry->process);
> + child_process_clear(&entry->process);
> + hashmap_remove(&cmd_process_map, entry, NULL);
> + free(entry);
> +}
> +
> +static struct cmd2process *start_protocol_filter(const char *cmd)
> +{
> + int ret = 1;
> + struct cmd2process *entry = NULL;
> + struct child_process *process = NULL;
These are unconditionally set below, so initializing to NULL
may hide future bugs.
> + struct strbuf nbuf = STRBUF_INIT;
> + struct string_list split = STRING_LIST_INIT_NODUP;
> + const char *argv[] = { NULL, NULL };
> + const char *header = "git-filter-protocol\nversion";
static const char header[] = "git-filter-protocol\nversion";
...might be smaller by avoiding the extra pointer
(but compilers ought to be able to optimize it)
> + entry = xmalloc(sizeof(*entry));
> + hashmap_entry_init(entry, strhash(cmd));
> + entry->cmd = cmd;
> + process = &entry->process;
> + ret &= strncmp(header, split.items[0].string, strlen(header)) == 0;
starts_with() is probably more readable, here.
> +static int apply_protocol_filter(const char *path, const char *src, size_t
> len,
> + int fd, struct strbuf *dst,
> const char *cmd,
> + const char *filter_type)
> +{
> + int ret = 1;
> + struct cmd2process *entry = NULL;
> + struct child_process *process = NULL;
I would leave process initialized, here, since it should
always be set below:
> + struct stat fileStat;
> + struct strbuf nbuf = STRBUF_INIT;
> + size_t nbuf_len;
> + char *strtol_end;
> + char c;
> +
> + if (!cmd || !*cmd)
> + return 0;
> +
> + if (!dst)
> + return 1;
> +
> + if (!cmd_process_map_init) {
> + cmd_process_map_init = 1;
> + hashmap_init(&cmd_process_map, (hashmap_cmp_fn)
> cmd2process_cmp, 0);
> + } else {
> + entry = find_protocol_filter_entry(cmd);
> + }
> +
> + if (!entry){
> + entry = start_protocol_filter(cmd);
> + if (!entry) {
> + stop_protocol_filter(entry);
stop_protocol_filter is a no-op, here, since entry is NULL
> + return 0;
> + }
> + }
> + process = &entry->process;
> +
> + sigchain_push(SIGPIPE, SIG_IGN);
> + switch (entry->protocol) {
> + case 1:
> + if (fd >= 0 && !src) {
> + ret &= fstat(fd, &fileStat) != -1;
> + len = fileStat.st_size;
There's a truncation bug when sizeof(size_t) < sizeof(off_t)
(and mixedCase is inconsistent with our style)
> +my $filelen = ;
> +chomp $filelen;
> +print $debug " $filelen";
> +
> +$filelen = int($filelen);
Calling int() here is unnecessary and may hide bugs if you
forget to check $debug. Perhaps a regexp check is safer:
$filelen =~ /\A\d+\z/ or die "bad filelen: $filelen\n";
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] format-patch: escape "From " lines recognized by mailsplit
Johannes Schindelin wrote: > I think that this change: > deserves to live in a separate patch... It would make the real change > stick out better. Good point, I actually wrote a looser check based on is_mboxrd_from before realizing the stricter check from mailsplit would probably be more appropriate to use by default. The following changes since commit 08bb3500a2a718c3c78b0547c68601cafa7a8fd9: Sixth batch of topics for 2.10 (2016-07-19 13:26:16 -0700) are available in the git repository at: git://bogomips.org/git-svn.git mboxo for you to fetch changes up to 4f769f1799db11f135948bf517f2d8d864fa778f: format-patch: escape "From " lines recognized by mailsplit (2016-07-23 21:38:40 +) ---- Eric Wong (2): mailinfo: extract is_from_line from mailsplit format-patch: escape "From " lines recognized by mailsplit builtin/mailsplit.c | 32 +--- mailinfo.c | 31 +++ mailinfo.h | 1 + pretty.c| 16 +--- t/t4014-format-patch.sh | 14 ++ 5 files changed, 60 insertions(+), 34 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] mailinfo: extract is_from_line from mailsplit
We will be reusing is_from_line for quoting format-patch output
in the next commit.
Suggested-by: Johannes Schindelin
Signed-off-by: Eric Wong
---
builtin/mailsplit.c | 32 +---
mailinfo.c | 31 +++
mailinfo.h | 1 +
3 files changed, 33 insertions(+), 31 deletions(-)
diff --git a/builtin/mailsplit.c b/builtin/mailsplit.c
index 3068168..bb8f9c9 100644
--- a/builtin/mailsplit.c
+++ b/builtin/mailsplit.c
@@ -8,41 +8,11 @@
#include "builtin.h"
#include "string-list.h"
#include "strbuf.h"
+#include "mailinfo.h"
static const char git_mailsplit_usage[] =
"git mailsplit [-d] [-f] [-b] [--keep-cr] -o
[(|)...]";
-static int is_from_line(const char *line, int len)
-{
- const char *colon;
-
- if (len < 20 || memcmp("From ", line, 5))
- return 0;
-
- colon = line + len - 2;
- line += 5;
- for (;;) {
- if (colon < line)
- return 0;
- if (*--colon == ':')
- break;
- }
-
- if (!isdigit(colon[-4]) ||
- !isdigit(colon[-2]) ||
- !isdigit(colon[-1]) ||
- !isdigit(colon[ 1]) ||
- !isdigit(colon[ 2]))
- return 0;
-
- /* year */
- if (strtol(colon+3, NULL, 10) <= 90)
- return 0;
-
- /* Ok, close enough */
- return 1;
-}
-
static struct strbuf buf = STRBUF_INIT;
static int keep_cr;
static int mboxrd;
diff --git a/mailinfo.c b/mailinfo.c
index 9f19ca1..0ebd953 100644
--- a/mailinfo.c
+++ b/mailinfo.c
@@ -1035,3 +1035,34 @@ void clear_mailinfo(struct mailinfo *mi)
strbuf_release(&mi->log_message);
}
+
+int is_from_line(const char *line, int len)
+{
+ const char *colon;
+
+ if (len < 20 || memcmp("From ", line, 5))
+ return 0;
+
+ colon = line + len - 2;
+ line += 5;
+ for (;;) {
+ if (colon < line)
+ return 0;
+ if (*--colon == ':')
+ break;
+ }
+
+ if (!isdigit(colon[-4]) ||
+ !isdigit(colon[-2]) ||
+ !isdigit(colon[-1]) ||
+ !isdigit(colon[ 1]) ||
+ !isdigit(colon[ 2]))
+ return 0;
+
+ /* year */
+ if (strtol(colon+3, NULL, 10) <= 90)
+ return 0;
+
+ /* Ok, close enough */
+ return 1;
+}
diff --git a/mailinfo.h b/mailinfo.h
index 93776a7..c1430a0 100644
--- a/mailinfo.h
+++ b/mailinfo.h
@@ -37,5 +37,6 @@ struct mailinfo {
extern void setup_mailinfo(struct mailinfo *);
extern int mailinfo(struct mailinfo *, const char *msg, const char *patch);
extern void clear_mailinfo(struct mailinfo *);
+int is_from_line(const char *line, int len);
#endif /* MAILINFO_H */
--
EW
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] format-patch: escape "From " lines recognized by mailsplit
Users have mistakenly copied "From " lines into commit messages
in the past, and will certainly make the same mistakes in the
future. Since not everyone uses mboxrd, yet, we should at least
prevent miss-split mails by always escaping "From " lines based
on the check used by mailsplit.
mailsplit will not perform unescaping by default, yet, as it
could cause further invocations of format-patch from old
versions of git to generate bad output. Propagating the mboxo
escaping is preferable to miss-split patches. Unescaping may
still be performed via "--mboxrd".
ref: https://public-inbox.org/git/20160606230248.ga15...@dcvr.yhbt.net/T/#u
Signed-off-by: Eric Wong
---
pretty.c| 16 +---
t/t4014-format-patch.sh | 14 ++
2 files changed, 27 insertions(+), 3 deletions(-)
diff --git a/pretty.c b/pretty.c
index 9fa42c2..898c0a3 100644
--- a/pretty.c
+++ b/pretty.c
@@ -10,6 +10,7 @@
#include "color.h"
#include "reflog-walk.h"
#include "gpg-interface.h"
+#include "mailinfo.h"
static char *user_format;
static struct cmt_fmt_map {
@@ -1745,9 +1746,18 @@ void pp_remainder(struct pretty_print_context *pp,
strbuf_add_tabexpand(sb, pp->expand_tabs_in_log,
line, linelen);
else {
- if (pp->fmt == CMIT_FMT_MBOXRD &&
- is_mboxrd_from(line, linelen))
- strbuf_addch(sb, '>');
+ switch (pp->fmt) {
+ case CMIT_FMT_EMAIL:
+ if (is_from_line(line, linelen))
+ strbuf_addch(sb, '>');
+ break;
+ case CMIT_FMT_MBOXRD:
+ if (is_mboxrd_from(line, linelen))
+ strbuf_addch(sb, '>');
+ break;
+ default:
+ break;
+ }
strbuf_add(sb, line, linelen);
}
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index 1206c48..8fa3982 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -1606,4 +1606,18 @@ test_expect_success 'format-patch --pretty=mboxrd' '
test_cmp expect actual
'
+test_expect_success 'format-patch From escaping' '
+ cat >msg <<-INPUT_END &&
+ somebody pasted format-patch output into a body
+
+ From Mon Sep 17 00:00:00 2001
+ INPUT_END
+
+ C=$(git commit-tree HEAD^^{tree} -p HEAD patch &&
+ git grep -h --no-index \
+ ">From " \
+ patch
+'
+
test_done
--
EW
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] format-patch: escape "From " lines recognized by mailsplit
Junio C Hamano wrote: > As a tool to produce mbox file, quoting like this in format-patch > output may make sense, I would think, but shouldn't send-email undo > this when sending individual patches? Yes, that sounds like a good idea. Thanks. Will followup in a day or two. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC 0/3] dumb HTTP transport speedups
Jakub Narębski wrote: > W dniu 2016-07-11 o 22:51, Eric Wong pisze: > > > TL;DR: dumb HTTP clone from a certain badly-packed repo goes from > > ~2 hours to ~30 min memory usage drops from 2G to 360M > > > > > > I hadn't packed the public repo at https://public-inbox.org/git > > for a few weeks. As an admin of a small server limited memory > > and CPU resources but fairly good bandwidth, I prefer clients > > use dumb HTTP for initial clones. > > Hopefully the solution / workaround for large initial clone > problem utilizing bundles (`git bundle`), which can be resumably > transferred, would get standarized and automated. I've been hoping to look at this more in coming weeks/months. It would be nice if bundles and packs could be unified somehow to avoid doubling storage on the server. > Do you use bitmap indices for speeding up fetches? Yes, but slow clients are still a problem since big responses keeps memory-hungry processes running while trickling (or waste disk space buffering the pack output up front) Static packfiles/bundles are nice since all the clients can share the same data on the server side as it's trickled out. > BTW. IMVHO the problem with dumb HTTP is the latency, not extra > bandwidth needed... I enabled persistent connections for 404s on loose objects for this reason :) We should probably be doing it across the board on 404s, just haven't gotten around to it... Increasing default parallelism should also help; but might hurt some servers which can't handle many connections... Hard to imagine people using antiquated prefork servers for slow clients in a post-Slowloris world, but maybe it happens? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1 3/3] convert: add filter..useProtocol option
Lars Schneider wrote:
> On 23 Jul 2016, at 10:14, Eric Wong wrote:
> > larsxschnei...@gmail.com wrote:
> >> +static struct cmd2process *start_protocol_filter(const char *cmd)
> >> +{
> >> + int ret = 1;
> >> + struct cmd2process *entry = NULL;
> >> + struct child_process *process = NULL;
> >
> > These are unconditionally set below, so initializing to NULL
> > may hide future bugs.
>
> OK. I thought it is generally a good thing to initialize a pointer with
> NULL. Can you explain to me how this might hide future bugs?
> I will remove the initialization.
Compilers complain about uninitialized variables. Blindly
setting them to NULL can allow them to be dereferenced;
triggering segfaults; especially if it's passed to a different
compilation unit the compiler can't see.
> >> +static int apply_protocol_filter(const char *path, const char *src,
> >> size_t len,
> >> + int fd, struct strbuf *dst,
> >> const char *cmd,
> >> + const char *filter_type)
> >> +{
> >> + if (fd >= 0 && !src) {
> >> + ret &= fstat(fd, &fileStat) != -1;
> >> + len = fileStat.st_size;
> >
> > There's a truncation bug when sizeof(size_t) < sizeof(off_t)
>
> OK. What would you suggest to do in that case? Should we just let the
> filter fail? Is there anything else we could do?
Anything which refers to something on disk (or will eventually
stored there, such as blobs) should evolve towards off_t rather
than size_t. We just discovered a bunch of 32-bit truncation
bugs the other week:
https://public-inbox.org/git/1466807902.28869.8.ca...@gmail.com/
If the protocol/ABI is frozen, it should probably fail;
and a 64-bit-off_t version for 32-bit systems should be defined.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] format-patch: escape "From " lines recognized by mailsplit
Junio C Hamano wrote: > Junio C Hamano writes: > > Eric Wong writes: > > > >> Users have mistakenly copied "From " lines into commit messages > >> in the past, and will certainly make the same mistakes in the > >> future. Since not everyone uses mboxrd, yet, we should at least > >> prevent miss-split mails by always escaping "From " lines based > >> on the check used by mailsplit. > >> > >> mailsplit will not perform unescaping by default, yet, as it > >> could cause further invocations of format-patch from old > >> versions of git to generate bad output. Propagating the mboxo > >> escaping is preferable to miss-split patches. Unescaping may > >> still be performed via "--mboxrd". > > > > As a tool to produce mbox file, quoting like this in format-patch > > output may make sense, I would think, but shouldn't send-email undo > > this when sending individual patches? > > Also, doesn't it break "git rebase" (non-interactive), or anything > that internally runs format-patch to individual files and then runs > am on each of them, anything that knows that each output file from > format-patch corresponds to a single change and there is no need to > split, badly if we do this unconditionally? Yes, rebase should probably unescape is_from_line matches. Anything which spawns an editor should probably warn/reprompt users on is_from_line() matches, too, to prevent user errors from sneaking in. > IOW, shouldn't this be an optional feature to format-patch that is > triggered by passing a new command line option that currently nobody > is passing? I added --pretty=mboxrd as the optional feature for this reason. It'll take a while for people to start using it (or perhaps make it the default in git 3.0). In the meantime, I would prefer extra ">" being injected rather than breaking mailsplit completely. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] config.mak.uname: correct perl path on FreeBSD
Junio C Hamano wrote: > Duy Nguyen writes: > > > On Mon, Jul 25, 2016 at 6:56 PM, Junio C Hamano wrote: > >> On Mon, Jul 25, 2016 at 9:21 AM, Nguyễn Thái Ngọc Duy > >> wrote: > >>> It looks the the symlink /usr/bin/perl (to /usr/local/bin/perl) has > >>> been removed at least on FreeBSD 10.3. See [1] for more information. > >>> > >>> [1] > >>> https://svnweb.freebsd.org/ports/head/UPDATING?r1=386270&r2=386269&pathrev=386270&diff_format=c Ah, I missed that. I guess that explains why nobody complained about the problem sooner. > >>> Signed-off-by: Nguyễn Thái Ngọc Duy > >>> --- > >>> Tested with fbsd 10.3, kvm image. But I suppose it's the same as real > >>> fbsd. > >> > >> Thanks; and we know that older (but not too old that we no longer care > >> about) > >> FreeBSD all have /usr/local/bin/perl? > > > > I'm no fbsd expert but from the first sentence in [1] "Perl has been > > removed from base more than ten years ago..." I would assume it meant > > "... removed from base, _to_ ports system" which means /usr/local for > > all package installation (for ten years for perl). So I think we are > > good. > > I guess we didn't follow through > > http://public-inbox.org/git/%3C20160720025630.GA71874%40plume%3E/ > > and allowed the thread to drift into a tangent? +Cc Dscho I've been meaning to followup on that, but had connectivity problems to my VM last week. I still prefer we use numeric comparisons for version numbers since numbers are... numeric. IOW, I prefer we go with my original patch. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] format-patch: escape "From " lines recognized by mailsplit
Junio C Hamano wrote: > Eric Wong writes: > > >> Also, doesn't it break "git rebase" (non-interactive), or anything > >> that internally runs format-patch to individual files and then runs > >> am on each of them, anything that knows that each output file from > >> format-patch corresponds to a single change and there is no need to > >> split, badly if we do this unconditionally? > > > > Yes, rebase should probably unescape is_from_line matches. > > This shouldn't matter for "git rebase", as it only reads from the > mbox "From " line to learn the > original commit and extract the log message directly from there. OK. > But a third-party script that wants to read format-patch output > would be forced to upgrade, which is a pain if we make this change > unconditionally. I suppose unconditionally having mailsplit unescape ">From ... $DATE" lines might be acceptable? It'll still propagate these mistakes to older versions of git, but those installations will eventually become fewer. > >> IOW, shouldn't this be an optional feature to format-patch that > >> is triggered by passing a new command line option that currently > >> nobody is passing? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] config.mak.uname: correct perl path on FreeBSD
Junio C Hamano wrote: > Eric Wong writes: > > Junio C Hamano wrote: > >> Duy Nguyen writes: > >> > On Mon, Jul 25, 2016 at 6:56 PM, Junio C Hamano > >> > wrote: > >> >> On Mon, Jul 25, 2016 at 9:21 AM, Nguyễn Thái Ngọc Duy > >> >> wrote: > > about the problem sooner. > > > >> >>> Signed-off-by: Nguyễn Thái Ngọc Duy > >> >>> --- > >> >>> Tested with fbsd 10.3, kvm image. But I suppose it's the same as real > >> >>> fbsd. > >> >> > >> >> Thanks; and we know that older (but not too old that we no longer care > >> >> about) > >> >> FreeBSD all have /usr/local/bin/perl? > >> > > >> > I'm no fbsd expert but from the first sentence in [1] "Perl has been > >> > removed from base more than ten years ago..." I would assume it meant > >> > "... removed from base, _to_ ports system" which means /usr/local for > >> > all package installation (for ten years for perl). So I think we are > >> > good. > >> > >> I guess we didn't follow through > >> > >> http://public-inbox.org/git/%3C20160720025630.GA71874%40plume%3E/ > >> > >> and allowed the thread to drift into a tangent? > > > > +Cc Dscho > > > > I've been meaning to followup on that, but had connectivity > > problems to my VM last week. I still prefer we use numeric > > comparisons for version numbers since numbers are... numeric. > > IOW, I prefer we go with my original patch. > > I tend to agree with you if we have to do "systems older than this > should use /usr/bin, others should use /usr/local/bin", but this > different incarnation of the same topic seems to claim that older > ones had /usr/local/bin forever anyway, and that was what made the > patch interesting. I dug around the freebsd history (git://github.com/freebsd/freebsd.git) (~1.5G) and the 3.x and 4.x releases with perl in base still contained many references to /usr/local/bin/perl It also seems FreeBSD 2.x releases were also perl-less in base; so yes, I'm alright with Duy's patch :) -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Jul 2016, #07; Mon, 25)
Lars Schneider wrote: > On 26 Jul 2016, at 00:50, Junio C Hamano wrote: > > > > * ew/git-svn-http-tests (2016-07-25) 2 commits > > - git svn: migrate tests to use lib-httpd > > - t/t91*: do not say how to avoid the tests > > > > Reuse the lib-httpd test infrastructure when testing the subversion > > integration that interacts with subversion repositories served over > > the http:// protocol. > > > > Will merge to 'next'. > > Do I understand correctly that this patch fixes $gmane/295291 ? > Nice! Should we add "GIT_SVN_TEST_HTTPD=true" to the .travis.yml, then? Maybe :) Unfortunately, that currently causes the svn file:// tests (using the same test numbers+files) to be skipped. We should probably define new test numbers and figure out a way to be able to always run file:// and http:// (and svn://) tests. On a side note, svn:// ought to be easy since svnserve supports --inetd and we can let the kernel bind a random port. We could be doing the same with git-daemon --inetd, even... -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 5/5] convert: add filter..process option
larsxschnei...@gmail.com wrote:
> +static off_t multi_packet_read(struct strbuf *sb, const int fd, const size_t
> size)
I'm no expert in C, but this might be const-correctness taken
too far. I think basing this on the read(2) prototype is less
surprising:
static ssize_t multi_packet_read(int fd, struct strbuf *sb, size_t size)
Also what Jeff said about off_t vs size_t, but my previous
emails may have confused you w.r.t. off_t usage...
> +static int multi_packet_write(const char *src, size_t len, const int in,
> const int out)
Same comment about over const ints above.
len can probably be off_t based on what is below; but you need
to process the loop in ssize_t-friendly chunks.
> +{
> + int ret = 1;
> + char header[4];
> + char buffer[8192];
> + off_t bytes_to_write;
What Jeff said, this should be ssize_t to match read(2) and xread
> + while (ret) {
> + if (in >= 0) {
> + bytes_to_write = xread(in, buffer, sizeof(buffer));
> + if (bytes_to_write < 0)
> + ret &= 0;
> + src = buffer;
> + } else {
> + bytes_to_write = len > LARGE_PACKET_MAX - 4 ?
> LARGE_PACKET_MAX - 4 : len;
> + len -= bytes_to_write;
> + }
> + if (!bytes_to_write)
> + break;
The whole ret &= .. style error handling is hard-to-follow and
here, a source of bugs. I think the expected convention on
hitting errors is:
1) stop whatever you're doing
2) cleanup
3) propagate the error to callers
"goto" is an acceptable way of accomplishing this.
For example, byte_to_write may still be negative at this point
(and interpreted as a really big number when cast to unsigned
size_t) and src/buffer could be stack garbage.
> + set_packet_header(header, bytes_to_write + 4);
> + ret &= write_in_full(out, &header, sizeof(header)) ==
> sizeof(header);
> + ret &= write_in_full(out, src, bytes_to_write) ==
> bytes_to_write;
> + }
> + ret &= write_in_full(out, "", 4) == 4;
> + return ret;
> +}
> +
> +static int apply_protocol_filter(const char *path, const char *src, size_t
> len,
> + int fd, struct strbuf *dst,
> const char *cmd,
> + const char *filter_type)
> +{
> + if (fd >= 0 && !src) {
> + ret &= fstat(fd, &file_stat) != -1;
> + len = file_stat.st_size;
Same truncation bug I noticed earlier; what I originally meant
is the `len' arg probably ought to be off_t, here, not size_t.
32-bit x86 Linux systems have 32-bit size_t (unsigned), but
large file support means off_t is 64-bits (signed).
Also, is it worth continuing this function if fstat fails?
> + }
> +
> + sigchain_push(SIGPIPE, SIG_IGN);
> +
> + packet_write(process->in, "%s\n", filter_type);
> + packet_write(process->in, "%s\n", path);
> + packet_write(process->in, "%zu\n", len);
I'm not sure if "%zu" is portable since we don't do C99 (yet?)
For 64-bit signed off_t, you can probably do:
packet_write(process->in, "%"PRIuMAX"\n", (uintmax_t)len);
Since we don't have PRIiMAX or intmax_t, here, and a negative
len would be a bug (probably from failed fstat) anyways.
> + ret &= multi_packet_write(src, len, fd, process->in);
multi_packet_write will probably fail if fstat failed above...
> + strbuf = packet_read_line(process->out, NULL);
And this may just block or timeout if multi_packet_write failed.
Naptime, I may look at the rest another day.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Alternatives to mid.gmane.org?
Duy Nguyen wrote: > On Thu, Jul 28, 2016 at 11:11 AM, Lars Schneider > wrote: > > Hi, > > > > gmane is down At least the following should work in case public-inbox fails: https://mid.mail-archive.com/%s https://marc.info/?i=%s public-inbox falls back to them on missing MIDs, along with https://lists.debian.org/msgid-search/%s I trip over power cords all the time, I'm hoping other people mirror/replicate the setup Jeff also linked to: https://public-inbox.org/git/20160710004813.ga20...@dcvr.yhbt.net/ Right now my email subscription is still an SPOF for the mirror, along with vger.kernel.org... > I read this and thought "temporarily" but apparently it's not [1]. A > lot of our links in the mail archive are gmane's :( > > [1] https://lars.ingebrigtsen.no/2016/07/28/the-end-of-gmane/ :< I'll see what I can do about getting a gmane NUM -> MID mapping up while NNTP still works... -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [ANNOUNCE] more archives of this list
Eric Wong wrote: > Code is AGPL-3.0+: git clone https://public-inbox.org/ > > > Additional mirrors or forks (perhaps different UIs) are very welcome, Btw, it's possible to do quote highlighting with user-side CSS: https://public-inbox.org/meta/20160709-user-side-css-example@11/ Will probably add classes for diff colors, too, since a git repository browser with mailing list integration will happen. For the moment, cgit + examples/cgit-commit-filter.lua allows searching subjects (at least non-merge ones). So the commit subject below is a link: https://bogomips.org/mirrors/git.git/commit/?id=7b35efd734e501f to: https://public-inbox.org/git/?x=t&q="fsck_walk():+optionally+name+objects+on+the+go" -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Alternatives to mid.gmane.org?
Duy Nguyen wrote: > I read this and thought "temporarily" but apparently it's not [1]. A > lot of our links in the mail archive are gmane's :( > > [1] https://lars.ingebrigtsen.no/2016/07/28/the-end-of-gmane/ I may not have time to integrate this extensibly into the public-inbox search engine today, but at least here is an NNTP article number to Message-ID mapping for non-NNTP users: https://public-inbox.org/.temp/gmane.comp.version-control.git-300599.txt.gz (~5 MB) Script used to generate this: https://public-inbox.org/meta/20160728220145.13024-...@80x24.org/ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-svn: allow --version to work anywhere
Junio C Hamano wrote: > Subject: [PATCH] t9100: portability fix > > Do not say "export VAR=VAL"; "VAR=VAL && export VAR" is always more > portable. Oops, sorry I should've caught that :x -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] t7063: work around FreeBSD's lazy mtime update feature
Nguyễn Thái Ngọc Duy wrote: > Let's start with the commit message of [1] from freebsd.git [2] > > Sync timestamp changes for inodes of special files to disk as late > as possible (when the inode is reclaimed). Temporarily only do > this if option UFS_LAZYMOD configured and softupdates aren't > enabled. UFS_LAZYMOD is intentionally left out of > /sys/conf/options. > > This is mainly to avoid almost useless disk i/o on battery powered > machines. It's silly to write to disk (on the next sync or when > the inode becomes inactive) just because someone hit a key or > something wrote to the screen or /dev/null. > > PR: 5577 [3] > > The short version of that, in the context of t7063, is that when a > directory is updated, its mtime may be updated later, not > immediately. This can be shown with a simple command sequence > > date; sleep 1; touch abc; rm abc; sleep 10; ls -lTd . Yikes. I guess FreeBSD doesn't have an in-memory inode cache it can keep up-to-date without flushing to disk? > One would expect that the date shown in `ls` would be one second from > `date`, but it's 10 seconds later. If we put another `ls -lTd .` in > front of `sleep 10`, then the date of the last `ls` comes as > expected. The first `ls` somehow forces mtime to be updated. Fwiw, `stat .` seems to have the same effect as `ls -lTd .`... > t7063 is really sensitive to directory mtime. When mtime is too "new", > git code suspects racy timestamps and will not trigger the shortcut in > untracked cache, in t7063.26 (or t7063.25 before this patch) and > eventually be detected in t7063.28 > > We have two options thanks to this special FreeBSD feature: > > 1) Stop supporting untracked cache on FreeBSD. Skip t7063 entirely >when running on FreeBSD > > 2) Work around this problem (using the same 'ls' trick) and continue >to support untracked cache on FreeBSD > > I initially wanted to go with 1) because I didn't know the exact > nature of this feature and feared that it would make untracked cache > work unreliably, using the cached version when it should not. > > Since the behavior of this thing is clearer now. The picture is not > that bad. If this indeed happens often, untracked cache would assume > racy condition more often and _fall back_ to non-untracked cache code > paths. Which means it may be less effective, but it will not show > wrong things. > > This patch goes with option 2. > > PS. For those who want to look further in FreeBSD source code, this > flag is now called IN_LAZYMOD. I can see it's effective in ext2 and > ufs. zfs is questionable. > > [1] 660e6408e6df99a20dacb070c5e7f9739efdf96d > [2] git://github.com/freebsd/freebsd.git > [3] https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=5577 > > Reported-by: Eric Wong > Signed-off-by: Nguyễn Thái Ngọc Duy > --- > This is only of those commits that commit messages are more important > than the patch itself. One of the good notes about directory mtime, > if we start to use it in more places in git. Thanks, Tested-by: Eric Wong > t/t7063-status-untracked-cache.sh | 4 > t/test-lib.sh | 6 ++ > 2 files changed, 10 insertions(+) > > diff --git a/t/t7063-status-untracked-cache.sh > b/t/t7063-status-untracked-cache.sh > index a971884..08fc586 100755 > --- a/t/t7063-status-untracked-cache.sh > +++ b/t/t7063-status-untracked-cache.sh > @@ -419,6 +419,10 @@ test_expect_success 'create/modify files, some of which > are gitignored' ' > rm base > ' > > +test_expect_success FREEBSD 'Work around lazy mtime update' ' > + ls -ld . >/dev/null > +' stat . >/dev/null would be more to the point of what is going on, here. But I also wonder if untracked cache itself could/should be doing this internally. (I'm not familiar with that code, of course) Thanks again for looking into this. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] t7063: work around FreeBSD's lazy mtime update feature
Eric Wong wrote: > Nguyễn Thái Ngọc Duy wrote: > > +test_expect_success FREEBSD 'Work around lazy mtime update' ' > > + ls -ld . >/dev/null > > +' > > stat . >/dev/null If there's some older FreeBSD w/o stat(1); "test -x ." ought to work, too, and it's faster being a shell builtin. I suspect some shell might be clever about optimizing away a more-obvious "test -d .", so I choose "test -x ." > would be more to the point of what is going on, here. But I > also wonder if untracked cache itself could/should be doing this > internally. Still wondering :> > (I'm not familiar with that code, of course) > > Thanks again for looking into this. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/2] add PAGER_ENV to build and core.pagerEnv to config
This series allows us to more easily configure platform-specific defaults at build-time and also allow users to override them at runtime via config. Previous discussion in this thread: https://public-inbox.org/git/52d87a79.6060...@rawbw.com/T/ The following changes since commit f8f7adce9fc50a11a764d57815602dcb818d1816: Sync with maint (2016-07-28 14:21:18 -0700) are available in the git repository at: git://bogomips.org/git-svn.git pager-env for you to fetch changes up to 1563ef177f9c1ee990bb3547f16bd7568a17379a: pager: implement core.pagerEnv in config (2016-08-01 00:51:42 +) Eric Wong (1): pager: implement core.pagerEnv in config Junio C Hamano (1): pager: move pager-specific setup into the build Documentation/config.txt | 7 +++ Makefile | 19 +-- config.mak.uname | 1 + git-sh-setup.sh | 8 +--- pager.c | 35 +++ t/t7006-pager.sh | 14 ++ 6 files changed, 75 insertions(+), 9 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] pager: implement core.pagerEnv in config
This allows overriding the build-time PAGER_ENV variable
at run-time.
Inspired by part 1 of an idea from Kyle J. McKay at:
https://public-inbox.org/git/62db6def-8b39-4481-ba06-245bf4523...@gmail.com/
Signed-off-by: Eric Wong
---
Documentation/config.txt | 7 +++
pager.c | 5 -
t/t7006-pager.sh | 14 ++
3 files changed, 25 insertions(+), 1 deletion(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 8b1aee4..6c20269 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -714,6 +714,13 @@ Likewise, when the `LV` environment variable is unset, Git
sets it
to `-c`. You can override this setting by exporting `LV` with
another value or setting `core.pager` to `lv +c`.
+core.pagerEnv::
+ Environment for running `core.pager`.
++
+Defaults to the value set at build, usually `LESS=FRX LV=-c`.
+On platforms where `more` and `less` are the same binary,
+`LESS=FRX LV=-c MORE=FRX` is appropriate.
+
core.whitespace::
A comma separated list of common whitespace problems to
notice. 'git diff' will use `color.diff.whitespace` to
diff --git a/pager.c b/pager.c
index 2f2cadc..cc2df7c 100644
--- a/pager.c
+++ b/pager.c
@@ -68,7 +68,10 @@ const char *git_pager(int stdout_is_tty)
static void setup_pager_env(struct argv_array *env)
{
- const char *pager_env = stringify(PAGER_ENV);
+ const char *pager_env;
+
+ if (git_config_get_value("core.pagerenv", &pager_env))
+ pager_env = stringify(PAGER_ENV);
while (*pager_env) {
struct strbuf buf = STRBUF_INIT;
diff --git a/t/t7006-pager.sh b/t/t7006-pager.sh
index e4fc5c8..0c482fc 100755
--- a/t/t7006-pager.sh
+++ b/t/t7006-pager.sh
@@ -456,4 +456,18 @@ test_expect_success 'command with underscores does not
complain' '
test_cmp expect actual
'
+test_expect_success TTY 'core.pagerEnv overrides build-time env' '
+ (
+ sane_unset LESS LV MORE &&
+ git config core.pagerEnv MORE=-R &&
+ PAGER="env >pager-env.out; wc" &&
+ export PAGER &&
+ test_terminal git log
+ ) &&
+ git config --unset core.pagerEnv &&
+ grep ^MORE=-R pager-env.out &&
+ grep -v ^LESS= pager-env.out &&
+ grep -v ^LV= pager-env.out
+'
+
test_done
--
EW
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] pager: move pager-specific setup into the build
From: Junio C Hamano
Allowing PAGER_ENV to be set at build-time allows us to move
pager-specific knowledge out of our build. Currently, this
allows us to set a better default for FreeBSD where more(1)
is the same binary as less(1).
This also prepares us for introducing a run-time config knob to
override the build-time environment in the next commit.
Originally-from:
https://public-inbox.org/git/xmqq61piw4yf@gitster.dls.corp.google.com/
Signed-off-by: Eric Wong
---
Makefile | 19 +--
config.mak.uname | 1 +
git-sh-setup.sh | 8 +---
pager.c | 32
4 files changed, 51 insertions(+), 9 deletions(-)
diff --git a/Makefile b/Makefile
index 6a13386..fe469a6 100644
--- a/Makefile
+++ b/Makefile
@@ -370,6 +370,14 @@ all::
# Define HAVE_BSD_SYSCTL if your platform has a BSD-compatible sysctl function.
#
# Define HAVE_GETDELIM if your system has the getdelim() function.
+#
+# Define PAGER_ENV to a SP separated VAR=VAL pairs to define
+# default environment variables to be passed when a pager is spawned, e.g.
+#
+#PAGER_ENV = LESS=FRX LV=-c
+#
+# to say "export LESS=FRX (and LV=-c) if the environment variable
+# LESS (and LV) is not set, respectively".
GIT-VERSION-FILE: FORCE
@$(SHELL_PATH) ./GIT-VERSION-GEN
@@ -1500,6 +1508,10 @@ ifeq ($(PYTHON_PATH),)
NO_PYTHON = NoThanks
endif
+ifndef PAGER_ENV
+PAGER_ENV = LESS=FRX LV=-c
+endif
+
QUIET_SUBDIR0 = +$(MAKE) -C # space to separate -C and subdir
QUIET_SUBDIR1 =
@@ -1579,6 +1591,7 @@ PYTHON_PATH_SQ = $(subst ','\'',$(PYTHON_PATH))
TCLTK_PATH_SQ = $(subst ','\'',$(TCLTK_PATH))
DIFF_SQ = $(subst ','\'',$(DIFF))
PERLLIB_EXTRA_SQ = $(subst ','\'',$(PERLLIB_EXTRA))
+PAGER_ENV_SQ = $(subst ','\'',$(PAGER_ENV))
# We must filter out any object files from $(GITLIBS),
# as it is typically used like:
@@ -1591,7 +1604,7 @@ PERLLIB_EXTRA_SQ = $(subst ','\'',$(PERLLIB_EXTRA))
LIBS = $(filter-out %.o, $(GITLIBS)) $(EXTLIBS)
BASIC_CFLAGS += -DSHA1_HEADER='$(SHA1_HEADER_SQ)' \
- $(COMPAT_CFLAGS)
+ $(COMPAT_CFLAGS) -DPAGER_ENV='$(PAGER_ENV_SQ)'
LIB_OBJS += $(COMPAT_OBJS)
# Quote for C
@@ -1753,7 +1766,7 @@ common-cmds.h: $(wildcard Documentation/git-*.txt)
SCRIPT_DEFINES = $(SHELL_PATH_SQ):$(DIFF_SQ):$(GIT_VERSION):\
$(localedir_SQ):$(NO_CURL):$(USE_GETTEXT_SCHEME):$(SANE_TOOL_PATH_SQ):\
- $(gitwebdir_SQ):$(PERL_PATH_SQ):$(SANE_TEXT_GREP)
+ $(gitwebdir_SQ):$(PERL_PATH_SQ):$(SANE_TEXT_GREP):$(PAGER_ENV)
define cmd_munge_script
$(RM) $@ $@+ && \
sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \
@@ -1766,6 +1779,7 @@ sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \
-e 's|@@GITWEBDIR@@|$(gitwebdir_SQ)|g' \
-e 's|@@PERL@@|$(PERL_PATH_SQ)|g' \
-e 's|@@SANE_TEXT_GREP@@|$(SANE_TEXT_GREP)|g' \
+-e 's|@@PAGER_ENV@@|$(PAGER_ENV_SQ)|g' \
$@.sh >$@+
endef
@@ -2173,6 +2187,7 @@ GIT-BUILD-OPTIONS: FORCE
@echo NO_PERL=\''$(subst ','\'',$(subst ','\'',$(NO_PERL)))'\' >>$@+
@echo NO_PYTHON=\''$(subst ','\'',$(subst ','\'',$(NO_PYTHON)))'\' >>$@+
@echo NO_UNIX_SOCKETS=\''$(subst ','\'',$(subst
','\'',$(NO_UNIX_SOCKETS)))'\' >>$@+
+ @echo PAGER_ENV=\''$(subst ','\'',$(subst ','\'',$(PAGER_ENV)))'\' >>$@+
ifdef TEST_OUTPUT_DIRECTORY
@echo TEST_OUTPUT_DIRECTORY=\''$(subst ','\'',$(subst
','\'',$(TEST_OUTPUT_DIRECTORY)))'\' >>$@+
endif
diff --git a/config.mak.uname b/config.mak.uname
index 17fed2f..2484dfb 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -209,6 +209,7 @@ ifeq ($(uname_S),FreeBSD)
HAVE_PATHS_H = YesPlease
GMTIME_UNRELIABLE_ERRORS = UnfortunatelyYes
HAVE_BSD_SYSCTL = YesPlease
+ PAGER_ENV = LESS=FRX LV=-c MORE=-R
endif
ifeq ($(uname_S),OpenBSD)
NO_STRCASESTR = YesPlease
diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index 0c34aa6..0f5a56f 100644
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh
@@ -163,9 +163,11 @@ git_pager() {
else
GIT_PAGER=cat
fi
- : "${LESS=-FRX}"
- : "${LV=-c}"
- export LESS LV
+ for vardef in @@PAGER_ENV@@
+ do
+ var=${vardef%%=*}
+ eval ": \${$vardef} && export $var"
+ done
eval "$GIT_PAGER" '"$@"'
}
diff --git a/pager.c b/pager.c
index 4bc0481..2f2cadc 100644
--- a/pager.c
+++ b/pager.c
@@ -63,14
Re: [PATCH 1/2] pager: move pager-specific setup into the build
"brian m. carlson" wrote: > On Mon, Aug 01, 2016 at 01:05:56AM +0000, Eric Wong wrote: > > + while (*cp && *cp == ' ') > > + cp++; > > So it looks like this function splits on spaces but doesn't provide any > escaping mechanism. Is there any case in which we want to accept > environment variables containing whitespace? I ask this as someone that > has EDITOR set to "gvim -f" on occasion and seeing how tools sometimes > handle that poorly. Yes, it's only split on spaces right now. While I don't think there's any current case where spaces would be useful/desirable; I suppose a 3rd patch in this series could add support for using split_cmdline (from alias.c)... -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [ANNOUNCE] git-series: track changes to a patch series over time
Christian Couder wrote: > On Fri, Jul 29, 2016 at 12:10 PM, Richard Ipsum > wrote: > > On Thu, Jul 28, 2016 at 11:40:55PM -0700, Josh Triplett wrote: > > [snip] > >> > >> I'd welcome any feedback, whether on the interface and workflow, the > >> internals and collaboration, ideas on presenting diffs of patch series, > >> or anything else. > > I'm particularly interested in trying to establish a standard for > > storing review data in git. I've got a prototype for doing that[3], > > and an example tool that uses it[4]. The tool is still incomplete/buggy > > though. > > There is also git-appraise (https://github.com/google/git-appraise) > written in Go to store code review data in Git. > It looks like it stores its data in git notes and can be integrated > with Rust (https://github.com/Nemo157/git-appraise-rs). I'm not convinced another format/standard is needed besides the email workflow we already use for git and kernel development. Rather, better ways to archive/search the emails is desirable. Fortunately, commit titles are rather unique :) I started archiving the git ML with public-inbox (which uses git): https://public-inbox.org/git/20160710004813.ga20...@dcvr.yhbt.net/T/ It can be easy to search by Subject (commit titles): https://public-inbox.org/git/?q=s:%22more+archives+of+this+list%22 Search (currently Xapian) will be tuned to parse things like filenames and diffs to allow searching within those. It is already somewhat email-aware, such as deprioritizing quoted text; and having a code repository browser with mail archive integration is in the works. I also see the reliance on an after-the-fact search engine (which can be tuned/replaced) as philosophically inline with what git does, too, such as not having rename tracking and doing delayed deltafication. Email also has the advantage of having existing tooling, and being (at least for now) federated without a single point of failure. vger.kernel.org can still be a major point of failure, which is why the "archives first" approach of public-inbox favors readers pulling messages over NNTP/HTTP/git (and maybe soon, POP3). -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [ANNOUNCE] git-series: track changes to a patch series over time
Josh Triplett wrote: > On Mon, Aug 01, 2016 at 07:55:54AM +, Eric Wong wrote: > > Christian Couder wrote: > > > On Fri, Jul 29, 2016 at 12:10 PM, Richard Ipsum > > > wrote: > > > > On Thu, Jul 28, 2016 at 11:40:55PM -0700, Josh Triplett wrote: > > > > [snip] > > > >> > > > >> I'd welcome any feedback, whether on the interface and workflow, the > > > >> internals and collaboration, ideas on presenting diffs of patch series, > > > >> or anything else. > > > > > > I'm particularly interested in trying to establish a standard for > > > > storing review data in git. I've got a prototype for doing that[3], > > > > and an example tool that uses it[4]. The tool is still incomplete/buggy > > > > though. > > I'm not convinced another format/standard is needed besides the > > email workflow we already use for git and kernel development. > > Not all projects use a patches-by-email workflow, or want to. To the > extent that tools and projects use some other workflow, standardizing > the format they use to store patch reviews (including per-line > annotations, approvals, test results, etc) seems preferable to having > each tool use its own custom format. I think standardizing on email conventions (such as what we already do with format-patch, request-pull, S-o-b trailers) would be a step in this direction and a good step to take. But yeah, I also hope git adopters can somehow be convinced to also adopt the workflow that built git itself. > > I also see the reliance on an after-the-fact search engine > > (which can be tuned/replaced) as philosophically inline with > > what git does, too, such as not having rename tracking and > > doing delayed deltafication. > > Storing review data in git doesn't mean it needs to end up in the > history of the project itself; it can use after-the-fact annotations on > a commit. Right. So on public-inbox.org/git today, one could search for after-the-fact annotations based on commit titles and maybe exact commit ID matches. A future goal might be to get search indexing working on commit ID substrings. So finding references to commit deadbeefcafe01234567890123467890abcdef00 could be done by searching for "commit deadbeefcafe" or even a shorter ID, and the following results could still be returned: 1. commit deadbeefcafe broke my cat feeder 2. commit deadbeef killed my cow > > Email also has the advantage of having existing tooling, and > > being (at least for now) federated without a single point of > > failure. > > Storing review data in git makes it easy to push and pull it, which can > provide the basis for a federated system. Every public-inbox exposed over HTTP(S) is git clonable[1], so it's possible to push/pull or have developers merge/combine inboxes with index-only operations. There's no UI for that, yet, and having a working tree checked out is inefficient with 300K uncompressed mails... But there needs to be way to message others about the existence of new pushes/pull-requests/reviews/etc; including users unable to clone or host 800M git repos; so that messaging system might as well be email. [1] git clone --mirror https://public-inbox.org/git/ That's not efficient, yet, though, at around 800M when the gzipped fast-export dump is around half that: https://public-inbox.org/git/20160710034745.ga20...@dcvr.yhbt.net/T/#u -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
