Re: Git compile warnings (under mac/clang)
Jeff King p...@peff.net writes:
diff --git a/fsck.c b/fsck.c
index 15cb8bd..8f8c82f 100644
--- a/fsck.c
+++ b/fsck.c
@@ -107,7 +107,7 @@ static int fsck_msg_severity(enum fsck_msg_id msg_id,
{
int severity;
-if (options-msg_severity msg_id = 0 msg_id FSCK_MSG_MAX)
+if (options-msg_severity ((unsigned int) msg_id) FSCK_MSG_MAX)
severity = options-msg_severity[msg_id];
else {
severity = msg_id_info[msg_id].severity;
-- snap --
What do you think? Michael, does this cause more Clang warnings,
or would it resolve the issue?
Hmm, yeah, that does not seem unreasonable, and is more localized.
Or we could force enum to be signed by defining FSCK_MSG_UNUSED to
be -1 at the very beginning of enum definition, without changing
anything else. Then msg_id 0 would become a very valid
protection against programming mistakes, no?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/7] Coding style fixes.
On Fri, Jan 23, 2015 at 4:09 AM, Alexander Kuleshov kuleshovm...@gmail.com wrote: I made separate patch for every file. Please, let me know if need to squash it into one commit. 2015-01-23 17:06 GMT+06:00 Alexander Kuleshov kuleshovm...@gmail.com: This patch set contatins minor style fixes. CodingGuidelines contains rule that the star must side with variable name. The whole series is Reviewed-by: Stefan Beller sbel...@google.com -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Git compile warnings (under mac/clang)
Hi Peff,
On 2015-01-23 19:37, Jeff King wrote:
On Fri, Jan 23, 2015 at 10:07:18AM -0800, Junio C Hamano wrote:
[...] one thing that puzzles me is that the current code looks
like:
if (options-msg_severity msg_id = 0 msg_id FSCK_MSG_MAX)
severity = options-msg_severity[msg_id];
else {
severity = msg_id_info[msg_id].severity;
...
}
So if the severity override list given by options exists, _and_ if we
are in the enum range, then we use that. Otherwise, we dereference the
global list. But wouldn't an out-of-range condition have the exact same
problem dereferencing that global list?
IOW, should this really be:
if (msg_id 0 || msg_id = FSCK_MSG_MAX)
die(BUG: broken enum);
if (options-msg_severity)
severity = options-msg_severity[msg_id];
else
severity = msg_id_info[msg_id].severity;
? And then you can spell that first part as assert(), which I suspect
(but did not test) may shut up clang's warnings.
To be quite honest, I assumed that Git's source code was assert()-free. But I
was wrong! So I'll go with that solution; it is by far the nicest one IMHO.
Thanks!
Dscho
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] diff: make -M -C mean the same as -C -M
Mike Hommey m...@glandium.org writes:
While -C implies -M, it is quite common to see both on example command lines
here and there. The unintuitive thing is that if -M appears after -C, then
copy detection is turned off because of how the command line arguments are
handled.
This is deliberate, see below.
Change this so that when both -C and -M appear, whatever their order, copy
detection is on.
Signed-off-by: Mike Hommey m...@glandium.org
---
Interestingly, I even found mentions of -C -M in this order for benchmarks,
on this very list (see 6555655.XSJ9EnW4BY@mako).
Aside from the general warning against taking everything you see on
this list as true, I think you are looking at an orange and talking
about an apple. $gmane/244581 is about git blame, and -M/-C
over there mean completely different things.
Imagine you have a file a/b/c/d/e/f/g/h/, where each alphabet
represents a line with more meaningful contents than a single-liner,
and slash represents an end of line, and you have changed the
contents of the file to e/f/g/h/a/b/c/d/.
blame -M is to tell the command to notice that you moved the first
four lines to the end (i.e. you did not do anything original and
just moved lines). Because this needs more processing to notice,
the feature is triggered by a -M option (you would see something
like e/f/g/h/ came from the original and then a/b/c/d/ are newly
added without the option). -M in blame is about showing such
movement of lines within the same file [*1*].
On the other hand -C in blame is about noticing contents that
were copied from other files.
In the context of git blame, -C and -M control orthogonal
concepts and it makes sense to use only one but not the other, or
both.
But -M given to git diff is not about such an orthogonal
concept.
Even though its source candidates are narrower than that of -C in
that -M chooses only from the set of files that disappeared while
-C also chooses from files that remain after the change, they are
both about which file did the whole thing come from?, and it makes
sense to have progression of -M -C -C -C. You can think
of these as a short-hand for --rename-copy-level={0,1,2,3} option
(where the level 0 -- do not do anything -- corresponds to giving no
-M/-C). It can be argued both ways: either we take the maximum of
the values given to --rename-copy-level options (which corresponds
to what your patch attempts to do), or the last one wins. We happen
to have implemented the latter long time ago and that is how
existing users expect things to work.
So, I dunno. I am saying that the semantics the current code gives
is *not* the only possibly correct one, and I am not saying that
your alternative interpretation is *wrong*, but I am not sure if it
makes sense to switch the semantics this late in the game.
[Footnote]
*1* diff cannot do anything magical about such a change. It can
only say that you removed the first four lines from the original,
and then added new four lines at the end (or it may say you added
new four lines at the beginning and removed four lines at the end).
There is no way for diff to say that these new four lines have any
relation to what you removed from the beginning of the file, with
any combination of options. This is inherent in its output format,
which is limited to express only deletions and additions.
diff.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/diff.c b/diff.c
index d1bd534..9081fd8 100644
--- a/diff.c
+++ b/diff.c
@@ -3670,7 +3670,8 @@ int diff_opt_parse(struct diff_options *options, const
char **av, int ac)
!strcmp(arg, --find-renames)) {
if ((options-rename_score = diff_scoreopt_parse(arg)) == -1)
return error(invalid argument to -M: %s, arg+2);
- options-detect_rename = DIFF_DETECT_RENAME;
+ if (options-detect_rename != DIFF_DETECT_COPY)
+ options-detect_rename = DIFF_DETECT_RENAME;
}
else if (!strcmp(arg, -D) || !strcmp(arg, --irreversible-delete)) {
options-irreversible_delete = 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: [PATCH] Documentation: what does git log --indexed-objects even mean?
On Fri, Jan 23, 2015 at 11:49:05AM -0800, Junio C Hamano wrote: 4fe10219 (rev-list: add --indexed-objects option, 2014-10-16) adds --indexed-objects option to rev-list, and it is only useful in the context of git rev-list and not git log. There are other object traversal options that do not make sense for git log that are shown in the manual page. Move the description of --indexed-objects to the object traversal section so that it sits together with its friends --objects, --objects-edge, etc. and then show them only in git rev-list documentation. Yeah, that makes sense to me. Acked-by: Jeff King p...@peff.net -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 compile warnings (under mac/clang)
Hi Peff,
On 2015-01-23 19:55, Jeff King wrote:
On Fri, Jan 23, 2015 at 07:46:36PM +0100, Johannes Schindelin wrote:
? And then you can spell that first part as assert(), which I suspect
(but did not test) may shut up clang's warnings.
To be quite honest, I assumed that Git's source code was
assert()-free. But I was wrong! So I'll go with that solution; it is
by far the nicest one IMHO.
OK, here it is as a patch on top of js/fsck-opt. Please feel free to
squash rather than leaving it separate.
I tested with clang-3.6, and it seems to make the warning go away.
-- 8 --
Subject: [PATCH] fsck_msg_severity: range-check enum with assert()
An enum is passed into the function, which we use to index a
fixed-size array. We double-check that the enum is within
range as a defensive measure. However, there are two
problems with this:
1. If it's not in range, we then use it to index another
array of the same size. Which will have the same problem.
We should probably die instead, as this condition
should not ever happen.
2. The bottom end of our range check is tautological
according to clang, which generates a warning. Clang is
not _wrong_, but the point is that we are trying to be
defensive against something that should never happen
(i.e., somebody abusing the enum).
We can solve both by switching to a separate assert().
Signed-off-by: Jeff King p...@peff.net
---
fsck.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/fsck.c b/fsck.c
index 15cb8bd..53c0849 100644
--- a/fsck.c
+++ b/fsck.c
@@ -107,7 +107,9 @@ static int fsck_msg_severity(enum fsck_msg_id msg_id,
{
int severity;
- if (options-msg_severity msg_id = 0 msg_id FSCK_MSG_MAX)
+ assert(msg_id = 0 msg_id FSCK_MSG_MAX);
+
+ if (options-msg_severity)
severity = options-msg_severity[msg_id];
else {
severity = msg_id_info[msg_id].severity;
I also ended up with that solution!
Thanks!
Dscho
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Git compile warnings (under mac/clang)
On Fri, Jan 23, 2015 at 10:07:18AM -0800, Junio C Hamano wrote:
diff --git a/fsck.c b/fsck.c
index 15cb8bd..8f8c82f 100644
--- a/fsck.c
+++ b/fsck.c
@@ -107,7 +107,7 @@ static int fsck_msg_severity(enum fsck_msg_id msg_id,
{
int severity;
- if (options-msg_severity msg_id = 0 msg_id FSCK_MSG_MAX)
+ if (options-msg_severity ((unsigned int) msg_id) FSCK_MSG_MAX)
severity = options-msg_severity[msg_id];
else {
severity = msg_id_info[msg_id].severity;
-- snap --
What do you think? Michael, does this cause more Clang warnings,
or would it resolve the issue?
Hmm, yeah, that does not seem unreasonable, and is more localized.
Or we could force enum to be signed by defining FSCK_MSG_UNUSED to
be -1 at the very beginning of enum definition, without changing
anything else. Then msg_id 0 would become a very valid
protection against programming mistakes, no?
Yeah, I think that would work, too. It is a little unfortunate in the
sense that it actually makes things _worse_ from the perspective of the
type system. That is, in the current code if you assume that everyone
else has followed the type rules, then an fsck_msg_id you get definitely
is indexable into various arrays. But if you add in a sentinel value,
now you (in theory) have to check for the sentinel value everywhere.
I'm not sure if that matters in practice, though, if you are going to be
defensive against people misusing the enum system in the first place
(e.g., you are worried about them passing a random int and having it
produce a segfault, you have to do range checks either way).
But of all the options outlined, I think I'd much rather just see an
assert() for something that should never happen, rather than mixing it
into the logic.
In that vein, one thing that puzzles me is that the current code looks
like:
if (options-msg_severity msg_id = 0 msg_id FSCK_MSG_MAX)
severity = options-msg_severity[msg_id];
else {
severity = msg_id_info[msg_id].severity;
...
}
So if the severity override list given by options exists, _and_ if we
are in the enum range, then we use that. Otherwise, we dereference the
global list. But wouldn't an out-of-range condition have the exact same
problem dereferencing that global list?
IOW, should this really be:
if (msg_id 0 || msg_id = FSCK_MSG_MAX)
die(BUG: broken enum);
if (options-msg_severity)
severity = options-msg_severity[msg_id];
else
severity = msg_id_info[msg_id].severity;
? And then you can spell that first part as assert(), which I suspect
(but did not test) may shut up clang's warnings.
-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 compile warnings (under mac/clang)
Jeff King p...@peff.net writes:
But of all the options outlined, I think I'd much rather just see an
assert() for something that should never happen, rather than mixing it
into the logic.
Surely.
In that vein, one thing that puzzles me is that the current code looks
like:
if (options-msg_severity msg_id = 0 msg_id FSCK_MSG_MAX)
severity = options-msg_severity[msg_id];
else {
severity = msg_id_info[msg_id].severity;
...
}
So if the severity override list given by options exists, _and_ if we
are in the enum range, then we use that. Otherwise, we dereference the
global list. But wouldn't an out-of-range condition have the exact same
problem dereferencing that global list?
IOW, should this really be:
if (msg_id 0 || msg_id = FSCK_MSG_MAX)
die(BUG: broken enum);
if (options-msg_severity)
severity = options-msg_severity[msg_id];
else
severity = msg_id_info[msg_id].severity;
? And then you can spell that first part as assert(), which I suspect
(but did not test) may shut up clang's warnings.
Sounds like a sensible fix to me.
--
To unsubscribe from this list: send the line unsubscribe 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 compile warnings (under mac/clang)
On Fri, Jan 23, 2015 at 07:46:36PM +0100, Johannes Schindelin wrote:
? And then you can spell that first part as assert(), which I suspect
(but did not test) may shut up clang's warnings.
To be quite honest, I assumed that Git's source code was
assert()-free. But I was wrong! So I'll go with that solution; it is
by far the nicest one IMHO.
OK, here it is as a patch on top of js/fsck-opt. Please feel free to
squash rather than leaving it separate.
I tested with clang-3.6, and it seems to make the warning go away.
-- 8 --
Subject: [PATCH] fsck_msg_severity: range-check enum with assert()
An enum is passed into the function, which we use to index a
fixed-size array. We double-check that the enum is within
range as a defensive measure. However, there are two
problems with this:
1. If it's not in range, we then use it to index another
array of the same size. Which will have the same problem.
We should probably die instead, as this condition
should not ever happen.
2. The bottom end of our range check is tautological
according to clang, which generates a warning. Clang is
not _wrong_, but the point is that we are trying to be
defensive against something that should never happen
(i.e., somebody abusing the enum).
We can solve both by switching to a separate assert().
Signed-off-by: Jeff King p...@peff.net
---
fsck.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/fsck.c b/fsck.c
index 15cb8bd..53c0849 100644
--- a/fsck.c
+++ b/fsck.c
@@ -107,7 +107,9 @@ static int fsck_msg_severity(enum fsck_msg_id msg_id,
{
int severity;
- if (options-msg_severity msg_id = 0 msg_id FSCK_MSG_MAX)
+ assert(msg_id = 0 msg_id FSCK_MSG_MAX);
+
+ if (options-msg_severity)
severity = options-msg_severity[msg_id];
else {
severity = msg_id_info[msg_id].severity;
--
2.3.0.rc1.287.g761fd19
--
To unsubscribe from this list: send the line unsubscribe 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 pull not ignoring the file which has been sent to the temporary ignore list
On Friday, January 23, 2015 11:31:40 AM you wrote: Arup Rakshit arupraks...@rocketmail.com writes: I asked git not to track any changes to the file .gitignore. To do so I did use the command - git update-index --assume-unchanged .gitignore. You are not asking Git to do anything. You promised Git that you will make no changes to .gitignore, and then broke that promise. Assume-unchanged is *not* Ignore changes to this path. Ok. How should I then ignore any local changes to the .gitignore file ? And while taking pull, git should skip this file ? -- Regards, Arup Rakshit Debugging is twice as hard as writing the code in the first place. Therefore, if you write the code as cleverly as possible, you are, by definition, not smart enough to debug it. --Brian Kernighan -- To unsubscribe from this list: send the line 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] Documentation: what does git log --indexed-objects even mean?
4fe10219 (rev-list: add --indexed-objects option, 2014-10-16) adds --indexed-objects option to rev-list, and it is only useful in the context of git rev-list and not git log. There are other object traversal options that do not make sense for git log that are shown in the manual page. Move the description of --indexed-objects to the object traversal section so that it sits together with its friends --objects, --objects-edge, etc. and then show them only in git rev-list documentation. Signed-off-by: Junio C Hamano gits...@pobox.com --- Documentation/rev-list-options.txt | 12 +++- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt index 2984f40..97ef2e8 100644 --- a/Documentation/rev-list-options.txt +++ b/Documentation/rev-list-options.txt @@ -172,11 +172,6 @@ explicitly. Pretend as if all objects mentioned by reflogs are listed on the command line as `commit`. ---indexed-objects:: - Pretend as if all trees and blobs used by the index are listed - on the command line. Note that you probably want to use - `--objects`, too. - --ignore-missing:: Upon seeing an invalid object name in the input, pretend as if the bad input was not given. @@ -644,6 +639,7 @@ Object Traversal These options are mostly targeted for packing of Git repositories. +ifdef::git-rev-list[] --objects:: Print the object IDs of any object referenced by the listed commits. `--objects foo ^bar` thus means ``send me @@ -662,9 +658,15 @@ These options are mostly targeted for packing of Git repositories. commits at the cost of increased time. This is used instead of `--objects-edge` to build ``thin'' packs for shallow repositories. +--indexed-objects:: + Pretend as if all trees and blobs used by the index are listed + on the command line. Note that you probably want to use + `--objects`, too. + --unpacked:: Only useful with `--objects`; print the object IDs that are not in packs. +endif::git-rev-list[] --no-walk[=(sorted|unsorted)]:: Only show the given commits, but do not traverse their ancestors. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv3 0/6] Fix bug in large transactions
version3:
patches 1,2,3 stayed completely as is, while patches 4,5 are new, patch 6 is
rewritten to first write the contents of the lock files before closing them.
This combines the series Enable large transactions v2 as sent out yesterday
with the follow up series [RFC PATCH 0/5] So you dislike the sequence of
system calls?
There is no write_in_full_to_lock_file wrapper any more, but write_ref_sha1
was reduced in functionality in patch 5.
This applies on top of origin/sb/atomic-push and results in a merge conflict
when merging it to origin/jk/lock-ref-sha1-basic-return-errors which looks
like
$git checkout origin/jk/lock-ref-sha1-basic-return-errors
$git merge enable_large_transactions
CONFLICT (content): Merge conflict in refs.c
$git diff
++ HEAD
+ lock-lock_fd = hold_lock_file_for_update(lock-lk, ref_file, lflags);
+ if (lock-lock_fd 0) {
++===
+ if (hold_lock_file_for_update(lock-lk, ref_file, lflags) 0) {
++ enable_large_transactions
which is best resolved as:
@@@ -2316,8 -2333,7 +2333,12 @@@ static struct ref_lock *lock_ref_sha1_b
goto error_return;
}
+if (hold_lock_file_for_update(lock-lk, ref_file, lflags) 0) {
last_errno = errno;
if (errno == ENOENT --attempts_remaining 0)
/*
version2:
* This applies on top of origin/sb/atomic-push though it will result in a one
line merge conflict with origin/jk/lock-ref-sha1-basic-return-errors when
merging to origin/next.
* It now uses the FILE* pointer instead of file descriptors. This
results in a combination of the 2 former patches refs.c: have
a write_in_full_to_lock_file wrapper and refs.c: write to a
lock file only once as the wrapper function is more adapted to
its consumers
* no need to dance around with char *pointers which may leak.
* another new patch sneaked into the series: Renaming ULIMIT in t7004
to ULIMIT_STACK_SIZE
That said, only the first and third patch are updated from the first version
of the patches. The others are new in the sense that rewriting them was cheaper
than keeping notes in between.
version1:
(reported as: git update-ref --stdin : too many open files, 2014-12-20)
First a test case is introduced to demonstrate the failure,
the patches 2-6 are little refactoring and the last patch
fixes the bug and also marks the bugs as resolved in the
test suite.
Unfortunately this applies on top of origin/next.
Any feedback would be welcome!
Thanks,
Stefan
Stefan Beller (6):
update-ref: test handling large transactions properly
t7004: rename ULIMIT test prerequisite to ULIMIT_STACK_SIZE
refs.c: remove lock_fd from struct ref_lock
refs.c: move static functions to close and commit refs
refs.c: remove unlock_ref and commit_ref from write_ref_sha1
refs.c: enable large transactions
refs.c| 93 +++
t/t1400-update-ref.sh | 28
t/t7004-tag.sh| 4 +--
3 files changed, 79 insertions(+), 46 deletions(-)
--
2.2.1.62.g3f15098
--
To unsubscribe from this list: send the line 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 pull not ignoring the file which has been sent to the temporary ignore list
Hi, I asked git not to track any changes to the file .gitignore. To do so I did use the command - git update-index --assume-unchanged .gitignore. [arup@sztukajedzenia]$ git status # On branch MajorUpgrade # Your branch is behind 'origin/MajorUpgrade' by 4 commits, and can be fast-forwarded. # (use git pull to update your local branch) # nothing to commit, working directory clean [arup@sztukajedzenia]$ git pull origin MajorUpgrade From rubyxcube.co.uk:sztuka-jedzenia/sztukajedzenia * branchMajorUpgrade - FETCH_HEAD Updating 59a1c07..d7b9cd3 error: Your local changes to the following files would be overwritten by merge: .gitignore Please, commit your changes or stash them before you can merge. Aborting [arup@sztukajedzenia]$ But you can see above, while I am taking `pull`, it is considering the file .gitignore. How should I tell `git pull` also not to consider the change the file, and skip it or something else ? -- Regards, Arup Rakshit Debugging is twice as hard as writing the code in the first place. Therefore, if you write the code as cleverly as possible, you are, by definition, not smart enough to debug it. --Brian Kernighan -- To unsubscribe from this list: send the line unsubscribe 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 pull not ignoring the file which has been sent to the temporary ignore list
Arup Rakshit arupraks...@rocketmail.com writes: I asked git not to track any changes to the file .gitignore. To do so I did use the command - git update-index --assume-unchanged .gitignore. You are not asking Git to do anything. You promised Git that you will make no changes to .gitignore, and then broke that promise. Assume-unchanged is *not* Ignore changes to this path. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv3 1/6] update-ref: test handling large transactions properly
Signed-off-by: Stefan Beller sbel...@google.com
Signed-off-by: Junio C Hamano gits...@pobox.com
---
Notes:
v2-v3:
no changes
t/t1400-update-ref.sh | 28
1 file changed, 28 insertions(+)
diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
index 7b4707b..47d2fe9 100755
--- a/t/t1400-update-ref.sh
+++ b/t/t1400-update-ref.sh
@@ -973,4 +973,32 @@ test_expect_success 'stdin -z delete refs works with
packed and loose refs' '
test_must_fail git rev-parse --verify -q $c
'
+run_with_limited_open_files () {
+ (ulimit -n 32 $@)
+}
+
+test_lazy_prereq ULIMIT_FILE_DESCRIPTORS 'run_with_limited_open_files true'
+
+test_expect_failure ULIMIT_FILE_DESCRIPTORS 'large transaction creating
branches does not burst open file limit' '
+(
+ for i in $(test_seq 33)
+ do
+ echo create refs/heads/$i HEAD
+ done large_input
+ run_with_limited_open_files git update-ref --stdin large_input
+ git rev-parse --verify -q refs/heads/33
+)
+'
+
+test_expect_failure ULIMIT_FILE_DESCRIPTORS 'large transaction deleting
branches does not burst open file limit' '
+(
+ for i in $(test_seq 33)
+ do
+ echo delete refs/heads/$i HEAD
+ done large_input
+ run_with_limited_open_files git update-ref --stdin large_input
+ test_must_fail git rev-parse --verify -q refs/heads/33
+)
+'
+
test_done
--
2.2.1.62.g3f15098
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv3 6/6] refs.c: enable large transactions
By closing the file descriptors after creating the lock file we are not
limiting the size of the transaction by the number of available file
descriptors.
When closing the file descriptors early, we also need to write the values
in early, if we don't want to reopen the files.
Signed-off-by: Stefan Beller sbel...@google.com
---
Notes:
version3:
* Do not reopen the files after closing them. Make sure we have
written all necessary information before closing the file.
Doing it that way enabled us to drop the patch [PATCH 4/6]
refs.c: Have a write_in_full_to_lock_file wrapping write_in_full
refs.c| 15 ---
t/t1400-update-ref.sh | 4 ++--
2 files changed, 14 insertions(+), 5 deletions(-)
diff --git a/refs.c b/refs.c
index 1bfc84b..2594b23 100644
--- a/refs.c
+++ b/refs.c
@@ -3752,6 +3752,17 @@ int ref_transaction_commit(struct ref_transaction
*transaction,
update-refname);
goto cleanup;
}
+ if (!is_null_sha1(update-new_sha1)) {
+ if (write_ref_sha1(update-lock, update-new_sha1,
+ update-msg)) {
+ strbuf_addf(err, Cannot write to the ref lock
'%s'.,
+ update-refname);
+ ret = TRANSACTION_GENERIC_ERROR;
+ goto cleanup;
+ }
+ }
+ /* Do not keep all lock files open at the same time. */
+ close_ref(update-lock);
}
/* Perform updates first so live commits remain referenced */
@@ -3759,9 +3770,7 @@ int ref_transaction_commit(struct ref_transaction
*transaction,
struct ref_update *update = updates[i];
if (!is_null_sha1(update-new_sha1)) {
- if (write_ref_sha1(update-lock, update-new_sha1,
- update-msg)
- || commit_ref(update-lock, update-new_sha1)) {
+ if (commit_ref(update-lock, update-new_sha1)) {
strbuf_addf(err, Cannot update the ref '%s'.,
update-refname);
ret = TRANSACTION_GENERIC_ERROR;
diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
index 47d2fe9..c593a1d 100755
--- a/t/t1400-update-ref.sh
+++ b/t/t1400-update-ref.sh
@@ -979,7 +979,7 @@ run_with_limited_open_files () {
test_lazy_prereq ULIMIT_FILE_DESCRIPTORS 'run_with_limited_open_files true'
-test_expect_failure ULIMIT_FILE_DESCRIPTORS 'large transaction creating
branches does not burst open file limit' '
+test_expect_success ULIMIT_FILE_DESCRIPTORS 'large transaction creating
branches does not burst open file limit' '
(
for i in $(test_seq 33)
do
@@ -990,7 +990,7 @@ test_expect_failure ULIMIT_FILE_DESCRIPTORS 'large
transaction creating branches
)
'
-test_expect_failure ULIMIT_FILE_DESCRIPTORS 'large transaction deleting
branches does not burst open file limit' '
+test_expect_success ULIMIT_FILE_DESCRIPTORS 'large transaction deleting
branches does not burst open file limit' '
(
for i in $(test_seq 33)
do
--
2.2.1.62.g3f15098
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv3 4/6] refs.c: move static functions to close and commit refs
By moving the functions up we don't need to have to declare them first
when using them in a later patch.
Signed-off-by: Stefan Beller sbel...@google.com
---
Notes:
new in v3
refs.c | 28 ++--
1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/refs.c b/refs.c
index 4066752..f1eefc7 100644
--- a/refs.c
+++ b/refs.c
@@ -2808,6 +2808,20 @@ static int rename_ref_available(const char *oldname,
const char *newname)
static int write_ref_sha1(struct ref_lock *lock, const unsigned char *sha1,
const char *logmsg);
+static int close_ref(struct ref_lock *lock)
+{
+ if (close_lock_file(lock-lk))
+ return -1;
+ return 0;
+}
+
+static int commit_ref(struct ref_lock *lock)
+{
+ if (commit_lock_file(lock-lk))
+ return -1;
+ return 0;
+}
+
int rename_ref(const char *oldrefname, const char *newrefname, const char
*logmsg)
{
unsigned char sha1[20], orig_sha1[20];
@@ -2898,20 +2912,6 @@ int rename_ref(const char *oldrefname, const char
*newrefname, const char *logms
return 1;
}
-static int close_ref(struct ref_lock *lock)
-{
- if (close_lock_file(lock-lk))
- return -1;
- return 0;
-}
-
-static int commit_ref(struct ref_lock *lock)
-{
- if (commit_lock_file(lock-lk))
- return -1;
- return 0;
-}
-
/*
* copy the reflog message msg to buf, which has been allocated sufficiently
* large, while cleaning up the whitespaces. Especially, convert LF to space,
--
2.2.1.62.g3f15098
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv3 5/6] refs.c: remove unlock_ref and commit_ref from write_ref_sha1
This makes write_ref_sha1 only write the the lock file, committing
needs to be done outside of that function. This will help us change
the ref_transaction_commit in a later patch.
Also instead of calling unlock_ref before each return in write_ref_sha1
we can call this after the call. This is a first step to split up
write_ref_sha1 into the write and commit phase which is done in a
later patch.
There is a call in each code path after write_ref_sha1 now. Even in
the last hunk in the error case, the 'goto cleanup;' will make sure
there is a call to unlock_ref.
Signed-off-by: Stefan Beller sbel...@google.com
---
Notes:
new in v3
refs.c | 40
1 file changed, 20 insertions(+), 20 deletions(-)
diff --git a/refs.c b/refs.c
index f1eefc7..1bfc84b 100644
--- a/refs.c
+++ b/refs.c
@@ -2815,8 +2815,11 @@ static int close_ref(struct ref_lock *lock)
return 0;
}
-static int commit_ref(struct ref_lock *lock)
+static int commit_ref(struct ref_lock *lock, const unsigned char *sha1)
{
+ if (!lock-force_write !hashcmp(lock-old_sha1, sha1))
+ return 0;
+
if (commit_lock_file(lock-lk))
return -1;
return 0;
@@ -2879,10 +2882,13 @@ int rename_ref(const char *oldrefname, const char
*newrefname, const char *logms
}
lock-force_write = 1;
hashcpy(lock-old_sha1, orig_sha1);
- if (write_ref_sha1(lock, orig_sha1, logmsg)) {
+ if (write_ref_sha1(lock, orig_sha1, logmsg)
+ || commit_ref(lock, orig_sha1)) {
+ unlock_ref(lock);
error(unable to write current sha1 into %s, newrefname);
goto rollback;
}
+ unlock_ref(lock);
return 0;
@@ -2896,8 +2902,11 @@ int rename_ref(const char *oldrefname, const char
*newrefname, const char *logms
lock-force_write = 1;
flag = log_all_ref_updates;
log_all_ref_updates = 0;
- if (write_ref_sha1(lock, orig_sha1, NULL))
+ if (write_ref_sha1(lock, orig_sha1, NULL)
+ || commit_ref(lock, orig_sha1))
error(unable to write current sha1 into %s, oldrefname);
+
+ unlock_ref(lock);
log_all_ref_updates = flag;
rollbacklog:
@@ -3067,22 +3076,19 @@ static int write_ref_sha1(struct ref_lock *lock,
errno = EINVAL;
return -1;
}
- if (!lock-force_write !hashcmp(lock-old_sha1, sha1)) {
- unlock_ref(lock);
+ if (!lock-force_write !hashcmp(lock-old_sha1, sha1))
return 0;
- }
+
o = parse_object(sha1);
if (!o) {
error(Trying to write ref %s with nonexistent object %s,
lock-ref_name, sha1_to_hex(sha1));
- unlock_ref(lock);
errno = EINVAL;
return -1;
}
if (o-type != OBJ_COMMIT is_branch(lock-ref_name)) {
error(Trying to write non-commit object %s to branch %s,
sha1_to_hex(sha1), lock-ref_name);
- unlock_ref(lock);
errno = EINVAL;
return -1;
}
@@ -3091,7 +3097,6 @@ static int write_ref_sha1(struct ref_lock *lock,
close_ref(lock) 0) {
int save_errno = errno;
error(Couldn't write %s, lock-lk-filename.buf);
- unlock_ref(lock);
errno = save_errno;
return -1;
}
@@ -3099,7 +3104,6 @@ static int write_ref_sha1(struct ref_lock *lock,
if (log_ref_write(lock-ref_name, lock-old_sha1, sha1, logmsg) 0 ||
(strcmp(lock-ref_name, lock-orig_ref_name)
log_ref_write(lock-orig_ref_name, lock-old_sha1, sha1, logmsg)
0)) {
- unlock_ref(lock);
return -1;
}
if (strcmp(lock-orig_ref_name, HEAD) != 0) {
@@ -3124,12 +3128,6 @@ static int write_ref_sha1(struct ref_lock *lock,
!strcmp(head_ref, lock-ref_name))
log_ref_write(HEAD, lock-old_sha1, sha1, logmsg);
}
- if (commit_ref(lock)) {
- error(Couldn't set %s, lock-ref_name);
- unlock_ref(lock);
- return -1;
- }
- unlock_ref(lock);
return 0;
}
@@ -3762,14 +3760,15 @@ int ref_transaction_commit(struct ref_transaction
*transaction,
if (!is_null_sha1(update-new_sha1)) {
if (write_ref_sha1(update-lock, update-new_sha1,
- update-msg)) {
- update-lock = NULL; /* freed by write_ref_sha1
*/
+ update-msg)
+ || commit_ref(update-lock, update-new_sha1)) {
strbuf_addf(err, Cannot update the ref '%s'.,
update-refname);
ret =
[PATCHv3 2/6] t7004: rename ULIMIT test prerequisite to ULIMIT_STACK_SIZE
During creation of the patch series our discussion we could have a
more descriptive name for the prerequisite for the test so it stays
unique when other limits of ulimit are introduced.
Signed-off-by: Stefan Beller sbel...@google.com
Signed-off-by: Junio C Hamano gits...@pobox.com
---
Notes:
v2-v3:
no changes
t/t7004-tag.sh | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index 796e9f7..06b8e0d 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -1463,10 +1463,10 @@ run_with_limited_stack () {
(ulimit -s 128 $@)
}
-test_lazy_prereq ULIMIT 'run_with_limited_stack true'
+test_lazy_prereq ULIMIT_STACK_SIZE 'run_with_limited_stack true'
# we require ulimit, this excludes Windows
-test_expect_success ULIMIT '--contains works in a deep repo' '
+test_expect_success ULIMIT_STACK_SIZE '--contains works in a deep repo' '
expect
i=1
while test $i -lt 8000
--
2.2.1.62.g3f15098
--
To unsubscribe from this list: send the line unsubscribe 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 pull not ignoring the file which has been sent to the temporary ignore list
On Fri, Jan 23, 2015 at 10:35 AM, Arup Rakshit arupraks...@rocketmail.com wrote: On Friday, January 23, 2015 11:31:40 AM you wrote: Arup Rakshit arupraks...@rocketmail.com writes: I asked git not to track any changes to the file .gitignore. To do so I did use the command - git update-index --assume-unchanged .gitignore. You are not asking Git to do anything. You promised Git that you will make no changes to .gitignore, and then broke that promise. Assume-unchanged is *not* Ignore changes to this path. Ok. How should I then ignore any local changes to the .gitignore file ? And while taking pull, git should skip this file ? Look at .git/info/exclude When looking for a reference to that path (I am bad at remembering which man page that is) I found https://help.github.com/articles/ignoring-files/ as Googles first hit, which advises to use git update-index --assume-unchanged path/to/file.txt Not sure if that is most helpful advice there. See http://git-scm.com/docs/gitignore instead -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv3 3/6] refs.c: remove lock_fd from struct ref_lock
The 'lock_fd' is the same as 'lk-fd'. No need to store it twice so remove
it. You may argue this introduces more coupling as we need to know more
about the internals of the lock file mechanism, but this will be solved in
a later patch.
No functional changes intended.
Signed-off-by: Stefan Beller sbel...@google.com
Signed-off-by: Junio C Hamano gits...@pobox.com
---
Notes:
v2-v3:
no changes
refs.c | 16 ++--
1 file changed, 6 insertions(+), 10 deletions(-)
diff --git a/refs.c b/refs.c
index 14e52ca..4066752 100644
--- a/refs.c
+++ b/refs.c
@@ -11,7 +11,6 @@ struct ref_lock {
char *orig_ref_name;
struct lock_file *lk;
unsigned char old_sha1[20];
- int lock_fd;
int force_write;
};
@@ -2259,7 +2258,6 @@ static struct ref_lock *lock_ref_sha1_basic(const char
*refname,
int attempts_remaining = 3;
lock = xcalloc(1, sizeof(struct ref_lock));
- lock-lock_fd = -1;
if (mustexist)
resolve_flags |= RESOLVE_REF_READING;
@@ -2335,8 +2333,8 @@ static struct ref_lock *lock_ref_sha1_basic(const char
*refname,
goto error_return;
}
- lock-lock_fd = hold_lock_file_for_update(lock-lk, ref_file, lflags);
- if (lock-lock_fd 0) {
+ if (hold_lock_file_for_update(lock-lk, ref_file, lflags) 0) {
+ last_errno = errno;
if (errno == ENOENT --attempts_remaining 0)
/*
* Maybe somebody just deleted one of the
@@ -2904,7 +2902,6 @@ static int close_ref(struct ref_lock *lock)
{
if (close_lock_file(lock-lk))
return -1;
- lock-lock_fd = -1;
return 0;
}
@@ -2912,7 +2909,6 @@ static int commit_ref(struct ref_lock *lock)
{
if (commit_lock_file(lock-lk))
return -1;
- lock-lock_fd = -1;
return 0;
}
@@ -3090,8 +3086,8 @@ static int write_ref_sha1(struct ref_lock *lock,
errno = EINVAL;
return -1;
}
- if (write_in_full(lock-lock_fd, sha1_to_hex(sha1), 40) != 40 ||
- write_in_full(lock-lock_fd, term, 1) != 1 ||
+ if (write_in_full(lock-lk-fd, sha1_to_hex(sha1), 40) != 40 ||
+ write_in_full(lock-lk-fd, term, 1) != 1 ||
close_ref(lock) 0) {
int save_errno = errno;
error(Couldn't write %s, lock-lk-filename.buf);
@@ -4047,9 +4043,9 @@ int reflog_expire(const char *refname, const unsigned
char *sha1,
status |= error(couldn't write %s: %s, log_file,
strerror(errno));
} else if ((flags EXPIRE_REFLOGS_UPDATE_REF)
- (write_in_full(lock-lock_fd,
+ (write_in_full(lock-lk-fd,
sha1_to_hex(cb.last_kept_sha1), 40) != 40 ||
-write_str_in_full(lock-lock_fd, \n) != 1 ||
+write_str_in_full(lock-lk-fd, \n) != 1 ||
close_ref(lock) 0)) {
status |= error(couldn't write %s,
lock-lk-filename.buf);
--
2.2.1.62.g3f15098
--
To unsubscribe from this list: send the line unsubscribe 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] diff: make -M -C mean the same as -C -M
On Fri, Jan 23, 2015 at 10:41:10AM -0800, Junio C Hamano wrote:
Mike Hommey m...@glandium.org writes:
While -C implies -M, it is quite common to see both on example command lines
here and there. The unintuitive thing is that if -M appears after -C, then
copy detection is turned off because of how the command line arguments are
handled.
This is deliberate, see below.
Change this so that when both -C and -M appear, whatever their order, copy
detection is on.
Signed-off-by: Mike Hommey m...@glandium.org
---
Interestingly, I even found mentions of -C -M in this order for benchmarks,
on this very list (see 6555655.XSJ9EnW4BY@mako).
Aside from the general warning against taking everything you see on
this list as true
The problem is not whether what is on the list is true or not, but that
people will use what they see.
I think you are looking at an orange and talking
about an apple. $gmane/244581 is about git blame, and -M/-C
over there mean completely different things.
I thought blame was using the diff option parser, and I was wrong.
(snip)
In the context of git blame, -C and -M control orthogonal
concepts and it makes sense to use only one but not the other, or
both.
In the context of blame both -C and -M |= a flags set, so one doesn't
override the other. You can place them in any order, the result will be
the same. Incidentally, -C includes the flag -M sets, so -C -M is
actually redundant. What -C and -M can be used for is set different
scores (-C9 -M8).
But -M given to git diff is not about such an orthogonal
concept.
Even though its source candidates are narrower than that of -C in
that -M chooses only from the set of files that disappeared while
-C also chooses from files that remain after the change, they are
both about which file did the whole thing come from?, and it makes
sense to have progression of -M -C -C -C. You can think
of these as a short-hand for --rename-copy-level={0,1,2,3} option
(where the level 0 -- do not do anything -- corresponds to giving no
-M/-C). It can be argued both ways: either we take the maximum of
the values given to --rename-copy-level options (which corresponds
to what your patch attempts to do), or the last one wins. We happen
to have implemented the latter long time ago and that is how
existing users expect things to work.
Are they? I, for one, wasn't. It is easy to understand that --foo=1
--foo=2 is going to take the last given --foo, but do people really
expect the last of -C -M to be used? Reading the code further, I can see
that it's even more confusing than that: -C -C wins over -M, wherever it
happens. So -C -C -M == -C -C ; -C -M == -M ; -M -C == -C.
At the very least, this should be spelled out in the documentation.
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 pull not ignoring the file which has been sent to the temporary ignore list
On Fri, Jan 23, 2015 at 1:14 PM, Junio C Hamano gits...@pobox.com wrote: Good answer for .gitignore. In general, you do not ignore local changes to tracked paths. I assumed Arup would want to ignore more than is in the upstream project, so you'd come up with an appendix to the .gitignore file because that file is rather obvious to find (it's printed when git pull modifies it, 'ls' just find it, you'd not look into .git/info/exclude by chance) Assuming you want to ignore less than the upstream project (delete some lines from .gitignore) it get's tricky in my opinion. Either have a local commit and just use 'git pull' to resolve the conflicts with upstream. The problem then arises if you want to publish your changes (such as pushing your changes and creating a pull request, then you have that commit included, which you maybe don't want to include) Mind, that I am talking about possible work flows to circumvent an assumed problem which would result in problems as described in the first mail. -- To unsubscribe from this list: send the line unsubscribe 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 submodule first time update with proxy
Hi, On Fri, Jan 23, 2015 at 3:50 PM, Robert Dailey rcdailey.li...@gmail.com wrote: I have a submodule using HTTP URL. I do this: $ git submodule init MySubmodule $ git submodule update MySubmodule The 2nd command fails because the HTTP URL cannot be resolved, this is because it requires a proxy. I have http.proxy setup properly in the .git/config of my parent git repository, so I was hoping the submodule update function would have a way to specify it to inherit the proxy value from the parent config. Your not the first to suggest it and you probably won't be the last. It is hard to decide _which_ config variables, if any, should propagate from the parent. What works for one use-case may not necessarily work for another. How can I set up my submodule? Probably the easiest thing would be to make your http.proxy configuration global i.e. $ git config --global http.proxy If you don't want to make it a global setting you can setup the submodule configuration after running init but before running update i.e. $ git submodule init MySubmodule $ (cd MySubmodule git config http.proxy ...) $ git submodule update MySubmodule -- To unsubscribe from this list: send the line unsubscribe 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 pull not ignoring the file which has been sent to the temporary ignore list
Stefan Beller sbel...@google.com writes: Assuming you want to ignore less than the upstream project (delete some lines from .gitignore) it get's tricky in my opinion. Why? Doesn't info/exclude allow negative ignore patterns? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Should copy/rename detection consider file overwrites?
On Fri, Jan 23, 2015 at 06:04:19AM -0500, Jeff King wrote: On Fri, Jan 23, 2015 at 10:29:08AM +0900, Mike Hommey wrote: While fooling around with copy/rename detection, I noticed that it doesn't detect the case where you copy or rename a file on top of another: $ git init $ (echo foo; echo bar) foo If I replace this with a longer input, like: cp /usr/share/dict/words foo $ git add foo $ git commit -m foo $ echo 0 bar $ git add bar $ git commit -m bar $ git mv -f foo bar $ git commit -m foobar $ git log --oneline --reverse 7dc2765 foo b0c837d bar 88caeba foobar $ git blame -s -C -C bar 88caebab 1) foo 88caebab 2) bar Then the blame shows me the initial foo commit. So I think it is mainly that your toy example is too small (I think we will do exact rename detection whatever the size is, but I expect we are getting hung up on the break detection between 0\n and foo\nbar\n). Err, I was afraid my testcase was too small. And that all boils down to this: num is optional but it is the lower bound on the number of alphanumeric characters that Git must detect as moving/copying between files for it to associate those lines with the parent commit. And the default value is 40. I can see how this is not trivially representable in e.g. git diff-tree, but shouldn't at least blame try to tell that those lines actually come from 7dc2765? diff-tree can show this, too, but you need to turn on break detection which will notice that bar has essentially been rewritten (and then consider its sides as candidates for rename detection). For example (with the longer input, as above): $ git diff-tree --name-status -M HEAD c6fe146b0c73adcbc4dbc2e58eb83af9007678bc M bar D foo $ git diff-tree --name-status -M -B HEAD c6fe146b0c73adcbc4dbc2e58eb83af9007678bc R100foo bar Presumably if you set the break score low enough, your original example would behave the same way, but I couldn't get it to work (I didn't look closely, but I imagine it is just so tiny that we hit the internal limits on how low you can set the score).o Oh. Good to know, thanks. 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 pull not ignoring the file which has been sent to the temporary ignore list
On Fri, Jan 23, 2015 at 2:26 PM, Junio C Hamano gits...@pobox.com wrote: Stefan Beller sbel...@google.com writes: Assuming you want to ignore less than the upstream project (delete some lines from .gitignore) it get's tricky in my opinion. Why? Doesn't info/exclude allow negative ignore patterns? I used negative patterns only once, so they did not come to my mind today. Apologies for not looking it up before replying. :( -- To unsubscribe from this list: send the line unsubscribe 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 pull not ignoring the file which has been sent to the temporary ignore list
Stefan Beller sbel...@google.com writes: Ok. How should I then ignore any local changes to the .gitignore file ? And while taking pull, git should skip this file ? Look at .git/info/exclude Good answer for .gitignore. In general, you do not ignore local changes to tracked paths. I found https://help.github.com/articles/ignoring-files/ as Googles first hit, which advises to use git update-index --assume-unchanged path/to/file.txt Not sure if that is most helpful advice there. The piece of advice in the last paragraph on that page is wrong (and it has been wrong from the day it was written). The gitignore(5) documentation used to have a similar incorrect piece of advice but we finally corrected it recently. Cf. http://thread.gmane.org/gmane.comp.version-control.git/260954/focus=261118 -- To unsubscribe from this list: send the line unsubscribe 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: [PATCH] t/lib-httpd: switch SANITY check for NOT_ROOT
On 2015-01-22 23.07, Junio C Hamano wrote:
Torsten Bögershausen tbo...@web.de writes:
If I run that sequence manually:
chmod 755 .
touch x
chmod a-w .
rm x
touch y
x is gone, (but shoudn't according to POSIX)
y is not created, access denied
Good (or is that Sad?).
diff --git a/t/test-lib.sh b/t/test-lib.sh
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -1039,7 +1039,17 @@ test_lazy_prereq NOT_ROOT '
# When the tests are run as root, permission tests will report that
# things are writable when they shouldn't be.
test_lazy_prereq SANITY '
- test_have_prereq POSIXPERM,NOT_ROOT
+ mkdir ds
+ touch ds/x
+ chmod -w ds
+ if rm ds/x
+ then
+ chmod +w ds
+ false
+ else
+ chmod +w ds
+ true
+ fi
'
It looks like a better approach overall.
Because we cannot know where $(pwd) is when lazy prereq is evaluated
(it typically is at the root of the trash hierarchy, but not always)
and we would not want to add, leave or remove random files in the
working tree that are not expected by the tests proper (e.g. a test
that counts untracked paths are not expecting ds/ to be there), your
actual fix may need to be a bit more careful, though.
Thanks.
Hm,
I think there are 2 different possiblities to go further,
either to always switch off SANITY for CYGWIN (or Windows in general).
I haven't tested anything, the idea came up while writing this email.
The other way is to go away from the hard coded we know we are root,
so SANITY must be false, or we know that Windows is not 100% POSIX,
and probe the OS/FS dynamically.
The following probably deserves the price for the most clumsy prerequisite
ever written.
(CopyPaste of a real patch into the mailer, not sure if it applies)
It has been tested under Mac OS, root@Mac OS, Cygwin / Msysgit
What do you think ?
-- 8 --
Subject: [PATCH 1/2] test-lib.sh: Improve SANITY
SANITY was not set when running as root,
but this is not 100% reliable for CYGWIN:
A file is allowed to be deleted when the containing
directory does not have write permissions.
Signed-off-by: Torsten Bögershausen tbo...@web.de
---
t/test-lib.sh | 20 +++-
1 file changed, 19 insertions(+), 1 deletion(-)
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 93f7cad..b8f736f 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -1038,8 +1038,26 @@ test_lazy_prereq NOT_ROOT '
# When the tests are run as root, permission tests will report that
# things are writable when they shouldn't be.
+# Special check for CYGWIN (or Windows in general):
+# A file can be deleted, even if the containing directory does'nt
+# have write permissions
test_lazy_prereq SANITY '
- test_have_prereq POSIXPERM,NOT_ROOT
+ dsdir=$$ds
+ mkdir $dsdir
+ touch $dsdir/x
+ chmod -w $dsdir
+ if rm $dsdir/x
+ then
+ chmod +w $dsdir
+ rm -rf $dsdir
+ echo 2 SANITY=false
+ false
+ else
+ chmod +w $dsdir
+ rm -rf $dsdir
+ echo 2 SANITY=true
+ true
+ fi
'
GIT_UNZIP=${GIT_UNZIP:-unzip}
--
Subject: [PATCH 2/2] t2026 needs SANITY
When running as root 'prune directories with unreadable gitdir' in t2026 fails.
Protect this TC with SANITY
Signed-off-by: Torsten Bögershausen tbo...@web.de
---
t/t2026-prune-linked-checkouts.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/t/t2026-prune-linked-checkouts.sh
b/t/t2026-prune-linked-checkouts.sh
index 170aefe..2936d52 100755
--- a/t/t2026-prune-linked-checkouts.sh
+++ b/t/t2026-prune-linked-checkouts.sh
@@ -33,7 +33,7 @@ EOF
! test -d .git/worktrees
'
-test_expect_success POSIXPERM 'prune directories with unreadable gitdir' '
+test_expect_success SANITY 'prune directories with unreadable gitdir' '
mkdir -p .git/worktrees/def/abc
: .git/worktrees/def/def
: .git/worktrees/def/gitdir
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-p4: correct --prepare-p4-only instructions
l...@diamand.org wrote on Fri, 23 Jan 2015 09:15 +: If you use git-p4 with the --prepare-p4-only option, then it prints the p4 command line to use. However, the command line was incorrect: the changelist specification must be supplied on standard input, not as an argument to p4. Signed-off-by: Luke Diamand l...@diamand.org --- git-p4.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/git-p4.py b/git-p4.py index ff132b2..90447de 100755 --- a/git-p4.py +++ b/git-p4.py @@ -1442,7 +1442,7 @@ class P4Submit(Command, P4UserMap): print+ self.clientPath print print To submit, use \p4 submit\ to write a new description, -print or \p4 submit -i %s\ to use the one prepared by \ +print or \p4 submit -i %s\ to use the one prepared by \ \git p4\. % fileName print You can delete the file \%s\ when finished. % fileName Looks obviously good here. Ack! -- Pete -- To unsubscribe from this list: send the line unsubscribe 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-p4 maintainership change
Pete Wyckoff p...@padd.com writes: Hi Junio. I'm fortunate enough to need no longer any git integration with Perforce (p4). I work only in git these days. Thus you might expect my interest in improving git-p4 would be waning. Luke, on the other hand, continues to need git-p4 and is active in improving it. I think you should consider accepting patches in that area from him directly. He's contributed many patches over the years and has helped users to debug their issues too. I'll certainly be available to comment on any dodgy code in there already, and can help with archeological, but will not likely do any substantive work to git-p4 in the near future. Thanks for your help during all these years, and thanks Luke for taking the area maintainership. -- To unsubscribe from this list: send the line 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-p4 maintainership change
Hi Junio. I'm fortunate enough to need no longer any git integration with Perforce (p4). I work only in git these days. Thus you might expect my interest in improving git-p4 would be waning. Luke, on the other hand, continues to need git-p4 and is active in improving it. I think you should consider accepting patches in that area from him directly. He's contributed many patches over the years and has helped users to debug their issues too. I'll certainly be available to comment on any dodgy code in there already, and can help with archeological, but will not likely do any substantive work to git-p4 in the near future. Luke: I think you have a couple patches outstanding; hope these can get merged to mainline soon. Thanks, -- Pete -- To unsubscribe from this list: send the line unsubscribe 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 --recurse-submodule does not recurse to sub-submodules (etc.)
Thanks, Jens. Incidentally, git submodule update --init --recursive Does exactly what expected – it updates sub/sub/submodules, so there is certainly some inconsistency in how the --recursive flag is handled here. i...@maxheld.de | http://www.maxheld.de | http://www.civicon.de | Mobil: +49 151 22958775 | Skype: maximilian.held Bremen International Graduate School of Social Sciences | Wiener Straße / Celsiusstraße | 28359 Bremen | Germany On Tue, Jan 20, 2015 at 10:21 PM, Jens Lehmann jens.lehm...@web.de wrote: Am 19.01.2015 um 21:19 schrieb Maximilian Held: I have a directory with nested submodules, such as: supermodule/submodule/sub-submodule/sub-sub-submodule When I cd to supermodule and do: git push --recurse-submodule=check (or on-demand), git only pushes the submodule, but not the sub-submodule etc. Maybe this is expected behavior and not a bug, but I thought it was pretty unintuitive. I expected that git would push, well, recursively. I agree this is unexpected and should be fixed. I suspect the fix would be to teach the push_submodule() function to use the same flags that were used for the push in the superproject. -- To unsubscribe from this list: send the line unsubscribe 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] diff: make -M -C mean the same as -C -M
Mike Hommey m...@glandium.org writes: In the context of git blame, -C and -M control orthogonal concepts and it makes sense to use only one but not the other, or both. In the context of blame both -C and -M |= a flags set, so one doesn't override the other. You can place them in any order, the result will be the same. Incidentally, -C includes the flag -M sets, so -C -M is actually redundant. What -C and -M can be used for is set different scores (-C9 -M8). Yes. That is what I meant by orthogonal and makese sense to use only one but not the other, or both. As they are not related with each other, it makes sense to mix them freely, unlike -C/-M given to diff. -- To unsubscribe from this list: send the line unsubscribe 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: [PATCH] t/lib-httpd: switch SANITY check for NOT_ROOT
Torsten Bögershausen tbo...@web.de writes:
It has been tested under Mac OS, root@Mac OS, Cygwin / Msysgit
What do you think ?
Except that we may want to be more careful to detect errors from the
initial mkdir and clean-up part (which should abort the test, not
just declare !SANITY), I think the basic idea is sound.
test_dir=$TRASH_DIRECTORY/.sanity-test-dir
! mkdir $test_dir
$test_dir/x
chmod -w $test_dir ||
error bug in test sript: cannot prepare .sanity-test-dir
rm $test_dir/x
status=$?
chmod +w $test_dir
rm -r $test_dir ||
error bug in test sript: cannot clean .sanity-test-dir
return $status
or something along that line?
-- 8 --
Subject: [PATCH 1/2] test-lib.sh: Improve SANITY
SANITY was not set when running as root,
but this is not 100% reliable for CYGWIN:
A file is allowed to be deleted when the containing
directory does not have write permissions.
Signed-off-by: Torsten Bögershausen tbo...@web.de
---
t/test-lib.sh | 20 +++-
1 file changed, 19 insertions(+), 1 deletion(-)
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 93f7cad..b8f736f 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -1038,8 +1038,26 @@ test_lazy_prereq NOT_ROOT '
# When the tests are run as root, permission tests will report that
# things are writable when they shouldn't be.
+# Special check for CYGWIN (or Windows in general):
+# A file can be deleted, even if the containing directory does'nt
+# have write permissions
test_lazy_prereq SANITY '
- test_have_prereq POSIXPERM,NOT_ROOT
+ dsdir=$$ds
+ mkdir $dsdir
+ touch $dsdir/x
+ chmod -w $dsdir
+ if rm $dsdir/x
+ then
+ chmod +w $dsdir
+ rm -rf $dsdir
+ echo 2 SANITY=false
+ false
+ else
+ chmod +w $dsdir
+ rm -rf $dsdir
+ echo 2 SANITY=true
+ true
+ fi
'
GIT_UNZIP=${GIT_UNZIP:-unzip}
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv3 6/6] refs.c: enable large transactions
Stefan Beller sbel...@google.com writes: By closing the file descriptors after creating the lock file we are not limiting the size of the transaction by the number of available file descriptors. When closing the file descriptors early, we also need to write the values in early, if we don't want to reopen the files. I am not sure if an early return in write_ref_sha1() is entirely correct. The unlock step was split out of write and commit functions in the previous step because you eventually want to be able to close the file descriptor that is open to $ref.lock early, while still keeping the $ref.lock file around to avoid others competing with you, so that you can limit the number of open file descriptors, no? If so, shouldn't the write function at least close the file descriptor even when it knows that the $ref.lock already has the correct object name? The call to close_ref() is never made when the early-return codepath is taken as far as I can see. Puzzled... -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv3 5/6] refs.c: remove unlock_ref and commit_ref from write_ref_sha1
On Fri, Jan 23, 2015 at 4:39 PM, Junio C Hamano gits...@pobox.com wrote:
Stefan Beller sbel...@google.com writes:
On Fri, Jan 23, 2015 at 3:57 PM, Junio C Hamano gits...@pobox.com wrote:
Stefan Beller sbel...@google.com writes:
-static int commit_ref(struct ref_lock *lock)
+static int commit_ref(struct ref_lock *lock, const unsigned char *sha1)
{
+ if (!lock-force_write !hashcmp(lock-old_sha1, sha1))
+ return 0;
if (commit_lock_file(lock-lk))
return -1;
return 0;
@@ -2879,10 +2882,13 @@ int rename_ref(const char *oldrefname, const char
*newrefname, const char *logms
}
lock-force_write = 1;
hashcpy(lock-old_sha1, orig_sha1);
- if (write_ref_sha1(lock, orig_sha1, logmsg)) {
+ if (write_ref_sha1(lock, orig_sha1, logmsg)
+ || commit_ref(lock, orig_sha1)) {
+ unlock_ref(lock);
This is not a new problem, but the two lines in pre-context of this
patch look strange.
Which (not new) problem are you talking about here? Do you have
a reference?
These two lines in pre-context of the hunk:
lock-force_write = 1;
hashcpy(lock-old_sha1, orig_sha1);
So these 2 lines (specially the force_write=1 line) is just there to trigger
the valid early exit path as you sent in the other mail :
if (!lock-force_write !hashcmp(lock-old_sha1, sha1))
return 0;
when we have the same sha1?
and you're saying that's a problem because hard to understand?
I am confused as well by now.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[ANNOUNCE] Git Merge, April 8-9, Paris
GitHub is organizing a Git-related conference to be held April 8-9, 2015, in Paris. Details here: http://git-merge.com/ The exact schedule is still being worked out, but there is going to be some dedicated time/space for Git (and libgit2 and JGit) developers to meet and talk to each other. If you have patches in Git, I'd encourage you to consider attending. If travel finances are a problem, please talk to me. GitHub may be able to defray the cost of travel. I hope to see people there! -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] git-new-workdir: support submodules
Ping! (now that the holidays are past) craig On Tue, Dec 23, 2014 at 1:51 PM, Craig Silverstein csilv...@khanacademy.org wrote: [Ack, I forgot to cc myself on the original patch so now I can't reply to it normally. Hopefully my workaround doesn't mess up the threading too badly.] Junio C Hamano gitster at pobox.com writes: H, does that mean that the submodule S in the original repository O's working tree and its checkout in the secondary working tree W created from O using git-new-workdir share the same repository location? More specifically: O/.git/ - original repository O/.git/index- worktree state in O O/S - submodule S's checkout in O O/S/.git- a gitfile pointing to O/.git/modules/S O/.git/modules/S- submodule S's repository contents O/.git/modules/S/config - submodule S's config W/.git/ - secondary working tree W/.git/config - symlink to O/.git/config W/.git/index- worktree state in W (independent of O) W/S - submodule S's checkout in W (independent of O) W/S/.git- a gitfile pointing to O/.git/modules/S Right until the last line. The .git file holds a relative path (at least, it always does in my experience), so W/S/.git will point to W/.git/modules/S. Also, to complete the story, my changes sets the following: W/.git/modules/S- secondary working tree for S W/.git/modules/S/config - symlink to O/.git/modules/S/config W/.git/modules/S/index- worktree state in W's S (independent of O and O's S) Doesn't a submodule checkout keep some state tied to the working tree in its repository configuration file? Do you mean, in 'config' itself? If so, I don't see it. (Though it's possible there are ways to use submodules that do keep working-tree state in the config file, and we just happen not to use those ways.) Here's what my webapp/.git/modules/khan-exercises/config looks like: --- [core] repositoryformatversion = 0 filemode = true bare = false logallrefupdates = true worktree = ../../../khan-exercises [remote origin] url = http://github.com/Khan/khan-exercises.git fetch = +refs/heads/*:refs/remotes/origin/* [branch master] remote = origin merge = refs/heads/master rebase = true [submodule test/qunit] url = https://github.com/jquery/qunit.git -- The only thing that seems vaguely working-tree related is the 'worktree' field, which is the field that motivated me to set up my patch the way it is. Wouldn't this change introduce problems by sharing O/.git/modules/S/config between the two checkouts? It's true that this change does result in sharing that file, so if that's problematic then you're right. I'm afraid I don't know all the things that can go into a submodule config file. (There are other things I don't know as well, such as: do we see .git files with 'gitdir: ...' contents only for submodules, or are there other ways to create them as well? Are 'gitdir' paths always relative? Are there special files in .git (or rather .git/modules/S) that exist only for submodules and not for 'normal' repos, that we need to worry about symlinking? I apologize for not knowing all these git internals, and hope you guys can help point out any gaps that affect this patch!) craig -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-new-workdir: support submodules
Craig Silverstein csilv...@khanacademy.org writes: Doesn't a submodule checkout keep some state tied to the working tree in its repository configuration file? Do you mean, in 'config' itself? If so, I don't see it. (Though it's possible there are ways to use submodules that do keep working-tree state in the config file, and we just happen not to use those ways.) Here's what my webapp/.git/modules/khan-exercises/config looks like: --- [core] repositoryformatversion = 0 filemode = true bare = false logallrefupdates = true worktree = ../../../khan-exercises [remote origin] url = http://github.com/Khan/khan-exercises.git fetch = +refs/heads/*:refs/remotes/origin/* [branch master] remote = origin merge = refs/heads/master rebase = true [submodule test/qunit] url = https://github.com/jquery/qunit.git -- The only thing that seems vaguely working-tree related is the 'worktree' field, which is the field that motivated me to set up my patch the way it is. That is the location of the working tree of the top-level superproject. Tied to the state of the submodule working tree appear in [submodule test/qunit] part. In one new-workdir checkout, that submodule may be submodule inited, while another one, it may not be. Or one new-workdir checkout's branch may check out a top-level project from today while the other one may have a top-level project from two years ago, and between these two checkouts of the top-level project, the settings of submodule.test/qunit.* variables may have to be different (perhaps even URL may have to point at two different repositories, one historical one to grab the state two years ago, the other current one). So sharing config between top-level checkouts may not be enough to support submodules (the patch title). -- To unsubscribe from this list: send the line unsubscribe 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 20/24] update-index: test the system before enabling untracked cache
On Fri, Jan 23, 2015 at 1:49 AM, Junio C Hamano gits...@pobox.com wrote: I am not (yet) enthused by the intrusiveness of the overall series, though. I think the gain justifies the series' complexity. Although I don't mind redoing the whole series if we find a better way. -- 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: [PATCHv3 5/6] refs.c: remove unlock_ref and commit_ref from write_ref_sha1
Stefan Beller sbel...@google.com writes:
-static int commit_ref(struct ref_lock *lock)
+static int commit_ref(struct ref_lock *lock, const unsigned char *sha1)
{
+ if (!lock-force_write !hashcmp(lock-old_sha1, sha1))
+ return 0;
if (commit_lock_file(lock-lk))
return -1;
return 0;
@@ -2879,10 +2882,13 @@ int rename_ref(const char *oldrefname, const char
*newrefname, const char *logms
}
lock-force_write = 1;
hashcpy(lock-old_sha1, orig_sha1);
- if (write_ref_sha1(lock, orig_sha1, logmsg)) {
+ if (write_ref_sha1(lock, orig_sha1, logmsg)
+ || commit_ref(lock, orig_sha1)) {
+ unlock_ref(lock);
This is not a new problem, but the two lines in pre-context of this
patch look strange. When the code is renaming into some ref, the
ref either would have no original SHA-1 (i.e. we are renaming to a
non-existing name) or have unrelated SHA-1 (i.e. we are overwriting
an existing one). For some (unknown to me) reason, however, the
code pretends that lock-old_sha1 has the new SHA-1 already before
we start to do the write or commit.
And because both write and commit tries to pretend to be no-op when
the caller tries to update a ref with the same SHA-1, but in this
codepath it does want the write to happen, it needs to set the
force_write bit set, which look like an unnecessary workaround.
Regardless of what this particular caller does, I am not sure if the
early-return codepath in commit_ref() is correct. From the callers'
point of view, it sometimes unlocks the ref (i.e. when a different
SHA-1 is written or force_write is set) and sometimes keeps the ref
locked (i.e. when early-return is taken). Shouldn't these two cases
behave identically? Or am I wrong to assume that the early return
using hashcmp(lock-old_sha1, sha1) is a mere optimization?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv3 5/6] refs.c: remove unlock_ref and commit_ref from write_ref_sha1
On Fri, Jan 23, 2015 at 3:57 PM, Junio C Hamano gits...@pobox.com wrote:
Stefan Beller sbel...@google.com writes:
-static int commit_ref(struct ref_lock *lock)
+static int commit_ref(struct ref_lock *lock, const unsigned char *sha1)
{
+ if (!lock-force_write !hashcmp(lock-old_sha1, sha1))
+ return 0;
if (commit_lock_file(lock-lk))
return -1;
return 0;
@@ -2879,10 +2882,13 @@ int rename_ref(const char *oldrefname, const char
*newrefname, const char *logms
}
lock-force_write = 1;
hashcpy(lock-old_sha1, orig_sha1);
- if (write_ref_sha1(lock, orig_sha1, logmsg)) {
+ if (write_ref_sha1(lock, orig_sha1, logmsg)
+ || commit_ref(lock, orig_sha1)) {
+ unlock_ref(lock);
This is not a new problem, but the two lines in pre-context of this
patch look strange.
Which (not new) problem are you talking about here? Do you have
a reference?
Regardless of what this particular caller does, I am not sure if the
early-return codepath in commit_ref() is correct. From the callers'
point of view, it sometimes unlocks the ref (i.e. when a different
SHA-1 is written or force_write is set) and sometimes keeps the ref
locked (i.e. when early-return is taken). Shouldn't these two cases
behave identically? Or am I wrong to assume that the early return
using hashcmp(lock-old_sha1, sha1) is a mere optimization?
I assumed it was not just an optimization as the test suite fails if I
touch that line. I'll look into cleaning it up in a more obvious fashion.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv3 6/6] refs.c: enable large transactions
On Fri, Jan 23, 2015 at 4:14 PM, Junio C Hamano gits...@pobox.com wrote: Stefan Beller sbel...@google.com writes: By closing the file descriptors after creating the lock file we are not limiting the size of the transaction by the number of available file descriptors. When closing the file descriptors early, we also need to write the values in early, if we don't want to reopen the files. I am not sure if an early return in write_ref_sha1() is entirely correct. The unlock step was split out of write and commit functions in the previous step because you eventually want to be able to close the file descriptor that is open to $ref.lock early, while still keeping the $ref.lock file around to avoid others competing with you, so that you can limit the number of open file descriptors, no? yeah that's the goal. Though as we're in one transaction, as soon as we have an early exit, the transaction will abort. If so, shouldn't the write function at least close the file descriptor even when it knows that the $ref.lock already has the correct object name? The call to close_ref() is never made when the early-return codepath is taken as far as I can see. The goto cleanup; will take care of unlocking (and closing fds of) all refs -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv3 5/6] refs.c: remove unlock_ref and commit_ref from write_ref_sha1
Stefan Beller sbel...@google.com writes:
On Fri, Jan 23, 2015 at 3:57 PM, Junio C Hamano gits...@pobox.com wrote:
Stefan Beller sbel...@google.com writes:
-static int commit_ref(struct ref_lock *lock)
+static int commit_ref(struct ref_lock *lock, const unsigned char *sha1)
{
+ if (!lock-force_write !hashcmp(lock-old_sha1, sha1))
+ return 0;
if (commit_lock_file(lock-lk))
return -1;
return 0;
@@ -2879,10 +2882,13 @@ int rename_ref(const char *oldrefname, const char
*newrefname, const char *logms
}
lock-force_write = 1;
hashcpy(lock-old_sha1, orig_sha1);
- if (write_ref_sha1(lock, orig_sha1, logmsg)) {
+ if (write_ref_sha1(lock, orig_sha1, logmsg)
+ || commit_ref(lock, orig_sha1)) {
+ unlock_ref(lock);
This is not a new problem, but the two lines in pre-context of this
patch look strange.
Which (not new) problem are you talking about here? Do you have
a reference?
These two lines in pre-context of the hunk:
lock-force_write = 1;
hashcpy(lock-old_sha1, orig_sha1);
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv3 6/6] refs.c: enable large transactions
Stefan Beller sbel...@google.com writes:
On Fri, Jan 23, 2015 at 4:14 PM, Junio C Hamano gits...@pobox.com wrote:
yeah that's the goal. Though as we're in one transaction, as soon
as we have an early exit, the transaction will abort.
An early exit I am talking about is this:
static int write_ref_sha1(struct ref_lock *lock,
const unsigned char *sha1, const char *logmsg)
{
static char term = '\n';
struct object *o;
if (!lock) {
errno = EINVAL;
return -1;
}
if (!lock-force_write !hashcmp(lock-old_sha1, sha1))
return 0;
It returns 0 and then the transaction side has this call in a loop:
if (!is_null_sha1(update-new_sha1)) {
if (write_ref_sha1(update-lock, update-new_sha1,
update-msg)) {
strbuf_addf(err, Cannot write to the ref lock
'%s'.,
update-refname);
ret = TRANSACTION_GENERIC_ERROR;
goto cleanup;
}
}
If so, shouldn't the write function at least close the file
descriptor even when it knows that the $ref.lock already has the
correct object name? The call to close_ref() is never made when the
early-return codepath is taken as far as I can see.
The goto cleanup; will take care of unlocking (and closing fds of) all refs
Yes, if write_ref_sha1() returns with non-zero signaling an error,
then the goto will trigger.
But if it short-cuts and returns zero, that goto will not be
reached.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv3 5/6] refs.c: remove unlock_ref and commit_ref from write_ref_sha1
Stefan Beller sbel...@google.com writes:
On Fri, Jan 23, 2015 at 4:39 PM, Junio C Hamano gits...@pobox.com wrote:
Stefan Beller sbel...@google.com writes:
On Fri, Jan 23, 2015 at 3:57 PM, Junio C Hamano gits...@pobox.com wrote:
Stefan Beller sbel...@google.com writes:
-static int commit_ref(struct ref_lock *lock)
+static int commit_ref(struct ref_lock *lock, const unsigned char *sha1)
{
+ if (!lock-force_write !hashcmp(lock-old_sha1, sha1))
+ return 0;
if (commit_lock_file(lock-lk))
return -1;
return 0;
@@ -2879,10 +2882,13 @@ int rename_ref(const char *oldrefname, const char
*newrefname, const char *logms
}
lock-force_write = 1;
hashcpy(lock-old_sha1, orig_sha1);
- if (write_ref_sha1(lock, orig_sha1, logmsg)) {
+ if (write_ref_sha1(lock, orig_sha1, logmsg)
+ || commit_ref(lock, orig_sha1)) {
+ unlock_ref(lock);
This is not a new problem, but the two lines in pre-context of this
patch look strange.
Which (not new) problem are you talking about here? Do you have
a reference?
These two lines in pre-context of the hunk:
lock-force_write = 1;
hashcpy(lock-old_sha1, orig_sha1);
So these 2 lines (specially the force_write=1 line) is just there to trigger
the valid early exit path as you sent in the other mail :
if (!lock-force_write !hashcmp(lock-old_sha1, sha1))
return 0;
when we have the same sha1?
and you're saying that's a problem because hard to understand?
What is the point of that hashcmp() in the first place? My
understanding is that
(1) lock-old_sha1 is to hold the original SHA-1 in the ref we took
the lock on.
(2) when not forcing, and when the two SHA-1 are the same, we
bypass and return early because overwriting the ref with the
same value is no-op.
Now, in that codepath, when the code is renaming into some ref, the
ref either would have no original SHA-1 (i.e. we are renaming to a
non-existing name) or have unrelated SHA-1 (i.e. we are overwriting
an existing one). For some (unknown to me) reason, however, the
code pretends that lock-old_sha1 has the new SHA-1 already before
we start to do the write or commit.
And because both write and commit tries to pretend to be no-op when
the caller tries to update a ref with the same SHA-1, but in this
codepath it does want the write to happen, it needs to set the
force_write bit set, which look like an unnecessary workaround.
Let me rephrase.
A natural way to write that caller, I think, would be more like
this:
... lock is given by the caller, probably with -old_sha1
... filled in; it is not likely to be orig_sha1, as it is
... either null (if locking to create a new ref) or some
... unrelated value read from the ref being overwritten by
... this rename_ref() operation.
write_ref_sha1(lock, orig_sha1, logmsg);
# which may do the don't write the same value optmization
# if we are renaming another ref that happens to have the
# same value, in which case it is OK. Otherwise this will
# overwrite without being forced.
... *IF* and only if there is some reason lock-old_sha1
... needs to match what is in the filesystem ref right now,
... then do
hashcpy(lock-old_sha1, orig_sha1);
... but probably there is no reason to do so.
The two lines hashcpy() and -force_write = 1 that appear before the
write_ref_sha1() do not conceptually make sense. The former does
not make sense because -old_sha1 is supposed to be the original
value that is already stored in the ref, to allow us optimize write
the same value case, and you are breaking that invariant by doing
hashcpy() before write_ref_sha1(). The lock-old_sha1 value does
not have anything to do with the (original) value of the ref we took
the lock on.
And -force_write = 1 becomes necessary only because the effect of
this nonsensical hashcpy() is to make the !hashcmp() used by the
short-cut logic trigger. Since the code needs to override that
logic, you need to say force before calling write_ref_sha1(). If
you didn't do the hashcpy(), you wouldn't have to say force, no?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/7] Coding style fixes.
I made separate patch for every file. Please, let me know if need to squash it into one commit. 2015-01-23 17:06 GMT+06:00 Alexander Kuleshov kuleshovm...@gmail.com: This patch set contatins minor style fixes. CodingGuidelines contains rule that the star must side with variable name. Alexander Kuleshov (7): show-branch: minor style fix clone: minor style fix test-hashmap: minor style fix http-backend: minor style fix refs: minor style fix quote: minor style fix fast-import: minor style fix builtin/clone.c | 2 +- builtin/show-branch.c | 2 +- fast-import.c | 2 +- http-backend.c| 2 +- quote.c | 2 +- refs.h| 2 +- test-hashmap.c| 4 ++-- 7 files changed, 8 insertions(+), 8 deletions(-) -- 2.3.0.rc1.275.g028c360 -- _ 0xAX -- To unsubscribe from this list: send the line unsubscribe 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 7/7] fast-import: minor style fix
On 2015-01-23 12.08, Alexander Kuleshov wrote: .. Asterisk must be next with variable .. But this is a function: -static char* make_fast_import_path(const char *path) +static char *make_fast_import_path(const char *path) (Sorry when I need to read this:) - Fixing style violations while working on a real change as a preparatory clean-up step is good, but otherwise avoid useless code churn for the sake of conforming to the style. Once it _is_ in the tree, it's not really worth the patch noise to go and fix it up. Cf. http://article.gmane.org/gmane.linux.kernel/943020 -- To unsubscribe from this list: send the line unsubscribe 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 compile warnings (under mac/clang)
On Fri, Jan 23, 2015 at 12:48:29PM +0100, Johannes Schindelin wrote:
This is what I have currently in the way of attempting to fix it (I
still believe that Clang is wrong to make this a warning, and causes
more trouble than it solves):
I agree. It is something we as the programmers cannot possibly know (the
compiler is free to decide which type however it likes) and its decision
does not impact the correctness of the code (the check is either useful
or tautological, and we cannot know which, so we are being warned about
being too careful!).
I guess you could argue that the standard defines enum-numbering to
start at 0, and increment by 1. Therefore we should know that no valid
enum value is less than 0. IOW, msg_id 0 being true must be the
result of a bug somewhere else in the program, where we assigned a value
outside of the enum range to the enum.
Pointed out by Michael Blume. Jeff King provided the pointer to a commit
fixing the same issue elsewhere in the Git source code.
It may be useful to reference the exact commit (3ce3ffb8) to help people
digging in the history (e.g., if we decide there is a better way to shut
up this warning and we need to find all the places to undo the
brain-damage).
- for (i = 0; i FSCK_MSG_MAX; i++) {
+ for (i = FSCK_MSG_MIN + 1; i FSCK_MSG_MAX; i++) {
Ugh. It is really a shame how covering up this warning requires
polluting so many places. I don't think we have a better way, though,
aside from telling people to use -Wno-tautological-compare (and I can
believe that it _is_ a useful warning in some other circumstances, so it
seems a shame to lose it).
Unless we are willing to drop the = 0 check completely. I think it is
valid to do so regardless of the compiler's representation decision due
to the numbering rules I mentioned above. It kind-of serves as a
cross-check that we haven't cast some random int into the enum, but I
think we would do better to find those callsites (since they are not
guaranteed to work, anyway; in addition to signedness, it might choose a
much smaller representation).
I do not see either side of the bounds check here:
+ if (options-msg_severity
+ msg_id FSCK_MSG_MIN msg_id FSCK_MSG_MAX)
as really doing anything. Any code which triggers it must already cause
undefined behavior, I think (with the exception of msg_id == FSCK_MSG_MAX,
but presumably that is something we never expect to happen, either).
-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 compile warnings (under mac/clang)
Hi Peff,
On 2015-01-22 23:01, Jeff King wrote:
On Thu, Jan 22, 2015 at 10:20:01PM +0100, Johannes Schindelin wrote:
On 2015-01-22 20:59, Stefan Beller wrote:
cc Johannes Schindelin johannes.schinde...@gmx.de who is working in
the fsck at the moment
On Thu, Jan 22, 2015 at 11:43 AM, Michael Blume blume.m...@gmail.com
wrote:
CC fsck.o
fsck.c:110:38: warning: comparison of unsigned enum expression = 0 is
always true [-Wtautological-compare]
if (options-msg_severity msg_id = 0 msg_id FSCK_MSG_MAX)
~~ ^ ~
According to A2.5.4 of The C Programming Language 2nd edition:
Identifiers declared as enumerators (see Par.A.8.4) are constants of
type int.
Therefore, the warning is incorrect: any assumption about enum fsck_msg_id
to be unsigned is false.
I'm not sure that made it to ANSI. C99 says (setion 6.7.2.2, paragraph
4):
Each enumerated type shall be compatible with char, a signed integer
type, or an unsigned integer type. The choice of type is
implementation-defined, but shall be capable of representing the
values of all the members of the enumeration.
I don't have a copy of C89, but this isn't mentioned in the (very
cursory) list of changes found in C99. Anyway, that's academic.
I think we dealt with a similar situation before, in
3ce3ffb840a1dfa7fcbafa9309fab37478605d08.
Ww. That commit got a chuckle out of me...
This is what I have currently in the way of attempting to fix it (I still
believe that Clang is wrong to make this a warning, and causes more trouble
than it solves):
-- snipsnap --
commit 11b4c713f77081bf8342e5c02055ae8e18d28e8b
Author: Johannes Schindelin johannes.schinde...@gmx.de
Date: Fri Jan 23 12:46:02 2015 +0100
fsck: fix clang -Wtautological-compare with unsigned enum
Clang warns that the fsck_msg_id enum is unsigned, missing that the
specification of the C language allows for C compilers interpreting
enums as signed.
To shut up Clang, we waste a full enum value just so that we compare
against an enum value without messing up the readability of the source
code.
Pointed out by Michael Blume. Jeff King provided the pointer to a commit
fixing the same issue elsewhere in the Git source code.
Signed-off-by: Johannes Schindelin johannes.schinde...@gmx.de
diff --git a/fsck.c b/fsck.c
index 15cb8bd..f76e7a9 100644
--- a/fsck.c
+++ b/fsck.c
@@ -65,6 +65,7 @@
#define MSG_ID(id, severity) FSCK_MSG_##id,
enum fsck_msg_id {
+ FSCK_MSG_MIN = 0,
FOREACH_MSG_ID(MSG_ID)
FSCK_MSG_MAX
};
@@ -76,6 +77,7 @@ static struct {
const char *id_string;
int severity;
} msg_id_info[FSCK_MSG_MAX + 1] = {
+ { NULL, -1 },
FOREACH_MSG_ID(MSG_ID)
{ NULL, -1 }
};
@@ -85,7 +87,7 @@ static int parse_msg_id(const char *text, int len)
{
int i, j;
- for (i = 0; i FSCK_MSG_MAX; i++) {
+ for (i = FSCK_MSG_MIN + 1; i FSCK_MSG_MAX; i++) {
const char *key = msg_id_info[i].id_string;
/* id_string is upper-case, with underscores */
for (j = 0; j len; j++) {
@@ -107,7 +109,8 @@ static int fsck_msg_severity(enum fsck_msg_id msg_id,
{
int severity;
- if (options-msg_severity msg_id = 0 msg_id FSCK_MSG_MAX)
+ if (options-msg_severity
+ msg_id FSCK_MSG_MIN msg_id FSCK_MSG_MAX)
severity = options-msg_severity[msg_id];
else {
severity = msg_id_info[msg_id].severity;
--
To unsubscribe from this list: send the line unsubscribe 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 submodule first time update with proxy
On Sat, Jan 24, 2015 at 5:45 PM, Robert Dailey rcdailey.li...@gmail.com wrote: On Fri, Jan 23, 2015 at 10:23 PM, Robert Dailey rcdailey.li...@gmail.com wrote: On Fri, Jan 23, 2015 at 4:13 PM, Chris Packham judge.pack...@gmail.com wrote: Hi, On Fri, Jan 23, 2015 at 3:50 PM, Robert Dailey rcdailey.li...@gmail.com wrote: I have a submodule using HTTP URL. I do this: $ git submodule init MySubmodule $ git submodule update MySubmodule The 2nd command fails because the HTTP URL cannot be resolved, this is because it requires a proxy. I have http.proxy setup properly in the .git/config of my parent git repository, so I was hoping the submodule update function would have a way to specify it to inherit the proxy value from the parent config. Your not the first to suggest it and you probably won't be the last. It is hard to decide _which_ config variables, if any, should propagate from the parent. What works for one use-case may not necessarily work for another. How can I set up my submodule? Probably the easiest thing would be to make your http.proxy configuration global i.e. $ git config --global http.proxy If you don't want to make it a global setting you can setup the submodule configuration after running init but before running update i.e. $ git submodule init MySubmodule $ (cd MySubmodule git config http.proxy ...) $ git submodule update MySubmodule For some reason, the init call does not create the submodule directory as you indicate. I also checked in .git/modules and it's not there either. OK I must be wrong about that. I was working from memory. Trying it now I see the error in my thinking $ git submodule init bar Submodule 'bar' (bar.git) registered for path 'bar' I thought this meant that bar/.git (and .git/modules/bar) had been created but as you point out I was wrong. Correction: I have to deinit the submodule then init again, then the submodule dir is created (but empty). That's the default state of an uninitialized submodule. When I run the git config command inside the submodule directory, it silently returns and does not indicate failure, however the final git submodule update command shows failure to access the remote and then subsequently the submodule empty directory is removed by Git. So it looks like the only solution to your problem right now is to use git config --global for your proxy configuration. -- To unsubscribe from this list: send the line unsubscribe 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 submodule first time update with proxy
On Fri, Jan 23, 2015 at 4:13 PM, Chris Packham judge.pack...@gmail.com wrote: Hi, On Fri, Jan 23, 2015 at 3:50 PM, Robert Dailey rcdailey.li...@gmail.com wrote: I have a submodule using HTTP URL. I do this: $ git submodule init MySubmodule $ git submodule update MySubmodule The 2nd command fails because the HTTP URL cannot be resolved, this is because it requires a proxy. I have http.proxy setup properly in the .git/config of my parent git repository, so I was hoping the submodule update function would have a way to specify it to inherit the proxy value from the parent config. Your not the first to suggest it and you probably won't be the last. It is hard to decide _which_ config variables, if any, should propagate from the parent. What works for one use-case may not necessarily work for another. How can I set up my submodule? Probably the easiest thing would be to make your http.proxy configuration global i.e. $ git config --global http.proxy If you don't want to make it a global setting you can setup the submodule configuration after running init but before running update i.e. $ git submodule init MySubmodule $ (cd MySubmodule git config http.proxy ...) $ git submodule update MySubmodule For some reason, the init call does not create the submodule directory as you indicate. I also checked in .git/modules and it's not there either. -- To unsubscribe from this list: send the line unsubscribe 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 submodule first time update with proxy
On Fri, Jan 23, 2015 at 10:23 PM, Robert Dailey rcdailey.li...@gmail.com wrote: On Fri, Jan 23, 2015 at 4:13 PM, Chris Packham judge.pack...@gmail.com wrote: Hi, On Fri, Jan 23, 2015 at 3:50 PM, Robert Dailey rcdailey.li...@gmail.com wrote: I have a submodule using HTTP URL. I do this: $ git submodule init MySubmodule $ git submodule update MySubmodule The 2nd command fails because the HTTP URL cannot be resolved, this is because it requires a proxy. I have http.proxy setup properly in the .git/config of my parent git repository, so I was hoping the submodule update function would have a way to specify it to inherit the proxy value from the parent config. Your not the first to suggest it and you probably won't be the last. It is hard to decide _which_ config variables, if any, should propagate from the parent. What works for one use-case may not necessarily work for another. How can I set up my submodule? Probably the easiest thing would be to make your http.proxy configuration global i.e. $ git config --global http.proxy If you don't want to make it a global setting you can setup the submodule configuration after running init but before running update i.e. $ git submodule init MySubmodule $ (cd MySubmodule git config http.proxy ...) $ git submodule update MySubmodule For some reason, the init call does not create the submodule directory as you indicate. I also checked in .git/modules and it's not there either. Correction: I have to deinit the submodule then init again, then the submodule dir is created (but empty). When I run the git config command inside the submodule directory, it silently returns and does not indicate failure, however the final git submodule update command shows failure to access the remote and then subsequently the submodule empty directory is removed by Git. -- To unsubscribe from this list: send the line unsubscribe 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 pull not ignoring the file which has been sent to the temporary ignore list
On Friday, January 23, 2015 01:14:03 PM you wrote: Stefan Beller sbel...@google.com writes: Ok. How should I then ignore any local changes to the .gitignore file ? And while taking pull, git should skip this file ? Look at .git/info/exclude Good answer for .gitignore. In general, you do not ignore local changes to tracked paths. There are some configuration files, like `database.yml`, where we generally put our local DB credentials and we don't want to share such things. That's why we always put related settings inside the .gitignore file. But while I will change it, git will not track the changes of the file, but .gitignore. That's why I used the first thread command. But when the time the came to take a `git pull`, I got to know about the mess. What should be the ideal decision in this case ? I found https://help.github.com/articles/ignoring-files/ as Googles first hit, which advises to use git update-index --assume-unchanged path/to/file.txt Not sure if that is most helpful advice there. Yes, I followed the same. The piece of advice in the last paragraph on that page is wrong (and it has been wrong from the day it was written). The gitignore(5) documentation used to have a similar incorrect piece of advice but we finally corrected it recently. Cf. http://thread.gmane.org/gmane.comp.version-control.git/260954/focus=261118 -- Regards, Arup Rakshit Debugging is twice as hard as writing the code in the first place. Therefore, if you write the code as cleverly as possible, you are, by definition, not smart enough to debug it. --Brian Kernighan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] git-p4: correct --prepare-p4-only instructions
If you use git-p4 with the --prepare-p4-only option, then it prints the p4 command line to use. However, the command line was incorrect: the changelist specification must be supplied on standard input, not as an argument to p4. Signed-off-by: Luke Diamand l...@diamand.org --- git-p4.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/git-p4.py b/git-p4.py index ff132b2..90447de 100755 --- a/git-p4.py +++ b/git-p4.py @@ -1442,7 +1442,7 @@ class P4Submit(Command, P4UserMap): print+ self.clientPath print print To submit, use \p4 submit\ to write a new description, -print or \p4 submit -i %s\ to use the one prepared by \ +print or \p4 submit -i %s\ to use the one prepared by \ \git p4\. % fileName print You can delete the file \%s\ when finished. % fileName -- 2.1.3.1037.g95a6691.dirty -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] git-p4: correct --prepare-p4-only instructions
This fixes a small error in the command that git-p4 suggests when the user gives the --prepare-p4-only option. It tells the user to use p4 submit -i filename but the p4 submit command reads a change specification on standard input. The correct command line is therefore: p4 submit -i filename Luke Diamand (1): git-p4: correct --prepare-p4-only instructions git-p4.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) -- 2.1.3.1037.g95a6691.dirty -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/7] Coding style fixes.
This patch set contatins minor style fixes. CodingGuidelines contains rule that the star must side with variable name. Alexander Kuleshov (7): show-branch: minor style fix clone: minor style fix test-hashmap: minor style fix http-backend: minor style fix refs: minor style fix quote: minor style fix fast-import: minor style fix builtin/clone.c | 2 +- builtin/show-branch.c | 2 +- fast-import.c | 2 +- http-backend.c| 2 +- quote.c | 2 +- refs.h| 2 +- test-hashmap.c| 4 ++-- 7 files changed, 8 insertions(+), 8 deletions(-) -- 2.3.0.rc1.275.g028c360 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/7] test-hashmap: minor style fix
Asterisk must be next with variable
Signed-off-by: Alexander Kuleshov kuleshovm...@gmail.com
---
test-hashmap.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/test-hashmap.c b/test-hashmap.c
index cc2891d..5f67120 100644
--- a/test-hashmap.c
+++ b/test-hashmap.c
@@ -14,13 +14,13 @@ static const char *get_value(const struct test_entry *e)
}
static int test_entry_cmp(const struct test_entry *e1,
- const struct test_entry *e2, const char* key)
+ const struct test_entry *e2, const char *key)
{
return strcmp(e1-key, key ? key : e2-key);
}
static int test_entry_cmp_icase(const struct test_entry *e1,
- const struct test_entry *e2, const char* key)
+ const struct test_entry *e2, const char *key)
{
return strcasecmp(e1-key, key ? key : e2-key);
}
--
2.3.0.rc1.275.g028c360
--
To unsubscribe from this list: send the line 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/7] http-backend: minor style fix
Asterisk must be next with variable
Signed-off-by: Alexander Kuleshov kuleshovm...@gmail.com
---
http-backend.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/http-backend.c b/http-backend.c
index b6c0484..7b5670b 100644
--- a/http-backend.c
+++ b/http-backend.c
@@ -515,7 +515,7 @@ static NORETURN void die_webcgi(const char *err, va_list
params)
exit(0); /* we successfully reported a failure ;-) */
}
-static char* getdir(void)
+static char *getdir(void)
{
struct strbuf buf = STRBUF_INIT;
char *pathinfo = getenv(PATH_INFO);
--
2.3.0.rc1.275.g028c360
--
To unsubscribe from this list: send the line 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/7] clone: minor style fix
Asterisk must be next with variable
Signed-off-by: Alexander Kuleshov kuleshovm...@gmail.com
---
builtin/clone.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/builtin/clone.c b/builtin/clone.c
index d262a4d..a1ca780 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -741,7 +741,7 @@ static void write_refspec_config(const char *src_ref_prefix,
static void dissociate_from_references(void)
{
- static const char* argv[] = { repack, -a, -d, NULL };
+ static const char *argv[] = { repack, -a, -d, NULL };
if (run_command_v_opt(argv, RUN_GIT_CMD|RUN_COMMAND_NO_STDIN))
die(_(cannot repack to clean up));
--
2.3.0.rc1.275.g028c360
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 5/7] refs: minor style fix
Asterisk must be next with variable Signed-off-by: Alexander Kuleshov kuleshovm...@gmail.com --- refs.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/refs.h b/refs.h index 425ecf7..bd8afe2 100644 --- a/refs.h +++ b/refs.h @@ -86,7 +86,7 @@ extern int for_each_branch_ref(each_ref_fn, void *); extern int for_each_remote_ref(each_ref_fn, void *); extern int for_each_replace_ref(each_ref_fn, void *); extern int for_each_glob_ref(each_ref_fn, const char *pattern, void *); -extern int for_each_glob_ref_in(each_ref_fn, const char *pattern, const char* prefix, void *); +extern int for_each_glob_ref_in(each_ref_fn, const char *pattern, const char *prefix, void *); extern int head_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data); extern int for_each_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data); -- 2.3.0.rc1.275.g028c360 -- To unsubscribe from this list: send the line 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 7/7] fast-import: minor style fix
Asterisk must be next with variable
Signed-off-by: Alexander Kuleshov kuleshovm...@gmail.com
---
fast-import.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fast-import.c b/fast-import.c
index 1b50923..fec67ca 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -3110,7 +3110,7 @@ static void parse_progress(void)
skip_optional_lf();
}
-static char* make_fast_import_path(const char *path)
+static char *make_fast_import_path(const char *path)
{
if (!relative_marks_paths || is_absolute_path(path))
return xstrdup(path);
--
2.3.0.rc1.275.g028c360
--
To unsubscribe from this list: send the line 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/7] quote: minor style fix
Asterisk must be next with variable
Signed-off-by: Alexander Kuleshov kuleshovm...@gmail.com
---
quote.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/quote.c b/quote.c
index 7920e18..02e9a3c 100644
--- a/quote.c
+++ b/quote.c
@@ -42,7 +42,7 @@ void sq_quote_buf(struct strbuf *dst, const char *src)
free(to_free);
}
-void sq_quote_argv(struct strbuf *dst, const char** argv, size_t maxlen)
+void sq_quote_argv(struct strbuf *dst, const char **argv, size_t maxlen)
{
int i;
--
2.3.0.rc1.275.g028c360
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Should copy/rename detection consider file overwrites?
On Fri, Jan 23, 2015 at 10:29:08AM +0900, Mike Hommey wrote: While fooling around with copy/rename detection, I noticed that it doesn't detect the case where you copy or rename a file on top of another: $ git init $ (echo foo; echo bar) foo If I replace this with a longer input, like: cp /usr/share/dict/words foo $ git add foo $ git commit -m foo $ echo 0 bar $ git add bar $ git commit -m bar $ git mv -f foo bar $ git commit -m foobar $ git log --oneline --reverse 7dc2765 foo b0c837d bar 88caeba foobar $ git blame -s -C -C bar 88caebab 1) foo 88caebab 2) bar Then the blame shows me the initial foo commit. So I think it is mainly that your toy example is too small (I think we will do exact rename detection whatever the size is, but I expect we are getting hung up on the break detection between 0\n and foo\nbar\n). I can see how this is not trivially representable in e.g. git diff-tree, but shouldn't at least blame try to tell that those lines actually come from 7dc2765? diff-tree can show this, too, but you need to turn on break detection which will notice that bar has essentially been rewritten (and then consider its sides as candidates for rename detection). For example (with the longer input, as above): $ git diff-tree --name-status -M HEAD c6fe146b0c73adcbc4dbc2e58eb83af9007678bc M bar D foo $ git diff-tree --name-status -M -B HEAD c6fe146b0c73adcbc4dbc2e58eb83af9007678bc R100foo bar Presumably if you set the break score low enough, your original example would behave the same way, but I couldn't get it to work (I didn't look closely, but I imagine it is just so tiny that we hit the internal limits on how low you can set the score). -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/7] show-branch: minor style fix
Asterisk must be next with variable
Signed-off-by: Alexander Kuleshov kuleshovm...@gmail.com
---
builtin/show-branch.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/builtin/show-branch.c b/builtin/show-branch.c
index 3a7ec55..e7684c8 100644
--- a/builtin/show-branch.c
+++ b/builtin/show-branch.c
@@ -6,7 +6,7 @@
#include parse-options.h
#include commit-slab.h
-static const char* show_branch_usage[] = {
+static const char *show_branch_usage[] = {
N_(git show-branch [-a | --all] [-r | --remotes] [--topo-order |
--date-order]\n
[--current] [--color[=when] | --no-color]
[--sparse]\n
[--more=n | --list | --independent | --merge-base]\n
--
2.3.0.rc1.275.g028c360
--
To unsubscribe from this list: send the line unsubscribe 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: regression with mergetool and answering n (backport fix / add tests)
Hi,
I am a bit surprised that this bug still exists in maint / v2.2.2.
Cherry-picking/merging 0ddedd4 fixes it.
Regards,
Daniel.
On 26.12.2014 02:12, Daniel Hahler wrote:
Hi David,
sorry for the confusion - the patch / fix I've mentioned was meant to be
applied on the commit that caused the regression and not current master.
Cheers,
Daniel.
On 26.12.2014 02:00, David Aguilar wrote:
On Tue, Dec 23, 2014 at 08:08:34PM +0100, Daniel Hahler wrote:
Hi,
this is in reply to the commits from David:
commit 0ddedd4d6b9b3e8eb3557d8ed28e1a0b354a25f8
Refs: v2.2.0-60-g0ddedd4
Merge: e886efd 1e86d5b
Author: Junio C Hamano gits...@pobox.com
AuthorDate: Fri Dec 12 14:31:39 2014 -0800
Commit: Junio C Hamano gits...@pobox.com
CommitDate: Fri Dec 12 14:31:39 2014 -0800
Merge branch 'da/difftool-mergetool-simplify-reporting-status'
Code simplification.
* da/difftool-mergetool-simplify-reporting-status:
mergetools: stop setting $status in merge_cmd()
mergetool: simplify conditionals
difftool--helper: add explicit exit statement
mergetool--lib: remove use of $status global
mergetool--lib: remove no-op assignment to $status from
setup_user_tool
I've ran into a problem, where git mergetool (using vimdiff) would add
the changes to the index, although you'd answered n after not
changing/saving
the merged file.
Thanks for the heads-up.
Do you perhaps have mergetool.vimdiff.trustExitCode defined, or
a similar setting?
If you saw the prompt then it should have aborted right after
you answered n.
The very last thing merge_cmd() for vimdiff does is call
check_unchanged(). We'll come back to check_unchanged() later.
I tried to reproduce this issue. Here's a transcript:
$ git status -s
UU file.txt
$ git mergetool -t vimdiff file.txt
Merging:
file.txt
Normal merge conflict for 'file.txt':
{local}: modified file
{remote}: modified file
4 files to edit
Enter :qall inside vim
file.txt seems unchanged.
Was the merge successful? [y/n] n
merge of file.txt failed
Continue merging other unresolved paths (y/n) ? n
$ git status -s
UU file.txt
That seemed to work fine. Any clues?
More notes below...
This regression has been introduced in:
commit 99474b6340dbcbe58f6c256fdee231cbadb060f4
Author: David Aguilar dav...@gmail.com
Date: Fri Nov 14 13:33:55 2014 -0800
difftool: honor --trust-exit-code for builtin tools
run_merge_tool() was not setting $status, which prevented the
exit code for builtin tools from being forwarded to the caller.
Capture the exit status and add a test to guarantee the behavior.
Reported-by: Adria Farres 14farr...@gmail.com
Signed-off-by: David Aguilar dav...@gmail.com
Signed-off-by: Junio C Hamano gits...@pobox.com
diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index c45a020..cce4f8c 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -221,6 +221,7 @@ run_merge_tool () {
else
run_diff_cmd $1
fi
+ status=$?
return $status
}
My fix has been the following, but I agree that the changes from David
are much better in general.
diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index cce4f8c..fa9acb1 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -105,6 +105,7 @@ check_unchanged () {
esac
done
fi
+ return $status
}
I don't think this fix does anything.
Here is all of check_unchanged() for context:
check_unchanged () {
if test $MERGED -nt $BACKUP
then
return 0
else
while true
do
echo $MERGED seems unchanged.
printf Was the merge successful? [y/n]
read answer || return 1
case $answer in
y*|Y*) return 0 ;;
n*|N*) return 1 ;;
esac
done
fi
}
The addition of return $status after the fi in the above fix
won't do anything because that code is unreachable.
We either return 0 or 1.
I haven't verified if it really fixes the regression, but if it does it
should get backported into the branches where the regression is present.
Also, the $status variable doesn't even exist anymore, so the
fix is suspect.
What platform are you on?
Also, there should be some tests for this.
I don't disagree with that ;-)
Let me know if you have any clues. I don't see anything obvious.
cheers,
--
http://daniel.hahler.de/
signature.asc
Description: OpenPGP digital signature
git push --repo option not working as described in git manual.
I am using git 2.2.2 and want to report an issue with git push --repo option. git 2.2.2 manual says that git push --repo=public will push to public only if the current branch does not track a remote branch. See http://git-scm.com/docs/git-push However, I see that git pushes even when the current branch is tracking a remote branch. Here is the test case (push default setting is simple, git version 2.2.2, Mac OS X 10.10.1): 1. I have a local branch master. 2. master tracks remote branch blah/master. Here blah is the remote repository. 3. While I am on my local master branch, I run git push --repo=silver 4. git pushes the local master branch to silver repository. 5. But per git manual, it shouldn't push to silver, as the local branch is tracking blah/master. Here is another test case (push default setting is simple, git version 2.2.2, Mac OS X 10.10.1): 1. I have a local branch whoa. 2. whoa tracks remote branch origin/whoa. Here origin is the remote repository. 3. While I am on my local whoa branch, I run git push --repo=blah 4. git pushes the local whoa branch to blah repository. 5. But per git manual, it shouldn't push to blah, as the local branch is tracking origin/whoa. Appreciate your help. Prem -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[GUILT 0/5] doc: less guilt-foo invocations, minor Makefile fixes
guilt no longer supports running commands on the guilt-add form. You need to use guilt add instead. This patch series updates most of the documentation to use the supported guilt add form. There is one known instance where I did not change the style: in the NAME section in Documentation/guilt-*.txt. The reason is that if I change it there, xmlto will create the man pages as e.g. guilt_add.1 instead of guilt-add.1, and I don't know how to fix that. Also, the git man pages (as of Git 2.1.0) still have git-add under the NAME heading of git-add(1), so it might be wise to follow suite. While working on this, I also found two minor issues with Documentation/Makefile. /ceder Per Cederqvist (5): Fix generation of Documentation/usage-%.txt. doc: guilt.xml depends on cmds.txt. doc: don't use guilt-foo invocations in examples. doc: don't use guilt-foo invocations in usage messages. doc: git doesn't use git-foo invocations. Documentation/.gitignore| 3 +++ Documentation/Makefile | 6 -- Documentation/guilt-add.txt | 4 ++-- Documentation/guilt-delete.txt | 2 +- Documentation/guilt-diff.txt| 2 +- Documentation/guilt-help.txt| 4 ++-- Documentation/guilt-new.txt | 6 +++--- Documentation/guilt-refresh.txt | 2 +- Documentation/guilt-repair.txt | 2 +- Documentation/guilt-rm.txt | 2 +- Documentation/guilt-select.txt | 4 ++-- Documentation/usage.sh | 8 +++- 12 files changed, 24 insertions(+), 21 deletions(-) -- 2.1.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
[GUILT 3/5] doc: don't use guilt-foo invocations in examples.
Note: there is one place where I replace guilt-repair with guilt repair instead of +guilt repair+. At least the version of docbook I'm using mishandles the + signs in that particular spot (even though it works properly for +guilt select+ in another file. I know too little docbook to be able to find the cause. Signed-off-by: Per Cederqvist ced...@opera.com --- Documentation/guilt-add.txt| 2 +- Documentation/guilt-delete.txt | 2 +- Documentation/guilt-diff.txt | 2 +- Documentation/guilt-help.txt | 4 ++-- Documentation/guilt-new.txt| 6 +++--- Documentation/guilt-repair.txt | 2 +- Documentation/guilt-select.txt | 4 ++-- 7 files changed, 11 insertions(+), 11 deletions(-) diff --git a/Documentation/guilt-add.txt b/Documentation/guilt-add.txt index 6d2785a..a276f09 100644 --- a/Documentation/guilt-add.txt +++ b/Documentation/guilt-add.txt @@ -24,7 +24,7 @@ EXAMPLES Create and add a new file example.c $ touch example.c - $ guilt-add example.c + $ guilt add example.c Author -- diff --git a/Documentation/guilt-delete.txt b/Documentation/guilt-delete.txt index ef57dc6..4e8c28c 100644 --- a/Documentation/guilt-delete.txt +++ b/Documentation/guilt-delete.txt @@ -25,7 +25,7 @@ EXAMPLES Delete a patch called 'foobar': - $ guilt-delete foobar + $ guilt delete foobar Author -- diff --git a/Documentation/guilt-diff.txt b/Documentation/guilt-diff.txt index 986ceca..0ee062c 100644 --- a/Documentation/guilt-diff.txt +++ b/Documentation/guilt-diff.txt @@ -18,7 +18,7 @@ OPTIONS --- -z:: Output a interdiff against the top-most applied patch. This should - produce the same diff as +guilt-new -f foo+. + produce the same diff as +guilt new -f foo+. path...:: Restrict diff output to a given set of files. diff --git a/Documentation/guilt-help.txt b/Documentation/guilt-help.txt index ed6a5cf..df0e0fb 100644 --- a/Documentation/guilt-help.txt +++ b/Documentation/guilt-help.txt @@ -18,11 +18,11 @@ EXAMPLES Open the guilt-status man page - $ guilt-help status + $ guilt help status Open the guilt man page - $ guilt-help + $ guilt help Author -- diff --git a/Documentation/guilt-new.txt b/Documentation/guilt-new.txt index a2c8a4c..698dcb7 100644 --- a/Documentation/guilt-new.txt +++ b/Documentation/guilt-new.txt @@ -42,16 +42,16 @@ EXAMPLES Create a new patch called 'foobar': - $ guilt-new foobar + $ guilt new foobar Create a patch called 'foo' and supply a patch description interactively: - $ guilt-new -e foo + $ guilt new -e foo Create a patch called 'bar' with a provided patch description and sign off on the patch: - $ guilt-new -s -m patch-fu bar + $ guilt new -s -m patch-fu bar Author -- diff --git a/Documentation/guilt-repair.txt b/Documentation/guilt-repair.txt index 4aa472b..4faf113 100644 --- a/Documentation/guilt-repair.txt +++ b/Documentation/guilt-repair.txt @@ -22,7 +22,7 @@ Perform various repository repairs. You must specify one mode of repair: WARNING: Running this command may result in commits and working directory changes being lost. You may want to create a new reference (e.g., branch, or reflog) to the original HEAD before using - guilt-repair. + guilt repair. --status:: Upgrade the status file from old format to new. diff --git a/Documentation/guilt-select.txt b/Documentation/guilt-select.txt index f7fb5f7..dd5833e 100644 --- a/Documentation/guilt-select.txt +++ b/Documentation/guilt-select.txt @@ -19,10 +19,10 @@ the following way: * An unguarded patch is always applied. * A patch with a positive guard is applied *only* if the guard is -selected with guilt-select. +selected with +guilt select+. * A patch with a negative guard is applied *unless* the guard is -selected with guilt-select. +selected with +guilt select+. OPTIONS --- -- 2.1.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
[GUILT 2/5] doc: guilt.xml depends on cmds.txt.
Specify an explicit dependency, to stop make from trying to generate guilt.xml if cmds.txt could not be created. The asciidoc will fail and produce an error message that might hide the original error message. The added dependency causes make to not remove the guilt.xml file. Add *.xml to .gitignore. Signed-off-by: Per Cederqvist ced...@opera.com --- Documentation/.gitignore | 3 +++ Documentation/Makefile | 2 ++ 2 files changed, 5 insertions(+) diff --git a/Documentation/.gitignore b/Documentation/.gitignore index c4f0588..9b8d4da 100644 --- a/Documentation/.gitignore +++ b/Documentation/.gitignore @@ -11,3 +11,6 @@ version.txt # Generated file dependency list doc.dep + +# Intermediate generated files +*.xml diff --git a/Documentation/Makefile b/Documentation/Makefile index ec3c9e8..2574125 100644 --- a/Documentation/Makefile +++ b/Documentation/Makefile @@ -60,6 +60,8 @@ cmds.txt: cmd-list.sh $(MAN1_TXT) guilt.7 guilt.html: guilt.txt footer.txt version.txt +guilt.xml: cmds.txt + clean: rm -f *.xml *.html *.1 *.7 doc.dep rm -f cmds.txt -- 2.1.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
[GUILT 1/5] Fix generation of Documentation/usage-%.txt.
The old rule worked, most of the time, but had several issues: - It depended on the corresponding guilt-*.txt file, but the usage.sh script actually reads ../guilt-foo. - Actually, each usage-%.txt depended on all guilt-*.txt files, so make had to do more work than necessary if a single file was altered. - The construct broke parallel make, which would spawn several usage.sh at once. This leads to unnecessary work, and could potentially result in broken usage files if the echo some_string some_file construct used by usage.sh isn't atomic. Fixed by letting the usage.sh script update a single file, and writing a proper implicit make rule. This makes parallel make work a lot better. There is a small downside, though, as usage.sh will now be run once for each command (if everything is regenerated). I think it is worth to pay that price to get the correctness. This command is still very fast compared to the docbook processing. Signed-off-by: Per Cederqvist ced...@opera.com --- Documentation/Makefile | 4 ++-- Documentation/usage.sh | 8 +++- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/Documentation/Makefile b/Documentation/Makefile index b6c3285..ec3c9e8 100644 --- a/Documentation/Makefile +++ b/Documentation/Makefile @@ -66,8 +66,8 @@ clean: rm -f usage-*.txt rm -f version.txt -usage-%.txt: $(MAN1_TXT) usage.sh - sh ./usage.sh +usage-guilt-%.txt: ../guilt-% usage.sh + sh ./usage.sh $ %.html : %.txt footer.txt version.txt $(ASCIIDOC) -b xhtml11 -d manpage -f asciidoc.conf $(ASCIIDOC_EXTRA) $ diff --git a/Documentation/usage.sh b/Documentation/usage.sh index 20fdca4..629f546 100644 --- a/Documentation/usage.sh +++ b/Documentation/usage.sh @@ -1,7 +1,5 @@ #!/bin/sh -for i in `ls ../guilt-*`; do - name=$(basename $i) - u=$(grep USAGE $i | sed 's/USAGE=//' | sed 's/$//') - echo '$name' $u usage-$name.txt -done +name=$(basename $1) +u=$(grep USAGE $1 | sed 's/USAGE=//' | sed 's/$//') +echo '$name' $u usage-$name.txt -- 2.1.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
[GUILT 4/5] doc: don't use guilt-foo invocations in usage messages.
Signed-off-by: Per Cederqvist ced...@opera.com --- Documentation/usage.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/usage.sh b/Documentation/usage.sh index 629f546..9cc49f7 --- a/Documentation/usage.sh +++ b/Documentation/usage.sh @@ -2,4 +2,4 @@ name=$(basename $1) u=$(grep USAGE $1 | sed 's/USAGE=//' | sed 's/$//') -echo '$name' $u usage-$name.txt +echo '`echo $name|sed -e 's/^guilt-/guilt /'`' $u usage-$name.txt -- 2.1.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
[GUILT 5/5] doc: git doesn't use git-foo invocations.
Make them into reference to the man pages instead. Signed-off-by: Per Cederqvist ced...@opera.com --- Documentation/guilt-add.txt | 2 +- Documentation/guilt-refresh.txt | 2 +- Documentation/guilt-rm.txt | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Documentation/guilt-add.txt b/Documentation/guilt-add.txt index a276f09..067b6ca 100644 --- a/Documentation/guilt-add.txt +++ b/Documentation/guilt-add.txt @@ -11,7 +11,7 @@ include::usage-guilt-add.txt[] DESCRIPTION --- -Adds the files specified to git using git-add making it available to guilt. +Adds the files specified to git using git-add(1) making it available to guilt. OPTIONS --- diff --git a/Documentation/guilt-refresh.txt b/Documentation/guilt-refresh.txt index 7757bdc..98076e3 100644 --- a/Documentation/guilt-refresh.txt +++ b/Documentation/guilt-refresh.txt @@ -23,7 +23,7 @@ OPTIONS Include a diffstat output in the patch file. Useful for cases where patches will be submitted with other tools. + -If the command line option is omitted, the corresponding git-config +If the command line option is omitted, the corresponding git-config(1) option guilt.diffstat will be queried. So this would enable diffstat output by default: diff --git a/Documentation/guilt-rm.txt b/Documentation/guilt-rm.txt index 71b49fe..cfe471e 100644 --- a/Documentation/guilt-rm.txt +++ b/Documentation/guilt-rm.txt @@ -11,7 +11,7 @@ include::usage-guilt-rm.txt[] DESCRIPTION --- -Removes the files specified from git using git-rm +Removes the files specified from git using git-rm(1). OPTIONS --- -- 2.1.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: Git compile warnings (under mac/clang)
On Fri, Jan 23, 2015 at 01:38:17PM +0100, Johannes Schindelin wrote:
Unless we are willing to drop the = 0 check completely. I think it is
valid to do so regardless of the compiler's representation decision due
to the numbering rules I mentioned above. It kind-of serves as a
cross-check that we haven't cast some random int into the enum, but I
think we would do better to find those callsites (since they are not
guaranteed to work, anyway; in addition to signedness, it might choose a
much smaller representation).
Yeah, well, this check is really more of a safety net in case I messed
up anything; I was saved so many times by my own defensive programming
that I try to employ it as much as I can.
Yeah, I am all in favor of defensive programming. But I am not sure that
it is defending much here, as we silently fall back to an alternate
value for the severity. Would we notice, or would that produce subtly
wrong results? IOW, would this be better as:
assert(msg_id = 0 msg_id FSCK_MSG_MAX);
or something?
-- snip --
diff --git a/fsck.c b/fsck.c
index 15cb8bd..8f8c82f 100644
--- a/fsck.c
+++ b/fsck.c
@@ -107,7 +107,7 @@ static int fsck_msg_severity(enum fsck_msg_id msg_id,
{
int severity;
- if (options-msg_severity msg_id = 0 msg_id FSCK_MSG_MAX)
+ if (options-msg_severity ((unsigned int) msg_id) FSCK_MSG_MAX)
severity = options-msg_severity[msg_id];
else {
severity = msg_id_info[msg_id].severity;
-- snap --
What do you think? Michael, does this cause more Clang warnings, or would it
resolve the issue?
Hmm, yeah, that does not seem unreasonable, and is more localized.
-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
[GUILT 2/2] Teach guilt graph the -x exclude-pattern option.
Some projects keep a ChangeLog which every commit modifies. This
makes the graph a very uninteresting single line of commits. It is
sometimes useful to see how the graph would look if we ignore the
ChangeLog file.
The new -x option is useful in situations like this. It can be
repeated several times to ignore many files. Each argument is saved
to a temporary file and grep -v -f $TEMPORARY is used to filter out
the file names you want to ignore.
Also added a minimal test case and documentation.
Signed-off-by: Per Cederqvist ced...@opera.com
---
Documentation/guilt-graph.txt | 5 +
guilt-graph | 24 ++--
regression/t-033.out | 12
regression/t-033.sh | 3 +++
4 files changed, 38 insertions(+), 6 deletions(-)
diff --git a/Documentation/guilt-graph.txt b/Documentation/guilt-graph.txt
index f43206e..eeed321 100644
--- a/Documentation/guilt-graph.txt
+++ b/Documentation/guilt-graph.txt
@@ -16,6 +16,11 @@ patches.
OPTIONS
---
+-x pattern::
+ Ignore files that matches the given grep pattern. Can be
+ repeated to ignore several files. This can be useful to ignore
+ for instance ChangeLog files that every commit modifies.
+
patchname::
Instead of starting with the topmost applied patch, start with
patchname.
diff --git a/guilt-graph b/guilt-graph
index d90c2f1..4d5fe46 100755
--- a/guilt-graph
+++ b/guilt-graph
@@ -3,7 +3,7 @@
# Copyright (c) Josef Jeff Sipek, 2007-2013
#
-USAGE=[patchname]
+USAGE=[-x exclude-pattern]... [patchname]
if [ -z $GUILT_VERSION ]; then
echo Invoking `basename $0` directly is no longer supported. 2
exit 1
@@ -11,6 +11,22 @@ fi
_main() {
+cache=$GUILT_DIR/$branch/.graphcache.$$
+xclude=$GUILT_DIR/$branch/.graphexclude.$$
+trap rm -rf \$cache\ \$xclude\ 0
+mkdir $cache
+$xclude
+
+while [ $# -gt 0 ]; do
+if [ $1 = -x ] [ $# -ge 2 ]; then
+ echo $2 $xclude
+ shift
+ shift
+else
+ break
+fi
+done
+
if [ $# -gt 1 ]; then
usage
fi
@@ -39,10 +55,6 @@ getfiles()
git diff-tree -r $1^ $1 | cut -f2
}
-cache=$GUILT_DIR/$branch/.graphcache.$$
-mkdir $cache
-trap rm -rf \$cache\ 0
-
disp digraph G {
current=$top
@@ -66,7 +78,7 @@ while [ $current != $base ]; do
rm -f $cache/dep
touch $cache/dep
- getfiles $current | while read f; do
+ getfiles $current | grep -v -f $xclude | while read f; do
# hash the filename
fh=`echo $f | sha1 | cut -d' ' -f1`
if [ -e $cache/$fh ]; then
diff --git a/regression/t-033.out b/regression/t-033.out
index c120d4f..1ed371f 100644
--- a/regression/t-033.out
+++ b/regression/t-033.out
@@ -88,3 +88,15 @@ digraph G {
ff2775f8d1dc753f635830adcc3a067e0b681e2d [label=a.patch]
891bc14b5603474c9743fd04f3da888644413dc5 -
ff2775f8d1dc753f635830adcc3a067e0b681e2d; // ?
}
+%% The same graph, but excluding deps introduced by file.txt.
+% guilt graph -x file.txt
+digraph G {
+# checking rev bc7df666a646739eaf559af23cab72f2bfd01f0e
+ bc7df666a646739eaf559af23cab72f2bfd01f0e
[label=a-\betterquicker'-patch.patch]
+# checking rev 891bc14b5603474c9743fd04f3da888644413dc5
+ 891bc14b5603474c9743fd04f3da888644413dc5 [label=c.patch]
+# checking rev c7014443c33d2b0237293687ceb9cbd38313df65
+ c7014443c33d2b0237293687ceb9cbd38313df65 [label=b.patch]
+# checking rev ff2775f8d1dc753f635830adcc3a067e0b681e2d
+ ff2775f8d1dc753f635830adcc3a067e0b681e2d [label=a.patch]
+}
diff --git a/regression/t-033.sh b/regression/t-033.sh
index 9fe1827..ae22914 100755
--- a/regression/t-033.sh
+++ b/regression/t-033.sh
@@ -59,3 +59,6 @@ cmd git add file.txt
cmd guilt refresh
fixup_time_info a-\betterquicker'-patch.patch
cmd guilt graph
+
+echo %% The same graph, but excluding deps introduced by file.txt.
+cmd guilt graph -x file.txt
--
2.1.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
[GUILT 1/2] guilt graph: Simplify getfiles.
git diff-tree by default emits TAB-separated fields. cut by defaults
processes TAB-separated fields. Simplify getfiles() by using TAB as
the separator.
Signed-off-by: Per Cederqvist ced...@opera.com
---
guilt-graph | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/guilt-graph b/guilt-graph
index 0857e0d..d90c2f1 100755
--- a/guilt-graph
+++ b/guilt-graph
@@ -36,7 +36,7 @@ fi
getfiles()
{
- git diff-tree -r $1^ $1 | tr '\t' ' ' | cut -d' ' -f6
+ git diff-tree -r $1^ $1 | cut -f2
}
cache=$GUILT_DIR/$branch/.graphcache.$$
--
2.1.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: [GUILT 3/5] doc: don't use guilt-foo invocations in examples.
On Fri, Jan 23, 2015 at 02:24:57PM +0100, Per Cederqvist wrote: Note: there is one place where I replace guilt-repair with guilt repair instead of +guilt repair+. At least the version of docbook I'm using mishandles the + signs in that particular spot (even though it works properly for +guilt select+ in another file. I know too little docbook to be able to find the cause. Yeah, a bit of a mystery to me too. Regardless, Signed-off-by: Josef 'Jeff' Sipek jef...@josefsipek.net Signed-off-by: Per Cederqvist ced...@opera.com --- Documentation/guilt-add.txt| 2 +- Documentation/guilt-delete.txt | 2 +- Documentation/guilt-diff.txt | 2 +- Documentation/guilt-help.txt | 4 ++-- Documentation/guilt-new.txt| 6 +++--- Documentation/guilt-repair.txt | 2 +- Documentation/guilt-select.txt | 4 ++-- 7 files changed, 11 insertions(+), 11 deletions(-) diff --git a/Documentation/guilt-add.txt b/Documentation/guilt-add.txt index 6d2785a..a276f09 100644 --- a/Documentation/guilt-add.txt +++ b/Documentation/guilt-add.txt @@ -24,7 +24,7 @@ EXAMPLES Create and add a new file example.c $ touch example.c - $ guilt-add example.c + $ guilt add example.c Author -- diff --git a/Documentation/guilt-delete.txt b/Documentation/guilt-delete.txt index ef57dc6..4e8c28c 100644 --- a/Documentation/guilt-delete.txt +++ b/Documentation/guilt-delete.txt @@ -25,7 +25,7 @@ EXAMPLES Delete a patch called 'foobar': - $ guilt-delete foobar + $ guilt delete foobar Author -- diff --git a/Documentation/guilt-diff.txt b/Documentation/guilt-diff.txt index 986ceca..0ee062c 100644 --- a/Documentation/guilt-diff.txt +++ b/Documentation/guilt-diff.txt @@ -18,7 +18,7 @@ OPTIONS --- -z:: Output a interdiff against the top-most applied patch. This should - produce the same diff as +guilt-new -f foo+. + produce the same diff as +guilt new -f foo+. path...:: Restrict diff output to a given set of files. diff --git a/Documentation/guilt-help.txt b/Documentation/guilt-help.txt index ed6a5cf..df0e0fb 100644 --- a/Documentation/guilt-help.txt +++ b/Documentation/guilt-help.txt @@ -18,11 +18,11 @@ EXAMPLES Open the guilt-status man page - $ guilt-help status + $ guilt help status Open the guilt man page - $ guilt-help + $ guilt help Author -- diff --git a/Documentation/guilt-new.txt b/Documentation/guilt-new.txt index a2c8a4c..698dcb7 100644 --- a/Documentation/guilt-new.txt +++ b/Documentation/guilt-new.txt @@ -42,16 +42,16 @@ EXAMPLES Create a new patch called 'foobar': - $ guilt-new foobar + $ guilt new foobar Create a patch called 'foo' and supply a patch description interactively: - $ guilt-new -e foo + $ guilt new -e foo Create a patch called 'bar' with a provided patch description and sign off on the patch: - $ guilt-new -s -m patch-fu bar + $ guilt new -s -m patch-fu bar Author -- diff --git a/Documentation/guilt-repair.txt b/Documentation/guilt-repair.txt index 4aa472b..4faf113 100644 --- a/Documentation/guilt-repair.txt +++ b/Documentation/guilt-repair.txt @@ -22,7 +22,7 @@ Perform various repository repairs. You must specify one mode of repair: WARNING: Running this command may result in commits and working directory changes being lost. You may want to create a new reference (e.g., branch, or reflog) to the original HEAD before using - guilt-repair. + guilt repair. --status:: Upgrade the status file from old format to new. diff --git a/Documentation/guilt-select.txt b/Documentation/guilt-select.txt index f7fb5f7..dd5833e 100644 --- a/Documentation/guilt-select.txt +++ b/Documentation/guilt-select.txt @@ -19,10 +19,10 @@ the following way: * An unguarded patch is always applied. * A patch with a positive guard is applied *only* if the guard is -selected with guilt-select. +selected with +guilt select+. * A patch with a negative guard is applied *unless* the guard is -selected with guilt-select. +selected with +guilt select+. OPTIONS --- -- 2.1.0 -- mainframe, n.: An obsolete device still used by thousands of obsolete companies serving billions of obsolete customers and making huge obsolete profits for their obsolete shareholders. And this year's run twice as fast as last year's. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [GUILT 1/5] Fix generation of Documentation/usage-%.txt.
On Fri, Jan 23, 2015 at 3:21 PM, Jeff Sipek jef...@josefsipek.net wrote:
On Fri, Jan 23, 2015 at 02:24:55PM +0100, Per Cederqvist wrote:
The old rule worked, most of the time, but had several issues:
- It depended on the corresponding guilt-*.txt file, but the usage.sh
script actually reads ../guilt-foo.
- Actually, each usage-%.txt depended on all guilt-*.txt files, so
make had to do more work than necessary if a single file was
altered.
- The construct broke parallel make, which would spawn several
usage.sh at once. This leads to unnecessary work, and could
potentially result in broken usage files if the echo some_string
some_file construct used by usage.sh isn't atomic.
Fixed by letting the usage.sh script update a single file, and writing
a proper implicit make rule. This makes parallel make work a lot
better.
Nice!
There is a small downside, though, as usage.sh will now be run once
for each command (if everything is regenerated). I think it is worth
to pay that price to get the correctness. This command is still very
fast compared to the docbook processing.
Given how much simple usage.sh got, I'm thinking it might be worth it to
just remove it, and just shove the rule into the makefile itself.
Ok, I tried to write it. I came up with the following. (Note: I have *not*
tested it.) It's not *that* ugly.
usage-guilt-%.txt: ../guilt-% usage.sh
echo '$(basename $)' `sed -n -e '/^USAGE=/{s/USAGE=//; s/$//; p;
q}' $` $@
What do you think? Too opaque? Your change looks good.
Too opaque, and not tested enough. It doesn't work, since make will
handle all $. You need to write $$ instead of $ in at least one of the
places. I would stick with usage.sh, as getting the quoting right when
you have make, shell, subshells, and sed all at the same time is just
too painful.
But it is of course up to you. You are the maintainer. :-)
/ceder
Jeff.
Signed-off-by: Per Cederqvist ced...@opera.com
---
Documentation/Makefile | 4 ++--
Documentation/usage.sh | 8 +++-
2 files changed, 5 insertions(+), 7 deletions(-)
diff --git a/Documentation/Makefile b/Documentation/Makefile
index b6c3285..ec3c9e8 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -66,8 +66,8 @@ clean:
rm -f usage-*.txt
rm -f version.txt
-usage-%.txt: $(MAN1_TXT) usage.sh
- sh ./usage.sh
+usage-guilt-%.txt: ../guilt-% usage.sh
+ sh ./usage.sh $
%.html : %.txt footer.txt version.txt
$(ASCIIDOC) -b xhtml11 -d manpage -f asciidoc.conf $(ASCIIDOC_EXTRA) $
diff --git a/Documentation/usage.sh b/Documentation/usage.sh
index 20fdca4..629f546 100644
--- a/Documentation/usage.sh
+++ b/Documentation/usage.sh
@@ -1,7 +1,5 @@
#!/bin/sh
-for i in `ls ../guilt-*`; do
- name=$(basename $i)
- u=$(grep USAGE $i | sed 's/USAGE=//' | sed 's/$//')
- echo '$name' $u usage-$name.txt
-done
+name=$(basename $1)
+u=$(grep USAGE $1 | sed 's/USAGE=//' | sed 's/$//')
+echo '$name' $u usage-$name.txt
--
2.1.0
--
The reasonable man adapts himself to the world; the unreasonable one
persists in trying to adapt the world to himself. Therefore all progress
depends on the unreasonable man.
- George Bernard Shaw
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [GUILT 1/5] Fix generation of Documentation/usage-%.txt.
On Fri, Jan 23, 2015 at 03:33:03PM +0100, Per Cederqvist wrote:
On Fri, Jan 23, 2015 at 3:21 PM, Jeff Sipek jef...@josefsipek.net wrote:
On Fri, Jan 23, 2015 at 02:24:55PM +0100, Per Cederqvist wrote:
The old rule worked, most of the time, but had several issues:
- It depended on the corresponding guilt-*.txt file, but the usage.sh
script actually reads ../guilt-foo.
- Actually, each usage-%.txt depended on all guilt-*.txt files, so
make had to do more work than necessary if a single file was
altered.
- The construct broke parallel make, which would spawn several
usage.sh at once. This leads to unnecessary work, and could
potentially result in broken usage files if the echo some_string
some_file construct used by usage.sh isn't atomic.
Fixed by letting the usage.sh script update a single file, and writing
a proper implicit make rule. This makes parallel make work a lot
better.
Nice!
There is a small downside, though, as usage.sh will now be run once
for each command (if everything is regenerated). I think it is worth
to pay that price to get the correctness. This command is still very
fast compared to the docbook processing.
Given how much simple usage.sh got, I'm thinking it might be worth it to
just remove it, and just shove the rule into the makefile itself.
Ok, I tried to write it. I came up with the following. (Note: I have *not*
tested it.) It's not *that* ugly.
usage-guilt-%.txt: ../guilt-% usage.sh
echo '$(basename $)' `sed -n -e '/^USAGE=/{s/USAGE=//; s/$//;
p; q}' $` $@
What do you think? Too opaque? Your change looks good.
Too opaque,
Between that and the other patch in the series that modifies usage.sh, your
patch is good as is.
Signed-off-by: Josef 'Jeff' Sipek jef...@josefsipek.net
and not tested enough. It doesn't work, since make will
handle all $. You need to write $$ instead of $ in at least one of the
places. I would stick with usage.sh, as getting the quoting right when
you have make, shell, subshells, and sed all at the same time is just
too painful.
And this is comming from the person that rewrote cmd/shouldfail in a way
that the average shell user will go whaaa?? :P (To be fair, I don't know
of a simpler way to make cmd/shouldfail.)
But it is of course up to you. You are the maintainer. :-)
Heh.
Jeff.
--
Real Programmers consider what you see is what you get to be just as bad a
concept in Text Editors as it is in women. No, the Real Programmer wants a
you asked for it, you got it text editor -- complicated, cryptic,
powerful, unforgiving, dangerous.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [GUILT 1/2] guilt graph: Simplify getfiles.
Neat.
Signed-off-by: Josef 'Jeff' Sipek jef...@josefsipek.net
On Fri, Jan 23, 2015 at 03:21:06PM +0100, Per Cederqvist wrote:
git diff-tree by default emits TAB-separated fields. cut by defaults
processes TAB-separated fields. Simplify getfiles() by using TAB as
the separator.
Signed-off-by: Per Cederqvist ced...@opera.com
---
guilt-graph | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/guilt-graph b/guilt-graph
index 0857e0d..d90c2f1 100755
--- a/guilt-graph
+++ b/guilt-graph
@@ -36,7 +36,7 @@ fi
getfiles()
{
- git diff-tree -r $1^ $1 | tr '\t' ' ' | cut -d' ' -f6
+ git diff-tree -r $1^ $1 | cut -f2
}
cache=$GUILT_DIR/$branch/.graphcache.$$
--
2.1.0
--
C is quirky, flawed, and an enormous success.
- Dennis M. Ritchie.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [GUILT 2/2] Teach guilt graph the -x exclude-pattern option.
On Fri, Jan 23, 2015 at 03:21:07PM +0100, Per Cederqvist wrote:
Some projects keep a ChangeLog which every commit modifies. This
makes the graph a very uninteresting single line of commits. It is
sometimes useful to see how the graph would look if we ignore the
ChangeLog file.
The new -x option is useful in situations like this. It can be
repeated several times to ignore many files. Each argument is saved
to a temporary file and grep -v -f $TEMPORARY is used to filter out
the file names you want to ignore.
Cool idea.
Also added a minimal test case and documentation.
Signed-off-by: Per Cederqvist ced...@opera.com
---
Documentation/guilt-graph.txt | 5 +
guilt-graph | 24 ++--
regression/t-033.out | 12
regression/t-033.sh | 3 +++
4 files changed, 38 insertions(+), 6 deletions(-)
diff --git a/Documentation/guilt-graph.txt b/Documentation/guilt-graph.txt
index f43206e..eeed321 100644
--- a/Documentation/guilt-graph.txt
+++ b/Documentation/guilt-graph.txt
@@ -16,6 +16,11 @@ patches.
OPTIONS
---
+-x pattern::
+ Ignore files that matches the given grep pattern. Can be
+ repeated to ignore several files. This can be useful to ignore
+ for instance ChangeLog files that every commit modifies.
+
patchname::
Instead of starting with the topmost applied patch, start with
patchname.
diff --git a/guilt-graph b/guilt-graph
index d90c2f1..4d5fe46 100755
--- a/guilt-graph
+++ b/guilt-graph
@@ -3,7 +3,7 @@
# Copyright (c) Josef Jeff Sipek, 2007-2013
#
-USAGE=[patchname]
+USAGE=[-x exclude-pattern]... [patchname]
if [ -z $GUILT_VERSION ]; then
echo Invoking `basename $0` directly is no longer supported. 2
exit 1
@@ -11,6 +11,22 @@ fi
_main() {
+cache=$GUILT_DIR/$branch/.graphcache.$$
+xclude=$GUILT_DIR/$branch/.graphexclude.$$
+trap rm -rf \$cache\ \$xclude\ 0
+mkdir $cache
+$xclude
+
+while [ $# -gt 0 ]; do
+if [ $1 = -x ] [ $# -ge 2 ]; then
+ echo $2 $xclude
+ shift
+ shift
+else
+ break
+fi
Spaces used for indentation. Otherwise looks good.
Signed-off-by: Josef 'Jeff' Sipek jef...@josefsipek.net
+done
+
if [ $# -gt 1 ]; then
usage
fi
@@ -39,10 +55,6 @@ getfiles()
git diff-tree -r $1^ $1 | cut -f2
}
-cache=$GUILT_DIR/$branch/.graphcache.$$
-mkdir $cache
-trap rm -rf \$cache\ 0
-
disp digraph G {
current=$top
@@ -66,7 +78,7 @@ while [ $current != $base ]; do
rm -f $cache/dep
touch $cache/dep
- getfiles $current | while read f; do
+ getfiles $current | grep -v -f $xclude | while read f; do
# hash the filename
fh=`echo $f | sha1 | cut -d' ' -f1`
if [ -e $cache/$fh ]; then
diff --git a/regression/t-033.out b/regression/t-033.out
index c120d4f..1ed371f 100644
--- a/regression/t-033.out
+++ b/regression/t-033.out
@@ -88,3 +88,15 @@ digraph G {
ff2775f8d1dc753f635830adcc3a067e0b681e2d [label=a.patch]
891bc14b5603474c9743fd04f3da888644413dc5 -
ff2775f8d1dc753f635830adcc3a067e0b681e2d; // ?
}
+%% The same graph, but excluding deps introduced by file.txt.
+% guilt graph -x file.txt
+digraph G {
+# checking rev bc7df666a646739eaf559af23cab72f2bfd01f0e
+ bc7df666a646739eaf559af23cab72f2bfd01f0e
[label=a-\betterquicker'-patch.patch]
+# checking rev 891bc14b5603474c9743fd04f3da888644413dc5
+ 891bc14b5603474c9743fd04f3da888644413dc5 [label=c.patch]
+# checking rev c7014443c33d2b0237293687ceb9cbd38313df65
+ c7014443c33d2b0237293687ceb9cbd38313df65 [label=b.patch]
+# checking rev ff2775f8d1dc753f635830adcc3a067e0b681e2d
+ ff2775f8d1dc753f635830adcc3a067e0b681e2d [label=a.patch]
+}
diff --git a/regression/t-033.sh b/regression/t-033.sh
index 9fe1827..ae22914 100755
--- a/regression/t-033.sh
+++ b/regression/t-033.sh
@@ -59,3 +59,6 @@ cmd git add file.txt
cmd guilt refresh
fixup_time_info a-\betterquicker'-patch.patch
cmd guilt graph
+
+echo %% The same graph, but excluding deps introduced by file.txt.
+cmd guilt graph -x file.txt
--
2.1.0
--
Computer Science is no more about computers than astronomy is about
telescopes.
- Edsger Dijkstra
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [GUILT 1/5] Fix generation of Documentation/usage-%.txt.
On Fri, Jan 23, 2015 at 02:24:55PM +0100, Per Cederqvist wrote:
The old rule worked, most of the time, but had several issues:
- It depended on the corresponding guilt-*.txt file, but the usage.sh
script actually reads ../guilt-foo.
- Actually, each usage-%.txt depended on all guilt-*.txt files, so
make had to do more work than necessary if a single file was
altered.
- The construct broke parallel make, which would spawn several
usage.sh at once. This leads to unnecessary work, and could
potentially result in broken usage files if the echo some_string
some_file construct used by usage.sh isn't atomic.
Fixed by letting the usage.sh script update a single file, and writing
a proper implicit make rule. This makes parallel make work a lot
better.
Nice!
There is a small downside, though, as usage.sh will now be run once
for each command (if everything is regenerated). I think it is worth
to pay that price to get the correctness. This command is still very
fast compared to the docbook processing.
Given how much simple usage.sh got, I'm thinking it might be worth it to
just remove it, and just shove the rule into the makefile itself.
Ok, I tried to write it. I came up with the following. (Note: I have *not*
tested it.) It's not *that* ugly.
usage-guilt-%.txt: ../guilt-% usage.sh
echo '$(basename $)' `sed -n -e '/^USAGE=/{s/USAGE=//; s/$//; p;
q}' $` $@
What do you think? Too opaque? Your change looks good.
Jeff.
Signed-off-by: Per Cederqvist ced...@opera.com
---
Documentation/Makefile | 4 ++--
Documentation/usage.sh | 8 +++-
2 files changed, 5 insertions(+), 7 deletions(-)
diff --git a/Documentation/Makefile b/Documentation/Makefile
index b6c3285..ec3c9e8 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -66,8 +66,8 @@ clean:
rm -f usage-*.txt
rm -f version.txt
-usage-%.txt: $(MAN1_TXT) usage.sh
- sh ./usage.sh
+usage-guilt-%.txt: ../guilt-% usage.sh
+ sh ./usage.sh $
%.html : %.txt footer.txt version.txt
$(ASCIIDOC) -b xhtml11 -d manpage -f asciidoc.conf $(ASCIIDOC_EXTRA) $
diff --git a/Documentation/usage.sh b/Documentation/usage.sh
index 20fdca4..629f546 100644
--- a/Documentation/usage.sh
+++ b/Documentation/usage.sh
@@ -1,7 +1,5 @@
#!/bin/sh
-for i in `ls ../guilt-*`; do
- name=$(basename $i)
- u=$(grep USAGE $i | sed 's/USAGE=//' | sed 's/$//')
- echo '$name' $u usage-$name.txt
-done
+name=$(basename $1)
+u=$(grep USAGE $1 | sed 's/USAGE=//' | sed 's/$//')
+echo '$name' $u usage-$name.txt
--
2.1.0
--
The reasonable man adapts himself to the world; the unreasonable one
persists in trying to adapt the world to himself. Therefore all progress
depends on the unreasonable man.
- George Bernard Shaw
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[GUILT 0/2] Teach guilt graph to ignore some files.
If you use a ChangeLog, all output from guilt graph will be a boring line of commits. By using guilt graph -x ChangeLog things will look more interesting. Also: simplify getfiles. (This work is also available on the guilt-graph-ignore-2015-v1 branch of the git://repo.or.cz/guilt/ceder.git repository. (That branch is based on the doc-dash-2015-v1 branch that contains my documentation fixes, so if you just want these two commits you will have to cherry-pick.)) /ceder Per Cederqvist (2): guilt graph: Simplify getfiles. Teach guilt graph the -x exclude-pattern option. Documentation/guilt-graph.txt | 5 + guilt-graph | 26 +++--- regression/t-033.out | 12 regression/t-033.sh | 3 +++ 4 files changed, 39 insertions(+), 7 deletions(-) -- 2.1.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: [GUILT 2/5] doc: guilt.xml depends on cmds.txt.
Signed-off-by: Josef 'Jeff' Sipek jef...@josefsipek.net On Fri, Jan 23, 2015 at 02:24:56PM +0100, Per Cederqvist wrote: Specify an explicit dependency, to stop make from trying to generate guilt.xml if cmds.txt could not be created. The asciidoc will fail and produce an error message that might hide the original error message. The added dependency causes make to not remove the guilt.xml file. Add *.xml to .gitignore. Signed-off-by: Per Cederqvist ced...@opera.com --- Documentation/.gitignore | 3 +++ Documentation/Makefile | 2 ++ 2 files changed, 5 insertions(+) diff --git a/Documentation/.gitignore b/Documentation/.gitignore index c4f0588..9b8d4da 100644 --- a/Documentation/.gitignore +++ b/Documentation/.gitignore @@ -11,3 +11,6 @@ version.txt # Generated file dependency list doc.dep + +# Intermediate generated files +*.xml diff --git a/Documentation/Makefile b/Documentation/Makefile index ec3c9e8..2574125 100644 --- a/Documentation/Makefile +++ b/Documentation/Makefile @@ -60,6 +60,8 @@ cmds.txt: cmd-list.sh $(MAN1_TXT) guilt.7 guilt.html: guilt.txt footer.txt version.txt +guilt.xml: cmds.txt + clean: rm -f *.xml *.html *.1 *.7 doc.dep rm -f cmds.txt -- 2.1.0 -- The obvious mathematical breakthrough would be development of an easy way to factor large prime numbers. - Bill Gates, The Road Ahead, pg. 265 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [GUILT 4/5] doc: don't use guilt-foo invocations in usage messages.
Ah, I see you changed usage.sh here. I guess that kinda invalidates my comment for patch 1/5. On Fri, Jan 23, 2015 at 02:24:58PM +0100, Per Cederqvist wrote: Signed-off-by: Per Cederqvist ced...@opera.com --- Documentation/usage.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/usage.sh b/Documentation/usage.sh index 629f546..9cc49f7 --- a/Documentation/usage.sh +++ b/Documentation/usage.sh @@ -2,4 +2,4 @@ name=$(basename $1) u=$(grep USAGE $1 | sed 's/USAGE=//' | sed 's/$//') -echo '$name' $u usage-$name.txt +echo '`echo $name|sed -e 's/^guilt-/guilt /'`' $u usage-$name.txt Tiny nitpick: spaces around the |, otherwise looks good. Signed-off-by: Josef 'Jeff' Sipek jef...@josefsipek.net -- 2.1.0 -- Si hoc legere scis nimium eruditionis habes. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [GUILT 5/5] doc: git doesn't use git-foo invocations.
Signed-off-by: Josef 'Jeff' Sipek jef...@josefsipek.net On Fri, Jan 23, 2015 at 02:24:59PM +0100, Per Cederqvist wrote: Make them into reference to the man pages instead. Signed-off-by: Per Cederqvist ced...@opera.com --- Documentation/guilt-add.txt | 2 +- Documentation/guilt-refresh.txt | 2 +- Documentation/guilt-rm.txt | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Documentation/guilt-add.txt b/Documentation/guilt-add.txt index a276f09..067b6ca 100644 --- a/Documentation/guilt-add.txt +++ b/Documentation/guilt-add.txt @@ -11,7 +11,7 @@ include::usage-guilt-add.txt[] DESCRIPTION --- -Adds the files specified to git using git-add making it available to guilt. +Adds the files specified to git using git-add(1) making it available to guilt. OPTIONS --- diff --git a/Documentation/guilt-refresh.txt b/Documentation/guilt-refresh.txt index 7757bdc..98076e3 100644 --- a/Documentation/guilt-refresh.txt +++ b/Documentation/guilt-refresh.txt @@ -23,7 +23,7 @@ OPTIONS Include a diffstat output in the patch file. Useful for cases where patches will be submitted with other tools. + -If the command line option is omitted, the corresponding git-config +If the command line option is omitted, the corresponding git-config(1) option guilt.diffstat will be queried. So this would enable diffstat output by default: diff --git a/Documentation/guilt-rm.txt b/Documentation/guilt-rm.txt index 71b49fe..cfe471e 100644 --- a/Documentation/guilt-rm.txt +++ b/Documentation/guilt-rm.txt @@ -11,7 +11,7 @@ include::usage-guilt-rm.txt[] DESCRIPTION --- -Removes the files specified from git using git-rm +Removes the files specified from git using git-rm(1). OPTIONS --- -- 2.1.0 -- The obvious mathematical breakthrough would be development of an easy way to factor large prime numbers. - Bill Gates, The Road Ahead, pg. 265 -- To unsubscribe from this list: send the line unsubscribe 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 compile warnings (under mac/clang)
Hi Peff,
On 2015-01-23 13:23, Jeff King wrote:
On Fri, Jan 23, 2015 at 12:48:29PM +0100, Johannes Schindelin wrote:
Pointed out by Michael Blume. Jeff King provided the pointer to a commit
fixing the same issue elsewhere in the Git source code.
It may be useful to reference the exact commit (3ce3ffb8) to help people
digging in the history (e.g., if we decide there is a better way to shut
up this warning and we need to find all the places to undo the
brain-damage).
Good point, thanks!
-for (i = 0; i FSCK_MSG_MAX; i++) {
+for (i = FSCK_MSG_MIN + 1; i FSCK_MSG_MAX; i++) {
Ugh. It is really a shame how covering up this warning requires
polluting so many places. I don't think we have a better way, though,
aside from telling people to use -Wno-tautological-compare (and I can
believe that it _is_ a useful warning in some other circumstances, so it
seems a shame to lose it).
Unless we are willing to drop the = 0 check completely. I think it is
valid to do so regardless of the compiler's representation decision due
to the numbering rules I mentioned above. It kind-of serves as a
cross-check that we haven't cast some random int into the enum, but I
think we would do better to find those callsites (since they are not
guaranteed to work, anyway; in addition to signedness, it might choose a
much smaller representation).
Yeah, well, this check is really more of a safety net in case I messed up
anything; I was saved so many times by my own defensive programming that I try
to employ it as much as I can.
But it does complicate the papering over Clang's overzealous warning, so I
could live with removing the check altogether.
On the other hand, I could do something even easier:
-- snip --
diff --git a/fsck.c b/fsck.c
index 15cb8bd..8f8c82f 100644
--- a/fsck.c
+++ b/fsck.c
@@ -107,7 +107,7 @@ static int fsck_msg_severity(enum fsck_msg_id msg_id,
{
int severity;
- if (options-msg_severity msg_id = 0 msg_id FSCK_MSG_MAX)
+ if (options-msg_severity ((unsigned int) msg_id) FSCK_MSG_MAX)
severity = options-msg_severity[msg_id];
else {
severity = msg_id_info[msg_id].severity;
-- snap --
What do you think? Michael, does this cause more Clang warnings, or would it
resolve the issue?
I do not see either side of the bounds check here:
+if (options-msg_severity
+msg_id FSCK_MSG_MIN msg_id FSCK_MSG_MAX)
as really doing anything. Any code which triggers it must already cause
undefined behavior, I think (with the exception of msg_id == FSCK_MSG_MAX,
but presumably that is something we never expect to happen, either).
Yep, it should not be triggered at all, but as I said, it is a nice defensive
programming measure to avoid segmentation faults in case of a bug.
Ciao,
Dscho
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
