Re: [PATCH 3/3] clone: add `--seed` shorthand
On Thu, May 21, 2015 at 06:05:31PM +0200, Johannes Schindelin wrote: +--seed repository:: + A convenient shorthand for `--dissociate --reference=repository`. + Since you want to advertise this as an easier way than `--dissociate --reference=repository`, it might make sense to avoid sending the reader that way, too. Maybe something like --seed repository:: Fetch objects from repository instead of the clone URL when possible. This is useful when a (possibly partial) clone already exists locally, to avoid transferring the same objects again. Yeah, I agree that is much better (it's the 4/3 I suggested in the cover letter). -Peff -- 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 3/3] clone: add `--seed` shorthand
Hi Philip, On 2015-05-21 21:45, Philip Oakley wrote: From: Johannes Schindelin johannes.schinde...@gmx.de On 2015-05-21 06:16, Jeff King wrote: diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt index f1f2a3f..ffeb03b 100644 --- a/Documentation/git-clone.txt +++ b/Documentation/git-clone.txt @@ -107,6 +107,9 @@ objects from the source repository into a pack in the cloned repository. transfer and stop borrowing from them after a clone is made by making necessary local copies of borrowed objects. +--seed repository:: + A convenient shorthand for `--dissociate --reference=repository`. + Since you want to advertise this as an easier way than `--dissociate --reference=repository`, it might make sense to avoid sending the reader that way, too. Maybe something like --seed repository:: Fetch objects from repository instead of the clone URL when possible. This is useful when a (possibly partial) clone already exists locally, to avoid transferring the same objects again. Would it be worth mentioning here that a bundle is a satisfactory alternative to repository? +--seed repository|bundle:: +Fetch objects from repository or bundle instead of the clone URL when possible. This is useful when a (possibly partial) clone already exists locally, to avoid transferring the same objects again. I haven't checked if the invocation would accept a bundle filename, but I'm presuming it can in the same way that clone does. The proof would lie in the pudding ;-) Would you mind testing whether it works with bundles? Ciao, Dscho -- 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: Troubleshoot clone issue to NFS.
On Fri, May 22, 2015 at 07:16:54AM +0700, Duy Nguyen wrote:
Is there anything else I can provide or test?
In builtin/index-pack.c, replace the line collision_test_needed =
has_sha1_file(sha1); with collision_test_needed = 0;. Security is
compromised but for this test it should be ok. Then clone again. I
hope the new number gets down close to v1.8.4.1.
Yeah, I think that is a good starting point. I timed a local clone
before and after 45e8a74; there is a small difference on my system
(about 5%), but it goes away with your suggestion.
The problem with has_sha1_file() prior to v1.8.4.2 is that it is racy
with respect to simultaneous operations; we might claim we do not have
an object, when in fact we do. As you noted, usually has_sha1_file()
returns true (i.e., we look up objects that we expect to have), and the
performance impact is minimal.
But for code paths where _not_ having the object is normal, the impact
is much greater. So I think there are two possibilities for improving
this:
1. Find places where we expect the object will not exist (like the
collision_test check you pointed out) and use a
has_sha1_file_fast that accepts that it may very occasionally
erroneously return false. In this case it would mean potentially
skipping a collision check, but I think that is OK. That could have
security implications, but only if an attacker:
a. has broken sha1 to generate a colliding object
b. can manipulate the victim into repacking in a loop
c. can manipulate the victim into fetching (or receiving a push)
simultaneously with (b)
at which point they can try to race the repack procedure to add
their colliding object to the repository. It seems rather unlikely
(especially part a).
2. Make reprepare_packed_git() cheaper in the common case that nothing
has changed. It would probably be enough to stat(objects/pack).
We know that packfiles themselves do not change; we may only add or
delete them. And since the hierarchy of objects/pack is flat, we
know that the mtime on that directory will change if any packs are
added or removed.
Of course, we are still doing an extra stat() for each has_sha1_file
call. Whether that helps for the NFS case depends on whether stat()
is significantly cheaper than opendir/readdir/closedir. On my local
disk, the hacky patch below did seem to give me back the 5% lost by
45e8a74 (I did it directly on master, since that old commit does
not have the stat_validity infrastructure).
---
diff --git a/cache.h b/cache.h
index c97d807..17c840c 100644
--- a/cache.h
+++ b/cache.h
@@ -1661,6 +1661,7 @@ int sane_execvp(const char *file, char *const argv[]);
*/
struct stat_validity {
struct stat_data *sd;
+ unsigned mode;
};
void stat_validity_clear(struct stat_validity *sv);
diff --git a/read-cache.c b/read-cache.c
index 36ff89f..1d8702f 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -2281,6 +2281,7 @@ void stat_validity_clear(struct stat_validity *sv)
{
free(sv-sd);
sv-sd = NULL;
+ sv-mode = 0;
}
int stat_validity_check(struct stat_validity *sv, const char *path)
@@ -2291,18 +2292,19 @@ int stat_validity_check(struct stat_validity *sv, const
char *path)
return sv-sd == NULL;
if (!sv-sd)
return 0;
- return S_ISREG(st.st_mode) !match_stat_data(sv-sd, st);
+ return sv-mode == st.st_mode !match_stat_data(sv-sd, st);
}
void stat_validity_update(struct stat_validity *sv, int fd)
{
struct stat st;
- if (fstat(fd, st) 0 || !S_ISREG(st.st_mode))
+ if (fstat(fd, st) 0)
stat_validity_clear(sv);
else {
if (!sv-sd)
sv-sd = xcalloc(1, sizeof(struct stat_data));
+ sv-mode = st.st_mode;
fill_stat_data(sv-sd, st);
}
}
diff --git a/sha1_file.c b/sha1_file.c
index ccc6dac..abe1be9 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1225,7 +1225,8 @@ static void report_pack_garbage(struct string_list *list)
report_helper(list, seen_bits, first, list-nr);
}
-static void prepare_packed_git_one(char *objdir, int local)
+static void prepare_packed_git_one(char *objdir, int local,
+ struct stat_validity *validity)
{
struct strbuf path = STRBUF_INIT;
size_t dirnamelen;
@@ -1235,6 +1236,12 @@ static void prepare_packed_git_one(char *objdir, int
local)
strbuf_addstr(path, objdir);
strbuf_addstr(path, /pack);
+
+ if (stat_validity_check(validity, path.buf)) {
+ strbuf_release(path);
+ return;
+ }
+
dir = opendir(path.buf);
if (!dir) {
if (errno != ENOENT)
@@ -1243,6 +1250,10 @@ static void prepare_packed_git_one(char *objdir, int
local)
strbuf_release(path);
return;
Re: [PATCH] doc: fix inconsistent spelling of packfile
On Thu, May 21, 2015 at 09:37:14AM -0700, Junio C Hamano wrote: Patrick Steinhardt p...@pks.im writes: Fix remaining instances where pack-file is used instead of packfile. Signed-off-by: Patrick Steinhardt p...@pks.im --- This patch now also fixes instances where we refer to EBNF-style command line parameters, as discussed by Junio and Peff. Thanks. diff --git a/Documentation/git-index-pack.txt b/Documentation/git-index-pack.txt index 7a4e055..49621da 100644 --- a/Documentation/git-index-pack.txt +++ b/Documentation/git-index-pack.txt @@ -9,9 +9,9 @@ git-index-pack - Build pack index file for an existing packed archive SYNOPSIS [verse] -'git index-pack' [-v] [-o index-file] pack-file +'git index-pack' [-v] [-o index-file] packfile 'git index-pack' --stdin [--fix-thin] [--keep] [-v] [-o index-file] - [pack-file] + [packfile] Hmm. The former is taking a concrete *.pack file on disk, and the latter is optionally writing the pack stream out to a *.pack file on disk. If we follow 'packfile' for the concept, 'pack-file' to refer to a file with .pack ending guideline, I'd think both should be 'pack-file'. Side note: because these invocations, especially the latter form, can take any filename, you could do: git index-pack foo.tmp mv foo.tmp $realfilename.pack in which case the arguments may not be files whose names end with .pack; it is just a file that holds pack data stream, so it could be argued that packfile is not incorrect. But the reason why you are doing the above is to ultimately create a *.pack file, and I'd say pack-file would be more correct. @@ -37,11 +37,11 @@ OPTIONS --stdin:: When this flag is provided, the pack is read from stdin - instead and a copy is then written to pack-file. If Likewise; we are writing to a *.pack file, written to is not talking about what (i.e. packfile, the pack data stream) is written but what accepts and holds that data stream in the end. - pack-file is not specified, the pack is written to + instead and a copy is then written to packfile. If + packfile is not specified, the pack is written to objects/pack/ directory of the current Git repository with a default name determined from the pack content. If - pack-file is not specified consider using --keep to + packfile is not specified consider using --keep to prevent a race condition between this process and 'git repack'. All of the above talk about that same entity on the filesystem that receives the pack data stream. diff --git a/Documentation/git-unpack-objects.txt b/Documentation/git-unpack-objects.txt index 894d20b..07d4329 100644 --- a/Documentation/git-unpack-objects.txt +++ b/Documentation/git-unpack-objects.txt @@ -9,7 +9,7 @@ git-unpack-objects - Unpack objects from a packed archive SYNOPSIS [verse] -'git unpack-objects' [-n] [-q] [-r] [--strict] pack-file +'git unpack-objects' [-n] [-q] [-r] [--strict] packfile This feeds the pack data stream to the command from its standard input, so would be a good change. Side note: if you have an on-disk file to feed this command from its standard input, it is more than likely that the file is a *.pack file, i.e. a pack-file. But in practice, the command is more often than not fed an output of another command via pipe, and it only cares about it being a pack data stream. So in that sense, both are correct but the updated one is more correct. diff --git a/Documentation/technical/pack-protocol.txt b/Documentation/technical/pack-protocol.txt index 812d857..fc09c63 100644 --- a/Documentation/technical/pack-protocol.txt +++ b/Documentation/technical/pack-protocol.txt @@ -465,7 +465,7 @@ contain all the objects that the server will need to complete the new references. All changes to this file are good. diff --git a/Documentation/user-manual.txt b/Documentation/user-manual.txt index 68978f5..7147519 100644 --- a/Documentation/user-manual.txt +++ b/Documentation/user-manual.txt @@ -3164,7 +3164,7 @@ objects. (Note that linkgit:git-tag[1] can also be used to create lightweight tags, which are not tag objects at all, but just simple references whose names begin with `refs/tags/`). -[[pack-files]] +[[packfiles]] Isn't this a xref target? Can you change it without changing all the referrers? I did not find any referrers pointing to this, so I just changed it. I do not know if there are any referrers from outside, so I guess I'll just revert that back to [[pack-files]] as it is not seen by users anyway. In any case, after doing the above two side notes, I am not sure if readers would appreciate our careful choice of words between packfile and pack-file when they read the
[Announce] submitGit for patch submission (was Diffing submodule does not yield complete logs)
On Tuesday, 19 May 2015, Stefan Beller sbel...@google.com wrote: On Tue, May 19, 2015 at 12:29 PM, Robert Dailey rcdailey.li...@gmail.com wrote: How do you send your patches inline? [snip] This workflow discussion was a topic at the GitMerge2015 conference, and there are essentially 2 groups, those who know how to send email and those who complain about it. A solution was agreed on by nearly all of the contributors. It would be awesome to have a git-to-email proxy, such that you could do a git push proxy master:refs/for/mailinglist and this proxy would convert the push into sending patch series to the mailing list. It could even convert the following discussion back into comments (on Github?) but as a first step we'd want to try out a one way proxy. Unfortunately nobody stepped up to actually do the work, yet :( Hello, I'm stepping up to do that work :) Or at least, I'm implementing a one-way GitHub PR - Mailing list tool, called submitGit: https://submitgit.herokuapp.com/ Here's what a user does: * create a PR on https://github.com/git/git * logs into https://submitgit.herokuapp.com/ with GitHub auth * selects their PR on https://submitgit.herokuapp.com/git/git/pulls * gets submitGit to email the PR as patches to themselves, in order to check it looks ok * when they're ready, get submitGit to send it to the mailing list on their behalf All discussion of the patch *stays* on the mailing list - I'm not attempting to change anything about the Git community process, other than make it easier for a wider group people to submit patches to the list. For hard-core contributors to Git, I'd imagine that git format-patch send-email remain the fastest way to do their work. But those tools are _unfamiliar to the majority of Git users_ - so submitGit aims to cater to those users, because they definitely have valuable contributions to make, which would be tragic to throw away. I've been working on submitGit in my spare time for the past few weeks, and there are still features I plan to add (like guiding the user to more 'correct' word wrapping, sign-off, etc), but given this discussion, I wanted to chime in and let people know what's here so far. It would be great if people could take the time to explore the tool (you don't have to raise a git/git PR in order to try sending one *to yourself*, for instance) and give feedback on list, or in GitHub issues: https://github.com/rtyley/submitgit/issues I've been lucky enough to discuss the ideas around submitGit with a few people at the Git-Merge conf, so thanks to Peff, Thomas Ferris Nicolaisen, and Emma Jane Hogbin Westby for listening to me (not to imply their endorsement of what I've done, just thanks for talking about it!). Roberto -- 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: Troubleshoot clone issue to NFS.
On Friday, May 22, 2015 @ 8:12 AM Jeff King did scribble:
In builtin/index-pack.c, replace the line collision_test_needed =
has_sha1_file(sha1); with collision_test_needed = 0;. Security is
compromised but for this test it should be ok. Then clone again. I
hope the new number gets down close to v1.8.4.1.
Yeah, I think that is a good starting point. I timed a local clone
before and after 45e8a74; there is a small difference on my system
(about 5%), but it goes away with your suggestion.
Tested this change on a couple of versions, first of all on the revision
where things go wrong for me:
~ $ bin/git --version
git version 1.8.4.1.g45e8a74.dirty
~ $ time bin/git clone https://github.com/git/git test
snip
real0m7.105s
user0m9.566s
sys 0m0.989s
~ $ time bin/git clone https://github.com/git/git /sami/test
snip
real0m14.411s
user0m9.703s
sys 0m1.374s
This is more in line with what I see normally. Also tested on master:
~ $ bin/git --version
git version 2.4.1.217.g6c1249c.dirty
~ $ time bin/git clone https://github.com/git/git test
snip
real0m5.946s
user0m9.111s
sys 0m1.332s
~ $ time bin/git clone https://github.com/git/git /sami/test
snip
real0m12.344s
user0m9.187s
sys 0m1.579s
So similar on the latest as well.
The problem with has_sha1_file() prior to v1.8.4.2 is that it is racy
with respect to simultaneous operations; we might claim we do not have
an object, when in fact we do. As you noted, usually has_sha1_file()
returns true (i.e., we look up objects that we expect to have), and the
performance impact is minimal.
But for code paths where _not_ having the object is normal, the impact
is much greater. So I think there are two possibilities for improving
this:
1. Find places where we expect the object will not exist (like the
collision_test check you pointed out) and use a
has_sha1_file_fast that accepts that it may very occasionally
erroneously return false. In this case it would mean potentially
skipping a collision check, but I think that is OK. That could have
security implications, but only if an attacker:
a. has broken sha1 to generate a colliding object
b. can manipulate the victim into repacking in a loop
c. can manipulate the victim into fetching (or receiving a push)
simultaneously with (b)
at which point they can try to race the repack procedure to add
their colliding object to the repository. It seems rather unlikely
(especially part a).
2. Make reprepare_packed_git() cheaper in the common case that nothing
has changed. It would probably be enough to stat(objects/pack).
We know that packfiles themselves do not change; we may only add or
delete them. And since the hierarchy of objects/pack is flat, we
know that the mtime on that directory will change if any packs are
added or removed.
Of course, we are still doing an extra stat() for each has_sha1_file
call. Whether that helps for the NFS case depends on whether stat()
is significantly cheaper than opendir/readdir/closedir. On my local
disk, the hacky patch below did seem to give me back the 5% lost by
45e8a74 (I did it directly on master, since that old commit does
not have the stat_validity infrastructure).
Also tested master with the patch provided:
~ $ bin/git --version
git version 2.4.1.217.g6c1249c.dirty
~ $ time git clone https://github.com/git/git test
real0m8.263s
user0m10.550s
sys 0m3.763s
~ $ time git clone https://github.com/git/git /sami/test
real1m3.286s
user0m12.149s
sys 0m9.192s
So the patch isn't reducing the time taken when cloning to NAS.
Here are the top calls from strace
% time seconds usecs/call callserrors syscall
-- --- --- - -
90.68 19.946171 46437398 45436 futex
4.631.017637 22 46141 5 read
2.370.521161 4141429 pread
0.730.161442 3 47130 9 write
0.420.093146 0188645188621 access
0.380.083033 26 3209 181 open
0.320.069587 0188613 1146 stat
0.230.050855 12 4082 3925 lstat
0.110.023317 8 2979 1 fstat
0.040.009134 0 35696 3 recvfrom
0.030.0076661917 4 wait4
0.020.004478 1 3923 madvise
0.010.002291 0 17858 poll
0.010.002155 0 17851 select
Thanks for looking into this.
Steve
N�r��yb�X��ǧv�^�){.n�+ا���ܨ}���Ơz�j:+v���zZ+��+zf���h���~i���z� �w���?��)ߢf
Re: [PATCH 1/2] rebase -i: demonstrate incorrect behavior of post-rewrite hook with exec
Eric Sunshine sunsh...@sunshineco.com writes: +test_expect_failure 'git rebase -i (exec)' ' + git reset --hard D + clear_hook_input + FAKE_LINES=edit 1 exec_false 2 git rebase -i B Broken -chain. Thanks, will add in v2. -- 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: [PATCH] submodule documentation: Reorder introductory paragraphs
From: Stefan Beller sbel...@google.com On Thu, May 21, 2015 at 1:03 PM, Philip Oakley philipoak...@iee.org wrote: +Submodules are not to be confused with remotes, which are meant +mainly for branches of the same project; This use of 'branches' didn't work for me. remotes are meant mainly for branches of the same project ? Maybe Submodules should not be confused with remote repositories, which are meant to track the same repository, just at another location; ... ? Though I'm not yet completely happy with that either. I like that better. I was going to check what the git glossary said a remote was, but the commute to work beckons... 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/4] ref-filter: add ref-filter API
karthik nayak karthik@gmail.com writes: I miss a high-level description of what the code is doing. Essentially, there's the complete repository list of refs, and you want to filter only some of them, right? From the name, I would guess that ref_filter is the structure describing how you are filtering, but from the code it seems to be the list you're filtering, not the filter. Reading this again, A bit confused by what you're trying to imply. Could you rephrase please? At some point, I'd expect something like filtered_list_of_refs = filer(full_list_of_refs, description_of_filter); That would remove some refs from full_list_of_refs according to description_of_filter. (totally invented code, only to show the idea) If there's a piece of code looking like this, then you need a data structure to store list of refs (full_list_of_refs and filtered_list_of_refs) and another to describe what you're doing with it (description_of_filter). The name ref_filter implies to me that it contains the description of the filter, but looking at the code it doesn't seem to be the case. -- 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: [PATCH 3/3] clone: add `--seed` shorthand
On Fri, May 22, 2015 at 08:37:56AM +0200, Johannes Schindelin wrote: +--seed repository|bundle:: +Fetch objects from repository or bundle instead of the clone URL when possible. This is useful when a (possibly partial) clone already exists locally, to avoid transferring the same objects again. I haven't checked if the invocation would accept a bundle filename, but I'm presuming it can in the same way that clone does. The proof would lie in the pudding ;-) Would you mind testing whether it works with bundles? I can't imagine that it would with my patch. The implementation is based on --reference, which is going to try to set up the bundle as an alternate. Having slept on it, I really think --seed should be fetch from the seed into temp refs, and not what I posted earlier. -Peff -- 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] submodule documentation: Reorder introductory paragraphs
From: Stefan Beller sbel...@google.com It's better to start the man page with a description of what submodules actually are instead of saying what they are not. Reorder the paragraphs such that the first short paragraph introduces the submodule concept, the second paragraph highlights the usage of the submodule command, the third paragraph giving background information, and finally the fourth paragraph discusing alternatives such as subtrees and remotes, which we don't want to be confused with. This ordering deepens the knowledge on submodules with each paragraph. First the basic questions like How/what will be answered, while the underlying concepts will be taught at a later time. Making sure it is not confused with subtrees and remotes is not really enhancing knowledge of submodules itself, but rather painting the big picture of git concepts, so you could also argue to have it as the second paragraph. Personally I think this may confuse readers, specially newcomers though. Additionally to reordering the paragraphs, they have been slightly reworded. Signed-off-by: Stefan Beller sbel...@google.com --- For now I used a part of Junios suggestion Submodules are not to be confused with remotes, which are other repositories of the same project; I like the are not to be confused part, as they warn the reader that there will be a paragraph not as concise but touching other commands and topics. Documentation/git-submodule.txt | 50 ++--- 1 file changed, 22 insertions(+), 28 deletions(-) diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt index 2c25916..d126c86 100644 --- a/Documentation/git-submodule.txt +++ b/Documentation/git-submodule.txt @@ -25,22 +25,17 @@ SYNOPSIS DESCRIPTION --- -Submodules allow foreign repositories to be embedded within -a dedicated subdirectory of the source tree, always pointed -at a particular commit. +This command will inspect, update and manage submodules. -They are not to be confused with remotes, which are meant mainly -for branches of the same project; submodules are meant for -different projects you would like to make part of your source tree, -while the history of the two projects still stays completely -independent and you cannot modify the contents of the submodule -from within the main project. -If you want to merge the project histories and want to treat the -aggregated whole as a single project from then on, you may want to -add a remote for the other project and use the 'subtree' merge strategy, -instead of treating the other project as a submodule. Directories -that come from both projects can be cloned and checked out as a whole -if you choose to go that route. +Submodules allow you to keep another Git repository in a subdirectory +of your repository. The other repository has its own history, which does not +interfere with the history of the current repository. This can be used to +have external dependencies such as libraries for example. + +When cloning or pulling a repository containing submodules however, +these will not be checked out by default; the 'init' and 'update' +subcommands will maintain submodules checked out and at +appropriate revision in your working tree. Submodules are composed from a so-called `gitlink` tree entry in the main repository that refers to a particular commit object @@ -51,19 +46,18 @@ describes the default URL the submodule shall be cloned from. The logical name can be used for overriding this URL within your local repository configuration (see 'submodule init'). -This command will manage the tree entries and contents of the -gitmodules file for you, as well as inspect the status of your -submodules and update them. -When adding a new submodule to the tree, the 'add' subcommand -is to be used. However, when pulling a tree containing submodules, -these will not be checked out by default; -the 'init' and 'update' subcommands will maintain submodules -checked out and at appropriate revision in your working tree. -You can briefly inspect the up-to-date status of your submodules -using the 'status' subcommand and get a detailed overview of the -difference between the index and checkouts using the 'summary' -subcommand. - +Submodules are not to be confused with remotes, which are other +repositories of the same project; I said (22 May 2015 20:47): if a nice well understood explanatory phrase can be found - I'm happy with yours. Many thanks submodules are meant for +different projects you would like to make part of your source tree, +while the history of the two projects still stays completely +independent and you cannot modify the contents of the submodule +from within the main project. +If you want to merge the project histories and want to treat the +aggregated whole as a single project from then on, you may want to +add a remote for the other project and use the 'subtree' merge strategy,
Re: [PATCH v6 2/2] mergetools: add winmerge as a builtin tool
[just wrapping up the unaswered questions in this thread]
On Wed, May 20, 2015 at 01:09:29PM +0200, SZEDER Gábor wrote:
Quoting David Aguilar dav...@gmail.com:
+translate_merge_tool_path() {
+# Use WinMergeU.exe if it exists in $PATH
+if type -p WinMergeU.exe /dev/null 21
+then
+printf WinMergeU.exe
+return
+fi
+
+# Look for WinMergeU.exe in the typical locations
+winmerge_exe=WinMerge/WinMergeU.exe
This variable is not used elsewhere, right? Then you might want to
mark it as local to make this clear.
local is a bash-ism, otherwise that'd be a good idea.
+for directory in $(env | grep -Ei '^PROGRAM(FILES(\(X86\))?|W6432)=' |
+cut -d '=' -f 2- | sort -u)
+do
+if test -n $directory test -x $directory/$winmerge_exe
+then
+printf '%s' $directory/$winmerge_exe
+return
+fi
+done
+
+printf WinMergeU.exe
Please pardon my ignorance and curiosity, but what is the purpose of
this last printf?
It outputs the same as in the case when winmerge is in $PATH at the
beginning of the function. However, if we reach this printf, then
winmerge is not in $PATH, so what will be executed?
This function maps what we call the tool (winmerge) to the actual executable.
That last printf provides the following behavior:
$ git difftool -t winmerge HEAD~
Viewing (1/1): 'mergetools/winmerge'
Launch 'winmerge' [Y/n]:
The diff tool winmerge is not available as 'WinMergeU.exe'
fatal: external diff died, stopping at mergetools/winmerge
It ensures that the user sees 'WinMergeU.exe' in the error message.
That way the user can resolve the problem by e.g. adjusting their $PATH,
or realizing that they don't have WinMergeU.exe installed.
--
David
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Announce] submitGit for patch submission (was Diffing submodule does not yield complete logs)
Hi, On 2015-05-22 19:14, Philip Oakley wrote: From: Johannes Schindelin johannes.schinde...@gmx.de On 2015-05-22 11:42, Johannes Schindelin wrote: On 2015-05-22 10:33, Roberto Tyley wrote: On Tuesday, 19 May 2015, Stefan Beller sbel...@google.com wrote: On Tue, May 19, 2015 at 12:29 PM, Robert Dailey rcdailey.li...@gmail.com wrote: How do you send your patches inline? [snip] This workflow discussion was a topic at the GitMerge2015 conference, and there are essentially 2 groups, those who know how to send email and those who complain about it. A solution was agreed on by nearly all of the contributors. It would be awesome to have a git-to-email proxy, such that you could do a git push proxy master:refs/for/mailinglist and this proxy would convert the push into sending patch series to the mailing list. It could even convert the following discussion back into comments (on Github?) but as a first step we'd want to try out a one way proxy. Unfortunately nobody stepped up to actually do the work, yet :( Hello, I'm stepping up to do that work :) Or at least, I'm implementing a one-way GitHub PR - Mailing list tool, called submitGit: https://submitgit.herokuapp.com/ Wow!!! I will make sure to test it with a couple of patches I want to submit anyway. I just tried this with https://github.com/git/git/pull/139 and would like to tell you about two wishes I had immediately: - If the author of a patch I am submitting is not myself, could submitGit maybe add that `From: ` line at the top of the mail? - The patch series is sent without a cover letter, but it would be *really* great if a path series consisting of more than one patch could have the initial comment of the Pull Request as a cover letter, with the link to the original Pull Request at the bottom? This would also be the mail to use in the In-reply-yo header instead of the first patch. Thanks so much! Dscho A separate request would be to be able to use PRs that are for forks of git/git, such as msysgit etc. (i.e. have a common --root), which would help in the upstreaming of some changes. I ask because I just logged in and my preparatory PR318 (https://github.com/msysgit/git/pull/318) for rejuvenating the msvc-build system wasn't listed, probably because of the forking. You can easily change what upstream your PR is intended for. For example, I made my PR from my own Git fork (which is based on msysgit/git) relative to git/git by entering the URL: https://github.com/git/git/compare/next...dscho:non-win-fixes?expand=true So there is no real need for anything extra: the only git.git project that requires emails (that I am aware of) for the submission process is git/git. Ciao, Dscho -- 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 v6 2/2] mergetools: add winmerge as a builtin tool
David Aguilar dav...@gmail.com writes: [just wrapping up the unaswered questions in this thread] ... On Wed, May 20, 2015 at 01:09:29PM +0200, SZEDER Gábor wrote: Thanks for clarifications. I think all is good now? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Announce] submitGit for patch submission (was Diffing submodule does not yield complete logs)
On Fri, May 22, 2015 at 1:04 PM, Junio C Hamano gits...@pobox.com wrote: On Fri, May 22, 2015 at 12:59 PM, Johannes Schindelin johannes.schinde...@gmx.de wrote: On 2015-05-22 21:23, Stefan Beller wrote: So first of all: Where do I find the Amazon SES account for submitGit, to register my email with? Also can I change the email in the process or change it before? FWIW I did not have to register my email. All I needed to do was to give submitGit permissions to read my personal email address and my public repositories. Hmph, I was asked way more than that (especially, read and write access). Does the site ask different authorizations depending on who you are? I was also asked for read/write on my copy of git, but as I am not the maintainer nor trusted in any way, I figured that's ok. I still have my local copy which would notice any changes on git push. The question I was asking was the only thing I could not answer or decide for myself. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Announce] submitGit for patch submission (was Diffing submodule does not yield complete logs)
Hi Stefan, On 2015-05-22 21:23, Stefan Beller wrote: Ok, I am trying it out now, all I have left is Register your email address (stefanbel...@googlemail.com) with submitGit's Amazon SES account in order for it to send emails from you. So first of all: Where do I find the Amazon SES account for submitGit, to register my email with? Also can I change the email in the process or change it before? FWIW I did not have to register my email. All I needed to do was to give submitGit permissions to read my personal email address and my public repositories. Ciao, Dscho -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Announce] submitGit for patch submission (was Diffing submodule does not yield complete logs)
From: Johannes Schindelin johannes.schinde...@gmx.de Hi, On 2015-05-22 19:14, Philip Oakley wrote: From: Johannes Schindelin johannes.schinde...@gmx.de On 2015-05-22 11:42, Johannes Schindelin wrote: On 2015-05-22 10:33, Roberto Tyley wrote: On Tuesday, 19 May 2015, Stefan Beller sbel...@google.com wrote: On Tue, May 19, 2015 at 12:29 PM, Robert Dailey rcdailey.li...@gmail.com wrote: How do you send your patches inline? [snip] This workflow discussion was a topic at the GitMerge2015 conference, and there are essentially 2 groups, those who know how to send email and those who complain about it. A solution was agreed on by nearly all of the contributors. It would be awesome to have a git-to-email proxy, such that you could do a git push proxy master:refs/for/mailinglist and this proxy would convert the push into sending patch series to the mailing list. It could even convert the following discussion back into comments (on Github?) but as a first step we'd want to try out a one way proxy. Unfortunately nobody stepped up to actually do the work, yet :( Hello, I'm stepping up to do that work :) Or at least, I'm implementing a one-way GitHub PR - Mailing list tool, called submitGit: https://submitgit.herokuapp.com/ Wow!!! I will make sure to test it with a couple of patches I want to submit anyway. I just tried this with https://github.com/git/git/pull/139 and would like to tell you about two wishes I had immediately: - If the author of a patch I am submitting is not myself, could submitGit maybe add that `From: ` line at the top of the mail? - The patch series is sent without a cover letter, but it would be *really* great if a path series consisting of more than one patch could have the initial comment of the Pull Request as a cover letter, with the link to the original Pull Request at the bottom? This would also be the mail to use in the In-reply-yo header instead of the first patch. Thanks so much! Dscho A separate request would be to be able to use PRs that are for forks of git/git, such as msysgit etc. (i.e. have a common --root), which would help in the upstreaming of some changes. I ask because I just logged in and my preparatory PR318 (https://github.com/msysgit/git/pull/318) for rejuvenating the msvc-build system wasn't listed, probably because of the forking. You can easily change what upstream your PR is intended for. For example, I made my PR from my own Git fork (which is based on msysgit/git) relative to git/git by entering the URL: https://github.com/git/git/compare/next...dscho:non-win-fixes?expand=true So there is no real need for anything extra: the only git.git project that requires emails (that I am aware of) for the submission process is git/git. Ciao, Dscho Do I read you right.. That it's necessary to create a PR on git/git before submitGit can be used. And that if I already have a PR which goes back to an alternate fork (e.g. my example), then I must move or duplicate that PR onto git/git before it can be used. [I find a few UI issues with the github interface in that I'd hoped to be able to see which outgoing PRs I have from my own fork but I can't see how to do that] regards 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
What's cooking in git.git (May 2015, #06; Fri, 22)
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 fourth batch of topics have been merged to 'master'.
The untracked cache series is in 'next', to give it a wider
exposure. I do not use it personally, but it is meant to make life
easier for those with large amount of untracked cruft in their
working trees. Please try it out and report successes (and of
course breakages, too).
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]
* dl/branch-error-message (2015-05-06) 1 commit
(merged to 'next' on 2015-05-11 at ed947ab)
+ branch: do not call a remote-tracking branch a remote branch
Error messages from git branch called remote-tracking branches as
remote branches.
* dl/subtree-avoid-tricky-echo (2015-05-08) 1 commit
(merged to 'next' on 2015-05-11 at 36d4f0e)
+ contrib/subtree: portability fix for string printing
git subtree script (in contrib/) used echo -n to produce
progress messages in a non-portable way.
* dl/subtree-push-no-squash (2015-05-07) 1 commit
(merged to 'next' on 2015-05-11 at 74d07ca)
+ contrib/subtree: there's no push --squash
git subtree script (in contrib/) does not have --squash option
when pushing, but the documentation and help text pretended as if
it did.
* ja/tutorial-asciidoctor-fix (2015-05-12) 1 commit
(merged to 'next' on 2015-05-19 at f15d940)
+ doc: fix unmatched code fences
A literal block in the tutorial had lines with unequal lengths to
delimit it from the rest of the document, which choke GitHub's
AsciiDoc renderer.
* jc/ignore-epipe-in-filter (2015-05-20) 2 commits
(merged to 'next' on 2015-05-20 at 2b14ed7)
+ filter_buffer_or_fd(): ignore EPIPE
+ copy.c: make copy_fd() report its status silently
Filter scripts were run with SIGPIPE disabled on the Git side,
expecting that they may not read what Git feeds them to filter.
We however treated a filter that does not read its input fully
before exiting as an error.
This changes semantics, but arguably in a good way. If a filter
can produce its output without consuming its input using whatever
magic, we now let it do so, instead of diagnosing it as a
programming error.
* jk/add-e-kill-editor (2015-05-12) 1 commit
(merged to 'next' on 2015-05-19 at 9e01174)
+ add: check return value of launch_editor
git add -e did not allow the user to abort the operation by
killing the editor.
* jk/asciidoc-markup-fix (2015-05-14) 9 commits
(merged to 'next' on 2015-05-19 at df0c63e)
+ doc: convert AsciiDoc {?foo} to ifdef::foo[]
+ doc: put example URLs and emails inside literal backticks
+ doc: drop backslash quoting of some curly braces
+ doc: convert \--option to --option
+ doc/add: reformat `--edit` option
+ doc: fix length of underlined section-title
+ doc: fix hanging +-continuation
+ doc: fix unquoted use of {type}
+ doc: fix misrendering due to `single quote'
Various documentation mark-up fixes to make the output more
consistent in general and also make AsciiDoctor (an alternative
formatter) happier.
* jk/skip-http-tests-under-no-curl (2015-05-07) 2 commits
(merged to 'next' on 2015-05-11 at a52b711)
+ tests: skip dav http-push tests under NO_EXPAT=NoThanks
+ t/lib-httpd.sh: skip tests if NO_CURL is defined
Test clean-up.
* jk/stripspace-asciidoctor-fix (2015-05-12) 1 commit
(merged to 'next' on 2015-05-19 at 12f9059)
+ doc: fix unmatched code fences in git-stripspace
A literal block in the tutorial had lines with unequal lengths to
delimit it from the rest of the document, which choke GitHub's
AsciiDoc renderer.
* lm/squelch-bg-progress (2015-05-19) 1 commit
(merged to 'next' on 2015-05-20 at 60916e6)
+ progress: treat no terminal as being in the foreground
The controlling tty-based heuristics to squelch progress output did
not consider that the process may not be talking to a tty at all
(e.g. sending the progress to sideband #2). This is a finishing
touch to a topic that is already in 'master'.
* ls/http-ssl-cipher-list (2015-05-08) 1 commit
(merged to 'next' on 2015-05-11 at 55764ce)
+ http: add support for specifying an SSL cipher list
Introduce http.url.SSLCipherList configuration variable to tweak
the list of cipher suite to be used with libcURL when talking with
https:// sites.
* mg/log-decorate-HEAD (2015-05-13) 2 commits
(merged to 'next' on 2015-05-19 at 009342b)
+ log: do not shorten decoration names too early
+ log: decorate HEAD with branch name under --decorate=full, too
The log --decorate enhancement in Git 2.4 that shows the commit
at the tip of the current branch e.g. HEAD - master, did not
work with --decorate=full.
* mh/clone-verbosity-fix (2015-05-19) 1 commit
(merged to 'next' on 2015-05-20 at
Re: [PATCH v2 01/18] t1404: new tests of ref D/F conflicts within transactions
On 05/14/2015 07:00 PM, Junio C Hamano wrote: Michael Haggerty mhag...@alum.mit.edu writes: But there is a i18n markings test, for which test-i18ngrep was invented for. Thanks for the info. I wasn't aware of that facility. So if I understand correctly, s/grep/test_i18ngrep/ will address your concern? That's fine with me. Thinking about it again, should these messages ever be translated, or are they plumbing messages that should never get translated? If the latter, then 'grep' is the right thing to do; in fact, it would be a bug in the test if we used test_i18ngrep. Side note: besides, I think gettext-poison tests have bitrot and are no longer very useful (if they ever were, that is). I don't really know how to decide which messages should be translatable and which not. Obviously 'update-ref' is a plumbing command, but these same error messages can be emitted by any reference transaction, such as when a push fails, in which case they are visible to the user. Some other errors that can result from ref transaction failures are tested in t1400 with 'grep'. I think using 'grep' is OK for now, and if they are internationalized in the future the breakage will be pretty obvious and straightforward to fix. Michael -- Michael Haggerty mhag...@alum.mit.edu -- 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] log: do not shorten decoration names too early
Jeff King p...@peff.net writes:
On Thu, May 14, 2015 at 03:25:33PM -0700, Junio C Hamano wrote:
@@ -90,6 +97,8 @@ static MAYBE_UNUSED elemtype *slabname## _at(struct
slabname *s, \
\
if (s-slab_count = nth_slab) {\
int i; \
+if (!add_if_missing)\
+return NULL;\
s-slab = xrealloc(s-slab, \
(nth_slab + 1) * sizeof(*s-slab)); \
stat_ ##slabname## realloc++; \
This skips extending the list of slabs if we would go past the nth slab.
But we don't fill in each slab in the first place. I.e., we may have 10
slabs, but only s-slab[10] is non-NULL.
A few lines below this, we xcalloc() it if necessary. I think that needs
to respect add_if_missing, as well.
Yup, thanks.
void unuse_commit_buffer(const struct commit *commit, const void *buffer)
{
-struct commit_buffer *v = buffer_slab_at(buffer_slab, commit);
-if (v-buffer != buffer)
+struct commit_buffer *v = buffer_slab_peek(buffer_slab, commit);
+if (v v-buffer != buffer)
free((void *)buffer);
}
I think you want:
if (!v || v-buffer != buffer)
here. IOW, we free it only if it is not our cached buffer, and it cannot
be if we do not have a cached buffer. It may be easier to read by
flipping the logic:
if (v v-buffer == buffer)
return; /* it is saved in the cache */
free((void *)buffer);
Or some variation on that.
I ended up doing it as a variant of the latter, free unless we have
v-buffer pointing at it.
Sorry for a long delay.
-- 8 --
Subject: [PATCH] commit-slab: introduce slabname##_peek() function
There is no API to ask Does this commit have associated data in
slab?. If an application wants to (1) parse just a few commits at
the beginning of a process, (2) store data for only these commits,
and then (3) start processing many commits, taking into account the
data stored (for a few of them) in the slab, the application would
use slabname##_at() to allocate a space to store data in (2), but
there is no API other than slabname##_at() to use in step (3). This
allocates and wasts new space for these commits the caller is only
interested in checking if they have data stored in step (2).
Introduce slabname##_peek(), which is similar to slabname##_at() but
returns NULL when there is no data already associated to it in such
a use case.
Helped-by: Jeff King p...@peff.net
Signed-off-by: Junio C Hamano gits...@pobox.com
---
commit-slab.h | 34 +-
commit.c | 28
2 files changed, 49 insertions(+), 13 deletions(-)
diff --git a/commit-slab.h b/commit-slab.h
index 375c9c7..9d12ce2 100644
--- a/commit-slab.h
+++ b/commit-slab.h
@@ -15,7 +15,13 @@
* - int *indegree_at(struct indegree *, struct commit *);
*
* This function locates the data associated with the given commit in
- * the indegree slab, and returns the pointer to it.
+ * the indegree slab, and returns the pointer to it. The location to
+ * store the data is allocated as necessary.
+ *
+ * - int *indegree_peek(struct indegree *, struct commit *);
+ *
+ * This function is similar to indegree_at(), but it will return NULL
+ * until a call to indegree_at() was made for the commit.
*
* - void init_indegree(struct indegree *);
* void init_indegree_with_stride(struct indegree *, int);
@@ -80,8 +86,9 @@ static MAYBE_UNUSED void clear_ ##slabname(struct slabname
*s)\
s-slab = NULL; \
} \
\
-static MAYBE_UNUSED elemtype *slabname## _at(struct slabname *s, \
- const struct commit *c) \
+static MAYBE_UNUSED elemtype *slabname## _at_peek(struct slabname *s, \
+ const struct commit *c, \
+ int add_if_missing) \
{ \
int nth_slab, nth_slot; \
\
@@ -90,6 +97,8 @@ static MAYBE_UNUSED elemtype *slabname## _at(struct slabname
*s, \
\
if (s-slab_count = nth_slab) {\
int i; \
+ if (!add_if_missing)\
+
Re: [PATCH] submodule documentation: Reorder introductory paragraphs
From: Stefan Beller sbel...@google.com
On Fri, May 22, 2015 at 10:35 AM, Philip Oakley philipoak...@iee.org
wrote:
From: Stefan Beller sbel...@google.com
On Fri, May 22, 2015 at 7:36 AM, Junio C Hamano gits...@pobox.com
wrote:
Stefan Beller sbel...@google.com writes:
On Thu, May 21, 2015 at 1:03 PM, Philip Oakley
philipoak...@iee.org
wrote:
+Submodules are not to be confused with remotes, which are meant
+mainly for branches of the same project;
This use of 'branches' didn't work for me. remotes are meant
mainly
for
branches of the same project ?
The branch in the original is used in a much wider sense than
usual branch (i.e. ref/heads/ thing you have locally); it refers to
forks of the same project but with a bit of twist. When you say
repository A is a fork of the same project as my local repository,
you would give an impression that A is not the authoritative copy
of
the project. But you can say my repository and that repository A
are branches of the same project, you give zero information as to
A's authoritativeness.
While this is correct, I think it is also confusing, because
'branch'
is a command which deals with local branches only in my perception
To deal with remote branches you need to use the commands
{remote, fetch, pull}.
So when someone mentions branch I need to think of local
operations
in one repository and not on different distributed histories.
If we are having difficulties defining a remote here (its not
defined in
gitglossary.txt anyway),
Now that we have a discussion on what remotes are, I'll send a patch
for that
as well.
why not simply put a full stop (period) after the
Submodules are not to be confused with remotes., and bypass the
problem,
avoiding digging the hole deeper.
I think we should dig deeper and point out the differences as it may
not be clear what
the differences are for new comers. Not digging deeper sounds to me
like saying
'git frotz' is not to be confused with 'git bar' FULL STOP AND I
WONT TELL YOU WHY!
Hi Stefan,
This was more of a case of simply a full stop, 'cos I can't easily tell
you why ;-). I've seen too many work situations (*) where colleagues
just dig deeper when they should stop digging, hence the note. It may be
that the style of reason could be changed. This is the final
introductory paragraph and was being pushed down partly because of this
problem (explaining things by saying what its not).
(*) The usual phrase in a report would be A moments thought will show
that ... for those concepts that would take two pages to explain and
would still be misunderstood by the unthinking folks.
That all said, if a nice well understood explanatory phrase can be found
then I'm all for it.
which is not helpful. (Why is the documentation pointing out there is
a difference to
that other command/concept, but not saying what is different?)
Submodules should not be confused with remote repositories,
which
are
meant to track the same repository, just at another location;
...
I do not think this is a great improvement. You now conflated
repository to mean project in the latter half of the sentence,
while you are trying to explain what a remote repository is.
That's true.
Your copy of git.git is not the same repository as mine; they have
different histories. Both repositories are used to work on the
same
project. submoules are not remotes, which are other repositories
of the same project, perhaps?
That makes sense.
If maybe that the feature we should pick on is the common root of the
development between the local and remote repository, and quite
distinct for
the submodule. This allows remotes to be on the same machine, as well
as
distant machines and server.
I don't think this is actually true for all remotes. Think of shallow
clones
(they have no root or a different root) or even subtrees which are
pulled
in via a remotes?
I'd avoided mentioning that potential explanation mud-hole on the same
basis that it would be hard.
The main thing about remotes is not being here (as in part of this
repository. As you point out it can be nearby in the local fs or even
on
another machine, or in the cloud)
It is I believe technically possible to have a submodule which is its
own
super project, with and without recursion, but would be very
atypical, and
would belong in the 'don't do that' category
regards
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: recovering from unordered stage entries in index error
Yes, that does turn up some interesting stuff. It looks like the repository contains some paths with non-ASCII characters, for example this one has some en-dashes (U+2013) in its name: $ svn ls -R svn://dev/trunk/curriculum/Fluency | grep Ninja__Beta Hurix work/source from May 2014/For_Anesh/06 Deliverables/Phase 2/FT3 – Ninja/FT3 – Ninja__Beta.zip $ svn ls -R svn://dev/trunk/curriculum/Fluency | grep Ninja__Beta | od -cx 000 H u r i x w o r k / s o u r c 7548697220786f776b72732f756f6372 020 e f r o m M a y 2 0 1 4 / 206572666d6f4d207961322031302f34 040 F o r _ A n e s h / 0 6 D e l 6f465f726e4173652f68363044206c65 060 i v e r a b l e s / P h a s e 766972656261656c2f73685073612065 100 2 / F T 3 342 200 223 N i n j a / 2f325446203380e22093694e6a6e2f61 120 F T 3 342 200 223 N i n j a _ _ B 5446203380e22093694e6a6e5f61425f 140 e t a . z i p \n 74652e61697a0a70 150 In the output of 'git ls-files', those paths appear quoted (there are almost 100 of them): $ git ls-files | grep Ninja__Beta curriculum/Fluency/Hurix work/source from May 2014/For_Anesh/06 Deliverables/Phase 2/FT3 \342\200\223 Ninja/FT3 \342\200\223 Ninja__Beta.zip $ git ls-files | grep ^\ | wc -l 89 In the diff you suggested, 'sort' puts those paths at the absolute top of the list, while plain old ls-files puts them inline with the rest of the contents of the curriculum/ subdir: $ grep -n Ninja__Beta Q R Q:36109:curriculum/Fluency/Hurix work/source from May 2014/For_Anesh/06 Deliverables/Phase 2/FT3 \342\200\223 Ninja/FT3 \342\200\223 Ninja__Beta.zip R:89:curriculum/Fluency/Hurix work/source from May 2014/For_Anesh/06 Deliverables/Phase 2/FT3 \342\200\223 Ninja/FT3 \342\200\223 Ninja__Beta.zip Also, I have the curriculum/Fluency/ directory marked as sparse-checkout: $ cat .git/info/sparse-checkout /* !/curriculum/Fluency/ !/curriculum/Problems/lisp/ !/curriculum/Problems/lisp_es/ !/curriculum/Problems/sdk/Geometry/ !/curriculum/Problems/sdk_es/Geometry/ !/curriculum/Problems/sdk/Test-Questions/ !/curriculum/Problems/sdk_es/Test-Questions/ !/curriculum/Problems/sdk/Grammar/ However, I tried to construct a test case that would reproduce this with a simple SVN repo containing a file created by 'touch make-git-svn-$(echo -e '\u201c')unhappy$(echo -e '\u201d')', but could not get it to fail. So there may be something more subtle going on here ... -Original Message- From: jch2...@gmail.com [mailto:jch2...@gmail.com] On Behalf Of Junio C Hamano Sent: Friday, May 22, 2015 15:25 To: McHenry, Matt Cc: git@vger.kernel.org Subject: Re: recovering from unordered stage entries in index error The message unordered stage entries in index comes only when two adjacent entries in the index are in a wrong order, e.g. test0 should come before test1 but somehow the index records them in the other way around. Doing something like this: $ git ls-files Q $ LANG=C LC_ALL=C sort Q R $ diff Q R may tell you which entries are wrong, even though it wouldn't show who made them wrong. (pardon top-posting, overlong lines and typos; sent from GMail web UI) On Tue, May 19, 2015 at 6:48 AM, McHenry, Matt mmche...@carnegielearning.com wrote: I've just upgraded my git from 2.0.5 to 2.3.6, and I'm now unable to run 'git svn fetch' in one of my repositories: $ git svn fetch fatal: unordered stage entries in index write-tree: command returned error: 128 'git status' shows a few untracked files but is otherwise clean. It looks like this check was introduced in 15999d0be8179fb7a2e6eafb931d25ed65df50aa, with the summary read_index_from(): catch out of order entries when reading an index file (first appearing in 2.2.0). Mailing list discussion looked like it implicated third-party tools. I don't recall running any other tools on this repo; it doesn't do much day-to-day other than a long series of 'git svn fetch'es. (But it's been around for a couple of years, so who knows.) At any rate, what can I do to recover from this situation? I tried to locate a path with multiple index entries like this, but got no results: $ git ls-files -s | cut -f 2-100 | sort | uniq -c | grep -v '^[ \t]*1 ' (I originally posted on SO at http://stackoverflow.com/questions/30264826/; I'll update that with any solutions that come up here, to ease future googling.) -- To unsubscribe from this list: send the line
Re: [BUG] git commit --date format parsing
On Fri, May 22, 2015 at 03:18:53PM +0200, Bastien Traverse wrote: $ git --version git version 2.4.1 $ uname -a Linux arch-clevo 4.0.4-1-ARCH #1 SMP PREEMPT Mon May 18 06:43:19 CEST 2015 x86_64 GNU/Linux $ mkdir test cd test/ $ git init $ touch test $ git add test 1. ISO 8601 (strict) $ git commit --date=2015-05-21T16∶31+02:00 -m Test commit to check date format parsing [master (root commit) fed9ae6] Test commit to check date format parsing Date: Thu May 21 02:00:00 2015 +0200 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 test -- gets the date right but confuses the timezone for the time OK, this is weird. When I tried to reproduce, I couldn't. But I had typed in the date string myself while reading your email in another window. And though I was sure that I had typed it correctly, just to be double-plus-sure I copied and pasted your string. And it failed! The date string in your email looks like this (using cut and paste): $ echo 2015-05-21T16∶31+02:00 | xxd : 3230 3135 2d30 352d 3231 5431 36e2 88b6 2015-05-21T16... 0010: 3331 2b30 323a 3030 0a 31+02:00. Your colon is actually UTF-8 for code point U+2236 (RATIO). So git's date parser does not recognize it, and punts to approxidate(), which does all manner of crazy guessing trying to figure out what you meant. -Peff -- 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 v6 2/2] mergetools: add winmerge as a builtin tool
On Fri, May 22, 2015 at 01:05:28PM -0700, Junio C Hamano wrote: David Aguilar dav...@gmail.com writes: [just wrapping up the unaswered questions in this thread] ... On Wed, May 20, 2015 at 01:09:29PM +0200, SZEDER Gábor wrote: Thanks for clarifications. I think all is good now? Yes, I think so. I just looked at what you have queued in pu and it looks good. Thanks all, -- David -- 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] prepare_packed_git: use stat_validity to avoid re-reading packs
When we try to read an object and fail, we call
reprepare_packed_git() to re-scan the list of packfiles.
This catches any racy cases in which somebody is repacking
or making new objects. The performance implications of
re-scanning the pack directory are not usually a big deal,
because failing to find an object is the unlikely case;
we typically die if the re-scan does not help.
Since 45e8a74 (has_sha1_file: re-check pack directory before
giving up, 2013-08-30), we do the same thing for
has_sha1_file. Some calls to has_sha1_file are in the same
boat: they expect most calls to return true. But some sites,
such as the collision test in index-pack.c, may make a large
number of calls, most of which they expect to be false. On a
local system, this can cause a performance slowdown of
around 5%. But on a system with high-latency system calls
(like NFS), it can be much worse.
Since the common case is that the pack directory has _not_
changed, we can improve the performance by using stat()
before doing the opendir()/readdir()/closedir() to re-scan
the directory. It's valid to check stat() for just the
single objects/pack directory because:
1. We know that packfiles themselves are not changed in
place; only new files are written, which would update
the mtime of the directory.
2. There are no subdirectories inside the pack directory.
Checking the single top-level directory can tell us
whether anything changed.
Signed-off-by: Jeff King p...@peff.net
---
I'm not sure what we want to do for the case where we don't have DIRFD.
We have to separately open the directory, but I'm not sure if regular
open() is even valid on a regular directory on all systems. We could
just skip the stat_validity entirely in that case.
cache.h | 1 +
git-compat-util.h | 4
sha1_file.c | 27 ---
3 files changed, 29 insertions(+), 3 deletions(-)
diff --git a/cache.h b/cache.h
index cdd279a..9c34a17 100644
--- a/cache.h
+++ b/cache.h
@@ -1216,6 +1216,7 @@ void stat_validity_update(struct stat_validity *sv, int
fd);
extern struct alternate_object_database {
struct alternate_object_database *next;
char *name;
+ struct stat_validity pack_validity;
char base[FLEX_ARRAY]; /* more */
} *alt_odb_list;
extern void prepare_alt_odb(void);
diff --git a/git-compat-util.h b/git-compat-util.h
index b7a97fb..8631e75 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -941,4 +941,8 @@ struct tm *git_gmtime_r(const time_t *, struct tm *);
#define getc_unlocked(fh) getc(fh)
#endif
+#if _POSIX_VERSION = 200809L
+#define HAVE_DIRFD
+#endif
+
#endif
diff --git a/sha1_file.c b/sha1_file.c
index ccc6dac..0e77838 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1225,7 +1225,8 @@ static void report_pack_garbage(struct string_list *list)
report_helper(list, seen_bits, first, list-nr);
}
-static void prepare_packed_git_one(char *objdir, int local)
+static void prepare_packed_git_one(char *objdir, int local,
+ struct stat_validity *validity)
{
struct strbuf path = STRBUF_INIT;
size_t dirnamelen;
@@ -1235,6 +1236,12 @@ static void prepare_packed_git_one(char *objdir, int
local)
strbuf_addstr(path, objdir);
strbuf_addstr(path, /pack);
+
+ if (stat_validity_check(validity, path.buf)) {
+ strbuf_release(path);
+ return;
+ }
+
dir = opendir(path.buf);
if (!dir) {
if (errno != ENOENT)
@@ -1243,6 +1250,19 @@ static void prepare_packed_git_one(char *objdir, int
local)
strbuf_release(path);
return;
}
+
+#ifdef HAVE_DIRFD
+ stat_validity_update(validity, dirfd(dir));
+#else
+ {
+ int fd = open(path.buf, O_RDONLY);
+ if (fd = 0) {
+ stat_validity_update(validity, fd);
+ close(fd);
+ }
+ }
+#endif
+
strbuf_addch(path, '/');
dirnamelen = path.len;
while ((de = readdir(dir)) != NULL) {
@@ -1348,15 +1368,16 @@ static void rearrange_packed_git(void)
static int prepare_packed_git_run_once = 0;
void prepare_packed_git(void)
{
+ static struct stat_validity validity;
struct alternate_object_database *alt;
if (prepare_packed_git_run_once)
return;
- prepare_packed_git_one(get_object_directory(), 1);
+ prepare_packed_git_one(get_object_directory(), 1, validity);
prepare_alt_odb();
for (alt = alt_odb_list; alt; alt = alt-next) {
alt-name[-1] = 0;
- prepare_packed_git_one(alt-base, 0);
+ prepare_packed_git_one(alt-base, 0, alt-pack_validity);
alt-name[-1] = '/';
}
rearrange_packed_git();
--
2.4.1.538.g69ac333
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to
Re: git status doesn't ignore GIT_DIR directory?
As far as I know, GIT_DIR was prepared as a mechanism to point at another .git directory that does not live inside the working tree, not as a mechanism to rename it to arbitrary name. E.g. $ git init $ mv .git .svn $ GIT_DIR=$(pwd)/.svn ; export GIT_DIR is not expected to work. On Fri, May 22, 2015 at 6:03 PM, Thorsten von Eicken t...@rightscale.com wrote: I renamed my .git directory to _git and I'm surprised that git status thinks that it's an untracked file: $ GIT_DIR=_git git status On branch master Your branch is up-to-date with 'origin/master'. Untracked files: (use git add file... to include in what will be committed) _git/ nothing added to commit but untracked files present (use git add to track) This doesn't seem to make sense, shouldn't git ignore whatever $GIT_DIR points to? Or is there some other magic option I need to use for it to do what I expect? -- 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 -- 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 status doesn't ignore GIT_DIR directory?
On Fri, May 22, 2015 at 07:03:48PM -0700, Junio C Hamano wrote: As far as I know, GIT_DIR was prepared as a mechanism to point at another .git directory that does not live inside the working tree, not as a mechanism to rename it to arbitrary name. E.g. $ git init $ mv .git .svn $ GIT_DIR=$(pwd)/.svn ; export GIT_DIR is not expected to work. This is not the first time we get this report. Perhaps we should document it. -- 8 -- Subject: git.txt: mention the special name .git in GIT_DIR Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- diff --git a/Documentation/git.txt b/Documentation/git.txt index 90c5f37..91d6dd2 100644 --- a/Documentation/git.txt +++ b/Documentation/git.txt @@ -691,6 +691,9 @@ Git so take care if using Cogito etc. specifies a path to use instead of the default `.git` for the base of the repository. The '--git-dir' command-line option also sets this value. ++ +Note that if GIT_DIR is set explicitly to a directory inside working +tree, the directory name must be .git. 'GIT_WORK_TREE':: Set the path to the root of the working tree. -- 8 -- -- 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 status doesn't ignore GIT_DIR directory?
On 5/22/2015 7:22 PM, Duy Nguyen wrote: On Fri, May 22, 2015 at 07:03:48PM -0700, Junio C Hamano wrote: As far as I know, GIT_DIR was prepared as a mechanism to point at another .git directory that does not live inside the working tree, not as a mechanism to rename it to arbitrary name. E.g. $ git init $ mv .git .svn $ GIT_DIR=$(pwd)/.svn ; export GIT_DIR is not expected to work. ++ +Note that if GIT_DIR is set explicitly to a directory inside working +tree, the directory name must be .git. This is not at all what I was hoping to hear, but thanks for the response. Thorsten -- 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/5] verify_lock(): return 0/-1 rather than struct ref_lock *
On Fri, May 22, 2015 at 4:34 PM, Michael Haggerty mhag...@alum.mit.edu wrote:
Its return value wasn't conveying any extra information, but it made
the reader wonder whether the ref_lock that it returned might be
different than the one that was passed to it. So change the function
to the traditional return 0 on success or a negative value on error.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
Bonus points for the documentation!
Reviewed-by: Stefan Beller sbel...@google.com
---
refs.c | 21 ++---
1 file changed, 14 insertions(+), 7 deletions(-)
diff --git a/refs.c b/refs.c
index 97043fd..4432bc9 100644
--- a/refs.c
+++ b/refs.c
@@ -2195,9 +2195,14 @@ static void unlock_ref(struct ref_lock *lock)
free(lock);
}
-/* This function should make sure errno is meaningful on error */
-static struct ref_lock *verify_lock(struct ref_lock *lock,
- const unsigned char *old_sha1, int mustexist)
+/*
+ * Verify that the reference locked by lock has the value old_sha1.
+ * Fail if the reference doesn't exist and mustexist is set. Return 0
+ * on success or a negative value on error. This function should make
+ * sure errno is meaningful on error.
+ */
+static int verify_lock(struct ref_lock *lock,
+ const unsigned char *old_sha1, int mustexist)
{
if (read_ref_full(lock-ref_name,
mustexist ? RESOLVE_REF_READING : 0,
@@ -2206,16 +2211,16 @@ static struct ref_lock *verify_lock(struct ref_lock
*lock,
error(Can't verify ref %s, lock-ref_name);
unlock_ref(lock);
errno = save_errno;
- return NULL;
+ return -1;
}
if (hashcmp(lock-old_sha1, old_sha1)) {
error(Ref %s is at %s but expected %s, lock-ref_name,
sha1_to_hex(lock-old_sha1), sha1_to_hex(old_sha1));
unlock_ref(lock);
errno = EBUSY;
- return NULL;
+ return -1;
}
- return lock;
+ return 0;
}
static int remove_empty_directories(const char *file)
@@ -2445,7 +2450,9 @@ static struct ref_lock *lock_ref_sha1_basic(const char
*refname,
goto error_return;
}
}
- return old_sha1 ? verify_lock(lock, old_sha1, mustexist) : lock;
+ if (old_sha1 verify_lock(lock, old_sha1, mustexist))
+ return NULL;
+ return lock;
error_return:
unlock_ref(lock);
--
2.1.4
--
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: recovering from unordered stage entries in index error
So maybe you can do GIT_TRACE=2 git svn fetch and post the output. I'd expect to see something like git read-tree sha1 before fatal: unorder You can then use git ls-tree sha1 to examine this tree, try to sort the file list with LANG=C sort and compare with the original list. There is no read-tree in the output (below). The sha1 that is mentioned, 74332b7, is the one for the current trunk: $ git svn log -1 --show-commit --oneline trunk r231655 | 74332b7 | CLT: changed skill from not compound to compound. So not surprisingly, I guess, I get basically the same results as with the ls-files commands earlier: files with Unicode chars are quoted and sort at the front: $ git ls-tree --name-only -r 74332b7d653cde7ba3b999cc7b0adcfd9d924440 ls-tree $ LANG=C LC_ALL=C sort ls-tree ls-tree-sorted-lc-all $ grep -n Ninja__Beta ls-tree* ls-tree:36974:curriculum/Fluency/Hurix work/source from May 2014/For_Anesh/06 Deliverables/Phase 2/FT3 \342\200\223 Ninja/FT3 \342\200\223 Ninja__Beta.zip ls-tree-sorted-lc-all:89:curriculum/Fluency/Hurix work/source from May 2014/For_Anesh/06 Deliverables/Phase 2/FT3 \342\200\223 Ninja/FT3 \342\200\223 Ninja__Beta.zip (Just sorting with LANG=C but without LC_ALL=C produced a ton of other differences, mostly around numeric vs. lexical ordering as far as I could tell.) I tried this same thing with my test repo, and it exhibits the same quoted-filename-sorts-to-the-top behaviour, but does not exhibit the git svn fetch write-tree error. $ GIT_TRACE=2 git svn fetch 22:21:16.683918 git.c:557 trace: exec: 'git-svn' 'fetch' 22:21:16.683945 run-command.c:351 trace: run_command: 'git-svn' 'fetch' 02:21:16.918593 git.c:348 trace: built-in: git 'rev-parse' '--git-dir' 02:21:16.920218 git.c:348 trace: built-in: git 'rev-parse' '--show-cdup' 02:21:16.921997 git.c:348 trace: built-in: git 'config' '--bool' '--get' 'svn.fetchall' 02:21:16.923609 git.c:348 trace: built-in: git 'config' '--int' '--get' 'svn.repack' 02:21:16.925164 git.c:348 trace: built-in: git 'config' '--get' 'svn.repackflags' 02:21:16.926706 git.c:348 trace: built-in: git 'config' '--get' 'svn.revision' 02:21:16.928847 git.c:348 trace: built-in: git 'config' '--bool' '--get' 'svn.nocheckout' 02:21:16.930410 git.c:348 trace: built-in: git 'config' '--bool' '--get' 'svn.useSvnsyncProps' 02:21:16.931963 git.c:348 trace: built-in: git 'config' '--bool' '--get' 'svn.localtime' 02:21:16.933538 git.c:348 trace: built-in: git 'config' '--get' 'svn.includepaths' 02:21:16.935107 git.c:348 trace: built-in: git 'config' '--get' 'svn.username' 02:21:16.936675 git.c:348 trace: built-in: git 'config' '--bool' '--get' 'svn.noauthcache' 02:21:16.940413 git.c:348 trace: built-in: git 'config' '--bool' '--get' 'svn.quiet' 02:21:16.942064 git.c:348 trace: built-in: git 'config' '--bool' '--get' 'svn.uselogauthor' 02:21:16.943696 git.c:348 trace: built-in: git 'config' '--bool' '--get' 'svn.noMetadata' 02:21:16.945344 git.c:348 trace: built-in: git 'config' '--bool' '--get' 'svn.useSvmProps' 02:21:16.947607 git.c:348 trace: built-in: git 'config' '--bool' '--get' 'svn.parent' 02:21:16.950737 git.c:348 trace: built-in: git 'config' '--bool' '--get' 'svn.addauthorfrom' 02:21:16.952532 git.c:348 trace: built-in: git 'config' '--get' 'svn.authorsprog' 02:21:16.954133 git.c:348 trace: built-in: git 'config' '--get' 'svn.ignorepaths' 02:21:16.955704 git.c:348 trace: built-in: git 'config' '--bool' '--get' 'svn.followparent' 02:21:16.957287 git.c:348 trace: built-in: git 'config' '--get' 'svn.configdir' 02:21:16.958930 git.c:348 trace: built-in: git 'config' '--get' 'svn.authorsfile' 02:21:16.962142 git.c:348 trace: built-in: git 'config' '--int' '--get' 'svn.logwindowsize' 02:21:16.963913 git.c:348 trace: built-in: git 'config' '--get' 'svn.ignorerefs' 02:21:16.966130 git.c:348 trace: built-in: git 'rev-parse' '--symbolic' '--all' 02:21:16.970537 git.c:348 trace: built-in: git 'config' '-l' 02:21:16.972410 git.c:348 trace: built-in: git 'config' '-l' 02:21:16.974187 git.c:348 trace: built-in: git 'config' '--bool' 'svn.useSvmProps' 02:21:16.976074 git.c:348 trace: built-in: git 'config' '-l' 02:21:17.136056 git.c:348 trace: built-in: git 'config' '--int' '--get' 'svn-remote.svn.branches-maxRev' 02:21:17.137928 git.c:348 trace: built-in: git 'config' '--int' '--get' 'svn-remote.svn.tags-maxRev' 02:21:17.140124 git.c:348 trace: built-in: git 'config' '--get' 'svn-remote.svn.url' 02:21:17.142192
Re: [PATCH v5 2/3] upload-pack: prepare to extend allow-tip-sha1-in-want
2015-05-22 0:07 GMT+02:00 Junio C Hamano gits...@pobox.com:
Fredrik Medley fredrik.med...@gmail.com writes:
To allow future extensions, e.g. allowing non-tip sha1, replace the
boolean allow_tip_sha1_in_want variable with the flag-style
allow_request_with_bare_object_name variable.
Signed-off-by: Fredrik Medley fredrik.med...@gmail.com
---
fetch-pack.c | 9 ++---
upload-pack.c | 20 +---
2 files changed, 19 insertions(+), 10 deletions(-)
diff --git a/fetch-pack.c b/fetch-pack.c
index 48526aa..699f586 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -43,7 +43,10 @@ static int marked;
#define MAX_IN_VAIN 256
static struct prio_queue rev_list = { compare_commits_by_commit_date };
-static int non_common_revs, multi_ack, use_sideband, allow_tip_sha1_in_want;
+static int non_common_revs, multi_ack, use_sideband;
+/* Allow specifying sha1 if it is a ref tip. */
+#define ALLOW_TIP_SHA1 01
+static int allow_unadvertised_object_request;
It is better to use unsigned int for these bit masks, as we are
not interested in the top-most bit getting special-cased by using a
signed type. I'll amend this (and the one in upload-pack.c) while
applying, so no need to resend only to correct these two, unless you
have other reasons to reroll.
Sounds like a good idea to change. Please amend it while applying.
Thanks.
Thank you too for the review.
diff --git a/upload-pack.c b/upload-pack.c
index 745fda8..726486b 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -35,7 +35,9 @@ static int multi_ack;
static int no_done;
static int use_thin_pack, use_ofs_delta, use_include_tag;
static int no_progress, daemon_mode;
-static int allow_tip_sha1_in_want;
+/* Allow specifying sha1 if it is a ref tip. */
+#define ALLOW_TIP_SHA1 01
+static int allow_unadvertised_object_request;
--
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 status doesn't ignore GIT_DIR directory?
I renamed my .git directory to _git and I'm surprised that git status thinks that it's an untracked file: $ GIT_DIR=_git git status On branch master Your branch is up-to-date with 'origin/master'. Untracked files: (use git add file... to include in what will be committed) _git/ nothing added to commit but untracked files present (use git add to track) This doesn't seem to make sense, shouldn't git ignore whatever $GIT_DIR points to? Or is there some other magic option I need to use for it to do what I expect? -- 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
Please Acknowledge My Proposal!!
Please Acknowledge My Proposal!! My name is Mr. Juan Martin Domingo a lawyer resident in Spain. I am writing to let you know I have some FUNDS I want to transfer and am seeking if you can be a beneficiary...Do not hesitate to Contact me for more information if interested: gva.abogad...@aim.com). Sincerely Juan Martin Domingo. -- 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 v5 3/3] upload-pack: optionally allow fetching reachable sha1
2015-05-22 0:15 GMT+02:00 Junio C Hamano gits...@pobox.com:
Fredrik Medley fredrik.med...@gmail.com writes:
--- a/Documentation/technical/protocol-capabilities.txt
+++ b/Documentation/technical/protocol-capabilities.txt
@@ -260,6 +260,13 @@ If the upload-pack server advertises this capability,
fetch-pack may
send want lines with SHA-1s that exist at the server but are not
advertised by upload-pack.
+allow-reachable-sha1-in-want
+--
This is an underline applied to one line prior, and their length
must match. I'll amend while applying (attached at end), so there
is no need to resend with correction unless you have other reasons
to do so.
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index 8a5f236..fdcc114 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -1120,6 +1120,61 @@ test_expect_success 'fetch exact SHA1' '
)
'
It looks like this new set of tests are well thought out; good job.
I spotted a few minor nits, though. All I'll amend while applying
so there is no need to resend only to correct them.
I agree on all your comments and your proposed amendment further down
looks good.
Should the test code contain the explanations you've written in this email?
+for configallowtipsha1inwant in true false
+do
+ test_expect_success shallow fetch reachable SHA1 (but not a ref),
allowtipsha1inwant=$configallowtipsha1inwant '
+ mk_empty testrepo
+ (
+ cd testrepo
+ git config uploadpack.allowtipsha1inwant
$configallowtipsha1inwant
+ git commit --allow-empty -m foo
+ git commit --allow-empty -m bar
+ )
+ SHA1=$(git --git-dir=testrepo/.git rev-parse HEAD^)
+ mk_empty shallow
+ (
+ cd shallow
+ test_must_fail git fetch --depth=1 ../testrepo/.git
$SHA1
This tries to fetch one before the tip with allowTipSHA1InWant set
to true or false; either should fail. Good.
+ git --git-dir=../testrepo/.git config
uploadpack.allowreachablesha1inwant true
+ git fetch --depth=1 ../testrepo/.git $SHA1
And regardless of allowTip setting, with allowReachable set to true,
fetching the reachable HEAD^ would succeed. Good.
+ git cat-file commit $SHA1 /dev/null
Minor nit; drop /dev/null, as test framework will squelch the
output by default, and when the test is run with -v option, the
output would help debugging the script.
+ )
+ '
+
+ test_expect_success deny fetch unreachable SHA1,
allowtipsha1inwant=$configallowtipsha1inwant '
+ mk_empty testrepo
+ (
+ cd testrepo
+ git config uploadpack.allowtipsha1inwant
$configallowtipsha1inwant
+ git commit --allow-empty -m foo
+ git commit --allow-empty -m bar
+ git commit --allow-empty -m xyz
+ )
Broken chain
+ SHA1_1=$(git --git-dir=testrepo/.git rev-parse HEAD^^)
+ SHA1_2=$(git --git-dir=testrepo/.git rev-parse HEAD^)
+ SHA1_3=$(git --git-dir=testrepo/.git rev-parse HEAD)
+ (
+ cd testrepo
+ git reset --hard $SHA1_2
We have one before the tip (SHA1_1), one after the tip and no longer
reachable (SHA1_3); SHA1_2 is sitting at the tip of a ref.
+ git cat-file commit $SHA1_3 /dev/null
+ git cat-file commit $SHA1_3 /dev/null
I think one of the latter two is $SHA1_1, i.e. you make sure SHA1_{1,2,3}
are there in testrepo.
Yes, that was intended.
+ )
+ mk_empty shallow
+ (
+ cd shallow
+ test_must_fail git fetch ../testrepo/.git $SHA1_3
+ test_must_fail git fetch ../testrepo/.git $SHA1_1
With allowTip only, whether it is set to true or false, fetching _1
or _3 that are not at tip will fail. Good.
+ git --git-dir=../testrepo/.git config
uploadpack.allowreachablesha1inwant true
+ git fetch ../testrepo/.git $SHA1_1
+ git cat-file commit $SHA1_1 /dev/null
With allowReachable, _1 which is reachable from tip is possible,
regardless of the setting of allowTip. Good.
+ test_must_fail git cat-file commit $SHA1_2 /dev/null
And fetching _1 will not pull in _2, which is _1's child, that we
did not ask for. Good (but it is probably not very relevant for the
purpose of these tests).
+ git fetch ../testrepo/.git $SHA1_2
+ git cat-file commit $SHA1_2 /dev/null
And of course, _2 can be fetched.
+ test_must_fail git fetch
Re: recovering from unordered stage entries in index error
On Sat, May 23, 2015 at 1:56 AM, McHenry, Matt mmche...@carnegielearning.com wrote: $ git svn fetch fatal: unordered stage entries in index write-tree: command returned error: 128 git-svn does not create the index manually. It uses update-index or read-tree to do that. While there's still a chance of bugs in update-index that produces broken index, it's probably read-tree in this case because it assumes good order from the source tree object, which is(?) generated by git-svn. And the write-tree message supports this (the code does read-tree then write-tree). So maybe you can do GIT_TRACE=2 git svn fetch and post the output. I'd expect to see something like git read-tree sha1 before fatal: unorder You can then use git ls-tree sha1 to examine this tree, try to sort the file list with LANG=C sort and compare with the original list. -- 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
[PATCH 0/5] Fix verify_lock() to report errors via strbuf
verify_lock() is a helper function called while committing reference transactions. But when it fails, instead of recording its error message in a strbuf to be passed back to the caller of ref_transaction_commit(), the error message was being written directly to stderr. Instead, report the errors via a strbuf so that they make it back to the caller of ref_transaction_commit(). The last two patches remove the capitalization of some error messages, to be consistent with our usual practice. These are a slight backwards incompatibility; if any scripts are trying to grep for these error message strings, they might have to be adjusted. So feel free to drop them if you think consistency across time is more important than consistency across commands. This is the patch series that I mentioned here [1]. It applies on top of mh/ref-directory-file [2] and is thematically a continuation of that series in the sense that it further cleans up the error handling within reference transactions. It would be easy to rebase to master if that is preferred. These patches are also available on my GitHub account [3] as branch verify-lock-strbuf-err. [1] http://article.gmane.org/gmane.comp.version-control.git/269006 [2] http://thread.gmane.org/gmane.comp.version-control.git/268778 [3] https://github.com/mhagger/git Michael Haggerty (5): verify_lock(): return 0/-1 rather than struct ref_lock * verify_lock(): on errors, let the caller unlock the lock verify_lock(): report errors via a strbuf verify_lock(): do not capitalize error messages ref_transaction_commit(): do not capitalize error messages refs.c| 40 ++-- t/t1400-update-ref.sh | 14 +++--- 2 files changed, 33 insertions(+), 21 deletions(-) -- 2.1.4 -- 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 4/5] verify_lock(): do not capitalize error messages
Our convention is for error messages to start with a lower-case
letter.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
refs.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/refs.c b/refs.c
index 625e69f..48aff79 100644
--- a/refs.c
+++ b/refs.c
@@ -2211,12 +2211,12 @@ static int verify_lock(struct ref_lock *lock,
mustexist ? RESOLVE_REF_READING : 0,
lock-old_sha1, NULL)) {
int save_errno = errno;
- strbuf_addf(err, Can't verify ref %s, lock-ref_name);
+ strbuf_addf(err, can't verify ref %s, lock-ref_name);
errno = save_errno;
return -1;
}
if (hashcmp(lock-old_sha1, old_sha1)) {
- strbuf_addf(err, Ref %s is at %s but expected %s,
+ strbuf_addf(err, ref %s is at %s but expected %s,
lock-ref_name,
sha1_to_hex(lock-old_sha1),
sha1_to_hex(old_sha1));
--
2.1.4
--
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 5/5] ref_transaction_commit(): do not capitalize error messages
Our convention is for error messages to start with a lower-case
letter.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
refs.c| 4 ++--
t/t1400-update-ref.sh | 14 +++---
2 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/refs.c b/refs.c
index 48aff79..1d60fcd 100644
--- a/refs.c
+++ b/refs.c
@@ -3856,7 +3856,7 @@ int ref_transaction_commit(struct ref_transaction
*transaction,
? TRANSACTION_NAME_CONFLICT
: TRANSACTION_GENERIC_ERROR;
reason = strbuf_detach(err, NULL);
- strbuf_addf(err, Cannot lock ref '%s': %s,
+ strbuf_addf(err, cannot lock ref '%s': %s,
update-refname, reason);
free(reason);
goto cleanup;
@@ -3883,7 +3883,7 @@ int ref_transaction_commit(struct ref_transaction
*transaction,
} else if (write_ref_sha1(update-lock,
update-new_sha1,
update-msg)) {
update-lock = NULL; /* freed by write_ref_sha1
*/
- strbuf_addf(err, Cannot update the ref '%s'.,
+ strbuf_addf(err, cannot update the ref '%s'.,
update-refname);
ret = TRANSACTION_GENERIC_ERROR;
goto cleanup;
diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
index 86fa468..9c133c1 100755
--- a/t/t1400-update-ref.sh
+++ b/t/t1400-update-ref.sh
@@ -519,7 +519,7 @@ test_expect_success 'stdin create ref works with path with
space to blob' '
test_expect_success 'stdin update ref fails with wrong old value' '
echo update $c $m $m~1 stdin
test_must_fail git update-ref --stdin stdin 2err
- grep fatal: Cannot lock ref '''$c''' err
+ grep fatal: cannot lock ref '''$c''' err
test_must_fail git rev-parse --verify -q $c
'
@@ -555,7 +555,7 @@ test_expect_success 'stdin update ref works with right old
value' '
test_expect_success 'stdin delete ref fails with wrong old value' '
echo delete $a $m~1 stdin
test_must_fail git update-ref --stdin stdin 2err
- grep fatal: Cannot lock ref '''$a''' err
+ grep fatal: cannot lock ref '''$a''' err
git rev-parse $m expect
git rev-parse $a actual
test_cmp expect actual
@@ -688,7 +688,7 @@ test_expect_success 'stdin update refs fails with wrong old
value' '
update $c ''
EOF
test_must_fail git update-ref --stdin stdin 2err
- grep fatal: Cannot lock ref '''$c''' err
+ grep fatal: cannot lock ref '''$c''' err
git rev-parse $m expect
git rev-parse $a actual
test_cmp expect actual
@@ -883,7 +883,7 @@ test_expect_success 'stdin -z create ref works with path
with space to blob' '
test_expect_success 'stdin -z update ref fails with wrong old value' '
printf $F update $c $m $m~1 stdin
test_must_fail git update-ref -z --stdin stdin 2err
- grep fatal: Cannot lock ref '''$c''' err
+ grep fatal: cannot lock ref '''$c''' err
test_must_fail git rev-parse --verify -q $c
'
@@ -899,7 +899,7 @@ test_expect_success 'stdin -z create ref fails when ref
exists' '
git rev-parse $c expect
printf $F create $c $m~1 stdin
test_must_fail git update-ref -z --stdin stdin 2err
- grep fatal: Cannot lock ref '''$c''' err
+ grep fatal: cannot lock ref '''$c''' err
git rev-parse $c actual
test_cmp expect actual
'
@@ -930,7 +930,7 @@ test_expect_success 'stdin -z update ref works with right
old value' '
test_expect_success 'stdin -z delete ref fails with wrong old value' '
printf $F delete $a $m~1 stdin
test_must_fail git update-ref -z --stdin stdin 2err
- grep fatal: Cannot lock ref '''$a''' err
+ grep fatal: cannot lock ref '''$a''' err
git rev-parse $m expect
git rev-parse $a actual
test_cmp expect actual
@@ -1045,7 +1045,7 @@ test_expect_success 'stdin -z update refs fails with
wrong old value' '
git update-ref $c $m
printf $F update $a $m $m update $b $m $m update $c $m
$Z stdin
test_must_fail git update-ref -z --stdin stdin 2err
- grep fatal: Cannot lock ref '''$c''' err
+ grep fatal: cannot lock ref '''$c''' err
git rev-parse $m expect
git rev-parse $a actual
test_cmp expect actual
--
2.1.4
--
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/5] verify_lock(): return 0/-1 rather than struct ref_lock *
Its return value wasn't conveying any extra information, but it made
the reader wonder whether the ref_lock that it returned might be
different than the one that was passed to it. So change the function
to the traditional return 0 on success or a negative value on error.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
refs.c | 21 ++---
1 file changed, 14 insertions(+), 7 deletions(-)
diff --git a/refs.c b/refs.c
index 97043fd..4432bc9 100644
--- a/refs.c
+++ b/refs.c
@@ -2195,9 +2195,14 @@ static void unlock_ref(struct ref_lock *lock)
free(lock);
}
-/* This function should make sure errno is meaningful on error */
-static struct ref_lock *verify_lock(struct ref_lock *lock,
- const unsigned char *old_sha1, int mustexist)
+/*
+ * Verify that the reference locked by lock has the value old_sha1.
+ * Fail if the reference doesn't exist and mustexist is set. Return 0
+ * on success or a negative value on error. This function should make
+ * sure errno is meaningful on error.
+ */
+static int verify_lock(struct ref_lock *lock,
+ const unsigned char *old_sha1, int mustexist)
{
if (read_ref_full(lock-ref_name,
mustexist ? RESOLVE_REF_READING : 0,
@@ -2206,16 +2211,16 @@ static struct ref_lock *verify_lock(struct ref_lock
*lock,
error(Can't verify ref %s, lock-ref_name);
unlock_ref(lock);
errno = save_errno;
- return NULL;
+ return -1;
}
if (hashcmp(lock-old_sha1, old_sha1)) {
error(Ref %s is at %s but expected %s, lock-ref_name,
sha1_to_hex(lock-old_sha1), sha1_to_hex(old_sha1));
unlock_ref(lock);
errno = EBUSY;
- return NULL;
+ return -1;
}
- return lock;
+ return 0;
}
static int remove_empty_directories(const char *file)
@@ -2445,7 +2450,9 @@ static struct ref_lock *lock_ref_sha1_basic(const char
*refname,
goto error_return;
}
}
- return old_sha1 ? verify_lock(lock, old_sha1, mustexist) : lock;
+ if (old_sha1 verify_lock(lock, old_sha1, mustexist))
+ return NULL;
+ return lock;
error_return:
unlock_ref(lock);
--
2.1.4
--
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/5] verify_lock(): report errors via a strbuf
Instead of writing error messages directly to stderr, write them to a
strbuf *err. In lock_ref_sha1_basic(), arrange for these errors to
be returned to its caller.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
refs.c | 19 ---
1 file changed, 12 insertions(+), 7 deletions(-)
diff --git a/refs.c b/refs.c
index 100a767..625e69f 100644
--- a/refs.c
+++ b/refs.c
@@ -2198,23 +2198,28 @@ static void unlock_ref(struct ref_lock *lock)
/*
* Verify that the reference locked by lock has the value old_sha1.
* Fail if the reference doesn't exist and mustexist is set. Return 0
- * on success or a negative value on error. This function should make
- * sure errno is meaningful on error.
+ * on success. On error, write an error message to err, set errno, and
+ * return a negative value.
*/
static int verify_lock(struct ref_lock *lock,
- const unsigned char *old_sha1, int mustexist)
+ const unsigned char *old_sha1, int mustexist,
+ struct strbuf *err)
{
+ assert(err);
+
if (read_ref_full(lock-ref_name,
mustexist ? RESOLVE_REF_READING : 0,
lock-old_sha1, NULL)) {
int save_errno = errno;
- error(Can't verify ref %s, lock-ref_name);
+ strbuf_addf(err, Can't verify ref %s, lock-ref_name);
errno = save_errno;
return -1;
}
if (hashcmp(lock-old_sha1, old_sha1)) {
- error(Ref %s is at %s but expected %s, lock-ref_name,
- sha1_to_hex(lock-old_sha1), sha1_to_hex(old_sha1));
+ strbuf_addf(err, Ref %s is at %s but expected %s,
+ lock-ref_name,
+ sha1_to_hex(lock-old_sha1),
+ sha1_to_hex(old_sha1));
errno = EBUSY;
return -1;
}
@@ -2448,7 +2453,7 @@ static struct ref_lock *lock_ref_sha1_basic(const char
*refname,
goto error_return;
}
}
- if (old_sha1 verify_lock(lock, old_sha1, mustexist)) {
+ if (old_sha1 verify_lock(lock, old_sha1, mustexist, err)) {
last_errno = errno;
goto error_return;
}
--
2.1.4
--
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/RFC 0/3] using stat() to avoid re-scanning pack dir
On Fri, May 22, 2015 at 03:02:45PM +, steve.nor...@thomsonreuters.com wrote: On Friday, May 22, 2015 @ 11:06 AM Duy Nguyen did write: Strange. Maybe there is something else... Anyway some numbers from me. This is nfs3 hosted by Raspberry Pi, accessed over wireless. I just run index-pack on git.git pack instead of full clone. - v1.8.4.1 34s - v1.8.4.2 519s (oh.. wireless..) - v1.8.4.2 without has_sha1_file() in index-pack.c 33s - v1.8.4.2 + Jeff's mtime patch 36s Just ran the test again and it was 12 seconds. Too many versions of git available on the machine and I think I might have run the wrong one: Thanks for re-checking. Here's a series that fixes the rough edges of the patch I sent earlier. I'd appreciate it if you can re-confirm that it improves things for you. [1/3]: stat_validity: handle non-regular files [2/3]: cache.h: move stat_validity definition up [3/3]: prepare_packed_git: use stat_validity to avoid re-reading packs I'm adding Michael to the cc, as I'm abusing the stat_validity code which we worked on in 2013. My big concern here is that using stat_validity with a directory is racy. It works for a regular file like packed-refs because that file is replaced atomically. We fill the validity using fstat() on the same descriptor we read the data from, and nobody modifies an already-written file. So we know it matches what we read. For a directory, I don't think that atomicity guarantee is there. Somebody can modify the direct while we're reading. For the most part, I think that is OK. We fstat() before reading any entries, so our fstat data will then become out of date, and we'll re-read next time. It would be a problem if opendir() looked at the entries at all (e.g., if it called getdents() under Linux before our first readdir() call, then our fstat is happening after the first read, and wouldn't match what we read). I don't have any reason to believe that any libc does that, but it is making an assumption about how opendir() is implemented. The other problem is that I'm not sure stat data is enough to notice when a directory changes. Certainly the mtime should change, but if you have only one-second resolution on your mtimes, we can be fooled. For a regular file that is replaced atomically, the inode will change (and probably the size, too). But I don't know if that is the case for a directory. Can writing an entry go undetected between two stat() calls (or something even more pathological, like deleting one entry and writing another one)? -Peff -- 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 0/5] Fix verify_lock() to report errors via strbuf
On Fri, May 22, 2015 at 4:34 PM, Michael Haggerty mhag...@alum.mit.edu wrote: verify_lock() is a helper function called while committing reference transactions. But when it fails, instead of recording its error message in a strbuf to be passed back to the caller of ref_transaction_commit(), the error message was being written directly to stderr. Instead, report the errors via a strbuf so that they make it back to the caller of ref_transaction_commit(). The last two patches remove the capitalization of some error messages, to be consistent with our usual practice. These are a slight backwards incompatibility; if any scripts are trying to grep for these error message strings, they might have to be adjusted. So feel free to drop them if you think consistency across time is more important than consistency across commands. This is the patch series that I mentioned here [1]. It applies on top of mh/ref-directory-file [2] and is thematically a continuation of that series in the sense that it further cleans up the error handling within reference transactions. It would be easy to rebase to master if that is preferred. I was confused at first as I only looked at the patches and the corresponding line numbers did not match with the files as currently open in my editor. (they are roughly origin/master) Now that I read the cover letter I can explain the line number being slightly different. :) The series looks good to me. These patches are also available on my GitHub account [3] as branch verify-lock-strbuf-err. [1] http://article.gmane.org/gmane.comp.version-control.git/269006 [2] http://thread.gmane.org/gmane.comp.version-control.git/268778 [3] https://github.com/mhagger/git Michael Haggerty (5): verify_lock(): return 0/-1 rather than struct ref_lock * verify_lock(): on errors, let the caller unlock the lock verify_lock(): report errors via a strbuf verify_lock(): do not capitalize error messages ref_transaction_commit(): do not capitalize error messages refs.c| 40 ++-- t/t1400-update-ref.sh | 14 +++--- 2 files changed, 33 insertions(+), 21 deletions(-) -- 2.1.4 -- 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 v5 2/3] upload-pack: prepare to extend allow-tip-sha1-in-want
Fredrik Medley fredrik.med...@gmail.com writes: +#define ALLOW_TIP_SHA1 01 +static int allow_unadvertised_object_request; It is better to use unsigned int for these bit masks, as we are not interested in the top-most bit getting special-cased by using a signed type. I'll amend this (and the one in upload-pack.c) while applying, so no need to resend only to correct these two, unless you have other reasons to reroll. Sounds like a good idea to change. Please amend it while applying. Alright, will do. 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 v2 01/18] t1404: new tests of ref D/F conflicts within transactions
Michael Haggerty mhag...@alum.mit.edu writes: I think using 'grep' is OK for now, and if they are internationalized in the future the breakage will be pretty obvious and straightforward to fix. Yeah, I think so, too. 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 v5 3/3] upload-pack: optionally allow fetching reachable sha1
Fredrik Medley fredrik.med...@gmail.com writes: 2015-05-22 0:15 GMT+02:00 Junio C Hamano gits...@pobox.com: It looks like this new set of tests are well thought out; good job. I spotted a few minor nits, though. All I'll amend while applying so there is no need to resend only to correct them. I agree on all your comments and your proposed amendment further down looks good. Thanks. Should the test code contain the explanations you've written in this email? No, I don't think so. I was just showing that thinking aloud while reviewing would be a good way to do a review. The practice (1) makes sure that the reviewer actually understood what the patch wanted to do (and the reviewee can point out misunderstandings if there are any); and (2) shows others that the reviewer actually read the patch ;-). The latter is primarily meant for other people who review the patches. I want to see people get in the habit of responding with something more than just a single-liner Reviewed-by:, which I often have hard time guessing if the reviewer really read the patch, or just skimmed without spending effort to spot issues, and this message was my attempt to lead with an example. Will squash in the fix-up in the message you are responding to. Let's move the topic to 'next' after that. 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/RFC 0/3] using stat() to avoid re-scanning pack dir
On Sat, May 23, 2015 at 8:19 AM, Duy Nguyen pclo...@gmail.com wrote: But people often just do open operation of a time and this racy is not an issue. Very bad proof reading. This should read But people often do one operation at a time.. -- 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
[PATCH 1/3] stat_validity: handle non-regular files
The stat_validity code was originally written to avoid
re-reading the packed-refs file when it has not changed. It
makes sure that the file continues to match S_ISREG() when
we check it.
However, we can use the same concept on a directory to see
whether it has been modified. Even though we still have to
touch the filesystem to do the stat, this can provide a
speedup even over opendir/readdir/closedir, especially on
high-latency filesystems like NFS.
This patch adds a mode field to stat_validity, which lets
us check that the mode has stayed the same (rather than
explicitly checking that it is a regular file).
As a bonus cleanup, we can stop allocating the embedded
stat_data on the heap. Prior to this patch, we needed to
represent the case where the file did not exist, and we used
sv-sd == NULL for that. Now we can simply check that
sv-mode is 0.
Signed-off-by: Jeff King p...@peff.net
---
cache.h | 3 ++-
read-cache.c | 16 ++--
2 files changed, 8 insertions(+), 11 deletions(-)
diff --git a/cache.h b/cache.h
index c97d807..2941e7e 100644
--- a/cache.h
+++ b/cache.h
@@ -1660,7 +1660,8 @@ int sane_execvp(const char *file, char *const argv[]);
* for the index.
*/
struct stat_validity {
- struct stat_data *sd;
+ struct stat_data sd;
+ unsigned mode;
};
void stat_validity_clear(struct stat_validity *sv);
diff --git a/read-cache.c b/read-cache.c
index 36ff89f..115b000 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -2279,8 +2279,7 @@ void *read_blob_data_from_index(struct index_state
*istate, const char *path, un
void stat_validity_clear(struct stat_validity *sv)
{
- free(sv-sd);
- sv-sd = NULL;
+ memset(sv, 0, sizeof(*sv));
}
int stat_validity_check(struct stat_validity *sv, const char *path)
@@ -2288,21 +2287,18 @@ int stat_validity_check(struct stat_validity *sv, const
char *path)
struct stat st;
if (stat(path, st) 0)
- return sv-sd == NULL;
- if (!sv-sd)
- return 0;
- return S_ISREG(st.st_mode) !match_stat_data(sv-sd, st);
+ return sv-mode == 0;
+ return sv-mode == st.st_mode !match_stat_data(sv-sd, st);
}
void stat_validity_update(struct stat_validity *sv, int fd)
{
struct stat st;
- if (fstat(fd, st) 0 || !S_ISREG(st.st_mode))
+ if (fstat(fd, st) 0)
stat_validity_clear(sv);
else {
- if (!sv-sd)
- sv-sd = xcalloc(1, sizeof(struct stat_data));
- fill_stat_data(sv-sd, st);
+ sv-mode = st.st_mode;
+ fill_stat_data(sv-sd, st);
}
}
--
2.4.1.538.g69ac333
--
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] cache.h: move stat_validity definition up
It would be nice to embed stat_validity structs inside other
structs defined in cache.h. We cannot get away with a
forward declaration, because using it in a struct definition
means the compiler needs the real size.
Signed-off-by: Jeff King p...@peff.net
---
cache.h | 56
1 file changed, 28 insertions(+), 28 deletions(-)
diff --git a/cache.h b/cache.h
index 2941e7e..cdd279a 100644
--- a/cache.h
+++ b/cache.h
@@ -1185,6 +1185,34 @@ extern int has_dirs_only_path(const char *name, int len,
int prefix_len);
extern void schedule_dir_for_removal(const char *name, int len);
extern void remove_scheduled_dirs(void);
+/*
+ * A struct to encapsulate the concept of whether a file has changed
+ * since we last checked it. This uses criteria similar to those used
+ * for the index.
+ */
+struct stat_validity {
+ struct stat_data sd;
+ unsigned mode;
+};
+
+void stat_validity_clear(struct stat_validity *sv);
+
+/*
+ * Returns 1 if the path is a regular file (or a symlink to a regular
+ * file) and matches the saved stat_validity, 0 otherwise. A missing
+ * or inaccessible file is considered a match if the struct was just
+ * initialized, or if the previous update found an inaccessible file.
+ */
+int stat_validity_check(struct stat_validity *sv, const char *path);
+
+/*
+ * Update the stat_validity from a file opened at descriptor fd. If
+ * the file is missing, inaccessible, or not a regular file, then
+ * future calls to stat_validity_check will match iff one of those
+ * conditions continues to be true.
+ */
+void stat_validity_update(struct stat_validity *sv, int fd);
+
extern struct alternate_object_database {
struct alternate_object_database *next;
char *name;
@@ -1654,34 +1682,6 @@ int checkout_fast_forward(const unsigned char *from,
int sane_execvp(const char *file, char *const argv[]);
-/*
- * A struct to encapsulate the concept of whether a file has changed
- * since we last checked it. This uses criteria similar to those used
- * for the index.
- */
-struct stat_validity {
- struct stat_data sd;
- unsigned mode;
-};
-
-void stat_validity_clear(struct stat_validity *sv);
-
-/*
- * Returns 1 if the path is a regular file (or a symlink to a regular
- * file) and matches the saved stat_validity, 0 otherwise. A missing
- * or inaccessible file is considered a match if the struct was just
- * initialized, or if the previous update found an inaccessible file.
- */
-int stat_validity_check(struct stat_validity *sv, const char *path);
-
-/*
- * Update the stat_validity from a file opened at descriptor fd. If
- * the file is missing, inaccessible, or not a regular file, then
- * future calls to stat_validity_check will match iff one of those
- * conditions continues to be true.
- */
-void stat_validity_update(struct stat_validity *sv, int fd);
-
int versioncmp(const char *s1, const char *s2);
#endif /* CACHE_H */
--
2.4.1.538.g69ac333
--
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 0/3] using stat() to avoid re-scanning pack dir
On Sat, May 23, 2015 at 6:51 AM, Jeff King p...@peff.net wrote: The other problem is that I'm not sure stat data is enough to notice when a directory changes. Certainly the mtime should change, but if you have only one-second resolution on your mtimes, we can be fooled. mtime may or may not change. I based my untracked-cache series entirely on this directory mtime and did some research about it. For UNIXy filesystems on UNIXy OSes, mtime should work as you expect. FAT on Windows does not (but FAT on Linux does..). NTFS works fine according to some MS document. No idea about CIFS. But people often just do open operation of a time and this racy is not an issue. It is only an issue on the busy server side, and you can make sure you run on the right fs+OS. -- 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: [PATCH 2/2] log: do not shorten decoration names too early
On Fri, May 22, 2015 at 02:21:16PM -0700, Junio C Hamano wrote: I ended up doing it as a variant of the latter, free unless we have v-buffer pointing at it. Thanks, this version looks good to me minus one micro-nit below. Sorry for a long delay. No problem. I'm sometimes amazed you find time to write any patches at all. :) -- 8 -- Subject: [PATCH] commit-slab: introduce slabname##_peek() function There is no API to ask Does this commit have associated data in slab?. If an application wants to (1) parse just a few commits at the beginning of a process, (2) store data for only these commits, and then (3) start processing many commits, taking into account the data stored (for a few of them) in the slab, the application would use slabname##_at() to allocate a space to store data in (2), but there is no API other than slabname##_at() to use in step (3). This allocates and wasts new space for these commits the caller is only interested in checking if they have data stored in step (2). s/wasts/wastes/ -Peff -- 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 status doesn't ignore GIT_DIR directory?
On Sat, May 23, 2015 at 09:22:56AM +0700, Duy Nguyen wrote: On Fri, May 22, 2015 at 07:03:48PM -0700, Junio C Hamano wrote: As far as I know, GIT_DIR was prepared as a mechanism to point at another .git directory that does not live inside the working tree, not as a mechanism to rename it to arbitrary name. E.g. $ git init $ mv .git .svn $ GIT_DIR=$(pwd)/.svn ; export GIT_DIR is not expected to work. This is not the first time we get this report. Perhaps we should document it. -- 8 -- Subject: git.txt: mention the special name .git in GIT_DIR Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- diff --git a/Documentation/git.txt b/Documentation/git.txt index 90c5f37..91d6dd2 100644 --- a/Documentation/git.txt +++ b/Documentation/git.txt @@ -691,6 +691,9 @@ Git so take care if using Cogito etc. specifies a path to use instead of the default `.git` for the base of the repository. The '--git-dir' command-line option also sets this value. ++ +Note that if GIT_DIR is set explicitly to a directory inside working +tree, the directory name must be .git. Isn't the requirement that it _ends_ with .git (that is, GIT_DIR=/path/to/foo.git would work) Mike -- 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: Diffing submodule does not yield complete logs for merge commits
On Tuesday, 19 May 2015, Stefan Beller sbel...@google.com wrote: On Tue, May 19, 2015 at 12:29 PM, Robert Dailey rcdailey.li...@gmail.com wrote: How do you send your patches inline? This workflow discussion was a topic at the GitMerge2015 conference, and there are essentially 2 groups, those who know how to send email and those who complain about it. A solution was agreed on by nearly all of the contributors. It would be awesome to have a git-to-email proxy, such that you could do a git push proxy master:refs/for/mailinglist and this proxy would convert the push into sending patch series to the mailing list. It could even convert the following discussion back into comments (on Github?) but as a first step we'd want to try out a one way proxy. Unfortunately nobody stepped up to actually do the work, yet :( I've replied to this on a separate announcement thread on the Git mailing list here: http://thread.gmane.org/gmane.comp.version-control.git/269699 ...I've created a new tool called submitGit, which aims to help. I am willing to review the typical workflow for contributing via git on mailing lists but I haven't seen any informative reading material on this. I just find using command line to email patches and dealing with other issues not worth the trouble. Lack of syntax highlighting, lack of monospace font, the fact that I'm basically forced to install mail client software just to contribute a single git patch. I'd be interested to know what you think! Roberto -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Announce] submitGit for patch submission (was Diffing submodule does not yield complete logs)
Hi Roberto, On 2015-05-22 10:33, Roberto Tyley wrote: On Tuesday, 19 May 2015, Stefan Beller sbel...@google.com wrote: On Tue, May 19, 2015 at 12:29 PM, Robert Dailey rcdailey.li...@gmail.com wrote: How do you send your patches inline? [snip] This workflow discussion was a topic at the GitMerge2015 conference, and there are essentially 2 groups, those who know how to send email and those who complain about it. A solution was agreed on by nearly all of the contributors. It would be awesome to have a git-to-email proxy, such that you could do a git push proxy master:refs/for/mailinglist and this proxy would convert the push into sending patch series to the mailing list. It could even convert the following discussion back into comments (on Github?) but as a first step we'd want to try out a one way proxy. Unfortunately nobody stepped up to actually do the work, yet :( Hello, I'm stepping up to do that work :) Or at least, I'm implementing a one-way GitHub PR - Mailing list tool, called submitGit: https://submitgit.herokuapp.com/ Wow!!! I will make sure to test it with a couple of patches I want to submit anyway. Thanks so much! Dscho -- 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: Troubleshoot clone issue to NFS.
On Fri, May 22, 2015 at 3:35 PM, steve.nor...@thomsonreuters.com wrote: Tested this change on a couple of versions, first of all on the revision where things go wrong for me: ... ~ $ time git clone https://github.com/git/git test real0m8.263s user0m10.550s sys 0m3.763s ~ $ time git clone https://github.com/git/git /sami/test real1m3.286s user0m12.149s sys 0m9.192s So the patch isn't reducing the time taken when cloning to NAS. Strange. Maybe there is something else... Anyway some numbers from me. This is nfs3 hosted by Raspberry Pi, accessed over wireless. I just run index-pack on git.git pack instead of full clone. - v1.8.4.1 34s - v1.8.4.2 519s (oh.. wireless..) - v1.8.4.2 without has_sha1_file() in index-pack.c 33s - v1.8.4.2 + Jeff's mtime patch 36s -- 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: [PATCH v4] send-email: Add simple email aliases format
On Fri, May 22, 2015 at 12:29 AM, Eric Sunshine sunsh...@sunshineco.com wrote: On Thu, May 21, 2015 at 11:40 PM, Allen Hubbe alle...@gmail.com wrote: This format is more simple than the other alias file formats, so it may be preferred by some users. [...] Signed-off-by: Allen Hubbe alle...@gmail.com --- diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt index 804554609def..38ade31e0c28 100644 --- a/Documentation/git-send-email.txt +++ b/Documentation/git-send-email.txt @@ -383,7 +383,29 @@ sendemail.aliasesFile:: sendemail.aliasFileType:: Format of the file(s) specified in sendemail.aliasesFile. Must be - one of 'mutt', 'mailrc', 'pine', 'elm', or 'gnus'. + one of 'simple', 'mutt', 'mailrc', 'pine', 'elm', or 'gnus'. ++ +If the format is 'simple', then the alias file format is described below. +Descriptions of the other file formats to the following formats can be found in +the documentation of the email program of the same name. The second sentence probably needs some proof-reading. Could you be more specific? ++ +This 'simple' format is is as follows. ++ + alias: address|alias[, address|alias...] ++ +Aliases are specified one per line. There is no line splitting. Anything on a +line after and including a `#` symbol is considered a comment, and is ignored. +Blank lines are ignored. I'm not convinced that gratuitously diverging from the sendmail/postfix 'aliases' format is warranted. In particular, that This isn't 'sendmail', as of v2. format recognizes a comment line only when '#' is the first non-whitespace character[1]; and does not consider '#' a comment-introducer anywhere else in the line. By recognizing '#' anywhere as a comment-introducer, you may be painting this format into a corner rather than leaving it open for someone later to extend it to be more sendmail/postfix-like by, for instance, supporting name quoting and line-continuation[1]. It depends what we want to do with this parser: accept existing sendmail aliases files in git, or enforce that git alias files are usable for sendmail. I really don't expect the second to ever happen. The first, maybe, but only if the alias file is edited to remove aliases of pipes and maildirs etc. The second may not work if we have comments to the right, or aliases of aliases, which sendmail does not claim to support. I don't know what sendmail would actually do with a '#' elsewhere. It only talks about having '#' at the beginning of a line, or in the alias name in quotes (which is not supported by this parser - proper handling of quoted strings is not easy). It doesn't say what sendmail does with '#' if the name is not quoted, and it doesn't define a meaning for '#' in the definition of an alias. If these other cases would be errors for sendmail, so what if they are not errors here? For the same reason, I'm not convinced that simple is a good name. I was worried about that back in v1 before going to v2, but I really don't have a strong opinion about the name. I already changed the name, at the suggestion of Junio. I'd like to hear a consensus from you two, or a tiebreaker from a third, before I change it again. sendmail may indeed be a more appropriate name, even if it means that this early implementation documents it as (currently) a subset of the richer sendmail/postfix 'aliases' format. By doing so, we leave the door open so a future person can implement additional features to bring it closer to that format. Or, a future person can write a sendmail parser that is closer to that format. [1]: http://www.postfix.org/aliases.5.html ++ +Example of the 'simple' format: ++ + alice: Alice W Land a...@example.com + bob: Robert Bobbyton b...@example.com + # this is a comment + # this is also a comment + chloe: ch...@example.com + abgroup: alice, bob # comment after an alias + bcgrp: bob, chloe, Other o...@example.com -- 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/4] ref-filter: add ref-filter API
On 05/22/2015 12:14 PM, Matthieu Moy wrote:
karthik nayak karthik@gmail.com writes:
I miss a high-level description of what the code is doing. Essentially,
there's the complete repository list of refs, and you want to filter
only some of them, right?
From the name, I would guess that ref_filter is the structure describing
how you are filtering, but from the code it seems to be the list you're
filtering, not the filter.
Reading this again, A bit confused by what you're trying to imply.
Could you rephrase please?
At some point, I'd expect something like
filtered_list_of_refs = filer(full_list_of_refs, description_of_filter);
That would remove some refs from full_list_of_refs according to
description_of_filter.
(totally invented code, only to show the idea)
If there's a piece of code looking like this, then you need a data
structure to store list of refs (full_list_of_refs and
filtered_list_of_refs) and another to describe what you're doing with it
(description_of_filter).
The name ref_filter implies to me that it contains the description of
the filter, but looking at the code it doesn't seem to be the case.
But it does just that, doesn't it?
strict ref_filter {
int count, alloc;
struct ref_filter_item **items;
const char **name_patterns;
};
If you see it does contain 'name_patterns' according to which it will
filter the given refs, but thats just the start, as 'for-each-ref' only
supports filtering based on the given pattern, eventually as I merge the
functionality of 'git tag -l' and 'git branch -l' it will contain more
filters like, 'contains_commit', 'merged' and so on. Eventually becoming
more of a filter description as you put it. I hope that clears out things :)
Regards,
Karthik
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Announce] submitGit for patch submission (was Diffing submodule does not yield complete logs)
On 5/22/2015 10:33, Roberto Tyley wrote: Hello, I'm stepping up to do that work :) Or at least, I'm implementing a one-way GitHub PR - Mailing list tool, called submitGit: https://submitgit.herokuapp.com/ That's fantastic! Me being the one who brought up that topic at the Git Merge contributor's summit, I can say this sounds *very* much like the tool I've envisioned. Many thanks for that, I'll be sure to give it a try as soon as I can. Best regards, Sebastian -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Announce] submitGit for patch submission (was Diffing submodule does not yield complete logs)
Am 22.05.2015 um 10:33 schrieb Roberto Tyley:
[...]
Hello, I'm stepping up to do that work :) Or at least, I'm implementing a
one-way GitHub PR - Mailing list tool, called submitGit:
https://submitgit.herokuapp.com/
That looks really promising!
I wonder if that wouldn't make a good addition to github's repository ui
in general ?
Thanks,
Stefan
--
/dev/random says: Diplomacy: The patriotic art of lying for one's country.
python -c print
'73746566616e2e6e616577654061746c61732d656c656b74726f6e696b2e636f6d'.decode('hex')
GPG Key fingerprint = 2DF5 E01B 09C3 7501 BCA9 9666 829B 49C5 9221 27AF
--
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/14] pull: pass verbosity, --progress flags to fetch and merge
Hi Johannes, On Thu, May 21, 2015 at 11:59 PM, Johannes Schindelin johannes.schinde...@gmx.de wrote: Hi Paul, On 2015-05-21 11:48, Paul Tan wrote: Unfortunately, the usage of strbuf means that we lose the ability to know if an option was not provided at all (the value is NULL). This is important as some of these options are used to override the default configured behavior. Would this not be implied by the strbuf's len being 0? You're right . I think I need more coffee as well ;-) It would mean adding STRBUF_INIT's to all of the option variables, but I don't think it is a problem. strbufs it is, then. You might also want to verify that arg is `NULL` when `unset != 0`. Something like this: Hmm, would there be a situation where arg is NULL when `unset != 0`? I forgot to say that my suggestion was purely meant as defensive coding. Yes, `arg` *should* be `NULL` when `unset != 0`. Should ;-) By the way, just to make sure: My comments and suggestions are no demands; you should feel very free to reject them when you feel that your code is better without the suggested changes. I am just trying to be helpful. Thanks, your comments have been really helpful. I really do appreciate it :). Regards, Paul -- 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
Query on git submodules
Hi there I am a design automation engineer supporting 200+ designers who use git for hardware design. We also use the submodule feature where we can have quite complex hierarchy's with 10+ layers. We have experience issues with re-use of design projects was we move from one derivative to another due to the complexity of the hierarchy along with lack of discipline (using absolute paths in .gitmodule files). To enforce more discipline I am currently working on a pre-commit hook to check the integrity of .gitmodule files. I would be interested in seeing how other users in the community find submodules for large scale projects and if there are any best known methods for using them. Operating system (specifically which version) - Suse Linux (SLES11SP2) Git version (git --version) - 1.7.12.2 Thanks, Sarah - Intel Ireland Limited (Branch) Collinstown Industrial Park, Leixlip, County Kildare, Ireland Registered Number: E902934 This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies. -- 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/4] ref-filter: add ref-filter API
On 05/22/2015 12:10 AM, Eric Sunshine wrote: On Thu, May 21, 2015 at 1:30 PM, karthik nayak karthik@gmail.com wrote: On 05/21/2015 12:37 AM, Eric Sunshine wrote: On Wed, May 20, 2015 at 9:18 AM, Karthik Nayak karthik@gmail.com wrote: Makefile | 1 + ref-filter.c | 73 ref-filter.h | 47 ++ 3 files changed, 121 insertions(+) create mode 100644 ref-filter.c create mode 100644 ref-filter.h A shortcoming of this approach is that it's not blame-friendly. Although those of us following this patch series know that much of the code in this patch was copied from for-each-ref.c, git-blame will not recognize this unless invoked in the very expensive git blame -C -C -C fashion (if I understand correctly). The most blame-friendly way to perform this re-organization is to have the code relocation (line removals and line additions) occur in one patch. There are multiple ways you could arrange to do so. One would be to first have a patch which introduces just a skeleton of the intended API, with do-nothing function implementations. A subsequent patch would then relocate the code from for-each-ref.c to ref-filter.c, and update for-each-ref.c to call into the new (now fleshed-out) API. Did you read Junio's suggestion on how I should re-order this WIP patch series ? That's somewhat on the lines of what you're suggesting. I'll probably be going ahead with that, not really sure about how blame works entirely so what do you think about that? Yes, Junio's response did a much better job of saying what I intended. Also, his response said something I meant to mention but forgot: namely that, to ease the review task, code movement should be pure movement, and not involve other changes. Anyhow, follow Junio's advice. He knows what he's talking about. ;-) Alright, Thanks for clearing that out. Regards, Karthik -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] send-email: Add simple email aliases format
On Thu, May 21, 2015 at 11:59 PM, Eric Sunshine sunsh...@sunshineco.com wrote:
On Thu, May 21, 2015 at 11:19 PM, Allen Hubbe alle...@gmail.com wrote:
On May 21, 2015 9:05 PM, Eric Sunshine sunsh...@sunshineco.com wrote:
On Thu, May 21, 2015 at 8:16 PM, Allen Hubbe alle...@gmail.com wrote:
+test_expect_success $PREREQ 'sendemail.aliasfiletype=simple' '
+ clean_fake_sendmail rm -fr outdir
+ git format-patch -1 -o outdir
+ {
+ echo alice: Alice W Land a...@example.com
+ echo bob: Robert Bobbyton b...@example.com
+ echo chloe: ch...@example.com
+ echo abgroup: alice, bob
+ echo bcgrp: bob, chloe, Other o...@example.com
+ } ~/.tmp-email-aliases
A here-doc would be easier to maintain and read:
A here-doc does not flow nicely in an indented block. Each line in
That's true if you use EOF here-doc, but not for -EOF, as I did in
the example. With -EOF, all leading tabs are stripped from the input
lines, including from the EOF line, which is why it can be indented to
the same level as the other code in the test. The added '\' in -\EOF
from my example indicates that you don't want/expect any interpolation
inside the here-doc. The -\EOF form is used extensively throughout
the Git test suite.
Alright.
--
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
[BUG] git commit --date format parsing
Hi * Trying to specify a commit (author) date using `--date` option yields unpredictable results that are incoherent with man git-commit: $ git --version git version 2.4.1 $ uname -a Linux arch-clevo 4.0.4-1-ARCH #1 SMP PREEMPT Mon May 18 06:43:19 CEST 2015 x86_64 GNU/Linux $ mkdir test cd test/ $ git init $ touch test $ git add test 1. ISO 8601 (strict) $ git commit --date=2015-05-21T16∶31+02:00 -m Test commit to check date format parsing [master (root commit) fed9ae6] Test commit to check date format parsing Date: Thu May 21 02:00:00 2015 +0200 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 test -- gets the date right but confuses the timezone for the time 2. git-log --date=iso8601 format: $ git commit --amend --date=2015-05-21 16∶31 +0200 -m Test commit to check date format parsing [master d2cdbf2] Test commit to check date format parsing Date: Thu May 21 14:37:37 2015 +0200 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 test -- gets the date right but uses current time, not specified one 3. date format git uses in output of commit command: $ git commit --amend --date=Thu May 21 16∶31 2015 +0200 ... Date: Sat May 21 14:40:08 2016 +0200 -- get the day and month right but not the year, uses current time 4. RFC 2822 $ git commit --amend --date=Thu, 21 May 2015 16∶31 +0200 ... Date: Thu May 21 15:01:03 2015 +0200 -- gets the date right but uses current time 5. Environment variable with ISO 8601 (strict) $ GIT_AUTHOR_DATE=2015-05-21T16∶31+02:00 git commit --amend ... Date: Thu May 21 15:04:30 2015 +0200 -- using the env var we get something better than 1. (not confusing timezone for time) but still not the specified date. Seeing the discussions there have been here around date parsing and ISO 8601 [1][2], I suggest only supporting the W3C’s suggested profile of ISO 8601 [3] to cut in the complexity. My use case for using the --date option to git-commit is to reconstruct the revision history of a set of files that were timestamped with YAML `date:` metadata, so as to see which files were added after which others etc. I was hoping to use a script to parse the YAML datetime metadata in git commit, but right now the time information would be lost, which is problematic. Besides this, documentation for git-commit is currently uncorrect since it suggests we can use RFC 2822 and ISO 8601 while this seems not to be the case. Thanks for your feedback, Bastien [1] http://thread.gmane.org/gmane.comp.version-control.git/256109 [2] http://thread.gmane.org/gmane.comp.version-control.git/52414/focus=52597 [3] http://www.w3.org/TR/NOTE-datetime -- 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] pull: handle --log=n
Hi Junio, On Fri, May 22, 2015 at 5:24 AM, Junio C Hamano gits...@pobox.com wrote: Paul Tan pyoka...@gmail.com writes: So, here's the re-rolled patch. Sigh, too late. I thought the previous round was good enough and the patch is already on 'next'. If the incremental change is still worth doing on top, please do so. Thanks. My bad, I should have checked 'next'. But it's okay, I re-rolled the patch because it seemed that people preferred test_commit. I personally don't have a strong opinion on this, so if you're fine with it then I'm okay as well. Thanks, Paul -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] rebase -i: fix post-rewrite hook with failed exec command
Usually, when 'git rebase' stops before completing the rebase, it is to
give the user an opportunity to edit a commit (e.g. with the 'edit'
command). In such cases, 'git rebase' leaves the sha1 of the commit being
rewritten in $state_dir/stopped-sha, and subsequent 'git rebase
--continue' will call the post-rewrite hook with this sha1 as old-sha1
argument to the post-rewrite hook.
The case of 'git rebase' stopping because of a failed 'exec' command is
different: it gives the opportunity to the user to examine or fix the
failure, but does not stop saying here's a commit to edit, use
--continue when you're done. So, there's no reason to call the
post-rewrite hook for 'exec' commands. If the user did rewrite the
commit, it would be with 'git commit --amend' which already called the
post-rewrite hook.
Fix the behavior to leave no stopped-sha file in case of failed exec
command, and teach 'git rebase --continue' to skip record_in_rewritten if
no stopped-sha file is found.
---
git-rebase--interactive.sh | 10 +-
t/t5407-post-rewrite-hook.sh | 2 +-
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 08e5d86..1c321e4 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -486,7 +486,7 @@ do_pick () {
}
do_next () {
- rm -f $msg $author_script $amend || exit
+ rm -f $msg $author_script $amend $state_dir/stopped-sha || exit
read -r command sha1 rest $todo
case $command in
$comment_char*|''|noop)
@@ -576,9 +576,6 @@ do_next () {
read -r command rest $todo
mark_action_done
printf 'Executing: %s\n' $rest
- # exec command doesn't take a sha1 in the todo-list.
- # = can't just use $sha1 here.
- git rev-parse --verify HEAD $state_dir/stopped-sha
${SHELL:-@SHELL_PATH@} -c $rest # Actual execution
status=$?
# Run in subshell because require_clean_work_tree can die.
@@ -874,7 +871,10 @@ first and then run 'git rebase --continue' again.
fi
fi
- record_in_rewritten $(cat $state_dir/stopped-sha)
+ if test -r $state_dir/stopped-sha
+ then
+ record_in_rewritten $(cat $state_dir/stopped-sha)
+ fi
require_clean_work_tree rebase
do_rest
diff --git a/t/t5407-post-rewrite-hook.sh b/t/t5407-post-rewrite-hook.sh
index 53a4062..06ffad6 100755
--- a/t/t5407-post-rewrite-hook.sh
+++ b/t/t5407-post-rewrite-hook.sh
@@ -212,7 +212,7 @@ EOF
verify_hook_input
'
-test_expect_failure 'git rebase -i (exec)' '
+test_expect_success 'git rebase -i (exec)' '
git reset --hard D
clear_hook_input
FAKE_LINES=edit 1 exec_false 2 git rebase -i B
---
https://github.com/git/git/pull/138
[PATCH 1/2] rebase -i: demonstrate incorrect behavior of post-rewrite
The 'exec' command is sending the current commit to stopped-sha, which is supposed to contain the original commit (before rebase). As a result, if an 'exec' command fails, the next 'git rebase --continue' will send the current commit as old-sha1 to the post-rewrite hook. The test currently fails with : --- expected.data 2015-05-21 17:55:29.0 + +++ [...]post-rewrite.data 2015-05-21 17:55:29.0 + @@ -1,2 +1,3 @@ 2362ae8e1b1b865e6161e6f0e165ffb974abf018 488028e9fac0b598b70cbeb594258a917e3f6fab +488028e9fac0b598b70cbeb594258a917e3f6fab 488028e9fac0b598b70cbeb594258a917e3f6fab babc8a4c7470895886fc129f1a015c486d05a351 8edffcc4e69a4e696a1d4bab047df450caf99507 --- t/t5407-post-rewrite-hook.sh | 17 + 1 file changed, 17 insertions(+) diff --git a/t/t5407-post-rewrite-hook.sh b/t/t5407-post-rewrite-hook.sh index ea2e0d4..53a4062 100755 --- a/t/t5407-post-rewrite-hook.sh +++ b/t/t5407-post-rewrite-hook.sh @@ -212,4 +212,21 @@ EOF verify_hook_input ' +test_expect_failure 'git rebase -i (exec)' ' + git reset --hard D + clear_hook_input + FAKE_LINES=edit 1 exec_false 2 git rebase -i B + echo something bar + git add bar + # Fails because of exec false + test_must_fail git rebase --continue + git rebase --continue + echo rebase expected.args + cat expected.data EOF +$(git rev-parse C) $(git rev-parse HEAD^) +$(git rev-parse D) $(git rev-parse HEAD) +EOF + verify_hook_input +' + test_done --- https://github.com/git/git/pull/138
Re: [PATCH 14/14] pull --rebase: error on no merge candidate cases
On Wed, May 20, 2015 at 12:27 AM, Junio C Hamano gits...@pobox.com wrote: Johannes Schindelin johannes.schinde...@gmx.de writes: - fprintf(stderr, - _(There are no candidates for merging among the refs that you just fetched.\n - Generally this means that you provided a wildcard refspec which had no\n - matches on the remote end.\n)); + if (opt_rebase) + fputs(_(There is no candidate for rebasing against among the refs that you just fetched.), stderr); The puts() function appends a newline while fputs() does not. Yup, so this update makes the command spew unterminated lines, which not something intended... Ugh Will put the \n back. And yes, I used fputs() because it seems wasteful to use fprintf() which will scan the string looking for any '%' formatting units, when we know there aren't. I will also update 05/14 to use fputs() as well where appropriate. Thanks, Paul -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] rebase -i: demonstrate incorrect behavior of post-rewrite
Matthieu Moy matthieu@imag.fr writes: The 'exec' command is sending the current commit to stopped-sha, which is supposed to contain the original commit (before rebase). As a result, if an 'exec' command fails, the next 'git rebase --continue' will send the current commit as old-sha1 to the post-rewrite hook. The test currently fails with : --- expected.data 2015-05-21 17:55:29.0 + +++ [...]post-rewrite.data 2015-05-21 17:55:29.0 + @@ -1,2 +1,3 @@ 2362ae8e1b1b865e6161e6f0e165ffb974abf018 488028e9fac0b598b70cbeb594258a917e3f6fab +488028e9fac0b598b70cbeb594258a917e3f6fab 488028e9fac0b598b70cbeb594258a917e3f6fab babc8a4c7470895886fc129f1a015c486d05a351 8edffcc4e69a4e696a1d4bab047df450caf99507 Indent displayed material like the above a bit, please. And please sign-off your patches. --- t/t5407-post-rewrite-hook.sh | 17 + 1 file changed, 17 insertions(+) diff --git a/t/t5407-post-rewrite-hook.sh b/t/t5407-post-rewrite-hook.sh index ea2e0d4..53a4062 100755 --- a/t/t5407-post-rewrite-hook.sh +++ b/t/t5407-post-rewrite-hook.sh @@ -212,4 +212,21 @@ EOF verify_hook_input ' +test_expect_failure 'git rebase -i (exec)' ' + git reset --hard D + clear_hook_input + FAKE_LINES=edit 1 exec_false 2 git rebase -i B + echo something bar + git add bar + # Fails because of exec false + test_must_fail git rebase --continue + git rebase --continue + echo rebase expected.args + cat expected.data EOF +$(git rev-parse C) $(git rev-parse HEAD^) +$(git rev-parse D) $(git rev-parse HEAD) +EOF By using a dash to start the here-document like this: cat expect -\EOF $(git rev-parse C) $(git rev-parse HEAD^) ... EOF you can tab-indent the contents and the end marker at the same level to make it easier to read. + verify_hook_input +' + test_done --- https://github.com/git/git/pull/138 -- 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] submodule documentation: Reorder introductory paragraphs
Stefan Beller sbel...@google.com writes: On Thu, May 21, 2015 at 1:03 PM, Philip Oakley philipoak...@iee.org wrote: +Submodules are not to be confused with remotes, which are meant +mainly for branches of the same project; This use of 'branches' didn't work for me. remotes are meant mainly for branches of the same project ? The branch in the original is used in a much wider sense than usual branch (i.e. ref/heads/ thing you have locally); it refers to forks of the same project but with a bit of twist. When you say repository A is a fork of the same project as my local repository, you would give an impression that A is not the authoritative copy of the project. But you can say my repository and that repository A are branches of the same project, you give zero information as to A's authoritativeness. Submodules should not be confused with remote repositories, which are meant to track the same repository, just at another location; ... I do not think this is a great improvement. You now conflated repository to mean project in the latter half of the sentence, while you are trying to explain what a remote repository is. Your copy of git.git is not the same repository as mine; they have different histories. Both repositories are used to work on the same project. submoules are not remotes, which are other repositories of the same project, perhaps? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] rebase -i: demonstrate incorrect behavior of post-rewrite
Junio C Hamano gits...@pobox.com writes: Matthieu Moy matthieu@imag.fr writes: The 'exec' command is sending the current commit to stopped-sha, which is supposed to contain the original commit (before rebase). As a result, if an 'exec' command fails, the next 'git rebase --continue' will send the current commit as old-sha1 to the post-rewrite hook. The test currently fails with : --- expected.data 2015-05-21 17:55:29.0 + +++ [...]post-rewrite.data 2015-05-21 17:55:29.0 + @@ -1,2 +1,3 @@ 2362ae8e1b1b865e6161e6f0e165ffb974abf018 488028e9fac0b598b70cbeb594258a917e3f6fab +488028e9fac0b598b70cbeb594258a917e3f6fab 488028e9fac0b598b70cbeb594258a917e3f6fab babc8a4c7470895886fc129f1a015c486d05a351 8edffcc4e69a4e696a1d4bab047df450caf99507 Indent displayed material like the above a bit, please. OK, will do. And please sign-off your patches. Ah, I was testing submitGit, and forgot that send-email was usually doing this for me. +cat expected.data EOF +$(git rev-parse C) $(git rev-parse HEAD^) +$(git rev-parse D) $(git rev-parse HEAD) +EOF By using a dash to start the here-document like this: cat expect -\EOF $(git rev-parse C) $(git rev-parse HEAD^) ... EOF you can tab-indent the contents and the end marker at the same level to make it easier to read. I usually do that but I just mimicked the surrounding code for consistency. If you really prefer the -\EOF I can resend with an additional modernize style patch before and this one properly formatted. -- 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: Troubleshoot clone issue to NFS.
On Friday, May 22, 2015 @ 11:06 AM Duy Nguyen did write: Strange. Maybe there is something else... Anyway some numbers from me. This is nfs3 hosted by Raspberry Pi, accessed over wireless. I just run index-pack on git.git pack instead of full clone. - v1.8.4.1 34s - v1.8.4.2 519s (oh.. wireless..) - v1.8.4.2 without has_sha1_file() in index-pack.c 33s - v1.8.4.2 + Jeff's mtime patch 36s Just ran the test again and it was 12 seconds. Too many versions of git available on the machine and I think I might have run the wrong one: ~ $ time bin/git clone https://github.com/git/git test Cloning into 'test'... remote: Counting objects: 186015, done. remote: Compressing objects: 100% (8/8), done. remote: Total 186015 (delta 8), reused 7 (delta 7), pack-reused 186000 Receiving objects: 100% (186015/186015), 61.33 MiB | 30.35 MiB/s, done. Resolving deltas: 100% (135512/135512), done. Checking connectivity... done. real0m6.931s user0m10.011s sys 0m2.685s ~ $ time bin/git clone https://github.com/git/git /sami/test Cloning into '/sami/test'... remote: Counting objects: 186015, done. remote: Compressing objects: 100% (8/8), done. remote: Total 186015 (delta 8), reused 7 (delta 7), pack-reused 186000 Receiving objects: 100% (186015/186015), 61.33 MiB | 29.91 MiB/s, done. Resolving deltas: 100% (135512/135512), done. Checking connectivity... done. Checking out files: 100% (2795/2795), done. real0m13.169s user0m9.961s sys 0m3.480s So I would say that was in line with the timings for the other versions I have tested. And looking back at the email from the morning I did run the wrong bin/git. Sorry, totally my fault on that. If I can test anything else then let me know. Thanks, Steve --
Re: [Announce] submitGit for patch submission (was Diffing submodule does not yield complete logs)
Hi Roberto, On 2015-05-22 11:42, Johannes Schindelin wrote: On 2015-05-22 10:33, Roberto Tyley wrote: On Tuesday, 19 May 2015, Stefan Beller sbel...@google.com wrote: On Tue, May 19, 2015 at 12:29 PM, Robert Dailey rcdailey.li...@gmail.com wrote: How do you send your patches inline? [snip] This workflow discussion was a topic at the GitMerge2015 conference, and there are essentially 2 groups, those who know how to send email and those who complain about it. A solution was agreed on by nearly all of the contributors. It would be awesome to have a git-to-email proxy, such that you could do a git push proxy master:refs/for/mailinglist and this proxy would convert the push into sending patch series to the mailing list. It could even convert the following discussion back into comments (on Github?) but as a first step we'd want to try out a one way proxy. Unfortunately nobody stepped up to actually do the work, yet :( Hello, I'm stepping up to do that work :) Or at least, I'm implementing a one-way GitHub PR - Mailing list tool, called submitGit: https://submitgit.herokuapp.com/ Wow!!! I will make sure to test it with a couple of patches I want to submit anyway. I just tried this with https://github.com/git/git/pull/139 and would like to tell you about two wishes I had immediately: - If the author of a patch I am submitting is not myself, could submitGit maybe add that `From: ` line at the top of the mail? - The patch series is sent without a cover letter, but it would be *really* great if a path series consisting of more than one patch could have the initial comment of the Pull Request as a cover letter, with the link to the original Pull Request at the bottom? This would also be the mail to use in the In-reply-yo header instead of the first patch. Thanks so much! Dscho -- 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
Pushing and pulling the result of `git replace` and objects/info/alternates
Hello, I have an 'integration repo' which contains other git repos as submodules. One of the submodules is to be split in two to extract a library. A common way of doing that is to use git-filter-branch. A disadvantage of that is that it results in duplicated partial-history in the extracted repo. So, git log shows the entire history, but there is not one canonical sha which represents the history at that point. The split repo will contain 'false history', and checking it out will not be useful. So, I want to avoid git-filter-branch. I have tried out using `git replace --graft` and .git/objects/info/alternates to 'refer to' the history in the origin repo instead of 'duplicating' it. This is similar to how Qt5 repos refer to Qt 4 history in a different repo. Question 1) Is this a reasonable thing to do for this scenario? Question 2) Is there a way to push the `git replace` result and the `info/alternates` content so that clients cloning the 'integration repo' do not have to do that 'manually' or with a 'setup-repo.sh' script? The sequence of commands below can be pasted into a tmp directory to see the scenario in action. Thanks! mkdir calculator cd calculator mkdir mainui libcalc echo print \hello\ mainui/app.py echo print \hello\ libcalc/adder.py echo print \hello\ libcalc/subtracter.py git init git add . git commit -am Initial commit git checkout `git rev-parse HEAD` cd .. mkdir appsuite cd appsuite git init git submodule add ../calculator git commit -m Add calculator submodule # Add other submodules in the suite... cd calculator echo print \goodbye\ libcalc/subtracter.py git add libcalc/subtracter.py git commit -am Fix bug in subtracter echo print \Hi\ libcalc/adder.py git add libcalc/adder.py git commit -am Make adder more efficient echo print \Hello, world!\ mainui/app.py git add mainui/app.py git commit -am Improve app echo print \hello, hello\ libcalc/multiplier.py git add libcalc/multiplier.py git commit -am Add multiplier cd .. git add calculator git commit -m Update calculator submodule mkdir compute cd calculator mv libcalc ../compute extraction_sha=`git rev-parse HEAD` git commit -am Remove libcalc from calculator repo -m It is moved to a new compute repo removal_sha=`git rev-parse HEAD` git push cd ../compute git init git add . git commit -m Create the compute repo. -m This commit will not be normally visible after the replace --graft below. echo This is the compute framework. It contains the libcalc library. README git add README git commit -m Initialize the compute repo. -m This has been extracted from calculator.git at $removal_sha git checkout `git rev-parse HEAD` cd .. mv compute .. git submodule add ../compute git add calculator compute git commit -m Split compute framework out of calculator repo. cd compute git log --oneline # We don't see older history from the calculator repo # Let's add alternates echo ../../calculator/objects ../.git/modules/compute/objects/info/alternates # ... and graft onto the extraction commit git replace --graft HEAD $extraction_sha git log --oneline # Great, now we see history from the calculator repo. cd ../.. git clone appsuite appsuite-clone cd appsuite-clone git submodule update --init cd compute ls ../.git/modules/compute/objects/info git log --oneline # The replacement and alternatives did not get cloned ... :( echo ../../calculator/objects ../.git/modules/compute/objects/info/alternates git replace --graft HEAD $extraction_sha # And now we see the history from the calculator repo. Great. But, it required user action after the clone. -- 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 14/14] pull --rebase: error on no merge candidate cases
Hi Paul, On 2015-05-22 15:48, Paul Tan wrote: On Wed, May 20, 2015 at 12:27 AM, Junio C Hamano gits...@pobox.com wrote: Johannes Schindelin johannes.schinde...@gmx.de writes: - fprintf(stderr, - _(There are no candidates for merging among the refs that you just fetched.\n - Generally this means that you provided a wildcard refspec which had no\n - matches on the remote end.\n)); + if (opt_rebase) + fputs(_(There is no candidate for rebasing against among the refs that you just fetched.), stderr); The puts() function appends a newline while fputs() does not. Yup, so this update makes the command spew unterminated lines, which not something intended... Ugh Will put the \n back. And yes, I used fputs() because it seems wasteful to use fprintf() which will scan the string looking for any '%' formatting units, when we know there aren't. I will also update 05/14 to use fputs() as well where appropriate. I believe the common thinking is that consistency beats speed in error messages, so it would be easier to read and maintain the code if all of those error messages were just using `fprintf(stderr, ...);`. It's not as if we spit out hundreds of thousands of error messages per second where that `%s` parsing would hurt ;-) Ciao, Dscho -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Announce] submitGit for patch submission (was Diffing submodule does not yield complete logs)
Roberto Tyley roberto.ty...@gmail.com writes: Hello, I'm stepping up to do that work :) Or at least, I'm implementing a one-way GitHub PR - Mailing list tool, called submitGit: https://submitgit.herokuapp.com/ Yay ;-) Here's what a user does: * create a PR on https://github.com/git/git * logs into https://submitgit.herokuapp.com/ with GitHub auth * selects their PR on https://submitgit.herokuapp.com/git/git/pulls Reasonable. * gets submitGit to email the PR as patches to themselves, in order to check it looks ok I can see you are trying to be careful by doing this, but I am not sure if this step would actually help. Those who are not familiar with Git development are not expected to know what is ok in their original commit, and if they find bad formatting done by submitGit (e.g. adds their PR message before the three-dash line instead of after it), they cannot do much about it anyway. * when they're ready, get submitGit to send it to the mailing list on their behalf Nice. All discussion of the patch *stays* on the mailing list Can you identify a reroll of an earlier submission? If you can use the in-reply-to and make it a follow-up to the previous round, that would be great. -- 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: Troubleshoot clone issue to NFS.
Duy Nguyen pclo...@gmail.com writes: Strange. Maybe there is something else... Anyway some numbers from me. This is nfs3 hosted by Raspberry Pi, accessed over wireless. I just run index-pack on git.git pack instead of full clone. So the checkout codepath to touch working tree is one difference? Are there other differences? - v1.8.4.1 34s - v1.8.4.2 519s (oh.. wireless..) - v1.8.4.2 without has_sha1_file() in index-pack.c 33s - v1.8.4.2 + Jeff's mtime patch 36s -- 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 v4] send-email: Add simple email aliases format
Allen Hubbe alle...@gmail.com writes: It depends what we want to do with this parser: accept existing sendmail aliases files in git, or enforce that git alias files are usable for sendmail. I really don't expect the second to ever happen. The first, maybe, but only if the alias file is edited to remove aliases of pipes and maildirs etc. The second may not work if we have comments to the right, or aliases of aliases, which sendmail does not claim to support. Let me step back a bit. Earlier you said your aim is not to use an alias file you already have and use with the MUA/MTA, but to have a collection of aliases to use with git-send-email only. Is there a reason to add support for a new format (whether it is compatible to or subset of postfix/sendmail format, or a totally new one) for that goal? What makes the existing formats unsuitable? -- 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 v4] send-email: Add simple email aliases format
On Fri, May 22, 2015 at 10:44 AM, Junio C Hamano gits...@pobox.com wrote: Allen Hubbe alle...@gmail.com writes: It depends what we want to do with this parser: accept existing sendmail aliases files in git, or enforce that git alias files are usable for sendmail. I really don't expect the second to ever happen. The first, maybe, but only if the alias file is edited to remove aliases of pipes and maildirs etc. The second may not work if we have comments to the right, or aliases of aliases, which sendmail does not claim to support. Let me step back a bit. Earlier you said your aim is not to use an alias file you already have and use with the MUA/MTA, but to have a collection of aliases to use with git-send-email only. Is there a reason to add support for a new format (whether it is compatible to or subset of postfix/sendmail format, or a totally new one) for that goal? What makes the existing formats unsuitable? It's just a matter of personal preference what is suitable or not, for me, in my environment, etc. Is there a reason I should use the alias format of some email client, if I don't use that email client? I'm not trying to force anything on anyone else by offering this, just another option that might be suitable for someone else, in their environment, as it is in mine. People who don't like it can choose a different option. People who don't like any of the options can write their own like I did, or is that not allowed for some reason? I've already shown that I am willing to change the name, write the documentation, write the tests, modify the syntax, and so on. I've done the work, from +6 lines to +57 lines, as requested. I'm not looking forward to v5, v6... v10 of what was a really really simple patch. If you don't like it, please don't string me along. This is not my job. If you think the patch is generally ok, but could be improved to be accepted, then let's together try to make v5 the last. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] rebase -i: demonstrate incorrect behavior of post-rewrite
Matthieu Moy matthieu@grenoble-inp.fr writes: And please sign-off your patches. Ah, I was testing submitGit, and forgot that send-email was usually doing this for me. Ah, should have noticed from the message-id. Roberto, isn't your threading of multi-patch series busted? Why is 1/2 a follow-up to 2/2? Do you have a time-machine ;-)? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] test_bitmap_walk: free bitmap with bitmap_free
On Thu, May 21, 2015 at 5:53 PM, Jeff King p...@peff.net wrote: Commit f86a374 (pack-bitmap.c: fix a memleak, 2015-03-30) noticed that we leak the result bitmap. But we should use bitmap_free rather than straight free, as the former remembers to free the bitmap array pointed to by the struct. Signed-off-by: Jeff King p...@peff.net --- Sorry, I should have noticed this when reviewing the original. No need to be sorry, me too. This looks good to me. pack-bitmap.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pack-bitmap.c b/pack-bitmap.c index 62a98cc..d64bd94 100644 --- a/pack-bitmap.c +++ b/pack-bitmap.c @@ -987,7 +987,7 @@ void test_bitmap_walk(struct rev_info *revs) else fprintf(stderr, Mismatch!\n); - free(result); + bitmap_free(result); } static int rebuild_bitmap(uint32_t *reposition, -- 2.4.1.528.g00591e3 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] doc: fix inconsistent spelling of packfile
Patrick Steinhardt p...@pks.im writes: Fix remaining instances where pack-file is used instead of packfile. Some places remain where we still use pack-file, This is the case when we explicitly refer to a file with a .pack extension as opposed to a data source providing a pack data stream. 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 v4] send-email: Add simple email aliases format
On Fri, May 22, 2015 at 8:12 AM, Allen Hubbe alle...@gmail.com wrote: On Fri, May 22, 2015 at 12:29 AM, Eric Sunshine sunsh...@sunshineco.com wrote: On Thu, May 21, 2015 at 11:40 PM, Allen Hubbe alle...@gmail.com wrote: +If the format is 'simple', then the alias file format is described below. +Descriptions of the other file formats to the following formats can be found in +the documentation of the email program of the same name. The second sentence probably needs some proof-reading. Could you be more specific? Unable to parse of the other file formats to the following formats. I'm guessing that the to the following formats portion doesn't belong. It depends what we want to do with this parser: accept existing sendmail aliases files in git, or enforce that git alias files are usable for sendmail. Aside from these possibilities (likely or unlikely), there is also the issue of breaking expectations. Precedence for this style 'aliases' format was set decades ago by sendmail. People are familiar with it and understand its strengths and weaknesses. Even if documented as not being sendmail-compatible, because it's so similar to sendmail 'aliases', people will expect it to work the same way, thus unless there's a good reason to diverge from that standard format, it makes sense to be compatible with it (even if only as a proper subset). I really don't expect the second to ever happen. The first, maybe, but only if the alias file is edited to remove aliases of pipes and maildirs etc. The second may not work if we have comments to the right, or aliases of aliases, which sendmail does not claim to support. It's not clear why you say that sendmail does not claim to support aliases of aliases. Although it's true that some sources, such as [1], fail to mention support explicitly, other more authoritative sources do[2]. Moreover, the 1993 sendmail book by Bryan Costales, with contributions from Eric Allman (the creator of sendmail), talks explicitly about expansion of aliases on the right-hand-side. Finally, since time immemorial (at least the early 1980's), every /etc/aliases file has contained the following mandatory entries: postmaster: root MAILER-DAEMON: postmaster which indicates clearly that alias expansion on the RHS is supported. [1]: http://www.feep.net/sendmail/tutorial/intro/aliases.html [2]: https://www.freebsd.org/cgi/man.cgi?query=aliasessektion=5 I don't know what sendmail would actually do with a '#' elsewhere. It only talks about having '#' at the beginning of a line, or in the alias name in quotes (which is not supported by this parser - proper handling of quoted strings is not easy). It doesn't say what sendmail does with '#' if the name is not quoted, and it doesn't define a meaning for '#' in the definition of an alias. If these other cases would be errors for sendmail, so what if they are not errors here? All the more reason to stick with the documented standard. When you diverge from it, you paint the format into a corner, thus closing the door on someone who wants to bring it more in line with the standard. For the same reason, I'm not convinced that simple is a good name. sendmail may indeed be a more appropriate name, even if it means that this early implementation documents it as (currently) a subset of the richer sendmail/postfix 'aliases' format. By doing so, we leave the door open so a future person can implement additional features to bring it closer to that format. Or, a future person can write a sendmail parser that is closer to that format. Yes, but git maintainers must continue to support your simple format even if someone comes along later and adds a more proper sendmail-like format alongside. That's why these questions and observations are being raised now: not to string you along and not because your proposal is necessarily undesirable, but because once such support lands in git, then it remains indefinitely and must be supported for the life of the project. Long-term project health is important, which is why it's desirable to consider these issues early, and to avoid painting ourselves into a corner. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Announce] submitGit for patch submission (was Diffing submodule does not yield complete logs)
On Fri, May 22, 2015 at 1:33 AM, Roberto Tyley roberto.ty...@gmail.com wrote: On Tuesday, 19 May 2015, Stefan Beller sbel...@google.com wrote: On Tue, May 19, 2015 at 12:29 PM, Robert Dailey rcdailey.li...@gmail.com wrote: How do you send your patches inline? [snip] This workflow discussion was a topic at the GitMerge2015 conference, and there are essentially 2 groups, those who know how to send email and those who complain about it. A solution was agreed on by nearly all of the contributors. It would be awesome to have a git-to-email proxy, such that you could do a git push proxy master:refs/for/mailinglist and this proxy would convert the push into sending patch series to the mailing list. It could even convert the following discussion back into comments (on Github?) but as a first step we'd want to try out a one way proxy. Unfortunately nobody stepped up to actually do the work, yet :( Hello, I'm stepping up to do that work :) Or at least, I'm implementing a one-way GitHub PR - Mailing list tool, called submitGit: Cool! I will try that with the next patch I want to submit. https://submitgit.herokuapp.com/ Here's what a user does: * create a PR on https://github.com/git/git When looking at https://github.com/git/git Git Source Code Mirror - This is a publish-only repository and all pull requests are ignored. Please follow Documentation/SubmittingPatches procedure for any of your improvements. Once this tool has proven to be usable (in a few days?), we want to reword that. Guidelines for submitting patches to Git. Half this document covers how to send a patch via email without it getting corrupted - which submitGit will do for you - but the other half is very useful, giving guidance on what good patches for the Git project should look like. If it turns out this tool is widely used we may want to consider splitting up SubmittingPatches inside git.git into two files, one dealing with the contents i.e. How to write good, reviewable commits, following the coding guide lines and having a proper sign off, and another document on how to get your contributions upstream (email, pull request, ...) * logs into https://submitgit.herokuapp.com/ with GitHub auth * selects their PR on https://submitgit.herokuapp.com/git/git/pulls * gets submitGit to email the PR as patches to themselves, in order to check it looks ok * when they're ready, get submitGit to send it to the mailing list on their behalf All discussion of the patch *stays* on the mailing list - I'm not attempting to change anything about the Git community process, other than make it easier for a wider group people to submit patches to the list. For hard-core contributors to Git, I'd imagine that git format-patch send-email remain the fastest way to do their work. But those tools are _unfamiliar to the majority of Git users_ - so submitGit aims to cater to those users, because they definitely have valuable contributions to make, which would be tragic to throw away. I've been working on submitGit in my spare time for the past few weeks, and there are still features I plan to add (like guiding the user to more 'correct' word wrapping, sign-off, etc), but given this discussion, I wanted to chime in and let people know what's here so far. It would be great if people could take the time to explore the tool (you don't have to raise a git/git PR in order to try sending one *to yourself*, for instance) and give feedback on list, or in GitHub issues: https://github.com/rtyley/submitgit/issues I've been lucky enough to discuss the ideas around submitGit with a few people at the Git-Merge conf, so thanks to Peff, Thomas Ferris Nicolaisen, and Emma Jane Hogbin Westby for listening to me (not to imply their endorsement of what I've done, just thanks for talking about it!). Wow! Thanks for doing this, Stefan Roberto -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Announce] submitGit for patch submission (was Diffing submodule does not yield complete logs)
Roberto Tyley roberto.ty...@gmail.com writes: Hello, I'm stepping up to do that work :) Or at least, I'm implementing a one-way GitHub PR - Mailing list tool, called submitGit: https://submitgit.herokuapp.com/ This is absolutely awsome. A few thoughts after testing the system to send two patches: * This is awesome :-). Really. * I found no way to specify a version like [PATCH v2 ...]. Similarly, there could be a way to say [RFC/PATCH ...]. * I'm used to 'git send-email' and I have an alias to add the --signoff for me. I just sent a series where I forgot the signed-off-by. The system could warn if I didn't signoff. * I had a text+subject for the pull-request, I was expecting a cover letter email with them. * A simple way to add below-triple-dash comments would be cool. For now, I guess we can do this by having the --- within the commit message. * The explanation on how to register one's email were unclear to me. It told me that I had to register my email without telling how, and that I had to send the series to myself before. Once I sent the series to myself I got better explanation, hence reversing the order of the advices would already be an improvement. * The link to the original PR could be more visible. For now, I can get to the PR by clicking the octocat icon, but perhaps a text click here to view the PR on GitHub or a link on the whole title would have been easier to find. * I missed a way to specify In-reply-to: to follow up to my previous version. * The submitGit link on the top left does nothing for me. I would expect to come back to the home page of submitGit. * For completess: Junio just noticed in another thread that the patch order is reversed, PATCH 2/2 predates PATCH 1/2 and the second is In-Reply-To: the first. * I was about to suggest that you post a comment on the GitHub pull request when sending the email, but it is already the case, with a link to Gmane. Very very cool. * Did I mention how awesome the application is already? 'Cause it really is! Thanks a lot for doing this! -- 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: [Announce] submitGit for patch submission (was Diffing submodule does not yield complete logs)
Roberto Tyley roberto.ty...@gmail.com writes: Here's what a user does: * create a PR on https://github.com/git/git * logs into https://submitgit.herokuapp.com/ with GitHub auth Hmm, this seems to request too much authorization, though. Repositories Public only This application will be able to read and write all public repo data. This includes the following: Code Issues Pull requests Wikis Settings Webhooks and services Deploy keys I really wanted to try this out, but I do not think, as the owner of a reasonably important repository, it would be irresponsible for me to grant write access to Code or Settings for my primary GitHub account. Also I think you reject an account that is too young (I found out about submitGit via your comment on a pull request to git/git and read its source before reading your message I am responding to, and that was the impression I got from the recent log messages there), so I cannot create and try with a throw-away account, either. That would mean that I cannot join the fun X-. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4] send-email: Add simple email aliases format
On Fri, May 22, 2015 at 2:01 PM, Allen Hubbe alle...@gmail.com wrote: On Fri, May 22, 2015 at 12:53 PM, Eric Sunshine sunsh...@sunshineco.com wrote: On Fri, May 22, 2015 at 8:12 AM, Allen Hubbe alle...@gmail.com wrote: For the same reason, I'm not convinced that simple is a good name. sendmail may indeed be a more appropriate name, even if it means that this early implementation documents it as (currently) a subset of the richer sendmail/postfix 'aliases' format. By doing so, we leave the door open so a future person can implement additional features to bring it closer to that format. Or, a future person can write a sendmail parser that is closer to that format. Yes, but git maintainers must continue to support your simple format even if someone comes along later and adds a more proper sendmail-like format alongside. Someone might implement a sendmail parser in the future, or perhaps never. So, there is the possibility. How strong of a reason is that to reject some other format that is based on a colon? Nobody has suggested that your format should be rejected. Rather, the issue raised regards gratuitous divergence from the sendmail 'aliases' format. You seem to be arguing in favor of gratuitous divergence (without explanation) despite the proposed proper subset approach serving your use-case just as well. What is the harm of the two side by side? This is only a small bit of code that really shouldn't require much maintenance. What is the harm to just leave it in? If the future sendmail parser happens to support the simple format, and the future maintainers determine the situation to be unacceptable, there is still a solution. Simply define both names 'simple' and 'sendmail' to refer to the same sendmail parser. The dead code can be removed. This simple solution doesn't work if your new format diverges from the sendmail 'aliases' format, which is why the issue is being raised now, in order to avoid painting ourselves into that corner. If, on the other hand, your new format remains a proper subset of sendmail 'aliases', then the simple solution does work; and, as a proper subset, it can just as well be named sendmail without hurting any future effort to implement missing functionality. -- 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: recovering from unordered stage entries in index error
Isn't this failure coming from git-svn that tries to write out a tree after it prepared whatever it wants to record in its (possibly temporary) index? I have a feeling that the index held by the end user is not broken. Ahh that would explain why ls-files works. Yep. I created a copy of this repo + wc via rsync and tried a couple of things. 'git svn rebase -l' worked fine, but didn't fix the error. Next, reset: $ git svn log --limit=1 | grep ^r r231655 | avuong | 2015-05-10 10:32:16 -0400 (Sun, 10 May 2015) | 2 lines $ git svn reset -r 231655 -p r231653 = 13a7f6d6a3f3e44ed1c8523b1a63d72fc4f0ddb9 (refs/remotes/trunk) $ git svn fetch fatal: unordered stage entries in index write-tree: command returned error: 128 So it doesn't seem to be specific to the revision being fetched. I could do a more drastic 'git svn reset', but as you can see I've already fetched a lot of revs, so I'd rather avoid re-fetching if possible. Thanks for your help so far -- any other ideas (or requests for further debugging info) are appreciated!
Re: [PATCH 14/14] pull --rebase: error on no merge candidate cases
On Fri, May 22, 2015 at 7:14 AM, Johannes Schindelin johannes.schinde...@gmx.de wrote: Hi Paul, On 2015-05-22 15:48, Paul Tan wrote: On Wed, May 20, 2015 at 12:27 AM, Junio C Hamano gits...@pobox.com wrote: Johannes Schindelin johannes.schinde...@gmx.de writes: - fprintf(stderr, - _(There are no candidates for merging among the refs that you just fetched.\n - Generally this means that you provided a wildcard refspec which had no\n - matches on the remote end.\n)); + if (opt_rebase) + fputs(_(There is no candidate for rebasing against among the refs that you just fetched.), stderr); The puts() function appends a newline while fputs() does not. Yup, so this update makes the command spew unterminated lines, which not something intended... Ugh Will put the \n back. And yes, I used fputs() because it seems wasteful to use fprintf() which will scan the string looking for any '%' formatting units, when we know there aren't. I will also update 05/14 to use fputs() as well where appropriate. I believe the common thinking is that consistency beats speed in error messages, so it would be easier to read and maintain the code if all of those error messages were just using `fprintf(stderr, ...);`. It's not as if we spit out hundreds of thousands of error messages per second where that `%s` parsing would hurt ;-) Ciao, Dscho As soon as we spit out one error message, the speed game is over anyway. (IO is slow, and I believe in the error case correctness is the most critical thing to get right, so no need to pay attention to performance). Though I don't mind having a fputs/fprintf mixture. I just happened to work on parts of code without any fputs before, that's why I brought up this discussion. Actually I think I'd prefer fputs when there is no %. -- 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] submodule documentation: Reorder introductory paragraphs
Stefan Beller sbel...@google.com writes: On Fri, May 22, 2015 at 7:36 AM, Junio C Hamano gits...@pobox.com wrote: Stefan Beller sbel...@google.com writes: On Thu, May 21, 2015 at 1:03 PM, Philip Oakley philipoak...@iee.org wrote: +Submodules are not to be confused with remotes, which are meant +mainly for branches of the same project; This use of 'branches' didn't work for me. remotes are meant mainly for branches of the same project ? The branch in the original is used in a much wider sense than usual branch (i.e. ref/heads/ thing you have locally); it refers to forks of the same project but with a bit of twist. When you say repository A is a fork of the same project as my local repository, you would give an impression that A is not the authoritative copy of the project. But you can say my repository and that repository A are branches of the same project, you give zero information as to A's authoritativeness. While this is correct, I think it is also confusing, because... Oh, no question about it. In modern Git parlance, it confuses by conflating 'branch' (which is local ref/heads/ thing) with something entirely different. I wasn't saying 'branch' is correct and we should keep the description that way. If you dig ancient list archives, you see Linus and I using 'branch' to mean your copy of the project quite often, and that is likely where the above phrase originated. It was one of those explaining historical background, nothing more. I probably should start prefixing all my explaining historical background sentences as such. I do not think this is a great improvement. You now conflated repository to mean project in the latter half of the sentence, while you are trying to explain what a remote repository is. That's true. Your copy of git.git is not the same repository as mine; they have different histories. Both repositories are used to work on the same project. submoules are not remotes, which are other repositories of the same project, perhaps? That makes sense. 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: [Announce] submitGit for patch submission (was Diffing submodule does not yield complete logs)
Ok, I am trying it out now, all I have left is Register your email address (stefanbel...@googlemail.com) with submitGit's Amazon SES account in order for it to send emails from you. So first of all: Where do I find the Amazon SES account for submitGit, to register my email with? Also can I change the email in the process or change it before? Thanks, Stefan -- 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: recovering from unordered stage entries in index error
The message unordered stage entries in index comes only when two adjacent entries in the index are in a wrong order, e.g. test0 should come before test1 but somehow the index records them in the other way around. Doing something like this: $ git ls-files Q $ LANG=C LC_ALL=C sort Q R $ diff Q R may tell you which entries are wrong, even though it wouldn't show who made them wrong. (pardon top-posting, overlong lines and typos; sent from GMail web UI) On Tue, May 19, 2015 at 6:48 AM, McHenry, Matt mmche...@carnegielearning.com wrote: I've just upgraded my git from 2.0.5 to 2.3.6, and I'm now unable to run 'git svn fetch' in one of my repositories: $ git svn fetch fatal: unordered stage entries in index write-tree: command returned error: 128 'git status' shows a few untracked files but is otherwise clean. It looks like this check was introduced in 15999d0be8179fb7a2e6eafb931d25ed65df50aa, with the summary read_index_from(): catch out of order entries when reading an index file (first appearing in 2.2.0). Mailing list discussion looked like it implicated third-party tools. I don't recall running any other tools on this repo; it doesn't do much day-to-day other than a long series of 'git svn fetch'es. (But it's been around for a couple of years, so who knows.) At any rate, what can I do to recover from this situation? I tried to locate a path with multiple index entries like this, but got no results: $ git ls-files -s | cut -f 2-100 | sort | uniq -c | grep -v '^[ \t]*1 ' (I originally posted on SO at http://stackoverflow.com/questions/30264826/; I'll update that with any solutions that come up here, to ease future googling.) -- 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 -- 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] submodule documentation: Reorder introductory paragraphs
It's better to start the man page with a description of what submodules actually are instead of saying what they are not. Reorder the paragraphs such that the first short paragraph introduces the submodule concept, the second paragraph highlights the usage of the submodule command, the third paragraph giving background information, and finally the fourth paragraph discusing alternatives such as subtrees and remotes, which we don't want to be confused with. This ordering deepens the knowledge on submodules with each paragraph. First the basic questions like How/what will be answered, while the underlying concepts will be taught at a later time. Making sure it is not confused with subtrees and remotes is not really enhancing knowledge of submodules itself, but rather painting the big picture of git concepts, so you could also argue to have it as the second paragraph. Personally I think this may confuse readers, specially newcomers though. Additionally to reordering the paragraphs, they have been slightly reworded. Signed-off-by: Stefan Beller sbel...@google.com --- For now I used a part of Junios suggestion Submodules are not to be confused with remotes, which are other repositories of the same project; I like the are not to be confused part, as they warn the reader that there will be a paragraph not as concise but touching other commands and topics. Documentation/git-submodule.txt | 50 ++--- 1 file changed, 22 insertions(+), 28 deletions(-) diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt index 2c25916..d126c86 100644 --- a/Documentation/git-submodule.txt +++ b/Documentation/git-submodule.txt @@ -25,22 +25,17 @@ SYNOPSIS DESCRIPTION --- -Submodules allow foreign repositories to be embedded within -a dedicated subdirectory of the source tree, always pointed -at a particular commit. +This command will inspect, update and manage submodules. -They are not to be confused with remotes, which are meant mainly -for branches of the same project; submodules are meant for -different projects you would like to make part of your source tree, -while the history of the two projects still stays completely -independent and you cannot modify the contents of the submodule -from within the main project. -If you want to merge the project histories and want to treat the -aggregated whole as a single project from then on, you may want to -add a remote for the other project and use the 'subtree' merge strategy, -instead of treating the other project as a submodule. Directories -that come from both projects can be cloned and checked out as a whole -if you choose to go that route. +Submodules allow you to keep another Git repository in a subdirectory +of your repository. The other repository has its own history, which does not +interfere with the history of the current repository. This can be used to +have external dependencies such as libraries for example. + +When cloning or pulling a repository containing submodules however, +these will not be checked out by default; the 'init' and 'update' +subcommands will maintain submodules checked out and at +appropriate revision in your working tree. Submodules are composed from a so-called `gitlink` tree entry in the main repository that refers to a particular commit object @@ -51,19 +46,18 @@ describes the default URL the submodule shall be cloned from. The logical name can be used for overriding this URL within your local repository configuration (see 'submodule init'). -This command will manage the tree entries and contents of the -gitmodules file for you, as well as inspect the status of your -submodules and update them. -When adding a new submodule to the tree, the 'add' subcommand -is to be used. However, when pulling a tree containing submodules, -these will not be checked out by default; -the 'init' and 'update' subcommands will maintain submodules -checked out and at appropriate revision in your working tree. -You can briefly inspect the up-to-date status of your submodules -using the 'status' subcommand and get a detailed overview of the -difference between the index and checkouts using the 'summary' -subcommand. - +Submodules are not to be confused with remotes, which are other +repositories of the same project; submodules are meant for +different projects you would like to make part of your source tree, +while the history of the two projects still stays completely +independent and you cannot modify the contents of the submodule +from within the main project. +If you want to merge the project histories and want to treat the +aggregated whole as a single project from then on, you may want to +add a remote for the other project and use the 'subtree' merge strategy, +instead of treating the other project as a submodule. Directories +that come from both projects can be cloned and checked out as a whole +if you choose to go that route. COMMANDS --
Re: [PATCH v4] send-email: Add simple email aliases format
Allen Hubbe alle...@gmail.com writes: On Fri, May 22, 2015 at 10:44 AM, Junio C Hamano gits...@pobox.com wrote: Let me step back a bit. Earlier you said your aim is not to use an alias file you already have and use with the MUA/MTA, but to have a collection of aliases to use with git-send-email only. Is there a reason to add support for a new format (whether it is compatible to or subset of postfix/sendmail format, or a totally new one) for that goal? What makes the existing formats unsuitable? It's just a matter of personal preference what is suitable or not, for me, in my environment, etc. Is there a reason I should use the alias format of some email client, if I don't use that email client? I do not think should is a good word in the context of that sentence, as nobody is forcing you to choose one of the existing formats. But one reason you might want to do so would be because git-send-email already knows about it. It is a different matter if you already use an email client that supports your new format and you are trying to reuse an alias file with that email client. But I got an impression that was not the case, so the choice seemed to me between - learning and using one of existing 5; and - inventing, adding support for, and using a new one. That felt to me was a choice that is clearly not in favor of the latter, and I was wondering if there were other reasons to shift the balance. For example, all of the existing formats are klunky and difficult to write might be why learning and using one of existing 5 is not a win, compared to inventing, ading support for, and using a new one. I do not know if that is the case, so I wanted to hear the reason why. I'm not trying to force anything on anyone else by offering this, just another option that might be suitable for someone else, in their environment, as it is in mine. People who don't like it can choose a different option. People who don't like any of the options can write their own like I did, or is that not allowed for some reason? We prefer not to carry dead code---when we add things, we would want to make sure it will be widely useful so that other people benefit. I've already shown that I am willing to change the name, write the documentation, write the tests, modify the syntax, and so on. I've done the work, from +6 lines to +57 lines, as requested. I'm not looking forward to v5, v6... v10 of what was a really really simple patch. If you don't like it, please don't string me along. This is not my job. Yeah, I know. A trade off from contributor's side is between (1) handing the maintenance to the upstream, so that a feature will stay available with minimum fuss in the future, or (2) having to carry one's own enhancement forward every time one updates from the upstream. On the other hand, a trade off from project's side is between (1) rejecting a half-way finished ware and hurting feelings of people and (2) accepting a half-way finished ware and having to spend engineering effort (e.g. making sure it fits to the rest of the system without adding dead weight) to polish it to the end. -- 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 v4] send-email: Add simple email aliases format
On Fri, May 22, 2015 at 12:53 PM, Eric Sunshine sunsh...@sunshineco.com wrote: On Fri, May 22, 2015 at 8:12 AM, Allen Hubbe alle...@gmail.com wrote: On Fri, May 22, 2015 at 12:29 AM, Eric Sunshine sunsh...@sunshineco.com wrote: On Thu, May 21, 2015 at 11:40 PM, Allen Hubbe alle...@gmail.com wrote: +If the format is 'simple', then the alias file format is described below. +Descriptions of the other file formats to the following formats can be found in +the documentation of the email program of the same name. The second sentence probably needs some proof-reading. Could you be more specific? Unable to parse of the other file formats to the following formats. I'm guessing that the to the following formats portion doesn't belong. Thanks. It's obvious now that it's pointed out. It's hard to proofread one's own writing. It depends what we want to do with this parser: accept existing sendmail aliases files in git, or enforce that git alias files are usable for sendmail. Aside from these possibilities (likely or unlikely), there is also the issue of breaking expectations. Precedence for this style 'aliases' format was set decades ago by sendmail. People are familiar with it and understand its strengths and weaknesses. Even if documented as not being sendmail-compatible, because it's so similar to sendmail 'aliases', people will expect it to work the same way, thus unless there's a good reason to diverge from that standard format, it makes sense to be compatible with it (even if only as a proper subset). I really don't expect the second to ever happen. The first, maybe, but only if the alias file is edited to remove aliases of pipes and maildirs etc. The second may not work if we have comments to the right, or aliases of aliases, which sendmail does not claim to support. It's not clear why you say that sendmail does not claim to support aliases of aliases. Although it's true that some sources, such as [1], fail to mention support explicitly, That's why I said it. other more authoritative sources do[2]. Moreover, the 1993 sendmail book by Bryan Costales, with contributions from Eric Allman (the creator of sendmail), talks explicitly about expansion of aliases on the right-hand-side. Finally, since time immemorial (at least the early 1980's), every /etc/aliases file has contained the following mandatory entries: postmaster: root MAILER-DAEMON: postmaster which indicates clearly that alias expansion on the RHS is supported. Ok, no harm then if aliases are supported. [1]: http://www.feep.net/sendmail/tutorial/intro/aliases.html [2]: https://www.freebsd.org/cgi/man.cgi?query=aliasessektion=5 I don't know what sendmail would actually do with a '#' elsewhere. It only talks about having '#' at the beginning of a line, or in the alias name in quotes (which is not supported by this parser - proper handling of quoted strings is not easy). It doesn't say what sendmail does with '#' if the name is not quoted, and it doesn't define a meaning for '#' in the definition of an alias. If these other cases would be errors for sendmail, so what if they are not errors here? All the more reason to stick with the documented standard. When you diverge from it, you paint the format into a corner, thus closing the door on someone who wants to bring it more in line with the standard. Giving this a different name leaves the door open to someone who wants to write a sendmail parser. Naming it sendmail and diverging from the standard would close that door. For the same reason, I'm not convinced that simple is a good name. sendmail may indeed be a more appropriate name, even if it means that this early implementation documents it as (currently) a subset of the richer sendmail/postfix 'aliases' format. By doing so, we leave the door open so a future person can implement additional features to bring it closer to that format. Or, a future person can write a sendmail parser that is closer to that format. Yes, but git maintainers must continue to support your simple format even if someone comes along later and adds a more proper sendmail-like format alongside. Someone might implement a sendmail parser in the future, or perhaps never. So, there is the possibility. How strong of a reason is that to reject some other format that is based on a colon? What is the harm of the two side by side? This is only a small bit of code that really shouldn't require much maintenance. What is the harm to just leave it in? If the future sendmail parser happens to support the simple format, and the future maintainers determine the situation to be unacceptable, there is still a solution. Simply define both names 'simple' and 'sendmail' to refer to the same sendmail parser. The dead code can be removed. That's why these questions and observations are being raised now: not to string you along and not because your proposal is necessarily undesirable, but
Re: [PATCH] submodule documentation: Reorder introductory paragraphs
On Fri, May 22, 2015 at 7:36 AM, Junio C Hamano gits...@pobox.com wrote:
Stefan Beller sbel...@google.com writes:
On Thu, May 21, 2015 at 1:03 PM, Philip Oakley philipoak...@iee.org wrote:
+Submodules are not to be confused with remotes, which are meant
+mainly for branches of the same project;
This use of 'branches' didn't work for me. remotes are meant mainly for
branches of the same project ?
The branch in the original is used in a much wider sense than
usual branch (i.e. ref/heads/ thing you have locally); it refers to
forks of the same project but with a bit of twist. When you say
repository A is a fork of the same project as my local repository,
you would give an impression that A is not the authoritative copy of
the project. But you can say my repository and that repository A
are branches of the same project, you give zero information as to
A's authoritativeness.
While this is correct, I think it is also confusing, because 'branch'
is a command which deals with local branches only in my perception
To deal with remote branches you need to use the commands
{remote, fetch, pull}.
So when someone mentions branch I need to think of local operations
in one repository and not on different distributed histories.
Submodules should not be confused with remote repositories, which are
meant to track the same repository, just at another location; ...
I do not think this is a great improvement. You now conflated
repository to mean project in the latter half of the sentence,
while you are trying to explain what a remote repository is.
That's true.
Your copy of git.git is not the same repository as mine; they have
different histories. Both repositories are used to work on the same
project. submoules are not remotes, which are other repositories
of the same project, perhaps?
That makes sense.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Announce] submitGit for patch submission (was Diffing submodule does not yield complete logs)
From: Johannes Schindelin johannes.schinde...@gmx.de Hi Roberto, On 2015-05-22 11:42, Johannes Schindelin wrote: On 2015-05-22 10:33, Roberto Tyley wrote: On Tuesday, 19 May 2015, Stefan Beller sbel...@google.com wrote: On Tue, May 19, 2015 at 12:29 PM, Robert Dailey rcdailey.li...@gmail.com wrote: How do you send your patches inline? [snip] This workflow discussion was a topic at the GitMerge2015 conference, and there are essentially 2 groups, those who know how to send email and those who complain about it. A solution was agreed on by nearly all of the contributors. It would be awesome to have a git-to-email proxy, such that you could do a git push proxy master:refs/for/mailinglist and this proxy would convert the push into sending patch series to the mailing list. It could even convert the following discussion back into comments (on Github?) but as a first step we'd want to try out a one way proxy. Unfortunately nobody stepped up to actually do the work, yet :( Hello, I'm stepping up to do that work :) Or at least, I'm implementing a one-way GitHub PR - Mailing list tool, called submitGit: https://submitgit.herokuapp.com/ Wow!!! I will make sure to test it with a couple of patches I want to submit anyway. I just tried this with https://github.com/git/git/pull/139 and would like to tell you about two wishes I had immediately: - If the author of a patch I am submitting is not myself, could submitGit maybe add that `From: ` line at the top of the mail? - The patch series is sent without a cover letter, but it would be *really* great if a path series consisting of more than one patch could have the initial comment of the Pull Request as a cover letter, with the link to the original Pull Request at the bottom? This would also be the mail to use in the In-reply-yo header instead of the first patch. Thanks so much! Dscho A separate request would be to be able to use PRs that are for forks of git/git, such as msysgit etc. (i.e. have a common --root), which would help in the upstreaming of some changes. I ask because I just logged in and my preparatory PR318 (https://github.com/msysgit/git/pull/318) for rejuvenating the msvc-build system wasn't listed, probably because of the forking. -- 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] submodule documentation: Reorder introductory paragraphs
From: Stefan Beller sbel...@google.com
On Fri, May 22, 2015 at 7:36 AM, Junio C Hamano gits...@pobox.com
wrote:
Stefan Beller sbel...@google.com writes:
On Thu, May 21, 2015 at 1:03 PM, Philip Oakley
philipoak...@iee.org wrote:
+Submodules are not to be confused with remotes, which are meant
+mainly for branches of the same project;
This use of 'branches' didn't work for me. remotes are meant
mainly for
branches of the same project ?
The branch in the original is used in a much wider sense than
usual branch (i.e. ref/heads/ thing you have locally); it refers to
forks of the same project but with a bit of twist. When you say
repository A is a fork of the same project as my local repository,
you would give an impression that A is not the authoritative copy of
the project. But you can say my repository and that repository A
are branches of the same project, you give zero information as to
A's authoritativeness.
While this is correct, I think it is also confusing, because 'branch'
is a command which deals with local branches only in my perception
To deal with remote branches you need to use the commands
{remote, fetch, pull}.
So when someone mentions branch I need to think of local operations
in one repository and not on different distributed histories.
If we are having difficulties defining a remote here (its not defined
in gitglossary.txt anyway), why not simply put a full stop (period)
after the Submodules are not to be confused with remotes., and bypass
the problem, avoiding digging the hole deeper.
Submodules should not be confused with remote repositories,
which are
meant to track the same repository, just at another location;
...
I do not think this is a great improvement. You now conflated
repository to mean project in the latter half of the sentence,
while you are trying to explain what a remote repository is.
That's true.
Your copy of git.git is not the same repository as mine; they have
different histories. Both repositories are used to work on the same
project. submoules are not remotes, which are other repositories
of the same project, perhaps?
That makes sense.
If maybe that the feature we should pick on is the common root of the
development between the local and remote repository, and quite distinct
for the submodule. This allows remotes to be on the same machine, as
well as distant machines and server.
It is I believe technically possible to have a submodule which is its
own super project, with and without recursion, but would be very
atypical, and would belong in the 'don't do that' category
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Announce] submitGit for patch submission (was Diffing submodule does not yield complete logs)
From: Junio C Hamano gits...@pobox.com Roberto Tyley roberto.ty...@gmail.com writes: Hello, I'm stepping up to do that work :) Or at least, I'm implementing a one-way GitHub PR - Mailing list tool, called submitGit: https://submitgit.herokuapp.com/ Yay ;-) Here's what a user does: * create a PR on https://github.com/git/git * logs into https://submitgit.herokuapp.com/ with GitHub auth * selects their PR on https://submitgit.herokuapp.com/git/git/pulls Reasonable. * gets submitGit to email the PR as patches to themselves, in order to check it looks ok I can see you are trying to be careful by doing this, but I am not sure if this step would actually help. Those who are not familiar with Git development are not expected to know what is ok in their original commit, and if they find bad formatting done by submitGit (e.g. adds their PR message before the three-dash line instead of after it), they cannot do much about it anyway. I still think this is valuable for the spotting of the dumb mistakes we all make, and notice after the fact when we see the email in the hard light of day. There will still be poor commit messages and the like, but at least the raw content is more likely to be as the author desired. * when they're ready, get submitGit to send it to the mailing list on their behalf Nice. All discussion of the patch *stays* on the mailing list Can you identify a reroll of an earlier submission? If you can use the in-reply-to and make it a follow-up to the previous round, that would be great. -- 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] submodule documentation: Reorder introductory paragraphs
On Fri, May 22, 2015 at 10:35 AM, Philip Oakley philipoak...@iee.org wrote:
From: Stefan Beller sbel...@google.com
On Fri, May 22, 2015 at 7:36 AM, Junio C Hamano gits...@pobox.com wrote:
Stefan Beller sbel...@google.com writes:
On Thu, May 21, 2015 at 1:03 PM, Philip Oakley philipoak...@iee.org
wrote:
+Submodules are not to be confused with remotes, which are meant
+mainly for branches of the same project;
This use of 'branches' didn't work for me. remotes are meant mainly
for
branches of the same project ?
The branch in the original is used in a much wider sense than
usual branch (i.e. ref/heads/ thing you have locally); it refers to
forks of the same project but with a bit of twist. When you say
repository A is a fork of the same project as my local repository,
you would give an impression that A is not the authoritative copy of
the project. But you can say my repository and that repository A
are branches of the same project, you give zero information as to
A's authoritativeness.
While this is correct, I think it is also confusing, because 'branch'
is a command which deals with local branches only in my perception
To deal with remote branches you need to use the commands
{remote, fetch, pull}.
So when someone mentions branch I need to think of local operations
in one repository and not on different distributed histories.
If we are having difficulties defining a remote here (its not defined in
gitglossary.txt anyway),
Now that we have a discussion on what remotes are, I'll send a patch for that
as well.
why not simply put a full stop (period) after the
Submodules are not to be confused with remotes., and bypass the problem,
avoiding digging the hole deeper.
I think we should dig deeper and point out the differences as it may
not be clear what
the differences are for new comers. Not digging deeper sounds to me like saying
'git frotz' is not to be confused with 'git bar' FULL STOP AND I
WONT TELL YOU WHY!
which is not helpful. (Why is the documentation pointing out there is
a difference to
that other command/concept, but not saying what is different?)
Submodules should not be confused with remote repositories, which
are
meant to track the same repository, just at another location; ...
I do not think this is a great improvement. You now conflated
repository to mean project in the latter half of the sentence,
while you are trying to explain what a remote repository is.
That's true.
Your copy of git.git is not the same repository as mine; they have
different histories. Both repositories are used to work on the same
project. submoules are not remotes, which are other repositories
of the same project, perhaps?
That makes sense.
If maybe that the feature we should pick on is the common root of the
development between the local and remote repository, and quite distinct for
the submodule. This allows remotes to be on the same machine, as well as
distant machines and server.
I don't think this is actually true for all remotes. Think of shallow clones
(they have no root or a different root) or even subtrees which are pulled
in via a remotes?
The main thing about remotes is not being here (as in part of this
repository. As you point out it can be nearby in the local fs or even on
another machine, or in the cloud)
It is I believe technically possible to have a submodule which is its own
super project, with and without recursion, but would be very atypical, and
would belong in the 'don't do that' category
--
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 v4] send-email: Add simple email aliases format
On Fri, May 22, 2015 at 1:17 PM, Junio C Hamano gits...@pobox.com wrote: Allen Hubbe alle...@gmail.com writes: On Fri, May 22, 2015 at 10:44 AM, Junio C Hamano gits...@pobox.com wrote: Let me step back a bit. Earlier you said your aim is not to use an alias file you already have and use with the MUA/MTA, but to have a collection of aliases to use with git-send-email only. Is there a reason to add support for a new format (whether it is compatible to or subset of postfix/sendmail format, or a totally new one) for that goal? What makes the existing formats unsuitable? It's just a matter of personal preference what is suitable or not, for me, in my environment, etc. Is there a reason I should use the alias format of some email client, if I don't use that email client? I do not think should is a good word in the context of that sentence, as nobody is forcing you to choose one of the existing formats. But one reason you might want to do so would be because git-send-email already knows about it. It is a different matter if you already use an email client that supports your new format and you are trying to reuse an alias file with that email client. But I got an impression that was not the case, so the choice seemed to me between - learning and using one of existing 5; and I imagine that's what most people would do, faced with the same issue. I did initially go look at those formats. Since I didn't really prefer any of them, I approached solving the problem in a different way. - inventing, adding support for, and using a new one. That felt to me was a choice that is clearly not in favor of the latter, and I was wondering if there were other reasons to shift the balance. For example, all of the existing formats are klunky and difficult to write might be why learning and using one of existing 5 is not a win, compared to inventing, ading support for, and using a new one. I do not know if that is the case, so I wanted to hear the reason why. That for example is it. Why should I have to type alias before each alias in the file? It's not in any way hard to do - it just serves no purpose other than to make the parser happy. Perhaps the keyword does serve a purpose in mutt, but for me it is pointless to type that. I'm not trying to force anything on anyone else by offering this, just another option that might be suitable for someone else, in their environment, as it is in mine. People who don't like it can choose a different option. People who don't like any of the options can write their own like I did, or is that not allowed for some reason? We prefer not to carry dead code---when we add things, we would want to make sure it will be widely useful so that other people benefit. 1 vote for useful. I realize this is self serving, but I hoped sharing it would benefit others. I've already shown that I am willing to change the name, write the documentation, write the tests, modify the syntax, and so on. I've done the work, from +6 lines to +57 lines, as requested. I'm not looking forward to v5, v6... v10 of what was a really really simple patch. If you don't like it, please don't string me along. This is not my job. Yeah, I know. A trade off from contributor's side is between (1) handing the maintenance to the upstream, so that a feature will stay available with minimum fuss in the future, or (2) having to carry one's own enhancement forward every time one updates from the upstream. (3) good citizenship in open source to share one's changes to the code. On the other hand, a trade off from project's side is between (1) rejecting a half-way finished ware and hurting feelings of people and (2) accepting a half-way finished ware and having to spend engineering effort (e.g. making sure it fits to the rest of the system without adding dead weight) to polish it to the end. I get that, in the general case, and especially for large features that affect a lot of the user base. How worried are you in this case, about (2), for such a small amount of code that now has a more extensive unit test case and documentation than any of the other options? -- 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
