Re: t5570 trap use in start/stop_git_daemon
On Fri, Feb 13, 2015 at 02:44:03AM -0500, Jeff King wrote:
On Thu, Feb 12, 2015 at 03:31:12PM -0500, Randall S. Becker wrote:
On the NonStop port, we found that trap was causing an issue with test
success for t5570. When start_git_daemon completes, the shell (ksh,bash) on
this platform is sending a signal 0 that is being caught and acted on by the
trap command within the start_git_daemon and stop_git_daemon functions. I am
taking this up with the operating system group,
Yeah, that seems wrong. If it were a subshell, even, I could see some
argument for it, but it seems odd to trap 0 when a function returns
(bash does have a RETURN trap, which AFAIK is bash-specific, but it
should not trigger a 0-trap).
Hmm, today I learned something new about ksh. Apparently when you use
the function keyword to define a function like:
function foo {
trap 'echo trapped' EXIT
}
echo before
foo
echo after
then the trap runs when the function exits! If you declare the same
function as:
foo() {
trap 'echo trapped' EXIT
}
it behaves differently. POSIX shell does not have the function keyword,
of course, and we are not using it here. Bash _does_ have the function
keyword, but seems to behave POSIX-y even when it is present. I.e.,
running the first script:
$ ksh foo.sh
before
trapped
after
$ bash foo.sh
before
after
trapped
$ dash foo.sh
foo.sh: 3: foo.sh: function: not found
foo.sh: 5: foo.sh: Syntax error: } unexpected
Switching to the second form, all three produce:
before
after
trapped
I don't know if that is all helpful to your bug-tracking or analysis,
but for whatever reason it looks like your ksh is using localized traps
for both forms of function. But as far as I know, bash has never behaved
that way (I just grepped its CHANGES file for mentions of trap and found
nothing likely).
-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: t5570 trap use in start/stop_git_daemon
Jeff King peff at peff.net writes:
On Fri, Feb 13, 2015 at 02:44:03AM -0500, Jeff King wrote:
On Thu, Feb 12, 2015 at 03:31:12PM -0500, Randall S. Becker wrote:
snip
Hmm, today I learned something new about ksh. Apparently when you use
the function keyword to define a function like:
function foo {
trap 'echo trapped' EXIT
}
echo before
foo
echo after
then the trap runs when the function exits! If you declare the same
function as:
foo() {
trap 'echo trapped' EXIT
}
it behaves differently. POSIX shell does not have the function keyword,
of course, and we are not using it here. Bash _does_ have the function
keyword, but seems to behave POSIX-y even when it is present. I.e.,
running the first script:
$ ksh foo.sh
before
trapped
after
$ bash foo.sh
before
after
trapped
$ dash foo.sh
foo.sh: 3: foo.sh: function: not found
foo.sh: 5: foo.sh: Syntax error: } unexpected
Switching to the second form, all three produce:
before
after
trapped
I don't know if that is all helpful to your bug-tracking or analysis,
but for whatever reason it looks like your ksh is using localized traps
for both forms of function. But as far as I know, bash has never behaved
that way (I just grepped its CHANGES file for mentions of trap and found
nothing likely).
-Peff
Both versions produce your first output on our platform
$ ksh foo1.sh
before
trapped
after
$ bash foo1.sh
before
after
trapped
$ ksh foo2.sh
before
trapped
after
$ bash foo2.sh
before
after
trapped
$
This might have been one (or even _the_) reason why we picked bash as our
SHELL_PATH in config.mak.uname (I don't remember, it's more than 2 years
ago), not sure which shell Randall's test used?
bye, Jojo
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] nd/multiple-work-trees updates
On do, 2015-02-12 at 14:57 -0800, Junio C Hamano wrote: Nguyễn Thái Ngọc Duy pclo...@gmail.com writes: These patches are on top of what's in 'pu'. They add --ignore-other-worktrees and make a note about current submodule support status. I don't think submodule support is ready yet even with Max Kirillov's series [1]. His 03/03 is already fixed in 'pu' though, so only 01/03 and 02/03 are new. [1] http://thread.gmane.org/gmane.comp.version-control.git/261107 With the understanding (perhaps a strongly-worded paragraph in the release notes) that this is not suitable for submodule users yet, is this in a good enough shape to go to 'next'? I've been using this for a while and really like it. However, it needs a few fixups to merge with next as there are a few merge conflicts. (A version of this branch that I stuck on top of next last week can be found at https://github.com/seveas/git/tree/nd/multiple-work-trees ) -- Dennis Kaarsemaker http://www.kaarsemaker.net -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: could not verify object error
On Thu, Feb 12, 2015 at 12:32:45PM +, Daniel Devlin wrote:
tag_contents =
object f849f9e28c7f36a826d4b451efb16516c0c3acc2\ntype #{type}\ntag #
{tagname}\ntagger #{username} #{email} #{Time.new.to_i} +\n\n#{message}
[...]
executecommand = printf \#{tag_contents}\ | git mktag
[...]
I have tried to run it on a vm with git 2.0.4 but keep
getting char 7 could not verify object error message.
Do you have object f849f9e28c... in the repository where you are running
mktag? If so, is it a commit object, as you are putting in the type
field? What does:
git cat-file -t f849f9e28c7f36a826d4b451efb16516c0c3acc2
show?
-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] nd/multiple-work-trees updates
On Fri, Feb 13, 2015 at 5:57 AM, Junio C Hamano gits...@pobox.com wrote: Nguyễn Thái Ngọc Duy pclo...@gmail.com writes: These patches are on top of what's in 'pu'. They add --ignore-other-worktrees and make a note about current submodule support status. I don't think submodule support is ready yet even with Max Kirillov's series [1]. His 03/03 is already fixed in 'pu' though, so only 01/03 and 02/03 are new. [1] http://thread.gmane.org/gmane.comp.version-control.git/261107 With the understanding (perhaps a strongly-worded paragraph in the release notes) that this is not suitable for submodule users yet, is this in a good enough shape to go to 'next'? I'm not aware of any problems on this series (except submodules). So, yes. -- 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
Bad object pointed under refs/head/
Hi, I was running some test with Git 1.8.4.5, then I accidentally met a problem that leaded to the following error, error: refs/heads/develop does not point to a valid object! Turns out that the sha in refs/heads/develop is a bad object id, this happened after merging a branch X to branch develop, but packed-refs is updated to a corrected sha. No other merges at that point. The fix is easy, just removed refs/heads/develop. So there were two sha created, one is updated to refs/heads/develop, and the other one which is corrected, updated to packed-refs, Weird. I am wondering if there is a way to prevent this happening? Is this an ancient bug? Many thanks -- GitCafe.com Share a cup of open source Zhang Zheng @simsicon -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Pack v4 again..
After taking 1.5 years vacation from pack v4, I plan to do something about it again. Will post more when I have some patches to discuss. Only one question for now (forgive me if I asked already, it's been quite some time) I think pack v4 does not deliver its best promise that walking a tree is simply following pointers and jumping from place to place. When we want to copy from the middle of another tree, we need to scan from the beginning of the tree. Tree offset cache helps, but the problem remains. What do you think about an alternative format that each copy instruction includes both index of the tree entry to copy from (i.e. what we store now) _and_ the byte offset from the beginning of the tree? With this byte offset, we know exactly where to start copying without scanning from the beginning. It will be a bit(?) bigger, but it's also faster. I imagine this is an optimization that can be done locally. The pack transferred over network does not have these byte offsets. After the pack is stored and verified by index-pack, we can rewrite it and add this info. The simplest way is use a fixed size for this offset (e.g. uint16_t or even uint8_t), add the place holder in copy instructions of all v4 trees. After that object offsets will not change again and we can start filling real offsets to placeholders. PS. The rebased version on recent master is here if anyone is interested https://github.com/pclouds/git/commits/pack-v4 -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] Remove duplicate #include
deheader (git://gitorious.org/deheader/deheader.git) found out that some .c files #include twice one and the same header file. This patch removes such occurrences and hence speeds up the compilation. Signed-off-by: Дилян Палаузов git-...@aegee.org --- builtin/fetch.c| 1 - trailer.c | 1 - transport-helper.c | 1 - userdiff.c | 1 - 4 files changed, 4 deletions(-) diff --git a/builtin/fetch.c b/builtin/fetch.c index 7b84d35..75a55e5 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -11,7 +11,6 @@ #include run-command.h #include parse-options.h #include sigchain.h -#include transport.h #include submodule.h #include connected.h #include argv-array.h diff --git a/trailer.c b/trailer.c index 623adeb..05b3859 100644 --- a/trailer.c +++ b/trailer.c @@ -1,7 +1,6 @@ #include cache.h #include string-list.h #include run-command.h -#include string-list.h #include commit.h #include trailer.h /* diff --git a/transport-helper.c b/transport-helper.c index 0224687..3652b16 100644 --- a/transport-helper.c +++ b/transport-helper.c @@ -5,7 +5,6 @@ #include commit.h #include diff.h #include revision.h -#include quote.h #include remote.h #include string-list.h #include thread-utils.h diff --git a/userdiff.c b/userdiff.c index fad52d6..2ccbee5 100644 --- a/userdiff.c +++ b/userdiff.c @@ -1,6 +1,5 @@ #include cache.h #include userdiff.h -#include cache.h #include attr.h static struct userdiff_driver *drivers; -- 2.3.0.1.g24c2b87 -- To unsubscribe from this list: send the line 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 6/8] reflog_expire(): ignore --updateref for symbolic references
On 02/12/2015 10:54 PM, Jeff King wrote:
On Mon, Feb 09, 2015 at 10:12:42AM +0100, Michael Haggerty wrote:
if (!(flags EXPIRE_REFLOGS_DRY_RUN)) {
+/*
+ * It doesn't make sense to adjust a reference pointed
+ * to by a symbolic ref based on expiring entries in
+ * the symbolic reference's reflog.
+ */
+int update = (flags EXPIRE_REFLOGS_UPDATE_REF)
+~(type REF_ISSYMREF);
+
Isn't this second clause tautological? Unless REF_ISSYMREF covers all
bits in type, then type REF_ISSYMREF must always have at least one
bit 0, and negating it becomes non-zero. Did you mean:
!(type REF_ISSYMREF)
?
You're right, of course. Thanks.
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
Kind reminder about your Invited Paper
Dear Professor I would like to ask you if you have uploaded your Invited Paper in our conferences in Vienna, Austria, March 15-17, 2015: www.inase.org Invited Authors have a special privileged position in the conference program with double time for their presentation. The number of Invited Papers do not surpass the 4-5% of the total number of papers and Invited Authors can be members of the committee of the next INASE Conferences. Extended Versions of all the Invited papers will be promoted for direct publication in 36 Collaborating ISI/SCI Journals (with Impact Factor from Thomson Reuters) Proceedings will be published both in CD-ROM and Hard-Copy by INASE Press and will be immediately indexed in ISI/SCI, SCOPUS, EI Compendex, IET(IEE), AMS, ACS, CiteSeerX, Zentralblatt, British Library, EBSCO, SWETS, EMBASE, CAS, Scholar Google Inform us that you uploaded your invited paper, very simply, by writing in the Field: Short CV of the Main Author, the phrase: invited-by-zoe-michel-...@vger.kernel.org So, kindly upload your invited paper with the correct INASE format until February 20, 2015. (For regular papers the deadline is February 15; for Invited Papers like yours, we are giving 5 additional days). All the papers of 2013, 2014 and 2015 are now in ISI and SCOPUS! Contact me for more details now as well as for giving you more assistance on this matter. @ See reports and photos from former INASE conferences in our web site Best Regards Dr. Zoe Michel INASE Headquarters Our Phone Number: 00359 89 56 49 370 In case that you do not intend to participate in any of our future conferences of 2015 or of the upcoming years (2016,...), please, send an email to i...@inase.org with Subject in the Subject Line of your Email exactly the phrase: DO NOT INVITE ME AGAIN git@vger.kernel.org -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 8/8] reflog_expire(): lock symbolic refs themselves, not their referent
On 02/12/2015 07:04 PM, Stefan Beller wrote: On Thu, Feb 12, 2015 at 8:52 AM, Michael Haggerty mhag...@alum.mit.edu wrote: However, another important question is whether other Git implementations have copied this unusual locking policy. If so, that would be a reason not to change it. I just pinged the libgit2 maintainer to find out their policy. Maybe you could find out about JGit? I could not really find reflog expiration for jgit for a while, but then I stumbled upon this: // TODO: implement reflog_expire(pm, repo); so I think it's safe to change it in git.git for now. Stefan: This locking policy doesn't only affect reflog expire; we also need to lock the reflog when we add a new entry to it, for example when updating refs/heads/master through HEAD. Do you know what JGit locks in that scenario? I had a discussion in IRC [1] with Carlos Martín Nieto about how libgit2 handles this: * When libgit2 updates a reference through a symref, it locks the pointed-to reference while updating the reflog for the symbolic ref. So, for example, git commit would hold refs/heads/master.lock while updating both logs/refs/heads/master and logs/HEAD. (This matches the current Git behavior.) * libgit2 doesn't support reflog expire, though it does have an API that allows users to overwrite the reflog with a specified list of entries. That API locks the reference itself; i.e., HEAD.lock. This disagrees with Git's current behavior. I also realized that Git's current policy is probably not tenable if one process is re-seating a symref at the same time as another process is expiring its reflog. The git reflog expire HEAD might grab refs/heads/master.lock then start rewriting logs/HEAD. Meanwhile, git checkout foo would grab HEAD.lock (not being blocked by the expire process), then rewrite it to ref: refs/heads/foo, then grab refs/heads/foo.lock before updating logs/HEAD. So both processes could be writing logs/HEAD at the same time. In fact, it's even worse. git checkout foo and git symbolic-ref HEAD refs/heads/foo release HEAD.lock *before* updating logs/HEAD--in other words, they append to logs/HEAD without holding *any* lock. What is the best way forward? I think the current locking policy is untenable, even aside from the bug in symbolic-ref. Switching to holding only HEAD.lock while updating logs/HEAD is the right solution, but it would introduce an incompatibility with old versions of Git and libgit2 (and maybe JGit?) Probably nobody would notice, but who knows? Therefore, I propose that we hold *both* HEAD.lock *and* refs/heads/master.lock when modifying logs/HEAD (even when running reflog expire or reflog delete). I think this will be compatible with old versions of Git and libgit2, and it will also fix some design problems mentioned above. Plus, it will prevent updates to the two reflogs from appearing out of order relative to each other. Someday, when old clients have been retired, we can optionally switch to locking *only* HEAD.lock when modifying logs/HEAD. What do people think? Michael [1] https://github.com/libgit2/libgit2/issues/2902 -- 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 v2 04/12] ref_transaction_update(): remove have_old parameter
On 02/12/2015 06:32 PM, Junio C Hamano wrote: On Thu, Feb 12, 2015 at 3:12 AM, Michael Haggerty mhag...@alum.mit.edu wrote: Instead, verify the reference's old value if and only if old_sha1 is non-NULL. ... @@ -3664,9 +3664,6 @@ int ref_transaction_update(struct ref_transaction *transaction, if (transaction-state != REF_TRANSACTION_OPEN) die(BUG: update called for transaction that is not open); - if (have_old !old_sha1) - die(BUG: have_old is true but old_sha1 is NULL); - In the old world, old_sha1 here used to be one of (1) NULL, (2) null_sha1[], or (3) a real object name. What is the rule in the new world? The new world is explained in the docstring in refs.h, which was updated in this same commit: If old_sha1 is non-NULL, then it the value that the reference should have had before the update, or null_sha1 if it must not have existed beforehand. The docstring is further revised later in the patch series to old_sha1 is the value that the reference must have before the update, or null_sha1 if it must not have existed beforehand. The old value is checked after the lock is taken to prevent races. If the old value doesn't agree with old_sha1, the whole transaction fails. If old_sha1 is NULL, then the previous value is not checked. The new rule is extended to ref_transaction_delete() in the subsequent commit. I like the new semantics because it removes redundancy in the interpretation of parameters. Is that explanation adequate? 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
Git submodule removal and replacement results in failures
Let's say I have a submodule set to directory foo. If I remove this submodule in 1 commit (git rm -f foo) and then in a 2nd commit after that, physically commit those files, the next person that does a `git submodule update --recursive` results in failure because it says it can't overwrite files. I'm guessing what happens here is that when the submodule is being updated, it is not de-inited first, so the 2nd commit is trying to be applied but the submodule files aren't removed first, so it thinks it is going to replace unversioned files. Is there a way to make the submodule files be removed during an update, so that subsequent commits that may add files to replace the submodule aren't overwriting unversioned files? 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 8/8] reflog_expire(): lock symbolic refs themselves, not their referent
Michael Haggerty mhag...@alum.mit.edu writes: I also realized that Git's current policy is probably not tenable if one process is re-seating a symref at the same time as another process is expiring its reflog. The git reflog expire HEAD might grab refs/heads/master.lock then start rewriting logs/HEAD. Meanwhile, git checkout foo would grab HEAD.lock (not being blocked by the expire process), then rewrite it to ref: refs/heads/foo, then grab refs/heads/foo.lock before updating logs/HEAD. So both processes could be writing logs/HEAD at the same time. ... Switching to holding only HEAD.lock while updating logs/HEAD is the right solution,... We convinced ourselves that not locking the symref is wrong, but have we actually convinced us that not locking the underlying ref, as long as we have a lock on the symref, is safe? To protect you, the holder of a lock on refs/remotes/origin/HEAD that happens to point at refs/remotes/origin/next, from somebody who is updating the underlying refs/remotes/origin/next directly without going through the symbolic ref (e.g. receive-pack), wouldn't the other party need to find any and all symbolic refs that point at the underlying ref and take locks on them? As dereferencing a symbolic ref in the forward direction is much cheaper than in the reverse, and because you need to dereference it anyway, I wonder if we want the endgame to be hold locks on both, not just hold a lock on the symlink. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 04/12] ref_transaction_update(): remove have_old parameter
Michael Haggerty mhag...@alum.mit.edu writes: On 02/12/2015 06:32 PM, Junio C Hamano wrote: On Thu, Feb 12, 2015 at 3:12 AM, Michael Haggerty mhag...@alum.mit.edu wrote: Instead, verify the reference's old value if and only if old_sha1 is non-NULL. ... @@ -3664,9 +3664,6 @@ int ref_transaction_update(struct ref_transaction *transaction, if (transaction-state != REF_TRANSACTION_OPEN) die(BUG: update called for transaction that is not open); - if (have_old !old_sha1) - die(BUG: have_old is true but old_sha1 is NULL); - In the old world, old_sha1 here used to be one of (1) NULL, (2) null_sha1[], or (3) a real object name. What is the rule in the new world? ... ... If old_sha1 is NULL, then the previous value is not checked. OK. That makes it perfectly clear that removing these lines is the right thing to do. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 8/8] reflog_expire(): lock symbolic refs themselves, not their referent
On Fri, Feb 13, 2015 at 10:26 AM, Junio C Hamano gits...@pobox.com wrote: Stefan Beller sbel...@google.com writes: On Fri, Feb 13, 2015 at 10:05 AM, Junio C Hamano gits...@pobox.com wrote: We convinced ourselves that not locking the symref is wrong, but have we actually convinced us that not locking the underlying ref, as long as we have a lock on the symref, is safe? To protect you, the holder of a lock on refs/remotes/origin/HEAD that happens to point at refs/remotes/origin/next, from somebody who is updating the underlying refs/remotes/origin/next directly without going through the symbolic ref (e.g. receive-pack), wouldn't the other party need to find any and all symbolic refs that point at the underlying ref and take locks on them? As we're just modifying the ref log of HEAD in this case, we don't bother with where the HEAD points to. The other party may change refs/remotes/origin/next without us noticing, but we don't care here as all we do is rewriting the ref log for HEAD. If the other party were to modify HEAD (point it to some other branch, or forward the branch pointed to), they'd be expected to lock HEAD and would fail as we have the lock? I was not talking about HEAD in what you are responding to, though. Don't we maintain a reflog on refs/remotes/origin/HEAD? Is it OK to allow its entries to become inconsistent with the underlying ref? Yes we do have a HEAD ref log, and that ref log would differ from the underlying ref log. I don't know if that is desired, but I would tend to not like it. So the other party updating the underlying ref would also need to lock HEAD then, which ... they don't. But they try writing to it nevertheless, in refs.c line 3121-3150 see the /* special hack */ comment. -- To unsubscribe from this list: send the line 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 8/8] reflog_expire(): lock symbolic refs themselves, not their referent
Stefan Beller sbel...@google.com writes: On Fri, Feb 13, 2015 at 10:05 AM, Junio C Hamano gits...@pobox.com wrote: We convinced ourselves that not locking the symref is wrong, but have we actually convinced us that not locking the underlying ref, as long as we have a lock on the symref, is safe? To protect you, the holder of a lock on refs/remotes/origin/HEAD that happens to point at refs/remotes/origin/next, from somebody who is updating the underlying refs/remotes/origin/next directly without going through the symbolic ref (e.g. receive-pack), wouldn't the other party need to find any and all symbolic refs that point at the underlying ref and take locks on them? As we're just modifying the ref log of HEAD in this case, we don't bother with where the HEAD points to. The other party may change refs/remotes/origin/next without us noticing, but we don't care here as all we do is rewriting the ref log for HEAD. If the other party were to modify HEAD (point it to some other branch, or forward the branch pointed to), they'd be expected to lock HEAD and would fail as we have the lock? I was not talking about HEAD in what you are responding to, though. Don't we maintain a reflog on refs/remotes/origin/HEAD? Is it OK to allow its entries to become inconsistent with the underlying ref? -- To unsubscribe from this list: send the line 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 8/8] reflog_expire(): lock symbolic refs themselves, not their referent
On Fri, Feb 13, 2015 at 8:26 AM, Michael Haggerty mhag...@alum.mit.edu wrote: What is the best way forward? Switching to holding only HEAD.lock while updating logs/HEAD is the right solution, but it would introduce an incompatibility with old versions of Git and libgit2 (and maybe JGit?) Probably nobody would notice, but who knows? Therefore, I propose that we hold *both* HEAD.lock *and* refs/heads/master.lock when modifying logs/HEAD (even when running reflog expire or reflog delete). I think this will be compatible with old versions of Git and libgit2, and it will also fix some design problems mentioned above. Plus, it will prevent updates to the two reflogs from appearing out of order relative to each other. Yeah, I agree on that. It's ok to lock both for now (it doesn't really harm the user). But we really need to document it in the source code why we lock both and that the 2nd lock can be removed after time has passed, something like: /* * We need to lock both the symbolic ref as well as * the dereferenced ref for now because of inconsistencies with * older and other implementations of git. Originally only the * dereferenced ref lock was held, but discussion($gmane/263555) * turned out, we actually want to hold the lock on the symbolic ref. * For now hold both locks until all implementation hold the lock on * the symbolic ref. (Feb/2015) */ Someday, when old clients have been retired, we can optionally switch to locking *only* HEAD.lock when modifying logs/HEAD. What do people think? Michael [1] https://github.com/libgit2/libgit2/issues/2902 -- 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 8/8] reflog_expire(): lock symbolic refs themselves, not their referent
On Fri, Feb 13, 2015 at 10:05 AM, Junio C Hamano gits...@pobox.com wrote: Michael Haggerty mhag...@alum.mit.edu writes: I also realized that Git's current policy is probably not tenable if one process is re-seating a symref at the same time as another process is expiring its reflog. The git reflog expire HEAD might grab refs/heads/master.lock then start rewriting logs/HEAD. Meanwhile, git checkout foo would grab HEAD.lock (not being blocked by the expire process), then rewrite it to ref: refs/heads/foo, then grab refs/heads/foo.lock before updating logs/HEAD. So both processes could be writing logs/HEAD at the same time. ... Switching to holding only HEAD.lock while updating logs/HEAD is the right solution,... We convinced ourselves that not locking the symref is wrong, but have we actually convinced us that not locking the underlying ref, as long as we have a lock on the symref, is safe? To protect you, the holder of a lock on refs/remotes/origin/HEAD that happens to point at refs/remotes/origin/next, from somebody who is updating the underlying refs/remotes/origin/next directly without going through the symbolic ref (e.g. receive-pack), wouldn't the other party need to find any and all symbolic refs that point at the underlying ref and take locks on them? As we're just modifying the ref log of HEAD in this case, we don't bother with where the HEAD points to. The other party may change refs/remotes/origin/next without us noticing, but we don't care here as all we do is rewriting the ref log for HEAD. If the other party were to modify HEAD (point it to some other branch, or forward the branch pointed to), they'd be expected to lock HEAD and would fail as we have the lock? As dereferencing a symbolic ref in the forward direction is much cheaper than in the reverse, and because you need to dereference it anyway, I wonder if we want the endgame to be hold locks on both, not just hold a lock on the symlink. That would be the safest option indeed. -- To unsubscribe from this list: send the line 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: Keep original author with git merge --squash?
Jeff King p...@peff.net writes: On Thu, Feb 12, 2015 at 03:32:37PM -0800, Junio C Hamano wrote: It also raises a question for the proposal in this thread: if there are multiple Author: lines, which one do we take? The first, or the last? I was siding with David's pay attention to in-buffer Author: only when all of them agree. When squash-merging a branch with two or more authors, we would attribute the authorship silently and automatically to you if you do not do anything special otherwise. That's probably reasonable. I was thinking more of a case where you made some fixups on top of somebody else's branch, and then used git rebase -i to squash them together. But I think we already use the authorship for the root of the squash in that case. This case collapses nicely if we make a slight tweak to your proposed behavior (or maybe this is what you meant). If there are multiple authors listed, we behave as if none was listed. That would leave the authorship as it behaves today (with the author of the first commit) if you do nothing, or you can override it by dropping all but one. I actually was (and am still) wondering that silently ignore all of them if there are multiple ones that contradict with each other is a bad idea, and that was why the last item on the possible alternatives list was to error out and ask clarification. -- To unsubscribe from this list: send the line 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] transport-helper: ask the helper to set the same options for import as for fetch
Mike Hommey m...@glandium.org writes: A remote helper is currently only told about the 'check-connectivity', 'cloning', and 'update-shallow' options when it supports the 'fetch' command, but not when it supports 'import' instead. Sounds sensible. Does the same issue exist for export vs push or do they happen to be coded to pass similar enough set of options already by copied and pasted code? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] SQUASH??? t9001: turn --no$option workarounds to --no-$option
These were done to work around older versions of Getopt::Long that did not take negation of a boolean --option as --no-option (but they happily took --nooption). I am inclined to squash this into the previous one. Signed-off-by: Junio C Hamano gits...@pobox.com --- t/t9001-send-email.sh | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh index af6a3e8..0513055 100755 --- a/t/t9001-send-email.sh +++ b/t/t9001-send-email.sh @@ -392,7 +392,7 @@ test_expect_success $PREREQ 'allow long lines with --no-validate' ' --from=Example nob...@example.com \ --to=nob...@example.com \ --smtp-server=$(pwd)/fake.sendmail \ - --novalidate \ + --no-validate \ $patches longline.patch \ 2errors ' @@ -426,7 +426,7 @@ test_expect_success $PREREQ 'In-Reply-To without --chain-reply-to' ' git send-email \ --from=Example nob...@example.com \ --to=nob...@example.com \ - --nochain-reply-to \ + --no-chain-reply-to \ --in-reply-to=$(cat expect) \ --smtp-server=$(pwd)/fake.sendmail \ $patches $patches $patches \ @@ -1067,7 +1067,7 @@ test_expect_success $PREREQ 'in-reply-to but no threading' ' --from=Example nob...@example.com \ --to=nob...@example.com \ --in-reply-to=in-reply...@example.com \ - --nothread \ + --no-thread \ $patches | grep In-Reply-To: in-reply...@example.com ' @@ -1077,7 +1077,7 @@ test_expect_success $PREREQ 'no in-reply-to and no threading' ' --dry-run \ --from=Example nob...@example.com \ --to=nob...@example.com \ - --nothread \ + --no-thread \ $patches $patches stdout ! grep In-Reply-To: stdout ' @@ -1088,7 +1088,7 @@ test_expect_success $PREREQ 'threading but no chain-reply-to' ' --from=Example nob...@example.com \ --to=nob...@example.com \ --thread \ - --nochain-reply-to \ + --no-chain-reply-to \ $patches $patches stdout grep In-Reply-To: stdout ' -- 2.3.0-191-geb1a277 -- To unsubscribe from this list: send the line 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 8/8] reflog_expire(): lock symbolic refs themselves, not their referent
On 02/13/2015 08:12 PM, Junio C Hamano wrote: [...] As we are trying to see a way to move forward to do the right thing around reflog, I was wondering if locking only the symbolic ref is a sensible endgame. The right thing being: When a symbolic ref S points at underlying ref R, and if R's tip changes from X to Y, we would want to know from the reflog of S that S used to point at X and then changed to point at Y. Let's first talk about an ideal world if we had complete support for symbolic references. Yes, I agree with your principle. Moreover, suppose that S and S2 *both* point at R, and there is a third symref S3 that points at symref S (symbolic refs can point at other symbolic refs!): X - R - S - S3 \ S2 Now, if R updated from X to Y (regardless of whether the update is via R directly or via one of the symrefs), then each of the four reflogs (R, S, S2, and S3) should gain a new entry reflecting the update. If S is reseated to point at R2 instead of R, then the reflogs for S and for S3 should each gain new entries What locks should we hold? In my opinion, we should hold the locks on exactly those references (symbolic or regular) whose reflogs we want to change. So in the first example, we should hold $GIT_DIR/$R.lock $GIT_DIR/$S.lock $GIT_DIR/$S2.lock, and $GIT_DIR/$S3.lock Ideally, we should acquire all of the locks before making any modifications. Now back to the real world. Currently, if R is changed *through* a symbolic reference S, then the reflogs for both R and S are updated, but not the reflogs for any other symbolic references that might point at R. If R is changed directly, then no symref's reflogs are affected, except for the special case that HEAD's reflog is changed if it points directly at R. This limitation is a hack to avoid having to walk symrefs backwards to find any symrefs that might be pointing at R. If a symref is reseated, then its reflog is changed but not that of any symrefs that might be pointed at it. It might actually not be extremely expensive to follow symrefs backwards. Symbolic references cannot be packed, so we would only have to scan the loose references; we could ignore packed refs. But it would still be a lot more expensive than just updating one file. I don't know that it's worth it, given that symbolic references are used so sparingly. I think that the rule about locks as expressed above can be carried over the the real world: We should hold the locks on exactly those references (symbolic or regular) whose reflogs we plan to change. We should acquire all of the locks before making any changes. 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 0/2] Getopt::Long workaround in send-email
On Feb 13, 2015, at 12:19, Junio C Hamano wrote: The first one is a replay of Kyle's workaround for older versions of Getopt::Long that did not take --no-option to negate a boolean option --option. The second one reverts the workarounds made to the test script over time, and should break if the first one does not work well for older Getopt::Long (I have no reason to suspect it would break, though). I am inclined to squash these into one commit before starting to merge them down to 'next' and then to 'master', after getting Tested-by: from those with older Getopt::Long (prior to 2.32). I have no objection to them being squashed together. -Kyle -- To unsubscribe from this list: send the line 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 8/8] reflog_expire(): lock symbolic refs themselves, not their referent
Stefan Beller sbel...@google.com writes: On Fri, Feb 13, 2015 at 10:26 AM, Junio C Hamano gits...@pobox.com wrote: Stefan Beller sbel...@google.com writes: On Fri, Feb 13, 2015 at 10:05 AM, Junio C Hamano gits...@pobox.com wrote: We convinced ourselves that not locking the symref is wrong, but have we actually convinced us that not locking the underlying ref, as long as we have a lock on the symref, is safe? To protect you, the holder of a lock on refs/remotes/origin/HEAD that happens to point at refs/remotes/origin/next, from somebody who is updating the underlying refs/remotes/origin/next directly without going through the symbolic ref (e.g. receive-pack), wouldn't the other party need to find any and all symbolic refs that point at the underlying ref and take locks on them? As we're just modifying the ref log of HEAD in this case, we don't bother with where the HEAD points to. The other party may change refs/remotes/origin/next without us noticing, but we don't care here as all we do is rewriting the ref log for HEAD. If the other party were to modify HEAD (point it to some other branch, or forward the branch pointed to), they'd be expected to lock HEAD and would fail as we have the lock? I was not talking about HEAD in what you are responding to, though. Don't we maintain a reflog on refs/remotes/origin/HEAD? Is it OK to allow its entries to become inconsistent with the underlying ref? Yes we do have a HEAD ref log, and that ref log would differ from the underlying ref log. I don't know if that is desired, but I would tend to not like it. HEAD (or refs/remotes/origin/HEAD) reflog and reflog for refs/heads/master (or refs/remotes/origin/next) would have to be different as long as we allow symbolic refs to be repointed to different refs. If HEAD refers to 'next' today, and at the tip of next sits commit X, the reflog for both of them would record the fact that they were pointing at X. If you repoint HEAD to point at 'master' (e.g. git checkout master) whose tip is at commit Y, then reflog for HEAD would record the fact that now HEAD points at Y, and reflogs for 'master' or 'next' would not change merely because you switched where HEAD points at. And there is anything to like or not to like about it. As we are trying to see a way to move forward to do the right thing around reflog, I was wondering if locking only the symbolic ref is a sensible endgame. The right thing being: When a symbolic ref S points at underlying ref R, and if R's tip changes from X to Y, we would want to know from the reflog of S that S used to point at X and then changed to point at Y. Replace S and R with either (HEAD, refs/heads/master) or (refs/remotes/origin/HEAD, refs/remotes/origin/next) in the above and we want both to be true. How best to achieve that, and what is the set of right ref(s) to take lock(s) on? I am not very much interested in how incorrect today's code might behave. That is not the central point when discussing what is the best way forward. So the other party updating the underlying ref would also need to lock HEAD then, Yes, that is what I meant. Your also can be read in two different ways (other party, too or HEAD, too), though, and I think we want both ;-). That is why I hinted that it was iffy to state that we only have to take the lock only on S and not on R, but only as a workaround to keep older implementation out we take both---once they get extinct we can get away with by taking a lock only on S. When pushing to update 'master' and 'next' into a repository whose 'HEAD' points at 'master', we would want to take locks on 'next' (no question), but is it sensible to take the lock on 'HEAD' and deliberately leave 'master' unlocked? Or is it more sensible to take all locks on the underlying refs involved (i.e. 'next' and 'master') and in addition any symbolic refs that are pointing at these refs involved (i.e. 'HEAD')? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] Win32: nanosecond-precision file times
Karsten Blees karsten.bl...@gmail.com writes: Am 13.02.2015 um 00:38 schrieb Junio C Hamano: We do have sec/nsec fields in cache_time structure, so I have nothing against updating the msysGit port to fill that value. Having said that, we do not enable the NSEC stuff by default on Unix for a reason. I'd expect those who know Windows filesystems well to pick the default there wisely ;-) -- To unsubscribe from this list: send the line 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 ls-files -X option is relative to repo root
I encountered some unexpected behavior with Git today and was hoping to either a) clear up my misconception or b) make a bug report. My question deals with the --exclude-from option to git-ls-files. It appears that paths passed to this option are relative to the root of the repository, not your current working directory. I would have expected the opposite, that the paths are relative to the working directory. I would expect to put a colon at the beginning of the path to make it relative to the repository root. Here's an example: $ tree -a -I .git . ├── .gitignore └── example_dir ├── .gitignore └── example_file # (all of these files are checked in, including example_file which is also in .gitignore) $ cat .gitignore # empty gitignore $ cat example_dir/.gitignore example_file $ (cd example_dir git ls-files --ignore --exclude-from=.gitignore) # No output because this references the git ignore at the project root, not example_dir. I expected this to output example_file. $ (cd example_dir git ls-files --ignore --exclude-from=example_dir/.gitignore) example_file # works for the reason above, but I expected this to break because example_dir/example_dir/.gitignore is not a file So, what do you think? Am I missing a git/*nix convention explaining options would be specified relative to the repository root? Or is this a git bug? Thanks for your time, Dan Finnie -- To unsubscribe from this list: send the line 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: Keep original author with git merge --squash?
On Fri, Feb 13, 2015 at 11:30:53AM -0800, Junio C Hamano wrote: This case collapses nicely if we make a slight tweak to your proposed behavior (or maybe this is what you meant). If there are multiple authors listed, we behave as if none was listed. That would leave the authorship as it behaves today (with the author of the first commit) if you do nothing, or you can override it by dropping all but one. I actually was (and am still) wondering that silently ignore all of them if there are multiple ones that contradict with each other is a bad idea, and that was why the last item on the possible alternatives list was to error out and ask clarification. Normally I like error out and ask the user as an approach to avoiding mistakes, but I can think of two bad side effects: 1. If we pre-populate the # Author: lines in git merge --squash, then if I run git commit on the result and don't explicitly take an action to clean up those comment fields, I get an error. That's kind of annoying. 2. Dumping the user out of git commit with an error isn't very elegant. They may have put significant work into writing the commit message. It's saved there in COMMIT_EDITMSG, but what is the easy path to them repeating their action where they left off? It seems like the potential for confusion comes from the same place as my complaint (1) above: the implicit-ness of the # Author: lines (git writes them, assumes you've looked at and manipulated them to your liking, and then reads them back in). What if there was a step required of the user to say really, I want to use this one? Like converting s/Author/Set-Author/, or taking away the # comment character (though that has its own confusions, as you noted earlier). -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] git-send-email.perl: support no- prefix with older GetOptions
From: Kyle J. McKay mack...@gmail.com
Only Perl version 5.8.0 or later is required, but that comes with
an older Getopt::Long (2.32) that does not support the 'no-'
prefix. Support for that was added in Getopt::Long version 2.33.
Since the help only mentions the 'no-' prefix and not the 'no'
prefix, add explicit support for the 'no-' prefix when running
with older GetOptions versions.
Reported-by: Tom G. Christensen t...@statsbiblioteket.dk
Signed-off-by: Kyle J. McKay mack...@gmail.com
Signed-off-by: Junio C Hamano gits...@pobox.com
---
git-send-email.perl | 10 ++
1 file changed, 10 insertions(+)
diff --git a/git-send-email.perl b/git-send-email.perl
index 3092ab3..a18a795 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -299,6 +299,7 @@ sub signal_handler {
bcc=s = \@bcclist,
no-bcc = \$no_bcc,
chain-reply-to! = \$chain_reply_to,
+ no-chain-reply-to = sub {$chain_reply_to = 0},
smtp-server=s = \$smtp_server,
smtp-server-option=s = \@smtp_server_options,
smtp-server-port=s = \$smtp_server_port,
@@ -311,25 +312,34 @@ sub signal_handler {
smtp-domain:s = \$smtp_domain,
identity=s = \$identity,
annotate! = \$annotate,
+ no-annotate = sub {$annotate = 0},
compose = \$compose,
quiet = \$quiet,
cc-cmd=s = \$cc_cmd,
suppress-from! = \$suppress_from,
+ no-suppress-from = sub {$suppress_from = 0},
suppress-cc=s = \@suppress_cc,
signed-off-cc|signed-off-by-cc! = \$signed_off_by_cc,
+ no-signed-off-cc|no-signed-off-by-cc = sub
{$signed_off_by_cc = 0},
cc-cover|cc-cover! = \$cover_cc,
+ no-cc-cover = sub {$cover_cc = 0},
to-cover|to-cover! = \$cover_to,
+ no-to-cover = sub {$cover_to = 0},
confirm=s = \$confirm,
dry-run = \$dry_run,
envelope-sender=s = \$envelope_sender,
thread! = \$thread,
+ no-thread = sub {$thread = 0},
validate! = \$validate,
+ no-validate = sub {$validate = 0},
transfer-encoding=s = \$target_xfer_encoding,
format-patch! = \$format_patch,
+ no-format-patch = sub {$format_patch = 0},
8bit-encoding=s = \$auto_8bit_encoding,
compose-encoding=s = \$compose_encoding,
force = \$force,
xmailer! = \$use_xmailer,
+ no-xmailer = sub {$use_xmailer = 0},
);
usage() if $help;
--
2.3.0-191-geb1a277
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/2] Getopt::Long workaround in send-email
The first one is a replay of Kyle's workaround for older versions of Getopt::Long that did not take --no-option to negate a boolean option --option. The second one revert the workarounds made to the test script over time, and should break if the first one does not work well for older Getopt::Long (I have no reason to suspect it would break, though). I am inclined to squash these into one commit before starting to merge them down to 'next' and then to 'master', after getting Tested-by: from those with older Getopt::Long (prior to 2.32). Obviously, tc/t9001-noxmailer topic will become unnecessary and be dropped when that happens. Thanks. Junio C Hamano (1): SQUASH??? t9001: turn --no$option workarounds to --no-$option Kyle J. McKay (1): git-send-email.perl: support no- prefix with older GetOptions git-send-email.perl | 10 ++ t/t9001-send-email.sh | 10 +- 2 files changed, 15 insertions(+), 5 deletions(-) -- 2.3.0-191-geb1a277 -- To unsubscribe from this list: send the line 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 ls-files -X option is relative to repo root
Daniel Finnie d...@danfinnie.com writes: My question deals with the --exclude-from option to git-ls-files. You will be fine if you remember that these plumbing commands work by first cd'ing to the top-level of the working tree (and adjust the paths given from the command line by prefixing the prefix to them). The input lines that come from --exclude-per-directory should go through the prefixing, but -X=file makes that single file affect the whole operation. It does not make sense to allow where you are to affect behaviour of the command, i.e. in these two invocations of ls-files: git ls-files -X /var/tmp/exclude -i cd example git ls-files -X /var/tmp/exclude -i if the same line in /var/tmp/exclude meant completely different things, it would be crazy. -- To unsubscribe from this list: send the line 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] transport-helper: ask the helper to set the same options for import as for fetch
On Fri, Feb 13, 2015 at 11:36:24AM -0800, Junio C Hamano wrote: Mike Hommey m...@glandium.org writes: A remote helper is currently only told about the 'check-connectivity', 'cloning', and 'update-shallow' options when it supports the 'fetch' command, but not when it supports 'import' instead. Sounds sensible. Does the same issue exist for export vs push or do they happen to be coded to pass similar enough set of options already by copied and pasted code? The issue exists: - export is given dry-run, pushcert and force. - push is given cas, dry-run and pushcert. (note: cas and pushcert are both not documented in gitremote-helpers.txt) Force is actually not necessary for push, because the push syntax itself includes the force instruction in the refspec given as argument. I haven't looked exactly what cas does and if it makes sense for export. (FWIW, I'm using push and import at the moment, so it's not a direct issue for me ; I don't support cas anyways) 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: git ls-files -X option is relative to repo root
Hi Junio, Thanks for the info and backstory. I didn't realize that the paths in the file specified by --exclude-from would be relative to the project root. That makes my original use case kind of silly (it's a long story, but I was curious which files were ignored by a subset of my .gitignore files). This makes the example I gave before wrong in that the contents of example_dir/.gitignore should have been relative to the project root. There are 2 sets of paths that can be relative to the working directory or the project root with --exclude-from: * The path to the file containing exclusion rules * The paths of the exclusion rules themselves (the contents of the file from the previous bullet point) I now see why the second needs to be relative to the project root. I still think it's more intuitive for the first (the path to the file containing exclusions) to be relative to the current directory. Your example of `git ls-files -X /var/tmp/exclude -i` uses an absolute path, let's look at one with a relative path. Say you wanted to check what files were being ignored from your .git/info/exclude file from a subdirectory of your project. I would expect to run `cd subdirectory git ls-files --ignore --other --exclude-from=../.git/info/exclude`. The correct command, though, is `cd subdirectory git ls-files --ignore --other --exclude-from=.git/info/exclude`, even though I'm in a subdirectory. Do you have any comments on why the path in --exclude-from=path is relative to the project root? Thanks, Dan On Fri, Feb 13, 2015 at 3:54 PM, Junio C Hamano gits...@pobox.com wrote: Junio C Hamano gits...@pobox.com writes: ... It does not make sense to allow where you are to affect behaviour of the command, i.e. in these two invocations of ls-files: git ls-files -X /var/tmp/exclude -i cd example git ls-files -X /var/tmp/exclude -i if the same line in /var/tmp/exclude meant completely different things, it would be crazy. To put it another way, think of --exclude-from as a way to specify a replacement for .git/info/excludes, and --exclude-per-directory as a way to specify a replacement for the in-tree .gitignore files. Historically, we did not have the --exclude-standard option from the beginning, and only after we gained experience with --exclude-from and --exclude-per-directory in our scripts, the --exclude-standard was added to codify the (then-) best-current-practice after the fact, and we used --exclude-from for exactly that purpose before then. cf. 8e7b07c8 (git-ls-files: add --exclude-standard, 2007-11-15) -- To unsubscribe from this list: send the line 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] hex.c: reduce memory footprint of sha1_to_hex static buffers
Stefan Beller sbel...@google.com writes:
41 bytes is the exact number of bytes needed for having the returned
hex string represented. 50 seems to be an arbitrary number, such
that there are no benefits from alignment to certain address boundaries.
Yes, with s/seems to be/is/;
This comes from e83c5163 (Initial revision of git, the information
manager from hell, 2005-04-07), and when dcb3450f (sha1_to_hex()
usage cleanup, 2006-05-03) introduced the 4 recycled buffers on
top, the underlying array was left at 50 bytes long.
You can now have I fixed Linus's bug badge ;-)
Signed-off-by: Stefan Beller sbel...@google.com
---
hex.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hex.c b/hex.c
index 9ebc050..cfd9d72 100644
--- a/hex.c
+++ b/hex.c
@@ -59,7 +59,7 @@ int get_sha1_hex(const char *hex, unsigned char *sha1)
char *sha1_to_hex(const unsigned char *sha1)
{
static int bufno;
- static char hexbuffer[4][50];
+ static char hexbuffer[4][41];
static const char hex[] = 0123456789abcdef;
char *buffer = hexbuffer[3 ++bufno], *buf = buffer;
int i;
--
To unsubscribe from this list: send the line 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] hex.c: reduce memory footprint of sha1_to_hex static buffers
On Fri, Feb 13, 2015 at 1:56 PM, Stefan Beller sbel...@google.com wrote: As I could not find any documentation on the magical 50 in the early days, I cc'd Linus in case there is something I did not think of yet. Nothing magical, it's just rounded up from 40 + NUL character. Linus -- To unsubscribe from this list: send the line 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] Remove duplicate #include
Hello Junio, in theory it speeds up, because the preprocessor has less work to do. In practice I don't know how much and this seems also irrelevant criterion for accepting this patch. Greetings Dilyan On 13.02.2015 22:15, Junio C Hamano wrote: Дилян Палаузов git-...@aegee.org writes: deheader (git://gitorious.org/deheader/deheader.git) found out that some .c files #include twice one and the same header file. This patch removes such occurrences and hence speeds up the compilation. Does it speed up? By how much? Any numbers? I do not see any reason to reject this change. Removing repeated inclusions of the same header is a good thing by itself [*1*]. Thanks. [Footnote] *1* If things break when repeated inclusions are removed, that would mean the headers were wrong in the first place. I do not think transport.h, string-list.h, quote.h and cache.h have any reason why they need to be included twice to work correctly, and in fact they are designed to be no-op when included twice. Signed-off-by: Дилян Палаузов git-...@aegee.org --- builtin/fetch.c| 1 - trailer.c | 1 - transport-helper.c | 1 - userdiff.c | 1 - 4 files changed, 4 deletions(-) diff --git a/builtin/fetch.c b/builtin/fetch.c index 7b84d35..75a55e5 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -11,7 +11,6 @@ #include run-command.h #include parse-options.h #include sigchain.h -#include transport.h #include submodule.h #include connected.h #include argv-array.h diff --git a/trailer.c b/trailer.c index 623adeb..05b3859 100644 --- a/trailer.c +++ b/trailer.c @@ -1,7 +1,6 @@ #include cache.h #include string-list.h #include run-command.h -#include string-list.h #include commit.h #include trailer.h /* diff --git a/transport-helper.c b/transport-helper.c index 0224687..3652b16 100644 --- a/transport-helper.c +++ b/transport-helper.c @@ -5,7 +5,6 @@ #include commit.h #include diff.h #include revision.h -#include quote.h #include remote.h #include string-list.h #include thread-utils.h diff --git a/userdiff.c b/userdiff.c index fad52d6..2ccbee5 100644 --- a/userdiff.c +++ b/userdiff.c @@ -1,6 +1,5 @@ #include cache.h #include userdiff.h -#include cache.h #include attr.h static struct userdiff_driver *drivers; -- To unsubscribe from this list: send the line 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 ls-files -X option is relative to repo root
Junio C Hamano gits...@pobox.com writes: ... It does not make sense to allow where you are to affect behaviour of the command, i.e. in these two invocations of ls-files: git ls-files -X /var/tmp/exclude -i cd example git ls-files -X /var/tmp/exclude -i if the same line in /var/tmp/exclude meant completely different things, it would be crazy. To put it another way, think of --exclude-from as a way to specify a replacement for .git/info/excludes, and --exclude-per-directory as a way to specify a replacement for the in-tree .gitignore files. Historically, we did not have the --exclude-standard option from the beginning, and only after we gained experience with --exclude-from and --exclude-per-directory in our scripts, the --exclude-standard was added to codify the (then-) best-current-practice after the fact, and we used --exclude-from for exactly that purpose before then. cf. 8e7b07c8 (git-ls-files: add --exclude-standard, 2007-11-15) -- To unsubscribe from this list: send the line 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 8/8] reflog_expire(): lock symbolic refs themselves, not their referent
Michael Haggerty mhag...@alum.mit.edu writes: Now back to the real world. Currently, if R is changed *through* a symbolic reference S, then the reflogs for both R and S are updated, but not the reflogs for any other symbolic references that might point at R. If R is changed directly, then no symref's reflogs are affected, except for the special case that HEAD's reflog is changed if it points directly at R. This limitation is a hack to avoid having to walk symrefs backwards to find any symrefs that might be pointing at R. Yup. It might actually not be extremely expensive to follow symrefs backwards. Symbolic references cannot be packed, so we would only have to scan the loose references; we could ignore packed refs. But it would still be a lot more expensive than just updating one file. I don't know that it's worth it, given that symbolic references are used so sparingly. I personally do not think it is worth it. I further think that it would be perfectly OK to do one of the following: - We only maintain reflogs for $GIT_DIR/HEAD; no other symrefs get their own reflog, and we only check $GIT_DIR/HEAD when updating refs/heads/* and no other refs for direct reference (i.e. HEAD - refs/something/else - refs/heads/master symref chain is ignored). - In addition to the above, we also maintain reflogs for $GIT_DIR/refs/remotes/*/HEAD but support only when they directly point into a remote tracking branch in the same hierarchy. $GIT_DIR/refs/remotes/foo/HEAD that points at $GIT_DIR/refs/remotes/bar/master is ignored and will get an undefined behaviour. I think that the rule about locks as expressed above can be carried over the the real world: We should hold the locks on exactly those references (symbolic or regular) whose reflogs we plan to change. We should acquire all of the locks before making any changes. Sure. -- To unsubscribe from this list: send the line 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] hex.c: reduce memory footprint of sha1_to_hex static buffers
On Fri, Feb 13, 2015 at 1:41 PM, Junio C Hamano gits...@pobox.com wrote: Stefan Beller sbel...@google.com writes: 41 bytes is the exact number of bytes needed for having the returned hex string represented. 50 seems to be an arbitrary number, such that there are no benefits from alignment to certain address boundaries. Yes, with s/seems to be/is/; This comes from e83c5163 (Initial revision of git, the information manager from hell, 2005-04-07), and when dcb3450f (sha1_to_hex() usage cleanup, 2006-05-03) introduced the 4 recycled buffers on top, the underlying array was left at 50 bytes long. You can now have I fixed Linus's bug badge ;-) I don't think it's a bug, it's just wasting memory? As I could not find any documentation on the magical 50 in the early days, I cc'd Linus in case there is something I did not think of yet. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git ls-files -X option is relative to repo root
Daniel Finnie d...@danfinnie.com writes: Do you have any comments on why the path in --exclude-from=path is relative to the project root? Not really. Because ls-files was designed to be used by Porcelain scripts, and because the first thing Porcelain scripts are expected to do is to learn the prefix and then cd to the root level of the working tree before doing anything else, path that is relative to the root level of the working tree ends up to be not so unnatural thing to be used with --exclude-from=path (e.g. .git/info/exclude). If it were relative to whatever subdirectory the invoker of the Porcelain script happened to be, Porcelain would have to do a lot more (e.g. in cd x/y myPorcelain ../../.git/info/exclude, the myPorcelain script would first have to learn the prefix is x/y, go up two levels, and then strip two ../ from ../../.git/info/exclude to turn it into .git/info/exclude when it runs ls-files). So that is a convenience explanation in retrospect, but Why is often a futile question to ask when talking about evolution, in which whatever works gets picked. -- To unsubscribe from this list: send the line 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: [msysGit] Re: Windows Bluescreen
Am 12.02.2015 um 14:21 schrieb Erik Friesen: Sorry, I don't know what this TOP posting problem is, and hitting reply only replies to the last sender. If you prefer, and you have some regular bugtracker, I could use that instead of email posting. To repro- Set up git user on local linux repo, in my case 192.168.0.100 On a windows 7 64bit machine- $ cd myproject $ git init $ git add . $ git commit -m 'initial commit' $ git remote add origin git@192.168.0.100:/opt/git/project.git $ git push origin master (Shamelessly copied from git page) Problem happens after entering password, it may or may not get started. The nature of a bluescreen doesn't make debugging and reproduction real easy. If it helps, I can get the dumps from those crashes. To move on, I moved back to local http pushes. I'm doing pushes over ssh on a daily basis against my linux server. And I'm using a Win7 64bit machine as well. Can you try if it makes any difference if you use ssh login with public keys instead of passwords? You can also try to add LogLevel Debug to ~/.ssh/config in order to get more verbose ssh 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
Re: [PATCH] Remove duplicate #include
Дилян Палаузов git-...@aegee.org writes: deheader (git://gitorious.org/deheader/deheader.git) found out that some .c files #include twice one and the same header file. This patch removes such occurrences and hence speeds up the compilation. Does it speed up? By how much? Any numbers? I do not see any reason to reject this change. Removing repeated inclusions of the same header is a good thing by itself [*1*]. Thanks. [Footnote] *1* If things break when repeated inclusions are removed, that would mean the headers were wrong in the first place. I do not think transport.h, string-list.h, quote.h and cache.h have any reason why they need to be included twice to work correctly, and in fact they are designed to be no-op when included twice. Signed-off-by: Дилян Палаузов git-...@aegee.org --- builtin/fetch.c| 1 - trailer.c | 1 - transport-helper.c | 1 - userdiff.c | 1 - 4 files changed, 4 deletions(-) diff --git a/builtin/fetch.c b/builtin/fetch.c index 7b84d35..75a55e5 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -11,7 +11,6 @@ #include run-command.h #include parse-options.h #include sigchain.h -#include transport.h #include submodule.h #include connected.h #include argv-array.h diff --git a/trailer.c b/trailer.c index 623adeb..05b3859 100644 --- a/trailer.c +++ b/trailer.c @@ -1,7 +1,6 @@ #include cache.h #include string-list.h #include run-command.h -#include string-list.h #include commit.h #include trailer.h /* diff --git a/transport-helper.c b/transport-helper.c index 0224687..3652b16 100644 --- a/transport-helper.c +++ b/transport-helper.c @@ -5,7 +5,6 @@ #include commit.h #include diff.h #include revision.h -#include quote.h #include remote.h #include string-list.h #include thread-utils.h diff --git a/userdiff.c b/userdiff.c index fad52d6..2ccbee5 100644 --- a/userdiff.c +++ b/userdiff.c @@ -1,6 +1,5 @@ #include cache.h #include userdiff.h -#include cache.h #include attr.h static struct userdiff_driver *drivers; -- To unsubscribe from this list: send the line 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] hex.c: reduce memory footprint of sha1_to_hex static buffers
41 bytes is the exact number of bytes needed for having the returned
hex string represented. 50 seems to be an arbitrary number, such
that there are no benefits from alignment to certain address boundaries.
Signed-off-by: Stefan Beller sbel...@google.com
---
hex.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hex.c b/hex.c
index 9ebc050..cfd9d72 100644
--- a/hex.c
+++ b/hex.c
@@ -59,7 +59,7 @@ int get_sha1_hex(const char *hex, unsigned char *sha1)
char *sha1_to_hex(const unsigned char *sha1)
{
static int bufno;
- static char hexbuffer[4][50];
+ static char hexbuffer[4][41];
static const char hex[] = 0123456789abcdef;
char *buffer = hexbuffer[3 ++bufno], *buf = buffer;
int i;
--
2.3.0.81.gc37f363
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/2] Getopt::Long workaround in send-email
On Fri, Feb 13, 2015 at 12:19:27PM -0800, Junio C Hamano wrote: The first one is a replay of Kyle's workaround for older versions of Getopt::Long that did not take --no-option to negate a boolean option --option. The second one revert the workarounds made to the test script over time, and should break if the first one does not work well for older Getopt::Long (I have no reason to suspect it would break, though). I am inclined to squash these into one commit before starting to merge them down to 'next' and then to 'master', after getting Tested-by: from those with older Getopt::Long (prior to 2.32). Obviously, tc/t9001-noxmailer topic will become unnecessary and be dropped when that happens. I think this is a good fix. It preserves the documented behavior even on less capable systems. -- 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] hex.c: reduce memory footprint of sha1_to_hex static buffers
Stefan Beller sbel...@google.com writes: On Fri, Feb 13, 2015 at 1:41 PM, Junio C Hamano gits...@pobox.com wrote: Stefan Beller sbel...@google.com writes: 41 bytes is the exact number of bytes needed for having the returned hex string represented. 50 seems to be an arbitrary number, such that there are no benefits from alignment to certain address boundaries. Yes, with s/seems to be/is/; This comes from e83c5163 (Initial revision of git, the information manager from hell, 2005-04-07), and when dcb3450f (sha1_to_hex() usage cleanup, 2006-05-03) introduced the 4 recycled buffers on top, the underlying array was left at 50 bytes long. You can now have I fixed Linus's bug badge ;-) I don't think it's a bug, it's just wasting memory? Yes and no ;-) As I already said above, 50 is just an arbitrary number that is round and enough to hold 40 bytes with trailing NUL, and the waste does not lead to behaviour that is different from what was intended, of course, so it would not crash. However, the wastage was bothersome enough to make you send a patch, so you can call it a bug. It was wasting readers time ;-) -- To unsubscribe from this list: send the line 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: t5570 trap use in start/stop_git_daemon
On 2015/02/13 3:58AM Joachim Schmitz wrote:
Jeff King peff at peff.net writes:
On Fri, Feb 13, 2015 at 02:44:03AM -0500, Jeff King wrote:
On Thu, Feb 12, 2015 at 03:31:12PM -0500, Randall S. Becker wrote:
snip
Hmm, today I learned something new about ksh. Apparently when you use
the function keyword to define a function like:
function foo {
trap 'echo trapped' EXIT
}
echo before
foo
echo after
then the trap runs when the function exits! If you declare the same
function as:
foo() {
trap 'echo trapped' EXIT
}
it behaves differently. POSIX shell does not have the function keyword,
of course, and we are not using it here. Bash _does_ have the function
keyword, but seems to behave POSIX-y even when it is present. I.e.,
running the first script:
$ ksh foo.sh
before
trapped
after
$ bash foo.sh
before
after
trapped
snip
Both versions produce your first output on our platform
$ ksh foo1.sh
before
trapped
after
$ bash foo1.sh
before
after
trapped
$ ksh foo2.sh
before
trapped
after
$ bash foo2.sh
before
after
trapped
$
This might have been one (or even _the_) reason why we picked bash as our
SHELL_PATH in config.mak.uname (I don't remember, it's more than 2 years
ago), not sure which shell Randall's test used?
I tested both for trying to get t5570 to work. No matter which, without
resetting the trap, function return would kill the git-daemon and the test
would fail.
--
To unsubscribe from this list: send the line 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 8/8] reflog_expire(): lock symbolic refs themselves, not their referent
On 02/13/2015 10:53 PM, Junio C Hamano wrote: Michael Haggerty mhag...@alum.mit.edu writes: Now back to the real world. Currently, if R is changed *through* a symbolic reference S, then the reflogs for both R and S are updated, but not the reflogs for any other symbolic references that might point at R. If R is changed directly, then no symref's reflogs are affected, except for the special case that HEAD's reflog is changed if it points directly at R. This limitation is a hack to avoid having to walk symrefs backwards to find any symrefs that might be pointing at R. Yup. It might actually not be extremely expensive to follow symrefs backwards. Symbolic references cannot be packed, so we would only have to scan the loose references; we could ignore packed refs. But it would still be a lot more expensive than just updating one file. I don't know that it's worth it, given that symbolic references are used so sparingly. I personally do not think it is worth it. I further think that it would be perfectly OK to do one of the following: - We only maintain reflogs for $GIT_DIR/HEAD; no other symrefs get their own reflog, and we only check $GIT_DIR/HEAD when updating refs/heads/* and no other refs for direct reference (i.e. HEAD - refs/something/else - refs/heads/master symref chain is ignored). - In addition to the above, we also maintain reflogs for $GIT_DIR/refs/remotes/*/HEAD but support only when they directly point into a remote tracking branch in the same hierarchy. $GIT_DIR/refs/remotes/foo/HEAD that points at $GIT_DIR/refs/remotes/bar/master is ignored and will get an undefined behaviour. Yes. The first is approximately the status quo, except that you would like explicitly to *suppress* creating reflogs for symbolic refs other than HEAD even if a reference is altered via the symbolic ref. The second makes sense, though I think reflogs for remote HEADs are far less useful than those for HEAD. So I think this is a low-priority project. I think that the rule about locks as expressed above can be carried over the the real world: We should hold the locks on exactly those references (symbolic or regular) whose reflogs we plan to change. We should acquire all of the locks before making any changes. Sure. I forgot to mention that if we want to retain lock-compatibility with older clients, then we *also* need to lock the reference pointed at by a symbolic ref when modifying the symbolic ref's reflog. This is often implied by the previous rule, but not when we reseat a symbolic reference or when we expire a symbolic reference's reflog. I will look at how hard this is to implement. If it is at all involved, then I might drop this patch from the current patch series and defer it to another one. 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: Bad object pointed under refs/head/
On 02/13/2015 11:06 AM, Zheng Zhang wrote: I was running some test with Git 1.8.4.5, then I accidentally met a problem that leaded to the following error, error: refs/heads/develop does not point to a valid object! Turns out that the sha in refs/heads/develop is a bad object id, this happened after merging a branch X to branch develop, but packed-refs is updated to a corrected sha. No other merges at that point. The fix is easy, just removed refs/heads/develop. So there were two sha created, one is updated to refs/heads/develop, and the other one which is corrected, updated to packed-refs, Weird. I am wondering if there is a way to prevent this happening? Is this an ancient bug? If you can find and document a way to reproduce this problem, then you will have a much better chance of finding somebody who is willing to look into it. Otherwise it's hard to know how to even get started. 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
