Re: [PATCH 2/3] hooks/post-receive-email: force log messages in UTF-8
On Sun, Aug 04, 2013 at 11:14:40AM -0700, Jonathan Nieder wrote:
> Alexey Shumkin wrote:
> > On Fri, Aug 02, 2013 at 04:23:38PM -0700, Jonathan Nieder wrote:
>
> >> 1. Log messages use the configured log output encoding, which is
> >> meant to be whatever encoding works best with local terminals
> >> (and does not have much to do with what encoding should be used
> >> for email)
> >>
> >> 2. Filenames are left as is: on Linux, usually UTF-8, and in the Mingw
> >> port (which uses Unicode filesystem APIs), always UTF-8
> >
> > I cannot say exactly if it makes sense for THIS patch, but I'd like to
> > remind about Cygwin port, which definitely does not use UTF-8 encoding
> > (in my case it is Windows-1251) for filenames.
> >
> >>
> >> 3. The "This is an automated email" preface uses a project description
> >> from .git/description, which is typically in UTF-8 to support
> >> gitweb.
>
> Thanks for clarifying. So in the context you describe, (1) is
> configurable, (2) is Windows-1251, (3) is unconfigurably UTF-8, and
> there is no way with current git facilities to force the email to use
> a single encoding unless (3) happens to contain no special characters.
>
> What is the value of the "[i18n] commitEncoding" setting in your
> project?
commitEncoding is equal to filenames' encoding, Windows-1251, of course.
> What encoding do the raw commit messages (shown with
> "git log --format=raw") use for their text, and what do they declare
> with an in-commit 'encoding' header, if any?
Well, despite `git log --help`
--8<--
raw
The raw format shows the entire commit exactly as stored in
the commit object"
--8<--
on a Linux box (UTF-8) I can see "readable" commit messages nevertheless
they are stored in 'Windows-1251' (so they are converted to UTF-8). To
be sure I've checked actual content of them with `git cat-file commit`
Actually, to be honest, I usually use modified version of Git (see
ecaee8050cec23eb4cf082512e907e3e52c20b57) in 'next' branch, that could
affect the results, so I've checked `git log --format=raw` with
unmodified v1.8.3.3 of Git.
But let's go back to the answer to your question. Commit encoding stored
as a header in a raw commit messages is 'Windows-1251'.
>
> Does everyone on this project use Cygwin?i
This is a "closed" (commercial) project and every developer uses Cygwin,
except me. I use a Linux box as a desktop (mail, IM, web-browsing; but
development goes on Cygwin). And sometimes I run utility scripts
included to that project on my desktop (as far as Linux works with files
much faster than Cygwin does ;))
Also, a Git server is a coLinux box (http://www.colinux.org/) on a
Windows Server 2003, but I guess, it does not much matter here.
> That should be fine, but
> I'd expect there to be problems as soon as someone wants to try the
> Mingw port ("Git for Windows").
Yep, one of our developers tried to use modern version of TortoiseGit
with MinGW port of Git. That was a failure. As far as since v1.7.9 MinGW
port transcodes filenames to store them internally in UTF-8. This
problem could be solved with converting once that non-ASCII filenames to
UTF-8, but I do not want to use MinGW port. I like Cygwin
"infrastructure" that is more Linux-like than MinGW.
>
> I wonder if there should be an "[i18n] repositoryPathEncoding"
> configuration item to support this kind of repository. Then git could
> be aware of the intended encoding of paths, could recode them for
> display to a terminal, and at least on Linux and Mingw could recode
> them for use in filenames on disk. "repositoryPathEncoding = none"
> would mean the current behavior of treating paths as raw sequences of
> bytes.
I'd be happy if such a setting exists. That could solve many problems
with cross-platform projects with non-ASCII filenames.
Indeed, MinGW port does resolve that problem somehow!
>
> What do you think?
> Jonathan
--
Alexey Shumkin
--
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: redundant message in builtin/rm.c
Junio C Hamano writes: > Thanks both. Perhaps we should do something like this? > > -- >8 -- > Subject: builtin/rm.c: consolidate error reporting for removing submodules Sounds good, yes. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- 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: Rewriting git-repack.sh in C
Stefan Beller writes: > Hello, > > I'd like to rewrite the repack shell script in C. > So I tried the naive approach reading the man page and > the script itself and write C program by matching each block/line > of the script with a function in C You should add one step here: check that tests are good, and possibly add some to improve coverage. It's easier to add tests when you already have a reference implementation. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- 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 replace: should it check for object type, and can it replace merges?
Hi, On Sat, Aug 3, 2013 at 5:13 PM, Philip Oakley wrote: > A recent comment http://stackoverflow.com/a/18027030/717355 on a question I > asked two years ago about 'grafts' and 'replace' indicates that users think > that 'git replace' can't replace a merge commit. The documentation doesn't > have any examples and gives the naive impression that one should only > replace a simple commit with another simple commit. I am sorry if the documentation gives this impression. I'd like to fix it, but I am not sure what should be changed. Should adding an example be enough? Or do you want it to state that explicitely? > Having looked at the code, I realised that anything can be replaced with > anything, which is perhaps not what was intended. The documentation says in the "BUGS" section: "And of course things may break if an object of one type is replaced by an object of another type (for example a blob replaced by a commit)." So yes it is a know bug. > A simple thought is that > the replace should be like-for-like with regard to object type, though that > would not include replacing a sub-module for a tree (and vice versa). > > Should 'git replace' check the object types to ensure they are sensible? It would probably be a good idea to do that, yeah. But I don't know much about submodules, so I can't say if replacing a sub-module for a tree (and vice versa) should be allowed. Or if there should be a --force-different-objects option for these kinds of special cases. > Would it be reasonable to add examples to indicate the range of > replacements, and how to prepare alternative merge commits, or is that a > hostage to fortune? Yeah, adding examples would be a good idea. I don't understand what do you mean with "range of replacements", though. I am not sure preparing alternative commits or merge commits should be an important part of the examples. There are many cases that could be interesting to different users: - replacing a non merge commit with a merge commit (if someone forgot to use --no-ff when merging for example) - replacing a merge commit with a non merge commit (if a rebase should have been done) - and of course replacing a non merge commit with a non merge commit, or a merge commit with a merge commit So I think explaining how another commit can be created from existing commits belongs to some other parts of the git documentation. Perhaps there could be such examples in the git hash-object and git filter-branch documentation and we could just point to them. Best, Christian. -- 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
Fwd: Bug: SEGV in git when applying the patches
Hi, When applying patch via git, it is doing sometimes SEGV. Please find more details in the attached crash logs. --- kenorb git_2013-07-08-150841_kenorbs-MacBook-Air.crash Description: Binary data git_2013-07-08-173513_kenorbs-MacBook-Air.crash Description: Binary data git_2013-07-08-201624_kenorbs-MacBook-Air.crash Description: Binary data
Re: Fwd: Bug: SEGV in git when applying the patches
On Mon, Aug 05, 2013 at 12:01:44PM +0100, Rafal W. wrote: > Hi, > When applying patch via git, it is doing sometimes SEGV. > Please find more details in the attached crash logs. This looks like the issue fixed by commit 212eb96 (apply: carefully strdup a possibly-NULL name, 2013-06-21), which is in Git 1.8.3.3 and later. -- 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: Fwd: Bug: SEGV in git when applying the patches
Thank you, I'll test with the newer version. --- kenorb On 5 August 2013 12:09, John Keeping wrote: > On Mon, Aug 05, 2013 at 12:01:44PM +0100, Rafal W. wrote: >> Hi, >> When applying patch via git, it is doing sometimes SEGV. >> Please find more details in the attached crash logs. > > This looks like the issue fixed by commit 212eb96 (apply: carefully > strdup a possibly-NULL name, 2013-06-21), which is in Git 1.8.3.3 and > later. -- 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 4/4] gitweb: make search help link less ugly
Jakub Narębski wrote: > > - -values => ['commit', 'grep', 'author', > > 'committer', 'pickaxe']) . > > + -values => ['commit', 'grep', 'author', > > 'committer', 'pickaxe']) . > > Nb. what changed here (in line above)? Whoops, tab damage. I will re-roll. Thanks for the review. Tony. -- f.anthony.n.finchhttp://dotat.at/ Forties, Cromarty: East, veering southeast, 4 or 5, occasionally 6 at first. Rough, becoming slight or moderate. Showers, rain at first. Moderate or good, occasionally poor at first.
Re: [PATCH 3/4] gitweb: omit the repository owner when it is unset
Jakub Narębski wrote: > On Tue, Jul 2, 2013 at 6:24 PM, Tony Finch wrote: > > > On the repository summary page, leave the whole owner line out if > > the repo does not have an owner, rather than displaying a labelled > > empty field.. > > Note that if $omit_owner is true, whole _column_ is skipped. There are two places where the owner is displayed: on the list of projects, and on each project's summary page. This change affects the summary page (where it removes a row, not a column) and it leaves the projects list alone. I'll make that clearer in the commit message (and fix the extraneous dot). Tony. -- f.anthony.n.finchhttp://dotat.at/ Forties, Cromarty: East, veering southeast, 4 or 5, occasionally 6 at first. Rough, becoming slight or moderate. Showers, rain at first. Moderate or good, occasionally poor at first.
Re: [PATCH] commit: reject non-characters
Peter Krefting: - /* U+FFFE and U+ are guaranteed non-characters. */ - if ((codepoint & 0x1e) == 0xfffe) + /* U+xxFFFE and U+xx are guaranteed non-characters. */ + if ((codepoint & 0xe) == 0xfffe) + return bad_offset; Drats, there is an F too many in the bitmask, it should be: + if ((codepoint & 0xfffe) == 0xfffe) -- \\// Peter - http://www.softwolves.pp.se/ -- 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 v3] gc: reject if another gc is running, unless --force is given
This may happen when `git gc --auto` is run automatically, then the
user, to avoid wait time, switches to a new terminal, keeps working
and `git gc --auto` is started again because the first gc instance has
not clean up the repository.
This patch tries to avoid multiple gc running, especially in --auto
mode. In the worst case, gc may be delayed 12 hours if a daemon reuses
the pid stored in gc-%s.pid.
uname() and kill(..,0) are added to MinGW compatibility layer so that
it'll work on Windows. Actually uname() is stub so it won't prevent
multiple gc running on a shared repo. But that's for Windows
contributors to step in.
Signed-off-by: Nguyễn Thái Ngọc Duy
---
- new code is taken out in a separate function for clarity
- file locking
- implementing kill(pid, 0) on windows (but not tested)
Documentation/git-gc.txt | 6 -
builtin/gc.c | 62
compat/mingw.c | 6 +
compat/mingw.h | 6 +
git-compat-util.h| 1 +
5 files changed, 80 insertions(+), 1 deletion(-)
diff --git a/Documentation/git-gc.txt b/Documentation/git-gc.txt
index 2402ed6..e158a3b 100644
--- a/Documentation/git-gc.txt
+++ b/Documentation/git-gc.txt
@@ -9,7 +9,7 @@ git-gc - Cleanup unnecessary files and optimize the local
repository
SYNOPSIS
[verse]
-'git gc' [--aggressive] [--auto] [--quiet] [--prune= | --no-prune]
+'git gc' [--aggressive] [--auto] [--quiet] [--prune= | --no-prune]
[--force]
DESCRIPTION
---
@@ -72,6 +72,10 @@ automatic consolidation of packs.
--quiet::
Suppress all progress reports.
+--force::
+ Force `git gc` to run even if there may be another `git gc`
+ instance running on this repository.
+
Configuration
-
diff --git a/builtin/gc.c b/builtin/gc.c
index 6be6c8d..1f33908 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -167,11 +167,66 @@ static int need_to_gc(void)
return 1;
}
+static int gc_running(int force)
+{
+ static struct lock_file lock;
+ struct utsname utsname;
+ struct stat st;
+ uintmax_t pid;
+ FILE *fp;
+ int fd, should_exit;
+
+ if (uname(&utsname))
+ strcpy(utsname.nodename, "unknown");
+
+ fd = hold_lock_file_for_update(&lock,
+ git_path("gc-%s.pid", utsname.nodename), 0);
+ if (!force) {
+ if (fd < 0)
+ return 1;
+
+ fp = fopen(git_path("gc-%s.pid", utsname.nodename), "r");
+ should_exit =
+ fp != NULL &&
+ !fstat(fileno(fp), &st) &&
+ /*
+* 12 hour limit is very generous as gc should
+* never take that long. On the other hand we
+* don't really need a strict limit here,
+* running gc --auto one day late is not a big
+* problem. --force can be used in manual gc
+* after the user verifies that no gc is
+* running.
+*/
+ time(NULL) - st.st_mtime <= 12 * 3600 &&
+ fscanf(fp, "%"PRIuMAX, &pid) == 1 &&
+ !kill(pid, 0);
+ if (fp != NULL)
+ fclose(fp);
+ if (should_exit) {
+ if (fd >= 0)
+ rollback_lock_file(&lock);
+ return 1;
+ }
+ }
+
+ if (fd >= 0) {
+ struct strbuf sb = STRBUF_INIT;
+ strbuf_addf(&sb, "%"PRIuMAX"\n", (uintmax_t) getpid());
+ write_in_full(fd, sb.buf, sb.len);
+ strbuf_release(&sb);
+ commit_lock_file(&lock);
+ }
+
+ return 0;
+}
+
int cmd_gc(int argc, const char **argv, const char *prefix)
{
int aggressive = 0;
int auto_gc = 0;
int quiet = 0;
+ int force = 0;
struct option builtin_gc_options[] = {
OPT__QUIET(&quiet, N_("suppress progress reporting")),
@@ -180,6 +235,7 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
PARSE_OPT_OPTARG, NULL, (intptr_t)prune_expire },
OPT_BOOLEAN(0, "aggressive", &aggressive, N_("be more thorough
(increased runtime)")),
OPT_BOOLEAN(0, "auto", &auto_gc, N_("enable auto-gc mode")),
+ OPT_BOOL(0, "force", &force, N_("force running gc even if there
may be another gc running")),
OPT_END()
};
@@ -225,6 +281,12 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
} else
add_repack_all_option();
+ if (gc_running(force)) {
+ if (auto_gc)
+ return 0; /* be quiet on --auto */
+ die(_("gc is already running (use --force if not)"));
+ }
About close() in commit_lock_file()
close() is added in commit_lock_file(), before rename(), by 4723ee9 (Close files opened by lock_file() before unlinking. - 2007-11-13), which is needed by Windows. But doesn't that create a gap between close() and rename() on other platforms where another process can replace .lock file with something else before rename() is executed? Should we enclose close() in #ifdef __MINGW32__ (and maybe __CYGWIN__)? -- Duy -- 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 (Aug 2013, #01; Thu, 1)
Thomas Rast writes: > Junio C Hamano writes: > >> * tr/log-full-diff-keep-true-parents (2013-08-01) 1 commit >> - log: use true parents for diff even when rewriting >> >> Output from "git log --full-diff -- " looked strange, >> because comparison was done with the previous ancestor that touched >> the specified , causing the patches for paths outside the >> pathspec to show more than the single commit has changed. >> >> I am not sure if that is necessarily a problem, though. Output >> from "git log --full-diff -2 -- " without this change >> will be applicable to some codebase, but after this change that >> will no longer be true (you will get only tiny parts of the change >> that were made by the two commits in question, while missing all >> the other changes). > > Hmm. Uwe's original complaint was that --stat -- as in "what other > things are touched when we modify foo" -- is nonsensical. > > In addition, applying what you describe above would be a very strange > form of rebase: one that squashes everything into the commits that > affect the given files. When would it ever make sense to do such > squashing? After all, you'd lose all the commit splits&messages in > between. I am not so worried about actually applying that kind of patch, but more about reviewing what happened in each of the single logical step shown. If you made two changes in two far-apart commits to files you are interested in and want to look at the changes made to other files to make each of them a complete solution, depending on how the change was split across files, you would get useful to useless result with your patch. In one extreme, if your commits are too fine-grained and you committed one path at a time, then "full-diff" will not show anything useful, as the commits that hit the pathspec do not have changes to any other paths. In another extreme, you may have preparatory changes to other paths in commits that immediately precede the commit that hits the pathspec and then finally conclude the topic by introducing a caller of the prepared codepath in other files to the file you are interested in, and your patch will show only the endgame change without showing the preparatory steps, which may or may not be useful, depending on what you are looking for. So while I do understand why sometimes you wish to see full diff "git log --full-diff -- " to match output from "git show" without pathspec, I am not sure doing it unconditionally and by default like your patch does is the best way to go. In the meantime: git rev-list --header --parents HEAD -- | git -p diff-tree -p --stdin would be one way to do so. -- 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] log doc: the argument to --encoding is not optional
Jonathan Nieder writes: > $ git log --encoding > fatal: Option '--encoding' requires a value > $ git rev-list --encoding > fatal: Option '--encoding' requires a value > > The argument to --encoding has always been mandatory. Unfortunately > manpages like git-rev-list(1), git-log(1), and git-show(1) have > described the option's syntax as "--encoding[=]" since it > was first documented. Clarify by removing the extra brackets. > > Signed-off-by: Jonathan Nieder > --- Thanks. > Documentation/git-rev-list.txt | 2 +- > Documentation/pretty-options.txt | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/Documentation/git-rev-list.txt b/Documentation/git-rev-list.txt > index 65ac27e0..045b37b8 100644 > --- a/Documentation/git-rev-list.txt > +++ b/Documentation/git-rev-list.txt > @@ -40,7 +40,7 @@ SYNOPSIS >[ \--right-only ] >[ \--cherry-mark ] >[ \--cherry-pick ] > - [ \--encoding[=] ] > + [ \--encoding= ] >[ \--(author|committer|grep)= ] >[ \--regexp-ignore-case | -i ] >[ \--extended-regexp | -E ] > diff --git a/Documentation/pretty-options.txt > b/Documentation/pretty-options.txt > index 5e499421..eea0e306 100644 > --- a/Documentation/pretty-options.txt > +++ b/Documentation/pretty-options.txt > @@ -28,7 +28,7 @@ people using 80-column terminals. > This is a shorthand for "--pretty=oneline --abbrev-commit" > used together. > > ---encoding[=]:: > +--encoding=:: > The commit objects record the encoding used for the log message > in their encoding header; this option can be used to tell the > command to re-code the commit log message in the encoding -- 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] t8001, t8002: fix "blame -L :literal" test on NetBSD
Sub-test 42 of t8001 and t8002 ("blame -L :literal") fails on NetBSD
with the following verbose output:
git annotate -L:main hello.c
Author F (expected 4, attributed 3) bad
Author G (expected 1, attributed 1) good
This is not caused by different behaviour of git blame or annotate on
that platform, but by different test input, in turn caused by a sed
command that forgets to add a newline on NetBSD. Here's the diff of the
commit that adds "goodbye" to hello.c, for Linux:
@@ -1,4 +1,5 @@
int main(int argc, const char *argv[])
{
puts("hello");
+ puts("goodbye");
}
We see that it adds an extra TAB, but that's not a problem. Here's the
same on NetBSD:
@@ -1,4 +1,4 @@
int main(int argc, const char *argv[])
{
puts("hello");
-}
+ puts("goodbye");}
It also adds an extra TAB, but it is missing the newline character
after the semicolon.
The following patch gets rid of the extra TAB at the beginning, but
more importantly adds the missing newline at the end in a (hopefully)
portable way, mentioned in http://sed.sourceforge.net/sedfaq4.html.
The diff becomes this, on both Linux and NetBSD:
@@ -1,4 +1,5 @@
int main(int argc, const char *argv[])
{
puts("hello");
+ puts("goodbye");
}
Signed-off-by: Rene Scharfe
---
This regression was introduced by 5a9830cb ("t8001/t8002 (blame):
add blame -L :funcname tests").
t/annotate-tests.sh | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/t/annotate-tests.sh b/t/annotate-tests.sh
index 0bfee00..d4e7f47 100644
--- a/t/annotate-tests.sh
+++ b/t/annotate-tests.sh
@@ -245,8 +245,8 @@ test_expect_success 'setup -L :regex' '
git commit -m "hello" &&
mv hello.c hello.orig &&
- sed -e "/}/i\\
- Qputs(\"goodbye\");" hello.c &&
+ sed -e "/}/ {x; s/$/Qputs(\"goodbye\");/; G;}" hello.c &&
GIT_AUTHOR_NAME="G" GIT_AUTHOR_EMAIL="g...@test.git" \
git commit -a -m "goodbye" &&
--
1.8.1.5
--
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: [BUG?] gc and impatience
Duy Nguyen writes: > I worry less about this. It's not the right model to have two machines > modify the same shared repository (gc --auto is only triggered when we > think there are new objects) even though I think we support it. I am a bit hesitant to dismiss with "It's not the right model", as the original of accessing the repository from two terminals while one clearly is being accessed busily by gc falls into the same category. > If > it's two _scripts_ modifying the same repo, I don't care as this is > more about user interaction. It can very well be two terminals, one on one machine each, both with the same human end-user interaction. -- 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 2/2] howto: Eliminate all tabs
Piotr Krukowiecki writes: > Isn't the howto documentation intended (mainly/also) for the users of git, > not the developers? And they have *.html version for that exact version, no? We used not to bother running asciidoc to Documentation/howto, so the *.txt versions had to be exposed to the end users because there was no other variant. That is no longer true, and *.txt versions are sources. -- 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] t8001, t8002: fix "blame -L :literal" test on NetBSD
On Mon, Aug 5, 2013 at 11:21 AM, René Scharfe wrote:
> Sub-test 42 of t8001 and t8002 ("blame -L :literal") fails on NetBSD
> with the following verbose output:
>
> git annotate -L:main hello.c
> Author F (expected 4, attributed 3) bad
> Author G (expected 1, attributed 1) good
>
> This is not caused by different behaviour of git blame or annotate on
> that platform, but by different test input, in turn caused by a sed
> command that forgets to add a newline on NetBSD. Here's the diff of the
> commit that adds "goodbye" to hello.c, for Linux:
>
> @@ -1,4 +1,5 @@
> int main(int argc, const char *argv[])
> {
> puts("hello");
> + puts("goodbye");
> }
>
> We see that it adds an extra TAB,
Curious. On Mac OS X, there is only a single tab.
> but that's not a problem. Here's the
> same on NetBSD:
>
> @@ -1,4 +1,4 @@
> int main(int argc, const char *argv[])
> {
> puts("hello");
> -}
> + puts("goodbye");}
>
> It also adds an extra TAB, but it is missing the newline character
> after the semicolon.
>
> The following patch gets rid of the extra TAB at the beginning, but
> more importantly adds the missing newline at the end in a (hopefully)
> portable way, mentioned in http://sed.sourceforge.net/sedfaq4.html.
Tested on Mac OS X. Works correctly.
> The diff becomes this, on both Linux and NetBSD:
>
> @@ -1,4 +1,5 @@
> int main(int argc, const char *argv[])
> {
> puts("hello");
> + puts("goodbye");
> }
>
> Signed-off-by: Rene Scharfe
> ---
> This regression was introduced by 5a9830cb ("t8001/t8002 (blame):
> add blame -L :funcname tests").
>
> t/annotate-tests.sh | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/t/annotate-tests.sh b/t/annotate-tests.sh
> index 0bfee00..d4e7f47 100644
> --- a/t/annotate-tests.sh
> +++ b/t/annotate-tests.sh
> @@ -245,8 +245,8 @@ test_expect_success 'setup -L :regex' '
> git commit -m "hello" &&
>
> mv hello.c hello.orig &&
> - sed -e "/}/i\\
> - Qputs(\"goodbye\");" hello.c &&
> + sed -e "/}/ {x; s/$/Qputs(\"goodbye\");/; G;}" + tr Q "\\t" >hello.c &&
Thanks.
Acked-by: Eric Sunshine
> GIT_AUTHOR_NAME="G" GIT_AUTHOR_EMAIL="g...@test.git" \
> git commit -a -m "goodbye" &&
--
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
git-http-backend vs gitweb pathinfo mode
Background:
You cam make the same URL work for gitwe and git clone as described in
git-http-backend(1). It says:
To serve gitweb at the same url, use a ScriptAliasMatch to only
those URLs that git http-backend can handle, and forward the rest
to gitweb:
ScriptAliasMatch \
"(?x)^/git/(.*/(HEAD | \
info/refs | \
objects/(info/[^/]+ | \
[0-9a-f]{2}/[0-9a-f]{38} | \
pack/pack-[0-9a-f]{40}\.(pack|idx)) | \
git-(upload|receive)-pack))$" \
/usr/libexec/git-core/git-http-backend/$1
ScriptAlias /git/ /var/www/cgi-bin/gitweb.cgi/
Gitweb has a friendly URL mode that uses pathinfo instead of query
parameters.
Problem:
In pathinfo mode, gitweb sometimes produces URLs ending in /HEAD which
match the git-http-backend regex and therefore get passed to the wrong
CGI.
For example, go to https://git.csx.cam.ac.uk/i/ucs/git/git.git/tree
and click on the gitweb subdirectory which takes you to
https://git.csx.cam.ac.uk/i/ucs/git/git.git/tree/HEAD:/gitweb
then click on [git/git.git] to go back, which takes you to
https://git.csx.cam.ac.uk/i/ucs/git/git.git/tree/HEAD
Half-arsed solution:
I have amended the regex to start like
"(?x)^/git/(.*/((?<=[.]git/)HEAD | \
which solves the problem for me since all my repos have names ending in
.git and this doesn't clash with any gitweb action names. I don't think
this is a general solution because some people like bare extensionless
repo names. On the other hand I don't think the regex should list all the
dozens of gitweb action names. So I'm not sure what the best fix is.
Tony.
--
f.anthony.n.finchhttp://dotat.at/
Forties, Cromarty: East, veering southeast, 4 or 5, occasionally 6 at first.
Rough, becoming slight or moderate. Showers, rain at first. Moderate or good,
occasionally poor at first.
--
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] Documentation/rev-list-options: add missing word in --*-parents
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
Re: [PATCH] log doc: the argument to --encoding is not optional
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
Re: git-http-backend vs gitweb pathinfo mode
Tony Finch wrote: > > For example, go to https://git.csx.cam.ac.uk/i/ucs/git/git.git/tree > and click on the gitweb subdirectory which takes you to > https://git.csx.cam.ac.uk/i/ucs/git/git.git/tree/HEAD:/gitweb > then click on [git/git.git] to go back, which takes you to > https://git.csx.cam.ac.uk/i/ucs/git/git.git/tree/HEAD Whoops, those are the internal not the external URLs... try: https://git.csx.cam.ac.uk/x/ucs/git/git.git/tree -> https://git.csx.cam.ac.uk/x/ucs/git/git.git/tree/HEAD:/gitweb -> https://git.csx.cam.ac.uk/x/ucs/git/git.git/tree/HEAD Tony. -- f.anthony.n.finchhttp://dotat.at/ Forties, Cromarty: East, veering southeast, 4 or 5, occasionally 6 at first. Rough, becoming slight or moderate. Showers, rain at first. Moderate or good, occasionally poor at first. -- 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: [BUG?] gc and impatience
Junio C Hamano wrote: > I am a bit hesitant to dismiss with "It's not the right model", as > the original of accessing the repository from two terminals while > one clearly is being accessed busily by gc falls into the same > category. As to why I think it makes sense: garbage collecting unreferenced objects has nothing to do with updating refs, or checking out a worktree. Think about my earlier "make push.default = current resolve HEAD early"; why would the user want to update the ref that is being pushed? She'd most likely want to continue working on another feature on some other branch, and that's perfectly fine. In long-running runtimes, garbage collection is absolutely essential to the performance. Often, stupidly written garbage collectors that stop-the-world (the execution of the program), compact the memory after collection, and then restart the program, can cause the user to throw that runtime out the window (Emacs has a really stupid one, by the way). Most modern runtimes have concurrent garbage collectors that are allocated very fine-grained slots by the scheduler: so, the program won't suddenly come to a grinding halt to do garbage collection. The reason it's so hard to do concurrent gc is because there can be races between data modification via variables (main program), and data being moved around in memory for compacting (gc). Having said all this, the problem is highly simplified in git, because the object store is a const-store. A particular key (sha-1) is guaranteed never to point to the wrong data. Frankly, even if there is concurrent access to the object store, the worst thing that can happen is that the gc didn't collect some dangling objects that were created during the gc run. Unless you have some irrational fear of introducing some unexpected behavior in some convoluted corner case, I really don't see what the problem is. I'm sure server-side implementations have to do it all the time: GitHub and Gerrit certainly doesn't say "I'm gc'ing; please pull after 10 mins". Perhaps they're more conservative than the client side about gc (space is cheap), but that's just a sane default. > It can very well be two terminals, one on one machine each, both > with the same human end-user interaction. Someone does an SSH my machine to a submarine in Russia over a slow connection. I remove an ordinary file, while she's trying to write to it. When did anyone make any guarantees about no races? What does git gc specifically have to do with this? For the record, you can easily mess up your worktree by running two different worktree updates (checkout/ merge) on two different terminals: nothing forbidding it. I don't see how _not_ forbidding gc on two different terminals is better than forbidding it. This is quite an obscure feature for few super-impatient people, and we haven't even advertised it in any documentation. Unless you can present an alternative now (patch-form, please), I think you're being irrationally conservative about 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/3] Fixes for OS X
A few changes recently broke my build on Mac 10.8, possibly because I have a more strict set of warnings/errors enabled. The first two handle minor problems with the use of APPLE_COMMON_CRYPTO, which was expanded for use in imap-send but had a couple of problems. The last is likely due to curl version skew between Dave Borowitz and myself. (see 912b2ac). There are a few notes on the patches indicating where I was less than sure about my solutions. Brian Gernhardt (3): Makefile: Fix APPLE_COMMON_CRYPTO with BLK_SHA1 OS X: Fix redeclaration of die warning t5551: Remove header from curl cookie file Makefile | 4 +++- git-compat-util.h | 20 ++-- t/t5551-http-fetch.sh | 6 ++ 3 files changed, 15 insertions(+), 15 deletions(-) -- 1.8.4.rc1.384.g0976a17.dirty -- 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/3] Makefile: Fix APPLE_COMMON_CRYPTO with BLK_SHA1
It used to be that APPLE_COMMON_CRYPTO did nothing when BLK_SHA1 was set. But APPLE_COMMON_CRYPTO is now used for more than just SHA1 (see 3ef2bca) so make sure that the appropriate libraries are always set. Signed-off-by: Brian Gernhardt --- Makefile | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 82f2e22..7051956 100644 --- a/Makefile +++ b/Makefile @@ -1182,6 +1182,9 @@ ifdef NEEDS_SSL_WITH_CRYPTO else LIB_4_CRYPTO = $(OPENSSL_LINK) -lcrypto endif +ifdef APPLE_COMMON_CRYPTO + LIB_4_CRYPTO += -framework Security -framework CoreFoundation +endif endif ifdef NEEDS_LIBICONV ifdef ICONVDIR @@ -1413,7 +1416,6 @@ ifdef PPC_SHA1 LIB_H += ppc/sha1.h else ifdef APPLE_COMMON_CRYPTO - LIB_4_CRYPTO += -framework Security -framework CoreFoundation COMPAT_CFLAGS += -DCOMMON_DIGEST_FOR_OPENSSL SHA1_HEADER = else -- 1.8.4.rc1.384.g0976a17.dirty -- 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] t5551: Remove header from curl cookie file
The URL included in the header appears to vary from curl version to curl version. Since we only care about the final few lines, only test them. However, make sure the blank line after the header is still included to make sure there are no extra cookie lines. Signed-off-by: Brian Gernhardt --- I suppose a sed invocation to strip out the URL or comments might be better, but this seemed simpler. t/t5551-http-fetch.sh | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/t/t5551-http-fetch.sh b/t/t5551-http-fetch.sh index 287d22b..8196af1 100755 --- a/t/t5551-http-fetch.sh +++ b/t/t5551-http-fetch.sh @@ -191,9 +191,6 @@ cat >cookies.txtcookies_tail.txt + test_cmp expect_cookies.txt cookies_tail.txt ' test -n "$GIT_TEST_LONG" && test_set_prereq EXPENSIVE -- 1.8.4.rc1.384.g0976a17.dirty -- 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] OS X: Fix redeclaration of die warning
compat/apple-common-crypto.h uses die() in one of its macros, but was included in git-compat-util.h before the definition of die. Fix by simply moving the relevant block after the die/error/warning declarations. Signed-off-by: Brian Gernhardt --- Not sure if this is the best place to move it to, but it's the earliest it can be in the file without causing errors. (Namely that clang has to guess what die() means in apple-common-crypto.h and guesses differently than the actual definition.) git-compat-util.h | 20 ++-- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/git-compat-util.h b/git-compat-util.h index af5f6bb..d60e28d 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -129,16 +129,6 @@ #include #endif -#ifndef NO_OPENSSL -#ifdef APPLE_COMMON_CRYPTO -#include "compat/apple-common-crypto.h" -#else -#include -#include -#endif /* APPLE_COMMON_CRYPTO */ -#include -#endif /* NO_OPENSSL */ - #if defined(__MINGW32__) /* pull in Windows compatibility stuff */ #include "compat/mingw.h" @@ -340,6 +330,16 @@ extern NORETURN void die_errno(const char *err, ...) __attribute__((format (prin extern int error(const char *err, ...) __attribute__((format (printf, 1, 2))); extern void warning(const char *err, ...) __attribute__((format (printf, 1, 2))); +#ifndef NO_OPENSSL +#ifdef APPLE_COMMON_CRYPTO +#include "compat/apple-common-crypto.h" +#else +#include +#include +#endif /* APPLE_COMMON_CRYPTO */ +#include +#endif /* NO_OPENSSL */ + /* * Let callers be aware of the constant return value; this can help * gcc with -Wuninitialized analysis. We restrict this trick to gcc, though, -- 1.8.4.rc1.384.g0976a17.dirty -- 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/RFC] log doc: explain --encoding=none and default output encoding
Jonathan Nieder writes: > Signed-off-by: Jonathan Nieder > --- > I'm not thrilled with the wording. This can probably be explained > more simply. Ideas? Some random thoughts on text, both before and after this patch. - The first stentence in this paragraph up to the semicolon is irrelevant (it is an implementation detail that allows us to re-encode in the first place, and the user does not care). - The use of word "default" is fuzzy. With this description, we want to tell the user to what encoding we reencode to by default, but it is easy to miss that the reencoding always happen for output with or without --encoding= option (which is not clear from the text, especially the original) and incorrectly assume that without --encoding= the recorded text is spit out without mangling. So perhaps along this line? --encoding=:: The encoding used to output the log messages in commit objects. When this option is not given, non-plumbing commands output messages in the commit log encoding (`i18n.commitEncoding`, or UTF-8 if the configuration variable is not set). `--encoding=none` lets you view the raw log message without any reencoding. > > Documentation/pretty-options.txt | 10 -- > 1 file changed, 8 insertions(+), 2 deletions(-) > > diff --git a/Documentation/pretty-options.txt > b/Documentation/pretty-options.txt > index 5e499421..e31fd494 100644 > --- a/Documentation/pretty-options.txt > +++ b/Documentation/pretty-options.txt > @@ -32,8 +32,14 @@ people using 80-column terminals. > The commit objects record the encoding used for the log message > in their encoding header; this option can be used to tell the > command to re-code the commit log message in the encoding > - preferred by the user. For non plumbing commands this > - defaults to UTF-8. > + preferred by the user. "--encoding=none" means to use the > + raw log message without paying attention to its encoding header. > ++ > +For non plumbing commands, the output encoding defaults to the commit > +encoding (as set using the `i18n.commitEncoding` variable, or UTF-8 > +by default). This default can be overridden using the > +`i18n.logOutputEncoding` configuration item. See linkgit:git-config[1] > +for details. > > --notes[=]:: > Show the notes (see linkgit:git-notes[1]) that annotate the -- 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 replace: should it check for object type, and can it replace merges?
From: "Christian Couder" Hi, On Sat, Aug 3, 2013 at 5:13 PM, Philip Oakley wrote: A recent comment http://stackoverflow.com/a/18027030/717355 on a question I asked two years ago about 'grafts' and 'replace' indicates that users think that 'git replace' can't replace a merge commit. The documentation doesn't have any examples and gives the naive impression that one should only replace a simple commit with another simple commit. I am sorry if the documentation gives this impression. I'd like to fix it, but I am not sure what should be changed. Should adding an example be enough? Or do you want it to state that explicitely? I did a quick markup which is shown below (an export of the commit from the gitk viewer) Having looked at the code, I realised that anything can be replaced with anything, which is perhaps not what was intended. The documentation says in the "BUGS" section: "And of course things may break if an object of one type is replaced by an object of another type (for example a blob replaced by a commit)." I hadn't seen that part, being 'hidden' at the end of the paragraph (that is, it's easy to miss;-). If my update was acceptable then that sentence could probably be deleted (though it may require the checks to actually be in the code, and not just a "must" imperative in my documentation update). So yes it is a know bug. A simple thought is that the replace should be like-for-like with regard to object type, though that would not include replacing a sub-module for a tree (and vice versa). Should 'git replace' check the object types to ensure they are sensible? It would probably be a good idea to do that, yeah. But I don't know much about submodules, so I can't say if replacing a sub-module for a tree (and vice versa) should be allowed. Or if there should be a --force-different-objects option for these kinds of special cases. An extra bit of thought made me realise that while a sub-module is represented as a special symbolic commit, it is still just an element of a tree object, so would still be a tree <-> tree replacement, so doesn't break the rule. Would it be reasonable to add examples to indicate the range of replacements, and how to prepare alternative merge commits, or is that a hostage to fortune? Yeah, adding examples would be a good idea. I don't understand what do you mean with "range of replacements", though. There were in two parts: 1) creating a replacement merge commit, and 2) creating a replacement tree, possibly with a sub-module in it. I am not sure preparing alternative commits or merge commits should be an important part of the examples. There are many cases that could be interesting to different users: - replacing a non merge commit with a merge commit (if someone forgot to use --no-ff when merging for example) - replacing a merge commit with a non merge commit (if a rebase should have been done) - and of course replacing a non merge commit with a non merge commit, or a merge commit with a merge commit So I think explaining how another commit can be created from existing commits belongs to some other parts of the git documentation. Yes, I just haven't looked yet. I think there are some examples in the plumbing command man pages. They'd just need referencing. Perhaps there could be such examples in the git hash-object and git filter-branch documentation and we could just point to them. Best, Christian. -- My quick markup, based on a local branch. ---8<--- commit c12c03462f8c65a593e702896b461f1c63d67ec5 Author: Philip Oakley Date: Sat Aug 3 20:20:05 2013 +0100 Doc: 'replace' the same object type, and mention merge commits Signed-off-by: Philip Oakley diff --git a/Documentation/git-replace.txt b/Documentation/git-replace.txt index e0b4057..2ab451c 100644 --- a/Documentation/git-replace.txt +++ b/Documentation/git-replace.txt @@ -20,6 +20,10 @@ The name of the 'replace' reference is the SHA-1 of the object that is replaced. The content of the 'replace' reference is the SHA-1 of the replacement object. +The type of the replacement object must be the same as that of the +object it replaces. Merge commits can be replaced by non-merge commits +and vice versa. + Unless `-f` is given, the 'replace' reference must not yet exist. Replacement references will be used by default by all Git commands --->8--- Philip -- 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 2/3] OS X: Fix redeclaration of die warning
Thanks Brian, Reviewed-by: Jeremy Huddleston Sequoia On Aug 5, 2013, at 8:59, Brian Gernhardt wrote: > compat/apple-common-crypto.h uses die() in one of its macros, but was > included in git-compat-util.h before the definition of die. > > Fix by simply moving the relevant block after the die/error/warning > declarations. > > Signed-off-by: Brian Gernhardt > --- > > Not sure if this is the best place to move it to, but it's the earliest it can > be in the file without causing errors. (Namely that clang has to guess what > die() means in apple-common-crypto.h and guesses differently than the actual > definition.) > > git-compat-util.h | 20 ++-- > 1 file changed, 10 insertions(+), 10 deletions(-) > > diff --git a/git-compat-util.h b/git-compat-util.h > index af5f6bb..d60e28d 100644 > --- a/git-compat-util.h > +++ b/git-compat-util.h > @@ -129,16 +129,6 @@ > #include > #endif > > -#ifndef NO_OPENSSL > -#ifdef APPLE_COMMON_CRYPTO > -#include "compat/apple-common-crypto.h" > -#else > -#include > -#include > -#endif /* APPLE_COMMON_CRYPTO */ > -#include > -#endif /* NO_OPENSSL */ > - > #if defined(__MINGW32__) > /* pull in Windows compatibility stuff */ > #include "compat/mingw.h" > @@ -340,6 +330,16 @@ extern NORETURN void die_errno(const char *err, ...) > __attribute__((format (prin > extern int error(const char *err, ...) __attribute__((format (printf, 1, 2))); > extern void warning(const char *err, ...) __attribute__((format (printf, 1, > 2))); > > +#ifndef NO_OPENSSL > +#ifdef APPLE_COMMON_CRYPTO > +#include "compat/apple-common-crypto.h" > +#else > +#include > +#include > +#endif /* APPLE_COMMON_CRYPTO */ > +#include > +#endif /* NO_OPENSSL */ > + > /* > * Let callers be aware of the constant return value; this can help > * gcc with -Wuninitialized analysis. We restrict this trick to gcc, though, > -- > 1.8.4.rc1.384.g0976a17.dirty > smime.p7s Description: S/MIME cryptographic signature
Re: [PATCH 1/3] Makefile: Fix APPLE_COMMON_CRYPTO with BLK_SHA1
Thanks Brian, Reviewed-by: Jeremy Huddleston Sequoia On Aug 5, 2013, at 8:59, Brian Gernhardt wrote: > It used to be that APPLE_COMMON_CRYPTO did nothing when BLK_SHA1 was > set. But APPLE_COMMON_CRYPTO is now used for more than just SHA1 (see > 3ef2bca) so make sure that the appropriate libraries are always set. > > Signed-off-by: Brian Gernhardt > --- > Makefile | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/Makefile b/Makefile > index 82f2e22..7051956 100644 > --- a/Makefile > +++ b/Makefile > @@ -1182,6 +1182,9 @@ ifdef NEEDS_SSL_WITH_CRYPTO > else > LIB_4_CRYPTO = $(OPENSSL_LINK) -lcrypto > endif > +ifdef APPLE_COMMON_CRYPTO > + LIB_4_CRYPTO += -framework Security -framework CoreFoundation > +endif > endif > ifdef NEEDS_LIBICONV > ifdef ICONVDIR > @@ -1413,7 +1416,6 @@ ifdef PPC_SHA1 > LIB_H += ppc/sha1.h > else > ifdef APPLE_COMMON_CRYPTO > - LIB_4_CRYPTO += -framework Security -framework CoreFoundation > COMPAT_CFLAGS += -DCOMMON_DIGEST_FOR_OPENSSL > SHA1_HEADER = > else > -- > 1.8.4.rc1.384.g0976a17.dirty > smime.p7s Description: S/MIME cryptographic signature
Re: [PATCH] git_mkstemps: improve test suite test
wor...@alum.mit.edu (Dale R. Worley) writes: > Commit 52749 fixes a bug regarding testing the return of an open() > call for success/failure. Improve the testsuite test for that fix by > removing the helper program 'test-close-fd-0' and replacing it with > the shell redirection '<&-'. (The redirection is Posix, so it should > be portable.) > > Signed-off-by: Dale Worley > --- Sorry, but I have no idea what commit you are talking about, and as far as I can see there is no such file test-close-fd-0.c in my tree. Puzzled... > Makefile |1 - > test-close-fd-0.c | 14 -- > 2 files changed, 0 insertions(+), 15 deletions(-) > delete mode 100644 test-close-fd-0.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] cherry-pick: allow "-" as abbreviation of '@{-1}'
Thomas Rast writes:
> Hiroshige Umino writes:
>
>> As "git cherry-pick -" or "git merge -" is convenient to
>> switch back to or merge the previous branch,
>> "git cherry-pick -" is abbreviation of "git cherry-pick @{-1}"
>> to pick up a commit from the previous branch conveniently.
>
> The first line is confusing. Did you mean to invoke the existing 'git
> *checkout* -' and 'git merge -' functionality as a reason why 'git
> cherry-pick -' should exist?
I think that is what was meant. Just like "-" abbreviation is handy
for users of "checkout" and "merge", "cherry-pick" might. I am not
sure if you would often cherry-pick from the previous branch, but
for the sake of completeness and uniformity, the guiding principle
could be "at any point on the command line where a branch name is
accepted, if a '-' could not possibly mean any other thing is wanted
(e.g. doing something on the standard input), it should stand as the
name of the previous branch".
I agree with everything you said in your review. The patch is going
in the right direction, but it needs a bit more work.
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
Re: What's cooking in git.git (Jul 2013, #09; Mon, 29)
Mark Levedahl writes: > I have been using this patch since Ramsey first sent it, have noticed > no trouble over that time but all of my work is with filemode=true > (has been since I started using git as Cygwin is a secondary platform > for me and interoperability with repos on Linux is an absolute > requirement). Torsten Bögershausen writes: > So I think we can and should remove compat/cygwin.[ch] without further > cooking, to be on the save side. 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
Re: [PATCH] fix typo in documentation of git-svn
Thanks, will apply after fixing whitespace damage to the 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] .mailmap: Multiple addresses of Michael S. Tsirkin
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
Re: git replace: should it check for object type, and can it replace merges?
Christian Couder writes: > Hi, > > On Sat, Aug 3, 2013 at 5:13 PM, Philip Oakley wrote: >> A recent comment http://stackoverflow.com/a/18027030/717355 on a question I >> asked two years ago about 'grafts' and 'replace' indicates that users think >> that 'git replace' can't replace a merge commit. The documentation doesn't >> have any examples and gives the naive impression that one should only >> replace a simple commit with another simple commit. > > I am sorry if the documentation gives this impression. > I'd like to fix it, but I am not sure what should be changed. > Should adding an example be enough? Or do you want it to state that > explicitely? > >> Having looked at the code, I realised that anything can be replaced with >> anything, which is perhaps not what was intended. > > The documentation says in the "BUGS" section: > > "And of course things may break if an object of one type is replaced > by an object of another type (for example a blob replaced by a > commit)." > > So yes it is a know bug. Is that even a BUG? The users are pretty much asking for it if they place an object name of a random wrong object themselves. I agree that a hand-holding wrapper "git replace" could help them to avoid mistakes, though. -- 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] Add missing test file for UTF-16.
Duy Nguyen writes: > On Sun, Aug 4, 2013 at 12:26 AM, brian m. carlson > wrote: >> The test file that the UTF-16 rejection test looks for is missing, but this >> went >> unnoticed because the test is expected to fail anyway; as a consequence, the >> test fails because the file containing the commit message is missing, and not >> because the test file contains a NUL byte. Fix this by including a sample >> text >> file containing a commit message encoded in UTF-16. > > Tested-by: Duy Nguyen > > and sorry, my bad. I think we need your sign-off in this patch. I think 37576c14 (commit_tree(): refuse commit messages that contain NULs, 2011-12-15) that marked this test with "test_expect_failure" is broken with or without this fix. It should be more like so: diff --git a/t/t3900-i18n-commit.sh b/t/t3900-i18n-commit.sh index 37ddabb..5e72d72 100755 --- a/t/t3900-i18n-commit.sh +++ b/t/t3900-i18n-commit.sh @@ -34,9 +34,9 @@ test_expect_success 'no encoding header for base case' ' test z = "z$E" ' -test_expect_failure 'UTF-16 refused because of NULs' ' +test_expect_success 'UTF-16 refused because of NULs' ' echo UTF-16 >F && - git commit -a -F "$TEST_DIRECTORY"/t3900/UTF-16.txt + test_must_fail git commit -a -F "$TEST_DIRECTORY"/t3900/UTF-16.txt ' -- 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] commit: reject non-characters
Peter Krefting writes: > Peter Krefting: > >> -/* U+FFFE and U+ are guaranteed non-characters. */ >> -if ((codepoint & 0x1e) == 0xfffe) >> +/* U+xxFFFE and U+xx are guaranteed non-characters. */ >> +if ((codepoint & 0xe) == 0xfffe) >> +return bad_offset; > > Drats, there is an F too many in the bitmask, it should be: > > +if ((codepoint & 0xfffe) == 0xfffe) Indeed. -- >8 -- Subject: [PATCH] commit: typofix for xxFFF[EF] check We wanted to catch all codepoints that ends with FFFE and , not with 0FFFE and 0. Noticed and corrected by Peter Krefting. Signed-off-by: Junio C Hamano --- commit.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/commit.c b/commit.c index 7dcfeea..38d8979 100644 --- a/commit.c +++ b/commit.c @@ -1306,7 +1306,7 @@ static int find_invalid_utf8(const char *buf, int len) if ((codepoint & 0x1ff800) == 0xd800) return bad_offset; /* U+xxFFFE and U+xx are guaranteed non-characters. */ - if ((codepoint & 0xe) == 0xfffe) + if ((codepoint & 0xfffe) == 0xfffe) return bad_offset; /* So are anything in the range U+FDD0..U+FDEF. */ if (codepoint >= 0xfdd0 && codepoint <= 0xfdef) -- 1.8.4-rc1-129-g1f3472b -- 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: About close() in commit_lock_file()
Duy Nguyen writes: > close() is added in commit_lock_file(), before rename(), by 4723ee9 > (Close files opened by lock_file() before unlinking. - 2007-11-13), > which is needed by Windows. But doesn't that create a gap between > close() and rename() on other platforms where another process can > replace .lock file with something else before rename() is executed? Interesting. > Should we enclose close() in #ifdef __MINGW32__ (and maybe > __CYGWIN__)? Or just have "close and retry" code after seeing rename() fails with some known errno, without singling out a particular platform? -- 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_mkstemps: improve test suite test
Junio C Hamano writes: > wor...@alum.mit.edu (Dale R. Worley) writes: > >> Commit 52749 fixes a bug regarding testing the return of an open() >> call for success/failure. Improve the testsuite test for that fix by >> removing the helper program 'test-close-fd-0' and replacing it with >> the shell redirection '<&-'. (The redirection is Posix, so it should >> be portable.) >> >> Signed-off-by: Dale Worley >> --- > > Sorry, but I have no idea what commit you are talking about, and as > far as I can see there is no such file test-close-fd-0.c in my tree. > > Puzzled... OK, let's do this on top of a77f106c (run-command: dup_devnull(): guard against syscalls failing, 2013-07-12) which is at the tip of tr/fd-gotcha-fixes that contains the earlier fix. -- >8 -- From: "Dale R. Worley" Date: Fri, 2 Aug 2013 20:27:23 -0400 Subject: [PATCH] t0070: test that git_mkstemps correctly checks return value of open() Signed-off-by: Dale R. Worley Signed-off-by: Junio C Hamano --- t/t0070-fundamental.sh | 4 1 file changed, 4 insertions(+) diff --git a/t/t0070-fundamental.sh b/t/t0070-fundamental.sh index da2c504..ff3776f 100755 --- a/t/t0070-fundamental.sh +++ b/t/t0070-fundamental.sh @@ -25,6 +25,10 @@ test_expect_success POSIXPERM 'mktemp to unwritable directory prints filename' ' grep "cannotwrite/test" err ' +test_expect_success 'git_mkstemps_mode does not fail if fd 0 is not open' ' + git commit --allow-empty -m message <&- +' + test_expect_success 'check for a bug in the regex routines' ' # if this test fails, re-build git with NO_REGEX=1 test-regex -- 1.8.4-rc1-129-g1f3472b -- 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: [BUG?] gc and impatience
Martin Fick wrote: > https://gerrit-review.googlesource.com/#/c/35215/ Very cool. Of what I understood: So, the problem is that my .git/objects/pack is polluted with little packs everytime I fetch (or push, if you're the server), and this is problematic from the perspective of a overtly (naively) aggressive gc that hammers out all fragmentation. So, on the first run, the little packfiles I have are all "consolidated" into big packfiles; you also write .keep files to say that "don't gc these big packs we just generated". In subsequent runs, the little packfiles from the fetch are absorbed into a pack that is immune to gc. You're also using a size heuristic, to consolidate similarly sized packfiles. You also have a --ratio to tweak the ratio of sizes. I've checked it in and started using it; so yeah: I'll chew on it for a few weeks. 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
Re: [PATCH] t8001, t8002: fix "blame -L :literal" test on NetBSD
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
Re: [PATCH 1/3] Makefile: Fix APPLE_COMMON_CRYPTO with BLK_SHA1
Brian Gernhardt writes: > It used to be that APPLE_COMMON_CRYPTO did nothing when BLK_SHA1 was > set. But APPLE_COMMON_CRYPTO is now used for more than just SHA1 (see > 3ef2bca) so make sure that the appropriate libraries are always set. > > Signed-off-by: Brian Gernhardt > --- > Makefile | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/Makefile b/Makefile > index 82f2e22..7051956 100644 > --- a/Makefile > +++ b/Makefile > @@ -1182,6 +1182,9 @@ ifdef NEEDS_SSL_WITH_CRYPTO > else > LIB_4_CRYPTO = $(OPENSSL_LINK) -lcrypto > endif > +ifdef APPLE_COMMON_CRYPTO > + LIB_4_CRYPTO += -framework Security -framework CoreFoundation > +endif > endif > ifdef NEEDS_LIBICONV > ifdef ICONVDIR > @@ -1413,7 +1416,6 @@ ifdef PPC_SHA1 > LIB_H += ppc/sha1.h > else > ifdef APPLE_COMMON_CRYPTO > - LIB_4_CRYPTO += -framework Security -framework CoreFoundation > COMPAT_CFLAGS += -DCOMMON_DIGEST_FOR_OPENSSL > SHA1_HEADER = > else Hmph. So the people previously tested this must have built imap-send without blk-sha1, which not just linked with these libs but also included the header file and defined the -DCOMMON_DIGEST_FOR_OPENSSL preprocessor macro. Building with blk-sha1 would not have worked for them. Now we always link with these libraries, even when building with blk-sha1. Do the COMPAT_CFLAGS and SHA1_HEADER pieces only needed when using the SHA1 digest implementation from CommonCrypto and nothing imap-send uses? -- 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 2/3] OS X: Fix redeclaration of die warning
Brian Gernhardt writes: > compat/apple-common-crypto.h uses die() in one of its macros, but was > included in git-compat-util.h before the definition of die. > > Fix by simply moving the relevant block after the die/error/warning > declarations. Puzzled. What needs fixing??? Ahh, that one is not just making #define macros, but defining static inline functions. I wonder if they need to be static inlines to be duplicated at each call sites in the first place. Wouldn't it be better to create a compat/something.c file to be linked with? > Signed-off-by: Brian Gernhardt > --- > > Not sure if this is the best place to move it to, but it's the earliest it > can > be in the file without causing errors. (Namely that clang has to guess what > die() means in apple-common-crypto.h and guesses differently than the actual > definition.) > > git-compat-util.h | 20 ++-- > 1 file changed, 10 insertions(+), 10 deletions(-) > > diff --git a/git-compat-util.h b/git-compat-util.h > index af5f6bb..d60e28d 100644 > --- a/git-compat-util.h > +++ b/git-compat-util.h > @@ -129,16 +129,6 @@ > #include > #endif > > -#ifndef NO_OPENSSL > -#ifdef APPLE_COMMON_CRYPTO > -#include "compat/apple-common-crypto.h" > -#else > -#include > -#include > -#endif /* APPLE_COMMON_CRYPTO */ > -#include > -#endif /* NO_OPENSSL */ > - > #if defined(__MINGW32__) > /* pull in Windows compatibility stuff */ > #include "compat/mingw.h" > @@ -340,6 +330,16 @@ extern NORETURN void die_errno(const char *err, ...) > __attribute__((format (prin > extern int error(const char *err, ...) __attribute__((format (printf, 1, > 2))); > extern void warning(const char *err, ...) __attribute__((format (printf, 1, > 2))); > > +#ifndef NO_OPENSSL > +#ifdef APPLE_COMMON_CRYPTO > +#include "compat/apple-common-crypto.h" > +#else > +#include > +#include > +#endif /* APPLE_COMMON_CRYPTO */ > +#include > +#endif /* NO_OPENSSL */ > + > /* > * Let callers be aware of the constant return value; this can help > * gcc with -Wuninitialized analysis. We restrict this trick to gcc, though, -- 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
[ANN] SubGit 2.0 Released
Hello all, Our team is proud to announce SubGit 2.0.0 release! New version is available for download at SubGit web site at http://subgit.com/ SubGit lets one to set up a bi-directional Git-SVN mirror, and thus it allows users to choose freely between Subversion and Git version control systems. SubGit is a perfect tool for those who's going to migrate from Subversion to Git as well as from Git to SVN. New version introduces the following major features: 1. Support for remote Subversion repositories; 2. One-shot import from Subversion to Git; 3. Flexible branches and tags layout; 4. Significant performance improvements. SubGit is a closed source Java application, which is free for use in Open Source and Academic projects, as well as in any teams with up to 10 committers. Besides, there are no limitations on the time you may evaluate SubGit in commercial or closed source projects. Atlassian Stash users can install SVN Mirror Add-on which is based on SubGit 2.0, so they can manage their Git-SVN mirrors right from Stash UI: https://marketplace.atlassian.com/plugins/org.tmatesoft.subgit.stash-svn-importer For the detailed release notes please refer to http://subgit.com/documentation/release-notes.html Documentation on remote Git-SVN mirror mode: http://subgit.com/book-remote/ Documentation on local Git-SVN mirror mode: http://subgit.com/book/ Documentation on one-shot Git-SVN import: http://subgit.com/book-remote/#import SubGit Issues Tracker: http://issues.tmatesoft.com/issues/SGT Follow us at https://twitter.com/subgit and https://plus.google.com/114128677298030695536 With Best Regards, Semyon Vadishev on behalf of SubGit Team, TMate Software, http://subgit.com/ - Git-SVN Mirror! http://svnkit.com/ - Java [Sub]Versioning Library! http://hg4j.com/ - Java Mercurial Library! http://sqljet.com/ - Java SQLite Library! -- 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 v3] gc: reject if another gc is running, unless --force is given
Nguyễn Thái Ngọc Duy writes:
> diff --git a/builtin/gc.c b/builtin/gc.c
> index 6be6c8d..1f33908 100644
> --- a/builtin/gc.c
> +++ b/builtin/gc.c
> @@ -167,11 +167,66 @@ static int need_to_gc(void)
> return 1;
> }
>
> +static int gc_running(int force)
Sounds like a bool asking "Is a GC running? Yes, or no?". Since
there is no room for "force" to enter in order to answer that
question, I have to guess that this function is somewhat misnamed.
> +{
> + static struct lock_file lock;
> + struct utsname utsname;
> + struct stat st;
> + uintmax_t pid;
> + FILE *fp;
> + int fd, should_exit;
> +
> + if (uname(&utsname))
> + strcpy(utsname.nodename, "unknown");
> +
> + fd = hold_lock_file_for_update(&lock,
> + git_path("gc-%s.pid", utsname.nodename), 0);
> + if (!force) {
> + if (fd < 0)
> + return 1;
> +
> + fp = fopen(git_path("gc-%s.pid", utsname.nodename), "r");
I would have imagined that you would use a lockfile gc.pid and write
nodename and pid to it (and if nodename matches, you know pid may
have a chance to actually match another instance of "gc", while
there will not way it matches if nodename is different, and do
something intelligent about it). By letting GC that is running on
another node to be completely unnoticed, this change is closing the
door to "do something intelligent about it", like giving it the same
12 hour limit.
> + should_exit =
> + fp != NULL &&
> + !fstat(fileno(fp), &st) &&
> + /*
> + * 12 hour limit is very generous as gc should
> + * never take that long. On the other hand we
> + * don't really need a strict limit here,
> + * running gc --auto one day late is not a big
> + * problem. --force can be used in manual gc
> + * after the user verifies that no gc is
> + * running.
> + */
> + time(NULL) - st.st_mtime <= 12 * 3600 &&
> + fscanf(fp, "%"PRIuMAX, &pid) == 1 &&
> + !kill(pid, 0);
> + if (fp != NULL)
> + fclose(fp);
> + if (should_exit) {
> + if (fd >= 0)
> + rollback_lock_file(&lock);
> + return 1;
> + }
> + }
> +
> + if (fd >= 0) {
> + struct strbuf sb = STRBUF_INIT;
> + strbuf_addf(&sb, "%"PRIuMAX"\n", (uintmax_t) getpid());
> + write_in_full(fd, sb.buf, sb.len);
> + strbuf_release(&sb);
> + commit_lock_file(&lock);
> + }
> +
> + return 0;
> +}
After reading what the whole function does, I think the purpose of
this function is to take gc-lock (with optionally force). Perhaps a
name along the lines of "lock_gc", "gc_lock", "lock_repo_for_gc",
would be more appropriate.
--
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 v3] gc: reject if another gc is running, unless --force is given
Junio C Hamano wrote: > [...] The other comments mostly make sense. > After reading what the whole function does, I think the purpose of > this function is to take gc-lock (with optionally force). Perhaps a > name along the lines of "lock_gc", "gc_lock", "lock_repo_for_gc", > would be more appropriate. The whole point of this exercise is to _not_ lock up the repo during gc, so I can do minimal commit/ worktree/ ref update operations when it's running. I can't expect the reflog to work, so complex history-rewriting operations should be avoided; that's about it, I think. -- 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: [BUG?] gc and impatience
On Monday, August 05, 2013 11:34:24 am Ramkumar Ramachandra wrote: > Martin Fick wrote: > > https://gerrit-review.googlesource.com/#/c/35215/ > > Very cool. Of what I understood: > > So, the problem is that my .git/objects/pack is polluted > with little packs everytime I fetch (or push, if you're > the server), and this is problematic from the > perspective of a overtly (naively) aggressive gc that > hammers out all fragmentation. So, on the first run, > the little packfiles I have are all "consolidated" into > big packfiles; you also write .keep files to say that > "don't gc these big packs we just generated". In > subsequent runs, the little packfiles from the fetch are > absorbed into a pack that is immune to gc. You're also > using a size heuristic, to consolidate similarly sized > packfiles. You also have a --ratio to tweak the ratio > of sizes. Yes, pretty much. I suspect that a smarter implementation would do a "less good job of packing" to save time also. I think this can be done by further limiting much of the lookups to the packs being packed (or some limited set of the greater packfiles). I admit I don't really understand how much the packing does today, but I believe it still looks at the larger packs with keeps to potentially deltafy against them, or to determine which objects are duplicated and thus should not be put into the new smaller packfiles? I say this because the time savings of this script is not as significant as I would have expected it to be (but the IO is). I think that it is possible to design a git gc using this rolling approach that would actually greatly reduce the time spent packing also. However, I don't think that can easily be done in a script like mine which just wraps itself around git gc. I hope that someone more familiar with git gc than me might take this on some day. :) > I've checked it in and started using it; so yeah: I'll > chew on it for a few weeks. The script also does some nasty timestamp manipulations that I am not proud of. They had significant time impacts for us, and likely could have been achieved some other way. They shouldn't be relevant to the packing algo though. I hope it doesn't interfere with the evaluation of the approach. Thanks for taking an interest in it, -Martin -- The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation -- 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: [PATCHv3 6/9] hash-object: Replace stdin parsing OPT_BOOLEAN by OPT_COUNTUP
Stefan Beller writes:
> This task emerged from b04ba2bb (parse-options: deprecate OPT_BOOLEAN,
> 2011-09-27). hash-object is a plumbing layer command, so better
> not change the input/output behavior for now.
>
> Unfortunately we have these lines relying on the count up mechanism of
> OPT_BOOLEAN:
>
> if (hashstdin > 1)
> errstr = "Multiple --stdin arguments are not supported";
>
> Maybe later, when the plumbing is refined (git 2.0?), we can drop that
> error message and replace the OPT_COUNTUP by OPT_BOOL.
I am of two minds about that as a future direction.
The original motivation of this is that it was too easy to see
git hash-object Makefile COPYING
to work as expected, and extend that knowledge to expect this
git hash-objects --stdin --stdin
to somehow work without thinking things through.
So it is not about refining plumbing, but is about educating users.
Because a popular system will always have influx of users yet to be
educated, I do not think it makes sense to drop this safety.
The patch itself, and others so far except 1 and 2 which are too big
for me to carefully review right now, seem to make sense.
Thanks.
> Signed-off-by: Stefan Beller
> ---
> builtin/hash-object.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/builtin/hash-object.c b/builtin/hash-object.c
> index 4aea5bb..d7fcf4c 100644
> --- a/builtin/hash-object.c
> +++ b/builtin/hash-object.c
> @@ -71,7 +71,7 @@ static const char *vpath;
> static const struct option hash_object_options[] = {
> OPT_STRING('t', NULL, &type, N_("type"), N_("object type")),
> OPT_BOOL('w', NULL, &write_object, N_("write the object into the object
> database")),
> - OPT_BOOLEAN( 0 , "stdin", &hashstdin, N_("read the object from stdin")),
> + OPT_COUNTUP( 0 , "stdin", &hashstdin, N_("read the object from stdin")),
> OPT_BOOL( 0 , "stdin-paths", &stdin_paths, N_("read file names from
> stdin")),
> OPT_BOOL( 0 , "no-filters", &no_filters, N_("store file as is without
> filters")),
> OPT_STRING( 0 , "path", &vpath, N_("file"), N_("process file as it were
> from this path")),
--
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: [PATCHv3 7/9] config parsing options: allow one flag multiple times
Stefan Beller writes:
> This task emerged from b04ba2bb (parse-options: deprecate OPT_BOOLEAN,
> 2011-09-27).
>
> This commit introduces a change for the users, after this patch
> you can pass one of the config level flags multiple times:
> Before:
> $ git config --global --global --list
> error: only one config file at a time.
> usage: ...
>
> Afterwards this will work. This is due to the following check in the code:
> if (use_global_config + use_system_config + use_local_config +
> !!given_config_file + !!given_config_blob > 1) {
> error("only one config file at a time.");
> usage_with_options(builtin_config_usage,
> builtin_config_options);
> }
Of course, you could further lose that "at most one of them" logic
by using CMDMODE. That will involve updating the logic that
currently looks at these three variables to look at one enum that
can have 4 states (nothing specified, and one of these three
specified), which will be more involved change, but the resulting
code may become simpler (I didn't try---I am just speculating).
Thanks.
>
> With OPT_BOOL instead of OPT_BOOLEAN the variables use_global_config,
> use_system_config, use_local_config will only have the value 0 if the
> command line option was not passed or 1 no matter how often the
> respective command line option was passed.
>
> Signed-off-by: Stefan Beller
> ---
> builtin/config.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/builtin/config.c b/builtin/config.c
> index da12fdb..4ab9e9a 100644
> --- a/builtin/config.c
> +++ b/builtin/config.c
> @@ -50,9 +50,9 @@ static int respect_includes = -1;
>
> static struct option builtin_config_options[] = {
> OPT_GROUP(N_("Config file location")),
> - OPT_BOOLEAN(0, "global", &use_global_config, N_("use global config
> file")),
> - OPT_BOOLEAN(0, "system", &use_system_config, N_("use system config
> file")),
> - OPT_BOOLEAN(0, "local", &use_local_config, N_("use repository config
> file")),
> + OPT_BOOL(0, "global", &use_global_config, N_("use global config file")),
> + OPT_BOOL(0, "system", &use_system_config, N_("use system config file")),
> + OPT_BOOL(0, "local", &use_local_config, N_("use repository config
> file")),
> OPT_STRING('f', "file", &given_config_file, N_("file"), N_("use given
> config file")),
> OPT_STRING(0, "blob", &given_config_blob, N_("blob-id"), N_("read
> config from given blob object")),
> OPT_GROUP(N_("Action")),
--
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] log: use true parents for diff when walking reflogs
Thomas Rast writes: > The reflog walking logic (git log -g) replaces the true parent list > with the preceding commit in the reflog. This results in bogus commit > diffs when combined with options such as -p; the diff is against the > reflog predecessor, not the parent of the commit. > > Save the true parents on the side, extending the functions from the > previous commit. The diff logic picks them up and uses them to show > the correct diffs. > > We do have to be somewhat careful about repeated calling of > save_parents(), since the reflog may list a commit more than once. We > now store (commit_list*)-1 to distinguish the "not saved yet" and > "root commit" cases. This lets us preserve an empty parent list even > if save_parents() is repeatedly called. > > Suggested-by: Jeff King > Signed-off-by: Thomas Rast > --- > > Jeff King wrote: >> >> Your description (and solution) make a lot of sense to me. Another code >> path that has a similar problem is the "-g" reflog walker. It rewrites >> the parents based on the reflog, and the diffs it produces are mostly >> useless (e.g., try "git stash list -p"). >> >> Should we be applying the same technique there? > > Good point. This is how. It applies on top of the other patch. Thanks. > It doesn't really help for 'git stash list -p', though, because > stashes are merge commits. Now they just don't show anything. > could try 'git stash list -p -m', though. Using --first-parent may be more convenient and useful. -- 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] remote-hg: add shared repo upgrade
From: Felipe Contreras
6796d49 (remote-hg: use a shared repository store) introduced a bug by
making the shared repository '.git/hg', which is already used before
that patch, so clones that happened before that patch, fail after that
patch, because there's no shared Mercurial repo.
It's trivial to upgrade to the new organization by copying the Mercurial
repo from one of the remotes (e.g. 'origin'), so let's do so.
Reported-by: Joern Hees
Signed-off-by: Felipe Contreras
---
contrib/remote-helpers/git-remote-hg | 21 -
1 file changed, 16 insertions(+), 5 deletions(-)
diff --git a/contrib/remote-helpers/git-remote-hg
b/contrib/remote-helpers/git-remote-hg
index 0194c67..02404dc 100755
--- a/contrib/remote-helpers/git-remote-hg
+++ b/contrib/remote-helpers/git-remote-hg
@@ -391,11 +391,22 @@ def get_repo(url, alias):
os.makedirs(dirname)
else:
shared_path = os.path.join(gitdir, 'hg')
-if not os.path.exists(shared_path):
-try:
-hg.clone(myui, {}, url, shared_path, update=False, pull=True)
-except:
-die('Repository error')
+
+# check and upgrade old organization
+hg_path = os.path.join(shared_path, '.hg')
+if os.path.exists(shared_path) and not os.path.exists(hg_path):
+repos = os.listdir(shared_path)
+for x in repos:
+local_hg = os.path.join(shared_path, x, 'clone', '.hg')
+if not os.path.exists(local_hg):
+continue
+shutil.copytree(local_hg, hg_path)
+
+# setup shared repo (if not there)
+try:
+hg.peer(myui, {}, shared_path, create=True)
+except error.RepoError:
+pass
if not os.path.exists(dirname):
os.makedirs(dirname)
--
1.7.9.5
--
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 02/11] t8001/t8002: blame: demonstrate -L bounds checking bug
Eric Sunshine writes: > A bounds checking bug allows the X in -LX to extend one line past the > end of file. For example, given a file with 5 lines, -L6 is accepted as > valid. Demonstrate this problem. > > While here, also add tests to check that the remaining cases of X and Y > in -LX,Y are handled correctly at and in the vicinity of end-of-file. > > Signed-off-by: Eric Sunshine > --- > t/annotate-tests.sh | 22 ++ > 1 file changed, 22 insertions(+) > > diff --git a/t/annotate-tests.sh b/t/annotate-tests.sh > index 3524eaf..02fbbf1 100644 > --- a/t/annotate-tests.sh > +++ b/t/annotate-tests.sh > @@ -225,10 +225,32 @@ test_expect_success 'blame -L /RE/,-N' ' > check_count -L/99/,-3 B 1 B2 1 D 1 > ' > > +# 'file' ends with an incomplete line, so 'wc' reports one fewer lines than > +# git-blame sees, hence the last line is actually $(wc...)+1. > +test_expect_success 'blame -L X (X == nlines)' ' > + n=$(expr $(wc -l + check_count -L$n C 1 > +' > + > +test_expect_failure 'blame -L X (X == nlines + 1)' ' > + n=$(expr $(wc -l + test_must_fail $PROG -L$n file > +' > + > test_expect_success 'blame -L X (X > nlines)' ' > test_must_fail $PROG -L12345 file > ' > +test_expect_success 'blame -L ,Y (Y == nlines)' ' > + n=$(expr $(wc -l + check_count -L,$n A 1 B 1 B1 1 B2 1 "A U Thor" 1 C 1 D 1 E 1 > +' > + > +test_expect_success 'blame -L ,Y (Y == nlines + 1)' ' > + n=$(expr $(wc -l + test_must_fail $PROG -L,$n file > +' > + This is somewhat curious. Does the problem triggers only on a file that ends with an incomplete line, or the test was inserted at this location only because it was convenient and the problem exists with or without the incomplete final line? I am guessing that it is the latter. Thanks. > test_expect_success 'blame -L ,Y (Y > nlines)' ' > test_must_fail $PROG -L,12345 file > ' -- 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] remote-hg: add shared repo upgrade
From: Felipe Contreras
6796d49 (remote-hg: use a shared repository store) introduced a bug by
making the shared repository '.git/hg', which is already used before
that patch, so clones that happened before that patch, fail after that
patch, because there's no shared Mercurial repo.
It's trivial to upgrade to the new organization by copying the Mercurial
repo from one of the remotes (e.g. 'origin'), so let's do so.
Reported-by: Joern Hees
Signed-off-by: Felipe Contreras
Signed-off-by: Antoine Pelisse
---
contrib/remote-helpers/git-remote-hg | 21 -
1 file changed, 16 insertions(+), 5 deletions(-)
diff --git a/contrib/remote-helpers/git-remote-hg
b/contrib/remote-helpers/git-remote-hg
index 0194c67..02404dc 100755
--- a/contrib/remote-helpers/git-remote-hg
+++ b/contrib/remote-helpers/git-remote-hg
@@ -391,11 +391,22 @@ def get_repo(url, alias):
os.makedirs(dirname)
else:
shared_path = os.path.join(gitdir, 'hg')
-if not os.path.exists(shared_path):
-try:
-hg.clone(myui, {}, url, shared_path, update=False, pull=True)
-except:
-die('Repository error')
+
+# check and upgrade old organization
+hg_path = os.path.join(shared_path, '.hg')
+if os.path.exists(shared_path) and not os.path.exists(hg_path):
+repos = os.listdir(shared_path)
+for x in repos:
+local_hg = os.path.join(shared_path, x, 'clone', '.hg')
+if not os.path.exists(local_hg):
+continue
+shutil.copytree(local_hg, hg_path)
+
+# setup shared repo (if not there)
+try:
+hg.peer(myui, {}, shared_path, create=True)
+except error.RepoError:
+pass
if not os.path.exists(dirname):
os.makedirs(dirname)
--
1.7.9.5
--
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] remote-hg: add shared repo upgrade
On Mon, Aug 5, 2013 at 2:22 PM, Antoine Pelisse wrote: > From: Felipe Contreras > > 6796d49 (remote-hg: use a shared repository store) introduced a bug by > making the shared repository '.git/hg', which is already used before > that patch, so clones that happened before that patch, fail after that > patch, because there's no shared Mercurial repo. > > It's trivial to upgrade to the new organization by copying the Mercurial > repo from one of the remotes (e.g. 'origin'), so let's do so. In addition to that, simplify the shared repo initialization; if the repository is shared, the pull on the child will use the parent's storage, so there's no need for the initial clone. And make sure the shared repository is always present. It seems pretty clear to me that we are talking about multiple patches here. -- Felipe Contreras -- 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 02/11] t8001/t8002: blame: demonstrate -L bounds checking bug
On Mon, Aug 5, 2013 at 3:29 PM, Junio C Hamano wrote: > Eric Sunshine writes: > >> A bounds checking bug allows the X in -LX to extend one line past the >> end of file. For example, given a file with 5 lines, -L6 is accepted as >> valid. Demonstrate this problem. >> >> While here, also add tests to check that the remaining cases of X and Y >> in -LX,Y are handled correctly at and in the vicinity of end-of-file. >> >> Signed-off-by: Eric Sunshine >> --- >> t/annotate-tests.sh | 22 ++ >> 1 file changed, 22 insertions(+) >> >> diff --git a/t/annotate-tests.sh b/t/annotate-tests.sh >> index 3524eaf..02fbbf1 100644 >> --- a/t/annotate-tests.sh >> +++ b/t/annotate-tests.sh >> @@ -225,10 +225,32 @@ test_expect_success 'blame -L /RE/,-N' ' >> check_count -L/99/,-3 B 1 B2 1 D 1 >> ' >> >> +# 'file' ends with an incomplete line, so 'wc' reports one fewer lines than >> +# git-blame sees, hence the last line is actually $(wc...)+1. >> +test_expect_success 'blame -L X (X == nlines)' ' >> + n=$(expr $(wc -l > + check_count -L$n C 1 >> +' >> + >> +test_expect_failure 'blame -L X (X == nlines + 1)' ' >> + n=$(expr $(wc -l > + test_must_fail $PROG -L$n file >> +' >> + >> test_expect_success 'blame -L X (X > nlines)' ' >> test_must_fail $PROG -L12345 file >> ' >> +test_expect_success 'blame -L ,Y (Y == nlines)' ' >> + n=$(expr $(wc -l > + check_count -L,$n A 1 B 1 B1 1 B2 1 "A U Thor" 1 C 1 D 1 E 1 >> +' >> + >> +test_expect_success 'blame -L ,Y (Y == nlines + 1)' ' >> + n=$(expr $(wc -l > + test_must_fail $PROG -L,$n file >> +' >> + > > This is somewhat curious. > > Does the problem triggers only on a file that ends with an > incomplete line, or the test was inserted at this location only > because it was convenient and the problem exists with or without the > incomplete final line? > > I am guessing that it is the latter. The problem exists with and without the incomplete line. The comment mentioning "incomplete line" and "wc" was inserted to explain why it was necessary to add one to the result of wc, which might not otherwise be obvious. The tests were inserted at this location because they are semantically related to the "blame -L ,Y (Y > nlines)" test which already exists in the file (quote just below this response). >> test_expect_success 'blame -L ,Y (Y > nlines)' ' >> test_must_fail $PROG -L,12345 file >> ' -- 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 02/11] t8001/t8002: blame: demonstrate -L bounds checking bug
On Mon, Aug 5, 2013 at 3:35 PM, Eric Sunshine wrote: > On Mon, Aug 5, 2013 at 3:29 PM, Junio C Hamano wrote: >> Eric Sunshine writes: >> >>> A bounds checking bug allows the X in -LX to extend one line past the >>> end of file. For example, given a file with 5 lines, -L6 is accepted as >>> valid. Demonstrate this problem. >>> >>> While here, also add tests to check that the remaining cases of X and Y >>> in -LX,Y are handled correctly at and in the vicinity of end-of-file. >>> >>> Signed-off-by: Eric Sunshine >>> --- >>> t/annotate-tests.sh | 22 ++ >>> 1 file changed, 22 insertions(+) >>> >>> diff --git a/t/annotate-tests.sh b/t/annotate-tests.sh >>> index 3524eaf..02fbbf1 100644 >>> --- a/t/annotate-tests.sh >>> +++ b/t/annotate-tests.sh >>> @@ -225,10 +225,32 @@ test_expect_success 'blame -L /RE/,-N' ' >>> check_count -L/99/,-3 B 1 B2 1 D 1 >>> ' >>> >>> +# 'file' ends with an incomplete line, so 'wc' reports one fewer lines than >>> +# git-blame sees, hence the last line is actually $(wc...)+1. >>> +test_expect_success 'blame -L X (X == nlines)' ' >>> + n=$(expr $(wc -l >> + check_count -L$n C 1 >> >> This is somewhat curious. >> >> Does the problem triggers only on a file that ends with an >> incomplete line, or the test was inserted at this location only >> because it was convenient and the problem exists with or without the >> incomplete final line? >> >> I am guessing that it is the latter. > > The problem exists with and without the incomplete line. The comment > mentioning "incomplete line" and "wc" was inserted to explain why it > was necessary to add one to the result of wc, which might not > otherwise be obvious. Would the comment be clearer if phrased like this? # We want to test -LX where X is the last line of the file, so we need # to compute the number of lines in the file, which normally would be # done via 'wc -l'. In this case, however, the last line of 'file' is # incomplete, so 'wc' reports one fewer than the actual line count. To # adjust for this anomaly, we must add one to the result of 'wc'. -- 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] remote-hg: add shared repo upgrade
On Mon, Aug 5, 2013 at 9:31 PM, Felipe Contreras wrote: > On Mon, Aug 5, 2013 at 2:22 PM, Antoine Pelisse wrote: >> From: Felipe Contreras >> >> 6796d49 (remote-hg: use a shared repository store) introduced a bug by >> making the shared repository '.git/hg', which is already used before >> that patch, so clones that happened before that patch, fail after that >> patch, because there's no shared Mercurial repo. >> >> It's trivial to upgrade to the new organization by copying the Mercurial >> repo from one of the remotes (e.g. 'origin'), so let's do so. > > In addition to that, simplify the shared repo initialization; if the > repository is shared, the pull on the child will use the parent's > storage, so there's no need for the initial clone. > > And make sure the shared repository is always present. It comes without saying that you can change this description if you want to :-) > It seems pretty clear to me that we are talking about multiple patches here. I'm not sure that's necessary. But I may be missing something. -- 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] remote-hg: fix path when cloning with tilde expansion
The current code fixes the path to make it absolute when cloning, but
doesn't consider tilde expansion, so that scenario fails throwing an
exception because /home/myuser/~/my/repository doesn't exists:
$ git clone hg::~/my/repository && cd repository && git fetch
Fix that by using python os.path.expanduser method.
Signed-off-by: Antoine Pelisse
---
contrib/remote-helpers/git-remote-hg |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/contrib/remote-helpers/git-remote-hg
b/contrib/remote-helpers/git-remote-hg
index 02404dc..4bbd296 100755
--- a/contrib/remote-helpers/git-remote-hg
+++ b/contrib/remote-helpers/git-remote-hg
@@ -1137,7 +1137,7 @@ def fix_path(alias, repo, orig_url):
url = urlparse.urlparse(orig_url, 'file')
if url.scheme != 'file' or os.path.isabs(url.path):
return
-abs_url = urlparse.urljoin("%s/" % os.getcwd(), orig_url)
+abs_url = os.path.abspath(os.path.expanduser(orig_url))
cmd = ['git', 'config', 'remote.%s.url' % alias, "hg::%s" % abs_url]
subprocess.call(cmd)
--
1.7.9.5
--
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 ALTERNATIVE v6.v3 4/6] config: parse http.. using urlmatch
Use the urlmatch_config_entry() to wrap the underlying http_options() two-level variable parser in order to set http. to the value with the most specific URL in the configuration. Signed-off-by: Jeff King Signed-off-by: Kyle J. McKay Signed-off-by: Junio C Hamano --- This version of 4/6 moves the tests to t0110 since urlmatch is now global. The config tests are removed since part 6/6 already has those and they no longer belong with the urlmatch normalization tests. The Makefile rule has been removed since it's no longer needed to build correctly as the test program no longer includes http.c. Other than those changes (and a minor rename to reflect the new location), this patch is identical to the previous v6.v2 4/6. .gitignore| 1 + Documentation/config.txt | 45 ++ Makefile | 3 + http.c| 13 ++- t/.gitattributes | 1 + t/t0110-urlmatch-normalization.sh | 177 ++ t/t0110/README| 9 ++ t/t0110/url-1 | Bin 0 -> 20 bytes t/t0110/url-10| Bin 0 -> 23 bytes t/t0110/url-11| Bin 0 -> 25 bytes t/t0110/url-2 | Bin 0 -> 20 bytes t/t0110/url-3 | Bin 0 -> 23 bytes t/t0110/url-4 | Bin 0 -> 23 bytes t/t0110/url-5 | Bin 0 -> 23 bytes t/t0110/url-6 | Bin 0 -> 23 bytes t/t0110/url-7 | Bin 0 -> 23 bytes t/t0110/url-8 | Bin 0 -> 23 bytes t/t0110/url-9 | Bin 0 -> 23 bytes test-urlmatch-normalization.c | 50 +++ 19 files changed, 298 insertions(+), 1 deletion(-) create mode 100755 t/t0110-urlmatch-normalization.sh create mode 100644 t/t0110/README create mode 100644 t/t0110/url-1 create mode 100644 t/t0110/url-10 create mode 100644 t/t0110/url-11 create mode 100644 t/t0110/url-2 create mode 100644 t/t0110/url-3 create mode 100644 t/t0110/url-4 create mode 100644 t/t0110/url-5 create mode 100644 t/t0110/url-6 create mode 100644 t/t0110/url-7 create mode 100644 t/t0110/url-8 create mode 100644 t/t0110/url-9 create mode 100644 test-urlmatch-normalization.c diff --git a/.gitignore b/.gitignore index 6669bf0c..b8524bfe 100644 --- a/.gitignore +++ b/.gitignore @@ -198,6 +198,7 @@ /test-string-list /test-subprocess /test-svn-fe +/test-urlmatch-normalization /test-wildmatch /common-cmds.h *.tar.gz diff --git a/Documentation/config.txt b/Documentation/config.txt index 6e53fc50..a81f3ab7 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -1513,6 +1513,51 @@ http.useragent:: of common USER_AGENT strings (but not including those like git/1.7.1). Can be overridden by the 'GIT_HTTP_USER_AGENT' environment variable. +http..*:: + Any of the http.* options above can be applied selectively to some urls. + For a config key to match a URL, each element of the config key is + compared to that of the URL, in the following order: ++ +-- +. Scheme (e.g., `https` in `https://example.com/`). This field + must match exactly between the config key and the URL. + +. Host/domain name (e.g., `example.com` in `https://example.com/`). + This field must match exactly between the config key and the URL. + +. Port number (e.g., `8080` in `http://example.com:8080/`). + This field must match exactly between the config key and the URL. + Omitted port numbers are automatically converted to the correct + default for the scheme before matching. + +. Path (e.g., `repo.git` in `https://example.com/repo.git`). The + path field of the config key must match the path field of the URL + either exactly or as a prefix of slash-delimited path elements. This means + a config key with path `foo/` matches URL path `foo/bar`. A prefix can only + match on a slash (`/`) boundary. Longer matches take precedence (so a config + key with path `foo/bar` is a better match to URL path `foo/bar` than a config + key with just path `foo/`). + +. User name (e.g., `user` in `https://u...@example.com/repo.git`). If + the config key has a user name it must match the user name in the + URL exactly. If the config key does not have a user name, that + config key will match a URL with any user name (including none), + but at a lower precedence than a config key with a user name. +-- ++ +The list above is ordered by decreasing precedence; a URL that matches +a config key's path is preferred to one that matches its user name. For example, +if the URL is `https://u...@example.com/foo/bar` a config key match of +`https://example.com/foo` will be preferred over a config key match of +`https://u...@example.com`. ++ +All URLs are normalized before attempting any matching (the password part, +if embedded in the URL, is always ignored for matching purposes) so that +equivalent urls that are sim
Re: [PATCH] remote-hg: fix path when cloning with tilde expansion
On Mon, Aug 5, 2013 at 3:12 PM, Antoine Pelisse wrote: > The current code fixes the path to make it absolute when cloning, but > doesn't consider tilde expansion, so that scenario fails throwing an > exception because /home/myuser/~/my/repository doesn't exists: > > $ git clone hg::~/my/repository && cd repository && git fetch > > Fix that by using python os.path.expanduser method. Shouldn't that be the job of the shell? (s/~/$HOME/) -- Felipe Contreras -- 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] remote-hg: fix path when cloning with tilde expansion
On Mon, Aug 5, 2013 at 10:30 PM, Felipe Contreras wrote: > On Mon, Aug 5, 2013 at 3:12 PM, Antoine Pelisse wrote: >> $ git clone hg::~/my/repository && cd repository && git fetch >> >> Fix that by using python os.path.expanduser method. > > Shouldn't that be the job of the shell? (s/~/$HOME/) I guess it is, as long as it looks like a path: $ echo ~ /home/myuser $ echo hg::~ hg::~ -- 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] remote-hg: add shared repo upgrade
Antoine Pelisse writes:
> From: Felipe Contreras
>
> 6796d49 (remote-hg: use a shared repository store) introduced a bug by
> making the shared repository '.git/hg', which is already used before
> that patch, so clones that happened before that patch, fail after that
> patch, because there's no shared Mercurial repo.
>
> It's trivial to upgrade to the new organization by copying the Mercurial
> repo from one of the remotes (e.g. 'origin'), so let's do so.
>
> ...
> +# check and upgrade old organization
> +hg_path = os.path.join(shared_path, '.hg')
> +if os.path.exists(shared_path) and not os.path.exists(hg_path):
> +repos = os.listdir(shared_path)
> +for x in repos:
> +local_hg = os.path.join(shared_path, x, 'clone', '.hg')
> +if not os.path.exists(local_hg):
> +continue
> +shutil.copytree(local_hg, hg_path)
The log message talks about "one of the remotes (e.g. 'origin')" and
you are creating a copy of one that you encounter in os.listdir(); I
may be missing some underlying assumptions but I wonder what happens
after you copy and create hg_path directory, which does not change
in the loop, to the remaining iterations of the loop. Is the untold
and obvious-to-those-who-are-familiar-with-this-codepath assumption
that it is guaranteed that there is at most one "*/clone/.hg" under
shared_path?
> +# setup shared repo (if not there)
> +try:
> +hg.peer(myui, {}, shared_path, create=True)
> +except error.RepoError:
> +pass
>
> if not os.path.exists(dirname):
> os.makedirs(dirname)
--
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 ALTERNATIVE v6.v3 4/6] config: parse http.. using urlmatch
"Kyle J. McKay" writes: > Use the urlmatch_config_entry() to wrap the underlying > http_options() two-level variable parser in order to set > http. to the value with the most specific URL in the > configuration. > > Signed-off-by: Jeff King > Signed-off-by: Kyle J. McKay > Signed-off-by: Junio C Hamano > --- Oops, what did we sign-off? > This version of 4/6 moves the tests to t0110 since urlmatch is now global. > The config tests are removed since part 6/6 already has those and they no > longer belong with the urlmatch normalization tests. > > The Makefile rule has been removed since it's no longer needed to build > correctly as the test program no longer includes http.c. > > Other than those changes (and a minor rename to reflect the new location), > this patch is identical to the previous v6.v2 4/6. Ahh, figures. Thanks. Peff, any comments? > diff --git a/t/t0110-urlmatch-normalization.sh > b/t/t0110-urlmatch-normalization.sh > new file mode 100755 > index ..8d6096d4 > --- /dev/null > +++ b/t/t0110-urlmatch-normalization.sh > @@ -0,0 +1,177 @@ > +#!/bin/sh > + > +test_description='urlmatch URL normalization' > +. ./test-lib.sh > + > +# The base name of the test url files > +tu="$TEST_DIRECTORY/t0110/url" > + > +# Note that only file: URLs should be allowed without a host It is somewhat unfortunate that the form most commonly used for pushing is not supported at all, i.e. host:path Current configuration set may not have anything interesting to affect the git-over-ssh push codepath, so in practice it may not matter, though. > +test_expect_success 'url authority' ' "authority" refers to the host part? (not a complaint, but is a question) > +test_expect_success 'url port checks' ' > + test-urlmatch-normalization "xyz://q...@some.host:" && This is presumably replaced by a default port for xyz:// scheme, whatever the default port is, in other words, it is as if no colon is given at the end? > + test-urlmatch-normalization "xyz://q...@some.host:456/" && > + ! test-urlmatch-normalization "xyz://q...@some.host:0" && > + ! test-urlmatch-normalization "xyz://q...@some.host:000" && Port #0 is disallowed? > + test-urlmatch-normalization "xyz://q...@some.host:001?" && Is it the same as specifying "xyz://q...@some.host:1?" and does it match "xyz://q...@some.host:1"? > + test-urlmatch-normalization "xyz://q...@some.host:065535#" && Ditto, for 65535 and without #-fragment at the end? > +test_expect_success 'url port normalization' ' > + test "$(test-urlmatch-normalization -p "http://x:800";)" = > "http://x:800/"; && > + test "$(test-urlmatch-normalization -p "http://x:0800";)" = > "http://x:800/"; && > + test "$(test-urlmatch-normalization -p "http://x:0800";)" = > "http://x:800/"; && > + test "$(test-urlmatch-normalization -p "http://x:065535";)" = > "http://x:65535/"; && > + test "$(test-urlmatch-normalization -p "http://x:1";)" = "http://x:1/"; && > + test "$(test-urlmatch-normalization -p "http://x:80";)" = "http://x/"; && > + test "$(test-urlmatch-normalization -p "http://x:080";)" = "http://x/"; && > + test "$(test-urlmatch-normalization -p "http://x:00080";)" = > "http://x/"; && > + test "$(test-urlmatch-normalization -p "https://x:443";)" = "https://x/"; > && > + test "$(test-urlmatch-normalization -p "https://x:0443";)" = > "https://x/"; && > + test "$(test-urlmatch-normalization -p "https://x:00443";)" = > "https://x/"; > +' OK, these answer most of the previous questions. > +# http://@foo specifies an empty user name but does not specify a password > +# http://foo specifies neither a user name nor a password > +# So they should not be equivalent > +test_expect_success 'url equivalents' ' > + test-urlmatch-normalization "httP://x" "Http://X/" && > + test-urlmatch-normalization "Http://%4d%65:%4d^%7...@the.host" > "hTTP://Me:%4D^p...@the.host:80/" && > + ! test-urlmatch-normalization "https://@x.y/^"; "httpS://x.y:443/^" && The comment is about this test, which seems to make sense. What is "^"? Just a random valid character that can appear in the path? (not a complaint, but is a question). -- 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
What's cooking in git.git (Aug 2013, #02; Mon, 5)
Here are the topics that have been cooking. Commits prefixed with '-' are only in 'pu' (proposed updates) while commits prefixed with '+' are in 'next'. The first release candidate v1.8.4-rc1 has been tagged; only regression fixes and l10n updates from now on. You can find the changes described here in the integration branches of the repositories listed at http://git-blame.blogspot.com/p/git-public-repositories.html -- [Graduated to "master"] * rj/cygwin-clarify-use-of-cheating-lstat (2013-07-18) 1 commit (merged to 'next' on 2013-08-01 at 3ebfe7c) + cygwin: Remove the Win32 l/stat() implementation Cygwin port added a "not quite correct but a lot faster and good enough for many lstat() calls that are only used to see if the working tree entity matches the index entry" lstat() emulation some time ago, and it started biting us in places. This removes it and uses the standard lstat() that comes with Cygwin. Recent topic that uses lstat on packed-refs file is broken when this cheating lstat is used, and this is a simplest fix that is also the cleanest direction to go in the long run. -- [New Topics] * es/blame-L-more (2013-08-05) 11 commits - blame: reject empty ranges -L,+0 and -L,-0 - t8001/t8002: blame: demonstrate acceptance of bogus -L,+0 and -L,-0 - blame: reject empty ranges -LX,+0 and -LX,-0 - t8001/t8002: blame: demonstrate acceptance of bogus -LX,+0 and -LX,-0 - log: fix -L bounds checking bug - t4211: retire soon-to-be unimplementable tests - t4211: log: demonstrate -L bounds checking bug - blame: fix -L bounds checking bug - t8001/t8002: blame: add empty file & partial-line tests - t8001/t8002: blame: demonstrate -L bounds checking bug - t8001/t8002: blame: decompose overly-large test More fixes to the code to parse the "-L" option in "log" and "blame". Will merge to and cook in 'next'. * jk/cat-file-batch-optim (2013-08-05) 1 commit - cat-file: only split on whitespace when %(rest) is used Rework the reverted change to `cat-file --batch-check`. Will merge to and cook in 'next'. * jn/post-receive-utf8 (2013-08-05) 3 commits - hooks/post-receive-email: set declared encoding to utf-8 - hooks/post-receive-email: force log messages in UTF-8 - hooks/post-receive-email: use plumbing instead of git log/show Update post-receive-email script to make sure the message contents and pathnames are encoded consistently in UTF-8. I have a feeling that it is a lost cause to solve the issue the topic tries to address in general, because the patch text can have payload in any encodings that are different from either the pathnames or the log message. Patches that touch paths that use an encoding that conflicts with the encoding of the payload and/or the log message could be transferred with core.quotepath set and patch generated as all binary, but that would be pretty much useless. * sb/parseopt-boolean-removal (2013-08-05) 9 commits - revert: use the OPT_CMDMODE for parsing, reducing code - checkout-index: Fix negations of even numbers of -n - config parsing options: allow one flag multiple times - hash-object: Replace stdin parsing OPT_BOOLEAN by OPT_COUNTUP - branch, commit, name-rev: ease up boolean conditions - checkout: remove superfluous local variable - log, format-patch: parsing uses OPT__QUIET - Replace deprecated OPT_BOOLEAN by OPT_BOOL - Remove deprecated OPTION_BOOLEAN for parsing arguments (this branch uses jc/parseopt-command-modes.) Convert most uses of OPT_BOOLEAN/OPTION_BOOLEAN that can use OPT_BOOL/OPTION_BOOLEAN which have much saner semantics, and turn remaining ones into OPT_SET_INT, OPT_COUNTUP, etc. as necessary; there seems to be some misconversion that makes many tests fail, though. -- [Stalled] * tf/gitweb-ss-tweak (2013-07-15) 4 commits - gitweb: make search help link less ugly - gitweb: omit the repository owner when it is unset - gitweb: vertically centre contents of page footer - gitweb: ensure OPML text fits inside its box Comments? * rj/read-default-config-in-show-ref-pack-refs (2013-06-17) 3 commits - ### DONTMERGE: needs better explanation on what config they need - pack-refs.c: Add missing call to git_config() - show-ref.c: Add missing call to git_config() The changes themselves are probably good, but it is unclear what basic setting needs to be read for which exact operation. Waiting for clarification. $gmane/228294 * jh/shorten-refname (2013-05-07) 4 commits - t1514: refname shortening is done after dereferencing symbolic refs - shorten_unambiguous_ref(): Fix shortening refs/remotes/origin/HEAD to origin - t1514: Demonstrate failure to correctly shorten "refs/remotes/origin/HEAD" - t1514: Add tests of shortening refnames in strict/loose mode When remotes/origin/HEAD is not a symbolic ref, "rev-parse --abbrev-ref remotes/origin/HEA
Re: [PATCH 02/11] t8001/t8002: blame: demonstrate -L bounds checking bug
Eric Sunshine writes: >> The problem exists with and without the incomplete line. The comment >> mentioning "incomplete line" and "wc" was inserted to explain why it >> was necessary to add one to the result of wc, which might not >> otherwise be obvious. > > Would the comment be clearer if phrased like this? > > # We want to test -LX where X is the last line of the file, so we need > # to compute the number of lines in the file, which normally would be > # done via 'wc -l'. In this case, however, the last line of 'file' is > # incomplete, so 'wc' reports one fewer than the actual line count. To > # adjust for this anomaly, we must add one to the result of 'wc'. If the patch picked a place where the test vector does not have to involve a file that ends with an incomplete line, the test would not have had to do anything tricky that required such a comment to explain why it is doing something mysterious. Such a change would be much cleaner and may be worth the effort, but I do not think just rewording the comment is worth the bother. I didn't see if there is such a place in the existing test sequence, though. -- 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 ALTERNATIVE v6.v3 4/6] config: parse http.. using urlmatch
On Aug 5, 2013, at 15:56, Junio C Hamano wrote: "Kyle J. McKay" writes: Use the urlmatch_config_entry() to wrap the underlying http_options() two-level variable parser in order to set http. to the value with the most specific URL in the configuration. Signed-off-by: Jeff King Signed-off-by: Kyle J. McKay Signed-off-by: Junio C Hamano --- Oops, what did we sign-off? Some code removal. No new additions. Actually this: On Aug 1, 2013, at 14:44, Junio C Hamano wrote: * jc/url-match (2013-07-31) 6 commits - config: "git config --get-urlmatch" parses section..key - builtin/config: refactor collect_config() - config: parse http.. using urlmatch - config: add generic callback wrapper to parse section..key - config: add helper to normalize and match URLs - http.c: fix parsing of http.sslCertPasswordProtected variable Reroll of km/http-curl-config-per-url topic. Peff raises many good points about the tests for http.* variables. Waiting for the discussion to conclude, hopefully with a replacement test. As requested. This version of 4/6 moves the tests to t0110 since urlmatch is now global. The config tests are removed since part 6/6 already has those and they no longer belong with the urlmatch normalization tests. The Makefile rule has been removed since it's no longer needed to build correctly as the test program no longer includes http.c. Other than those changes (and a minor rename to reflect the new location), this patch is identical to the previous v6.v2 4/6. Ahh, figures. Thanks. The remaining tests, by the way, have not changed. They are identical to previous versions. Peff, any comments? diff --git a/t/t0110-urlmatch-normalization.sh b/t/t0110-urlmatch- normalization.sh new file mode 100755 index ..8d6096d4 --- /dev/null +++ b/t/t0110-urlmatch-normalization.sh @@ -0,0 +1,177 @@ +#!/bin/sh + +test_description='urlmatch URL normalization' +. ./test-lib.sh + +# The base name of the test url files +tu="$TEST_DIRECTORY/t0110/url" + +# Note that only file: URLs should be allowed without a host It is somewhat unfortunate that the form most commonly used for pushing is not supported at all, i.e. host:path That is an SSH extension and they are certainly not URLs according to RFC 3986 because that would require every host to be its own scheme. Also, host:path cannot in the general case, be unambiguously translated to a URL. For example, repo.or.cz:srv/git/alt-git, has no translation. It is different from repo.or.cz:/srv/git/alt-git which does have a translation. There's no guarantee that inserting a '/' will not change the meaning of the URL (that only happens to be the case on repo.or.cz because all the ssh git users in the chroot jail have a '/' home directory). Current configuration set may not have anything interesting to affect the git-over-ssh push codepath, so in practice it may not matter, though. +test_expect_success 'url authority' ' "authority" refers to the host part? (not a complaint, but is a question) It refers to this production from RFC 3986 Section "3.2 Authority": authority = [ userinfo "@" ] host [ ":" port ] +test_expect_success 'url port checks' ' + test-urlmatch-normalization "xyz://q...@some.host:" && This is presumably replaced by a default port for xyz:// scheme, whatever the default port is, in other words, it is as if no colon is given at the end? Yes. The "port" production above is: port = *DIGIT which means 0 or more digits. + test-urlmatch-normalization "xyz://q...@some.host:456/" && + ! test-urlmatch-normalization "xyz://q...@some.host:0" && + ! test-urlmatch-normalization "xyz://q...@some.host:000" && Port #0 is disallowed? Intentionally so. The comments from urlmatch.c talk about this: /* * Port number must be all digits with leading 0s removed * and since all the protocols we deal with have a 16-bit * port number it must also be in the range 1..65535 * 0 is not allowed because that means "next available" * on just about every system and therefore cannot be used */ + test-urlmatch-normalization "xyz://q...@some.host:001?" && Is it the same as specifying "xyz://q...@some.host:1?" and does it match "xyz://q...@some.host:1"? + test-urlmatch-normalization "xyz://q...@some.host:065535#" && Ditto, for 65535 and without #-fragment at the end? +test_expect_success 'url port normalization' ' + test "$(test-urlmatch-normalization -p "http://x:800";)" = "http:// x:800/" && + test "$(test-urlmatch-normalization -p "http://x:0800";)" = "http://x:800/"; && + test "$(test-urlmatch-normalization -p "http://x:0800";)" = "http://x:800/"; && + test "$(test-urlmatch-normalization -p "http://x:065535";)" = "http://x:65535/"; && + test "$(test-urlmatch-normalization -p "http://x:1";)" = "http://x: 1/" && + test "$(test-urlmatch-normalization -p "http://x:80";)" = "http:// x/" && + test "$(test-urlmatch-normalization
Re: [PATCH 3/3] t5551: Remove header from curl cookie file
On Mon, Aug 5, 2013 at 8:59 AM, Brian Gernhardt wrote: > > The URL included in the header appears to vary from curl version to > curl version. Since we only care about the final few lines, only test > them. However, make sure the blank line after the header is still > included to make sure there are no extra cookie lines. > > Signed-off-by: Brian Gernhardt > --- > > I suppose a sed invocation to strip out the URL or comments might be better, > but this seemed simpler. > > t/t5551-http-fetch.sh | 6 ++ > 1 file changed, 2 insertions(+), 4 deletions(-) > > diff --git a/t/t5551-http-fetch.sh b/t/t5551-http-fetch.sh > index 287d22b..8196af1 100755 > --- a/t/t5551-http-fetch.sh > +++ b/t/t5551-http-fetch.sh > @@ -191,9 +191,6 @@ cat >cookies.txt < 127.0.0.1 FALSE /smart_cookies/ FALSE 0 othername > othervalue > EOF > cat >expect_cookies.txt < -# Netscape HTTP Cookie File > -# http://curl.haxx.se/docs/http-cookies.html > -# This file was generated by libcurl! Edit at your own risk. > > 127.0.0.1 FALSE /smart_cookies/ FALSE 0 othername > othervalue > 127.0.0.1 FALSE /smart_cookies/repo.git/info/ FALSE 0 name > value > @@ -202,7 +199,8 @@ test_expect_success 'cookies stored in http.cookiefile > when http.savecookies set > git config http.cookiefile cookies.txt && > git config http.savecookies true && > git ls-remote $HTTPD_URL/smart_cookies/repo.git master && > - test_cmp expect_cookies.txt cookies.txt > + tail -3 cookies.txt > cookies_tail.txt Would it make more sense to ignore comments entirely? I.e. instead of taking the tail, pipe through sed 's/#.*//'. Thanks for catching this by the way; you can probably guess that I only checked with a single curl version. > > + test_cmp expect_cookies.txt cookies_tail.txt > ' > > test -n "$GIT_TEST_LONG" && test_set_prereq EXPENSIVE > -- > 1.8.4.rc1.384.g0976a17.dirty > -- 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: [BUG?] gc and impatience
Duy Nguyen wrote: > Good point. I think that is because gc does not check if gc is already > running. Adding such a check should not be too hard. I think gc could > save its pid in $GIT_DIR/auto-gc.pid. The next auto-gc checks this, if > the pid is valid, skip auto-gc. Check. I also talked about gc not catching SIGINT properly: I'm looking the issue. > Or you could just make a cron job to gc all repos every week and the > problem goes away ;-) Fundamentally, we need to fix these problems: 1. Don't make the repo unusable when a gc is running: I don't expect anything more than minor annoyances after your patch is checked in. 2. Improve the IO profile, so gc doesn't aggressively hammer out tiny fragmentations. For this, git-exproll.sh is definitely a step in the right direction. 3. Improve how gc fundamentally works, so we can minimize rebuilds and CPU time. jc's git merge-pack is interesting, but I'm not very hopeful about a naive incremental-packing. We need to keep the major undeltified objects near the top of the file, and build an idx sorted by SHA-1; mangling the offsets in the header after a packfile has been written is both complicated and dangerous (we might introduce subtle bugs corrupting the packfile), I think. I haven't thought about it hard enough though. We'll tackle these problems bit by bit in future patches. 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] git exproll: steps to tackle gc aggression
This is the rough explanation I wrote down after reading it:
So, the problem is that my .git/objects/pack is polluted with little
packs everytime I fetch (or push, if you're the server), and this is
problematic from the perspective of a overtly (naively) aggressive gc
that hammers out all fragmentation. So, on the first run, the little
packfiles I have are all "consolidated" into big packfiles; you also
write .keep files to say that "don't gc these big packs we just
generated". In subsequent runs, the little packfiles from the fetch are
absorbed into a pack that is immune to gc. You're also using a size
heuristic, to consolidate similarly sized packfiles. You also have a
--ratio to tweak the ratio of sizes.
From: Martin Fick
See: https://gerrit-review.googlesource.com/#/c/35215/
Thread: http://thread.gmane.org/gmane.comp.version-control.git/231555
(Martin's emails are missing from the archive)
---
Not a candidate for inclusion, given how difficult it is to convince
our conservative maintainer to add anything new to the tree.
I'm posting this in the interest of visibility: I've checked it into
my repo and started using it. I encourage everyone else to do the
same, so we can learn from it and discuss how to improve gc.
contrib/exproll/git-exproll.sh | 566 +
1 file changed, 566 insertions(+)
create mode 100755 contrib/exproll/git-exproll.sh
diff --git a/contrib/exproll/git-exproll.sh b/contrib/exproll/git-exproll.sh
new file mode 100755
index 000..9526d9f
--- /dev/null
+++ b/contrib/exproll/git-exproll.sh
@@ -0,0 +1,566 @@
+#!/bin/bash
+# Copyright (c) 2012, Code Aurora Forum. All rights reserved.
+#
+# Redistribution and use in source and binary forms, with or without
+# modification, are permitted provided that the following conditions are
+# met:
+## Redistributions of source code must retain the above copyright
+# notice, this list of conditions and the following disclaimer.
+## Redistributions in binary form must reproduce the above
+# copyright notice, this list of conditions and the following
+# disclaimer in the documentation and/or other materials provided
+# with the distribution.
+## Neither the name of Code Aurora Forum, Inc. nor the names of its
+# contributors may be used to endorse or promote products derived
+# from this software without specific prior written permission.
+#
+# THIS SOFTWARE IS PROVIDED "AS IS" AND ANY EXPRESS OR IMPLIED
+# WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF
+# MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT
+# ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS
+# BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+# CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+# SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR
+# BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY,
+# WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE
+# OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN
+# IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+usage() { # error_message
+
+cat <<-EOF
+ usage: $(basename $0) [-unvt] [--noref] [--nolosse] [-r|--ratio
number]
+ [git gc option...] git.repo
+
+ -u|-husage/help
+ -v verbose
+ -n dry-run don't actually repack anything
+ -t touch treat repo as if it had been touched
+ --noref avoid extra ref packing timestamp checking
+ --noloosedo not run just because there are loose
object dirs
+(repacking may still run if they are
referenced)
+ -r ratio packfile ratio to aim for (default 10)
+
+ git gc optionwill be passed as args to git gc
+
+ git.repo to run gc against
+
+ Garbage collect using a pseudo logarithmic packfile maintenance
+ approach. This approach attempts to minimize packfile churn
+ by keeping several generations of varying sized packfiles around
+ and only consolidating packfiles (or loose objects) which are
+ either new packfiles, or packfiles close to the same size as
+ another packfile.
+
+ An estimate is used to predict when rollups (one consolidation
+ would cause another consolidation) would occur so that this
+ rollup can be done all at once via a single repack. This
reduces
+ both the runtime and the pack file churn in rollup cases.
+
+ Approach: plan each consolidation by creating a table like this:
+
+ Id Keep Size Sha1(or consolidation list)
Actions(repack down
Re: [BUG?] gc and impatience
Martin Fick wrote: > I hope > that someone more familiar with git gc than me might take > this on some day. :) More likely scenario: someone who is unfamiliar with it will read and patch it little by little :) -- 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: [[TIG][PATCH] 1/3] Add log_select function to find commit from context in log view
On Fri, Aug 2, 2013 at 8:23 PM, Kumar Appaiah wrote:
> This commit introduces and uses the log_select function to find the
> correct commit in the unsplit log view. In the log view, if one
> scrolls down across a commit line, the current commit (as displayed in
> the status bar) gets updated, but not so when scrolling upward across
> a commit. The log_select function handles this scenario to to the
s/to to/to do/
> ``right thing''. In addition, it introduces the log_state structure as
> the private entry of the log view to hold a flag that decides whether
> to re-evaluate the current commit based on scrolling.
>
> Signed-off-by: Kumar Appaiah
> ---
> tig.c | 50 --
> 1 file changed, 48 insertions(+), 2 deletions(-)
>
> diff --git a/tig.c b/tig.c
> index 72f132a..dd4b0f4 100644
> --- a/tig.c
> +++ b/tig.c
> @@ -4384,6 +4384,33 @@ pager_select(struct view *view, struct line *line)
> }
> }
>
> +struct log_state {
> + bool update_commit_ref;
> +};
> +
> +static void
> +log_select(struct view *view, struct line *line)
> +{
> + struct log_state *state = (struct log_state *) view->private;
> +
> + if (state->update_commit_ref && line->lineno > 1) {
> + /* We need to recalculate the previous commit,
> + since the user has likely scrolled up. */
I'd prefer that state->update_commit_ref is given another name so it
won't be necessary to have these comments everywhere the field is
used, for example recalculate_commit_context. The comment could be
moved to the declaration in struct log_state to explain its use.
Multi-line comments should start with '*' for each additonal line, e.g.
/* bla bla
* bla bla */
> + const struct line *commit_line = find_prev_line_by_type(view,
> line, LINE_COMMIT);
> +
> + if (commit_line)
> + string_copy_rev(view->ref, (char *)
> (commit_line->data + STRING_SIZE("commit ")));
You mentioned elsewhere that this looked funny, and I guess you are
right. I will extract this into a utility method so you can simply
call: string_copy_rev_from_line(view->ref, commit_line); ...
> + }
> + if (line->type == LINE_COMMIT) {
> + char *text = (char *)line->data + STRING_SIZE("commit ");
> +
> + if (!view_has_flags(view, VIEW_NO_REF))
> + string_copy_rev(view->ref, text);
... and: string_copy_rev_from_line(view->ref, line);
> + }
> + string_copy_rev(ref_commit, view->ref);
> + state->update_commit_ref = FALSE;
> +}
> +
> static bool
> pager_open(struct view *view, enum open_flags flags)
> {
> @@ -4427,11 +4454,30 @@ log_open(struct view *view, enum open_flags flags)
> static enum request
> log_request(struct view *view, enum request request, struct line *line)
> {
> + struct log_state *state = (struct log_state *) view->private;
There's no need to cast view->private here.
> +
> switch (request) {
> case REQ_REFRESH:
> load_refs();
> refresh_view(view);
> return REQ_NONE;
> +
> + case REQ_MOVE_UP:
> + case REQ_PREVIOUS:
> + if (line->type == LINE_COMMIT && line->lineno > 1) {
> + /* We are at a commit, and heading upward. We
> + force log_select to find the previous
> + commit above, from the context. */
Please delete this comment.
> + state->update_commit_ref = TRUE;
> + }
> + return pager_request(view, request, line);
There's not really any reason to call pager_request here, since it
only handles REQ_ENTER.
> +
> + case REQ_MOVE_PAGE_UP:
> + case REQ_MOVE_PAGE_DOWN:
> + /* We need to figure out the right commit again. */
Please delete this this comment.
> + state->update_commit_ref = TRUE;
> + return pager_request(view, request, line);
Calling pager_request again.
> +
> default:
> return pager_request(view, request, line);
> }
> @@ -4441,13 +4487,13 @@ static struct view_ops log_ops = {
> "line",
> { "log" },
> VIEW_ADD_PAGER_REFS | VIEW_OPEN_DIFF | VIEW_SEND_CHILD_ENTER |
> VIEW_NO_PARENT_NAV,
> - 0,
> + sizeof(struct log_state),
> log_open,
> pager_read,
> pager_draw,
> log_request,
> pager_grep,
> - pager_select,
> + log_select,
> };
>
> struct diff_state {
> --
> 1.8.3.2
>
--
Jonas Fonseca
--
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: [[TIG][PATCH] 2/3] Display correct diff the context in split log view
On Fri, Aug 2, 2013 at 8:23 PM, Kumar Appaiah wrote: > In the log view, when scrolling across a commit, the diff view should > automatically switch to the commit whose context the cursor is on in > the log view. This commit changes things to catch the REQ_ENTER in the > log view and handle recalculation of the commit and diff display from > log_request, rather than delegating it to pager_request. In addition, > it also gets rid of unexpected upward scrolling of the log view. > > Fixes GH #155 > > Signed-Off-By: Kumar Appaiah > --- > NEWS | 1 + > tig.c | 12 > 2 files changed, 13 insertions(+) > > diff --git a/NEWS b/NEWS > index 0394407..f59e517 100644 > --- a/NEWS > +++ b/NEWS > @@ -46,6 +46,7 @@ Bug fixes: > - Fix rendering glitch for branch names. > - Do not apply diff styling to untracked files in the stage view. (GH #153) > - Fix tree indentation for entries containing combining characters. (GH > #170) > + - Introduce a more natural context-sensitive log display. (GH #155) > > tig-1.1 > --- > diff --git a/tig.c b/tig.c > index dd4b0f4..53947b7 100644 > --- a/tig.c > +++ b/tig.c > @@ -4478,6 +4478,18 @@ log_request(struct view *view, enum request request, > struct line *line) > state->update_commit_ref = TRUE; > return pager_request(view, request, line); > > + case REQ_ENTER: > + /* Recalculate the correct commit for the context. */ See my dislike for this type of comments. ;) > + state->update_commit_ref = TRUE; > + > + open_view(view, REQ_VIEW_DIFF, OPEN_SPLIT); This is called every time the user presses up/down. There should be a check that compares the VIEW(REQ_VIEW_DIFF)->ref to ref_commit. > + update_view_title(view); This can be deleted. pager_request require this hack due to the automatic scrolling (if I recall correctly). > + > + /* We don't want to delegate this to pager_request, > + since we don't want the extra scrolling of the log > + view. */ This explanation should IMO go into the commit message and not a comment since it is somewhat confusing unless you are familiar with the previous behaviour. > + return REQ_NONE; > + > default: > return pager_request(view, request, line); This line can be changed to `return request;` > } > -- > 1.8.3.2 > -- Jonas Fonseca -- 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: [[TIG][PATCH] 1/3] Add log_select function to find commit from context in log view
On Fri, Aug 2, 2013 at 8:23 PM, Kumar Appaiah wrote:
> diff --git a/tig.c b/tig.c
> index 72f132a..dd4b0f4 100644
> --- a/tig.c
> +++ b/tig.c
> @@ -4427,11 +4454,30 @@ log_open(struct view *view, enum open_flags flags)
> static enum request
> log_request(struct view *view, enum request request, struct line *line)
> {
> + struct log_state *state = (struct log_state *) view->private;
> +
> switch (request) {
> case REQ_REFRESH:
> load_refs();
> refresh_view(view);
> return REQ_NONE;
> +
> + case REQ_MOVE_UP:
> + case REQ_PREVIOUS:
> + if (line->type == LINE_COMMIT && line->lineno > 1) {
> + /* We are at a commit, and heading upward. We
> + force log_select to find the previous
> + commit above, from the context. */
> + state->update_commit_ref = TRUE;
> + }
> + return pager_request(view, request, line);
> +
> + case REQ_MOVE_PAGE_UP:
> + case REQ_MOVE_PAGE_DOWN:
> + /* We need to figure out the right commit again. */
> + state->update_commit_ref = TRUE;
> + return pager_request(view, request, line);
> +
> default:
> return pager_request(view, request, line);
> }
I forgot to mention there is one use case this doesn't currently
handle, namely jumping to a specific line using ':'. Other
than detecting this by tracking the current line number in log_state I
haven't come up with a good way to detect that a recalculation is
required.
--
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: [TIG][PATCH 0/3] Refactoring of the log view
On Fri, Aug 2, 2013 at 8:23 PM, Kumar Appaiah wrote: > These set of patches refactor the log view to provide a behaviour that > is quite similar to, say, e-mail with Mutt. The key improvements are: > > - The current commit is inferred based on the context. For example, if > you focus on the commit message of a particular commit, the correct > commit is inferred automagically. > > - Scrolling the log view when the diff is open shows the correct > commit on the screen, rather than have to scroll up and cross the > commit line to display the screen. Thanks, great improvements. I am still considering whether to queue them until after the next release or include them. > I have decided to revert 888611dd5d407775245d574a3dc5c01b5963a5ba, > since the behaviour with the updated scrolling pattern is much more > consistent. OK, makes sense. The next step will be to find out how to highlight the diff stat in the log view. :-D -- 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: [[TIG][PATCH] 1/3] Add log_select function to find commit from context in log view
Dear Jonas,
Thanks for the patient review.
On Mon, Aug 05, 2013 at 11:27:44PM -0400, Jonas Fonseca wrote:
> On Fri, Aug 2, 2013 at 8:23 PM, Kumar Appaiah
> wrote:
> > This commit introduces and uses the log_select function to find the
> > correct commit in the unsplit log view. In the log view, if one
> > scrolls down across a commit line, the current commit (as displayed in
> > the status bar) gets updated, but not so when scrolling upward across
> > a commit. The log_select function handles this scenario to to the
>
> s/to to/to do/
Done.
> > ``right thing''. In addition, it introduces the log_state structure as
> > the private entry of the log view to hold a flag that decides whether
> > to re-evaluate the current commit based on scrolling.
> >
> > Signed-off-by: Kumar Appaiah
> > ---
> > tig.c | 50 --
> > 1 file changed, 48 insertions(+), 2 deletions(-)
> >
> > diff --git a/tig.c b/tig.c
> > index 72f132a..dd4b0f4 100644
> > --- a/tig.c
> > +++ b/tig.c
> > @@ -4384,6 +4384,33 @@ pager_select(struct view *view, struct line *line)
> > }
> > }
> >
> > +struct log_state {
> > + bool update_commit_ref;
> > +};
> > +
> > +static void
> > +log_select(struct view *view, struct line *line)
> > +{
> > + struct log_state *state = (struct log_state *) view->private;
> > +
> > + if (state->update_commit_ref && line->lineno > 1) {
> > + /* We need to recalculate the previous commit,
> > + since the user has likely scrolled up. */
>
> I'd prefer that state->update_commit_ref is given another name so it
> won't be necessary to have these comments everywhere the field is
> used, for example recalculate_commit_context. The comment could be
> moved to the declaration in struct log_state to explain its use.
Done.
> Multi-line comments should start with '*' for each additonal line, e.g.
>
> /* bla bla
>* bla bla */
Done.
> > + const struct line *commit_line =
> > find_prev_line_by_type(view, line, LINE_COMMIT);
> > +
> > + if (commit_line)
> > + string_copy_rev(view->ref, (char *)
> > (commit_line->data + STRING_SIZE("commit ")));
>
> You mentioned elsewhere that this looked funny, and I guess you are
> right. I will extract this into a utility method so you can simply
> call: string_copy_rev_from_line(view->ref, commit_line); ...
I will wait on this. The next iteration of the patch will still have
this problem.
> > + }
> > + if (line->type == LINE_COMMIT) {
> > + char *text = (char *)line->data + STRING_SIZE("commit ");
> > +
> > + if (!view_has_flags(view, VIEW_NO_REF))
> > + string_copy_rev(view->ref, text);
>
> ... and: string_copy_rev_from_line(view->ref, line);
I understand.
> > + }
> > + string_copy_rev(ref_commit, view->ref);
> > + state->update_commit_ref = FALSE;
> > +}
> > +
> > static bool
> > pager_open(struct view *view, enum open_flags flags)
> > {
> > @@ -4427,11 +4454,30 @@ log_open(struct view *view, enum open_flags flags)
> > static enum request
> > log_request(struct view *view, enum request request, struct line *line)
> > {
> > + struct log_state *state = (struct log_state *) view->private;
>
> There's no need to cast view->private here.
Done.
> > +
> > switch (request) {
> > case REQ_REFRESH:
> > load_refs();
> > refresh_view(view);
> > return REQ_NONE;
> > +
> > + case REQ_MOVE_UP:
> > + case REQ_PREVIOUS:
> > + if (line->type == LINE_COMMIT && line->lineno > 1) {
> > + /* We are at a commit, and heading upward. We
> > + force log_select to find the previous
> > + commit above, from the context. */
>
> Please delete this comment.
Done.
> > + state->update_commit_ref = TRUE;
> > + }
> > + return pager_request(view, request, line);
>
> There's not really any reason to call pager_request here, since it
> only handles REQ_ENTER.
Done.
> > +
> > + case REQ_MOVE_PAGE_UP:
> > + case REQ_MOVE_PAGE_DOWN:
> > + /* We need to figure out the right commit again. */
>
> Please delete this this comment.
Done.
> > + state->update_commit_ref = TRUE;
> > + return pager_request(view, request, line);
>
> Calling pager_request again.
Done.
I will send in another patch to review shortly.
Thanks!
Kumar
--
Kumar Appaiah
--
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
[[TIG][PATCH v2] 0/3] Refactoring the log view
This is a second iteration. This handles the following: - remove unneeded comments - remove unneeded pager_request calls - rename update_commit_ref to recalculate_commit_context Now, the only thing missing is the recalculation of commits when the line number is changed. The trouble there is that using the : approach doesn't call log_request, so we need to come up with a smarter way to communicate the line number change, I guess. Thanks for all the feedback! Kumar Appaiah (3): Add log_select function to find commit from context in log view Display correct diff the context in split log view Revert "Scroll diff with arrow keys in log view" NEWS | 1 + tig.c | 64 +--- 2 files changed, 58 insertions(+), 7 deletions(-) -- 1.8.3.2 -- 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
[[TIG][PATCH v2] 1/3] Add log_select function to find commit from context in log view
This commit introduces and uses the log_select function to find the
correct commit in the unsplit log view. In the log view, if one
scrolls down across a commit line, the current commit (as displayed in
the status bar) gets updated, but not so when scrolling upward across
a commit. The log_select function handles this scenario to do the
``right thing''. In addition, it introduces the log_state structure as
the private entry of the log view to hold a flag that decides whether
to re-evaluate the current commit based on scrolling.
Signed-off-by: Kumar Appaiah
---
tig.c | 50 +++---
1 file changed, 47 insertions(+), 3 deletions(-)
diff --git a/tig.c b/tig.c
index 4759f1d..845153f 100644
--- a/tig.c
+++ b/tig.c
@@ -4383,6 +4383,35 @@ pager_select(struct view *view, struct line *line)
}
}
+struct log_state {
+ /* We need to recalculate the previous commit, when the user
+* scrolls up or uses the page up/down in the log
+* view. recalculate_commit_context is used as a flag to
+* indicate this. */
+ bool recalculate_commit_context;
+};
+
+static void
+log_select(struct view *view, struct line *line)
+{
+ struct log_state *state = view->private;
+
+ if (state->recalculate_commit_context && line->lineno > 1) {
+ const struct line *commit_line = find_prev_line_by_type(view,
line, LINE_COMMIT);
+
+ if (commit_line)
+ string_copy_rev(view->ref, (char *) (commit_line->data
+ STRING_SIZE("commit ")));
+ }
+ if (line->type == LINE_COMMIT) {
+ char *text = (char *)line->data + STRING_SIZE("commit ");
+
+ if (!view_has_flags(view, VIEW_NO_REF))
+ string_copy_rev(view->ref, text);
+ }
+ string_copy_rev(ref_commit, view->ref);
+ state->recalculate_commit_context = FALSE;
+}
+
static bool
pager_open(struct view *view, enum open_flags flags)
{
@@ -4426,11 +4455,26 @@ log_open(struct view *view, enum open_flags flags)
static enum request
log_request(struct view *view, enum request request, struct line *line)
{
+ struct log_state *state = (struct log_state *) view->private;
+
switch (request) {
case REQ_REFRESH:
load_refs();
refresh_view(view);
- return REQ_NONE;
+ return request;
+
+ case REQ_MOVE_UP:
+ case REQ_PREVIOUS:
+ if (line->type == LINE_COMMIT && line->lineno > 1) {
+ state->recalculate_commit_context = TRUE;
+ }
+ return request;
+
+ case REQ_MOVE_PAGE_UP:
+ case REQ_MOVE_PAGE_DOWN:
+ state->recalculate_commit_context = TRUE;
+ return request;
+
default:
return pager_request(view, request, line);
}
@@ -4440,13 +4484,13 @@ static struct view_ops log_ops = {
"line",
{ "log" },
VIEW_ADD_PAGER_REFS | VIEW_OPEN_DIFF | VIEW_SEND_CHILD_ENTER |
VIEW_NO_PARENT_NAV,
- 0,
+ sizeof(struct log_state),
log_open,
pager_read,
pager_draw,
log_request,
pager_grep,
- pager_select,
+ log_select,
};
struct diff_state {
--
1.8.3.2
--
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
[[TIG][PATCH v2] 2/3] Display correct diff the context in split log view
In the log view, when scrolling across a commit, the diff view should automatically switch to the commit whose context the cursor is on in the log view. This commit changes things to catch the REQ_ENTER in the log view and handle recalculation of the commit and diff display from log_request, rather than delegating it to pager_request. In addition, it also gets rid of unexpected upward scrolling of the log view. Fixes GH #155 Signed-Off-By: Kumar Appaiah --- NEWS | 1 + tig.c | 9 - 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/NEWS b/NEWS index 076ac9d..1b0f737 100644 --- a/NEWS +++ b/NEWS @@ -49,6 +49,7 @@ Bug fixes: - Ignore unrepresentable characters when transliterating text for rendering. - Transliterate text to output encoding before trimming it to avoid misalignment. (GH #86) + - Introduce a more natural context-sensitive log display. (GH #155) tig-1.1 --- diff --git a/tig.c b/tig.c index 845153f..256b589 100644 --- a/tig.c +++ b/tig.c @@ -4475,8 +4475,15 @@ log_request(struct view *view, enum request request, struct line *line) state->recalculate_commit_context = TRUE; return request; + case REQ_ENTER: + state->recalculate_commit_context = TRUE; + if (VIEW(REQ_VIEW_DIFF)->ref != ref_commit) + open_view(view, REQ_VIEW_DIFF, OPEN_SPLIT); + update_view_title(view); + return request; + default: - return pager_request(view, request, line); + return request; } } -- 1.8.3.2 -- 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
[[TIG][PATCH v2] 3/3] Revert "Scroll diff with arrow keys in log view"
This reverts commit 888611dd5d407775245d574a3dc5c01b5963a5ba. This is
because, in the re-engineered log view, scrolling the log with the
arrows now updates the diff in the diff view when the screen is
split. This resembles the earlier behaviour, and is also what users of
software like Mutt (which uses the pager view concept) would expect.
Signed-Off-By: Kumar Appaiah
Conflicts:
tig.c
---
tig.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/tig.c b/tig.c
index 256b589..5f564a5 100644
--- a/tig.c
+++ b/tig.c
@@ -1905,7 +1905,6 @@ enum view_flag {
VIEW_STDIN = 1 << 8,
VIEW_SEND_CHILD_ENTER = 1 << 9,
VIEW_FILE_FILTER= 1 << 10,
- VIEW_NO_PARENT_NAV = 1 << 11,
};
#define view_has_flags(view, flag) ((view)->ops->flags & (flag))
@@ -3773,7 +3772,7 @@ view_driver(struct view *view, enum request request)
case REQ_NEXT:
case REQ_PREVIOUS:
- if (view->parent && !view_has_flags(view->parent,
VIEW_NO_PARENT_NAV)) {
+ if (view->parent) {
int line;
view = view->parent;
@@ -4490,7 +4489,7 @@ log_request(struct view *view, enum request request,
struct line *line)
static struct view_ops log_ops = {
"line",
{ "log" },
- VIEW_ADD_PAGER_REFS | VIEW_OPEN_DIFF | VIEW_SEND_CHILD_ENTER |
VIEW_NO_PARENT_NAV,
+ VIEW_ADD_PAGER_REFS | VIEW_OPEN_DIFF | VIEW_SEND_CHILD_ENTER,
sizeof(struct log_state),
log_open,
pager_read,
--
1.8.3.2
--
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] remote-hg: add shared repo upgrade
On Mon, Aug 5, 2013 at 11:02 PM, Junio C Hamano wrote:
> Antoine Pelisse writes:
> Is the untold
> and obvious-to-those-who-are-familiar-with-this-codepath assumption
> that it is guaranteed that there is at most one "*/clone/.hg" under
> shared_path?
No, there is no such assumption.
That is why we create a repository just below if it doesn't exist (no
copy was found).
That's also why I don't see how we could split the patch.
We could improve that part of the commit message:
It's trivial to upgrade to the new organization by copying the Mercurial
repo from one of the remotes (e.g. 'origin'), so let's do so. If
we can't find
any existing repo, we create an empty one.
>> +# setup shared repo (if not there)
>> +try:
>> +hg.peer(myui, {}, shared_path, create=True)
>> +except error.RepoError:
>> +pass
>>
>> if not os.path.exists(dirname):
>> os.makedirs(dirname)
--
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] remote-hg: add shared repo upgrade
Antoine Pelisse writes: > On Mon, Aug 5, 2013 at 11:02 PM, Junio C Hamano wrote: >> Antoine Pelisse writes: >> Is the untold >> and obvious-to-those-who-are-familiar-with-this-codepath assumption >> that it is guaranteed that there is at most one "*/clone/.hg" under >> shared_path? > > No, there is no such assumption. > That is why we create a repository just below if it doesn't exist (no > copy was found). > That's also why I don't see how we could split the patch. > > We could improve that part of the commit message: > > It's trivial to upgrade to the new organization by copying the Mercurial > repo from one of the remotes (e.g. 'origin'), so let's do so. If > we can't find > any existing repo, we create an empty one. That is fine, and I do not (yet) have an opinion on this patch needing to be further split. Quoting that part I was asking about again: > +# check and upgrade old organization > +hg_path = os.path.join(shared_path, '.hg') > +if os.path.exists(shared_path) and not os.path.exists(hg_path): > +repos = os.listdir(shared_path) > +for x in repos: > +local_hg = os.path.join(shared_path, x, 'clone', '.hg') > +if not os.path.exists(local_hg): > +continue > +shutil.copytree(local_hg, hg_path) if you can have more than one 'x' such that local_hg = os.path.join(shared_path, x, 'clone', '.hg') exists, that means in repos[], there are two (or more) x1,and x2, and in this loop you will run shutil.copytree(local_hg, hg_path) twice, once for local_hg derived from x1 and another time from x2, both to the same hg_path directory that does not change inside the loop. shutil.copytree(src, dst) however creates leading paths down to dst and it would barf when dst already exists, no? That is what I was puzzled about the code. The log message says "we can copy from one of them if exists, so let's do so", which makes sense, and a code structure that may match would have looked like so: for x in repos: '''pick one at random, copy it and leave''' copytree() break else: '''nothing to be copied, do it the hard way by cloning''' but that is not what I saw so that is where my confusion came from. -- 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: [PATCHv3 0/9] Removing deprecated parsing macros
Thanks. Queued this at the tip of 'pu'. There seem to be some fallouts found in the test suite, though. -- 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: About close() in commit_lock_file()
Am 8/5/2013 16:23, schrieb Duy Nguyen: > close() is added in commit_lock_file(), before rename(), by 4723ee9 > (Close files opened by lock_file() before unlinking. - 2007-11-13), > which is needed by Windows. But doesn't that create a gap between > close() and rename() on other platforms where another process can > replace .lock file with something else before rename() is executed? First, files manipulated by commit_lock_file() are to be opened only using lock_file() by definition. Opening such a file in with open() or fopen() or renaming it via rename() without using the lockfile.[ch] API is possible regardless of whether commit_lock_file() has closed the file or not. Such manipulation is already undefined behavior (from Git's point of view), and there is nothing we can do about "misbehaving" processes. Second, lock_file() uses O_CREAT|O_EXCL, which fails when the file exists, regardless of whether it is open or not. Conclusion: There is no problem. -- Hannes -- 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 v3] gc: reject if another gc is running, unless --force is given
Ramkumar Ramachandra writes: > Junio C Hamano wrote: >> [...] > > The other comments mostly make sense. > >> After reading what the whole function does, I think the purpose of >> this function is to take gc-lock (with optionally force). Perhaps a >> name along the lines of "lock_gc", "gc_lock", "lock_repo_for_gc", >> would be more appropriate. > > The whole point of this exercise is to _not_ lock up the repo during > gc,... I do not think it is a misnomer to call the entity that locks other instances of gc's "a lock on the repository for gc". Nothing in Duy's code suggests any other commands paying attention to this mechanism and stalling, and I think my comments were clear enough that I was not suggesting such a change. So I am not sure what you are complaining. -- 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 v2] git-p4: Ask "p4" to interpret View setting
In Perforce, View setting of p4 client can describe
-//depot/project/files/*.xls //client/project/files/*.xls
to exclude Excel files.
But "git p4 --use-client-spec" cannot support '*'.
In git-p4.py, "map_in_client" method analyzes View setting and return
client file path.
So I modify the method to just ask p4.
> Let me play with this for a bit. I wonder about the performance
> aspects of doing a "p4 fstat" for every file. Would it be
> possible to do one or a few batch queries higher up somewhere?
To reduce p4 access, it cache result of asking "client path".
And addition, "fstat" depends on sync status, so modify to use "p4
where" instead of "fstat".
Signed-off-by: KazukiSaitoh
---
git-p4.py | 53 ++-
t/lib-git-p4.sh | 1 +
t/t9807-git-p4-submit.sh | 2 +-
t/t9809-git-p4-client-view.sh | 65 +++
4 files changed, 64 insertions(+), 57 deletions(-)
diff --git a/git-p4.py b/git-p4.py
index 31e71ff..8ec8eb4 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -1819,15 +1819,6 @@ class View(object):
variables."""
self.ends_triple_dot = False
-# There are three wildcards allowed in p4 views
-# (see "p4 help views"). This code knows how to
-# handle "..." (only at the end), but cannot deal with
-# "%%n" or "*". Only check the depot_side, as p4 should
-# validate that the client_side matches too.
-if re.search(r'%%[1-9]', self.path):
-die("Can't handle %%n wildcards in view: %s" % self.path)
-if self.path.find("*") >= 0:
-die("Can't handle * wildcards in view: %s" % self.path)
triple_dot_index = self.path.find("...")
if triple_dot_index >= 0:
if triple_dot_index != len(self.path) - 3:
@@ -1903,6 +1894,7 @@ class View(object):
#
def __init__(self):
self.mappings = []
+self.client_spec_path_cache = {} # Caching result of p4
query, use for "--use-client-spec".
def append(self, view_line):
"""Parse a view line, splitting it into depot and client
@@ -1965,44 +1957,17 @@ class View(object):
depot file should live. Returns "" if the file should
not be mapped in the client."""
-paths_filled = []
-client_path = ""
-
-# look at later entries first
-for m in self.mappings[::-1]:
-
-# see where will this path end up in the client
-p = m.map_depot_to_client(depot_path)
-
-if p == "":
-# Depot path does not belong in client. Must remember
-# this, as previous items should not cause files to
-# exist in this path either. Remember that the list is
-# being walked from the end, which has higher precedence.
-# Overlap mappings do not exclude previous mappings.
-if not m.overlay:
-paths_filled.append(m.client_side)
+if self.client_spec_path_cache.has_key(depot_path):
+return self.client_spec_path_cache[depot_path]
-else:
-# This mapping matched; no need to search any further.
-# But, the mapping could be rejected if the client path
-# has already been claimed by an earlier mapping (i.e.
-# one later in the list, which we are walking backwards).
-already_mapped_in_client = False
-for f in paths_filled:
-# this is View.Path.match
-if f.match(p):
-already_mapped_in_client = True
-break
-if not already_mapped_in_client:
-# Include this file, unless it is from a line that
-# explicitly said to exclude it.
-if not m.exclude:
-client_path = p
-
-# a match, even if rejected, always stops the search
+client_path = ""
+where_result = p4CmdList(['where', depot_path])
+for res in where_result:
+if res["code"] != "error" and not res.has_key("unmap"):
+client_path = res["path"].replace(getClientRoot()+"/", "")
break
+self.client_spec_path_cache[depot_path] = client_path
return client_path
class P4Sync(Command, P4UserMap):
diff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh
index 2098b9b..0d631dc 100644
--- a/t/lib-git-p4.sh
+++ b/t/lib-git-p4.sh
@@ -48,6 +48,7 @@ P4DPORT=$((10669 + ($testid - $git_p4_test_start)))
P4PORT=localhost:$P4DPORT
P4CLIENT=client
P4EDITOR=:
+P4CHARSET=""
export P4PORT P4CLIENT P4EDITOR
db="$TRASH_DIRECTORY/db"
diff --git a/t/t9807-git-p4-submit.sh b/t/t9807-git-p4-submit.sh
index 1fb7bc7..4caf36e 100755
--- a/t/t9807-git-p4-submit.sh
+++ b/t/t9807-git-p4-su
Re: [PATCH v3] gc: reject if another gc is running, unless --force is given
Junio C Hamano wrote: >>> After reading what the whole function does, I think the purpose of >>> this function is to take gc-lock (with optionally force). Perhaps a >>> name along the lines of "lock_gc", "gc_lock", "lock_repo_for_gc", >>> would be more appropriate. >> >> The whole point of this exercise is to _not_ lock up the repo during >> gc,... > > I do not think it is a misnomer to call the entity that locks other > instances of gc's "a lock on the repository for gc". Nothing in > Duy's code suggests any other commands paying attention to this > mechanism and stalling, and I think my comments were clear enough > that I was not suggesting such a change. > > So I am not sure what you are complaining. Not complaining; I wrote it down because of your "lock_repo_for_gc" suggestion. -- 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] remote-hg: add shared repo upgrade
On Tue, Aug 6, 2013 at 8:36 AM, Junio C Hamano wrote: > Antoine Pelisse writes: > Quoting that part I was asking about again: > >> +# check and upgrade old organization >> +hg_path = os.path.join(shared_path, '.hg') >> +if os.path.exists(shared_path) and not os.path.exists(hg_path): >> +repos = os.listdir(shared_path) >> +for x in repos: >> +local_hg = os.path.join(shared_path, x, 'clone', '.hg') >> +if not os.path.exists(local_hg): >> +continue >> +shutil.copytree(local_hg, hg_path) > > if you can have more than one 'x' such that OK, Sorry for the misunderstanding, I read "at least one", instead of "at most one". Yes, I think "break" is missing right after copytree(). -- 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
