Re: [PATCH 2/2] prompt: respect GIT_TERMINAL_PROMPT to disable terminal prompts
On Thu, Dec 04, 2014 at 01:33:53PM -0800, Junio C Hamano wrote: Jeff King p...@peff.net writes: On Thu, Dec 04, 2014 at 10:24:09AM -0800, Junio C Hamano wrote: I wish this covered a lot more than just this part from an end-user's point of view, but this is definitely one of the most important code paths the mechanism should cover. Which parts do you mean? Stuff like git add -i? No, more like tag -s that eventually leads to somebody prompting for the passphrase to unlock your GPG key---and from an end user's point of view, that somebody is Git. Ah, yeah, I definitely agree that GIT_TERMINAL_PROMPT should work there, too. Of course, from _our_ point of view, that somebody is not us. We do not have direct control, certainly from this codepath. Right, but in theory we can provoke gpg to do what we want when we spawn it. However, having had zero luck in convincing it to stop asking me for a passphrase recently in another thread, I do not know what magic command line option is required. :( I think it would be OK to merge the git handling of GIT_TERMINAL_PROMPT (i.e., the patch I sent), and somebody who runs into the issue with gpg and can figure out how to tame it can scratch their own itch later. I hate leaving things half-implemented or inconsistent, but I also don't know how to make gpg do what we want. And doing a partial solution seems better to me than holding the credential.c half hostage. -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] introduce git root
On Fri, Dec 05, 2014 at 03:27:17AM +0100, Christian Couder wrote: For example, to chose the editor all the following could apply: GIT_SEQUENCE_EDITOR env variable sequence.editor config variable GIT_EDITOR env variable core.editor config variable VISUAL env variable EDITOR env variable editor configured at compile time and the user or our own scripts right now cannot easily know which editor should be used when editing the sequence list. I think we're violently agreeing. Taking all of those inputs and computing the final value for a git-sequence editor is exactly what git var should be doing. IMHO it is a bug that when GIT_SEQUENCE_EDITOR was introduced, there was not a matching update to compute it from git var (I didn't even know sequence.editor existed until now!). I am not opposed to that at all; that's the point of git var, and the computables we are talking about. I am only opposed to mixing the namespace for computables with config. I.e., there is no point in asking git var for the core.editor config. It is not a computable value, and we already have a way of accessing it (git-config, which can also _write_ the value, something that git-var will never be able to do for computable values). I do not think git var --exec-path is a good idea, nor GIT_EXEC_PATH for the environment-variable confusion you mentioned. I was thinking of just creating a new namespace, like: git var exec-path git var author-ident I agree that this is nice, but I wonder what we would do for the sequence editor and the default editor. Maybe: git var sequence-editor git var editor Again, I think we're mostly agreeing. Context and hierarchy and falling back are good things. Whatever we call the variables, editor and sequence-editor and foo-editor should have a predictable and consistent form. I like the idea of foo-editor automatically falling back to editor even if we don't know what foo is. But the one place I do not agree is: I think sequence.editor and core.editor are better because: - they use the same syntax as the config variables, so they are easier to remember and to discover, and I really don't like using core.editor here, because it has the same name as a config variable, but it is _not_ the config variable. It happens to use the config variable as one of the inputs to its computation, but in many cases: git config core.editor and git var core.editor will produce different values. They are entirely different namespaces. Using the same syntax and name seems unnecessarily confusing to me. Even still using dotted hierarchies, but giving them different names (e.g., editor, editor.sequence, editor.foo) would make it more obvious that they are not the same thing. -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] gc: support temporarily preserving garbage
On Thu, Dec 04, 2014 at 10:04:29AM -0800, Junio C Hamano wrote: If it is a problem (and again, I am just guessing), I'd imagine you would need a similar setup to what you proposed for unlink(): before renaming packed-refs.lock into packed-refs, hard-link it into your trash area. And you'd probably want to intercept rename() there, to catch all places where we use this technique. Also we need to take it into account that it is not an issue unique to Git that the server side may expire these .nfsX entries left by an NFS client (silly rename) to keep files that have been removed or renamed away alive. Aren't there a knob on the NFS server end to control how long these are kept unexpired to avoid stale filehandle errors, so that not just Git but all applications running on NFS client machines will not be hurt by it? Working it around at the application program level for each and every application that runs on a machine that can NFS mount filesystems from elsewhere may be simply madness, no? Thanks. That is the vague feeling I have about the series, but without having actually _seen_ the problem or knowing to what extent it affects git, I didn't want to say anything so definite (e.g., I am only guessing that rename() is a problem, but from the original description, it sounds quite plausible that it is). If this can be fixed outside of git, I very much agree that is the best path forward. -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's configure script --mandir doesn't work
On Thu, Dec 04, 2014 at 04:25:32PM -0700, Stephen Fisher wrote: I'm installing Git 2.2.0 from source distribution on NetBSD 6.1.5 (amd64) and when I specify --mandir=/usr/local/man, it still installs man pages in the default /usr/local/share/man directory. Is there a fix available for this? It works fine for me here (Debian): tar xzf git-2.2.0.tar.gz cd git-2.2.0 ./configure --prefix=/tmp/foo --mandir=/tmp/bar make install-man puts the manpages into /tmp/bar. Can you elaborate on the commands you're running? After running the configure script, can you confirm that mandir is set appropriately in config.mak.autogen? -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
CALL FOR DISCUSSION (REPLY URGENTLY NEEDED)
Dear Friend, I am William Keith,Personal banker and account officer to the late UWE Gamballa who was murdered in South Africa, before his death he was the CEO of Porsche Motors. My urgent need for a foreign partner that made me to contact You for this transaction. I got your contact from yahoo Tourist search while I was searching for a foreign partner. As this message might meet you in utmost surprise. However, it is all to be sure of your capability and reliability to champion this business opportunity when I prayed to good Lord about you. Before his death he left the sum of 22 Million USD with the ABSA Bank of South Africa were currently work as a banker, and all attempt to communicate with his Family was to no avail, and he never included any next of Kin on this account and until now no one has stepped forward to lay claims on the entire funds. Therefore, I require your partnership to stand as his next of kin to enable me prepare all legal paper works in your name to have the funds moved out in your name, and if you agree do respond back to me to enable me furnish you with more details as to the way forward in how this can be achieved. Immediately you receive this letter. Please indicate your Willingness by sending your information to enable us enter into the official stage of this transaction. For more clarification and easy communication. I await your response. Thanks. Yours Faithfully, Mr.William Keith ABSA Bank, Johannesburg, South Africa. -- To unsubscribe from this list: send the line 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] commit: ignore assume-unchanged files in commmit file mode
In the same spirit of 7fce6e3 (commit: correctly respect skip-worktree bit - 2009-12-14), if a file is marked unchanged, skip it. Noticed-by: Sérgio Basto ser...@serjux.com Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- builtin/commit.c | 2 +- t/t2106-update-index-assume-unchanged.sh | 10 ++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/builtin/commit.c b/builtin/commit.c index e108c53..ee3de12 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -252,7 +252,7 @@ static int list_paths(struct string_list *list, const char *with_tree, if (!ce_path_match(ce, pattern, m)) continue; item = string_list_insert(list, ce-name); - if (ce_skip_worktree(ce)) + if (ce-ce_flags (CE_VALID | CE_SKIP_WORKTREE)) item-util = item; /* better a valid pointer than a fake one */ } diff --git a/t/t2106-update-index-assume-unchanged.sh b/t/t2106-update-index-assume-unchanged.sh index 99d858c..dc332f5 100755 --- a/t/t2106-update-index-assume-unchanged.sh +++ b/t/t2106-update-index-assume-unchanged.sh @@ -21,4 +21,14 @@ test_expect_success 'do not switch branches with dirty file' \ git update-index --assume-unchanged file test_must_fail git checkout master' +test_expect_success 'commit paths ignore assume-unchanged files' ' + : anotherfile + git add anotherfile + echo dirty anotherfile + git commit -m one -- file anotherfile + git diff --name-only HEAD^ HEAD actual + echo anotherfile expected + test_cmp expected actual +' + test_done -- 2.2.0.60.gb7b3c64 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: bug report on update-index --assume-unchanged
On Fri, Dec 5, 2014 at 1:55 PM, Johannes Sixt j...@kdbg.org wrote: Actually, it's a user error. When you set --assume-unchanged, then you give a promise to git that you do not change the files, and git does not have to check itself whether there is a change. But since you did not keep your promise, you get what you deserve. ;-) You are correct about the original idea behind --assume-unchanged. But over the time I think we bend over a bit and sort of support these use cases. For example, aecda37 (do not overwrite files marked assume unchanged - 2010-05-01). The change is one-liner, so I don't mind doing it. -- 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: [RFC/PATCH 2/2] branch: allow -f with -m and -d
Junio C Hamano schrieb am 04.12.2014 um 20:13:
Michael J Gruber g...@drmicha.warpmail.net writes:
-f/--force is the standard way to force an action, and is used by branch
for the recreation of existing branches, but not for deleting unmerged
branches nor for renaming to an existing branch.
Make -m -f equivalent to -M and -d -f equivalent to -D, i.e.
allow -f/--force to be used with -m/-d also.
I like that goal. And I agree with your s/force_create/force/g
remark on the cover, too.
Signed-off-by: Michael J Gruber g...@drmicha.warpmail.net
---
builtin/branch.c | 9 +++--
t/t3200-branch.sh | 5 +
2 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/builtin/branch.c b/builtin/branch.c
index 3b79c50..8ea04d7 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -848,7 +848,7 @@ int cmd_branch(int argc, const char **argv, const char
*prefix)
OPT_BOOL('l', create-reflog, reflog, N_(create the branch's
reflog)),
OPT_BOOL(0, edit-description, edit_description,
N_(edit the description for the branch)),
-OPT__FORCE(force_create, N_(force creation (when already
exists))),
+OPT__FORCE(force_create, N_(force creation, move/rename,
deletion)),
{
OPTION_CALLBACK, 0, no-merged, merge_filter_ref,
N_(commit), N_(print only not merged branches),
@@ -891,7 +891,7 @@ int cmd_branch(int argc, const char **argv, const char
*prefix)
if (with_commit || merge_filter != NO_FILTER)
list = 1;
-if (!!delete + !!rename + !!force_create + !!new_upstream +
+if (!!delete + !!rename + !!new_upstream +
This puzzled me but earlier -f implied creation and no other mode
(hence it was an error to give it together with delete and other
modes), but now -f is merely a do forcibly whatever mode of
operation other option determines that does not conflict.
What should -f -u and -f -l do, then, though?
list + unset_upstream 1)
usage_with_options(builtin_branch_usage, options);
I would say there is nothing to force, so we ignore -f there.
Alternatively, we could warn about that. While I do consider forcing
something that doesn't need force a mistake in other contexts, I would
not apply that thinking to the -f option.
Michael
--
To unsubscribe from this list: send the line 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 04/23] expire_reflog(): remove unused parameter
On 12/05/2014 12:28 AM, Jonathan Nieder wrote: Michael Haggerty wrote:
It was called unused, so at least it was self-consistent.
The missing context is that this was a callback function that had to
match the each_ref_fn signature [...]
With or without a note in the commit message explaining that,
Reviewed-by: Jonathan Nieder jrnie...@gmail.com
[...]
--- a/builtin/reflog.c
+++ b/builtin/reflog.c
@@ -349,7 +349,7 @@ static int push_tip_to_list(const char *refname,
const unsigned char *sha1, int
return 0;
}
-static int expire_reflog(const char *ref, const unsigned char *sha1,
int unused, void *cb_data)
+static int expire_reflog(const char *ref, const unsigned char *sha1,
void *cb_data)
{
struct cmd_reflog_expire_cb *cmd = cb_data;
On second thought: why not update the last parameter to be a 'struct
cmd_reflog_expire_cb *' instead of 'void *' while at it, like this?
[...]
Thanks for the explanation, the review, and the suggestion. I will
expand the commit to be don't implement each_ref_fn anymore and
incorporate all of your suggestions.
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 06/23] expire_reflog(): exit early if the reference has no reflog
On 12/05/2014 12:53 AM, Jonathan Nieder wrote: Michael Haggerty wrote: [Subject: expire_reflog(): exit early if the reference has no reflog] The caller moves on to expire other reflogs, so it's not exiting. return early, maybe? Except the function already returned early. I think the purpose of this patch is to simplify the no-reflog case by handling it separately. Anyway, that's just nitpicking about the subject line. With s/exit/return/ it should be clear that this is a refactoring change, which for someone looking at the shortlog is the important thing. Good suggestion. I will change the commit message. 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: SPAM: RE: FW: [cygwin] Cygwin's git says error: failed to read delta-pack base object
-Original Message-
From: git-ow...@vger.kernel.org
[mailto:git-ow...@vger.kernel.org] On Behalf Of Jason Pyeron
Sent: Thursday, December 04, 2014 16:34
To: git@vger.kernel.org
Cc: 'brian m. carlson'
Subject: SPAM: RE: FW: [cygwin] Cygwin's git says error:
failed to read delta-pack base object
-Original Message-
From: brian m. carlson
Sent: Wednesday, December 03, 2014 19:55
On Wed, Dec 03, 2014 at 06:31:18PM -0500, Jason Pyeron wrote:
I remember hitting this a while ago, but just gave up.
It seems to be a problem for others too.
Any ideas on how to debug this so it can be patched?
-Original Message-
From: Dave Lindbergh
Sent: Wednesday, December 03, 2014 18:07
To: cygwin
Aha - you're right.
It works fine on a local NTFS volume.
I get the error when I do it on Z:, which is mapped to a
network drive
(on another Windows box).
Is there a workaround? Why does this happen?
I don't think anyone is sure. My wild guess is that
there's something
about the way that Cygwin wraps Windows calls that makes it
fail in this
case. It might be interesting to run the Windows and
Cygwin versions
under an strace equivalent and see where things differ.
[this was posted to the cygwin list]
http://nerdfever.com/files/strace.txt
It's an interesting problem, but I don't have any Windows
systems, so I
can't look into it.
If it becomes a little less magic, I will chomp on the
problem, but I cannot make a predictable test case.
Corrina at Cygwin devined the strace (see below) and I am working on a test
cases and hacks.
Pseudo code and observations
./sha1_file.c:write_loose_object(sha1)
{
filename=sha1_file_name(sha1)
(fd,tmp_file)=create_tmpfile(filename)
write_buffer(fd)
close_sha1_file(fd)
return move_temp_to_file(tmp_file, filename)
}
move_temp_to_file(tmpfile, filename)
{
// I am thinking about forcing renames to see if the problem exists then as
well
// if that works then a per repo config option allowing for forced renames
if (OBJECT_CREATION_USES_RENAMES) goto try_rename
else if link(tmpfile,filename)
if (failed except exist failures)
{
try_rename:
rename(tmpfile, filename)
if (ok) goto out
}
unlink_or_warn(tmpfile)
if (failed except exist failures) return error
out:
}
write_sha1_file(sha1)
{
return write_loose_object(sha1)
}
-Original Message-
From: Corinna Vinschen
Sent: Friday, December 05, 2014 6:35
To: cyg...@cygwin.com
snip/
What I found in the strace is this:
- Create file Z:\pic32mx-bmf\.git\objects\30\tmp_obj_YljwNZ
- open file, write something, close file.
- link (Z:\pic32mx-bmf\.git\objects\30\tmp_obj_YljwNZ,
Z:\pic32mx-bmf\.git\objects\30\0bdeb2fd209d24afb865584da10b78aa8fefc4)
succeeds.
- unlink (Z:\pic32mx-bmf\.git\objects\30\tmp_obj_YljwNZ) succeeds
- Trying to open
Z:\pic32mx-bmf\.git\objects\30\0bdeb2fd209d24afb865584da10b78aa8fefc4
but the file doesn't exist and NtCreateFile fails with status
0xC034, STATUS_OBJECT_NAME_NOT_FOUND -- ENOENT.
- Subsequent unlink (Z:\pic32mx-bmf\.git\objects\30) fails with a
STATUS_DIRECTORY_NOT_EMPTY -- ENOTEMPTY.
- git seems to be prepared for such a case, the parent process calls
opendir/readdir on the directory. Enumerating the files in
Z:\pic32mx-bmf\.git\objects\30 shows the entries ., .. and
0bdeb2fd209d24afb865584da10b78aa8fefc4.
- Then git calls lstat on the file, which results in NtOpenFile
returning status STATUS_OBJECT_NAME_NOT_FOUND again.
- From a POSIX POV this means somebody else deleted the file,
so the dir is empty now. Git tries to delete the directory again,
which again results in STATUS_DIRECTORY_NOT_EMPTY -- ENOTEMPTY
and, internally, a sharing violation which disallows to move the
directory out of the way.
This looks suspiciously like a bug in the remote filesystem. Link
succeeded, so there are two links to the same file in the directory.
Unlinking link 1 succeeds, so there's still one link to the
file in the
directory, but link 2 is inaccessible as if the file has been deleted
completely. Thus, a full POSIX git on this drive is broken.
Can you please run
/usr/lib/csih/getVolInfo /cygdrive/z
and paste the output here? Maybe I can workaround this in the next
Cygwin version.
Corinna
--
-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
- -
- Jason Pyeron PD Inc. http://www.pdinc.us -
- Principal Consultant 10 West 24th Street #100-
- +1 (443) 269-1555 x333Baltimore, Maryland 21218 -
- -
-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
This message is copyright PD Inc, subject to license 20080407P00.
--
To unsubscribe from this list: send the
RE: FW: [cygwin] Cygwin's git says error: failed to read delta-pack base object
-Original Message-
From: Jason Pyeron
Sent: Thursday, December 04, 2014 16:34
To: git@vger.kernel.org
Cc: 'brian m. carlson'
Subject: RE: FW: [cygwin] Cygwin's git says error:
failed to read delta-pack base object
-Original Message-
From: brian m. carlson
Sent: Wednesday, December 03, 2014 19:55
On Wed, Dec 03, 2014 at 06:31:18PM -0500, Jason Pyeron wrote:
I remember hitting this a while ago, but just gave up.
It seems to be a problem for others too.
Any ideas on how to debug this so it can be patched?
-Original Message-
From: Dave Lindbergh
Sent: Wednesday, December 03, 2014 18:07
To: cygwin
Aha - you're right.
It works fine on a local NTFS volume.
I get the error when I do it on Z:, which is mapped to a
network drive
(on another Windows box).
Is there a workaround? Why does this happen?
I don't think anyone is sure. My wild guess is that
there's something
about the way that Cygwin wraps Windows calls that makes it
fail in this
case. It might be interesting to run the Windows and
Cygwin versions
under an strace equivalent and see where things differ.
[this was posted to the cygwin list]
http://nerdfever.com/files/strace.txt
It's an interesting problem, but I don't have any Windows
systems, so I
can't look into it.
If it becomes a little less magic, I will chomp on the
problem, but I cannot make a predictable test case.
Corrina at Cygwin devined the strace (see below) and I am working on a test
cases and hacks.
Pseudo code and observations
./sha1_file.c:write_loose_object(sha1)
{
filename=sha1_file_name(sha1)
(fd,tmp_file)=create_tmpfile(filename)
write_buffer(fd)
close_sha1_file(fd)
return move_temp_to_file(tmp_file, filename)
}
move_temp_to_file(tmpfile, filename)
{
// I am thinking about forcing renames to see if the problem exists then as
well
// if that works then a per repo config option allowing for forced renames
if (OBJECT_CREATION_USES_RENAMES) goto try_rename
else if link(tmpfile,filename)
if (failed except exist failures)
{
try_rename:
rename(tmpfile, filename)
if (ok) goto out
}
unlink_or_warn(tmpfile)
if (failed except exist failures) return error
out:
}
write_sha1_file(sha1)
{
return write_loose_object(sha1)
}
-Original Message-
From: Corinna Vinschen
Sent: Friday, December 05, 2014 6:35
To: cyg...@cygwin.com
snip/
What I found in the strace is this:
- Create file Z:\pic32mx-bmf\.git\objects\30\tmp_obj_YljwNZ
- open file, write something, close file.
- link (Z:\pic32mx-bmf\.git\objects\30\tmp_obj_YljwNZ,
Z:\pic32mx-bmf\.git\objects\30\0bdeb2fd209d24afb865584da10b78aa8fefc4)
succeeds.
- unlink (Z:\pic32mx-bmf\.git\objects\30\tmp_obj_YljwNZ) succeeds
- Trying to open
Z:\pic32mx-bmf\.git\objects\30\0bdeb2fd209d24afb865584da10b78aa8fefc4
but the file doesn't exist and NtCreateFile fails with status
0xC034, STATUS_OBJECT_NAME_NOT_FOUND -- ENOENT.
- Subsequent unlink (Z:\pic32mx-bmf\.git\objects\30) fails with a
STATUS_DIRECTORY_NOT_EMPTY -- ENOTEMPTY.
- git seems to be prepared for such a case, the parent process calls
opendir/readdir on the directory. Enumerating the files in
Z:\pic32mx-bmf\.git\objects\30 shows the entries ., .. and
0bdeb2fd209d24afb865584da10b78aa8fefc4.
- Then git calls lstat on the file, which results in NtOpenFile
returning status STATUS_OBJECT_NAME_NOT_FOUND again.
- From a POSIX POV this means somebody else deleted the file,
so the dir is empty now. Git tries to delete the directory again,
which again results in STATUS_DIRECTORY_NOT_EMPTY -- ENOTEMPTY
and, internally, a sharing violation which disallows to move the
directory out of the way.
This looks suspiciously like a bug in the remote filesystem. Link
succeeded, so there are two links to the same file in the directory.
Unlinking link 1 succeeds, so there's still one link to the
file in the
directory, but link 2 is inaccessible as if the file has been deleted
completely. Thus, a full POSIX git on this drive is broken.
Can you please run
/usr/lib/csih/getVolInfo /cygdrive/z
and paste the output here? Maybe I can workaround this in the next
Cygwin version.
Corinna
--
-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
- -
- Jason Pyeron PD Inc. http://www.pdinc.us -
- Principal Consultant 10 West 24th Street #100-
- +1 (443) 269-1555 x333Baltimore, Maryland 21218 -
- -
-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
This message is copyright PD Inc, subject to license 20080407P00.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More
Re: Enhancement Request: locale git option
On 12/04/2014 09:55 PM, Jeff King wrote: On Thu, Dec 04, 2014 at 06:21:40PM +0100, Ævar Arnfjörð Bjarmason wrote: That is one of the many reasons why I proposed to have a dictionary of the main technical terms for each language before we even localise git in that language. In an ideal word, we would provide a simple solution for looking these terms up both ways. I don't think we're going to have localised man pages any time soon, are we? I think that's a great idea, and one that's only blocked on someone (hint hint) sending patches for it. It would be neat-o to have something to make translating the docs easier, i.e. PO files for sections of the man pages. There's tools to help with that which we could use. But there's no reason for us not to have translated glossaries in the meantime. By the way, there has been fairly significant volunteer effort put into translating Pro Git (e.g., http://git-scm.com/book/de/v1). I have no idea if the terms they use are similar to the terms we use in the localized messages. It might make sense to: 1. Coordinate with those translators to make sure that the glossary terms are consistent. 2. Figure out how to harness those translators for manpage work. Why did Pro Git get so much volunteer translation done, and the manpages didn't? Did they advertise to the right people? Have an interface that made it easier for non-technical people to get involved? -Peff (Thanks for the pointer, excellent book) I do not know who was first, and who came later, but http://git-scm.com/book/de/v1/Git-Grundlagen-%C3%84nderungen-am-Repository-nachverfolgen uses versioniert as tracked LANG=de_DE.UTF-8 git status gives: nichts zum Commit vorgemerkt, aber es gibt unbeobachtete Dateien (benutzen Sie git add zum Beobachten) Does it make sense to replace beobachten with versionieren ? -- To unsubscribe from this list: send the line 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: error: core.autocrlf=input conflicts with core.eol=crlf
On 12/05/2014 09:10 AM, Torsten Bögershausen wrote: On 05.12.14 06:42, Alex Stangl wrote: Hi, There seems to be problems with the checks in the git code for conflicts between config values of core.autocrlf and core.eol. Because the various config files are read in separate passes, and the conflict check happens on the fly, it creates a situation where the order of the config file entries matters. This seems like a bug or at least a POLA violation -- ordering of lines within a section of a config file is not usually significant. Example: User has core.autocrlf=input in his ~/.gitconfig. In his project-level .git/config he wants to set core.autocrlf=false and core.eol=crlf. If the core.autocrlf=false comes first, then all is well and no conflict is reported. If the core.eol=crlf line comes first, then at the time this line is getting parsed, core.autocrlf is still set at input from ~/.gitconfig, and execution aborts: error: core.autocrlf=input conflicts with core.eol=crlf It seems like the conflict check should be made at the end of the config file parsing, not on the fly. I was tempted to create a patch, however I am unfamilar with the codebase, and didn't understand all the places where the config file parsing is called, so I'm not sure of the ramifications of any proposed change. A benefit of the current approach is that it reports the line number where it aborted in the config file. Trying to retain this and at the same time defer the check until the end could get complicated. Besides interaction between multiple levels of config files, the same sort of ordering issue can arise in conjunction with command-line config overrides. Example: User has core.autocrlf=input in his project-level .git/config. This command works fine: $ git -c core.autocrlf=false -c core.eol=crlf status This command blows up with conflict error: $ git -c core.eol=crlf -c core.autocrlf=false status I tested with git versions 1.9.3 and 2.1.0. Yes, this is a bug, if someone has a patch: it is welcome. Or a test case showing the problem is welcome too. Beside that, I am working on a patch to remove this restriction completely. I think that it is a legal combination to have a .gitattributes file like this: *.txt text And then set core.autocrlf=input # to avoid CRLF in the repo for e.g. *.c files, and core.eol=crlf # to have *.txt in CRLF in the working tree Which means do not touch any files, -- To unsubscribe from this list: send the line 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: error: core.autocrlf=input conflicts with core.eol=crlf
On Fri, Dec 05, 2014 at 04:55:51PM +0100, Torsten B?gershausen wrote: On 12/05/2014 09:10 AM, Torsten B?gershausen wrote: Or a test case showing the problem is welcome too. I mentioned some examples in my previous post. Here they are in condensed form. They assume core.eol isn't set in your global config. These work fine: $ mkdir -p /tmp/ex1_$$;cd /tmp/ex1_$$;git init;git config --global core.autocrlf input;git config core.autocrlf false;git config core.eol crlf;git status $ mkdir -p /tmp/ex3_$$;cd /tmp/ex3_$$;git init;git config --global core.autocrlf input;git -c core.autocrlf=false -c core.eol=crlf status These equivalents fail: $ mkdir -p /tmp/ex2_$$;cd /tmp/ex2_$$;git init;git config --global core.autocrlf input;git config core.eol crlf;git config core.autocrlf false;git status $ mkdir -p /tmp/ex4_$$;cd /tmp/ex4_$$;git init;git config --global core.autocrlf input;git -c core.eol=crlf -c core.autocrlf=false status Alex -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: bug report on update-index --assume-unchanged
Hi, On Sex, 2014-12-05 at 17:52 +0700, Duy Nguyen wrote: On Fri, Dec 5, 2014 at 1:55 PM, Johannes Sixt j...@kdbg.org wrote: Actually, it's a user error. When you set --assume-unchanged, then you give a promise to git that you do not change the files, and git does not have to check itself whether there is a change. But since you did not keep your promise, you get what you deserve. ;-) No, I marked with assume-unchanged *after* change the file , and not before. Else don't see what is the point of assume-unchanged if you really don't change the file. You are correct about the original idea behind --assume-unchanged. But over the time I think we bend over a bit and sort of support these use cases. For example, aecda37 (do not overwrite files marked assume unchanged - 2010-05-01). The change is one-liner, so I don't mind doing it. I think is the right thing Thanks, -- Sérgio M. B. -- To unsubscribe from this list: send the line 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] prompt: respect GIT_TERMINAL_PROMPT to disable terminal prompts
Jeff King p...@peff.net writes: On Thu, Dec 04, 2014 at 01:33:53PM -0800, Junio C Hamano wrote: Of course, from _our_ point of view, that somebody is not us. We do not have direct control, certainly from this codepath. Belated typofix - s/from /not /. Right, but in theory we can provoke gpg to do what we want when we spawn it. However, having had zero luck in convincing it to stop asking me for a passphrase recently in another thread, I do not know what magic command line option is required. :( I think it would be OK to merge the git handling of GIT_TERMINAL_PROMPT (i.e., the patch I sent), and somebody who runs into the issue with gpg and can figure out how to tame it can scratch their own itch later. I hate leaving things half-implemented or inconsistent, but I also don't know how to make gpg do what we want. And doing a partial solution seems better to me than holding the credential.c half hostage. Oh, no question about that. You are making things better by advancing one step at a time. Queued and will be moving to 'next' shortly. -- To unsubscribe from this list: send the line 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 07/23] expire_reflog(): use a lock_file for rewriting the reflog file
On Thu, Dec 04, 2014 at 06:51:36PM -0800, ronnie sahlberg wrote:
On Thu, Dec 4, 2014 at 3:08 PM, Michael Haggerty mhag...@alum.mit.edu
wrote:
We don't actually need the locking functionality, because we already
hold the lock on the reference itself,
No. You do need the lock.
The ref is locked only during transaction_commit()
Michael is saying, that you never want to modify the reflog, if you're not
holding
the lock on the corresponding ref. Or in other words, you may modify the
reflog, if
the corresponding ref is held. This used to be this way back then.
If you don't want to lock the reflog file and instead rely on the lock on
the ref itself you will need to
rework your patches so that the lock on the ref is taken already during,
for example, transaction_update_ref() instead.
But without doing those changes and moving the ref locking from _commit()
to _update_ref() you will risk reflog corruption/surprises
if two operations collide and both rewrite the reflog without any lock held.
which is how the reflog file is
locked. But the lock_file code still does some of the bookkeeping for
us and is more careful than the old code here was.
For example:
* Correctly handle the case that the reflog lock file already exists
for some reason or cannot be opened.
* Correctly clean up the lockfile if the program dies.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
builtin/reflog.c | 37 ++---
1 file changed, 22 insertions(+), 15 deletions(-)
diff --git a/builtin/reflog.c b/builtin/reflog.c
index a282e60..d344d45 100644
--- a/builtin/reflog.c
+++ b/builtin/reflog.c
@@ -349,12 +349,14 @@ static int push_tip_to_list(const char *refname,
const unsigned char *sha1, int
return 0;
}
+static struct lock_file reflog_lock;
+
static int expire_reflog(const char *refname, const unsigned char *sha1,
void *cb_data)
{
struct cmd_reflog_expire_cb *cmd = cb_data;
struct expire_reflog_cb cb;
struct ref_lock *lock;
- char *log_file, *newlog_path = NULL;
+ char *log_file;
struct commit *tip_commit;
struct commit_list *tips;
int status = 0;
@@ -372,10 +374,14 @@ static int expire_reflog(const char *refname, const
unsigned char *sha1, void *c
unlock_ref(lock);
return 0;
}
+
log_file = git_pathdup(logs/%s, refname);
if (!cmd-dry_run) {
- newlog_path = git_pathdup(logs/%s.lock, refname);
- cb.newlog = fopen(newlog_path, w);
+ if (hold_lock_file_for_update(reflog_lock, log_file, 0)
0)
+ goto failure;
+ cb.newlog = fdopen_lock_file(reflog_lock, w);
+ if (!cb.newlog)
+ goto failure;
}
cb.cmd = cmd;
@@ -423,10 +429,9 @@ static int expire_reflog(const char *refname, const
unsigned char *sha1, void *c
}
if (cb.newlog) {
- if (fclose(cb.newlog)) {
- status |= error(%s: %s, strerror(errno),
- newlog_path);
- unlink(newlog_path);
+ if (close_lock_file(reflog_lock)) {
+ status |= error(Couldn't write %s: %s, log_file,
+ strerror(errno));
} else if (cmd-updateref
(write_in_full(lock-lock_fd,
sha1_to_hex(cb.last_kept_sha1), 40) != 40
||
@@ -434,21 +439,23 @@ static int expire_reflog(const char *refname, const
unsigned char *sha1, void *c
close_ref(lock) 0)) {
status |= error(Couldn't write %s,
lock-lk-filename.buf);
- unlink(newlog_path);
- } else if (rename(newlog_path, log_file)) {
- status |= error(cannot rename %s to %s,
- newlog_path, log_file);
- unlink(newlog_path);
+ rollback_lock_file(reflog_lock);
+ } else if (commit_lock_file(reflog_lock)) {
+ status |= error(cannot rename %s.lock to %s,
+ log_file, log_file);
} else if (cmd-updateref commit_ref(lock)) {
status |= error(Couldn't set %s, lock-ref_name);
- } else {
- adjust_shared_perm(log_file);
}
}
- free(newlog_path);
free(log_file);
unlock_ref(lock);
return status;
+
+ failure:
+ rollback_lock_file(reflog_lock);
+ free(log_file);
+ unlock_ref(lock);
+
Re: [PATCH/RFC] doc: document error handling functions and conventions (Re: [PATCH 03/14] copy_fd: pass error message back through a strbuf)
Jeff King p...@peff.net writes: The only downside I can think of is that we may truncate the message in exceptional circumstances. But is it really any less helpful to say: error: unable to open file: some-incredibly-long-filename-aa... than printing out an extra 100 lines of a? And I mean the ... literally. I think mkerror() should indicate the truncation with a ..., just so that it is clear to the user. It should almost never happen, but when it does, it can be helpful to show the user that yes, we know we are truncating the message, and it is not that git truncated your filename during the operation. Is this truncation really a concern, and/or is there some other downside I'm not thinking of? I am more worried about variable length part pushing the information that is given later out to the right, e.g. error: missing file '%s' prevents us from doing X. Chomping to [1024] is not a good strategy for that kind of message; abbreviating %s to /path/name/... (again, with literally ...) would be. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 2/2] branch: allow -f with -m and -d
Michael J Gruber g...@drmicha.warpmail.net writes: What should -f -u and -f -l do, then, though? list + unset_upstream 1) usage_with_options(builtin_branch_usage, options); I would say there is nothing to force, so we ignore -f there. OK, and that is what the updated code does. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: bug report on update-index --assume-unchanged
Sérgio Basto ser...@serjux.com writes: On Sex, 2014-12-05 at 17:52 +0700, Duy Nguyen wrote: On Fri, Dec 5, 2014 at 1:55 PM, Johannes Sixt j...@kdbg.org wrote: Actually, it's a user error. When you set --assume-unchanged, then you give a promise to git that you do not change the files, and git does not have to check itself whether there is a change. But since you did not keep your promise, you get what you deserve. ;-) No, I marked with assume-unchanged *after* change the file , and not before. Else don't see what is the point of assume-unchanged if you really don't change the file. That unchanged is relative to what is in the index. Your promise is these paths I will not modify and in return you gain performance in git status as the promise allows Git not to check with lstat() if the files in the workng tree was modified and instead assume that you didn't change them. That is the point of assume-unchanged bit. If however you did something that made Git notice that you changed these paths marked with assume-unchanged bit anyway, then Git will, well, notice that they are not unchanged as you promised. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: bug report on update-index --assume-unchanged
Duy Nguyen pclo...@gmail.com writes: You are correct about the original idea behind --assume-unchanged. But over the time I think we bend over a bit and sort of support these use cases. For example, aecda37 (do not overwrite files marked assume unchanged - 2010-05-01). The change is one-liner, so I don't mind doing it. I think that was a misguided change to make the semantics muddy and to break the existing users who use the bit for its intended purpose (i.e. to avoid lstat() by promising that it is not necessary), and not bending over to support. Offhand, I doubt we would want to add more of the same kind. -- To unsubscribe from this list: send the line 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 09/23] expire_reflog(): extract two policy-related functions
On Fri, Dec 05, 2014 at 12:08:21AM +0100, Michael Haggerty wrote:
Extract two functions, reflog_expiry_prepare() and
reflog_expiry_cleanup(), from expire_reflog(). This is a further step
towards separating the code for deciding on expiration policy from the
code that manages the physical expiration.
This change requires a couple of local variables from expire_reflog()
to be turned into fields of struct expire_reflog_cb. More
reorganization of the callback data will follow in later commits.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
Reviewed-by: Stefan Beller sbel...@google.com
---
In fact, the work done in reflog_expire_cleanup() doesn't really need
to be done via a callback, because it doesn't need to be done while
the reference lock is held. But the symmetry between prepare and
cleanup is kindof nice. Perhaps some future policy decision will want
to do some final work under the reference lock?
But it would be easy to get rid of this third callback function and
have the callers do the work themselves after calling expire_reflog().
I don't have a string feeling either way.
builtin/reflog.c | 94
+++-
1 file changed, 52 insertions(+), 42 deletions(-)
diff --git a/builtin/reflog.c b/builtin/reflog.c
index 7bc6e0f..ebfa635 100644
--- a/builtin/reflog.c
+++ b/builtin/reflog.c
@@ -43,6 +43,8 @@ struct expire_reflog_cb {
unsigned long mark_limit;
struct cmd_reflog_expire_cb *cmd;
unsigned char last_kept_sha1[20];
+ struct commit *tip_commit;
+ struct commit_list *tips;
};
struct collected_reflog {
@@ -363,6 +365,54 @@ static int push_tip_to_list(const char *refname, const
unsigned char *sha1, int
return 0;
}
+static void reflog_expiry_prepare(const char *refname,
+ const unsigned char *sha1,
+ struct expire_reflog_cb *cb)
+{
+ if (!cb-cmd-expire_unreachable || !strcmp(refname, HEAD)) {
+ cb-tip_commit = NULL;
+ cb-unreachable_expire_kind = UE_HEAD;
+ } else {
+ cb-tip_commit = lookup_commit_reference_gently(sha1, 1);
+ if (!cb-tip_commit)
+ cb-unreachable_expire_kind = UE_ALWAYS;
+ else
+ cb-unreachable_expire_kind = UE_NORMAL;
+ }
+
+ if (cb-cmd-expire_unreachable = cb-cmd-expire_total)
+ cb-unreachable_expire_kind = UE_ALWAYS;
+
+ cb-mark_list = NULL;
+ cb-tips = NULL;
+ if (cb-unreachable_expire_kind != UE_ALWAYS) {
+ if (cb-unreachable_expire_kind == UE_HEAD) {
+ struct commit_list *elem;
+ for_each_ref(push_tip_to_list, cb-tips);
+ for (elem = cb-tips; elem; elem = elem-next)
+ commit_list_insert(elem-item, cb-mark_list);
+ } else {
+ commit_list_insert(cb-tip_commit, cb-mark_list);
+ }
+ cb-mark_limit = cb-cmd-expire_total;
+ mark_reachable(cb);
+ }
+}
+
+static void reflog_expiry_cleanup(struct expire_reflog_cb *cb)
+{
+ if (cb-unreachable_expire_kind != UE_ALWAYS) {
+ if (cb-unreachable_expire_kind == UE_HEAD) {
+ struct commit_list *elem;
+ for (elem = cb-tips; elem; elem = elem-next)
+ clear_commit_marks(elem-item, REACHABLE);
+ free_commit_list(cb-tips);
+ } else {
+ clear_commit_marks(cb-tip_commit, REACHABLE);
+ }
+ }
+}
+
static struct lock_file reflog_lock;
static int expire_reflog(const char *refname, const unsigned char *sha1,
void *cb_data)
@@ -371,8 +421,6 @@ static int expire_reflog(const char *refname, const
unsigned char *sha1, void *c
struct expire_reflog_cb cb;
struct ref_lock *lock;
char *log_file;
- struct commit *tip_commit;
- struct commit_list *tips;
int status = 0;
memset(cb, 0, sizeof(cb));
@@ -400,47 +448,9 @@ static int expire_reflog(const char *refname, const
unsigned char *sha1, void *c
cb.cmd = cmd;
- if (!cmd-expire_unreachable || !strcmp(refname, HEAD)) {
- tip_commit = NULL;
- cb.unreachable_expire_kind = UE_HEAD;
- } else {
- tip_commit = lookup_commit_reference_gently(sha1, 1);
- if (!tip_commit)
- cb.unreachable_expire_kind = UE_ALWAYS;
- else
- cb.unreachable_expire_kind = UE_NORMAL;
- }
-
- if (cmd-expire_unreachable = cmd-expire_total)
- cb.unreachable_expire_kind = UE_ALWAYS;
-
- cb.mark_list = NULL;
- tips = NULL;
- if (cb.unreachable_expire_kind != UE_ALWAYS) {
- if (cb.unreachable_expire_kind == UE_HEAD) {
-
Re: [PATCH] for_each_reflog_ent_reverse: turn leftover check into assertion
Jeff King p...@peff.net writes: I _think_ the patch below is also applicable to the code before my boundary-fixing patch. But the rearranging also made me more confident in it. Yeah, thanks for a fix. -- 8 -- Subject: for_each_reflog_ent_reverse: turn leftover check into assertion Our loop should always process all lines, even if we hit the beginning of the file. We have a conditional after the loop ends to double-check that there is nothing left and to process it. But this should never happen, and is a sign of a logic bug in the loop. Let's turn it into a BUG assertion. Signed-off-by: Jeff King p...@peff.net --- Of course I cannot say something like this can never happen; the old code was wrong to handle this case without a nagging feeling that I am missing something, so extra careful eyes are appreciated (and are why I would rather have an assert here than removing the code and silently dropping lines if I am wrong). refs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/refs.c b/refs.c index ccb8834..1f77fa6 100644 --- a/refs.c +++ b/refs.c @@ -3451,7 +3451,7 @@ int for_each_reflog_ent_reverse(const char *refname, each_reflog_ent_fn fn, void } if (!ret sb.len) - ret = show_one_reflog_ent(sb, fn, cb_data); + die(BUG: reverse reflog parser had leftover data); fclose(logfp); strbuf_release(sb); -- To unsubscribe from this list: send the line 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 07/23] expire_reflog(): use a lock_file for rewriting the reflog file
On Thu, Dec 04, 2014 at 04:23:31PM -0800, Jonathan Nieder wrote:
Michael Haggerty wrote:
We don't actually need the locking functionality, because we already
hold the lock on the reference itself, which is how the reflog file is
locked. But the lock_file code still does some of the bookkeeping for
us and is more careful than the old code here was.
As you say, the ref lock takes care of mutual exclusion, so we do not
have to be too careful about compatibility with other tools that might
not know to lock the reflog. And this is not tying our hands for a
future when I might want to lock logs/refs/heads/topic/1 while
logs/refs/heads/topic still exists as part of the implementation of
git mv topic/1 topic.
Stefan and I had forgotten about that guarantee when looking at that
kind of operation --- thanks for the reminder.
Should updates to the HEAD reflog acquire HEAD.lock? (They don't
currently.)
[...]
--- a/builtin/reflog.c
+++ b/builtin/reflog.c
@@ -349,12 +349,14 @@ static int push_tip_to_list(const char *refname,
const unsigned char *sha1, int
return 0;
}
+static struct lock_file reflog_lock;
If this lockfile is only used in that one function, it can be declared
inside the function.
If it is meant to be used throughout the 'git reflog' command, then it
can go near the top of the file.
After the series completes, this lock is only used in reflog_expire.
So I'd rather move it inside the function? Then we could run the reflog_expire
function in parallel for different locks in theory?
+
static int expire_reflog(const char *refname, const unsigned char *sha1,
void *cb_data)
{
struct cmd_reflog_expire_cb *cmd = cb_data;
struct expire_reflog_cb cb;
struct ref_lock *lock;
- char *log_file, *newlog_path = NULL;
+ char *log_file;
struct commit *tip_commit;
struct commit_list *tips;
int status = 0;
@@ -372,10 +374,14 @@ static int expire_reflog(const char *refname, const
unsigned char *sha1, void *c
unlock_ref(lock);
return 0;
}
+
log_file = git_pathdup(logs/%s, refname);
if (!cmd-dry_run) {
- newlog_path = git_pathdup(logs/%s.lock, refname);
- cb.newlog = fopen(newlog_path, w);
+ if (hold_lock_file_for_update(reflog_lock, log_file, 0) 0)
+ goto failure;
hold_lock_file_for_update doesn't print a message. Code to print one
looks like
if (hold_lock_file_for_update(reflog_lock, log_file, 0) 0) {
unable_to_lock_message(log_file, errno, err);
error(%s, err.buf);
goto failure;
}
(A patch in flight changes that to
if (hold_lock_file_for_update(reflog_lock, log_file, 0, err) 0) {
error(%s, err.buf);
goto failure;
}
)
+ cb.newlog = fdopen_lock_file(reflog_lock, w);
+ if (!cb.newlog)
+ goto failure;
Hm. lockfile.c::fdopen_lock_file ought to use xfdopen to make this
case impossible. And xfdopen should use try_to_free_routine() and
try again on failure.
[...]
@@ -423,10 +429,9 @@ static int expire_reflog(const char *refname, const
unsigned char *sha1, void *c
}
if (cb.newlog) {
- if (fclose(cb.newlog)) {
- status |= error(%s: %s, strerror(errno),
- newlog_path);
- unlink(newlog_path);
+ if (close_lock_file(reflog_lock)) {
+ status |= error(Couldn't write %s: %s, log_file,
+ strerror(errno));
Style nit: error messages usually start with a lowercase letter
(though I realize nearby examples are already inconsistent).
commit_lock_file() can take care of the close_lock_file automatically.
[...]
@@ -434,21 +439,23 @@ static int expire_reflog(const char *refname, const
unsigned char *sha1, void *c
close_ref(lock) 0)) {
status |= error(Couldn't write %s,
lock-lk-filename.buf);
- unlink(newlog_path);
- } else if (rename(newlog_path, log_file)) {
- status |= error(cannot rename %s to %s,
- newlog_path, log_file);
- unlink(newlog_path);
+ rollback_lock_file(reflog_lock);
+ } else if (commit_lock_file(reflog_lock)) {
+ status |= error(cannot rename %s.lock to %s,
+ log_file, log_file);
Most callers say unable to commit reflog '%s', log_file to hedge their
bets in case the close failed (which may be what you were avoiding
above.
errno is meaningful when commit_lock_file fails, making a more
detailed diagnosis from strerror(errno) possible.
Thanks,
Jonathan
--
To
Re: [PATCH 07/23] expire_reflog(): use a lock_file for rewriting the reflog file
Stefan Beller sbel...@google.com writes: +static struct lock_file reflog_lock; If this lockfile is only used in that one function, it can be declared inside the function. If it is meant to be used throughout the 'git reflog' command, then it can go near the top of the file. After the series completes, this lock is only used in reflog_expire. So I'd rather move it inside the function? Then we could run the reflog_expire function in parallel for different locks in theory? I am not sure about the parallel part, but I would imagine that it is an essential prerequisite to move this outside the client code if we want to later replace the backing storage of refs and reflogs outside the filesystem, so from that point of view, I think the suggestion 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: [PATCH 07/23] expire_reflog(): use a lock_file for rewriting the reflog file
On Fri, Dec 5, 2014 at 11:32 AM, Junio C Hamano gits...@pobox.com wrote: Stefan Beller sbel...@google.com writes: +static struct lock_file reflog_lock; If this lockfile is only used in that one function, it can be declared inside the function. If it is meant to be used throughout the 'git reflog' command, then it can go near the top of the file. After the series completes, this lock is only used in reflog_expire. So I'd rather move it inside the function? Then we could run the reflog_expire function in parallel for different locks in theory? I am not sure about the parallel part, but I would imagine that it is an essential prerequisite to move this outside the client code if we want to later replace the backing storage of refs and reflogs outside the filesystem, so from that point of view, I think the suggestion makes sense. Thanks. Sorry for the confusion. With parallel I mean, that in theory we could have multiple threads running reflog expire at the same time in the same program. Having the static struct lock_file reflog_lock; around, we can only process one reflog at a time, as that is holding the lock_file struct. I am not saying we want to go multi-threading any time soon if at all. Just that it would be easier to do, if not having the lock file as a file-global variable instead of a variable inside a function. 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
[PATCH] refs.c: let fprintf handle the formatting
Instead of calculating, whether to put a plus or minus sign, offload
the responsibilty to the fprintf function.
Signed-off-by: Stefan Beller sbel...@google.com
---
refs.c | 7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/refs.c b/refs.c
index 40c5591..1147216 100644
--- a/refs.c
+++ b/refs.c
@@ -3972,12 +3972,9 @@ static int expire_reflog_ent(unsigned char *osha1,
unsigned char *nsha1,
printf(prune %s, message);
} else {
if (cb-newlog) {
- char sign = (tz 0) ? '-' : '+';
- int zone = (tz 0) ? (-tz) : tz;
- fprintf(cb-newlog, %s %s %s %lu %c%04d\t%s,
+ fprintf(cb-newlog, %s %s %s %lu %+05d\t%s,
sha1_to_hex(osha1), sha1_to_hex(nsha1),
- email, timestamp, sign, zone,
- message);
+ email, timestamp, tz, message);
hashcpy(cb-last_kept_sha1, nsha1);
}
if (cb-flags EXPIRE_REFLOGS_VERBOSE)
--
2.2.0
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] refs.c: let fprintf handle the formatting
On Fri, Dec 5, 2014 at 11:53 AM, Stefan Beller sbel...@google.com wrote:
Instead of calculating, whether to put a plus or minus sign, offload
the responsibilty to the fprintf function.
Signed-off-by: Stefan Beller sbel...@google.com
---
Ah! I forgot the --notes switch when sending out the email:
Michael,
please queue this on top of your reflog series.
More to come.
Thanks,
Stefan
refs.c | 7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/refs.c b/refs.c
index 40c5591..1147216 100644
--- a/refs.c
+++ b/refs.c
@@ -3972,12 +3972,9 @@ static int expire_reflog_ent(unsigned char *osha1,
unsigned char *nsha1,
printf(prune %s, message);
} else {
if (cb-newlog) {
- char sign = (tz 0) ? '-' : '+';
- int zone = (tz 0) ? (-tz) : tz;
- fprintf(cb-newlog, %s %s %s %lu %c%04d\t%s,
+ fprintf(cb-newlog, %s %s %s %lu %+05d\t%s,
sha1_to_hex(osha1), sha1_to_hex(nsha1),
- email, timestamp, sign, zone,
- message);
+ email, timestamp, tz, message);
hashcpy(cb-last_kept_sha1, nsha1);
}
if (cb-flags EXPIRE_REFLOGS_VERBOSE)
--
2.2.0
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
GIT: cherry-picking includes changes from a different commit
I noticed when cherry-picking a commit it includes changes from this commit and also changes from a different commit as the one I was picking. I was in contact with jast on IRC, who noticed the issue, but didn't had time to look further. For a description of my problem, please read http://stackoverflow.com/questions/27220638/git-cherry-pick-info-about-picke d-commits git clone git://github.com/MythTV/mythtv.git git checkout fixes/0.27 git cherry-pick 30df98ce5d11b69d0b5c5114a9007cdfc79a7e9b # from master # commit also picked: 17f17e1fc51b3b4017e08f5ea35c8a7b5a64eeec # resulting in a conflict For the commits and before/conflict files, see https://gist.github.com/FritzHerbers/4f9b0990b6bca15a70eb As nobody could believe me, that changes from another commit are included, is something wrong, or is it new behavior? Why did it happen? With which command, can I have a listing of intermediate commits picked? How can I automate such situations? Fritz git version 1.9.1 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: bug report on update-index --assume-unchanged
From: Junio C Hamano gits...@pobox.com Sérgio Basto ser...@serjux.com writes: On Sex, 2014-12-05 at 17:52 +0700, Duy Nguyen wrote: On Fri, Dec 5, 2014 at 1:55 PM, Johannes Sixt j...@kdbg.org wrote: Actually, it's a user error. When you set --assume-unchanged, then you give a promise to git that you do not change the files, and git does not have to check itself whether there is a change. But since you did not keep your promise, you get what you deserve. ;-) No, I marked with assume-unchanged *after* change the file , and not before. Else don't see what is the point of assume-unchanged if you really don't change the file. That unchanged is relative to what is in the index. Your promise is these paths I will not modify and in return you gain performance in git status as the promise allows Git not to check with lstat() if the files in the workng tree was modified and instead assume that you didn't change them. That is the point of assume-unchanged bit. If however you did something that made Git notice that you changed these paths marked with assume-unchanged bit anyway, then Git will, well, notice that they are not unchanged as you promised. The problem here is that there is no guidance on what those actions are that may make git 'notice'. The man page git-update-index isn't as clear as it could be. Using --really-refresh being one option that would make git notice, but I wouldn't know when that is used. Part of the implied question is why git commit . would notice when when git commit -a didn't appear to. So it's unclear as to what the user should have expected. (Note, I don't use assume-unchanged myself so this is more about supporting the user/manual clarification. It is mentioned moderately often on stackoverflow etc.) -- 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 07/23] expire_reflog(): use a lock_file for rewriting the reflog file
Stefan Beller sbel...@google.com writes: After the series completes, this lock is only used in reflog_expire. So I'd rather move it inside the function? Then we could run the reflog_expire function in parallel for different locks in theory? I am not sure about the parallel part, but I would imagine that it is an essential prerequisite to move this outside the client code if we want to later replace the backing storage of refs and reflogs outside the filesystem, so from that point of view, I think the suggestion makes sense. Thanks. Sorry for the confusion. With parallel I mean,... There is no confusion. I understand exactly what you meant. What I said was not sure was if parallel is a practical enough possiblity to include into the set of value propositions the suggested change to move the lock out of the client may give us. In other words, With this change, doing a parallel will become a lot easier---Really? It probably is not one of the harder part of the problem if you really want to go parallel was the discourse I had in my mind. ;-) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: bug report on update-index --assume-unchanged
Philip Oakley philipoak...@iee.org writes: The problem here is that there is no guidance on what those actions are that may make git 'notice' I think the guidance the users need is the one j6t has given already in the upthread: If you are promising Git you are not going to touch a path, do not touch it. Bad things may happen. There is no need to say if you touched this way or that way, then you might get lucky. because the lucky part is not designed. As we find more codepaths that can rely on the promise by the user, we may decide to take advantage of that promise even further and the lucky/unlucky equation _will_ change when that happens. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: bug report on update-index --assume-unchanged
Junio C Hamano gits...@pobox.com writes: If however you did something that made Git notice that you changed these paths marked with assume-unchanged bit anyway, then Git will, well, notice that they are not unchanged as you promised. By the way, this cuts both ways. I would not bother checking with the current codebase, but I know it used to be that when we have a blob object name and need a temporary file that holds the content for read-only purpose (e.g. passing a pair of files to external diff driver), we allowed Git to reuse files in the working tree whose blob object name we knew. This is of course because it is faster than inflating the blob contents out of the object store and writing a new file. That codepath is allowed to borrow the working tree file as such a temporary file for read-only purpose, when a file is stat-clean (i.e. its contents is known to match the blob object recorded in the index). A file with assume-unchanged bit set is treated exactly the same way, because the user promised not to modify it. If the user broke the promise, then an external diff driver would have been given a file whose contents does not actually match the wanted blob object, resulting in an incorrect diff output. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Announcing a new (prototype) git-remote-hg tool
Hi, I've (re)started work on a longstanding idea of mine of having a git tool talking the mercurial wire protocol directly. I'm now at a stage where the tool can clone and pull/fetch from mercurial. As it is a prototype, there are many things that it doesn't handle (like named branches, bookmarks, phases, obsolescence markers), but it currently transposes a complete mercurial dag to git and maintains metadata about the original mercurial data. Code on https://github.com/glandium/git-remote-hg It doesn't support push, but support for that should come in the coming weeks. More background on http://glandium.org/blog/?p=3382 This is meant to be a prototype, and will stay that way for now. It's a validation that this can actually work. Now that I have pull support I know I can make it push. I'm currently evaluating what the final tool would look like. I'm *very* tempted to implement it in C, based on core git code, because there are many things that this helper does that would be so much easier to do with direct access to git's guts. And that wouldn't require more dependencies than git currently has: it would just need curl and ssh, and git already uses both. If I were to go in that direction, would you consider integrating it in git core? If not, would you rather see git helpers to make this git-remote-hg helper more efficient? Cheers, 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: [PATCH] Show number of commits being rebased interactively
I see nobody commented on this, which probably fell into cracks. Even though I am personally not very interested, I obviously am not the only user of Git, and there may be others who are interested in a change like this. Onno Kortmann o...@gmx.net writes: Hi again, oops, I realized that my MUA mangled the patch, even though it shouldn't. Here it is again, with a bit more description. --- These lines above --- will become the only log message text, which is probably not what you intended. Use -- 8 -- marker instead (that is a perforation line with a pair of scissors on it)? During 'rebase -i', one wrong edit in a long rebase session might inadvertently drop commits. This change shows the total number of commits in the comments below the commit list. After the rebase edit, the number can be quickly compared to the line number of the last commit - by scrolling to the last entry in the rebase TODO list. This gives peace of mind that no commits have been lost in the edit. Signed-off-by: Onno Kortmann o...@gmx.net --- git-rebase--interactive.sh | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh index b64dd28..b266dc0 100644 --- a/git-rebase--interactive.sh +++ b/git-rebase--interactive.sh @@ -1031,9 +1031,11 @@ test -s $todo || echo noop $todo test -n $autosquash rearrange_squash $todo test -n $cmd add_exec_commands $todo +commitcount=$(git stripspace --strip-comments $todo | wc -l) Does this count the number of commits? I suspect it at least needs to filter x|exec out. cat $todo EOF -$comment_char Rebase $shortrevisions onto $shortonto +$comment_char Rebase $shortrevisions onto $shortonto ($commitcount commit(s)) EOF append_todo_help git stripspace --comment-lines $todo \EOF -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: bug report on update-index --assume-unchanged
On Sex, 2014-12-05 at 10:30 -0800, Junio C Hamano wrote: Your promise is these paths I will not modify and in return you gain performance in git status yeah so --assume-unchanged is for administrators of git , like I write I change first and --assume-unchanged after and never change it again . But if it is a tool for administration of git , I don't what to say ... put it in a gitadmin command . I hate git and this is one one the reason . -- Sérgio M. B. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: bug report on update-index --assume-unchanged
Sérgio Basto ser...@serjux.com writes: On Sex, 2014-12-05 at 10:30 -0800, Junio C Hamano wrote: Your promise is these paths I will not modify and in return you gain performance in git status yeah so --assume-unchanged is for administrators of git ,... Not at all. Administrators would typically not know (and they would not want to know) what part of your particular project tree you as an individual developer is not working on day to day. This was added primarily by normal users (or as a response to a request from normal users) on slow filesystems (e.g. Cygwin). When they are working on one part of their project tree and know they are not touching other parts, they wanted to skip slowness of having to lstat(2) all paths in the project tree to detect what changed during git status. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: bug report on update-index --assume-unchanged
From: Junio C Hamano gits...@pobox.com Philip Oakley philipoak...@iee.org writes: The problem here is that there is no guidance on what those actions are that may make git 'notice' I think the guidance the users need is the one j6t has given already in the upthread: If you are promising Git you are not going to touch a path, do not touch it. Bad things may happen. There is no need to say if you touched this way or that way, then you might get lucky. because the lucky part is not designed. As we find more codepaths that can rely on the promise by the user, we may decide to take advantage of that promise even further and the lucky/unlucky equation _will_ change when that happens. However the man page's statement 'When the assume unchanged bit is on, Git stops checking the working tree files for possible modifications, so you need to manually unset the bit to tell Git when you change the working tree file.' can easily be understood the way Sergio has described. Git stops checking so it won't notice any changes, which is a contract it doesn't keep. Perhaps the man page itself needs rewording to be more firm that the user should NOT change the file. The contract is with the user not to change, rather than a contract by Git not to look. Even with that man page change, it would not solve many of the user X-Y problems where what they want is an --ignore-this flag for a file which does give that alternate contract that 'git won't look until the flag has been cleared. -- 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: Announcing a new (prototype) git-remote-hg tool
Mike Hommey wrote: I'm currently evaluating what the final tool would look like. I'm *very* tempted to implement it in C, based on core git code, because there are many things that this helper does that would be so much easier to do with direct access to git's guts. And that wouldn't require more dependencies than git currently has: it would just need curl and ssh, and git already uses both. If I were to go in that direction, would you consider integrating it in git core? Yes --- I would like this a lot. The general trend has been to carry fewer contrib-style tools in-tree, since the problem of discovering tools built on top of git is not as hard as it used to be. What you describe above seems to be a bit of an exception: - libgit.a in its current state evolves too quickly for it to be convenient for out-of-tree tools to use. cgit http://git.zx2c4.com/cgit/ uses git pinned to a particular version as a submodule to get around this, which is fussy and has bad implications for remembering to get security updates. - an in-tree user of libgit.a would be useful as a reference example to use to try to make libgit.a into be a better library internally (and eventually expose e.g. by merging with libgit2 as something outside tools can link to, I hope) - if it makes sense to help people using the current remote helper in contrib to migrate to this, it could be convenient for users In other words, although in the long term I would be happiest if libgit becomes good enough to let this project live in a separate tree and link to it, it's tempting to build this in-tree because we're not there yet. Some other alternatives: - using libgit2 https://libgit2.github.com/ - improving git plumbing (e.g., with new fast-import commands) or exposing a small library with a stable API for the tool's use I haven't thought it through carefully but at the moment I like the in-tree approach best. Thanks, Jonathan -- To unsubscribe from this list: send the line 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] Improve --assume-unchanged in the git update-index man page
Hi Junio, this is my quick attempt at a patch for the man page. In $gmane/260837 the --assume-unchanged flag was reported as buggy because of a misunderstanding about what it is being promised. This patch looks to be more assertive in stating what promise is being made. While at it correct a confusing pluralisation of the option. Philip Oakley (1): doc: make clear --assume-unchanged's user contract Documentation/git-update-index.txt | 9 + 1 file changed, 5 insertions(+), 4 deletions(-) -- 1.9.4.msysgit.0 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] doc: make clear --assume-unchanged's user contract
Many users misunderstand the --assume-unchanged contract, believing it means Git won't look at the flagged file. Be explicit that the --assume-unchanged contract is by the user that they will NOT change the file so that Git does not need to look (and expend, fore example, lstat(2) cycles) Signed-off-by: Philip Oakley philipoak...@iee.org --- Documentation/git-update-index.txt | 9 + 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/Documentation/git-update-index.txt b/Documentation/git-update-index.txt index 929869b..c045509 100644 --- a/Documentation/git-update-index.txt +++ b/Documentation/git-update-index.txt @@ -82,13 +82,14 @@ OPTIONS Set the execute permissions on the updated files. --[no-]assume-unchanged:: - When these flags are specified, the object names recorded + When this flag is specified, the object names recorded for the paths are not updated. Instead, these options set and unset the assume unchanged bit for the - paths. When the assume unchanged bit is on, Git stops + paths. When the assume unchanged bit is on, the user promise + is not to change the file, so Git stops checking the working tree files for possible - modifications, so you need to manually unset the bit to - tell Git when you change the working tree file. This is + modifications, so when you change the working tree file you + need to manually unset the bit to tell Git . This is sometimes helpful when working with a big project on a filesystem that has very slow lstat(2) system call (e.g. cifs). -- 1.9.4.msysgit.0 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
git bundle vs git rev-list
Hello all – I am working to create a wrapper around git bundle to synchronize of git repos via sneakernet from network ‘a’ to network ‘b’ transfer on a fairly frequent basis (daily to weekly). Network ‘b’ has a gatekeeper who is persnickety about what content might end up on his network. The gatekeeper wants to know about the content being transferred. I’ve come up with a scheme to list the final form of all files included in the bundle in whole or in part, see the psuedo code below: # BEGIN PSEUDOCODE #Create the bundle git bundle create out.bundle --all --since=last_bundle_date #Get list of commits included_commits = git rev-list --all --since=last_bundle_date #For each commit, get the immediate parent(s), and find objects in its parents' tree that are not in its tree foreach commit in included_commits: #Get all blobs in this commit's tree, map blob to file name CommitBlobsMapToFilename = Process(git ls-tree -r commit) #Now find the parent commit(s) ParentCommits = git rev-list --parents -n 1 commit foreach parent in ParentCommits: #Get all blobs in the parent's tree ParentBlobsMapToFilename = Process(git ls-tree -r parent) #Find blobs in this commit's tree that are not in the parent's commit tree NewBlobs = setdiff(CommitBlobsMapToFilename , ParentBlobsMapToFilename); #Write each new blob contents to a unique filename foreach blob in NewBlobs filename = CommitBlobsMapToFilename(blob) filename = makeUnique(filename) git show blob filename # END PSEUDOCODE This scheme has worked well, but this is approach is predicated on the assumption that git bundle create –all –since=last_bundle uses the same commits that are returned by git rev-list --all --since=last_bundle However, I’ve noticed a scenario where that is not the case. I create a bundle using --since=yesterday, where no activity has been made within the past few days. As expected, 'git rev-list --all --since=yesterday' returns 0 commits. However, the command 'git bundle create --all --since=yesterday' creates a bundle containing the full history. Tags seem to be the culprit, but I don’t know why. I do notice in the output of git bundle that it mentions “skipping ref …” and “skipping tag …”, and sure enough all branches and most tags are shown as being skipped. However there are a few tags that are missing from that list. If I use --branches rather than --all as the limiter, then all is well. In that case, git rev-list still returns 0 commits, and git bundle reports that it is refusing to make an empty bundle, as expected. So after all that, I have a two questions: 1. Any thoughts on why a tag would be included by 'git bundle', when 'git rev-list' with the same arguments returns empty? 2. Is there a way to list commits contained in the bundle file itself? This seems like it would be more robust than trying to re-create the commit list via 'git rev-list'. Thanks, Jesse -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Accept-language test fails on Mac OS
Test #25 'git client sends Accept-Language based on LANGUAGE, LC_ALL,
LC_MESSAGES and LANG' in t5550 fails consistently on my mac, and has
since the test was introduced. Test 26 and 27 ('git client sends
Accept-Language with many preferred languages' and 'git client does
not send Accept-Language') seem fine.
I'm building git with NO_GETTEXT=1, which may be an issue? But in that
case the test should probably be gated on gettext?
--
To unsubscribe from this list: send the line 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: Announcing a new (prototype) git-remote-hg tool
From: Mike Hommey m...@glandium.org Hi, I've (re)started work on a longstanding idea of mine of having a git tool talking the mercurial wire protocol directly. I'm now at a stage where the tool can clone and pull/fetch from mercurial. As it is a prototype, there are many things that it doesn't handle (like named branches, bookmarks, phases, obsolescence markers), but it currently transposes a complete mercurial dag to git and maintains metadata about the original mercurial data. Code on https://github.com/glandium/git-remote-hg It doesn't support push, but support for that should come in the coming weeks. More background on http://glandium.org/blog/?p=3382 This is meant to be a prototype, and will stay that way for now. It's a validation that this can actually work. Now that I have pull support I know I can make it push. I'm currently evaluating what the final tool would look like. I'm *very* tempted to implement it in C, based on core git code, because there are many things that this helper does that would be so much easier to do with direct access to git's guts. And that wouldn't require more dependencies than git currently has: it would just need curl and ssh, and git already uses both. If I were to go in that direction, would you consider integrating it in git core? If not, would you rather see git helpers to make this git-remote-hg helper more efficient? You may also be interested in https://felipec.wordpress.com/2013/08/26/whats-new-in-git-v1-8-4-remote-hg/ and https://github.com/felipec/git-remote-hg. Though Filipe's unique work style hasn't found favour locally. see also https://github.com/buchuki/gitifyhg/wiki/List-of-git-hg-bridges 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] doc: make clear --assume-unchanged's user contract
Philip Oakley philipoak...@iee.org writes: Many users misunderstand the --assume-unchanged contract, believing it means Git won't look at the flagged file. Yeah, I suspect that many of them come from how-tos floating on the 'net (e.g. stackoverflow) that are misguided or outright incorrect. I've stopped correcting people who advise you can flip this bit and squelch your changes from appearing in diffs number of years ago. Be explicit that the --assume-unchanged contract is by the user that they will NOT change the file so that Git does not need to look (and expend, fore example, lstat(2) cycles) Yes, but so that Git does not ... part is already very clear in the existing part of the document. Stressing that this is the user making a promise to help Git help the user is indeed a good idea. Signed-off-by: Philip Oakley philipoak...@iee.org --- Documentation/git-update-index.txt | 9 + 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/Documentation/git-update-index.txt b/Documentation/git-update-index.txt index 929869b..c045509 100644 --- a/Documentation/git-update-index.txt +++ b/Documentation/git-update-index.txt @@ -82,13 +82,14 @@ OPTIONS Set the execute permissions on the updated files. --[no-]assume-unchanged:: - When these flags are specified, the object names recorded + When this flag is specified, the object names recorded Thanks. We are talking about a single flag bit. for the paths are not updated. Instead, these options set and unset the assume unchanged bit for the this option sets/unsets? - paths. When the assume unchanged bit is on, Git stops + paths. When the assume unchanged bit is on, the user promise + is not to change the file, so Git stops the user promises not to. checking the working tree files for possible - modifications, so you need to manually unset the bit to - tell Git when you change the working tree file. This is + modifications, so when you change the working tree file you + need to manually unset the bit to tell Git . This is This reads much better than the original, but you may want to go a bit further. How about removing the original a bit more, like so: ... the user promises not to change the file and allows Git to assume that the working tree file matches what is recorded in the index. If you want to change the working tree file, you need to unset the bit to tell Git. This is sometimes helpful when working with a big project on a filesystem that has very slow lstat(2) system call (e.g. cifs). 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: Accept-language test fails on Mac OS
Michael Blume blume.m...@gmail.com writes:
Test #25 'git client sends Accept-Language based on LANGUAGE, LC_ALL,
LC_MESSAGES and LANG' in t5550 fails consistently on my mac, and has
since the test was introduced. Test 26 and 27 ('git client sends
Accept-Language with many preferred languages' and 'git client does
not send Accept-Language') seem fine.
I'm building git with NO_GETTEXT=1, which may be an issue? But in that
case the test should probably be gated on gettext?
I recall queuing a SQUASH??? on top of the posted patch; does these
tests pass with it reverted?
--
To unsubscribe from this list: send the line 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: Announcing a new (prototype) git-remote-hg tool
On Fri, Dec 05, 2014 at 02:13:19PM -0800, Jonathan Nieder wrote: Mike Hommey wrote: I'm currently evaluating what the final tool would look like. I'm *very* tempted to implement it in C, based on core git code, because there are many things that this helper does that would be so much easier to do with direct access to git's guts. And that wouldn't require more dependencies than git currently has: it would just need curl and ssh, and git already uses both. If I were to go in that direction, would you consider integrating it in git core? Yes --- I would like this a lot. I'm concerned that this tool will have drawbacks that Felipe's remote-hg does not. And I can well imagine that it may, as that tool builds on Mercurial's API, which will probably handle some corner cases differently. This isn't to disparage Mike's attempt; it will probably have some upsides, too. But given that the approaches are so different, it does not seem obvious to me that one will always be better than the other. One of the nice things about spinning remote-hg out of the core repo is that it means we do not have to endorse a particular implementation, and they can compete with each other on their merits. I would very much hate to see Felipe's remote-hg project wither and die just because another implementation becomes the de facto standard by being included in git.git. It's a proven tool, and this new thing is not yet. It's a shame that both squat on the name remote-hg, because it makes it difficult to tell the two apart. But of course that is the only way to make git clone hg::... work. Maybe we need a layer of indirection? :) - libgit.a in its current state evolves too quickly for it to be convenient for out-of-tree tools to use. cgit http://git.zx2c4.com/cgit/ uses git pinned to a particular version as a submodule to get around this, which is fussy and has bad implications for remembering to get security updates. I'm not sure that this approach is any better than carrying something in contrib/ in git.git. If I refactor a function in libgit.a, I notice breakage in the callers because it no longer compiles, or because I am thorough and look at the implications to git callers. I do _not_ want to be responsible for making sure that contrib/* still builds. That is the problem of the maintainer of the contrib/ project in question. That may sound a little selfish, but I think that is what it means to be in contrib, and not in the regular tree. So once you realize that is the burden of the contrib/ author to fix breakages, then the process is: git pull cd contrib/c-remote-hg make # oops, it broke fix fix fix That is not any different than: git submodule add git... git submodule update make # oops, it broke fix fix fix The hard part is not how you pull changes from the new git into your tree. It is the fact that upstream may be breaking the interface behind your back. And your best bet is to aggressively merge with upstream, rather than trying to track only occasional release versions. Of course, if you meant to _really_ carry it in-tree, not in contrib/, then none of that applies. But then I worry doubly about the endorsement issue. -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: Accept-language test fails on Mac OS
On Fri, Dec 5, 2014 at 2:51 PM, Junio C Hamano gits...@pobox.com wrote:
Michael Blume blume.m...@gmail.com writes:
Test #25 'git client sends Accept-Language based on LANGUAGE, LC_ALL,
LC_MESSAGES and LANG' in t5550 fails consistently on my mac, and has
since the test was introduced. Test 26 and 27 ('git client sends
Accept-Language with many preferred languages' and 'git client does
not send Accept-Language') seem fine.
I'm building git with NO_GETTEXT=1, which may be an issue? But in that
case the test should probably be gated on gettext?
I recall queuing a SQUASH??? on top of the posted patch; does these
tests pass with it reverted?
The test fails both on pu and on 7567fad which is prior to the
SQUASH??? commit, so the squash does not seem to change anything.
--
To unsubscribe from this list: send the line 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 bundle vs git rev-list
Jesse Hopkins jesse.h...@gmail.com writes: 2. Is there a way to list commits contained in the bundle file itself? This seems like it would be more robust than trying to re-create the commit list via 'git rev-list'. git bundle list-heads o.bndl shows the positive endpoints, but there is no corresponding git bundle list-prereq that shows the prerequisite commits. Running git bundle verify o.bndl in an empty directory will show the negative endpoints that are required to be in the receiving repository in its error message, e.g. $ git bundle verify ~/w/git.git/o.bndle error: Repository lacks these prerequisite commits: error: bf404025edf1d7f5a69aa07cbaa88622e9d528df error: 15ab2081fff5b234ec5705a8645d39c1fdcf204c ... so collecting them would be one way to substitute list-prereq. Once you learned the positive and negative endpoints, running git rev-list --objects $positive_ones --not $negative_ones should list all the objects contained in the bundle. -- To unsubscribe from this list: send the line 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: Announcing a new (prototype) git-remote-hg tool
Jeff King wrote: One of the nice things about spinning remote-hg out of the core repo is that it means we do not have to endorse a particular implementation, and they can compete with each other on their merits. True. [...] It's a shame that both squat on the name remote-hg, because it makes it difficult to tell the two apart. But of course that is the only way to make git clone hg::... work. Maybe we need a layer of indirection? :) If the helpers are roughly interchangeable (that is, if you can switch between fetching using each one into the same on-disk git repository), then picking one to symlink as git-remote-hg in your $PATH should be enough. If they don't have that level of interoperability, then there's an argument to be made that the URLs shouldn't be the same. Unfortunately url.*.insteadof rules are resolved at fetch time instead of being resolved once and the result recorded in .git/config. So yes, it seems like a way to have abbreviations for URLs (e.g., hg:: meaning hg+mh:: or hg+fc::) that get resolved at clone time would be useful. It's a layer of indirection we don't provide. :/ Thanks, Jonathan -- To unsubscribe from this list: send the line 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 bundle vs git rev-list
On Fri, Dec 05, 2014 at 03:36:18PM -0700, Jesse Hopkins wrote: 1. Any thoughts on why a tag would be included by 'git bundle', when 'git rev-list' with the same arguments returns empty? I think the answer to this is found in the git rev-list manpage: List commits that are reachable by following the parent links from the given commit(s), but exclude commits that are reachable from the one(s) given with a ^ in front of them. The operative word here is commits. A bundle might include one or more tag objects, or unannotated tags, even though no new commits were available within the time frame. -- brian m. carlson / brian with sandals: Houston, Texas, US +1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187 signature.asc Description: Digital signature
Re: [PATCH v2] t0027: check the eol conversion warnings
Torsten Bögershausen tbo...@web.de writes:
Depending on the file content, eol parameters and .gitattributes
git add may give a warning when the eol of a file will change
when the file is checked out again.
There are 2 different warnings, either CRLF will be replaced... or
LF will be replaced
Let t0027 check for these warnings:
call create_file_in_repo() with additional parameters,
which will be used to call check_warning().
When a file has eol=lf or eol=crlf in .gitattributes, it is handled
as text and should be normalized.
Add missing test cases in t0027.
Signed-off-by: Torsten Bögershausen tbo...@web.de
---
Thanks; nobody seems to have shown interest in this and it fell
through the cracks it seems. I didn't make a connection to the
previous discussion when I saw this v2, and backburnered it.
The patch is clear that the change to check the expected X will be
replaced by Y is added to existing combinations, and also for the
lf crlf cases the existing tests were not checking earlier.
Will queue.
Changes since V1:
- Simplified the diff
- Fixed a bug (LF_mix_CR.err was mixed with CRLF_mix_LF)
- Changed the commit message
t/t0027-auto-crlf.sh | 82
++--
1 file changed, 66 insertions(+), 16 deletions(-)
diff --git a/t/t0027-auto-crlf.sh b/t/t0027-auto-crlf.sh
index 2a4a6c1..452320d 100755
--- a/t/t0027-auto-crlf.sh
+++ b/t/t0027-auto-crlf.sh
@@ -55,16 +55,41 @@ create_gitattributes () {
esac
}
+check_warning () {
+ case $1 in
+ LF_CRLF) grep LF will be replaced by CRLF $2;;
+ CRLF_LF) grep CRLF will be replaced by LF $2;;
+ '')
+ expect
+ grep will be replaced by $2 actual
+ test_cmp expect actual
+ ;;
+ *) false ;;
+ esac
+}
+
create_file_in_repo () {
crlf=$1
attr=$2
+ lfname=$3
+ crlfname=$4
+ lfmixcrlf=$5
+ lfmixcr=$6
+ crlfnul=$7
create_gitattributes $attr
+ pfx=crlf_${crlf}_attr_${attr}
for f in LF CRLF LF_mix_CR CRLF_mix_LF CRLF_nul
do
- pfx=crlf_${crlf}_attr_${attr}_$f.txt
- cp $f $pfx git -c core.autocrlf=$crlf add $pfx
+ fname=${pfx}_$f.txt
+ cp $f $fname
+ git -c core.autocrlf=$crlf add $fname 2${pfx}_$f.err
done
- git commit -m core.autocrlf $crlf
+ git commit -m core.autocrlf $crlf
+ check_warning $lfname ${pfx}_LF.err
+ check_warning $crlfname ${pfx}_CRLF.err
+ check_warning $lfmixcrlf ${pfx}_CRLF_mix_LF.err
+ check_warning $lfmixcr ${pfx}_LF_mix_CR.err
+ check_warning $crlfnul ${pfx}_CRLF_nul.err
}
check_files_in_repo () {
@@ -140,22 +165,47 @@ test_expect_success 'setup master' '
'
-test_expect_success 'create files' '
- create_file_in_repo false
- create_file_in_repo true
- create_file_in_repo input
- create_file_in_repo false auto
- create_file_in_repo true auto
- create_file_in_repo input auto
+warn_LF_CRLF=LF will be replaced by CRLF
+warn_CRLF_LF=CRLF will be replaced by LF
+
+test_expect_success 'add files empty attr' '
+ create_file_in_repo false
+ create_file_in_repo true LF_CRLF LF_CRLF
+ create_file_in_repo input CRLF_LF CRLF_LF
+'
+
+test_expect_success 'add files attr=auto' '
+ create_file_in_repo false auto CRLF_LF CRLF_LF
+ create_file_in_repo true auto LF_CRLF LF_CRLF
+ create_file_in_repo input auto CRLF_LF CRLF_LF
+'
+
+test_expect_success 'add files attr=text' '
+ create_file_in_repo false text CRLF_LF CRLF_LF
CRLF_LF
+ create_file_in_repo true text LF_CRLF LF_CRLF
LF_CRLF
+ create_file_in_repo input text CRLF_LF CRLF_LF
CRLF_LF
+'
+
+test_expect_success 'add files attr=-text' '
+ create_file_in_repo false -text
+ create_file_in_repo true -text
+ create_file_in_repo input -text
+'
+
+test_expect_success 'add files attr=lf' '
+ create_file_in_repo false lf CRLF_LF CRLF_LF
CRLF_LF
+ create_file_in_repo true lf CRLF_LF CRLF_LF
CRLF_LF
+ create_file_in_repo input lf CRLF_LF CRLF_LF
CRLF_LF
+'
- create_file_in_repo false text
- create_file_in_repo true text
- create_file_in_repo input text
+test_expect_success 'add files attr=crlf' '
+ create_file_in_repo false crlf LF_CRLF LF_CRLF
LF_CRLF
+ create_file_in_repo true crlf LF_CRLF LF_CRLF
LF_CRLF
+ create_file_in_repo input crlf LF_CRLF LF_CRLF
LF_CRLF
+'
-
Re: Announcing a new (prototype) git-remote-hg tool
On Fri, Dec 05, 2014 at 05:59:30PM -0500, Jeff King wrote: On Fri, Dec 05, 2014 at 02:13:19PM -0800, Jonathan Nieder wrote: Mike Hommey wrote: I'm currently evaluating what the final tool would look like. I'm *very* tempted to implement it in C, based on core git code, because there are many things that this helper does that would be so much easier to do with direct access to git's guts. And that wouldn't require more dependencies than git currently has: it would just need curl and ssh, and git already uses both. If I were to go in that direction, would you consider integrating it in git core? Yes --- I would like this a lot. I'm concerned that this tool will have drawbacks that Felipe's remote-hg does not. And I can well imagine that it may, as that tool builds on Mercurial's API, which will probably handle some corner cases differently. FWIW, my tool only uses the mercurial code for the wire protocol. This can (and if I go the C route, will) be implemented without using mercurial code, it's really not a hard problem. This isn't to disparage Mike's attempt; it will probably have some upsides, too. But given that the approaches are so different, it does not seem obvious to me that one will always be better than the other. One of the nice things about spinning remote-hg out of the core repo is that it means we do not have to endorse a particular implementation, and they can compete with each other on their merits. I would very much hate to see Felipe's remote-hg project wither and die just because another implementation becomes the de facto standard by being included in git.git. It's a proven tool, and this new thing is not yet. Note that I'm only talking about an hypothetical long term goal. If there's not even a slim chance that this may end up in git core, or in the git.git repo, I'm not sure it's worth writing the tool in C at all, considering the burden for users. IOW, I'm only trying to assess if I should follow my temptation or not. But I can probably reassess after I actually get my prototype to do more than it does now. But maybe there are ways to make it work for users outside of git.git even if it's in C. I don't know. It's a shame that both squat on the name remote-hg, because it makes it difficult to tell the two apart. But of course that is the only way to make git clone hg::... work. Maybe we need a layer of indirection? :) Yeah, that's an unfortunate consequence of how remote helpers work. There are already two different git-remote-hgs (there's felipe's, and there's another one using hg-git under the hood) that I know of. I'm adding a third one. For what it's worth, none of the existing one is satisfying on repos the size of Mozilla's, and apparently noone at Mozilla uses them because of that. Add to that the disk space inefficiency of actually keeping a copy of the mercurial repo locally. The existing tools can likely be improved to scale better, but that wouldn't change the disk space problem. 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: [PATCH] doc: make clear --assume-unchanged's user contract
Junio C Hamano gits...@pobox.com writes: This reads much better than the original, but you may want to go a bit further. How about removing the original a bit more, like so: ... So here is what I came up to squash into your version when queuing it to 'pu'. -- 8 -- Subject: [PATCH] SQUASH??? - Further matching of number and verb; we are talking about a single flag bit. - The bit tells Git that it can assume that the files marked as such are unchanged. Mentioning Git stops checking does not help the reader, as it is only one possible consequence of what that assumption allows Git to do, but (1) there are things other than stop checking that Git can do based on that assumption; and (2) Git is not obliged to stop checking; it merely is allowed to. - Drop the stale and incorrect information about poor-man's ignore, which is not this bit is about at all. Signed-off-by: Junio C Hamano gits...@pobox.com --- Documentation/git-update-index.txt | 17 +++-- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/Documentation/git-update-index.txt b/Documentation/git-update-index.txt index f1f5f7f..da1ccbc 100644 --- a/Documentation/git-update-index.txt +++ b/Documentation/git-update-index.txt @@ -79,20 +79,17 @@ OPTIONS --[no-]assume-unchanged:: When this flag is specified, the object names recorded - for the paths are not updated. Instead, these options - set and unset the assume unchanged bit for the - paths. When the assume unchanged bit is on, the user promise - is not to change the file, so Git stops - checking the working tree files for possible - modifications, so when you change the working tree file you - need to manually unset the bit to tell Git . This is + for the paths are not updated. Instead, this option + sets/unsets the assume unchanged bit for the + paths. When the assume unchanged bit is on, the user + promises not to change the file and allows Git to assume + that the working tree file matches what is recorded in + the index. If you want to change the working tree file, + you need to unset the bit to tell Git. This is sometimes helpful when working with a big project on a filesystem that has very slow lstat(2) system call (e.g. cifs). + -This option can be also used as a coarse file-level mechanism -to ignore uncommitted changes in tracked files (akin to what -`.gitignore` does for untracked files). Git will fail (gracefully) in case it needs to modify this file in the index e.g. when merging in a commit; thus, in case the assumed-untracked file is changed upstream, -- 2.2.0-155-g395db3e -- To unsubscribe from this list: send the line 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 bundle vs git rev-list
brian m. carlson sand...@crustytoothpaste.net writes: On Fri, Dec 05, 2014 at 03:36:18PM -0700, Jesse Hopkins wrote: 1. Any thoughts on why a tag would be included by 'git bundle', when 'git rev-list' with the same arguments returns empty? I think the answer to this is found in the git rev-list manpage: List commits that are reachable by following the parent links from the given commit(s), but exclude commits that are reachable from the one(s) given with a ^ in front of them. The operative word here is commits. A bundle might include one or more tag objects, or unannotated tags, even though no new commits were available within the time frame. Is this what a recent git bundle create change in 2.1.1 and 2.2 fixed? The Release Notes to them seem to have this entry: * git bundle create with date-range specification were meant to exclude tags outside the range, but it did not work correctly. -- To unsubscribe from this list: send the line 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 bundle vs git rev-list
On Fri, Dec 05, 2014 at 03:40:06PM -0800, Junio C Hamano wrote: brian m. carlson sand...@crustytoothpaste.net writes: On Fri, Dec 05, 2014 at 03:36:18PM -0700, Jesse Hopkins wrote: 1. Any thoughts on why a tag would be included by 'git bundle', when 'git rev-list' with the same arguments returns empty? I think the answer to this is found in the git rev-list manpage: List commits that are reachable by following the parent links from the given commit(s), but exclude commits that are reachable from the one(s) given with a ^ in front of them. The operative word here is commits. A bundle might include one or more tag objects, or unannotated tags, even though no new commits were available within the time frame. Is this what a recent git bundle create change in 2.1.1 and 2.2 fixed? The Release Notes to them seem to have this entry: * git bundle create with date-range specification were meant to exclude tags outside the range, but it did not work correctly. That certainly could be the case. I was thinking that perhaps someone had created a tag recently, but your explanation is more likely. -- brian m. carlson / brian with sandals: Houston, Texas, US +1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187 signature.asc Description: Digital signature
Re: Announcing a new (prototype) git-remote-hg tool
On Fri, Dec 05, 2014 at 03:13:30PM -0800, Jonathan Nieder wrote: Jeff King wrote: One of the nice things about spinning remote-hg out of the core repo is that it means we do not have to endorse a particular implementation, and they can compete with each other on their merits. True. [...] It's a shame that both squat on the name remote-hg, because it makes it difficult to tell the two apart. But of course that is the only way to make git clone hg::... work. Maybe we need a layer of indirection? :) If the helpers are roughly interchangeable (that is, if you can switch between fetching using each one into the same on-disk git repository), then picking one to symlink as git-remote-hg in your $PATH should be enough. If they don't have that level of interoperability, then there's an argument to be made that the URLs shouldn't be the same. I don't think Felipe's and the one that uses hg-git under the hood are already interoperable. Mine is also different from both. They should all produce the same git trees. They don't produce the same commits. They also don't share the same metadata. 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: bug report on update-index --assume-unchanged
On Sex, 2014-12-05 at 20:48 +, Philip Oakley wrote: The problem here is that there is no guidance on what those actions are that may make git 'notice'. The man page git-update-index isn't as clear as it could be. Using --really-refresh being one option that would make git notice, but I wouldn't know when that is used. Part of the implied question is why git commit . would notice when when git commit -a didn't appear to. So it's unclear as to what the user should have expected. I agree with this sentence, this is a bug because: git commit -a ( and -a means all ) is incoherent with git commit . This is stupid because when I want commit part of the tree, commit includes one file that is not included when I say to commit all . So maybe you should fix, git commit -a to be coherent . (Note, I don't use assume-unchanged myself so this is more about supporting the user/manual clarification. It is mentioned moderately often on stackoverflow etc.) yeap Sorry I don't have time to read all messages in thread , but I'm going to test git with the patch suggest in this thread , at least, I solve my problem for some time ... Thanks, -- Sérgio M. B. -- To unsubscribe from this list: send the line 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] fetch-pack: don't resend known-common refs in find_common
On Sun, Oct 26, 2014 at 8:39 AM, Dennis Kaarsemaker
den...@kaarsemaker.net wrote:
On Wed, Oct 22, 2014 at 10:11:40AM -0700, Junio C Hamano wrote:
Dennis Kaarsemaker den...@kaarsemaker.net writes:
On di, 2014-10-21 at 10:56 -0700, Junio C Hamano wrote:
Dennis Kaarsemaker den...@kaarsemaker.net writes:
By not clearing the request buffer in stateless-rpc mode, fetch-pack
would keep sending already known-common commits, leading to ever bigger
http requests, eventually getting too large for git-http-backend to
handle properly without filling up the pipe buffer in inflate_request.
---
I'm still not quite sure whether this is the right thing to do, but make
test still passes :) The new testcase demonstrates the problem, when
running t5551 with EXPENSIVE, this test will hang without the patch to
fetch-pack.c and succeed otherwise.
IIUC, because stateless is just that, i.e. the server-end does not
keep track of what is already known, not telling what is known to be
common in each request would fundamentally break the protocol. Am I
mistaken?
That sounds plausible, but why then does the fetch complete with this
line removed, and why does 'make test' still pass?
The fetch-pack program tries to help the upload-pack program(s)
running on the other end find what nodes in the graph both
repositories have in common by sending what the repository on its
end has. Some commits may not be known by the other side (e.g. your
new commits that haven't been pushed there that are made on a branch
forked from the common history), and some others may be known
(i.e. you drilled down the history from the tips of your refs and
reached a commit that you fetched from the common history
previously). The latter are ACKed by the upload-pack process and
are remembered to be re-sent to the _next_ incarnation of the
upload-pack process when stateless RPC is in use.
With your patch, you stop telling the upload-pack process what these
two programs already found to be common in their exchange. After
the first exchange, fetch-pack and upload-pack may have noticed that
both ends have version 2.0, but because you do not convey that fact
to the other side, the new incarnation of upload-pack may end up
deciding that the version 1.9 is the newest common commit between
the two, and sending commits between 1.9 and 2.0.
If you imagine an extreme case, it would be easy to see why the
fetch completes and make test passes are not sufficient to say
anything about this change. Even if you break the protocol in in a
way different from your patch, by not sending any have, such a
butchered fetch-pack will become fetch everything from scratch,
aka clone. The end result will still have correct history and
fetch completes would be true.
But I'd prefer deferring a more detailed analysis/explanation to
Shawn, as stateless RPC is his creation.
Junio's statement above about the world is correct.
Thanks for the explanation, that makes it quite clear that this approach
is wrong. The patch below (apologies for the formatting, I'm not quite
sure how to use format-patch in this situation) does it differently: by
buffering upload-pack's output until it has read all the input, the new
test still succeeds and again 'make test' passes.
This probably introduces latency into the traditional bidirectional
multi_ack protocol.
@@ -384,15 +385,19 @@ static int get_common_commits(void)
if (multi_ack == 2 got_common
!got_other ok_to_give_up()) {
sent_ready = 1;
- packet_write(1, ACK %s ready\n, last_hex);
+ packet_buf_write(resp_buf, ACK %s ready\n,
last_hex);
}
if (have_obj.nr == 0 || multi_ack)
- packet_write(1, NAK\n);
+ packet_buf_write(resp_buf, NAK\n);
By buffering and delaying these when !stateless_rpc we defer telling
the peer in a multi_ack exchange. That delays the peer from cutting
off its communication by about 1RTT.
--
To unsubscribe from this list: send the line 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] document string_list_clear
Signed-off-by: Stefan Beller sbel...@google.com
---
Just stumbled accross this one and wasn't sure if it also frees up
the memory involved.
string-list.h | 5 +
1 file changed, 5 insertions(+)
diff --git a/string-list.h b/string-list.h
index 494eb5d..99ecc44 100644
--- a/string-list.h
+++ b/string-list.h
@@ -21,6 +21,11 @@ struct string_list {
void string_list_init(struct string_list *list, int strdup_strings);
void print_string_list(const struct string_list *p, const char *text);
+
+/*
+ * Clears the string list, so it has zero items. All former items are freed.
+ * If free_util is true, all util pointers are also freed.
+ */
void string_list_clear(struct string_list *list, int free_util);
/* Use this function to call a custom clear function on each util pointer */
--
2.2.0
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] document string_list_clear
Stefan Beller wrote:
Signed-off-by: Stefan Beller sbel...@google.com
---
Just stumbled accross this one and wasn't sure if it also frees up
the memory involved.
string-list.h | 5 +
1 file changed, 5 insertions(+)
Sounds reasonable. Documentation/technical/api-string-list.txt
documents these functions more fully. The right balance between
documenting things in two places vs. adding see also pointers vs.
just putting the highlights in one of the two places isn't obvious to
me.
[...]
--- a/string-list.h
+++ b/string-list.h
@@ -21,6 +21,11 @@ struct string_list {
void string_list_init(struct string_list *list, int strdup_strings);
void print_string_list(const struct string_list *p, const char *text);
+
+/*
+ * Clears the string list, so it has zero items. All former items are freed.
+ * If free_util is true, all util pointers are also freed.
+ */
void string_list_clear(struct string_list *list, int free_util);
The api doc says
Free a string_list. The `string` pointer of the items will be freed in
case the `strdup_strings` member of the string_list is set. The second
parameter controls if the `util` pointer of the items should be freed
or not.
One option here would be to say
Free a string_list. See Documentation/technical/api-string-list.txt
for details.
That reminds me: why do we call this string_list_clear instead of
string_list_free?
Thanks,
Jonathan
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
no-xmailer tests fail under Mac OS
Failures start from commit d2384abff7a6181fd7b9a51af7e780aa21e5cb8d (refs/bisect/bad) Author: Luis Henriques hen...@camandro.org Date: Thu Dec 4 19:11:30 2014 + test/send-email: --[no-]xmailer tests Add tests for the --[no-]xmailer option. Signed-off-by: Luis Henriques hen...@camandro.org Signed-off-by: Junio C Hamano gits...@pobox.com but continue with Junio's SQUASH??? commit at b728d078 Verbose output follows expecting success: do_xmailer_test 1 --xmailer do_xmailer_test 0 --no-xmailer 0001-add-master.patch not ok 109 - --[no-]xmailer without any configuration # # do_xmailer_test 1 --xmailer # do_xmailer_test 0 --no-xmailer # expecting success: test_config sendemail.xmailer true do_xmailer_test 1 do_xmailer_test 0 --no-xmailer do_xmailer_test 1 --xmailer 0001-add-master.patch not ok 110 - --[no-]xmailer with sendemail.xmailer=true # # test_config sendemail.xmailer true # do_xmailer_test 1 # do_xmailer_test 0 --no-xmailer # do_xmailer_test 1 --xmailer # expecting success: test_config sendemail.xmailer false do_xmailer_test 0 do_xmailer_test 0 --no-xmailer do_xmailer_test 1 --xmailer 0001-add-master.patch not ok 111 - --[no-]xmailer with sendemail.xmailer=false # # test_config sendemail.xmailer false # do_xmailer_test 0 # do_xmailer_test 0 --no-xmailer # do_xmailer_test 1 --xmailer # # failed 3 among 111 test(s) 1..111 -- To unsubscribe from this list: send the line 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/8] refs.c: let fprintf handle the formatting
Instead of calculating, whether to put a plus or minus sign, offload
the responsibilty to the fprintf function.
Signed-off-by: Stefan Beller sbel...@google.com
---
Notes:
This patch was already sent as a single patch to the list,
but not yet acknowledge in any way, so I am including it here.
This series goes on top of Michaels series
https://github.com/mhagger/git/tree/reflog-expire-api-v1
refs.c | 7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/refs.c b/refs.c
index 40c5591..1147216 100644
--- a/refs.c
+++ b/refs.c
@@ -3972,12 +3972,9 @@ static int expire_reflog_ent(unsigned char *osha1,
unsigned char *nsha1,
printf(prune %s, message);
} else {
if (cb-newlog) {
- char sign = (tz 0) ? '-' : '+';
- int zone = (tz 0) ? (-tz) : tz;
- fprintf(cb-newlog, %s %s %s %lu %c%04d\t%s,
+ fprintf(cb-newlog, %s %s %s %lu %+05d\t%s,
sha1_to_hex(osha1), sha1_to_hex(nsha1),
- email, timestamp, sign, zone,
- message);
+ email, timestamp, tz, message);
hashcpy(cb-last_kept_sha1, nsha1);
}
if (cb-flags EXPIRE_REFLOGS_VERBOSE)
--
2.2.0
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/8] refs.c: add transaction function to append to the reflog
Currently a transaction can include one or more reflog updates. But these
are not part of the contract of the transaction committing all or nothing.
Introduce a function transaction_update_reflog, which adds a reflog update
to a transaction. Later patches will update code that writes to reflogs to
use this function.
This will allow us to write code such as:
t = transaction_begin()
transaction_truncate_reflog(t, foo); // introduced by next patch
loop-over-something...
if (want_reflog_entry(...))
transaction_reflog_update(t, foo, 0, message);
transaction_commit(t)
transaction_ref_update still updates the reflog along with its ref update.
A later patch will make it use transaction_update_reflog internally.
Unlike transaction_update_ref, this writes out the proposed contents of the
reflog to a temporary file at transaction_reflog_update time instead of
waiting for the transaction waiting to be committed. This avoids an
explosion of memory usage when writing lots of reflog updates within a
single transaction.
This requires changing where the temporary file is located. If the temporary
file is located at $GIT_DIR/logs/$REF_NAME.lock, then a transaction that
tries to remove refs/heads/foo and introduce refs/heads/foo/bar would run
into a directory/file conflict when trying to write to
$GIT_DIR/logs/refs/heads/foo/bar.lock because the reflog for branch foo is
not safe to remove yet.
We work around this by placing the temporary files at
$GIT_DIR/logs/lock/$REF_NAME.lock Putting the temporary file under .git/logs
ensures it's likely that it will be in the same file system as the reflogs
and can be atomically renamed into place.
During one transaction we allow to make multiple reflog updates to the same
ref. This means we only need to lock the reflog once during the first update
that touches the reflog and that all further updates can just write the
reflog entry since the reflog is already locked.
transaction_update_reflog should lock the corresponding ref to avoid having
to compete with other reflog updates, but this will be deferred to a later
patch for simplicity.
Signed-off-by: Stefan Beller sbel...@google.com
---
Notes:
This has been sent to the list before, but now it is split up
into this and the next patch.
The commit message was overhauled, a huge thanks to Jonathan!
There are no major changes in the code.
refs.c | 94 --
1 file changed, 92 insertions(+), 2 deletions(-)
diff --git a/refs.c b/refs.c
index aad44d5..d767418 100644
--- a/refs.c
+++ b/refs.c
@@ -3559,19 +3559,30 @@ struct transaction {
struct ref_update **ref_updates;
size_t alloc;
size_t nr;
+
+ /*
+* Sorted list of reflogs to be committed.
+* the util points to the lock_file.
+*/
+ struct string_list reflog_updates;
enum transaction_state state;
};
struct transaction *transaction_begin(struct strbuf *err)
{
+ struct transaction *ret;
+
assert(err);
- return xcalloc(1, sizeof(struct transaction));
+ ret = xcalloc(1, sizeof(struct transaction));
+ ret-reflog_updates.strdup_strings = 1;
+ return ret;
}
void transaction_free(struct transaction *transaction)
{
int i;
+ struct string_list_item *item;
if (!transaction)
return;
@@ -3581,6 +3592,12 @@ void transaction_free(struct transaction *transaction)
free(transaction-ref_updates[i]);
}
free(transaction-ref_updates);
+
+ for_each_string_list_item(item, transaction-reflog_updates) {
+ rollback_lock_file(item-util);
+ }
+ string_list_clear(transaction-reflog_updates, 0);
+
free(transaction);
}
@@ -3631,6 +3648,71 @@ int transaction_update_ref(struct transaction
*transaction,
return 0;
}
+/*
+ * Append a reflog entry for refname.
+ */
+static int transaction_update_reflog(struct transaction *transaction,
+const char *refname,
+const unsigned char *new_sha1,
+const unsigned char *old_sha1,
+const char *email,
+unsigned long timestamp, int tz,
+const char *msg,
+struct strbuf *err)
+{
+ struct lock_file *lock;
+ struct strbuf buf = STRBUF_INIT;
+ struct string_list_item *item;
+
+ if (transaction-state != TRANSACTION_OPEN)
+ die(BUG: update_reflog called for transaction that is not
open);
+
+ item = string_list_insert(transaction-reflog_updates, refname);
+ if (!item-util) {
+ int infd;
+ char *path = git_path(logs/locks/%s, refname);
+ lock = xcalloc(1,
[PATCH 7/8] refs.c: rename log_ref_setup to create_reflog
From: Ronnie Sahlberg sahlb...@google.com
log_ref_setup is used to do several semi-related things:
* Sometimes it will create a new reflog including missing parent
directories and cleaning up any conflicting stale directories
in the path.
* Fill in a filename buffer for the full path to the reflog.
* Unconditionally re-adjust the permissions for the file.
This function is only called from two places: In checkout.c, where
it is always used to create a reflog and in refs.c log_ref_write,
where it is used to create a reflog sometimes, and sometimes just
to fill in the filename.
Rename log_ref_setup to create_reflog and change it to only take the
refname as an argument to make its signature similar to delete_reflog
and reflog_exists. Change create_reflog to ignore log_all_ref_updates
and unconditionally create the reflog when called. Since checkout.c
always wants to create a reflog we can call create_reflog directly and
avoid the temp-and-log_all_ref_update dance.
In log_ref_write, only call create_reflog, iff we want to create a reflog
and if the reflog does not yet exist. This means that for the common case
where the log already exists we now only need to perform a single lstat()
instead of a open(O_CREAT)+lstat()+close().
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
Signed-off-by: Stefan Beller sbel...@google.com
---
Notes:
This is also just carried along from sending it last time.
builtin/checkout.c | 8 +---
refs.c | 22 +-
refs.h | 8 +++-
3 files changed, 17 insertions(+), 21 deletions(-)
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 5410dac..8550b6d 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -587,19 +587,13 @@ static void update_refs_for_switch(const struct
checkout_opts *opts,
if (opts-new_branch) {
if (opts-new_orphan_branch) {
if (opts-new_branch_log !log_all_ref_updates) {
- int temp;
- char log_file[PATH_MAX];
char *ref_name = mkpath(refs/heads/%s,
opts-new_orphan_branch);
- temp = log_all_ref_updates;
- log_all_ref_updates = 1;
- if (log_ref_setup(ref_name, log_file,
sizeof(log_file))) {
+ if (create_reflog(ref_name)) {
fprintf(stderr, _(Can not do reflog
for '%s'\n),
opts-new_orphan_branch);
- log_all_ref_updates = temp;
return;
}
- log_all_ref_updates = temp;
}
}
else
diff --git a/refs.c b/refs.c
index 295ea09..2effd66 100644
--- a/refs.c
+++ b/refs.c
@@ -2943,16 +2943,16 @@ static int copy_msg(char *buf, const char *msg)
}
/* This function must set a meaningful errno on failure */
-int log_ref_setup(const char *refname, char *logfile, int bufsize)
+int create_reflog(const char *refname)
{
int logfd, oflags = O_APPEND | O_WRONLY;
+ char logfile[PATH_MAX];
- git_snpath(logfile, bufsize, logs/%s, refname);
- if (log_all_ref_updates
- (starts_with(refname, refs/heads/) ||
-starts_with(refname, refs/remotes/) ||
-starts_with(refname, refs/notes/) ||
-!strcmp(refname, HEAD))) {
+ git_snpath(logfile, sizeof(logfile), logs/%s, refname);
+ if (starts_with(refname, refs/heads/) ||
+ starts_with(refname, refs/remotes/) ||
+ starts_with(refname, refs/notes/) ||
+ !strcmp(refname, HEAD)) {
if (safe_create_leading_directories(logfile) 0) {
int save_errno = errno;
error(unable to create directory for %s, logfile);
@@ -3021,16 +3021,20 @@ static int log_ref_write_fd(int fd, const unsigned char
*old_sha1,
static int log_ref_write(const char *refname, const unsigned char *old_sha1,
const unsigned char *new_sha1, const char *msg)
{
- int logfd, result, oflags = O_APPEND | O_WRONLY;
+ int logfd, result = 0, oflags = O_APPEND | O_WRONLY;
char log_file[PATH_MAX];
if (log_all_ref_updates 0)
log_all_ref_updates = !is_bare_repository();
- result = log_ref_setup(refname, log_file, sizeof(log_file));
+ if (log_all_ref_updates !reflog_exists(refname))
+ result = create_reflog(refname);
+
if (result)
return result;
+ git_snpath(log_file, sizeof(log_file), logs/%s, refname);
+
logfd = open(log_file, oflags);
if (logfd 0)
return 0;
diff --git a/refs.h b/refs.h
index 40607d6..7599f81 100644
--- a/refs.h
+++ b/refs.h
@@ -183,11 +183,6
[PATCH 3/8] refs.c: rename transaction.updates to transaction.ref_updates
The updates are only holding refs not reflogs, so express it to the reader.
Signed-off-by: Stefan Beller sbel...@google.com
Reviewed-by: Jonathan Nieder jrnie...@gmail.com
---
Notes:
no changes since last sending it.
refs.c | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/refs.c b/refs.c
index 9f2628b..aad44d5 100644
--- a/refs.c
+++ b/refs.c
@@ -3556,7 +3556,7 @@ enum transaction_state {
* as atomically as possible. This structure is opaque to callers.
*/
struct transaction {
- struct ref_update **updates;
+ struct ref_update **ref_updates;
size_t alloc;
size_t nr;
enum transaction_state state;
@@ -3577,10 +3577,10 @@ void transaction_free(struct transaction *transaction)
return;
for (i = 0; i transaction-nr; i++) {
- free(transaction-updates[i]-msg);
- free(transaction-updates[i]);
+ free(transaction-ref_updates[i]-msg);
+ free(transaction-ref_updates[i]);
}
- free(transaction-updates);
+ free(transaction-ref_updates);
free(transaction);
}
@@ -3591,8 +3591,8 @@ static struct ref_update *add_update(struct transaction
*transaction,
struct ref_update *update = xcalloc(1, sizeof(*update) + len + 1);
strcpy((char *)update-refname, refname);
- ALLOC_GROW(transaction-updates, transaction-nr + 1,
transaction-alloc);
- transaction-updates[transaction-nr++] = update;
+ ALLOC_GROW(transaction-ref_updates, transaction-nr + 1,
transaction-alloc);
+ transaction-ref_updates[transaction-nr++] = update;
return update;
}
@@ -3714,7 +3714,7 @@ int transaction_commit(struct transaction *transaction,
int ret = 0, delnum = 0, i;
const char **delnames;
int n = transaction-nr;
- struct ref_update **updates = transaction-updates;
+ struct ref_update **updates = transaction-ref_updates;
assert(err);
--
2.2.0
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 8/8] refs.c: allow deleting refs with a broken sha1
From: Ronnie Sahlberg sahlb...@google.com
Add back support to make it possible to delete refs that have a broken
sha1.
Add new internal flags REF_ALLOW_BROKEN and RESOLVE_REF_ALLOW_BAD_SHA1
to pass intent from branch.c that we are willing to allow
resolve_ref_unsafe and lock_ref_sha1_basic to allow broken refs.
Since these refs can not actually be resolved to a sha1, they instead resolve
to null_sha1 when these flags are used.
For example, the ref:
echo Broken ref .git/refs/heads/foo-broken-1
can now be deleted using git branch -d foo-broken-1
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
Signed-off-by: Stefan Beller sbel...@google.com
---
Notes:
Changes to previously sent version:
* do not introduce yet another flag, but stick with REF_DELETING
which was meant for this purpose.
builtin/branch.c | 5 +++--
cache.h | 7 +++
refs.c| 5 +
t/t3200-branch.sh | 8
4 files changed, 23 insertions(+), 2 deletions(-)
diff --git a/builtin/branch.c b/builtin/branch.c
index 3b79c50..5fe1cac 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -238,7 +238,8 @@ static int delete_branches(int argc, const char **argv, int
force, int kinds,
target = resolve_ref_unsafe(name,
RESOLVE_REF_READING
| RESOLVE_REF_NO_RECURSE
- | RESOLVE_REF_ALLOW_BAD_NAME,
+ | RESOLVE_REF_ALLOW_BAD_NAME
+ | RESOLVE_REF_ALLOW_BAD_SHA1,
sha1, flags);
if (!target) {
error(remote_branch
@@ -255,7 +256,7 @@ static int delete_branches(int argc, const char **argv, int
force, int kinds,
continue;
}
- if (delete_ref(name, sha1, REF_NODEREF)) {
+ if (delete_ref(name, sha1, REF_NODEREF|REF_DELETING)) {
error(remote_branch
? _(Error deleting remote branch '%s')
: _(Error deleting branch '%s'),
diff --git a/cache.h b/cache.h
index 99ed096..61e61af 100644
--- a/cache.h
+++ b/cache.h
@@ -1000,10 +1000,17 @@ extern int read_ref(const char *refname, unsigned char
*sha1);
* resolved. The function returns NULL for such ref names.
* Caps and underscores refers to the special refs, such as HEAD,
* FETCH_HEAD and friends, that all live outside of the refs/ directory.
+ *
+ * RESOLVE_REF_ALLOW_BAD_SHA1 when this flag is set and the ref contains
+ * an invalid sha1, resolve_ref_unsafe will clear the sha1 argument,
+ * set the REF_ISBROKEN flag and return the refname.
+ * This allows using resolve_ref_unsafe to check for existence of such
+ * broken refs.
*/
#define RESOLVE_REF_READING 0x01
#define RESOLVE_REF_NO_RECURSE 0x02
#define RESOLVE_REF_ALLOW_BAD_NAME 0x04
+#define RESOLVE_REF_ALLOW_BAD_SHA1 0x08
extern const char *resolve_ref_unsafe(const char *ref, int resolve_flags,
unsigned char *sha1, int *flags);
extern char *resolve_refdup(const char *ref, int resolve_flags, unsigned char
*sha1, int *flags);
diff --git a/refs.c b/refs.c
index 2effd66..94b766f 100644
--- a/refs.c
+++ b/refs.c
@@ -1587,6 +1587,10 @@ const char *resolve_ref_unsafe(const char *refname, int
resolve_flags, unsigned
(buffer[40] != '\0' !isspace(buffer[40]))) {
if (flags)
*flags |= REF_ISBROKEN;
+ if (resolve_flags RESOLVE_REF_ALLOW_BAD_SHA1)
{
+ hashclr(sha1);
+ return refname;
+ }
errno = EINVAL;
return NULL;
}
@@ -2265,6 +2269,7 @@ static struct ref_lock *lock_ref_sha1_basic(const char
*refname,
resolve_flags |= RESOLVE_REF_READING;
if (flags REF_DELETING) {
resolve_flags |= RESOLVE_REF_ALLOW_BAD_NAME;
+ resolve_flags |= RESOLVE_REF_ALLOW_BAD_SHA1;
if (flags REF_NODEREF)
resolve_flags |= RESOLVE_REF_NO_RECURSE;
}
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index 432921b..1ea0d2c 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -163,6 +163,14 @@ test_expect_success 'git branch --list -d t should fail' '
test_path_is_missing .git/refs/heads/t
'
+test_expect_success 'git branch -d can delete ref with broken sha1' '
+ echo pointing nowhere .git/refs/heads/brokensha1
+ test_when_finished rm -f .git/refs/heads/brokensha1
+ git branch -d brokensha1
+ git branch output
+ ! grep brokensha1 output
+'
+
test_expect_success 'git branch --column' '
[PATCH 2/8] refs.c: rename the transaction functions
Rename the transaction functions. Remove the leading ref_ from the
names and append _ref to the names for functions that create/delete/
update sha1 refs.
The goal in the long term is to have different things running through
the transaction api, such as the .git/config file. The dream is to
have any possible modification passing the transaction api.
Even if we don't achieve this goal, this change still makes sense
as it's slightly easier to read without the ref_ prefixes.
[sb: cherry-picked on top of mhagger/reflog-expire-api-v1]
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
Signed-off-by: Stefan Beller sbel...@google.com
Reviewed-by: Jonathan Nieder jrnie...@gmail.com
---
Notes:
no changes since last sending it.
branch.c | 13 +
builtin/commit.c | 10 +++
builtin/fetch.c| 12
builtin/receive-pack.c | 13 -
builtin/replace.c | 10 +++
builtin/tag.c | 10 +++
builtin/update-ref.c | 26 -
fast-import.c | 22 +++---
refs.c | 78 +-
refs.h | 35 +++---
sequencer.c| 12
walker.c | 10 +++
12 files changed, 126 insertions(+), 125 deletions(-)
diff --git a/branch.c b/branch.c
index 4bab55a..c8462de 100644
--- a/branch.c
+++ b/branch.c
@@ -279,16 +279,17 @@ void create_branch(const char *head,
log_all_ref_updates = 1;
if (!dont_change_ref) {
- struct ref_transaction *transaction;
+ struct transaction *transaction;
struct strbuf err = STRBUF_INIT;
- transaction = ref_transaction_begin(err);
+ transaction = transaction_begin(err);
if (!transaction ||
- ref_transaction_update(transaction, ref.buf, sha1,
- null_sha1, 0, !forcing, msg, err) ||
- ref_transaction_commit(transaction, err))
+ transaction_update_ref(transaction, ref.buf, sha1,
+ null_sha1, 0, !forcing, msg,
+ err) ||
+ transaction_commit(transaction, err))
die(%s, err.buf);
- ref_transaction_free(transaction);
+ transaction_free(transaction);
strbuf_release(err);
}
diff --git a/builtin/commit.c b/builtin/commit.c
index e108c53..f50b7df 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1673,7 +1673,7 @@ int cmd_commit(int argc, const char **argv, const char
*prefix)
struct stat statbuf;
struct commit *current_head = NULL;
struct commit_extra_header *extra = NULL;
- struct ref_transaction *transaction;
+ struct transaction *transaction;
struct strbuf err = STRBUF_INIT;
if (argc == 2 !strcmp(argv[1], -h))
@@ -1804,17 +1804,17 @@ int cmd_commit(int argc, const char **argv, const char
*prefix)
strbuf_insert(sb, 0, reflog_msg, strlen(reflog_msg));
strbuf_insert(sb, strlen(reflog_msg), : , 2);
- transaction = ref_transaction_begin(err);
+ transaction = transaction_begin(err);
if (!transaction ||
- ref_transaction_update(transaction, HEAD, sha1,
+ transaction_update_ref(transaction, HEAD, sha1,
current_head
? current_head-object.sha1 : NULL,
0, !!current_head, sb.buf, err) ||
- ref_transaction_commit(transaction, err)) {
+ transaction_commit(transaction, err)) {
rollback_index_files();
die(%s, err.buf);
}
- ref_transaction_free(transaction);
+ transaction_free(transaction);
unlink(git_path(CHERRY_PICK_HEAD));
unlink(git_path(REVERT_HEAD));
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 7b84d35..0be0b09 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -404,7 +404,7 @@ static int s_update_ref(const char *action,
{
char msg[1024];
char *rla = getenv(GIT_REFLOG_ACTION);
- struct ref_transaction *transaction;
+ struct transaction *transaction;
struct strbuf err = STRBUF_INIT;
int ret, df_conflict = 0;
@@ -414,23 +414,23 @@ static int s_update_ref(const char *action,
rla = default_rla.buf;
snprintf(msg, sizeof(msg), %s: %s, rla, action);
- transaction = ref_transaction_begin(err);
+ transaction = transaction_begin(err);
if (!transaction ||
- ref_transaction_update(transaction, ref-name, ref-new_sha1,
+ transaction_update_ref(transaction, ref-name, ref-new_sha1,
ref-old_sha1, 0, check_old, msg, err))
goto fail;
- ret =
[PATCH 5/8] refs.c: add transaction function to delete the reflog
This continues the work of the previous patch as reflogs not
only grow, but also need a cut sometimes. This patch introduces
transaction_delete_reflog as part of the transaction API to
delete the reflog.
This function serves two purposes. It can be used to actually
delete the reflog as the name indicates. The other purpose is
truncation of the reflog and rewriting it.
Signed-off-by: Stefan Beller sbel...@google.com
---
refs.c | 62 +-
1 file changed, 61 insertions(+), 1 deletion(-)
diff --git a/refs.c b/refs.c
index d767418..57f4941 100644
--- a/refs.c
+++ b/refs.c
@@ -3649,6 +3649,56 @@ int transaction_update_ref(struct transaction
*transaction,
}
/*
+ * Delete the reflog for the given refname.
+ *
+ */
+static int transaction_delete_reflog(struct transaction *transaction,
+ const char *refname,
+ struct strbuf *err)
+{
+ struct lock_file *lock;
+ struct string_list_item *item;
+
+ if (transaction-state != TRANSACTION_OPEN)
+ die(BUG: delete_reflog called for transaction that is not
open);
+
+ item = string_list_insert(transaction-reflog_updates, refname);
+
+ if (!item-util) {
+ char *path = git_path(logs/locks/%s, refname);
+ lock = xcalloc(1, sizeof(struct lock_file));
+ item-util = lock;
+ if (safe_create_leading_directories(path)) {
+ strbuf_addf(err, could not create leading directories
of '%s': %s,
+ path, strerror(errno));
+ goto failure;
+ }
+
+ if (hold_lock_file_for_update(lock, path, 0) 0) {
+ unable_to_lock_message(path, errno, err);
+ goto failure;
+ }
+ /* The empty file indicates transaction_commit to
+* delete the reflog */
+ return 0;
+ }
+
+ /* The transaction already writes to this reflog. Clear it. */
+ lock = item-util;
+ if (lseek(lock-fd, 0, SEEK_SET) 0 ||
+ ftruncate(lock-fd, 0)) {
+ strbuf_addf(err, cannot truncate reflog '%s': %s,
+ refname, strerror(errno));
+ goto failure;
+ }
+ return 0;
+
+failure:
+ transaction-state = TRANSACTION_CLOSED;
+ return -1;
+}
+
+/*
* Append a reflog entry for refname.
*/
static int transaction_update_reflog(struct transaction *transaction,
@@ -3885,7 +3935,17 @@ int transaction_commit(struct transaction *transaction,
/* Commit all reflog updates*/
for_each_string_list_item(item, transaction-reflog_updates) {
struct lock_file *lock = item-util;
- commit_lock_file_to(lock, git_path(logs/%s, item-string));
+
+ /* If the lock file is empty we want to delete the reflog*/
+ off_t filepos = lseek(lock-fd, 0, SEEK_END);
+ if (filepos 0) {
+ ret = TRANSACTION_GENERIC_ERROR;
+ goto cleanup;
+ }
+ if (filepos)
+ commit_lock_file_to(lock, git_path(logs/%s,
item-string));
+ else
+ remove_path(git_path(logs/%s, item-string));
}
clear_loose_ref_cache(ref_cache);
--
2.2.0
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/8] Making reflog modifications part of the transactions API
This goes on top of Michaels series. The idea of this series is make the reflogs being part of the transaction API, so it will be part of the contract of transaction_commit to either commit all the changes or none at all. Currently when using the transaction API to change refs, also reflogs are changed. But the changes to the reflogs just happen as a side effect and not as part of the atomic part of changes we want to commit altogether. Ronnie Sahlberg (3): refs.c: use a reflog transaction when writing during expire refs.c: rename log_ref_setup to create_reflog refs.c: allow deleting refs with a broken sha1 Stefan Beller (5): refs.c: let fprintf handle the formatting refs.c: rename the transaction functions refs.c: rename transaction.updates to transaction.ref_updates refs.c: add transaction function to append to the reflog refs.c: add transaction function to delete the reflog branch.c | 13 +- builtin/branch.c | 5 +- builtin/checkout.c | 8 +- builtin/commit.c | 10 +- builtin/fetch.c| 12 +- builtin/receive-pack.c | 13 +- builtin/replace.c | 10 +- builtin/tag.c | 10 +- builtin/update-ref.c | 26 ++-- cache.h| 7 + fast-import.c | 22 +-- refs.c | 359 ++--- refs.h | 43 +++--- sequencer.c| 12 +- t/t3200-branch.sh | 8 ++ walker.c | 10 +- 16 files changed, 359 insertions(+), 209 deletions(-) -- 2.2.0 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 6/8] refs.c: use a reflog transaction when writing during expire
From: Ronnie Sahlberg sahlb...@google.com
Use a transaction for all updates during expire_reflog.
[sb: This was once a patch series on its own. I cherry-picked these
patches on top of Michaels series to cleanup the refs api. So any
anomalies and bugs may be introduced by me.]
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
Signed-off-by: Stefan Beller sbel...@google.com
---
Notes:
Maybe we can leave out the first patch of the series
as this one deletes the changes made in the first patch of the series.
Originally authored by Ronnie for the reflogs.c file,
cherrypicked over to the refs.c file by Stefan
refs.c | 85 +++---
1 file changed, 35 insertions(+), 50 deletions(-)
diff --git a/refs.c b/refs.c
index 57f4941..295ea09 100644
--- a/refs.c
+++ b/refs.c
@@ -4100,8 +4100,10 @@ struct expire_reflog_cb {
unsigned int flags;
reflog_expiry_select_fn *select_fn;
void *policy_cb;
- FILE *newlog;
+ struct transaction *t;
+ const char *refname;
unsigned char last_kept_sha1[20];
+ struct strbuf *err;
};
static int expire_reflog_ent(unsigned char *osha1, unsigned char *nsha1,
@@ -4116,15 +4118,18 @@ static int expire_reflog_ent(unsigned char *osha1,
unsigned char *nsha1,
if ((*cb-select_fn)(osha1, nsha1, email, timestamp, tz,
message, policy_cb)) {
- if (!cb-newlog)
+ if (!cb-t)
printf(would prune %s, message);
else if (cb-flags EXPIRE_REFLOGS_VERBOSE)
printf(prune %s, message);
} else {
- if (cb-newlog) {
- fprintf(cb-newlog, %s %s %s %lu %+05d\t%s,
- sha1_to_hex(osha1), sha1_to_hex(nsha1),
- email, timestamp, tz, message);
+ if (cb-t) {
+ if (transaction_update_reflog(cb-t, cb-refname,
+ nsha1, osha1,
+ email, timestamp,
+ tz, message,
+ cb-err))
+ return -1;
hashcpy(cb-last_kept_sha1, nsha1);
}
if (cb-flags EXPIRE_REFLOGS_VERBOSE)
@@ -4133,8 +4138,6 @@ static int expire_reflog_ent(unsigned char *osha1,
unsigned char *nsha1,
return 0;
}
-static struct lock_file reflog_lock;
-
extern int reflog_expire(const char *refname, const unsigned char *sha1,
unsigned int flags,
reflog_expiry_prepare_fn prepare_fn,
@@ -4143,66 +4146,48 @@ extern int reflog_expire(const char *refname, const
unsigned char *sha1,
void *policy_cb_data)
{
struct expire_reflog_cb cb;
- struct ref_lock *lock;
- char *log_file;
int status = 0;
+ struct strbuf err = STRBUF_INIT;
memset(cb, 0, sizeof(cb));
cb.flags = flags;
cb.policy_cb = policy_cb_data;
cb.select_fn = select_fn;
+ cb.err = err;
+ cb.refname = refname;
/*
-* we take the lock for the ref itself to prevent it from
-* getting updated.
+* todo: we need to take the lock for the ref itself to
+* prevent it from getting updated.
*/
- lock = lock_ref_sha1_basic(refname, sha1, NULL, 0, NULL);
- if (!lock)
- return error(cannot lock ref '%s', refname);
- if (!reflog_exists(refname)) {
- unlock_ref(lock);
- return 0;
+ cb.t = transaction_begin(err);
+ if (!cb.t) {
+ status |= error(%s, err.buf);
+ goto cleanup;
}
-
- log_file = git_pathdup(logs/%s, refname);
- if (!(flags EXPIRE_REFLOGS_DRY_RUN)) {
- if (hold_lock_file_for_update(reflog_lock, log_file, 0) 0)
- goto failure;
- cb.newlog = fdopen_lock_file(reflog_lock, w);
- if (!cb.newlog)
- goto failure;
+ if (transaction_delete_reflog(cb.t, cb.refname, err)) {
+ status |= error(%s, err.buf);
+ goto cleanup;
}
(*prepare_fn)(refname, sha1, cb.policy_cb);
for_each_reflog_ent(refname, expire_reflog_ent, cb);
(*cleanup_fn)(cb.policy_cb);
+
if (!(flags EXPIRE_REFLOGS_DRY_RUN)) {
- if (close_lock_file(reflog_lock)) {
- status |= error(Couldn't write %s: %s, log_file,
- strerror(errno));
- } else if ((flags EXPIRE_REFLOGS_UPDATE_REF)
- (write_in_full(lock-lock_fd,
- sha1_to_hex(cb.last_kept_sha1), 40) != 40 ||
Re: GIT: cherry-picking includes changes from a different commit
If you're referring to what 86ff53a7a178 has it has resolved the merge conflict in a way that is almost certainly wrong. If you turn on diff3 markers you can see that LHS deleted clone_test while RHS added a function right after it. The correct resolution is to include RHS's new stuff but not that function whereas 86ff53a7a178 includes everything from the RHS side of the conflict marker (thus including/adding clone_test). Keith Thus spake Ruud Huynen, at Fri, Dec 05, 2014 at 09:04:28PM +0100: From: Ruud Huynen ruud.huy...@hispeed.ch To: git@vger.kernel.org Subject: GIT: cherry-picking includes changes from a different commit Date: Fri, 5 Dec 2014 21:04:28 +0100 I noticed when cherry-picking a commit it includes changes from this commit and also changes from a different commit as the one I was picking. I was in contact with jast on IRC, who noticed the issue, but didn't had time to look further. For a description of my problem, please read http://stackoverflow.com/questions/27220638/git-cherry-pick-info-about-picke d-commits git clone git://github.com/MythTV/mythtv.git git checkout fixes/0.27 git cherry-pick 30df98ce5d11b69d0b5c5114a9007cdfc79a7e9b # from master # commit also picked: 17f17e1fc51b3b4017e08f5ea35c8a7b5a64eeec # resulting in a conflict For the commits and before/conflict files, see https://gist.github.com/FritzHerbers/4f9b0990b6bca15a70eb As nobody could believe me, that changes from another commit are included, is something wrong, or is it new behavior? Why did it happen? With which command, can I have a listing of intermediate commits picked? How can I automate such situations? Fritz git version 1.9.1 -- To unsubscribe from this list: send the line 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: Announcing a new (prototype) git-remote-hg tool
On Fri, Dec 05, 2014 at 03:13:30PM -0800, Jonathan Nieder wrote: It's a shame that both squat on the name remote-hg, because it makes it difficult to tell the two apart. But of course that is the only way to make git clone hg::... work. Maybe we need a layer of indirection? :) If the helpers are roughly interchangeable (that is, if you can switch between fetching using each one into the same on-disk git repository), then picking one to symlink as git-remote-hg in your $PATH should be enough. That may be enough. For the most part you do not need to agree with other members of the project on which implementation to use. My experience with import tools has been that either: 1. you are using them personally (because you do not like the upstream's choice of VCS and would prefer to transparently work in your favorite tool), or 2. there is a group of developers who want to use git, but somebody provides an unofficial git mirror. They do not have to agree on the tool, because they just use git directly from the mirror. So it is mostly a personal choice. But the two confusions I'd still anticipate are: a. It's difficult to even _talk_ about the tools, because the names are the same (so searching for tips on the tool, reporting bugs, etc, are harder than necessary). b. You may want different tools for different projects. If one tool is much more efficient, you may need it for a large repo (e.g., mozilla). But another tool may provide other features, and you would prefer it for smaller repos. This is largely speculation, though, and I do not actively use the tools myself. So I'd be happy to push off dealing with it until it itches enough for somebody to scratch. -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 bundle vs git rev-list
On Fri, Dec 05, 2014 at 03:36:18PM -0700, Jesse Hopkins wrote: #Create the bundle git bundle create out.bundle --all --since=last_bundle_date Others pointed out that a bug in the handling of --since may be the culprit here. However, I'd encourage you to use actual sha1s, as they are going to be more robust (especially in the face of any clock skew in the commit timestamps). You should be able to follow a procedure like: 1. On day 1, create a bundle from scratch: git bundle create out.bundle --all 2. Before you send it out, record its tips in the local repository for later reference: git fetch out.bundle +refs/*:refs/remotes/bundle/* 3. On day 2, create a bundle from the previously recorded tips: git bundle create out.bundle --all --not --remotes=bundle 4. Update your tips in the same way: git fetch out.bundle +refs/*:refs/remotes/bundle/* and so on for day 3 and onward. Note that this is not the only way to store those tips (I just did it using git refs because it's simple to manipulate). You could also just store it in a file: # checkpoint git ls-remote out.bundle | cut -f1 | sort -u tips # make incremental bundle git bundle create out.bundle --all --not $(cat tips) This also makes it easy to recover if the other side ever gets out of sync (say you create and checkpoint a bundle on the sending side, but it never makes it to the remote; how do you know where to start from?). You can always get the latest set of tips from the remote by running: git ls-remote . | cut -f1 | sort -u tips on it and then sneaker-netting the tips file back to the sender. -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] document string_list_clear
On Fri, Dec 05, 2014 at 06:04:58PM -0800, Jonathan Nieder wrote:
That reminds me: why do we call this string_list_clear instead of
string_list_free?
Because it does not free the struct itself, and because you can then use
the result again. I think we try to draw a distinction between:
/* Free resources, but do not reinitialize! */
void string_list_free_data(struct string_list *s)
{
int i;
for (i = 0; i s-nr; i++)
free(s-items[i].string);
free(s-items[i];
}
/* Free resources, and go back to initialized but empty state */
void string_list_clear(struct string_list *s)
{
string_list_free_data(s);
s-nr = s-alloc = 0;
s-items = NULL;
}
/* Free all resources from dynamically allocated structure */
void string_list_free(struct string_list *s)
{
string_list_clear(s);
free(s);
}
Ideally we use consistent names to distinguish between them. We are not
always consistent, though (probably strbuf_release should be called
strbuf_clear for consistency). But I think we are fairly consistent that
_free() means ...and free the pointer, too.
In general, we try to avoid the first as a public interface (because it
is error-prone when somebody tries to reuse the list, and the extra work
of zero-ing the pointer is not enough to care about).
We also tend to avoid the third, because it is quite often not the
business of the object whether it was dynamically constructed or not
(exceptions tend to be node-oriented structures like linked lists and
trees; c.f. cache_tree and commit_list).
Maybe that should go into CodingGuidelines? It's not really style,
exactly, but it is convention.
-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] document string_list_clear
On Fri, Dec 05, 2014 at 06:04:58PM -0800, Jonathan Nieder wrote: Stefan Beller wrote: Signed-off-by: Stefan Beller sbel...@google.com --- Just stumbled accross this one and wasn't sure if it also frees up the memory involved. string-list.h | 5 + 1 file changed, 5 insertions(+) Sounds reasonable. Documentation/technical/api-string-list.txt documents these functions more fully. The right balance between documenting things in two places vs. adding see also pointers vs. just putting the highlights in one of the two places isn't obvious to me. Also, I forgot to mention: if we consistently put the API docs into the header files and extracted it automatically into standalone documents, we would not need to have two places. This is something I've been meaning to look at for a long time, but it never quite makes the priority list. And my past experiences with tools like doxygen has been that they are complicated and have a lot of dependencies. It's been a long time since I've tried, though. -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: no-xmailer tests fail under Mac OS
On Fri, Dec 05, 2014 at 06:05:24PM -0800, Michael Blume wrote: Failures start from commit d2384abff7a6181fd7b9a51af7e780aa21e5cb8d (refs/bisect/bad) Author: Luis Henriques hen...@camandro.org Date: Thu Dec 4 19:11:30 2014 + test/send-email: --[no-]xmailer tests Add tests for the --[no-]xmailer option. Signed-off-by: Luis Henriques hen...@camandro.org Signed-off-by: Junio C Hamano gits...@pobox.com but continue with Junio's SQUASH??? commit at b728d078 The commit contains: + test z$(grep ^X-Mailer: out | wc -l) = z$expected We have had trouble in the past with wc -l output not being strictly portable. I do not recall offhand which systems, but it is a good bet that this is the culprit. Doing: grep ^X-Mailer: out mailer test_line_count = $expected mailer should fix it. It might be even nicer to actually compare the x-mailer line we find to an expected output, but that may introduce complications if the value changes with the version or something (you'd have to sanitize the output, and then I do not know that the test is really buying much over just seeing whether it exists). -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 log --pretty=format:%H$t%aN$t%s$t%G? --show-signature
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Thanks for the input, Junio. On Thu, Dec 04, 2014 at 13:11:15 -0800, Junio C Hamano wrote: I am however not quite sure what conclusion you are trying to drive at by contrasting approaches #2 and #3. The perceived problem of approach #2, if I am reading you correctly, is that the merge is what you vouch for but the commits on the side branch are not signed so there is no way for you (as the merge creator) to point fingers to when the result of the merge turns out to be problematic. The argument for approach #3 would be that it would give you (as the merge creator) somebody to point fingers to if you forced others who ask you to pull from them to sign their commits. I had to take another look at the article to see if my opinions have since changed. My argument for Option #3 (signing each commit) was that it explicitly denotes that a particular commit was reviewed; signing a merge commit states that implicitly. I personally sign any commit on master; usually, this is a merge commit. But this is because I have a high level of confidence in the integrity of my system, my ability to notice commits that are not likely to be my own, and because the auditing requirements of my software are, well, non-existent. My wife was recently watching Elf (the movie), which had an interesting example, so I'll make use of it: One of the characters works for a publishing company. A children's book was printed, but was missing content on the last two pages. The character was responsible for signing off on the book. His boss stormed in, and the character used the excuse that something must have gone wrong during printing; but his boss pulled out the proofs that went to press---each page had the character's initials, including the blank ones. Signing each commit is like initialing each page. Of course, these scenarios are drastically different---a page of a book is its own finished result, whereas a commit is more likely to be a single component of a larger feature. But adapt it how you will. Another possibility is that a malicious commit could be hidden within a changeset by introducing another commit that later reverts the change; there would be nothing in the diff between `topic' and `HEAD', but an operation like `bisect' could check out the commit and run malicious code. So the question is then: does the signing of the merge commit indicate review of the diff, or review of each commit? If the review of each commit doesn't matter, does the history matter? In which case, is squashing the better option? Does it absolve you from blame if you can say with certainty (thanks to GPG keys on them) that those commits on the side branch that adds unwanted (from 'maint' policy's point of view) new feature were made by somebody else, because the project used the approach #3? Not really. You might be thinking that I'm suggesting that the merge commit carry a different signature than the commits on the side branch. That's certainly an option, but wasn't what I was referring to, since the GPG-signature of the author doesn't indicate that the code was reviewed by the person responsible for doing so. And no additional review data (e.g. a Signed-off-by) can be added to the commit object without invalidating the signature of the author (except by notes, or a similar concept). So in this case, if you truly do want to maintain the signature of the author (I would!), you must sign the merge commit and indicate the review of each commit in some other manner, or rely on the implicit assumption that each commit has been reviewed. And that may not be a problem. Instead, Option #3 was stating that the person responsible for merging would also be responsible for signing each individual commit, as a means of stating yep, this was reviewed. When you sign your merge with merge -S, you are vouching for the contents of the whole tree, not just I made this merge, but I don't have anything to do with what it pulled in. It does not really matter to the end users where the changes came from. You are certifying that git diff HEAD^ HEAD after making the merge is what you are pleased with by signing the merge. And `git diff HEAD^ HEAD' may show nothing wrong, as I mentioned above. But otherwise, yes, you're correct. The goal would not be to absolve blame in this scenario. The only goal of signing each commit would be to provide confidence to those who may care about the depth of the review process. Rather, it is the _absence_ of a GPG-signed merge that would be the problem, since you can't assert your identity in that case. If you have GPG-signed the merge, then you've avoided the problems in the horror story part of the article, but you've introduced a whole new set of them if you introduced bad changes, since you now can't deny it. ;) But ultimately, the responsibility lies on the person who creates the topmost merge and advances the tip of the history the users
Re: no-xmailer tests fail under Mac OS
On Fri, Dec 5, 2014 at 9:34 PM, Jeff King p...@peff.net wrote: On Fri, Dec 05, 2014 at 06:05:24PM -0800, Michael Blume wrote: Failures start from commit d2384abff7a6181fd7b9a51af7e780aa21e5cb8d (refs/bisect/bad) Author: Luis Henriques hen...@camandro.org Date: Thu Dec 4 19:11:30 2014 + test/send-email: --[no-]xmailer tests Add tests for the --[no-]xmailer option. Signed-off-by: Luis Henriques hen...@camandro.org Signed-off-by: Junio C Hamano gits...@pobox.com but continue with Junio's SQUASH??? commit at b728d078 The commit contains: + test z$(grep ^X-Mailer: out | wc -l) = z$expected We have had trouble in the past with wc -l output not being strictly portable. I do not recall offhand which systems, but it is a good bet that this is the culprit. Doing: grep ^X-Mailer: out mailer test_line_count = $expected mailer should fix it. It might be even nicer to actually compare the x-mailer line we find to an expected output, but that may introduce complications if the value changes with the version or something (you'd have to sanitize the output, and then I do not know that the test is really buying much over just seeing whether it exists). -Peff Actually need to drop the '', but yes, that works perfectly, 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: no-xmailer tests fail under Mac OS
On Fri, Dec 05, 2014 at 10:27:40PM -0800, Michael Blume wrote:
We have had trouble in the past with wc -l output not being strictly
portable. I do not recall offhand which systems, but it is a good bet
that this is the culprit. Doing:
grep ^X-Mailer: out mailer
test_line_count = $expected mailer
should fix it. It might be even nicer to actually compare the x-mailer
line we find to an expected output, but that may introduce complications
if the value changes with the version or something (you'd have to
sanitize the output, and then I do not know that the test is really
buying much over just seeing whether it exists).
-Peff
Actually need to drop the '', but yes, that works perfectly, thanks =)
Ah, right, we might be looking for 0 sometimes. The right way to do it
without destroying the -chaining is:
{ grep ^X-Mailer: out || true }
test_line_count = $expected mailer
-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: no-xmailer tests fail under Mac OS
On Fri, Dec 05, 2014 at 11:07:37PM -0800, Michael Blume wrote:
Ah, right, we might be looking for 0 sometimes. The right way to do it
without destroying the -chaining is:
{ grep ^X-Mailer: out || true }
test_line_count = $expected mailer
Hmm, it doesn't look like that helper is -chained though? So it
seems like we could just do without the
You're right, but that is IMHO a bug. We would not notice if send-email
or format-patch barfed, and we are expecting to find no X-Mailer (we
wouldn't, but for the wrong reason).
It should also be using test_config in the last two tests.
-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
