Re: [PATCH 7/8] combine-diff: Fast changed-to-all-parents paths scanning
Kirill Smelkov k...@mns.spb.ru writes: As was recently shown (c839f1bd combine-diff: optimize combine_diff_path sets intersection), combine-diff runs very slowly. In that commit we optimized paths sets intersection, but that accounted only for ~ 25% of the slowness, and as my tracing showed, for linux.git v3.10..v3.11, for merges a lot of time is spent computing diff(commit,commit^2) just to only then intersect that huge diff to almost small set of files from diff(commit,commit^1). That's because at present, to compute combine-diff, for first finding paths, that every parent touches, we use the following combine-diff property/definition: D(A,P1...Pn) = D(A,P1) ^ ... ^ D(A,Pn) (w.r.t. paths) where D(A,P1...Pn) is combined diff between commit A, and parents Pi and D(A,Pi) is usual two-tree diff Pi..A and A ^ B means what??? I do like the approach of walking the tree entries and stop as shallowly as possible without recursing. -- To unsubscribe from this list: send the line unsubscribe 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/8] combine-diff: Fast changed-to-all-parents paths scanning
Junio C Hamano gits...@pobox.com writes: Kirill Smelkov k...@mns.spb.ru writes: As was recently shown (c839f1bd combine-diff: optimize combine_diff_path sets intersection), combine-diff runs very slowly. In that commit we optimized paths sets intersection, but that accounted only for ~ 25% of the slowness, and as my tracing showed, for linux.git v3.10..v3.11, for merges a lot of time is spent computing diff(commit,commit^2) just to only then intersect that huge diff to almost small set of files from diff(commit,commit^1). That's because at present, to compute combine-diff, for first finding paths, that every parent touches, we use the following combine-diff property/definition: D(A,P1...Pn) = D(A,P1) ^ ... ^ D(A,Pn) (w.r.t. paths) where D(A,P1...Pn) is combined diff between commit A, and parents Pi and D(A,Pi) is usual two-tree diff Pi..A and A ^ B means what??? Silly me; obviously it is the set intersection operator. We probably could instead use the current set of paths as literal pathspec to compute subsequent paths, i.e. D(A,Pi,PS) is two tree diff P1..A limited to paths PS D(A,P1...Pn) = D(A,P1,[]) ^ D(A,P2,D(A,P1,[])) ^ ... D(A,Pn,D(A,P1...Pn-1)) if we did not want to reinvent the whole tree walking thing, which would add risks for new bugs and burden to maintain this and the usual two-tree diff tree walking in sync. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] howto/maintain-git.txt: new version numbering scheme
Philip Oakley philipoak...@iee.org writes: If we are progressing from V1.9 to V2.0 quickly (one cycle?), which I understand is the plan, then mixing the minor development items (patch series which progress to master) with the maintenance fixes over the next few months, thus only having 1.9.x releases, sounds reasonable. If there is going to be separate maintenance fixes from the patch series developments then keeping to the previous 1.9.x.y for maintenance would be better. Will the new rapid counting continue after V2.0, such that we get to V2.9 - V3.0 rather more quickly than V1.0 - V2.0 ? The key discriminator would be to say when V2.0 will be out for deciding the V1.9 sequence. I do not quite follow. The time distance between v1.9 and v2.0 should not affect anything. If it is a long road, there may be v1.10, v1.11, v1.12, ... before we have v2.0. If not, v2.0 may immediately follow v1.9 as a new feature release. There may be maintenance releases based on v1.9 that does not add any new features. Right now, if you count the maintenance releases, there are potentially four kinds of version gaps: - Between v1.8.5 and v1.8.5.1, there are fixes but no new features; - Between v1.8.5 and v1.8.6, there are new features but no compatibility worries; - Between v1.8.6 and v1.9.0, there are new features, no compatibility worries, but somehow the jump is larger than the one between v1.8.5 and v1.8.6; and - Between v1.9.0 and v2.0.0, there are new features and also compatibility concerns. Switching to 2-digit scheme and calling the upcoming one v1.9 (and the next major one v2.0) was meant to make the naming more flat, as the third item in the above list somehow the jump is larger does not seem to add much value to the end users. So the logical numbering becomes more like this: - Between v1.9 and v1.9.1, there are fixes but no new features; - Between v1.9.x and v1.10, there are new features but no compatibility worries; - Between v1.9.x and v2.0, there are new features and also compatibility concerns. With a twist, though. There seem to be many places where at least three digits are assumed to exist in our version numbers, so in order to make life easier, the updated document says vX.Y (a feature release) will identify itself as vX.Y.0 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 7/8] combine-diff: Fast changed-to-all-parents paths scanning
Kirill Smelkov k...@mns.spb.ru writes: + parents_sha1 = xmalloc(nparent * sizeof(parents_sha1[0])); + for (i = 0; i nparent; i++) + parents_sha1[i] = parents-sha1[i]; + + /* fake list head, so worker can assume it is non-NULL */ + struct combine_diff_path paths_head; decl-after-statement; please fix. + paths_head.next = NULL; + + struct strbuf base; likewise. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 5/5] setup: Don't dereference in-tree symlinks for absolute paths
Martin Erik Werner martinerikwer...@gmail.com writes:
Then it seems like one could get rid of npath completely:
Yes. And you need to remove its definition as well to avoid unused
variable warning.
Will queue with an obvious fix-up.
Thanks.
diff --git a/setup.c b/setup.c
index 230505c..dd120cd 100644
--- a/setup.c
+++ b/setup.c
@@ -88,21 +88,17 @@ char *prefix_path_gently(const char *prefix, int len,
if (is_absolute_path(orig)) {
char *npath;
- npath = xmalloc(strlen(path) + 1);
+ sanitized = xmalloc(strlen(path) + 1);
if (remaining_prefix)
*remaining_prefix = 0;
- if (normalize_path_copy_len(npath, path, remaining_prefix)) {
- free(npath);
+ if (normalize_path_copy_len(sanitized, path, remaining_prefix))
{
+ free(sanitized);
return NULL;
}
- if (abspath_part_inside_repo(npath)) {
- free(npath);
+ if (abspath_part_inside_repo(sanitized)) {
+ free(sanitized);
return NULL;
}
-
- sanitized = xmalloc(strlen(npath) + 1);
- strcpy(sanitized, npath);
- free(npath);
} else {
sanitized = xmalloc(len + strlen(path) + 1);
if (len)
at the cost of 'sanitized' always being the length of path, regardless
if it's shorter, or even a NUL string.
--
Martin Erik Werner martinerikwer...@gmail.com
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] fast-import.c: always honor the filename case
Torsten Bögershausen tbo...@web.de writes: [] So to summarize, when fast-import uses strncmp_icase (what fast-import does now) import on a repository where ignorecase=true is wrong. My patch, fast-import.c: always honor the filename case fixes this. Can you verify? Thanks in advance, Reuben Yes, I can verify. My feeling is that a) the fast-export should generate the rename the other way around. Would that be feasable ? Or generate a real rename ? (I'm not using fast-export or import myself) I do not think this matters. Or at least, it should not matter. As Peff said in the nearby message, core.ignorecase is completely about the _filesystem_, and that git should generally be case-sensitive internally. And fast-import is about reading internal representation of paths and data to populate the repository, without having to guess what pathnames were meant to be used---the guess is only needed if we need to consult the filesystem that loses case information. The change made by 50906e04 (Support case folding in git fast-import when core.ignorecase=true, 2010-10-03) should probably be half reverted by making the case-squashing an optional feature, that could be used even on a system with case sensitive filesystems. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
What's cooking in git.git (Feb 2014, #01; Mon, 3)
Here are the topics that have been cooking. Commits prefixed with
'-' are only in 'pu' (proposed updates) while commits prefixed with
'+' are in 'next'.
The tip of 'master' is at 1.9-rc2.
You can find the changes described here in the integration branches
of the repositories listed at
http://git-blame.blogspot.com/p/git-public-repositories.html
--
[New Topics]
* bs/stdio-undef-before-redef (2014-01-31) 1 commit
(merged to 'next' on 2014-01-31 at 9874918)
+ git-compat-util.h: #undef (v)snprintf before #define them
When we replace broken macros from stdio.h in git-compat-util.h,
#undef them to avoid re-definition warnings from the C
preprocessor.
Will cook in 'next'.
* ep/varscope (2014-01-31) 7 commits
(merged to 'next' on 2014-01-31 at d198f5d)
+ builtin/gc.c: reduce scope of variables
+ builtin/fetch.c: reduce scope of variable
+ builtin/commit.c: reduce scope of variables
+ builtin/clean.c: reduce scope of variable
+ builtin/blame.c: reduce scope of variables
+ builtin/apply.c: reduce scope of variables
+ bisect.c: reduce scope of variable
Shrink lifetime of variables by moving their definitions to an
inner scope where appropriate.
Will cook in 'next'.
* mw/symlinks (2014-02-03) 5 commits
- setup: don't dereference in-tree symlinks for absolute paths
- setup: add 'abspath_part_inside_repo' function
- t0060: add tests for prefix_path when path begins with work tree
- t0060: add test for prefix_path when path == work tree
- t0060: add test for manipulating symlinks via absolute paths
All subcommands that take pathspecs mishandled an in-tree symbolic
link when given it as a full path from the root (which arguably is
a sick way to use pathspecs). git ls-files -s $(pwd)/RelNotes in
our tree is an easy reproduction recipe.
We may want to add tests to illustrate symptoms that are visible to
the end user, but the updated code looked reasonable.
Will merge to 'next'.
* ks/diff-c-with-diff-order-more (2014-02-03) 5 commits
- combine-diff: move changed-paths scanning logic into its own function
- combine-diff: move show_log_first logic/action out of paths scanning
- tree-diff: no need to pass match to skip_uninteresting()
- tree-diff: no need to manually verify that there is no mode change for a path
- tests: add checking that combine-diff emits only correct paths
(this branch uses ks/diff-c-with-diff-order.)
By avoiding running full two-way diff between the resulting
revision and each of its N parents, combine-diff can be sped up
significantly.
Not quite sure if we want another custom tree walker for it, or it
should be written by using existing two-way diff with the result of
earlier intersect_path() as pathspec.
--
[Stalled]
* jk/color-for-more-pagers (2014-01-17) 4 commits
- pager: disable colors for some known-bad configurations
- DONOTMERGE: needs matching change to git-sh-setup
- setup_pager: set MORE=R
- setup_pager: refactor LESS/LV environment setting
'more' implementation of BSD wants to be told with MORE=R
environment before it shows colored output, while 'more' on some
other platforms will die when seeing MORE=R environment.
It appears that we are coming to the consensus that trying to be
too intimately knowledgeable about quirks of various pager
implementations on different platforms is a losing proposition.
Waiting for a reroll.
* po/everyday-doc (2014-01-27) 1 commit
- Make 'git help everyday' work
This may make the said command to emit something, but the source is
not meant to be formatted into a manual pages to begin with, and
also its contents are a bit stale. It may be a good first step in
the right direction, but needs more work to at least get the
mark-up right before public consumption.
Will hold.
* jk/branch-at-publish-rebased (2014-01-17) 5 commits
- t1507 (rev-parse-upstream): fix typo in test title
- implement @{publish} shorthand
- branch_get: provide per-branch pushremote pointers
- branch_get: return early on error
- sha1_name: refactor upstream_mark
Give an easier access to the tracking branches from other side in
a triangular workflow by introducing B@{publish} that works in a
similar way to how B@{upstream} does.
Will hold.
* rb/merge-prepare-commit-msg-hook (2014-01-10) 4 commits
- merge: drop unused arg from abort_commit method signature
- merge: make prepare_to_commit responsible for write_merge_state
- t7505: ensure cleanup after hook blocks merge
- t7505: add missing
Expose more merge states (e.g. $GIT_DIR/MERGE_MODE) to hooks that
run during git merge. The log message stresses too much on one
hook, prepare-commit-msg, but it would equally apply to other hooks
like post-merge, I think.
Waiting for a reroll.
* jl/submodule-recursive-checkout (2013-12-26) 5 commits
- Teach checkout to recursively checkout submodules
- submodule: teach unpack_trees() to
Re: [PATCH v5 4/5] setup: Add 'abspath_part_inside_repo' function
Martin Erik Werner martinerikwer...@gmail.com writes: Will you add that test or should I place it in the series with you as author? Either is fine. Thanks. On Mon, Feb 03, 2014 at 01:00:48PM -0800, Junio C Hamano wrote: Martin Erik Werner martinerikwer...@gmail.com writes: The path being exactly equal to the work tree is handled separately, since then there is no directory separator between the work tree and in-repo part. What is an in-repo part? Whatever it is, I am not sure if I follow that logic. After the while (*path) loop checks each level, you check the whole path---wouldn't that code handle that special case already? Given /dir1/repo/dir2/file I've used 'in-repo' to refer to dir2/file, sometimes interchangably with part inside the work tree which might be better terminology, and should replace it? Yes, inside the working tree is much clearer than inside repo, because the word repository often is used to mean only the stuff inside $GIT_DIR, i.e. what controls the working tree files. I was trying to convey that if path is simply /dir/repo, then the while loop method of replacing a '/' and checking from the beginning won't work for the last level, since it has no terminating '/' to replace, so hence it's a special case, mentioning the part inside the work tree is arguably confusing in that case, since there isn't really one, maybe it should be left out completely, since the check each level explanation covers it already? I dunno about the explanation, but it still looks strange to have the special case to deal with /dir/repo before you enter the while loop, and then also have code immediately after the loop that seems to handle the same case. Isn't the latter one redundant? Because it is probably the normal case not to have any symbolic link in the leading part (hence not having to dereference them), I think checking is work_tree[] a prefix of path[] early is justified from performance point of view, though. /* + * No checking if the path is in fact an absolute path is done, and it must + * already be normalized. This is not quite parsable to me. Hmm, what about The input parameter must contain an absolute path, and it must already be normalized. OK. + const char* work_tree = get_git_work_tree(); + if (!work_tree) + return -1; + wtlen = strlen(work_tree); + len = strlen(path); I expect that this is called from a callsite that _knows_ how long path[] is. Shouldn't len be a parameter to this function instead? Currently, it actually doesn't, since 'normalize_path_copy_len' is called on it prior, which can mess with the string length. OK, strlen() here is perfectly fine, then. Hmph How do our callers treat an empty path? In other words, should these three be equivalent? cd /a git ls-files /a cd /a git ls-files cd /a git ls-files . Here I have only gone by the assumption that previous code did the right thing,... Good to know. And the answer to the above question I think is yes, these should be equivalent. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC 2/2] receive-pack: hint that the user can stop git push at auto gc time
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes: Housekeeping jobs like auto gc generally should not get in the way. Users who are pushing may not want to wait until auto gc is done on the server. Give a hint for those users that it's safe now to break git push and stop waiting. Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- This bandage patch may be a good compromise between running auto gc and not annoying users much. If I'm not mistaken, when ^C on git push this way, gc will still be running until it needs to print something out (which it should not normally because of --quiet). The user won't see gc errors, but the user generally can't do much anyway. If you are over local transport, I would think you would kill the both ends. Also, wouldn't killing git push before it is done talking with the receive-pack stop it before it has a chance to update the remote tracking refs to pretend as if it fetched from there immediately after a push? So, no. I do not think we should ever encourage if this bothers you, you can ^C it. Making it not to bother is fine, though. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC 2/2] receive-pack: hint that the user can stop git push at auto gc time
Junio C Hamano gits...@pobox.com writes: Nguyễn Thái Ngọc Duy pclo...@gmail.com writes: Housekeeping jobs like auto gc generally should not get in the way. Users who are pushing may not want to wait until auto gc is done on the server. Give a hint for those users that it's safe now to break git push and stop waiting. Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- This bandage patch may be a good compromise between running auto gc and not annoying users much. If I'm not mistaken, when ^C on git push this way, gc will still be running until it needs to print something out (which it should not normally because of --quiet). The user won't see gc errors, but the user generally can't do much anyway. If you are over local transport, I would think you would kill the both ends. Also, wouldn't killing git push before it is done talking with the receive-pack stop it before it has a chance to update the remote tracking refs to pretend as if it fetched from there immediately after a push? So, no. I do not think we should ever encourage if this bothers you, you can ^C it. Making it not to bother is fine, though. Instead of adding a boolean --break-ok that is hidden, why not adding an exposed boolean --daemonize, and let auto-gc run in the background? With the recent do not let more than one gc run at the same time, that should give a lot more pleasant end user experience, 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 7/8] combine-diff: Fast changed-to-all-parents paths scanning
Kirill Smelkov k...@mns.spb.ru writes: if we did not want to reinvent the whole tree walking thing, which would add risks for new bugs and burden to maintain this and the usual two-tree diff tree walking in sync. Junio, I understand it is not good to duplicate code and increase maintenance risks Instead I propose we switch to the new tree walker completely. I am not fundamentally opposed to the idea. We'll see what happens. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-tag.txt: commit for --contains is optional
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes: This goes far back to e84fb2f (branch --contains: default to HEAD - 2008-07-08) where the same parsing code is shared with builtin/tag.c. git-branch.txt correctly states that commit for --contains is optional while git-tag.txt does not. Correct it. Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- Documentation/git-tag.txt | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt index c418c44..404257d 100644 --- a/Documentation/git-tag.txt +++ b/Documentation/git-tag.txt @@ -103,8 +103,9 @@ OPTIONS + This option is only applicable when listing tags without annotation lines. ---contains commit:: - Only list tags which contain the specified commit. +--contains [commit]:: + Only list tags which contain the specified commit (HEAD if not + specified). Thanks. --points-at object:: Only list tags of the given object. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] reset: support --mixed --intent-to-add mode
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes:
@@ -128,13 +129,20 @@ static void update_index_from_diff(struct
diff_queue_struct *q,
one-path);
add_cache_entry(ce, ADD_CACHE_OK_TO_ADD |
ADD_CACHE_OK_TO_REPLACE);
+ } else if (*intent_to_add) {
+ int pos = cache_name_pos(one-path, strlen(one-path));
+ if (pos 0)
+ die(_(%s does not exist in index),
+ one-path);
+ set_intent_to_add(the_index, active_cache[pos]);
While I do not have any problem with adding an optional keep lost
paths as intent-to-add entries feature, I am not sure why this has
to be so different from the usual add-cache-entry codepath. The
if/elseif chain you are touching inside this loop does:
- If the tree you are resetting to has something at the path
(which is different from the current index, obviously), create
a cache entry to represent that state from the tree and stuff
it in the index;
- Otherwise, the tree you are resetting to does not have that
path. We used to say remove it from the index, but now we have
an option to instead add it as an intent-to-add entry.
So, why doesn't the new codepath do exactly the same thing as the
first branch of the if/else chain and call add_cache_entry but with
a ce marked with CE_INTENT_TO_ADD? That would parallel what happens
in git add -N better, I would think, 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 v6 5/6] setup: Add 'abspath_part_inside_repo' function
Martin Erik Werner martinerikwer...@gmail.com writes: + const char* work_tree = get_git_work_tree(); Style: asterisk sticks to the variable, not type. No need to resend---all patches looked reasonable. Thanks, will queue. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 4/6] t0060: Add tests for prefix_path when path begins with work tree
Torsten Bögershausen tbo...@web.de writes: On 2014-02-04 15.25, Martin Erik Werner wrote: t/t0060-path-utils.sh | 10 ++ 1 file changed, 10 insertions(+) diff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh index b8e92e1..c0a14f6 100755 --- a/t/t0060-path-utils.sh +++ b/t/t0060-path-utils.sh @@ -201,6 +201,16 @@ test_expect_success 'prefix_path works with only absolute path to work tree' ' test_cmp expected actual ' +test_expect_success 'prefix_path rejects absolute path to dir with same beginning as work tree' ' +test_must_fail test-path-utils prefix_path prefix $(pwd)a +' + +test_expect_success 'prefix_path works with absolute path to a symlink to work tree having same beginning as work tree' ' +git init repo +ln -s repo repolink +test a = $(cd repo test-path-utils prefix_path prefix $(pwd)/../repolink/a) +' I think we need SYMLINKS here. Yes. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git log history simplification problem
Miklos Vajna vmik...@collabora.co.uk writes: Hi, I was trying to understand the history of a piece of code in LibreOffice and I'm facing a behaviour of git-log which is not something I can explain. I'm not sure if this is a git bug or a user error. ;) Here is the situation: git clone git://anongit.freedesktop.org/libreoffice/core cd core git log --full-history -p -S'mnTitleBarHeight =' sd/source/ui/dlg/PaneDockingWindow.cxx Lack of -m is what I would first suspect when somebody misunderstands merge simplification. I am not saying that will be the issue, but merely pointing out that that is the first thing that jumps at me when I view the above command line. Here the first output I get from git-log is b390fae1706b9c511158a03e4fd61f263be4e511, where you can see that the commit *added* that string. So it should be there on master, I would assume. But then I run: git grep 'mnTitleBarHeight =' sd and it's not there. Am I missing something, as in e.g. even with --full-history git-log does some simplification? Thanks, Miklos -- To unsubscribe from this list: send the line unsubscribe 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] blame.c: prepare_lines should not call xrealloc for every line
David Kastrup d...@gnu.org writes:
Making a single preparation run for counting the lines will avoid memory
fragmentation. Also, fix the allocated memory size which was wrong
when sizeof(int *) != sizeof(int), and would have been too small
for sizeof(int *) sizeof(int), admittedly unlikely.
Signed-off-by: David Kastrup d...@gnu.org
---
builtin/blame.c | 40
1 file changed, 24 insertions(+), 16 deletions(-)
diff --git a/builtin/blame.c b/builtin/blame.c
index e44a6bb..522986d 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -1772,25 +1772,33 @@ static int prepare_lines(struct scoreboard *sb)
{
const char *buf = sb-final_buf;
unsigned long len = sb-final_buf_size;
- int num = 0, incomplete = 0, bol = 1;
+ const char *end = buf + len;
+ const char *p;
+ int *lineno;
+
+ int num = 0, incomplete = 0;
Is there any significance to the blank line between these two
variable definitions?
+
+ for (p = buf;;) {
+ if ((p = memchr(p, '\n', end-p)) == NULL)
+ break;
+ ++num, ++p;
You have a peculiar style that is somewhat distracting. Why isn't
this more like so?
for (p = buf; p++, num++; ) {
p = memchr(p, '\n', end - p);
if (!p)
break;
}
which I think is the prevalent style in our codebase. The same for
the other loop we see in the new code below.
- favor post-increment unless you use it as rvalue and need
pre-increment;
- SP around each binary ops e.g. 'end - p';
- avoid assignments in conditionals when you do not have to.
+ }
- if (len buf[len-1] != '\n')
+ if (len end[-1] != '\n')
incomplete++; /* incomplete line at the end */
OK, so far we counted num complete lines and incomplete may be
one if there is an incomplete line after them.
- while (len--) {
- if (bol) {
- sb-lineno = xrealloc(sb-lineno,
- sizeof(int *) * (num + 1));
- sb-lineno[num] = buf - sb-final_buf;
- bol = 0;
- }
- if (*buf++ == '\n') {
- num++;
- bol = 1;
- }
+
+ sb-lineno = lineno = xmalloc(sizeof(int) * (num + incomplete + 1));
OK, this function is called only once, so we know sb-lineno is NULL
originally and there is no reason to start from xrealloc().
+ for (p = buf;;) {
+ *lineno++ = p-buf;
+ if ((p = memchr(p, '\n', end-p)) == NULL)
+ break;
+ ++p;
}
- sb-lineno = xrealloc(sb-lineno,
- sizeof(int *) * (num + incomplete + 1));
These really *were* unnecessary reallocations.
Thanks for catching them, but this patch needs heavy style fixes.
- sb-lineno[num + incomplete] = buf - sb-final_buf;
+
+ if (incomplete)
+ *lineno++ = len;
+
sb-num_lines = num + incomplete;
return sb-num_lines;
}
--
To unsubscribe from this list: send the line unsubscribe 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] blame.c: prepare_lines should not call xrealloc for every line
David Kastrup d...@gnu.org writes: Whitespace error in line 1778. Should I be reposting? Heh, let me try to clean it up first and then repost for your review. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] blame.c: prepare_lines should not call xrealloc for every line
Junio C Hamano gits...@pobox.com writes:
David Kastrup d...@gnu.org writes:
Whitespace error in line 1778. Should I be reposting?
Heh, let me try to clean it up first and then repost for your
review.
Thanks.
-- 8 --
From: David Kastrup d...@gnu.org
Making a single preparation run for counting the lines will avoid memory
fragmentation. Also, fix the allocated memory size which was wrong
when sizeof(int *) != sizeof(int), and would have been too small
for sizeof(int *) sizeof(int), admittedly unlikely.
Signed-off-by: David Kastrup d...@gnu.org
---
One logic difference from what was posted is that sb-lineno[num]
is filled with the length of the entire buffer when the file ends
with a complete line. I do not remember if the rest of the logic
actually depends on it (I think I use lineno[n+1] - lineno[n] to
find the end of line, so this may matter for the last line),
though.
The original code dates back to 2006 when the author of the code
was not paid for doing anything for Git but was doing it as a
weekend and evening hobby, so it may not be so surprising to find
this kind of what was I thinking when I wrote it inefficiency in
such a code with $0 monetary value ;-)
builtin/blame.c | 37 +
1 file changed, 21 insertions(+), 16 deletions(-)
diff --git a/builtin/blame.c b/builtin/blame.c
index e44a6bb..2364661 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -1772,25 +1772,30 @@ static int prepare_lines(struct scoreboard *sb)
{
const char *buf = sb-final_buf;
unsigned long len = sb-final_buf_size;
- int num = 0, incomplete = 0, bol = 1;
+ const char *end = buf + len;
+ const char *p;
+ int *lineno;
+ int num = 0, incomplete = 0;
+
+ for (p = buf; p end; p++) {
+ p = memchr(p, '\n', end - p);
+ if (!p)
+ break;
+ num++;
+ }
- if (len buf[len-1] != '\n')
+ if (len end[-1] != '\n')
incomplete++; /* incomplete line at the end */
- while (len--) {
- if (bol) {
- sb-lineno = xrealloc(sb-lineno,
- sizeof(int *) * (num + 1));
- sb-lineno[num] = buf - sb-final_buf;
- bol = 0;
- }
- if (*buf++ == '\n') {
- num++;
- bol = 1;
- }
+
+ sb-lineno = lineno = xmalloc(sizeof(int) * (num + incomplete + 1));
+
+ for (p = buf; p end; p++) {
+ *lineno++ = p - buf;
+ p = memchr(p, '\n', end - p);
+ if (!p)
+ break;
}
- sb-lineno = xrealloc(sb-lineno,
- sizeof(int *) * (num + incomplete + 1));
- sb-lineno[num + incomplete] = buf - sb-final_buf;
+ sb-lineno[num + incomplete] = len;
sb-num_lines = num + incomplete;
return sb-num_lines;
}
--
To unsubscribe from this list: send the line unsubscribe 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] blame.c: prepare_lines should not call xrealloc for every line
David Kastrup d...@gnu.org writes: Anybody know offhand what I should be including here? It looks like Git has some fallback definitions of its own, so it's probably not just string.h I should include? In general, no *.c files outside the compatibility layer should include anything #include system-header.h, as there seem to be finicky systems that care about order of inclusion and feature macro definitions, all of which are meant to be handled by including git-compat-util.h as the very first thing. -- To unsubscribe from this list: send the line unsubscribe 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] blame.c: prepare_lines should not call xrealloc for every line
David Kastrup d...@gnu.org writes: Junio C Hamano gits...@pobox.com writes: David Kastrup d...@gnu.org writes: Anybody know offhand what I should be including here? It looks like Git has some fallback definitions of its own, so it's probably not just string.h I should include? In general, no *.c files outside the compatibility layer should include anything #include system-header.h, as there seem to be finicky systems that care about order of inclusion and feature macro definitions, all of which are meant to be handled by including git-compat-util.h as the very first thing. Ok, and that one's not yet in blame.c. Will include, thanks. No, don't. Some well-known *.h files of ourse, most notably cache.h, are known to include compat-util as the first thing, and that is what *.c files typically include. -- To unsubscribe from this list: send the line unsubscribe 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] blame.c: prepare_lines should not call xrealloc for every line
David Kastrup d...@gnu.org writes:
Ok, I now wrote
for (p = buf;; num++, p++) {
p = memchr(p, '\n', end - p);
if (!p)
break;
}
Looks still wrong (perhaps this is a taste issue).
num++ is not loop control, but the real action of this
loop to count lines. It is better left inside.
p++ is loop control, and belongs to the third part of
for(;;).
Isn't the normal continuation condition p end?
so something like
for (p = buf; p end; p++) {
p = find the end of this line
if (!p)
break;
num++;
}
perhaps?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] blame.c: prepare_lines should not call xrealloc for every line
David Kastrup d...@gnu.org writes:
so something like
for (p = buf; p end; p++) {
p = find the end of this line
if (!p)
break;
num++;
}
perhaps?
Would crash on incomplete last line.
Hmph, even with if !p? From your other message:
+ for (p = buf;; num++) {
+ p = memchr(p, '\n', end - p);
+ if (p) {
+ p++;
+ continue;
}
+ break;
+ }
If you flip the if statement around that would be the same as:
+ for (p = buf;; num++) {
+ p = memchr(p, '\n', end - p);
+ if (!p)
break;
p++;
+ }
And with loop action not in the control part, that would be the
same as:
for (p = buf; ; p++) {
p = memchr...
if (!p)
break;
num++;
}
no? Would this crash on incomplete last line?
Ahh, p end in for (p = buf; p end; p++) { is not correct;
you depend on overrunning the end of the buffer to feed length 0 to
memchr and returning NULL inside the loop. So to be equivalent to
your version, the contination condition needs to be
for (p = buf; p = end; p++) {
But that last round of the loop will be no-op, so p end vs p =
end does not make any difference. It is not even strictly
necessary because memchr() limits the scan to end, but it would
still be a good belt-and-suspenders defensive coding practice, I
would think.
+ for (p = buf;;) {
+ *lineno++ = p - buf;
+ p = memchr(p, '\n', end - p);
+ if (p) {
+ p++;
+ continue;
}
+ break;
}
Can we do the same transformation here?
for (p = buf;;) {
*lineno++ = p - buf;
p = memchr...
if (!p)
break;
p++;
}
which is the same as
for (p = buf; ; p++) {
*lineno++ = p - buf;
p = memchr...
if (!p)
break;
}
and the latter has the loop control p++ at where it belongs to. The
post-condition of one iteration of the body of the loop being p
points at the terminating LF of this line, incrementing the pointer
is how the loop control advances the pointer to the beginning of the
next line.
If we wanted to have a belt-and-suspenders safety, we need p =
end here, not p end, because of how p is used when it is past
the last LF. As we want to make these two loops look similar, that
means we should use p = end in the previous loop as well.
This was fun ;-)
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] setup_pager: set MORE=R
Jeff King p...@peff.net writes: But there's another set of people that I was intending to help with the patch, which is people that have set up LESS, and did not necessarily care about the R flag in the past (e.g., for many years before git came along, I set LESS=giM, and never even knew that R existed). Since git comes out of the box these days with color and the pager turned on, that means people with such a setup see broken output from day one. And I think it is Git's fault here, not the user or the packager. I am not particularly itnterested in whose fault it is ;-) If the user sets LESS himself, he knows how to set it (and if he is setting it automatically for all of his sessions, he knows where to do so), and would know better than Git about less, his pager of choice. If he did not know about R and did not see color, that is even better. Now he knows and his update to his LESS settings will let him view colors in outputs from programs that are not Git. So I think there is nothing to be done. But I did want to mention it in case somebody else can come up with some clever solution. :) Sure. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] reset: support --mixed --intent-to-add mode
Junio C Hamano gits...@pobox.com writes:
While I do not have any problem with adding an optional keep lost
paths as intent-to-add entries feature, I am not sure why this has
to be so different from the usual add-cache-entry codepath. The
if/elseif chain you are touching inside this loop does:
- If the tree you are resetting to has something at the path
(which is different from the current index, obviously), create
a cache entry to represent that state from the tree and stuff
it in the index;
- Otherwise, the tree you are resetting to does not have that
path. We used to say remove it from the index, but now we have
an option to instead add it as an intent-to-add entry.
So, why doesn't the new codepath do exactly the same thing as the
first branch of the if/else chain and call add_cache_entry but with
a ce marked with CE_INTENT_TO_ADD? That would parallel what happens
in git add -N better, I would think, no?
In other words, something along this line, perhaps?
-- 8 --
From: Nguyễn Thái Ngọc Duy pclo...@gmail.com
Date: Tue, 4 Feb 2014 09:20:09 +0700
When --mixed is used, entries could be removed from index if the
target ref does not have them. When reset is used in preparation for
commit spliting (in a dirty worktree), it could be hard to track what
files to be added back. The new option --intent-to-add simplifies it
by marking all removed files intent-to-add.
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
Documentation/git-reset.txt | 5 -
builtin/reset.c | 38 ++
cache.h | 1 +
read-cache.c| 4 ++--
t/t7102-reset.sh| 9 +
5 files changed, 42 insertions(+), 15 deletions(-)
diff --git a/Documentation/git-reset.txt b/Documentation/git-reset.txt
index f445cb3..a077ba0 100644
--- a/Documentation/git-reset.txt
+++ b/Documentation/git-reset.txt
@@ -10,7 +10,7 @@ SYNOPSIS
[verse]
'git reset' [-q] [tree-ish] [--] paths...
'git reset' (--patch | -p) [tree-ish] [--] [paths...]
-'git reset' [--soft | --mixed | --hard | --merge | --keep] [-q] [commit]
+'git reset' [--soft | --mixed [-N] | --hard | --merge | --keep] [-q] [commit]
DESCRIPTION
---
@@ -60,6 +60,9 @@ section of linkgit:git-add[1] to learn how to operate the
`--patch` mode.
Resets the index but not the working tree (i.e., the changed files
are preserved but not marked for commit) and reports what has not
been updated. This is the default action.
++
+If `-N` is specified, removed paths are marked as intent-to-add (see
+linkgit:git-add[1]).
--hard::
Resets the index and working tree. Any changes to tracked files in the
diff --git a/builtin/reset.c b/builtin/reset.c
index 6004803..d363bc5 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -116,25 +116,32 @@ static void update_index_from_diff(struct
diff_queue_struct *q,
struct diff_options *opt, void *data)
{
int i;
+ int intent_to_add = *(int *)data;
for (i = 0; i q-nr; i++) {
struct diff_filespec *one = q-queue[i]-one;
- if (one-mode !is_null_sha1(one-sha1)) {
- struct cache_entry *ce;
- ce = make_cache_entry(one-mode, one-sha1, one-path,
- 0, 0);
- if (!ce)
- die(_(make_cache_entry failed for path '%s'),
- one-path);
- add_cache_entry(ce, ADD_CACHE_OK_TO_ADD |
- ADD_CACHE_OK_TO_REPLACE);
- } else
+ int is_missing = !(one-mode !is_null_sha1(one-sha1));
+ struct cache_entry *ce;
+
+ if (is_missing !intent_to_add) {
remove_file_from_cache(one-path);
+ continue;
+ }
+
+ ce = make_cache_entry(one-mode, one-sha1, one-path,
+ 0, 0);
+ if (!ce)
+ die(_(make_cache_entry failed for path '%s'),
+ one-path);
+ if (is_missing)
+ mark_intent_to_add(ce);
+ add_cache_entry(ce, ADD_CACHE_OK_TO_ADD |
ADD_CACHE_OK_TO_REPLACE);
}
}
static int read_from_tree(const struct pathspec *pathspec,
- unsigned char *tree_sha1)
+ unsigned char *tree_sha1,
+ int intent_to_add)
{
struct diff_options opt;
@@ -142,6 +149,7 @@ static int read_from_tree(const struct pathspec *pathspec,
copy_pathspec(opt.pathspec, pathspec);
opt.output_format = DIFF_FORMAT_CALLBACK;
opt.format_callback = update_index_from_diff;
+ opt.format_callback_data = intent_to_add;
if (do_diff_cache(tree_sha1, opt))
return 1;
@@ -258,6 +266,7 @@ int
[Bug] branch.*.merge interpreted too strictly by tracking logic
Start from an empty repository like so:
: gitster track; git init
Initialized empty Git repository in /var/tmp/x/track/.git/
: gitster track/master; git commit --allow-empty -m initial
[master (root-commit) abdcd1c] initial
: gitster track/master; git branch foo
: gitster track/master; git branch bar
: gitster track/master; git commit --allow-empty -m second
[master 78e28f4] second
Now, 'master' has two commits, while 'foo' and 'bar' both are one
commit behind, pointing at 'master^1'.
Let's tell these branches that they are both supposed to be building
on top of 'master'.
: gitster track/master; git config branch.foo.remote .
: gitster track/master; git config branch.foo.merge refs/heads/master
: gitster track/master; git config branch.bar.remote .
: gitster track/master; git config branch.bar.merge master
The difference between the two is that 'foo' spells the @{upstream}
branch in full (which is the recommended practice), while 'bar' is
loose and asks for 'master'.
Let's see what happens on these two branches. First 'foo':
: gitster track/master; git checkout foo
Switched to branch 'foo'
Your branch is behind 'master' by 1 commit, and can be
fast-forwarded.
(use git pull to update your local branch)
: gitster track/foo; git pull
From .
* branchmaster - FETCH_HEAD
Updating abdcd1c..78e28f4
Fast-forward
The 'checkout' correctly reports that 'foo' is building on (local)
'master'. 'pull' works as expected, of course.
Now, here is the bug part. The same thing on 'bar':
: gitster track/foo; git checkout bar
Switched to branch 'bar'
Your branch is based on 'master', but the upstream is gone.
(use git branch --unset-upstream to fixup)
It knows about 'master', but what is the upstream is gone? It is
our local repository and it definitely is not gone.
But 'pull' of course works as expected (this behaviour is older and
stable for a long time since even before @{upstream} was invented).
: gitster track/bar; git pull
From .
* branchmaster - FETCH_HEAD
Updating abdcd1c..78e28f4
Fast-forward
I suspect that the real culprit is somewhere in wt-status.c
: gitster track/bar; git status
On branch bar
Your branch is based on 'master', but the upstream is gone.
(use git branch --unset-upstream to fixup)
nothing to commit, working directory clean
Resolving @{upstream} works just fine for both.
: gitster track/bar; git rev-parse --symbolic-full-name foo@{upstream}
refs/heads/master
: gitster track/bar; git rev-parse --symbolic-full-name bar@{upstream}
refs/heads/master
Thanks.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] setup_pager: set MORE=R
Jeff King p...@peff.net writes: The safest thing would be to turn off auto-color when LESS (or any of the pager environment variables) is set at all (and not worry about whether R is set; only know that _we_ are not setting it, so we cannot count on it). But that would potentially inconvenience a lot of people whose default color would suddenly go away. That is just as safe as disabling color for everybody, isn't it? Half of existing users have LESS with R, and the other half do not have LESS at all. The former will be harmed, the latter will not see any difference. Oh, and then new users who do not know R for LESS will not even notice that Git could support coloured output. Those among them who read manpages and find --color option will then see ESC[33m in their output and we are back to where we started X-. So I think we are already at the safest place. Those who see ESC[33m will know they are missing some good stuff and can ask around, which is better than doing anything else at this point. I think that is the same conclusion as yours, there is nothing to be done. -- To unsubscribe from this list: send the line unsubscribe 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/9] remerge diff proof of concept/RFC
Thomas Rast t...@thomasrast.ch writes: log --remerge-diff: show what the conflict resolution changed Yay ;-) This series explores another angle, which I call remerge diff. It works by re-doing the merge in core, using features from patches 1-3 and 7. Likely that will result in conflicts, which are formatted in the usual way. Then it diffs this remerge against the merge's tree that is recorded in history. Yup, that is the obvious way to recreate what would have been shown to the person who recorded the merge result. I like that approach. So I would welcome comments, and/or better ideas, on the following proposed resolution: * Implement what I described last, to take care of delete/modify conflicts. Naively, I would have thought that - If the recorded result of the merge does not have the path, then show the deletion of the contents relative to the side that modified it. - If the recorded result of the merge has the path, then show the change between the side that modified it and the recorded result. might be more useful. Then we will know what we lost in full (if the recorded result is to delete it), but it is very likely that we won't see anything if the recorded result blindly took what the modifying side left. Comparing with the synthetic stages 2+3 will at least show us there was something funky going on, so your approach would be reasonable in this case. * Punt on more complex conflicts, by removing those files from the index, and emitting a warning about those files instead. Sensible. -- To unsubscribe from this list: send the line 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] repack.c: rename and unlink pack file if it exists
This comment in builtin/repack.c:
First see if there are packs of the same name and if so
if we can move them out of the way (this can happen if we
repacked immediately after packing fully).
When a repo was fully repacked, and is repacked again, we may run
into the situation that new packfiles have the same name as
already existing ones (traditionally packfiles have been named after
the list of names of objects in them, so repacking all the objects
in a single pack would have produced a packfile with the same name).
The logic is to rename the existing ones into filename like
old-XXX, create the new ones and then remove the old- ones.
When something went wrong in the middle, this sequence is rolled
back by renaming the old- files back.
The renaming into old- did not work as designed, because
file_exists() was done on the wrong file name. This could give
problems for a repo on a network share under Windows, as reported by
Jochen Haag zwanzi...@googlemail.com.
Formulate the filenames objects/pack/pack-.{pack,idx} correctly
(the code originally lacked pack- prefix when checking if the file
exists).
Also rename the variables to match what they are used for:
fname for the final name of the new packfile, fname_old for the name
of the existing one, and fname_tmp for the temporary name pack-objects
created the new packfile in.
Signed-off-by: Torsten Bögershausen tbo...@web.de
Signed-off-by: Junio C Hamano gits...@pobox.com
---
* Somehow this came to my private mailbox without Cc to list, so I
am forwarding it.
I think with 1190a1ac (pack-objects: name pack files after
trailer hash, 2013-12-05), repacking the same set of objects may
have less chance of producing colliding names, especially if you
are on a box with more than one core, but it still would be a
good idea to get this part right in the upcoming release.
The bug in the first hunk will cause us not to find any existing
file, even when there is a name clash. And then we try to
immediately the final rename without any provision for rolling
back. The other two hunks are pure noise renaming variables.
I think the patch looks correct, but I'd appreciate a second set
of eyeballs.
Thanks.
builtin/repack.c | 24
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/builtin/repack.c b/builtin/repack.c
index bca7710..3b01353 100644
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -260,7 +260,7 @@ int cmd_repack(int argc, const char **argv, const char
*prefix)
for_each_string_list_item(item, names) {
for (ext = 0; ext 2; ext++) {
char *fname, *fname_old;
- fname = mkpathdup(%s/%s%s, packdir,
+ fname = mkpathdup(%s/pack-%s%s, packdir,
item-string, exts[ext]);
if (!file_exists(fname)) {
free(fname);
@@ -316,33 +316,33 @@ int cmd_repack(int argc, const char **argv, const char
*prefix)
/* Now the ones with the same name are out of the way... */
for_each_string_list_item(item, names) {
for (ext = 0; ext 2; ext++) {
- char *fname, *fname_old;
+ char *fname, *fname_tmp;
struct stat statbuffer;
fname = mkpathdup(%s/pack-%s%s,
packdir, item-string, exts[ext]);
- fname_old = mkpathdup(%s-%s%s,
+ fname_tmp = mkpathdup(%s-%s%s,
packtmp, item-string, exts[ext]);
- if (!stat(fname_old, statbuffer)) {
+ if (!stat(fname_tmp, statbuffer)) {
statbuffer.st_mode = ~(S_IWUSR | S_IWGRP |
S_IWOTH);
- chmod(fname_old, statbuffer.st_mode);
+ chmod(fname_tmp, statbuffer.st_mode);
}
- if (rename(fname_old, fname))
- die_errno(_(renaming '%s' failed), fname_old);
+ if (rename(fname_tmp, fname))
+ die_errno(_(renaming '%s' into '%s' failed),
fname_tmp, fname);
free(fname);
- free(fname_old);
+ free(fname_tmp);
}
}
/* Remove the old- files */
for_each_string_list_item(item, names) {
for (ext = 0; ext 2; ext++) {
- char *fname;
- fname = mkpath(%s/old-pack-%s%s,
+ char *fname_old;
+ fname_old = mkpath(%s/old-%s%s,
packdir,
item-string,
exts[ext
Re: [PATCH] repack.c: rename and unlink pack file if it exists
Junio C Hamano gits...@pobox.com writes:
This comment in builtin/repack.c:
...
Oops; there was supposed to be these lines before anythning else:
From: Torsten Bögershausen tbo...@web.de
Date: Sun Feb 2 16:09:56 2014 +0100
First see if there are packs of the same name and if so
if we can move them out of the way (this can happen if we
repacked immediately after packing fully).
When a repo was fully repacked, and is repacked again, we may run
into the situation that new packfiles have the same name as
already existing ones (traditionally packfiles have been named after
the list of names of objects in them, so repacking all the objects
in a single pack would have produced a packfile with the same name).
The logic is to rename the existing ones into filename like
old-XXX, create the new ones and then remove the old- ones.
When something went wrong in the middle, this sequence is rolled
back by renaming the old- files back.
The renaming into old- did not work as designed, because
file_exists() was done on the wrong file name. This could give
problems for a repo on a network share under Windows, as reported by
Jochen Haag zwanzi...@googlemail.com.
Formulate the filenames objects/pack/pack-.{pack,idx} correctly
(the code originally lacked pack- prefix when checking if the file
exists).
Also rename the variables to match what they are used for:
fname for the final name of the new packfile, fname_old for the name
of the existing one, and fname_tmp for the temporary name pack-objects
created the new packfile in.
Signed-off-by: Torsten Bögershausen tbo...@web.de
Signed-off-by: Junio C Hamano gits...@pobox.com
---
* Somehow this came to my private mailbox without Cc to list, so I
am forwarding it.
I think with 1190a1ac (pack-objects: name pack files after
trailer hash, 2013-12-05), repacking the same set of objects may
have less chance of producing colliding names, especially if you
are on a box with more than one core, but it still would be a
good idea to get this part right in the upcoming release.
The bug in the first hunk will cause us not to find any existing
file, even when there is a name clash. And then we try to
immediately the final rename without any provision for rolling
back. The other two hunks are pure noise renaming variables.
I think the patch looks correct, but I'd appreciate a second set
of eyeballs.
Thanks.
builtin/repack.c | 24
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/builtin/repack.c b/builtin/repack.c
index bca7710..3b01353 100644
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -260,7 +260,7 @@ int cmd_repack(int argc, const char **argv, const char
*prefix)
for_each_string_list_item(item, names) {
for (ext = 0; ext 2; ext++) {
char *fname, *fname_old;
- fname = mkpathdup(%s/%s%s, packdir,
+ fname = mkpathdup(%s/pack-%s%s, packdir,
item-string, exts[ext]);
if (!file_exists(fname)) {
free(fname);
@@ -316,33 +316,33 @@ int cmd_repack(int argc, const char **argv, const char
*prefix)
/* Now the ones with the same name are out of the way... */
for_each_string_list_item(item, names) {
for (ext = 0; ext 2; ext++) {
- char *fname, *fname_old;
+ char *fname, *fname_tmp;
struct stat statbuffer;
fname = mkpathdup(%s/pack-%s%s,
packdir, item-string, exts[ext]);
- fname_old = mkpathdup(%s-%s%s,
+ fname_tmp = mkpathdup(%s-%s%s,
packtmp, item-string, exts[ext]);
- if (!stat(fname_old, statbuffer)) {
+ if (!stat(fname_tmp, statbuffer)) {
statbuffer.st_mode = ~(S_IWUSR | S_IWGRP |
S_IWOTH);
- chmod(fname_old, statbuffer.st_mode);
+ chmod(fname_tmp, statbuffer.st_mode);
}
- if (rename(fname_old, fname))
- die_errno(_(renaming '%s' failed), fname_old);
+ if (rename(fname_tmp, fname))
+ die_errno(_(renaming '%s' into '%s' failed),
fname_tmp, fname);
free(fname);
- free(fname_old);
+ free(fname_tmp);
}
}
/* Remove the old- files */
for_each_string_list_item(item, names) {
for (ext = 0; ext 2; ext++) {
- char *fname;
- fname = mkpath(%s/old-pack-%s%s,
+ char *fname_old
Re: [PATCH 2/2] reset: support --mixed --intent-to-add mode
Duy Nguyen pclo...@gmail.com writes: On Tue, Feb 04, 2014 at 02:25:25PM -0800, Junio C Hamano wrote: Junio C Hamano gits...@pobox.com writes: While I do not have any problem with adding an optional keep lost paths as intent-to-add entries feature, I am not sure why this has to be so different from the usual add-cache-entry codepath. The if/elseif chain you are touching inside this loop does: - If the tree you are resetting to has something at the path (which is different from the current index, obviously), create a cache entry to represent that state from the tree and stuff it in the index; - Otherwise, the tree you are resetting to does not have that path. We used to say remove it from the index, but now we have an option to instead add it as an intent-to-add entry. So, why doesn't the new codepath do exactly the same thing as the first branch of the if/else chain and call add_cache_entry but with a ce marked with CE_INTENT_TO_ADD? That would parallel what happens in git add -N better, I would think, no? In other words, something along this line, perhaps? snip Yes. But you need something like this on top to actually set CE_INTENT_TO_ADD Yes, indeed. I wonder why your new test did not notice it, though ;-) -- 8 -- diff --git a/read-cache.c b/read-cache.c index 325d193..87f1367 100644 --- a/read-cache.c +++ b/read-cache.c @@ -585,6 +585,7 @@ void mark_intent_to_add(struct cache_entry *ce) if (write_sha1_file(, 0, blob_type, sha1)) die(cannot create an empty blob in the object database); hashcpy(ce-sha1, sha1); + ce-ce_flags |= CE_INTENT_TO_ADD; } int add_to_index(struct index_state *istate, const char *path, struct stat *st, int flags) -- 8 -- -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] userdiff: update Ada patterns
Adrian Johnson ajohn...@redneon.com writes: -|[0-9][-+0-9#_.eE] +|[-+]?[0-9][0-9#_.aAbBcCdDeEfF]*([eE][+-]?[0-9_]+)? This would match a lot wider than what I read you said you wanted to match in your previous message. Does -04##4_3_2Ee-9 count as a number, for example, or can we just ignore such syntactically incorrect sequence? Maybe I am misunderstanding the purpose of the word diff regexes. I thought the purpose of the word regex is to split lines into words, not determine what is syntactically correct. I agree that the purpose is former---So you could have just said the latter ;-). Any other nitpick, anybody? Otherwise I'll queue this version. -- To unsubscribe from this list: send the line unsubscribe 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/8] combine-diff: Fast changed-to-all-parents paths scanning
Kirill Smelkov k...@mns.spb.ru writes:
Only, before I clean things up, I'd like to ask - would the following
patch be accepted
8 ---
diff --git a/tree-walk.c b/tree-walk.c
index 79dba1d..4dc86c7 100644
--- a/tree-walk.c
+++ b/tree-walk.c
@@ -37,7 +37,7 @@ static void decode_tree_entry(struct tree_desc *desc, const
char *buf, unsigned
/* Initialize the descriptor entry */
desc-entry.path = path;
- desc-entry.mode = mode;
+ desc-entry.mode = canon_mode(mode);
desc-entry.sha1 = (const unsigned char *)(path + len);
}
diff --git a/tree-walk.h b/tree-walk.h
index ae04b64..ae7fb3a 100644
--- a/tree-walk.h
+++ b/tree-walk.h
@@ -16,7 +16,7 @@ struct tree_desc {
static inline const unsigned char *tree_entry_extract(struct tree_desc
*desc, const char **pathp, unsigned int *modep)
{
*pathp = desc-entry.path;
- *modep = canon_mode(desc-entry.mode);
+ *modep = desc-entry.mode;
return desc-entry.sha1;
}
8 ---
?
Doesn't desc point into and walks over the data we read from the
tree object directly?
We try to keep (tree|commit)-buffer intact and that is done pretty
deliberately. While you are walking a tree or parsing a commit,
somebody else, perhaps called indirectly by a helper function you
call, may call lookup_object() for the same object, get the copy
that has already been read and start using it. This kind of change
will introduce bugs that are hard to debug unless it is done very
carefully (e.g. starting from making tree.buffer into a const pointer
and propagating constness throughout the system), which might not be
worth the pain.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] reset: support --mixed --intent-to-add mode
Junio C Hamano gits...@pobox.com writes: Duy Nguyen pclo...@gmail.com writes: On Tue, Feb 04, 2014 at 02:25:25PM -0800, Junio C Hamano wrote: Junio C Hamano gits...@pobox.com writes: While I do not have any problem with adding an optional keep lost paths as intent-to-add entries feature, I am not sure why this has to be so different from the usual add-cache-entry codepath. The if/elseif chain you are touching inside this loop does: - If the tree you are resetting to has something at the path (which is different from the current index, obviously), create a cache entry to represent that state from the tree and stuff it in the index; - Otherwise, the tree you are resetting to does not have that path. We used to say remove it from the index, but now we have an option to instead add it as an intent-to-add entry. So, why doesn't the new codepath do exactly the same thing as the first branch of the if/else chain and call add_cache_entry but with a ce marked with CE_INTENT_TO_ADD? That would parallel what happens in git add -N better, I would think, no? In other words, something along this line, perhaps? snip Yes. But you need something like this on top to actually set CE_INTENT_TO_ADD Yes, indeed. I wonder why your new test did not notice it, though ;-) ... and the answer turns out to be that it was not testing the right thing. On top of that faulty version, this will fix it. Your suggestion to move CE_INTENT_TO_ADD to mark-intent-to-add makes sense but a caller needs to be adjusted to drop the duplicated flag manipulation. read-cache.c | 3 +-- t/t7102-reset.sh | 6 ++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/read-cache.c b/read-cache.c index 325d193..5b8102a 100644 --- a/read-cache.c +++ b/read-cache.c @@ -584,6 +584,7 @@ void mark_intent_to_add(struct cache_entry *ce) unsigned char sha1[20]; if (write_sha1_file(, 0, blob_type, sha1)) die(cannot create an empty blob in the object database); + ce-ce_flags |= CE_INTENT_TO_ADD; hashcpy(ce-sha1, sha1); } @@ -613,8 +614,6 @@ int add_to_index(struct index_state *istate, const char *path, struct stat *st, ce-ce_namelen = namelen; if (!intent_only) fill_stat_cache_info(ce, st); - else - ce-ce_flags |= CE_INTENT_TO_ADD; if (trust_executable_bit has_symlinks) ce-ce_mode = create_ce_mode(st_mode); diff --git a/t/t7102-reset.sh b/t/t7102-reset.sh index 642920a..bc0846f 100755 --- a/t/t7102-reset.sh +++ b/t/t7102-reset.sh @@ -539,6 +539,12 @@ test_expect_success 'reset -N keeps removed files as intent-to-add' ' echo new-file new-file git add new-file git reset -N HEAD + + tree=$(git write-tree) + git ls-tree $tree new-file actual + expect + test_cmp expect actual + git diff --name-only actual echo new-file expect test_cmp expect actual -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: bash completion patch
Matthieu Moy matthieu@grenoble-inp.fr writes: 乙酸鋰 ch3co...@gmail.com writes: add --recurse-submodules Thanks for the patch, but it cannot be included as-is. Please, read Documentation/SubmittingPatches in Git's source tree. In particular, the signed-off-by part. Also, don't use attachments to send you patches (git send-email can help) and don't forget to Cc Junio if you think your patch is ready for inclusion. Heh, thanks. Everybody seems to think anything they send out to the list is ready for inclusion, so the last part may not be a piece of advice that is practically very useful, though ;-) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/4] Teach diff_tree_sha1() to accept NULL sha1 for empty trees
All four looked sensible; will queue. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 06/13] Makefile: store GIT-* sentinel files in MAKE/
Jeff King p...@peff.net writes:
This patch moves all of the generated GIT-* files into
MAKE/*, with one exception: GIT-VERSION-FILE. This could be
moved along with the rest, but there is a reasonable chance
that there are some out-of-tree scripts that may peek at it
(whereas things like GIT-CFLAGS are purely internal, and we
update all internal references).
OK. We do have internal references to include the version file to
Makefiles in subdirectories, which we could adjust if we wanted to.
I tend to agree that leaving it there would be a safer thing to do,
but at the same time I think it is OK to move it if we wanted to.
The third parties need to adjust and they are capable of adjusting.
Thanks. The changes look good. I do not have a strong opinion on
the name, but I do agree a separate directory unclutters things in a
very good way.
Signed-off-by: Jeff King p...@peff.net
---
I'm not married to the name MAKE, but I do think the separate
directory is a win for simplicity and avoiding repetition (as you can
see in the diffstat). Other suggestions welcome.
.gitignore| 8 ---
MAKE/.gitignore | 1 +
Makefile | 66
+--
perl/Makefile | 10
t/perf/run| 2 +-
t/test-lib.sh | 2 +-
t/valgrind/analyze.sh | 4 ++--
7 files changed, 42 insertions(+), 51 deletions(-)
create mode 100644 MAKE/.gitignore
diff --git a/.gitignore b/.gitignore
index b5f9def..586a067 100644
--- a/.gitignore
+++ b/.gitignore
@@ -1,11 +1,3 @@
-/GIT-BUILD-OPTIONS
-/GIT-CFLAGS
-/GIT-LDFLAGS
-/GIT-PREFIX
-/GIT-PERL-DEFINES
-/GIT-PYTHON-VARS
-/GIT-SCRIPT-DEFINES
-/GIT-USER-AGENT
/GIT-VERSION-FILE
/bin-wrappers/
/git
diff --git a/MAKE/.gitignore b/MAKE/.gitignore
new file mode 100644
index 000..72e8ffc
--- /dev/null
+++ b/MAKE/.gitignore
@@ -0,0 +1 @@
+*
diff --git a/Makefile b/Makefile
index 60dc53b..7fecdf1 100644
--- a/Makefile
+++ b/Makefile
@@ -1564,10 +1564,10 @@ endif
# usage: $(eval $(call make-var,FN,DESC,CONTENTS))
#
# Create a rule to write $CONTENTS (which should come from a make variable)
-# to GIT-$FN, but only if not already there. This can be used to create a
+# to MAKE/$FN, but only if not already there. This can be used to create a
# dependency on a Makefile variable. Prints $DESC to the user.
define make-var
-GIT-$1: FORCE
+MAKE/$1: FORCE
@VALUE='$$(subst ','\'',$3)'; \
if test xVALUE != x`cat $$@ 2/dev/null`; then \
echo 2 * new $2; \
@@ -1658,7 +1658,7 @@ all:: profile-clean
endif
endif
-all:: $(ALL_PROGRAMS) $(SCRIPT_LIB) $(BUILT_INS) $(OTHER_PROGRAMS)
GIT-BUILD-OPTIONS
+all:: $(ALL_PROGRAMS) $(SCRIPT_LIB) $(BUILT_INS) $(OTHER_PROGRAMS)
MAKE/BUILD-OPTIONS
ifneq (,$X)
$(QUIET_BUILT_IN)$(foreach p,$(patsubst %$X,%,$(filter
%$X,$(ALL_PROGRAMS) $(BUILT_INS) git$X)), test -d '$p' -o '$p' -ef '$p$X' ||
$(RM) '$p';)
endif
@@ -1714,25 +1714,25 @@ strip: $(PROGRAMS) git$X
# dependencies here will not need to change if the force-build
# details change some day.
-git.sp git.s git.o: GIT-PREFIX
+git.sp git.s git.o: MAKE/PREFIX
git.sp git.s git.o: EXTRA_CPPFLAGS = \
'-DGIT_HTML_PATH=$(htmldir_relative_SQ)' \
'-DGIT_MAN_PATH=$(mandir_relative_SQ)' \
'-DGIT_INFO_PATH=$(infodir_relative_SQ)'
-git$X: git.o GIT-LDFLAGS $(BUILTIN_OBJS) $(GITLIBS)
+git$X: git.o MAKE/LDFLAGS $(BUILTIN_OBJS) $(GITLIBS)
$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ git.o \
$(BUILTIN_OBJS) $(ALL_LDFLAGS) $(LIBS)
help.sp help.s help.o: common-cmds.h
-builtin/help.sp builtin/help.s builtin/help.o: common-cmds.h GIT-PREFIX
+builtin/help.sp builtin/help.s builtin/help.o: common-cmds.h MAKE/PREFIX
builtin/help.sp builtin/help.s builtin/help.o: EXTRA_CPPFLAGS = \
'-DGIT_HTML_PATH=$(htmldir_relative_SQ)' \
'-DGIT_MAN_PATH=$(mandir_relative_SQ)' \
'-DGIT_INFO_PATH=$(infodir_relative_SQ)'
-version.sp version.s version.o: GIT-VERSION-FILE GIT-USER-AGENT
+version.sp version.s version.o: GIT-VERSION-FILE MAKE/USER-AGENT
version.sp version.s version.o: EXTRA_CPPFLAGS = \
'-DGIT_VERSION=$(GIT_VERSION)' \
'-DGIT_USER_AGENT=$(GIT_USER_AGENT_CQ_SQ)'
@@ -1773,12 +1773,12 @@ sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \
$@.sh $@+
endef
-$(patsubst %.sh,%,$(SCRIPT_SH)) : % : %.sh GIT-SCRIPT-DEFINES
+$(patsubst %.sh,%,$(SCRIPT_SH)) : % : %.sh MAKE/SCRIPT-DEFINES
$(QUIET_GEN)$(cmd_munge_script) \
chmod +x $@+ \
mv $@+ $@
-$(SCRIPT_LIB) : % : %.sh GIT-SCRIPT-DEFINES
+$(SCRIPT_LIB) : % : %.sh MAKE/SCRIPT-DEFINES
$(QUIET_GEN)$(cmd_munge_script) \
mv $@+ $@
@@ -1797,14 +1797,14 @@ perl/PM.stamp: FORCE
{ cmp $@+ $@ /dev/null 2/dev/null || mv $@+ $@; } \
$(RM) $@+
-perl/perl.mak: GIT-CFLAGS GIT-PREFIX perl/Makefile perl/Makefile.PL
Re: [PATCH 09/13] Makefile: add c-quote helper function
Jeff King p...@peff.net writes: We have to c-quote strings coming from Makefile variables when we pass them to the compiler via -D. Now that we can use $(call) in our Makefile, we can factor out the quoting to make things easier to read. We can also apply it more consistently, as there were many spots that should have been C-quoting but did not. For example: make prefix='foo\bar' would produce an exec_cmd.o with a broken prefix. Signed-off-by: Jeff King p...@peff.net --- Makefile | 58 +- 1 file changed, 29 insertions(+), 29 deletions(-) diff --git a/Makefile b/Makefile index 868872f..b1c3fb4 100644 --- a/Makefile +++ b/Makefile @@ -1568,6 +1568,17 @@ endif sqi = $(subst ','\'',$1) sq = '$(call sqi,$1)' +# usage: $(call cq,CONTENTS) +# +# Quote the value as appropriate for a C string literal. +cq = $(subst ,\,$(subst \,\\,$1)) + +# usage: $(call scq,CONTENTS) +# +# Quote the value as C string inside a shell string. Good for passing strings +# on the command line via -DFOO=$(call # scq,$(FOO)). call # scq??? +scq = $(call sq,$(call cq,$1)) + # usage: $(eval $(call make-var,FN,DESC,CONTENTS)) # # Create a rule to write $CONTENTS (which should come from a make variable) @@ -1617,28 +1628,17 @@ LIB_OBJS += $(COMPAT_OBJS) # Quote for C ifdef DEFAULT_EDITOR -DEFAULT_EDITOR_CQ = $(subst ,\,$(subst \,\\,$(DEFAULT_EDITOR))) -DEFAULT_EDITOR_CQ_SQ = $(subst ','\'',$(DEFAULT_EDITOR_CQ)) - -BASIC_CFLAGS += -DDEFAULT_EDITOR='$(DEFAULT_EDITOR_CQ_SQ)' +BASIC_CFLAGS += -DDEFAULT_EDITOR=$(call scq,$(DEFAULT_EDITOR)) endif ifdef DEFAULT_PAGER -DEFAULT_PAGER_CQ = $(subst ,\,$(subst \,\\,$(DEFAULT_PAGER))) -DEFAULT_PAGER_CQ_SQ = $(subst ','\'',$(DEFAULT_PAGER_CQ)) - -BASIC_CFLAGS += -DDEFAULT_PAGER='$(DEFAULT_PAGER_CQ_SQ)' +BASIC_CFLAGS += -DDEFAULT_PAGER=$(call scq,$(DEFAULT_PAGER)) endif ifdef SHELL_PATH -SHELL_PATH_CQ = $(subst ,\,$(subst \,\\,$(SHELL_PATH))) -SHELL_PATH_CQ_SQ = $(subst ','\'',$(SHELL_PATH_CQ)) - -BASIC_CFLAGS += -DSHELL_PATH='$(SHELL_PATH_CQ_SQ)' +BASIC_CFLAGS += -DSHELL_PATH=$(call scq,$(SHELL_PATH)) endif -GIT_USER_AGENT_CQ = $(subst ,\,$(subst \,\\,$(GIT_USER_AGENT))) -GIT_USER_AGENT_CQ_SQ = $(subst ','\'',$(GIT_USER_AGENT_CQ)) $(eval $(call make-var,USER-AGENT,user agent string,$(GIT_USER_AGENT))) ifdef DEFAULT_HELP_FORMAT @@ -1723,9 +1723,9 @@ strip: $(PROGRAMS) git$X git.sp git.s git.o: MAKE/PREFIX git.sp git.s git.o: EXTRA_CPPFLAGS = \ - '-DGIT_HTML_PATH=$(htmldir_relative_SQ)' \ - '-DGIT_MAN_PATH=$(mandir_relative_SQ)' \ - '-DGIT_INFO_PATH=$(infodir_relative_SQ)' + -DGIT_HTML_PATH=$(call scq,$(htmldir_relative)) \ + -DGIT_MAN_PATH=$(call scq,$(mandir_relative)) \ + -DGIT_INFO_PATH=$(call scq,$(infodir_relative)) git$X: git.o MAKE/LDFLAGS $(BUILTIN_OBJS) $(GITLIBS) $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ git.o \ @@ -1735,14 +1735,14 @@ help.sp help.s help.o: common-cmds.h builtin/help.sp builtin/help.s builtin/help.o: common-cmds.h MAKE/PREFIX builtin/help.sp builtin/help.s builtin/help.o: EXTRA_CPPFLAGS = \ - '-DGIT_HTML_PATH=$(htmldir_relative_SQ)' \ - '-DGIT_MAN_PATH=$(mandir_relative_SQ)' \ - '-DGIT_INFO_PATH=$(infodir_relative_SQ)' + -DGIT_HTML_PATH=$(call scq,$(htmldir_relative)) \ + -DGIT_MAN_PATH=$(call scq,$(mandir_relative)) \ + -DGIT_INFO_PATH=$(call scq,$(infodir_relative)) version.sp version.s version.o: GIT-VERSION-FILE MAKE/USER-AGENT version.sp version.s version.o: EXTRA_CPPFLAGS = \ - '-DGIT_VERSION=$(GIT_VERSION)' \ - '-DGIT_USER_AGENT=$(GIT_USER_AGENT_CQ_SQ)' + -DGIT_VERSION=$(call scq,$(GIT_VERSION)) \ + -DGIT_USER_AGENT=$(call scq,$(GIT_USER_AGENT)) $(BUILT_INS): git$X $(QUIET_BUILT_IN)$(RM) $@ \ @@ -2020,25 +2020,25 @@ endif exec_cmd.sp exec_cmd.s exec_cmd.o: MAKE/PREFIX exec_cmd.sp exec_cmd.s exec_cmd.o: EXTRA_CPPFLAGS = \ - '-DGIT_EXEC_PATH=$(gitexecdir_SQ)' \ - '-DBINDIR=$(bindir_relative_SQ)' \ - '-DPREFIX=$(prefix_SQ)' + -DGIT_EXEC_PATH=$(call scq,$(gitexecdir)) \ + -DBINDIR=$(call scq,$(bindir_relative)) \ + -DPREFIX=$(call scq,$(prefix)) builtin/init-db.sp builtin/init-db.s builtin/init-db.o: MAKE/PREFIX builtin/init-db.sp builtin/init-db.s builtin/init-db.o: EXTRA_CPPFLAGS = \ - -DDEFAULT_GIT_TEMPLATE_DIR='$(template_dir_SQ)' + -DDEFAULT_GIT_TEMPLATE_DIR=$(call scq,$(template_dir)) config.sp config.s config.o: MAKE/PREFIX config.sp config.s config.o: EXTRA_CPPFLAGS = \ - -DETC_GITCONFIG='$(ETC_GITCONFIG_SQ)' + -DETC_GITCONFIG=$(call scq,$(ETC_GITCONFIG)) attr.sp attr.s attr.o: MAKE/PREFIX attr.sp attr.s attr.o: EXTRA_CPPFLAGS = \ - -DETC_GITATTRIBUTES='$(ETC_GITATTRIBUTES_SQ)' + -DETC_GITATTRIBUTES=$(call scq,$(ETC_GITATTRIBUTES)) gettext.sp gettext.s gettext.o: MAKE/PREFIX gettext.sp gettext.s
Re: [PATCH 08/13] Makefile: introduce sq function for shell-quoting
Jeff King p...@peff.net writes: Since we have recently abolished the prohibition on $(call) in our Makefile, we can use it to factor out the repeated shell-quoting we do. There are two upsides: 1. It is much more readable than inline calls to $(subst ','\''). 2. It is short enough that we can do away with the _SQ variants of many variables (note that we do not do this just yet, as there is a little more cleanup needed first). Yay. But many instances are not really any more readable (e.g., see the first hunk below). ... .PHONY: install-perl-script install-sh-script install-python-script install-sh-script: $(SCRIPT_SH_INS) - $(INSTALL) $^ '$(DESTDIR_SQ)$(gitexec_instdir_SQ)' + $(INSTALL) $^ $(call sq,$(DESTDIR)$(gitexec_instdir)) Hmph, I do not see it as bad as the make-var, which forces you to say $(eval $(call ...)); this $(call sq, ...) is fairly readable. -- To unsubscribe from this list: send the line unsubscribe 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 10/13] Makefile: drop *_SQ variables
Again, Yay! -- To unsubscribe from this list: send the line unsubscribe 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 11/13] Makefile: auto-build C strings from make variables
Jeff King p...@peff.net writes:
diff --git a/script/mkcstring b/script/mkcstring
new file mode 100644
index 000..c01f430
--- /dev/null
+++ b/script/mkcstring
@@ -0,0 +1,18 @@
+#!/bin/sh
+
+name=$1; shift
+
+c_quote() {
+ sed 's/\\//g; s//\\/'
No 'g' for the second one?
+}
+
+cat -EOF
+#ifndef MAKE_${name}_H
+#define MAKE_${name}_H
+
+/* Auto-generated by mkcstring */
+
+#define MAKE_${name} $(c_quote)
+
+#endif /* MAKE_${name}_H */
+EOF
diff --git a/version.c b/version.c
index 6106a80..f68a93b 100644
--- a/version.c
+++ b/version.c
@@ -1,8 +1,10 @@
#include git-compat-util.h
#include version.h
#include strbuf.h
+#include MAKE/USER-AGENT-string.h
+#include MAKE/VERSION-string.h
-const char git_version_string[] = GIT_VERSION;
+const char git_version_string[] = MAKE_VERSION;
const char *git_user_agent(void)
{
@@ -11,7 +13,7 @@ const char *git_user_agent(void)
if (!agent) {
agent = getenv(GIT_USER_AGENT);
if (!agent)
- agent = GIT_USER_AGENT;
+ agent = MAKE_USER_AGENT;
}
return agent;
--
To unsubscribe from this list: send the line unsubscribe 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 12/13] Makefile: teach scripts to include make variables
Jeff King p...@peff.net writes:
define cmd_munge_script
$(RM) $@ $@+ \
+{ \
+includes=$(filter MAKE/%.sh,$^); \
+if ! test -z $$includes; then \
+ cat $$includes; \
+fi \
sed -e '1s|#!.*/sh|#!$(call sqi,$(SHELL_PATH))|' \
-e 's|@SHELL_PATH@|$(call sqi,$(SHELL_PATH))|' \
--e 's|@@DIFF@@|$(call sqi,$(DIFF))|' \
-e 's|@@LOCALEDIR@@|$(call sqi,$(localedir))|g' \
-e 's/@@NO_CURL@@/$(NO_CURL)/g' \
-e 's/@@USE_GETTEXT_SCHEME@@/$(USE_GETTEXT_SCHEME)/g' \
-e $(BROKEN_PATH_FIX) \
-e 's|@@GITWEBDIR@@|$(call sqi,$(gitwebdir))|g' \
-e 's|@@PERL@@|$(call sqi,$(PERL_PATH))|g' \
-$@.sh $@+
+$@.sh; \
+} $@+
endef
Sorry, but I am not quite sure what is going on here.
- if $includes does not exist, cat $includes will barf but that is
OK;
- if $includes is empty, there is no point running cat so that is
OK;
- if $includes is not empty, we want to cat.
And then after emitting that piece, we start processing the *.sh
source file, replacing she-bang line?
diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index fffa3c7..627d289 100644
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh
@@ -285,7 +285,7 @@ clear_local_git_env() {
# remove lines from $1 that are not in $2, leaving only common lines.
create_virtual_base() {
sz0=$(wc -c $1)
- @@DIFF@@ -u -La/$1 -Lb/$1 $1 $2 | git apply --no-add
+ $MAKE_DIFF -u -La/$1 -Lb/$1 $1 $2 | git apply --no-add
sz1=$(wc -c $1)
This would mean that after this mechanism is extensively employed
throughout our codebase, any random environment variable the user
has whose name happens to begin with MAKE_ will interfere with us
(rather, we will override such a variable while we run). Having to
carve out our own namespace in such a way is OK, but we would want
to see that namespace somewhat related to the name of our project,
not to the name of somebody else's like make, no?
# If we do not have enough common material, it is not
diff --git a/script/mksh b/script/mksh
new file mode 100644
index 000..d41e77a
--- /dev/null
+++ b/script/mksh
@@ -0,0 +1,4 @@
+#!/bin/sh
+
+name=$1; shift
+printf MAKE_%s='%s'\n $name $(sed s/'/'\\''/g)
--
To unsubscribe from this list: send the line unsubscribe 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/8] combine-diff: Fast changed-to-all-parents paths scanning
Kirill Smelkov k...@navytux.spb.ru writes: I agree object data should be immutable for good. The only thing I'm talking about here is mode, which is parsed from a tree buffer and is stored in separate field: Ah, I do not see any problem in that case, then. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] repack.c: rename and unlink pack file if it exists
... and this is the other half that is supposed to be only about
renaming variables.
-- 8 --
From: Torsten Bögershausen tbo...@web.de
Date: Sun, 2 Feb 2014 16:09:56 +0100
Subject: [PATCH] repack.c: rename a few variables
Rename the variables to match what they are used for: fname for the
final name of the new packfile, fname_old for the name of the
existing one, and fname_tmp for the temporary name pack-objects
created the new packfile in.
Signed-off-by: Torsten Bögershausen tbo...@web.de
Signed-off-by: Junio C Hamano gits...@pobox.com
---
builtin/repack.c | 22 +++---
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/builtin/repack.c b/builtin/repack.c
index fe31577..3b01353 100644
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -316,33 +316,33 @@ int cmd_repack(int argc, const char **argv, const char
*prefix)
/* Now the ones with the same name are out of the way... */
for_each_string_list_item(item, names) {
for (ext = 0; ext 2; ext++) {
- char *fname, *fname_old;
+ char *fname, *fname_tmp;
struct stat statbuffer;
fname = mkpathdup(%s/pack-%s%s,
packdir, item-string, exts[ext]);
- fname_old = mkpathdup(%s-%s%s,
+ fname_tmp = mkpathdup(%s-%s%s,
packtmp, item-string, exts[ext]);
- if (!stat(fname_old, statbuffer)) {
+ if (!stat(fname_tmp, statbuffer)) {
statbuffer.st_mode = ~(S_IWUSR | S_IWGRP |
S_IWOTH);
- chmod(fname_old, statbuffer.st_mode);
+ chmod(fname_tmp, statbuffer.st_mode);
}
- if (rename(fname_old, fname))
- die_errno(_(renaming '%s' failed), fname_old);
+ if (rename(fname_tmp, fname))
+ die_errno(_(renaming '%s' into '%s' failed),
fname_tmp, fname);
free(fname);
- free(fname_old);
+ free(fname_tmp);
}
}
/* Remove the old- files */
for_each_string_list_item(item, names) {
for (ext = 0; ext 2; ext++) {
- char *fname;
- fname = mkpath(%s/old-%s%s,
+ char *fname_old;
+ fname_old = mkpath(%s/old-%s%s,
packdir,
item-string,
exts[ext]);
- if (remove_path(fname))
- warning(_(removing '%s' failed), fname);
+ if (remove_path(fname_old))
+ warning(_(removing '%s' failed), fname_old);
}
}
--
1.9-rc2-217-g24a8b2e
--
To unsubscribe from this list: send the line unsubscribe 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] repack.c: rename and unlink pack file if it exists
Jeff King p...@peff.net writes: The minimal fix you posted below does make sense to me as a stopgap, and we can look into dropping the code entirely during the next cycle. It would be nice to have a test to cover this case, though. Sounds sensible. Run repack -a -d once, and then another while forcing it to be single threaded, or something? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] blame.c: prepare_lines should not call xrealloc for every line
David Kastrup d...@gnu.org writes:
Junio C Hamano gits...@pobox.com writes:
which I think is the prevalent style in our codebase. The same for
the other loop we see in the new code below.
- avoid assignments in conditionals when you do not have to.
commit a77a48c259d9adbe7779ca69a3432e493116b3fd
Author: Junio C Hamano gits...@pobox.com
Date: Tue Jan 28 13:55:59 2014 -0800
combine-diff: simplify intersect_paths() further
[...]
+ while ((p = *tail) != NULL) {
Because we can.
Be reasonable. You cannot sensibly rewrite it to
p = *tail;
while (p) {
...
p = *tail;
}
when you do not know how ... part would evolve in the future.
if ((p = *tail) != NULL) {
...
is a totally different issue.
--
To unsubscribe from this list: send the line unsubscribe 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] repack.c: rename and unlink pack file if it exists
Jeff King p...@peff.net writes:
On Tue, Feb 04, 2014 at 03:40:15PM -0800, Junio C Hamano wrote:
* Somehow this came to my private mailbox without Cc to list, so I
am forwarding it.
I think with 1190a1ac (pack-objects: name pack files after
trailer hash, 2013-12-05), repacking the same set of objects may
have less chance of producing colliding names, especially if you
are on a box with more than one core, but it still would be a
good idea to get this part right in the upcoming release.
Actually, since 1190a1ac, if you have repacked and gotten the same pack
name, then you do not have to do any rename dance at all; you can throw
away what you just generated because you know that it is byte-for-byte
identical.
You could collide with a pack created by an older version of git that
used the original scheme, but that is quite unlikely (on the order of
2^-160).
Yes, so in that sense this is not so urgent, but I'm tempted to
split the original patch into two and merge only the first one to
'master' before -rc3 (see below). The renaming of the variables
added enough noise to cause me fail to spot a change mixed within.
-- 8 --
From: Torsten Bögershausen tbo...@web.de
Date: Sun, 2 Feb 2014 16:09:56 +0100
When a repo was fully repacked, and is repacked again, we may run
into the situation that new packfiles have the same name as
already existing ones (traditionally packfiles have been named after
the list of names of objects in them, so repacking all the objects
in a single pack would have produced a packfile with the same name).
The logic is to rename the existing ones into filename like
old-XXX, create the new ones and then remove the old- ones.
When something went wrong in the middle, this sequence is rolled
back by renaming the old- files back.
The renaming into old- did not work as intended, because
file_exists() was done on XXX, not pack-XXX. Also when rolling
back the change, the code tried to rename old-pack-XXX but the
saved ones are named old-XXX, so this couldn't have worked.
Signed-off-by: Torsten Bögershausen tbo...@web.de
Signed-off-by: Junio C Hamano gits...@pobox.com
---
builtin/repack.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/builtin/repack.c b/builtin/repack.c
index bca7710..fe31577 100644
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -260,7 +260,7 @@ int cmd_repack(int argc, const char **argv, const char
*prefix)
for_each_string_list_item(item, names) {
for (ext = 0; ext 2; ext++) {
char *fname, *fname_old;
- fname = mkpathdup(%s/%s%s, packdir,
+ fname = mkpathdup(%s/pack-%s%s, packdir,
item-string, exts[ext]);
if (!file_exists(fname)) {
free(fname);
@@ -337,7 +337,7 @@ int cmd_repack(int argc, const char **argv, const char
*prefix)
for_each_string_list_item(item, names) {
for (ext = 0; ext 2; ext++) {
char *fname;
- fname = mkpath(%s/old-pack-%s%s,
+ fname = mkpath(%s/old-%s%s,
packdir,
item-string,
exts[ext]);
--
1.9-rc2-217-g24a8b2e
--
To unsubscribe from this list: send the line unsubscribe 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] blame.c: prepare_lines should not call xrealloc for every line
David Kastrup d...@gnu.org writes: It's snake oil making debugging harder. OK, that is a sensible argument. This was fun ;-) At the expense of seriously impacting my motivation to do any further code cleanup on Git. Well, I said it was fun because I was learning from a new person who haven't made much technical or code aethesitics discussion here so far. If teaching others frustrates you and stops contributing to the project, that is a loss. But the styles matter, as the known pattern in the existing code is what lets our eyes coast over unimportant details of the code while reviewing and understanding. I tend to be pickier when reviewing code from new people who are going to make large contributions for exactly that reason---new blood bringing in new ideas is fine, but I'd want to see those new ideas backed by solid thiniking, and that means they may have to explain themselves to those who are new to their ideas. -- To unsubscribe from this list: send the line unsubscribe 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] repack.c: rename and unlink pack file if it exists
Jeff King p...@peff.net writes:
On Wed, Feb 05, 2014 at 12:31:34PM -0800, Junio C Hamano wrote:
Jeff King p...@peff.net writes:
The minimal fix you posted below does make sense to me as a stopgap, and
we can look into dropping the code entirely during the next cycle. It
would be nice to have a test to cover this case, though.
Sounds sensible. Run repack -a -d once, and then another while
forcing it to be single threaded, or something?
Certainly that's the way to trigger the code, but doing this:
diff --git a/t/t7700-repack.sh b/t/t7700-repack.sh
index b45bd1e..6647ba1 100755
--- a/t/t7700-repack.sh
+++ b/t/t7700-repack.sh
@@ -162,7 +162,12 @@ test_expect_success 'objects made unreachable by grafts
only are kept' '
git reflog expire --expire=$test_tick --expire-unreachable=$test_tick
--all
git repack -a -d
git cat-file -t $H1
- '
+'
+
+test_expect_success 'repack can handle generating the same pack again' '
+ git -c pack.threads=1 repack -ad
+ git -c pack.threads=1 repack -ad
+'
test_done
...does not seem to fail, and it does not seem to leave any cruft in
place. So maybe I am misunderstanding the thing the patch is meant to
fix. Is it that we simply do not replace the pack in this instance?
Yes. Not just the command finishing OK, but the packfile left by
the first repack needs to be left intact. One bug was that after
learning that a new packfile needs to be installed, the command
did not check existing .git/objects/pack/pack-.{pack,idx}, but
checked .git/objects/pack/.{pack,idx}, deciding that there is
nothing that needs to be saved by renaming with s|pack-|old-|. This
would have caused it to rename the new packfile left by pack-object
at .git/objects/pack/.tmp-$pid-pack-.{pack,idx} directly to the
final .git/objects/pack/pack-.{pack,idx}, which would work only
on filesystems that allow renaming over an existing file.
Another bug fixed by Torsten was in the codepath to roll the rename
back from .git/objects/pack/old-.{pack,idx} to the original (the
command tried to rename .git/objects/pack/old-pack-.{pack,idx}
which would not have been the ones it renamed), but because of the
earlier bug, it would never have triggered in the first place.
I guess we would have to generate a pack with the identical set of
objects, then, but somehow different in its pack parameters (perhaps
turning off deltas?).
Unfortunately, that would trigger different codepath on v1.9-rc0 and
later with 1190a1ac (pack-objects: name pack files after trailer
hash, 2013-12-05), as it is likely not to name the packfiles the
same.
We could use test-chmtime to reset the timestamp of the packfile
generated by the first repack to somewhere reasonably old and then
rerun the repack to see that it is a different file, which may be
more portable than inspecting the inum of the packfile.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Bug] branch.*.merge interpreted too strictly by tracking logic
Jeff King p...@peff.net writes: Is it legal to put an unqualified ref there? A wise man once said[1]: Actually, it is broken in a lot of places. for-each-ref relies on the same code as git status, git checkout, etc, which will all fail to display tracking info. I believe the same code is also used for updating tracking branches on push. So I'm not sure if it was ever intended to be a valid setting. It wasn't. Some places may accept them gracefully by either being extra nice or by accident. I don't recall us ever doing anything after that. I don't have a problem with making it work, of course, but I am not sure if it is really a bug. Once people get used to us being extra nice in some places, other less nice places start looking more and more like bugs. It is an unfortunate fact of life, but fixing them up is a good thing for users. As long as we can make those less nice places nicer uniformly without bending backwards or introducing unnecessary ambiguities, that is, and I think this one can be done without such downsides. -- To unsubscribe from this list: send the line unsubscribe 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] repack.c: rename and unlink pack file if it exists
Jeff King p...@peff.net writes: ... So the fact that this bug exists doesn't really produce any user-visible behavior, and hopefully post-release we would drop the code entirely, and the test would have no reason to exist. Heh, thanks, and I agree with the reasoning for the longer-term direction. Perhaps I can/should hold it off that minimal fix-up patch from -rc3, then? I am on the fence but I already started my today's integration cycle _with_ the fix merged to 'master', so... -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Bug] branch.*.merge interpreted too strictly by tracking logic
Jeff King p...@peff.net writes: Did your report come out of a real case, or was it just something you noticed? Some git-wrappers (like repo) are reported to muck with the configuration files. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Documentation: fix typos in man pages
Øystein Walle oys...@gmail.com writes: Signed-off-by: Øystein Walle oys...@gmail.com --- In July I sent in some typo fixes but it halted in a discussion on UK vs. US English and so forth ($gmane/231331). There were some actual typo fixes in there though (in addition to the opinionated typo fixes). Powering up my old laptop for the first time in months I noticed that I had them idling in a branch, and that incidentally none of them has been fixed. Hopefully I've managed to seperate the genuine typo fixes from the rest. Wonderful. All of them looked fine. I however am already deep in today's integration cycle, so it will appear in the public branches tomorrow. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 7/8] combine-diff: Fast changed-to-all-parents paths scanning
Kirill Smelkov k...@navytux.spb.ru writes: On Wed, Feb 05, 2014 at 11:42:41AM -0800, Junio C Hamano wrote: Kirill Smelkov k...@navytux.spb.ru writes: I agree object data should be immutable for good. The only thing I'm talking about here is mode, which is parsed from a tree buffer and is stored in separate field: Ah, I do not see any problem in that case, then. Thanks. Thanks, that simplifies things for me. Surely. Be careful when traversing N-trees in parallel---you may have to watch out for the entry ordering rules that sorts the following paths in the order shown: a a-b a/b a_b Inside a single tree, you cannot have 'a' and 'a/b' at the same time, but one tree may have 'a' (without 'a/b') while another one may have 'a/b' (without 'a'), and walking them in parallel has historically been one source of funny bugs. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
What's cooking in git.git (Feb 2014, #02; Wed, 5)
Here are the topics that have been cooking. Commits prefixed with
'-' are only in 'pu' (proposed updates) while commits prefixed with
'+' are in 'next'.
v1.9.0-rc3 is expected to happen this weekend or early next week.
You can find the changes described here in the integration branches
of the repositories listed at
http://git-blame.blogspot.com/p/git-public-repositories.html
--
[New Topics]
* aj/ada-diff-word-pattern (2014-02-05) 1 commit
- userdiff: update Ada patterns
Will merge to 'next' and then to 'master'.
* jk/makefile (2014-02-05) 16 commits
- FIXUP
- move LESS/LV pager environment to Makefile
- Makefile: teach scripts to include make variables
- FIXUP
- Makefile: auto-build C strings from make variables
- Makefile: drop *_SQ variables
- FIXUP
- Makefile: add c-quote helper function
- Makefile: introduce sq function for shell-quoting
- Makefile: always create files via make-var
- Makefile: store GIT-* sentinel files in MAKE/
- Makefile: prefer printf to echo for GIT-*
- Makefile: use tempfile/mv strategy for GIT-*
- Makefile: introduce make-var helper function
- Makefile: fix git-instaweb dependency on gitweb
- Makefile: drop USE_GETTEXT_SCHEME from GIT-CFLAGS
* ks/tree-diff-walk (2014-02-05) 4 commits
- revision: convert to using diff_tree_sha1()
- line-log: convert to using diff_tree_sha1()
- tree-diff: convert diff_root_tree_sha1() to just call diff_tree_sha1 with
old=NULL
- tree-diff: allow diff_tree_sha1 to accept NULL sha1
Will merge to 'next'.
* nd/reset-intent-to-add (2014-02-05) 1 commit
- reset: support --mixed --intent-to-add mode
Will merge to 'next'.
* nd/tag-doc (2014-02-04) 1 commit
- git-tag.txt: commit for --contains is optional
Will merge to 'next' and then to 'master'.
* nd/test-rename-reset (2014-02-04) 1 commit
- t7101, t7014: rename test files to indicate what that file is for
Will merge to 'next'.
* tb/repack-fix-renames (2014-02-05) 1 commit
- repack.c: rename a few variables
Perhaps unneeded, as the longer-term plan is to drop the codeblock
this change touches.
Will discard.
* tr/remerge-diff (2014-02-05) 6 commits
- log --remerge-diff: show what the conflict resolution changed
- merge-recursive: allow storing conflict hunks in index
- Fold all merge diff variants into an enum
- combine-diff: do not pass revs-dense_combined_merges redundantly
- log: add a merge base inspection option
- pretty: refactor add_merge_info() into parts
(this branch uses tr/merge-recursive-index-only.)
log -p output learns a new way to let users inspect a merge
commit by showing the differences between the automerged result
with conflicts the person who recorded the merge would have seen
and the final conflict resolution that was recorded in the merge.
RFC.
* ow/manpages-typofix (2014-02-05) 1 commit
- Documentation: fix typos in man pages
Various typofixes, all looked correct.
Will merge to 'next' and then to 'master'.
--
[Stalled]
* po/everyday-doc (2014-01-27) 1 commit
- Make 'git help everyday' work
This may make the said command to emit something, but the source is
not meant to be formatted into a manual pages to begin with, and
also its contents are a bit stale. It may be a good first step in
the right direction, but needs more work to at least get the
mark-up right before public consumption.
Will hold.
* jk/branch-at-publish-rebased (2014-01-17) 5 commits
- t1507 (rev-parse-upstream): fix typo in test title
- implement @{publish} shorthand
- branch_get: provide per-branch pushremote pointers
- branch_get: return early on error
- sha1_name: refactor upstream_mark
Give an easier access to the tracking branches from other side in
a triangular workflow by introducing B@{publish} that works in a
similar way to how B@{upstream} does.
Will hold.
* rb/merge-prepare-commit-msg-hook (2014-01-10) 4 commits
- merge: drop unused arg from abort_commit method signature
- merge: make prepare_to_commit responsible for write_merge_state
- t7505: ensure cleanup after hook blocks merge
- t7505: add missing
Expose more merge states (e.g. $GIT_DIR/MERGE_MODE) to hooks that
run during git merge. The log message stresses too much on one
hook, prepare-commit-msg, but it would equally apply to other hooks
like post-merge, I think.
Waiting for a reroll.
* jl/submodule-recursive-checkout (2013-12-26) 5 commits
- Teach checkout to recursively checkout submodules
- submodule: teach unpack_trees() to update submodules
- submodule: teach unpack_trees() to repopulate submodules
- submodule: teach unpack_trees() to remove submodule contents
- submodule: prepare for recursive checkout of submodules
Expecting a reroll.
* jc/graph-post-root-gap (2013-12-30) 3 commits
- WIP: document what we want at the end
- graph: remove unused code a bit
- graph: stuff the current commit into graph-columns[]
Re: [PATCH 2/2] reset: support --mixed --intent-to-add mode
On Wed, Feb 5, 2014 at 3:48 PM, Duy Nguyen pclo...@gmail.com wrote: No no. I found that duplicate, but I did not suggest removing it because it is needed there.. Hmph, if that is the case, we probably should make it the responsibility of the calling side to actually mark ce-flags with the bit (which would also mean the function must be renamed to make it clear that it does not mark). -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] reset: support --mixed --intent-to-add mode
Junio C Hamano gits...@pobox.com writes:
On Wed, Feb 5, 2014 at 3:48 PM, Duy Nguyen pclo...@gmail.com wrote:
No no. I found that duplicate, but I did not suggest removing it
because it is needed there..
Hmph, if that is the case, we probably should make it the
responsibility of the calling side to actually mark ce-flags with the
bit (which would also mean the function must be renamed to make it
clear that it does not mark).
After looking at the codepath that uses the record_intent_to_add()
before this patch, I am coming to the conclusion that it is the
right thing to do after all. The code appears in this section:
if (!intent_only) {
if (index_path(ce-sha1, path, st, HASH_WRITE_OBJECT))
return error(unable to index file %s, path);
} else
record_intent_to_add(ce);
which tells (at least) me: We are not adding the contents of this
path, so we do not run index_path(); instead we call this helper
function to set the object name in ce to represent an intent-to-add
entry.
So I'll rename it to set_object_name_for_intent_to_add_entry() or
something, restore that flag manipulation back to the caller, and
add another to the new caller, and requeue.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: attr.c doesn't honor --work-tree option
Lasse Makholm lasse.makh...@gmail.com writes:
Here's a repro with -DDEBUG_ATTR=1 and a printf() in read_attr_from_file():
$ cd /tmp/
$ mkdir -p attr-test/repo
$ cd attr-test/repo
$ git init
Initialized empty Git repository in /tmp/attr-test/repo/.git/
$ echo 'dir/* filter=foo' .gitattributes
$
Inside the working tree, it works:
$ ~/src/git.git/git check-attr -a dir/file
Does check-ignore misbehave the same way?
I suspect that is this because check-attr is not a command that
requires a working tree. The command was written primarily as a
debugging aid that can be used anywhere as long as you have a
repository to read strings from either its standard input or its
arguments, and gives them directly to check_attr(), but it does so
without first going to the top of the real working tree like
check-ignore does.
Forcing it to go to the top of the working tree (see the attached
one-liner, but note that I didn't test it) may give you want you
want.
git.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/git.c b/git.c
index 7cf2953..314ec9f 100644
--- a/git.c
+++ b/git.c
@@ -342,7 +342,7 @@ static struct cmd_struct commands[] = {
{ branch, cmd_branch, RUN_SETUP },
{ bundle, cmd_bundle, RUN_SETUP_GENTLY },
{ cat-file, cmd_cat_file, RUN_SETUP },
- { check-attr, cmd_check_attr, RUN_SETUP },
+ { check-attr, cmd_check_attr, RUN_SETUP | NEED_WORK_TREE },
{ check-ignore, cmd_check_ignore, RUN_SETUP | NEED_WORK_TREE },
{ check-mailmap, cmd_check_mailmap, RUN_SETUP },
{ check-ref-format, cmd_check_ref_format },
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] t0003: do not chdir the whole test process
Moving to some other directory and letting the remainder of the test pieces to expect that they start there is a bad practice. The test that contains chdir itself may fail (or by mistake skipped via the GIT_SKIP_TESTS mechanism) in which case the remainder may operate on files in unexpected places. Signed-off-by: Junio C Hamano gits...@pobox.com --- * This is purely a preparatory clean-up in the test script I'll be adding a new test to in the next patch. t/t0003-attributes.sh | 52 +-- 1 file changed, 30 insertions(+), 22 deletions(-) diff --git a/t/t0003-attributes.sh b/t/t0003-attributes.sh index febc45c..0554b13 100755 --- a/t/t0003-attributes.sh +++ b/t/t0003-attributes.sh @@ -197,39 +197,47 @@ test_expect_success 'root subdir attribute test' ' ' test_expect_success 'setup bare' ' - git clone --bare . bare.git - cd bare.git + git clone --bare . bare.git ' test_expect_success 'bare repository: check that .gitattribute is ignored' ' ( - echo f test=f - echo a/i test=a/i - ) .gitattributes - attr_check f unspecified - attr_check a/f unspecified - attr_check a/c/f unspecified - attr_check a/i unspecified - attr_check subdir/a/i unspecified + cd bare.git + ( + echo f test=f + echo a/i test=a/i + ) .gitattributes + attr_check f unspecified + attr_check a/f unspecified + attr_check a/c/f unspecified + attr_check a/i unspecified + attr_check subdir/a/i unspecified + ) ' test_expect_success 'bare repository: check that --cached honors index' ' - GIT_INDEX_FILE=../.git/index \ - git check-attr --cached --stdin --all ../stdin-all | - sort actual - test_cmp ../specified-all actual + ( + cd bare.git + GIT_INDEX_FILE=../.git/index \ + git check-attr --cached --stdin --all ../stdin-all | + sort actual + test_cmp ../specified-all actual + ) ' test_expect_success 'bare repository: test info/attributes' ' ( - echo f test=f - echo a/i test=a/i - ) info/attributes - attr_check f f - attr_check a/f f - attr_check a/c/f f - attr_check a/i a/i - attr_check subdir/a/i unspecified + cd bare.git + ( + echo f test=f + echo a/i test=a/i + ) info/attributes + attr_check f f + attr_check a/f f + attr_check a/c/f f + attr_check a/i a/i + attr_check subdir/a/i unspecified + ) ' test_done -- 1.9-rc2-233-ged4ee9f -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] check-attr: move to the top of working tree when in non-bare repository
Lasse Makholm noticed that running git check-attr from a place totally unrelated to $GIT_DIR and $GIT_WORK_TREE does not give expected results. I think it is because the command does not say it wants to call setup_work_tree(). We still need to support use cases where only a bare repository is involved, so unconditionally requiring a working tree would not work well. Instead, make a call only in a non-bare repository. We may want to see if we want to do a similar fix in the opposite direction to check-ignore. The command unconditionally requires a working tree, but it should be usable in a bare repository just like check-attr attempts to be. Signed-off-by: Junio C Hamano gits...@pobox.com --- builtin/check-attr.c | 3 +++ t/t0003-attributes.sh | 10 ++ 2 files changed, 13 insertions(+) diff --git a/builtin/check-attr.c b/builtin/check-attr.c index 075d01d..f29d6c3 100644 --- a/builtin/check-attr.c +++ b/builtin/check-attr.c @@ -94,6 +94,9 @@ int cmd_check_attr(int argc, const char **argv, const char *prefix) struct git_attr_check *check; int cnt, i, doubledash, filei; + if (!is_bare_repository()) + setup_work_tree(); + git_config(git_default_config, NULL); argc = parse_options(argc, argv, prefix, check_attr_options, diff --git a/t/t0003-attributes.sh b/t/t0003-attributes.sh index 0554b13..6e6aef5 100755 --- a/t/t0003-attributes.sh +++ b/t/t0003-attributes.sh @@ -196,6 +196,16 @@ test_expect_success 'root subdir attribute test' ' attr_check subdir/a/i unspecified ' +test_expect_success 'using --git-dir and --work-tree' ' + mkdir unreal real + git init real + echo file test=in-real real/.gitattributes + ( + cd unreal + attr_check file in-real --git-dir ../real/.git --work-tree ../real + ) +' + test_expect_success 'setup bare' ' git clone --bare . bare.git ' -- 1.9-rc2-233-ged4ee9f -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/6] pack-protocol.txt: clarify 'obj-id' in the last ACK after 'done'
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes: It's introduced in 1bd8c8f (git-upload-pack: Support the multi_ack protocol - 2005-10-28) but probably better documented in the commit message of 78affc4 (Add multi_ack_detailed capability to fetch-pack/upload-pack - 2009-10-30) Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- Documentation/technical/pack-protocol.txt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Documentation/technical/pack-protocol.txt b/Documentation/technical/pack-protocol.txt index c73b62f..eb0b8a1 100644 --- a/Documentation/technical/pack-protocol.txt +++ b/Documentation/technical/pack-protocol.txt @@ -338,7 +338,8 @@ during a prior round. This helps to ensure that at least one common ancestor is found before we give up entirely. Once the 'done' line is read from the client, the server will either -send a final 'ACK obj-id' or it will send a 'NAK'. The server only sends +send a final 'ACK obj-id' or it will send a 'NAK'. 'obj-id' is the +last SHA-1 determined to be common. The server only sends I'd suggest rewording it to: 'obj-is' is the object name of the last commit determined to be common I do not mind too much if it used colloquial SHA-1 in place of object name, but what is common is not the name but the object the name refers to, so the last commit part is a moderately strong suggestion. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/6] protocol-capabilities.txt: document no-done
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes: See 3e63b21 (upload-pack: Implement no-done capability - 2011-03-14) and 761ecf0 (fetch-pack: Implement no-done capability - 2011-03-14) for more information. Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- Documentation/technical/protocol-capabilities.txt | 12 1 file changed, 12 insertions(+) diff --git a/Documentation/technical/protocol-capabilities.txt b/Documentation/technical/protocol-capabilities.txt index cb40ebb..cb2f5eb 100644 --- a/Documentation/technical/protocol-capabilities.txt +++ b/Documentation/technical/protocol-capabilities.txt @@ -75,6 +75,18 @@ This is an extension of multi_ack that permits client to better understand the server's in-memory state. See pack-protocol.txt, section Packfile Negotiation for more information. +no-done +--- +This capability should be only used with smart HTTP protocol. If +multi_ack_detailed and no-done are both present, then the sender is +free to immediately send a pack following its first ACK obj-id ready +message. + +Without no-done in smart HTTP protocol, the server session would end +and the client has to make another trip to send done and the server +can send the pack. no-done removes the last round and thus slightly +reduces latency. + thin-pack - Looks good; thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/6] fetch-pack: fix deepen shallow over smart http with no-done cap
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes:
In smart http, upload-pack adds new shallow lines at the beginning of
each rpc response. Only shallow lines from the first rpc call are
useful. After that they are thrown away. It's designed this way
because upload-pack is stateless and has no idea when its shallow
lines are helpful or not.
So after refs are negotiated with multi_ack_detailed and both sides
happy. The server sends ACK obj-id ready, terminates the rpc call
Is the above ... and both sides are happy, the server sends ...?
Lack of a verb makes it hard to guess.
and waits for the final rpc round. The client sends done. The server
sends another response, which also has shallow lines at the beginning,
and the last ACK obj-id line.
When no-done is active, the last round is cut out, the server sends
ACK obj-id ready and ACK obj-id in the same rpc
response. fetch-pack is updated to recognize this and not send
done. However it still tries to consume shallow lines, which are
never sent.
Update the code, make sure to skip consuming shallow lines when
no-done is enabled.
Reported-by: Jeff King p...@peff.net
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
Thanks.
Do I understand the situation correctly if I said The call to
consume-shallow-list has been here from the very beginning, but it
should have been adjusted like this patch when no-done was
introduced.?
fetch-pack.c | 3 ++-
t/t5537-fetch-shallow.sh | 24
2 files changed, 26 insertions(+), 1 deletion(-)
diff --git a/fetch-pack.c b/fetch-pack.c
index 90fdd49..f061f1f 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -439,7 +439,8 @@ done:
}
strbuf_release(req_buf);
- consume_shallow_list(args, fd[0]);
+ if (!got_ready || !no_done)
+ consume_shallow_list(args, fd[0]);
while (flushes || multi_ack) {
int ack = get_ack(fd[0], result_sha1);
if (ack) {
diff --git a/t/t5537-fetch-shallow.sh b/t/t5537-fetch-shallow.sh
index b0fa738..fb11073 100755
--- a/t/t5537-fetch-shallow.sh
+++ b/t/t5537-fetch-shallow.sh
@@ -200,5 +200,29 @@ EOF
)
'
+# This test is tricky. We need large enough haves that fetch-pack
+# will put pkt-flush in between. Then we need a have the the server
+# does not have, it'll send ACK %s ready
+test_expect_success 'add more commits' '
+ (
+ cd shallow
+ for i in $(seq 10); do
+ git checkout --orphan unrelated$i
+ test_commit unrelated$i /dev/null
+ git push -q $HTTPD_DOCUMENT_ROOT_PATH/repo.git
refs/heads/unrelated$i:refs/heads/unrelated$i
+ git push -q ../clone/.git refs/heads/unrelated$i:refs/heads/unrelated$i
+ done
+ git checkout master
+ test_commit new
+ git push $HTTPD_DOCUMENT_ROOT_PATH/repo.git master
+ )
+ (
+ cd clone
+ git checkout --orphan newnew
+ test_commit new-too
+ git fetch --depth=2
+ )
+'
+
stop_httpd
test_done
--
To unsubscribe from this list: send the line unsubscribe 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/6] Fix the shallow deepen bug with no-done
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes: Reported by Jeff [1]. Junio spotted it right but nobody made any move since then. Hrm. Was I supposed to make any move at that point? The discussion ended by me asking a question what happens if we did X? and there was no discussion. The main fix is 6/6. The rest is just updates while I was looking at it. 2/6 may need fast track though. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] t0003: do not chdir the whole test process
Jonathan Nieder jrnie...@gmail.com writes: Junio C Hamano wrote: Moving to some other directory and letting the remainder of the test pieces to expect that they start there is a bad practice. I agree with the above, and I like the patch... The test that contains chdir itself may fail (or by mistake skipped via the GIT_SKIP_TESTS mechanism) in which case the remainder may operate on files in unexpected places. ... but this logic seems wrong. I don't think we've ever supported setup tests failing or being skipped in the past. The first set-up test, yes, but something in the middle added as an afterthought? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] check-attr: move to the top of working tree when in non-bare repository
Jonathan Nieder jrnie...@gmail.com writes: Someone asked in a private reply how this interacts with t0003. It was me mistakenly using reply not reply all. t0003 tries check-attr in a bare repository. The question is, is that a desirable feature, and are people relying on it? Running tar-tree from a public distribution point comes to mind. bootstrap-attr-stack seems to have reference to is-bare-repository to validate the attribute direction to read from the index, but I tend to think what it really wants is to read from HEAD not the index. How do I use the only-look-at-HEAD mode from a non-bare repo? Is You don't a good answer? Use of --cached when your index matches HEAD is the equivalent, and if the index differs from HEAD, you must have had a reason to add that change to .gitattributes to the index, so I think it is reasonable to honour that change. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] t0003: do not chdir the whole test process
Jonathan Nieder jrnie...@gmail.com writes: For a while I've been wanting to teach GIT_SKIP_TESTS not to skip tests with 'setup' or 'set up' in their name, but I never got around to it. Yeah, that would be a good thing. As part of doing so, we might want to come up with a way to test the tests, randomly skipping pieces that are not setup and find ones that break the later tests when skipped, and mark test scripts that fail such a test for fixing. If I try to skip the setup test this patch touches, then there is no bare.git and lots of later tests fail. Perhaps it would be better for each test to do rm -fr bare.git git clone --bare . bare.git ( cd bare.git ... ) for itself to make the state easier to think about. That is a better and worse way to do it at the same time ;-) It definitely is better from maintainability POV to keep each test as independent as possible. It however also is worse if it forces us to be repetitive X-. On the other hand I agree that the 'cd' here is a bad practice. I just don't think it's about skipping setup --- instead, it's about it being hard to remember the cwd in general. Exactly. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] line-log: convert to using diff_tree_sha1()
Thomas Rast t...@thomasrast.ch writes: Kirill Smelkov k...@mns.spb.ru writes: Since diff_tree_sha1() can now accept empty trees via NULL sha1, we could just call it without manually reading trees into tree_desc and duplicating code. Cc: Thomas Rast t...@thomasrast.ch Signed-off-by: Kirill Smelkov k...@mns.spb.ru --- line-log.c | 26 ++ 1 file changed, 2 insertions(+), 24 deletions(-) You have to love a diffstat like that :-) Thanks. Yes, indeed. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 10/17] trailer: if no input file is passed, read from stdin
Christian Couder chrisc...@tuxfamily.org writes:
It is simpler and more natural if the git interpret-trailers
is made a filter as its output already goes to sdtout.
sdtout???
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
builtin/interpret-trailers.c | 2 +-
t/t7513-interpret-trailers.sh | 7 +++
trailer.c | 15 +--
3 files changed, 17 insertions(+), 7 deletions(-)
diff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c
index 04b0ae2..ae8e561 100644
--- a/builtin/interpret-trailers.c
+++ b/builtin/interpret-trailers.c
@@ -23,7 +23,7 @@ int cmd_interpret_trailers(int argc, const char **argv,
const char *prefix)
struct option options[] = {
OPT_BOOL(0, trim-empty, trim_empty, N_(trim empty
trailers)),
- OPT_FILENAME(0, infile, infile, N_(use message from file)),
+ OPT_FILENAME(0, infile, infile, N_(use message from file,
instead of stdin)),
OPT_END()
};
diff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh
index 8be333c..f5ef81f 100755
--- a/t/t7513-interpret-trailers.sh
+++ b/t/t7513-interpret-trailers.sh
@@ -205,4 +205,11 @@ test_expect_success 'using ifMissing = doNothing' '
test_cmp expected actual
'
+test_expect_success 'with input from stdin' '
+ cat complex_message_body expected
+ printf Bug #42\nFixes: \nAcked-by= \nAcked-by= Junio\nAcked-by=
Peff\nReviewed-by: \nSigned-off-by: \n expected
+ git interpret-trailers review: fix=53 cc=Linus ack: Junio
fix=22 bug: 42 ack: Peff complex_message actual
+ test_cmp expected actual
+'
+
test_done
diff --git a/trailer.c b/trailer.c
index 8681aed..73a65e0 100644
--- a/trailer.c
+++ b/trailer.c
@@ -464,8 +464,13 @@ static struct strbuf **read_input_file(const char
*infile)
{
struct strbuf sb = STRBUF_INIT;
- if (strbuf_read_file(sb, infile, 0) 0)
- die_errno(_(could not read input file '%s'), infile);
+ if (infile) {
+ if (strbuf_read_file(sb, infile, 0) 0)
+ die_errno(_(could not read input file '%s'), infile);
+ } else {
+ if (strbuf_read(sb, fileno(stdin), 0) 0)
+ die_errno(_(could not read from stdin));
+ }
return strbuf_split(sb, '\n');
}
@@ -530,10 +535,8 @@ void process_trailers(const char *infile, int
trim_empty, int argc, const char *
git_config(git_trailer_config, NULL);
- /* Print the non trailer part of infile */
- if (infile) {
- process_input_file(infile, infile_tok_first, infile_tok_last);
- }
+ /* Print the non trailer part of infile (or stdin if infile is NULL) */
+ process_input_file(infile, infile_tok_first, infile_tok_last);
arg_tok_first = process_command_line_args(argc, argv);
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 01/14] Add data structures and basic functions for commit trailers
Christian Couder chrisc...@tuxfamily.org writes:
We will use a doubly linked list to store all information
about trailers and their configuration.
This way we can easily remove or add trailers to or from
trailer lists while traversing the lists in either direction.
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
Makefile | 1 +
trailer.c | 48
2 files changed, 49 insertions(+)
create mode 100644 trailer.c
diff --git a/Makefile b/Makefile
index b4af1e2..ec90feb 100644
--- a/Makefile
+++ b/Makefile
@@ -871,6 +871,7 @@ LIB_OBJS += submodule.o
LIB_OBJS += symlinks.o
LIB_OBJS += tag.o
LIB_OBJS += trace.o
+LIB_OBJS += trailer.o
LIB_OBJS += transport.o
LIB_OBJS += transport-helper.o
LIB_OBJS += tree-diff.o
diff --git a/trailer.c b/trailer.c
new file mode 100644
index 000..f129b5a
--- /dev/null
+++ b/trailer.c
@@ -0,0 +1,48 @@
+#include cache.h
+/*
+ * Copyright (c) 2013 Christian Couder chrisc...@tuxfamily.org
+ */
+
+enum action_where { WHERE_AFTER, WHERE_BEFORE };
+enum action_if_exist { EXIST_ADD_IF_DIFFERENT,
EXIST_ADD_IF_DIFFERENT_NEIGHBOR,
+EXIST_ADD, EXIST_OVERWRITE, EXIST_DO_NOTHING };
+enum action_if_missing { MISSING_ADD, MISSING_DO_NOTHING };
All these names and conf_info below are not named to be specific
to this little tool. Can I assume that these will never be exposed
to the rest of the system? If so, they are fine.
+struct conf_info {
+ char *name;
+ char *key;
+ char *command;
+ enum action_where where;
+ enum action_if_exist if_exist;
+ enum action_if_missing if_missing;
It still feels somewhat strange. It is true that an item can be
either exist or missing and it is understandable that it tempts
you to split that into two, but EXIST_OVERWRITE will not trigger
either WHERE_AFTER or WHERE_BEFORE action.
+};
+
+struct trailer_item {
+ struct trailer_item *previous;
+ struct trailer_item *next;
+ const char *token;
+ const char *value;
+ struct conf_info *conf;
+};
+
+static inline int same_token(struct trailer_item *a, struct trailer_item *b,
int alnum_len)
+{
+ return !strncasecmp(a-token, b-token, alnum_len);
+}
+
+static inline int same_value(struct trailer_item *a, struct trailer_item *b)
+{
+ return !strcasecmp(a-value, b-value);
+}
+
+static inline int same_trailer(struct trailer_item *a, struct trailer_item
*b, int alnum_len)
+{
+ return same_token(a, b, alnum_len) same_value(a, b);
+}
All these inlines look premature optimization that can be
delegated to any decent compiler, don't they?
+/* Get the length of buf from its beginning until its last alphanumeric
character */
+static inline size_t alnum_len(const char *buf, int len)
+{
+ while (--len = 0 !isalnum(buf[len]));
Style:
while (--len = 0 !isalnum(buf[len]))
;
You may add a comment on the empty statement to make it stand out
even more, i.e.
; /* nothing */
+ return (size_t) len + 1;
This is somewhat unfortunate. if the caller wants to receive
size_t, perhaps it should be passing in size_t (or ssize_t) to the
function? Hard to guess without an actual caller, though.
+}
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 01/14] Add data structures and basic functions for commit trailers
Christian Couder chrisc...@tuxfamily.org writes:
+ enum action_if_exist if_exist;
+ enum action_if_missing if_missing;
Probably if_exists is more gramatically correct.
if (x-if_exists) {
... do this ...
}
would read well, but not x-if_exist.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 02/14] trailer: process trailers from file and arguments
Christian Couder chrisc...@tuxfamily.org writes:
This patch implements the logic that process trailers
from file and arguments.
At the beginning trailers from file are in their own
infile_tok doubly linked list, and trailers from
arguments are in their own arg_tok doubly linked list.
The lists are traversed and when an arg_tok should be
applied, it is removed from its list and inserted
into the infile_tok list.
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
trailer.c | 187
++
1 file changed, 187 insertions(+)
diff --git a/trailer.c b/trailer.c
index f129b5a..ba0cfe0 100644
--- a/trailer.c
+++ b/trailer.c
@@ -46,3 +46,190 @@ static inline size_t alnum_len(const char *buf, int len)
while (--len = 0 !isalnum(buf[len]));
return (size_t) len + 1;
}
+
+static void add_arg_to_infile(struct trailer_item *infile_tok,
+ struct trailer_item *arg_tok)
+{
+ if (arg_tok-conf-where == WHERE_AFTER) {
+ arg_tok-next = infile_tok-next;
+ infile_tok-next = arg_tok;
+ arg_tok-previous = infile_tok;
+ if (arg_tok-next)
+ arg_tok-next-previous = arg_tok;
+ } else {
+ arg_tok-previous = infile_tok-previous;
+ infile_tok-previous = arg_tok;
+ arg_tok-next = infile_tok;
+ if (arg_tok-previous)
+ arg_tok-previous-next = arg_tok;
+ }
+}
+
+static int check_if_different(struct trailer_item *infile_tok,
+ struct trailer_item *arg_tok,
+ int alnum_len, int check_all)
+{
+ enum action_where where = arg_tok-conf-where;
+ do {
+ if (!infile_tok)
+ return 1;
+ if (same_trailer(infile_tok, arg_tok, alnum_len))
+ return 0;
+ /*
+ * if we want to add a trailer after another one,
+ * we have to check those before this one
+ */
+ infile_tok = (where == WHERE_AFTER) ? infile_tok-previous :
infile_tok-next;
+ } while (check_all);
+ return 1;
+}
+
+static void apply_arg_if_exist(struct trailer_item *infile_tok,
+struct trailer_item *arg_tok,
+int alnum_len)
+{
+ switch (arg_tok-conf-if_exist) {
+ case EXIST_DO_NOTHING:
+ free(arg_tok);
+ break;
+ case EXIST_OVERWRITE:
+ free((char *)infile_tok-value);
+ infile_tok-value = xstrdup(arg_tok-value);
+ free(arg_tok);
+ break;
+ case EXIST_ADD:
+ add_arg_to_infile(infile_tok, arg_tok);
+ break;
+ case EXIST_ADD_IF_DIFFERENT:
+ if (check_if_different(infile_tok, arg_tok, alnum_len, 1))
+ add_arg_to_infile(infile_tok, arg_tok);
+ else
+ free(arg_tok);
+ break;
+ case EXIST_ADD_IF_DIFFERENT_NEIGHBOR:
+ if (check_if_different(infile_tok, arg_tok, alnum_len, 0))
+ add_arg_to_infile(infile_tok, arg_tok);
+ else
+ free(arg_tok);
+ break;
Makes me wonder if people want a rule to say if the same key
already exists, regardless of the value.
+ }
+}
+
+static void remove_from_list(struct trailer_item *item,
+ struct trailer_item **first)
+{
+ if (item-next)
+ item-next-previous = item-previous;
+ if (item-previous)
+ item-previous-next = item-next;
+ else
+ *first = item-next;
+}
Will callers free the item that now is not on the list?
+static struct trailer_item *remove_first(struct trailer_item **first)
+{
+ struct trailer_item *item = *first;
+ *first = item-next;
+ if (item-next) {
+ item-next-previous = NULL;
+ item-next = NULL;
+ }
+ return item;
+}
+
+static void process_infile_tok(struct trailer_item *infile_tok,
+struct trailer_item **arg_tok_first,
+enum action_where where)
+{
+ struct trailer_item *arg_tok;
+ struct trailer_item *next_arg;
+
+ int tok_alnum_len = alnum_len(infile_tok-token,
strlen(infile_tok-token));
+ for (arg_tok = *arg_tok_first; arg_tok; arg_tok = next_arg) {
+ next_arg = arg_tok-next;
+ if (same_token(infile_tok, arg_tok, tok_alnum_len)
+ arg_tok-conf-where == where) {
+ remove_from_list(arg_tok, arg_tok_first);
+ apply_arg_if_exist(infile_tok, arg_tok, tok_alnum_len);
+ /*
+ * If arg has been added to infile,
+ * then we need to process it
Re: [PATCH v5 04/14] trailer: process command line trailer arguments
Christian Couder chrisc...@tuxfamily.org writes:
This patch parses the trailer command line arguments
and put the result into an arg_tok doubly linked
list.
No the patch doesn't parse anything ;-).
Parse the command line arguments and
+static void parse_trailer(struct strbuf *tok, struct strbuf *val, const char
*trailer)
+{
+ const char *end = strchr(trailer, '=');
+ if (!end)
+ end = strchr(trailer, ':');
How would you explain the behaviour of the above code for this
input?
frotz: nitfol=xyzzy
Perhaps strcspn()?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 07/14] trailer: add interpret-trailers command
Christian Couder chrisc...@tuxfamily.org writes:
diff --git a/git.c b/git.c
index 3799514..1420b58 100644
--- a/git.c
+++ b/git.c
@@ -383,6 +383,7 @@ static void handle_internal_command(int argc, const char
**argv)
{ index-pack, cmd_index_pack, RUN_SETUP_GENTLY },
{ init, cmd_init_db },
{ init-db, cmd_init_db },
+ { interpret-trailers, cmd_interpret_trailers, RUN_SETUP },
{ log, cmd_log, RUN_SETUP },
{ ls-files, cmd_ls_files, RUN_SETUP },
{ ls-remote, cmd_ls_remote, RUN_SETUP_GENTLY },
Does this even need to have a git repository? What is the RUN_SETUP
for?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 08/14] trailer: add tests for git interpret-trailers
Christian Couder chrisc...@tuxfamily.org writes: + +cat basic_message 'EOF' +subject + +body +EOF + +cat complex_message_body 'EOF' +my subject + +my body which is long +and contains some special +chars like : = ? ! + +EOF + +# We want one trailing space at the end of each line. +# Let's use sed to make sure that these spaces are not removed +# by any automatic tool. +sed -e 's/ Z$/ /' complex_message_trailers -\EOF +Fixes: Z +Acked-by: Z +Reviewed-by: Z +Signed-off-by: Z +EOF Please put things like these inside the first setup test. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 09/14] trailer: if no input file is passed, read from stdin
Christian Couder chrisc...@tuxfamily.org writes:
It is simpler and more natural if the git interpret-trailers
is made a filter as its output already goes to sdtout.
sdtout?
Why isn't this a pure filter without any infile parameter in the
first place?
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
builtin/interpret-trailers.c | 2 +-
t/t7513-interpret-trailers.sh | 7 +++
trailer.c | 15 +--
3 files changed, 17 insertions(+), 7 deletions(-)
diff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c
index 04b0ae2..ae8e561 100644
--- a/builtin/interpret-trailers.c
+++ b/builtin/interpret-trailers.c
@@ -23,7 +23,7 @@ int cmd_interpret_trailers(int argc, const char **argv,
const char *prefix)
struct option options[] = {
OPT_BOOL(0, trim-empty, trim_empty, N_(trim empty
trailers)),
- OPT_FILENAME(0, infile, infile, N_(use message from file)),
+ OPT_FILENAME(0, infile, infile, N_(use message from file,
instead of stdin)),
OPT_END()
};
diff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh
index 8be333c..f5ef81f 100755
--- a/t/t7513-interpret-trailers.sh
+++ b/t/t7513-interpret-trailers.sh
@@ -205,4 +205,11 @@ test_expect_success 'using ifMissing = doNothing' '
test_cmp expected actual
'
+test_expect_success 'with input from stdin' '
+ cat complex_message_body expected
+ printf Bug #42\nFixes: \nAcked-by= \nAcked-by= Junio\nAcked-by=
Peff\nReviewed-by: \nSigned-off-by: \n expected
+ git interpret-trailers review: fix=53 cc=Linus ack: Junio
fix=22 bug: 42 ack: Peff complex_message actual
+ test_cmp expected actual
+'
+
test_done
diff --git a/trailer.c b/trailer.c
index 186316f..108e104 100644
--- a/trailer.c
+++ b/trailer.c
@@ -483,8 +483,13 @@ static struct strbuf **read_input_file(const char
*infile)
{
struct strbuf sb = STRBUF_INIT;
- if (strbuf_read_file(sb, infile, 0) 0)
- die_errno(_(could not read input file '%s'), infile);
+ if (infile) {
+ if (strbuf_read_file(sb, infile, 0) 0)
+ die_errno(_(could not read input file '%s'), infile);
+ } else {
+ if (strbuf_read(sb, fileno(stdin), 0) 0)
+ die_errno(_(could not read from stdin));
+ }
return strbuf_split(sb, '\n');
}
@@ -551,10 +556,8 @@ void process_trailers(const char *infile, int
trim_empty, int argc, const char *
git_config(git_trailer_config, NULL);
- /* Print the non trailer part of infile */
- if (infile) {
- process_input_file(infile, infile_tok_first, infile_tok_last);
- }
+ /* Print the non trailer part of infile (or stdin if infile is NULL) */
+ process_input_file(infile, infile_tok_first, infile_tok_last);
arg_tok_first = process_command_line_args(argc, argv);
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 10/14] trailer: execute command from 'trailer.name.command'
Christian Couder chrisc...@tuxfamily.org writes:
+#define TRAILER_ARG_STRING $ARG
No need to support users who may want to use a string that happens
to match this substring literally as part of the command line?
struct trailer_item {
struct trailer_item *previous;
struct trailer_item *next;
@@ -56,6 +60,13 @@ static inline int contains_only_spaces(const char *str)
return !*s;
}
+static inline void strbuf_replace(struct strbuf *sb, const char *a, const
char *b)
Same comment about inline for an earlier patch applies.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 12/14] trailer: set author and committer env variables
Christian Couder chrisc...@tuxfamily.org writes:
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
trailer.c | 30 +-
1 file changed, 29 insertions(+), 1 deletion(-)
diff --git a/trailer.c b/trailer.c
index 98187fc..b5de616 100644
--- a/trailer.c
+++ b/trailer.c
@@ -1,5 +1,6 @@
#include cache.h
#include run-command.h
+#include argv-array.h
/*
* Copyright (c) 2013 Christian Couder chrisc...@tuxfamily.org
*/
@@ -433,14 +434,40 @@ static int read_from_command(struct child_process *cp,
struct strbuf *buf)
return 0;
}
+static void setup_ac_env(struct argv_array *env, const char *ac_name, const
char *ac_mail, const char *(*read)(int))
+{
+ if (!getenv(ac_name) || !getenv(ac_mail)) {
+ struct ident_split ident;
+ const char *namebuf, *mailbuf;
+ int namelen, maillen;
+ const char *ac_info = read(IDENT_NO_DATE);
This is far too confusing. Name it read_fn() or something.
+ if (split_ident_line(ident, ac_info, strlen(ac_info)))
+ return;
+
+ namelen = ident.name_end - ident.name_begin;
+ namebuf = ident.name_begin;
+
+ maillen = ident.mail_end - ident.mail_begin;
+ mailbuf = ident.mail_begin;
+
+ argv_array_pushf(env, %s=%.*s, ac_name, namelen, namebuf);
+ argv_array_pushf(env, %s=%.*s, ac_mail, maillen, mailbuf);
+ }
+}
+
static const char *apply_command(const char *command, const char *arg)
{
+ struct argv_array env = ARGV_ARRAY_INIT;
struct strbuf cmd = STRBUF_INIT;
struct strbuf buf = STRBUF_INIT;
struct child_process cp;
const char *argv[] = {NULL, NULL};
const char *result = ;
+ setup_ac_env(env, GIT_AUTHOR_NAME, GIT_AUTHOR_EMAIL,
git_author_info);
+ setup_ac_env(env, GIT_COMMITTER_NAME, GIT_COMMITTER_EMAIL,
git_committer_info);
+
strbuf_addstr(cmd, command);
if (arg)
strbuf_replace(cmd, TRAILER_ARG_STRING, arg);
@@ -448,7 +475,7 @@ static const char *apply_command(const char *command,
const char *arg)
argv[0] = cmd.buf;
memset(cp, 0, sizeof(cp));
cp.argv = argv;
- cp.env = local_repo_env;
+ cp.env = env.argv;
cp.no_stdin = 1;
cp.out = -1;
cp.use_shell = 1;
@@ -459,6 +486,7 @@ static const char *apply_command(const char *command,
const char *arg)
result = strbuf_detach(buf, NULL);
strbuf_release(cmd);
+ argv_array_clear(env);
return result;
}
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 12/14] trailer: set author and committer env variables
Christian Couder chrisc...@tuxfamily.org writes:
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
trailer.c | 30 +-
1 file changed, 29 insertions(+), 1 deletion(-)
diff --git a/trailer.c b/trailer.c
index 98187fc..b5de616 100644
--- a/trailer.c
+++ b/trailer.c
@@ -1,5 +1,6 @@
#include cache.h
#include run-command.h
+#include argv-array.h
/*
* Copyright (c) 2013 Christian Couder chrisc...@tuxfamily.org
*/
@@ -433,14 +434,40 @@ static int read_from_command(struct child_process *cp,
struct strbuf *buf)
return 0;
}
+static void setup_ac_env(struct argv_array *env, const char *ac_name, const
char *ac_mail, const char *(*read)(int))
+{
+ if (!getenv(ac_name) || !getenv(ac_mail)) {
Whoever is using this tool while replaying an existing commit likely
wants to export these two variables _into_ this command from the
outside, and this function will not interfere with it.
But for other uses, do we need to export these variables? After
all, this matters _only_ when the command we spawn wants to know
the idents to be used, but they can ask git-var themselves to cover
both cases, whether the environment that called this tool already
had the ident environment variables or it didn't.
So I am not sure what value this step is adding to the series.
+ struct ident_split ident;
+ const char *namebuf, *mailbuf;
+ int namelen, maillen;
+ const char *ac_info = read(IDENT_NO_DATE);
+
+ if (split_ident_line(ident, ac_info, strlen(ac_info)))
+ return;
+
+ namelen = ident.name_end - ident.name_begin;
+ namebuf = ident.name_begin;
+
+ maillen = ident.mail_end - ident.mail_begin;
+ mailbuf = ident.mail_begin;
+
+ argv_array_pushf(env, %s=%.*s, ac_name, namelen, namebuf);
+ argv_array_pushf(env, %s=%.*s, ac_mail, maillen, mailbuf);
+ }
+}
+
static const char *apply_command(const char *command, const char *arg)
{
+ struct argv_array env = ARGV_ARRAY_INIT;
struct strbuf cmd = STRBUF_INIT;
struct strbuf buf = STRBUF_INIT;
struct child_process cp;
const char *argv[] = {NULL, NULL};
const char *result = ;
+ setup_ac_env(env, GIT_AUTHOR_NAME, GIT_AUTHOR_EMAIL,
git_author_info);
+ setup_ac_env(env, GIT_COMMITTER_NAME, GIT_COMMITTER_EMAIL,
git_committer_info);
+
strbuf_addstr(cmd, command);
if (arg)
strbuf_replace(cmd, TRAILER_ARG_STRING, arg);
@@ -448,7 +475,7 @@ static const char *apply_command(const char *command,
const char *arg)
argv[0] = cmd.buf;
memset(cp, 0, sizeof(cp));
cp.argv = argv;
- cp.env = local_repo_env;
+ cp.env = env.argv;
cp.no_stdin = 1;
cp.out = -1;
cp.use_shell = 1;
@@ -459,6 +486,7 @@ static const char *apply_command(const char *command,
const char *arg)
result = strbuf_detach(buf, NULL);
strbuf_release(cmd);
+ argv_array_clear(env);
return result;
}
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/6] fetch-pack: fix deepen shallow over smart http with no-done cap
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes: diff --git a/t/t5537-fetch-shallow.sh b/t/t5537-fetch-shallow.sh index b0fa738..fb11073 100755 --- a/t/t5537-fetch-shallow.sh +++ b/t/t5537-fetch-shallow.sh @@ -200,5 +200,29 @@ EOF ) ' +# This test is tricky. We need large enough haves that fetch-pack +# will put pkt-flush in between. Then we need a have the the server +# does not have, it'll send ACK %s ready +test_expect_success 'add more commits' ' + ( + cd shallow + for i in $(seq 10); do + git checkout --orphan unrelated$i + test_commit unrelated$i /dev/null + git push -q $HTTPD_DOCUMENT_ROOT_PATH/repo.git refs/heads/unrelated$i:refs/heads/unrelated$i + git push -q ../clone/.git refs/heads/unrelated$i:refs/heads/unrelated$i In addition to two problems Eric and Peff noticed, chain is broken between these two pushes. I initially didn't notice it but it became obvious after reformatting to fix the indentation. Here is the difference between the posted series and what I queued after applying the changes suggested during the review. Thanks. Documentation/technical/pack-protocol.txt | 4 +-- Documentation/technical/protocol-capabilities.txt | 10 +++ t/t5537-fetch-shallow.sh | 34 +-- 3 files changed, 26 insertions(+), 22 deletions(-) diff --git a/Documentation/technical/pack-protocol.txt b/Documentation/technical/pack-protocol.txt index eb0b8a1..39c6410 100644 --- a/Documentation/technical/pack-protocol.txt +++ b/Documentation/technical/pack-protocol.txt @@ -338,8 +338,8 @@ during a prior round. This helps to ensure that at least one common ancestor is found before we give up entirely. Once the 'done' line is read from the client, the server will either -send a final 'ACK obj-id' or it will send a 'NAK'. 'obj-id' is the -last SHA-1 determined to be common. The server only sends +send a final 'ACK obj-id' or it will send a 'NAK'. 'obj-id' is the object +name of the last commit determined to be common. The server only sends ACK after 'done' if there is at least one common base and multi_ack or multi_ack_detailed is enabled. The server always sends NAK after 'done' if there is no common base found. diff --git a/Documentation/technical/protocol-capabilities.txt b/Documentation/technical/protocol-capabilities.txt index cb2f5eb..e174343 100644 --- a/Documentation/technical/protocol-capabilities.txt +++ b/Documentation/technical/protocol-capabilities.txt @@ -77,15 +77,15 @@ section Packfile Negotiation for more information. no-done --- -This capability should be only used with smart HTTP protocol. If +This capability should only be used with the smart HTTP protocol. If multi_ack_detailed and no-done are both present, then the sender is free to immediately send a pack following its first ACK obj-id ready message. -Without no-done in smart HTTP protocol, the server session would end -and the client has to make another trip to send done and the server -can send the pack. no-done removes the last round and thus slightly -reduces latency. +Without no-done in the smart HTTP protocol, the server session would +end and the client has to make another trip to send done before +the server can send the pack. no-done removes the last round and +thus slightly reduces latency. thin-pack - diff --git a/t/t5537-fetch-shallow.sh b/t/t5537-fetch-shallow.sh index fb11073..1413caf 100755 --- a/t/t5537-fetch-shallow.sh +++ b/t/t5537-fetch-shallow.sh @@ -201,26 +201,30 @@ EOF ' # This test is tricky. We need large enough haves that fetch-pack -# will put pkt-flush in between. Then we need a have the the server +# will put pkt-flush in between. Then we need a have the server # does not have, it'll send ACK %s ready test_expect_success 'add more commits' ' ( - cd shallow - for i in $(seq 10); do - git checkout --orphan unrelated$i - test_commit unrelated$i /dev/null - git push -q $HTTPD_DOCUMENT_ROOT_PATH/repo.git refs/heads/unrelated$i:refs/heads/unrelated$i - git push -q ../clone/.git refs/heads/unrelated$i:refs/heads/unrelated$i - done - git checkout master - test_commit new - git push $HTTPD_DOCUMENT_ROOT_PATH/repo.git master + cd shallow + for i in $(test_seq 10) + do + git checkout --orphan unrelated$i + test_commit unrelated$i + git push -q $HTTPD_DOCUMENT_ROOT_PATH/repo.git \ + refs/heads/unrelated$i:refs/heads/unrelated$i + git push -q ../clone/.git \ + refs/heads/unrelated$i:refs/heads/unrelated$i || + exit 1 + done + git checkout master + test_commit new + git push $HTTPD_DOCUMENT_ROOT_PATH/repo.git master )
Re: [PATCH v5 07/14] trailer: add interpret-trailers command
Christian Couder christian.cou...@gmail.com writes:
On Fri, Feb 7, 2014 at 1:10 AM, Junio C Hamano gits...@pobox.com wrote:
Christian Couder chrisc...@tuxfamily.org writes:
diff --git a/git.c b/git.c
index 3799514..1420b58 100644
--- a/git.c
+++ b/git.c
@@ -383,6 +383,7 @@ static void handle_internal_command(int argc, const
char **argv)
{ index-pack, cmd_index_pack, RUN_SETUP_GENTLY },
{ init, cmd_init_db },
{ init-db, cmd_init_db },
+ { interpret-trailers, cmd_interpret_trailers, RUN_SETUP },
{ log, cmd_log, RUN_SETUP },
{ ls-files, cmd_ls_files, RUN_SETUP },
{ ls-remote, cmd_ls_remote, RUN_SETUP_GENTLY },
Does this even need to have a git repository? What is the RUN_SETUP
for?
It needs to read git config files, but it could work without reading them too.
I will have another look at it.
Of course. At this point in the series while reviewing 7/14 there
was no config [*1*] and that was why I was scratching my head.
[Footnote]
*1* Flipping the series structure to a top-down fashion, having an
almost no-op command that fails all the new tests in the beginning
and then building the internal incrementally, might be a worthwhile
change, but it is *not* worth the effort to add the command without
RUN_SETUP at 7/14 and then change the same line to have RUN_SETUP
when you start to need it could be an option; I am *not* suggesting
that.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 10/14] trailer: execute command from 'trailer.name.command'
Christian Couder chrisc...@tuxfamily.org writes: Signed-off-by: Christian Couder chrisc...@tuxfamily.org execute command from ... is fine, but I wish there were more details before S-o-b line. Is is not worth explaining what happens to the output, and what the facility is used for in general? Is it to give a string used for the value part? Key: Value string? Is the command allowed to say Put an entry with this string before the existing one, after the existing one, or replace the existing one? Can it say Upon inspection of the existing entry, it is no longer necessary to have it in the footer---remove it? -- To unsubscribe from this list: send the line unsubscribe 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/6] Fix the shallow deepen bug with no-done
Jonathan Nieder jrnie...@gmail.com writes: Duy Nguyen wrote: Don't take it the wrong way. I was just summarizing the last round. It surprised me though that this went under my radar. Perhaps a bug tracker is not a bad idea after all (if Jeff went missing, this bug could fall under the crack) I'm happy to plug - http://bugs.debian.org/cgi-bin/pkgreport.cgi?src=git;include=tags:upstream - http://packages.qa.debian.org/common/index.html (email subscription link: source package = git; under Advanced it's possible to subscribe to bug-tracking system emails and skip e.g. the automated build stuff) - https://www.debian.org/Bugs/Reporting (bug reporting interface - unfortunately the important part is buried under Sending the bug report via e-mail) again. :) Then I'd add my bits ;-) http://git-blame.blogspot.com/p/leftover-bits.html Admittedly I haven't touched it for a while, as I usually update it during a lull, often in the pre-release -rc freeze period, but the list has been quite active during -rc this cycle. I probably should start dropping any new topics on the list and find leftover bits during this cycle. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 07/14] trailer: add interpret-trailers command
Junio C Hamano gits...@pobox.com writes: ... RUN_SETUP at 7/14 and then change the same line to have RUN_SETUP when you start to need it could be an option; I am *not* suggesting that. Sorry, typo. s/could be an option;/;/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
What's cooking in git.git (Feb 2014, #03; Fri, 7)
Here are the topics that have been cooking. Commits prefixed with
'-' are only in 'pu' (proposed updates) while commits prefixed with
'+' are in 'next'.
The tip of 'master' has been tagged as v1.9.0-rc3. As a workaround
to make life easier for third-party tools, the upcoming major
release will be called Git 1.9.0 (not Git 1.9). The first
maintenance release for it will be Git 1.9.1, and the major
release after Git 1.9.0 will either be Git 2.0.0 or Git
1.10.0.
The list ended up relatively active during the pre-release feature
freeze period in this cycle, and we still see new topics appearing
for the next cycle. I do not know if that is a good thing or not.
Let's spend some time to ensure that there is no brown paper bag
bugs remaining on the master front until the final, scheduled to
happen sometime late next week.
You can find the changes described here in the integration branches
of the repositories listed at
http://git-blame.blogspot.com/p/git-public-repositories.html
--
[Graduated to master]
* aj/ada-diff-word-pattern (2014-02-05) 1 commit
(merged to 'next' on 2014-02-06 at 8062b22)
+ userdiff: update Ada patterns
* nd/tag-doc (2014-02-04) 1 commit
(merged to 'next' on 2014-02-06 at 9f02991)
+ git-tag.txt: commit for --contains is optional
* ow/manpages-typofix (2014-02-05) 1 commit
(merged to 'next' on 2014-02-06 at b482b8f)
+ Documentation: fix typos in man pages
--
[New Topics]
* jc/check-attr-honor-working-tree (2014-02-06) 2 commits
- check-attr: move to the top of working tree when in non-bare repository
- t0003: do not chdir the whole test process
git check-attr when (trying to) work on a repository with a
working tree did not work well when the working tree was specified
via --work-tree (and obviously with --git-dir).
The command also works in a bare repository but it reads from the
(possibly stale, irrelevant and/or nonexistent) index, which may
need to be fixed to read from HEAD, but that is a completely
separate issue. As a related tangentto this separate issue, we may
want to also fix check-ignore, which refuses to work in a bare
repository, to also operate in a bare one.
* nd/http-fetch-shallow-fix (2014-02-07) 6 commits
- fetch-pack: fix deepen shallow over smart http with no-done cap
- protocol-capabilities.txt: document no-done
- protocol-capabilities.txt: refer multi_ack_detailed back to pack-protocol.txt
- pack-protocol.txt: clarify 'obj-id' in the last ACK after 'done'
- t5538: fix default http port
- test: rename http fetch and push test files
Attempting to deepen a shallow repository by fetching over smart
HTTP transport failed in the protocol exchange, when no-done
extension was used. The fetching side waited for the list of
shallow boundary commits after the sending end stopped talking to
it.
Will merge to 'next'.
--
[Stalled]
* po/everyday-doc (2014-01-27) 1 commit
- Make 'git help everyday' work
This may make the said command to emit something, but the source is
not meant to be formatted into a manual pages to begin with, and
also its contents are a bit stale. It may be a good first step in
the right direction, but needs more work to at least get the
mark-up right before public consumption.
Will hold.
* jk/branch-at-publish-rebased (2014-01-17) 5 commits
- t1507 (rev-parse-upstream): fix typo in test title
- implement @{publish} shorthand
- branch_get: provide per-branch pushremote pointers
- branch_get: return early on error
- sha1_name: refactor upstream_mark
Give an easier access to the tracking branches from other side in
a triangular workflow by introducing B@{publish} that works in a
similar way to how B@{upstream} does.
Meant to be used as a basis for whatever Ram wants to build on.
Will hold.
* rb/merge-prepare-commit-msg-hook (2014-01-10) 4 commits
- merge: drop unused arg from abort_commit method signature
- merge: make prepare_to_commit responsible for write_merge_state
- t7505: ensure cleanup after hook blocks merge
- t7505: add missing
Expose more merge states (e.g. $GIT_DIR/MERGE_MODE) to hooks that
run during git merge. The log message stresses too much on one
hook, prepare-commit-msg, but it would equally apply to other hooks
like post-merge, I think.
Waiting for a reroll.
* jl/submodule-recursive-checkout (2013-12-26) 5 commits
- Teach checkout to recursively checkout submodules
- submodule: teach unpack_trees() to update submodules
- submodule: teach unpack_trees() to repopulate submodules
- submodule: teach unpack_trees() to remove submodule contents
- submodule: prepare for recursive checkout of submodules
Expecting a reroll.
* jc/graph-post-root-gap (2013-12-30) 3 commits
- WIP: document what we want at the end
- graph: remove unused code a bit
- graph: stuff the current commit into graph-columns[]
[ANNOUNCE] Git v1.9.0-rc3
to sha1_object_info() that was used to
obtain the metainfo (e.g. type size) about the object. This led
callers to weird inconsistencies.
(merge 663a856 cc/replace-object-info later to maint).
* git cat-file --batch=, an admittedly useless command, did not
behave very well.
(merge 6554dfa jk/cat-file-regression-fix later to maint).
* git rev-parse revs -- paths did not implement the usual
disambiguation rules the commands in the git log family used in
the same way.
(merge 62f162f jk/rev-parse-double-dashes later to maint).
* git mv A B/, when B does not exist as a directory, should error
out, but it didn't.
(merge c57f628 mm/mv-file-to-no-such-dir-with-slash later to maint).
* A workaround to an old bug in glibc prior to glibc 2.17 has been
retired; this would remove a side effect of the workaround that
corrupts system error messages in non-C locales.
* SSL-related options were not passed correctly to underlying socket
layer in git send-email.
(merge 5508f3e tr/send-email-ssl later to maint).
* git commit -v appends the patch to the log message before
editing, and then removes the patch when the editor returned
control. However, the patch was not stripped correctly when the
first modified path was a submodule.
(merge 1a72cfd jl/commit-v-strip-marker later to maint).
* git fetch --depth=0 was a no-op, and was silently ignored.
Diagnose it as an error.
(merge 5594bca nd/transport-positive-depth-only later to maint).
* Remote repository URL expressed in scp-style host:path notation are
parsed more carefully (e.g. foo/bar:baz is local, [::1]:/~user asks
to connect to user's home directory on host at address ::1.
(merge a2036d7 tb/clone-ssh-with-colon-for-port later to maint).
* git diff -- ':(icase)makefile' was unnecessarily rejected at the
command line parser.
(merge 887c6c1 nd/magic-pathspec later to maint).
* git cat-file --batch-check=ok did not check the existence of
the named object.
(merge 4ef8d1d sb/sha1-loose-object-info-check-existence later to maint).
* git am --abort sometimes complained about not being able to write
a tree with an 0{40} object in it.
(merge 77b43ca jk/two-way-merge-corner-case-fix later to maint).
* Two processes creating loose objects at the same time could have
failed unnecessarily when the name of their new objects started
with the same byte value, due to a race condition.
(merge b2476a6 jh/loose-object-dirs-creation-race later to maint).
Changes since v1.9-rc2 are as follows:
Adrian Johnson (1):
userdiff: update Ada patterns
Junio C Hamano (3):
Git 1.8.5.4
howto/maintain-git.txt: new version numbering scheme
Git 1.9.0-rc3
Nguyễn Thái Ngọc Duy (1):
git-tag.txt: commit for --contains is optional
Torsten Bögershausen (1):
repack.c: rename and unlink pack file if it exists
Øystein Walle (1):
Documentation: fix typos in man pages
--
To unsubscribe from this list: send the line unsubscribe 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] gitweb: Added syntax highlight support for golang
Pavan Kumar Sunkara pavan.sss1...@gmail.com writes:
Golang is quickly becoming one of the major programming languages.
This change switches on golang syntax highlight support by default
in gitweb rather than asking the users to do it using config files.
Looks trivially harmless ;-)
I haven't touched this part of our system, but the patch makes me
wonder if there is a way for us to _ask_ the installed 'highlight'
binary what languages it knows about. This hash is used only in
guess_file_syntax sub, and it may not be unreasonable to populate it
lazily there, or at least generate this part by parsing output from
'highlight -p' at build-install time.
Signed-off-by: Pavan Kumar Sunkara pavan.sss1...@gmail.com
---
gitweb/gitweb.perl |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index bf7fd67..aa6fcfd 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -273,7 +273,7 @@ our %highlight_basename = (
our %highlight_ext = (
# main extensions, defining name of syntax;
# see files in /usr/share/highlight/langDefs/ directory
- (map { $_ = $_ } qw(py rb java css js tex bib xml awk bat ini spec tcl
sql)),
+ (map { $_ = $_ } qw(py rb java go css js tex bib xml awk bat ini spec
tcl sql)),
# alternate extensions, see /etc/highlight/filetypes.conf
(map { $_ = 'c' } qw(c h)),
(map { $_ = 'sh' } qw(sh bash zsh ksh)),
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [WIP/PATCH 4/9] Teach merge the --[no-]recurse-submodules option
Jens Lehmann jens.lehm...@web.de writes: I think the user needs to sort things out, just like she has to do when a file has a merge conflict. But unfortunately we cannot use conflict markers here, so I'd propose the following: * When merge proposes a merge resolution (which it does today by telling the user Found a possible merge resolution for the submodule ... [use] git update-index --cacheinfo 16 ...) that commit should be checked out in the submodule but not staged. Then the user can simply add and commit. * If the merge resolution is not obvious to merge, it leaves the submodule in an unmerged state, the local commit still being checked out. The user has to manually do the merge in the submodule and commits that in the superproject. Does that make sense? The latter one does not worry me too much. For the former, add and commit at the top-level makes perfect sense, and the commit should be checked out in the submodule is a necessary step to sanity-check and prepare for that add and commit step, but what does checked out in the submodule exactly mean? Do we detach the HEAD at the commit? Do we advance the tip of the branch of the submodule to that commit? Do we know/require/care if such a move always fast-forwards? -- To unsubscribe from this list: send the line unsubscribe 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] gitweb: Added syntax highlight support for golang
Pavan Kumar Sunkara pavan.sss1...@gmail.com writes: Sorry. I misunderstood your message. Yes, I guess lazy loading the supported file extensions would be better. But not all highlighters support `-p` option. So, I think its better to leave it to the user. Yes, those highlighters that do not support `-p` may have to rely on the hard-coded list %highlight_ext. But with the same line of reasoning, not all versions of highligher supports 'go' language, so it's better to leave that to the user, no? The version of 'highlight' you may have may know about 'go', and somebody else's 'highlight' may not yet. A hard-coded list that appears in %highlight_ext will be correct for only one of you while the other between you two needs to customize it to his system. Note that I was not talking about removing the configurability. Even with lazy loading and/or auto-genearting at build-install time when 'highlight -p' is available, the users still want to be able to customize, and supporting that is fine. But for those whose 'highlight' does support '-p', it will help to lazily discover the list of supported languages and/or enumarate them at build-install time. They do not have to keep adding new language (or removing it from the list we give as the upstream) to adjust it to their system. In any case, the comment was not about this patch from you, but about the future direction for the code it touches in general. In other words, it did not mean because it does not update the mechanism to lazily discover the list of languages, and instead added yet another language to the existing one, it is not an acceptable solution to start supporting 'go'. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/2] Ignore trailing spaces in .gitignore
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes: Trailing spaces are invisible in most standard editors (*). git diff does show trailing spaces by default. But that does not help newly written .gitignore files. And trailing spaces are the source of frustration when writing .gitignore. So let's ignore them. Nobody sane would put a trailing space in file names. But we could be careful and do it in two steps: warn first, then ignore trailing spaces. Another option is merge two patches in one and be done with it. People can still quote trailing spaces, which will not be ignored (and much more visible). Quoting comes with a cost of doing fnmatch(). But Hmph, sorry but I fail to see why we need to incur cost for fnmatch(). We read and parse the file and keep them as internal strings, so your unquoting (and complaining the unquoted trailng spaces) can be done at the parse time, while keeping the trailing spaces the user explicitly told us to keep by quoting in the internal string that we eventually feed fnmatch() with _after_ unquoting, no? 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: [PATCH 0/2] Ignore trailing spaces in .gitignore
Junio C Hamano gits...@pobox.com writes: Nguyễn Thái Ngọc Duy pclo...@gmail.com writes: Trailing spaces are invisible in most standard editors (*). git diff does show trailing spaces by default. But that does not help newly written .gitignore files. And trailing spaces are the source of frustration when writing .gitignore. So let's ignore them. Nobody sane would put a trailing space in file names. But we could be careful and do it in two steps: warn first, then ignore trailing spaces. Another option is merge two patches in one and be done with it. People can still quote trailing spaces, which will not be ignored (and much more visible). Quoting comes with a cost of doing fnmatch(). But Hmph, sorry but I fail to see why we need to incur cost for fnmatch(). We read and parse the file and keep them as internal strings, so your unquoting (and complaining the unquoted trailng spaces) can be done at the parse time, while keeping the trailing spaces the user explicitly told us to keep by quoting in the internal string that we eventually feed fnmatch() with _after_ unquoting, no? Puzzled... Heh, silly me. Yes, we _could_ parse and strip \ into if we wanted to, but we can just give \ and let fnmatch(3) do its thing, and that is the right thing to do for a rare corner case like this (i.e. deliberate trailing spaces). So I think I understand your reasoning, and I agree with it. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 02/14] trailer: process trailers from file and arguments
Christian Couder chrisc...@tuxfamily.org writes: This is what if_exists and if_missing are all about. Either: the same key already exists regardless of the value and, in this case, what happens depends on what has been specified using the if_exists configuration variable. Or: the same key DOES NOT already exists regardless of the value and in this case, what happens depends on what has been specified using the if_missing configuration variable. Hmm, how should things like signed-off-by be handled? You may want to allow many entries with the same key with distinct values, but duplicated values may want to be handled differently (currently we only avoid to place a duplicate key, value consecutively, but keys with different semantics (e.g. fixes-bug: #bugid) may want to say unless the same key with the same value exists, append it at the end. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/6] fetch-pack: fix deepen shallow over smart http with no-done cap
Duy Nguyen pclo...@gmail.com writes: On Fri, Feb 07, 2014 at 10:01:08AM -0800, Junio C Hamano wrote: Here is the difference between the posted series and what I queued after applying the changes suggested during the review. Thanks. I was going to send a reroll after the received comments. Could you put this on top of 6/6, just to make sure future changes in t5537 (maybe more or less commits created..) does not change the test behavior? It fixes the test name too. I originally thought, ok let's create commits in one test and do fetch in another. But it ended up in the same test and I forgot to update test name. Surely, and thanks for being careful. Will squash it in. -- 8 -- diff --git a/t/t5537-fetch-shallow.sh b/t/t5537-fetch-shallow.sh index 1413caf..b300383 100755 --- a/t/t5537-fetch-shallow.sh +++ b/t/t5537-fetch-shallow.sh @@ -203,7 +203,7 @@ EOF # This test is tricky. We need large enough haves that fetch-pack # will put pkt-flush in between. Then we need a have the server # does not have, it'll send ACK %s ready -test_expect_success 'add more commits' ' +test_expect_success 'no shallow lines after receiving ACK ready' ' ( cd shallow for i in $(test_seq 10) @@ -224,7 +224,9 @@ test_expect_success 'add more commits' ' cd clone git checkout --orphan newnew test_commit new-too - git fetch --depth=2 + GIT_TRACE_PACKET=$TRASH_DIRECTORY/trace git fetch --depth=2 + grep fetch-pack ACK .* ready ../trace + ! grep fetch-pack done ../trace ) ' -- 8 -- -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/6] t5538: fix default http port
Jeff King p...@peff.net writes: Here it is as a Real Patch™. I just based it on master, so it can replace your 5537/5538 fix in your series. Thanks, looks good. Will put this at the bottom and rebuild the nd/http-fetch-shallow-fix series on top. 1. Is there anybody who has apache installed who would _not_ want to bring it up for the test? 2. Is there anybody for whom the failure mode of bringing up apache would be unpleasant (e.g., if it hangs the tests or something)? For (1), we could perhaps have a GIT_NO_TEST_HTTPD to avoid it. For (2), I suspect we may need to make our error handling more robust, but getting people to run it is the first step to figuring out what the problems are. If we go this route, we should probably do the same for GIT_TEST_GIT_DAEMON in t5570 (and for that matter, we should probably do the same for the port numbers). All good points. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: bash completion patch
Thomas Rast t...@thomasrast.ch writes: Junio C Hamano gits...@pobox.com writes: Matthieu Moy matthieu@grenoble-inp.fr writes: [...] don't forget to Cc Junio if you think your patch is ready for inclusion. Heh, thanks. Everybody seems to think anything they send out to the list is ready for inclusion, so the last part may not be a piece of advice that is practically very useful, though ;-) That happens to me a lot, too. Perhaps it would be a clearer signal if you had an alias (or just something like gitster+patch) that we can send it to if we mean please include instead of what do you think of this? The intention from regulars like you I can read from the tone of the message (or if you want to you can mention it in the log message). If a clearer signal is really needed, perhaps we should say something like: Send any patch that has not been reviewed on the list fist to the list and area experts (you can learn who they are by running git blame and git shortlog on the part of the system you are touching) for review. Once the patch gains list consensus that it is a good change, and the maintainer hasn't picked it up (perhaps it fell through cracks), resend it to the maintainer with Cc: to the list. We could phrase it more brutally: If it is the first time a particular patch is sent to the list, it almost always is not ready for inclusion. but I do not think that is a good idea. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/2] gc: config option for running --auto in background
Duy Nguyen pclo...@gmail.com writes:
On Mon, Feb 10, 2014 at 6:03 PM, Erik Faye-Lund kusmab...@gmail.com wrote:
`gc --auto` takes time and can block the user temporarily (but not any
- if (!quiet)
- fprintf(stderr,
- _(Auto packing the repository for
optimum performance. You may also\n
- run \git gc\ manually. See
- \git help gc\ for more
information.\n));
+ if (!quiet) {
+ if (detach_auto)
+ fprintf(stderr, _(Auto packing the
repository in background for optimum performance.\n));
+ else
+ fprintf(stderr, _(Auto packing the
repository for optimum performance.\n));
+ fprintf(stderr, _(See \git help gc\ for manual
housekeeping.\n));
+ }
+ if (detach_auto)
+ /*
+* failure to daemonize is ok, we'll continue
+* in foreground
+*/
+ daemonize();
While I agree that it should be OK, shouldn't we warn the user?
If --quiet is set, we should not be printing anyway. If not, I thinkg
we could only print auto packing in background.. when we actually
can do that, else just print the old message. It means an #ifdef
NO_POSIX_GOODIES here again though..
Didn't you change it not to die but return nosys or something?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/2] daemon: move daemonize() to libgit.a
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes:
diff --git a/setup.c b/setup.c
index 6c3f85f..b09a412 100644
--- a/setup.c
+++ b/setup.c
@@ -787,3 +787,27 @@ void sanitize_stdfds(void)
if (fd 2)
close(fd);
}
+
+int daemonize(void)
+{
+#ifdef NO_POSIX_GOODIES
+ errno = -ENOSYS;
Negated?
+ return -1;
+#else
+ switch (fork()) {
+ case 0:
+ break;
+ case -1:
+ die_errno(fork failed);
+ default:
+ exit(0);
+ }
+ if (setsid() == -1)
+ die_errno(setsid failed);
+ close(0);
+ close(1);
+ close(2);
+ sanitize_stdfds();
+ return 0;
+#endif
+}
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Documentation about push.default=upstream is confusing
Ingo Rohloff lund...@gmx.de writes: To handle that I setup several remote tracking branches called: repo1_master (tracks repo1/master) repo2_master (tracks repo2/master) reap3_master (tracks repo3/master) Now without push.default=upstream I would have to either always explicitly do something like: git push repo1 repo1_master:master git push repo2 repo2_master:master If you think about your interaction with people who are only looking at repo1 alone, you _are_ using a centralized workflow. You are not the only one who update their 'master'; other people push there to update that 'master' and you pull it in to keep you up to date and further build your changes on top. Such an interaction with other people by using repo1 as the shared meeting point is well served by the push-to-upstream mechanism, and that kind of interaction is called centralized workflow. The illustration from you is running one centralized workflow with each of the three repositories. The de-centralized workflow the message hints (but does not talk about) is different. It is not uncommon to pull from one place and then to push the result out to your own publishing branch (e.g. clone from anna, fetch to keep up to date with her, work on it, publish to your repository to tell her to fetch from you), and push-to-upstream is not very well suited for that topology. You may clone her for-bob branch, but you do not push it back to her repository to update her for-bob branch. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/2] gc: config option for running --auto in background
On Mon, Feb 10, 2014 at 10:43 AM, Junio C Hamano gits...@pobox.com wrote: If --quiet is set, we should not be printing anyway. If not, I thinkg we could only print auto packing in background.. when we actually can do that, else just print the old message. It means an #ifdef NO_POSIX_GOODIES here again though.. Didn't you change it not to die but return nosys or something? Ah, the problem is that it is too late to take back ... will do so in the background when you noticed that daemonize() did not succeed, so you would need a way to see if we can daemonize() before actually doing so if you want to give different messages. int can_daemonize(void) could be an answer that is nicer than NO_POSIX_GOODIES, but I am not sure if it is worth it. -- To unsubscribe from this list: send the line unsubscribe 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] hashmap.h: make sure map entries are tightly packed
Karsten Blees karsten.bl...@gmail.com writes: ... At the very least we shouldn't stall the rest of the patch series on a hunch that the last (unfortunately non-standard) patch may break on some legacy system. Oh, no question about it. I was planning split out the last one and merge the rest to 'master' after the current cycle. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
