Re: What's cooking in git.git (Aug 2013, #06; Tue, 27)
Am 8/27/2013 23:48, schrieb Jeff King: The counterarguments I can see are: 1. Who cares? If you want to know whether pack-objects will choke on your huge config value, then run pack-objects. 2. Such a check would involve knowing which type we use internally to look at packSizeLimit, and that is utterly undocumented (and subject to change; e.g., it seems kind of senseless that we have a 4G pack-size limit on 32-bit platforms, and we may want to fix that). So if you do not buy the argument that communicating git's internal range checks is useful, then we can simply say --int is magically long on every platform, and you can use it for everything numeric. And implement it with int64_t. You may be able to read or write some values for certain keys that git will barf on internally, but that is git's problem. I'm in the camp of these (counter) arguments. When my shell script asks for 'git config --int 3g', I expect to be returned a positive 10-digit. What would I care which type Git or any other tool is using internally? I only care whether my shell can work with numbers that large. Or the next tool that I feed the number to. But that's my business, not Git's. The one thing it doesn't get you is that you can currently set unsigned values to -1 in the config to have them treated as ULONG_MAX. This is undocumented and as far as I know not used by anyone. And it better stays that way. Magic numbers should be encoded with magic strings in the config file. -- Hannes -- To unsubscribe from this list: send the line unsubscribe 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/5] replace: forbid replacing an object with one of a different type
Junio C Hamano gits...@pobox.com writes:
Christian Couder chrisc...@tuxfamily.org writes:
Users replacing an object with one of a different type were not
prevented to do so, even if it was obvious, and stated in the doc,
that bad things would result from doing that.
To avoid mistakes, it is better to just forbid that though.
There is no case where one object can be replaced with one of a
different type while keeping the history valid, because:
* Annotated tags contain the type of the tagged object.
If you replace the tagged object and the tag at the same time,
wouldn't that make the resulting history valid again?
Granted, there may not be a strong reason to reuse the object name
of the tagged object in such a case, but this there may not be is
merely I do not think of offhand, so I am not sure what workflow
of other people we are breaking with this change. A light-weight
tag may already point at the tagged object (in other words, the
object name of the tagged object is known to the outside world) and
that could be a reason why you would need to reuse the object name
of that object while changing its type.
I dunno.
Hrm, you're right, that's a flaw in my logic. You could do the same in
all other cases too, e.g. replace a tree so that an entry is of a
different type and at the same time change the type of the object
itself. You however have to carefully go through all objects that refer
to the one that was replaced, and fix the type in all of them.
It still seems an extremely unsafe thing to do with trees: especially
for small trees there is a small probability that you will generate the
same tree again in the future (by having the same blobs in the directory
again) and record it as a subtree or the 'tree' field of a commit. The
history would then again be invalid.
Should we add a --force flag of some sort to allow the user to do this,
while keeping the normal safety checks?
--
Thomas Rast
trast@{inf,student}.ethz.ch
--
To unsubscribe from this list: send the line unsubscribe 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: git subtree split - revert commit followed by a merge
Hi, I think I understand what's going on. If there are no net changes in one of the two branches, the id of the merge commit will be the same as that of the last commit on the other branch (pass the -d option to 'git subtree split' to see the new commit id's of the split out commits). In that case, git-subtree will drop the merge commit. I believe this is done because for regular commits, this means that the commit does not make changes to files in the subtree directory. I'm not sure I'm right about this, as I would expect the merge commit's hash to depend on the hashes of its parent commits. I was thinking the solution would be to simply adjust the copy_or_skip function to always copy merge commits. However, this may result in the produced commit history to differ from before (unnecessary merge commits were previously left out), leading to other problems. A solution might be to check for the presence of such commit + revert-commit branches. This can be done by checking for each parent of a merge commit if it is the ancestor of any of the other parents (using git merge-base --is-ancestor). Best regards, Brecht On Tue, 27 Aug 2013 15:09:30 +0200, Machiels, Brecht brecht.machi...@kla-tencor.com wrote: Hello: I'm running into the problem described in this mailing list post: http://thread.gmane.org/gmane.comp.version-control.git/202645 'git subtree split' fails to reconstruct the history when a revert commit is followed by a merge commit. I have slightly adjusted the test script provided by Fabien in his mailing list post: git init # create a directory that is going to be split mkdir doc echo TEST doc/README git add doc # commit A git commit -a -mfirst version # create a branch with a new commit (Z) git checkout -b test echo TEST doc/README1 git add doc/README1 git commit -a -madded README1 git checkout master # modify the README file (commit B) echo TEST_ doc/README git commit -a -msecond version # revert the change (commit C) echo TEST doc/README git commit -a -mrevert second version # or use git revert HEAD^ # split git subtree split --prefix=doc --branch=TARGET # add another commit (to a file *not* in the subtree dir) echo BLA BLA git add BLA git commit -a -mthird version # adding another commit to a file in the subtree dir will fix things #echo MEH doc/MEH #git add doc #git commit -a -mfourth version # the log will show the 3 commits as expected (including B and C) GIT_PAGER= git log --oneline TARGET # merge the test branch git merge -mmerged test test # attempt to re-split; this will fail git subtree split --prefix=doc --branch=TARGET # see what history split generates git subtree split --prefix=doc --branch=TARGET2 I have discovered that if the revert commit is followed by another commit that makes changes in the subtree directory, the split will work as expected (see fourth version above). See also this related SO question where I ask for a workaround: http://stackoverflow.com/questions/18465867 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC/PATCH] status: introduce status.displayCommentChar to disable display of #
Historically, git status needed to prefix each output line with '#' so
that the output could be added as comment to the commit message. This
prefix comment has no real purpose when git status is ran from the
command-line, and this may distract users from the real content.
Allow the user to disable this prefix comment. In the long run, if users
like the non-prefix output, it may make sense to flip the default value
to true.
Obviously, status.displayCommentChar applies to git status but is
ignored by git commit, so the status is still commented in
COMMIT_EDITMSG.
Signed-off-by: Matthieu Moy matthieu@imag.fr
---
As a beginner (long ago), I found this comment-prefixed output really
weird. I got used to it, so it stopped bugging me and I didn't do
anything to fix it.
Documentation/config.txt | 5 +
builtin/commit.c | 5 +
t/t7508-status.sh| 20
wt-status.c | 26 +++---
wt-status.h | 1 +
5 files changed, 50 insertions(+), 7 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index ec57a15..dacf4b9 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2118,6 +2118,11 @@ status.branch::
Set to true to enable --branch by default in linkgit:git-status[1].
The option --no-branch takes precedence over this variable.
+status.displayCommentChar::
+ If set to false, linkgit:git-status[1] will not prefix each
+ line with the comment char (`core.commentchar`, i.e. `#` by
+ default). Defaults to true.
+
status.showUntrackedFiles::
By default, linkgit:git-status[1] and linkgit:git-commit[1] show
files which are not currently tracked by Git. Directories which
diff --git a/builtin/commit.c b/builtin/commit.c
index 10acc53..6d6b9d2 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1182,6 +1182,10 @@ static int git_status_config(const char *k, const char
*v, void *cb)
s-use_color = git_config_colorbool(k, v);
return 0;
}
+ if (!strcmp(k, status.displaycommentchar)) {
+ s-display_comment_char = git_config_bool(k, v);
+ return 0;
+ }
if (!prefixcmp(k, status.color.) || !prefixcmp(k, color.status.)) {
int slot = parse_status_slot(k, 13);
if (slot 0)
@@ -1496,6 +1500,7 @@ int cmd_commit(int argc, const char **argv, const char
*prefix)
gitmodules_config();
git_config(git_commit_config, s);
status_format = STATUS_FORMAT_NONE; /* Ignore status.short */
+ s.display_comment_char = 1; /* Ignore status.displayCommentChar */
determine_whence(s);
s.colopts = 0;
diff --git a/t/t7508-status.sh b/t/t7508-status.sh
index ac3d0fe..7736426 100755
--- a/t/t7508-status.sh
+++ b/t/t7508-status.sh
@@ -113,6 +113,26 @@ test_expect_success 'status (2)' '
test_i18ncmp expect output
'
+test_expect_success 'status with status.displayCommentChar=false' '
+ $PERL_PATH -pi -e s/^\# //; s/^\#$//; s/^#\t/\t/ expect
+ git -c status.displayCommentChar=false status output
+ test_i18ncmp expect output
+'
+
+test_expect_success 'commit ignores status.displayCommentChar=false in
COMMIT_EDITMSG' '
+ cat .git/editor -\EOF
+ #! /bin/sh
+ cp $1 output
+EOF
+ chmod 755 .git/editor
+ (
+ EDITOR=.git/editor
+ export EDITOR
+ test_must_fail git commit
+ )
+ ! grep ^[^#] output
+'
+
cat expect \EOF
# On branch master
# Changes to be committed:
diff --git a/wt-status.c b/wt-status.c
index cb24f1f..765599d 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -45,9 +45,11 @@ static void status_vprintf(struct wt_status *s, int at_bol,
const char *color,
strbuf_vaddf(sb, fmt, ap);
if (!sb.len) {
- strbuf_addch(sb, comment_line_char);
- if (!trail)
- strbuf_addch(sb, ' ');
+ if (s-display_comment_char) {
+ strbuf_addch(sb, comment_line_char);
+ if (!trail)
+ strbuf_addch(sb, ' ');
+ }
color_print_strbuf(s-fp, color, sb);
if (trail)
fprintf(s-fp, %s, trail);
@@ -58,7 +60,7 @@ static void status_vprintf(struct wt_status *s, int at_bol,
const char *color,
eol = strchr(line, '\n');
strbuf_reset(linebuf);
- if (at_bol) {
+ if (at_bol s-display_comment_char) {
strbuf_addch(linebuf, comment_line_char);
if (*line != '\n' *line != '\t')
strbuf_addch(linebuf, ' ');
@@ -128,6 +130,7 @@ void wt_status_prepare(struct wt_status *s)
s-untracked.strdup_strings = 1;
s-ignored.strdup_strings = 1;
s-show_branch = -1; /*
Re: Eclipse
Hi Ron, On 28/08/13 03:10, Ron Tregaskis - NOAA Federal wrote: Does git work with Eclipse? Ron -- Eclipse does have git integration courtesy of the EGit plugin. At $dayjob most of us are running eclipse 3.7 a.k.a Indigo. When we started with an earlier eclipse version we had to install the git plugin manually but these days it's bundled with the CDT release [1] (and I assume most other flavours of eclipse[2]). EGit is built on JGit which sometimes lags behind the core git in terms of features. But for the majority of users you won't notice. I still use core git for most of my day-to-day work but plenty of others do everything from within eclipse. I've also found that Eclipse's affinity for svn-like vcses doesn't always line up with the git/dvcs way of doing things but that seems to get better with each release. Hope that helps, - C -- [1] - http://www.eclipse.org/downloads/packages/eclipse-ide-cc-developers/keplerr [2] - http://www.eclipse.org/downloads/packages/eclipse-ide-java-developers/keplerr -- To unsubscribe from this list: send the line unsubscribe 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] status: introduce status.displayCommentChar to disable display of #
Am 8/28/2013 10:32, schrieb Matthieu Moy: Historically, git status needed to prefix each output line with '#' so that the output could be added as comment to the commit message. This prefix comment has no real purpose when git status is ran from the command-line, and this may distract users from the real content. Allow the user to disable this prefix comment. In the long run, if users like the non-prefix output, it may make sense to flip the default value to true. Obviously, status.displayCommentChar applies to git status but is ignored by git commit, so the status is still commented in COMMIT_EDITMSG. Signed-off-by: Matthieu Moy matthieu@imag.fr --- As a beginner (long ago), I found this comment-prefixed output really weird. I got used to it,... You have my sympathy. How does your solution work when dirty submodules are involved and submodule status is included? +test_expect_success 'status with status.displayCommentChar=false' ' + $PERL_PATH -pi -e s/^\# //; s/^\#$//; s/^#\t/\t/ expect Perl's -i does not work on Windows when no backup file extension is given. Therefore, please use a temporary file or ... -pi.bak ... + git -c status.displayCommentChar=false status output + test_i18ncmp expect output +' -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
$10.5m
ttn/Dear Contact Western union offfice with your name,Adres,City,Country,phone no and ur ID,for your $10.5m comenstion,Mr. Frank Edward Email: (uw...@yahoo.fr)+229 68035435 but note that you will be receiving $20,000 usd everyday until your $10.5M completed transferred to you Regards Dr JOHNSON WALKER +229 68035435 -- To unsubscribe from this list: send the line unsubscribe 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] status: introduce status.displayCommentChar to disable display of #
Johannes Sixt j.s...@viscovery.net writes: How does your solution work when dirty submodules are involved and submodule status is included?f Badly ;-). I didn't notice the subcommand call for submodules summary. It's a bit more tricky to get right, as the git sumbodule summary --for-status call did not know whether it was called from commit or from status. I fixed this by adding a --[no-]-display-comment-char to git submodule summary. +test_expect_success 'status with status.displayCommentChar=false' ' +$PERL_PATH -pi -e s/^\# //; s/^\#$//; s/^#\t/\t/ expect Perl's -i does not work on Windows when no backup file extension is given. Therefore, please use a temporary file or ... -pi.bak ... OK I didn't know that. I went the old good sed+mv way. New series comming. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC/PATCH v2 1/3] builtin/stripspace.c: fix broken indentation
Signed-off-by: Matthieu Moy matthieu@imag.fr
---
I normally don't like whitespace-only patches, but the indentation was
seriously broken here.
Can safely be dropped.
builtin/stripspace.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/builtin/stripspace.c b/builtin/stripspace.c
index e981dfb..1259ed7 100644
--- a/builtin/stripspace.c
+++ b/builtin/stripspace.c
@@ -89,11 +89,11 @@ int cmd_stripspace(int argc, const char **argv, const char
*prefix)
if (argc == 2) {
if (!strcmp(argv[1], -s) ||
- !strcmp(argv[1], --strip-comments)) {
-strip_comments = 1;
+ !strcmp(argv[1], --strip-comments)) {
+ strip_comments = 1;
} else if (!strcmp(argv[1], -c) ||
-!strcmp(argv[1], --comment-lines)) {
-mode = COMMENT_LINES;
+ !strcmp(argv[1], --comment-lines)) {
+ mode = COMMENT_LINES;
} else {
mode = INVAL;
}
--
1.8.4.11.g9db5bc7.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC/PATCH v2 2/3] submodule: introduce --[no-]display-comment-char
Signed-off-by: Matthieu Moy matthieu@imag.fr
---
(--for-status was undocumented, so I didn't bother documenting this either)
git-submodule.sh | 20
1 file changed, 16 insertions(+), 4 deletions(-)
diff --git a/git-submodule.sh b/git-submodule.sh
index 2979197..ac0fdad 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -966,6 +966,7 @@ set_name_rev () {
cmd_summary() {
summary_limit=-1
for_status=
+ display_comment_char=t
diff_cmd=diff-index
# parse $args after submodule ... summary.
@@ -981,6 +982,12 @@ cmd_summary() {
--for-status)
for_status=$1
;;
+ --display-comment-char)
+ display_comment_char=t
+ ;;
+ --no-display-comment-char)
+ display_comment_char=
+ ;;
-n|--summary-limit)
summary_limit=$2
isnumber $summary_limit || usage
@@ -1151,13 +1158,18 @@ cmd_summary() {
echo
done |
if test -n $for_status; then
+ if test -n $display_comment_char; then
+ filter_cmd='git stripspace -c'
+ else
+ filter_cmd=cat
+ fi
if [ -n $files ]; then
- gettextln Submodules changed but not updated: | git
stripspace -c
+ gettextln Submodules changed but not updated: |
$filter_cmd
else
- gettextln Submodule changes to be committed: | git
stripspace -c
+ gettextln Submodule changes to be committed: |
$filter_cmd
fi
- printf \n | git stripspace -c
- git stripspace -c
+ printf \n | $filter_cmd
+ $filter_cmd
else
cat
fi
--
1.8.4.11.g9db5bc7.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC/PATCH v2 3/3] status: introduce status.displayCommentChar to disable display of #
Historically, git status needed to prefix each output line with '#' so
that the output could be added as comment to the commit message. This
prefix comment has no real purpose when git status is ran from the
command-line, and this may distract users from the real content.
Allow the user to disable this prefix comment. In the long run, if users
like the non-prefix output, it may make sense to flip the default value
to true.
Obviously, status.displayCommentChar applies to git status but is
ignored by git commit, so the status is still commented in
COMMIT_EDITMSG.
Signed-off-by: Matthieu Moy matthieu@imag.fr
---
Change since v1:
* removed perl -pi use.
* test and fix submodule summary.
Documentation/config.txt | 5 +
builtin/commit.c | 9 +
t/t7508-status.sh| 43 +++
wt-status.c | 37 +
wt-status.h | 1 +
5 files changed, 83 insertions(+), 12 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index ec57a15..dacf4b9 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2118,6 +2118,11 @@ status.branch::
Set to true to enable --branch by default in linkgit:git-status[1].
The option --no-branch takes precedence over this variable.
+status.displayCommentChar::
+ If set to false, linkgit:git-status[1] will not prefix each
+ line with the comment char (`core.commentchar`, i.e. `#` by
+ default). Defaults to true.
+
status.showUntrackedFiles::
By default, linkgit:git-status[1] and linkgit:git-commit[1] show
files which are not currently tracked by Git. Directories which
diff --git a/builtin/commit.c b/builtin/commit.c
index 10acc53..d4cfced 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1182,6 +1182,10 @@ static int git_status_config(const char *k, const char
*v, void *cb)
s-use_color = git_config_colorbool(k, v);
return 0;
}
+ if (!strcmp(k, status.displaycommentchar)) {
+ s-display_comment_char = git_config_bool(k, v);
+ return 0;
+ }
if (!prefixcmp(k, status.color.) || !prefixcmp(k, color.status.)) {
int slot = parse_status_slot(k, 13);
if (slot 0)
@@ -1496,6 +1500,11 @@ int cmd_commit(int argc, const char **argv, const char
*prefix)
gitmodules_config();
git_config(git_commit_config, s);
status_format = STATUS_FORMAT_NONE; /* Ignore status.short */
+ /*
+* Ignore status.displayCommentChar: we do need comments in
+* COMMIT_EDITMSG.
+*/
+ s.display_comment_char = 1;
determine_whence(s);
s.colopts = 0;
diff --git a/t/t7508-status.sh b/t/t7508-status.sh
index ac3d0fe..d0d7c4a 100755
--- a/t/t7508-status.sh
+++ b/t/t7508-status.sh
@@ -113,6 +113,39 @@ test_expect_success 'status (2)' '
test_i18ncmp expect output
'
+strip_comments () {
+ sed s/^\# //; s/^\#$//; s/^#\t/\t/ $1 $1.tmp
+ rm $1 mv $1.tmp $1
+}
+
+test_expect_success 'status with status.displayCommentChar=false' '
+ strip_comments expect
+ git -c status.displayCommentChar=false status output
+ test_i18ncmp expect output
+'
+
+test_expect_success 'setup fake editor' '
+ cat .git/editor -\EOF
+ #! /bin/sh
+ cp $1 output
+EOF
+ chmod 755 .git/editor
+'
+
+commit_template_commented () {
+ (
+ EDITOR=.git/editor
+ export EDITOR
+ # Fails due to empty message
+ test_must_fail git commit
+ )
+ ! grep '^[^#]' output
+}
+
+test_expect_success 'commit ignores status.displayCommentChar=false in
COMMIT_EDITMSG' '
+ commit_template_commented
+'
+
cat expect \EOF
# On branch master
# Changes to be committed:
@@ -872,6 +905,16 @@ test_expect_success 'status submodule summary' '
test_i18ncmp expect output
'
+test_expect_success 'status submodule summary with
status.displayCommentChar=false' '
+ strip_comments expect
+ git -c status.displayCommentChar=false status output
+ test_i18ncmp expect output
+'
+
+test_expect_success 'commit with submodule summary ignores displayCommentChar'
'
+ commit_template_commented
+'
+
cat expect EOF
M dir1/modified
A dir2/added
diff --git a/wt-status.c b/wt-status.c
index cb24f1f..97068d5 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -45,9 +45,11 @@ static void status_vprintf(struct wt_status *s, int at_bol,
const char *color,
strbuf_vaddf(sb, fmt, ap);
if (!sb.len) {
- strbuf_addch(sb, comment_line_char);
- if (!trail)
- strbuf_addch(sb, ' ');
+ if (s-display_comment_char) {
+ strbuf_addch(sb, comment_line_char);
+ if (!trail)
+
Re: [PATCH 4/6] upload-pack: delegate rev walking in shallow fetch to pack-objects
Hi Nguy, On Fri, Aug 16, 2013 at 04:52:05PM +0700, Nguyễn Thái Ngọc Duy wrote: upload-pack has a special rev walking code for shallow recipients. It works almost like the similar code in pack-objects except: 1. in upload-pack, graft points could be added for deepening 2. also when the repository is deepened, the shallow point will be moved further away from the tip, but the old shallow point will be marked as edge to produce more efficient packs. See 6523078 (make shallow repository deepening more network efficient - 2009-09-03) pass the file to pack-objects via --shallow-file. This will override $GIT_DIR/shallow and give pack-objects the exact repository shape that upload-pack has. mark edge commits by revision command arguments. Even if old shallow points are passed as --not revisions as in this patch, they will not be picked up by mark_edges_uninteresting() because this function looks up to parents for edges, while in this case the edge is the children, in the opposite direction. This will be fixed in the next patch when all given uninteresting commits are marked as edges. This says the next patch but it really refers to 6/6, not 5/6. Patch 6/6 has the same problem (it says previous patch). Perhaps patches 4 and 5 should just be swapped? Gr. Matthijs -- To unsubscribe from this list: send the line unsubscribe 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] During a shallow fetch, prevent sending over unneeded objects
Hi Duy, I thought a bit but my thoughts often get stuck if I don't write them down in form of code :-) so this is what I got so far. 4/6 is a good thing in my opinion, but I might overlook something 6/6 is about this thread. The series looks good to me, though I don't know enough about the code to do detailed analysis. In any case, I agree that 4/6 is a good change, it removes a bunch of similar code for the shallow special case (which is now no longer a completely separate special case). The total series also seems to actually fix the problem I reported. I'll resend the testcase from my original patch as well, which now passes with your series applied. Thanks for diving into this! Gr. Matthijs -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git-p4 out of memory for very large repository
On Mon, Aug 26, 2013 at 09:47:56AM -0400, Corey Thompson wrote: You are correct that git-fast-import is killed by the OOM killer, but I was unclear about which process was malloc()ing so much memory that the OOM killer got invoked (as other completely unrelated processes usually also get killed when this happens). Unless there's one gigantic file in one change that gets removed by another change, I don't think that's the problem; as I mentioned in another email, the machine has 32GB physical memory and the largest single file in the current head is only 118MB. Even if there is a very large transient file somewhere in the history, I seriously doubt it's tens of gigabytes in size. I have tried watching it with top before, but it takes several hours before it dies. I haven't been able to see any explosion of memory usage, even within the final hour, but I've never caught it just before it dies, either. I suspect that whatever the issue is here, it happens very quickly. If I'm unable to get through this today using the incremental p4 sync method you described, I'll try running a full-blown clone overnight with top in batch mode writing to a log file to see whether it catches anything. Thanks again, Corey Unforunately I have not made much progress. The incremental sync method fails with the output pasted below. The change I specified is only one change number above where that repo was cloned... So I tried a 'git p4 rebase' overnight with top running, and as I feared I did not see anything out of the ordinary. git, git-fast-import, and git-p4 all hovered under 1.5% MEM the entire time, right up until death. The last entry in my log shows git-fast-import at 0.8%, with git and git-p4 at 0.0% and 0.1%, respectively. I could try again with a more granular period, but I feel like this method is ultimately a goose chase. Corey $ git p4 sync //path/to/some/branch@505859 Doing initial import of //path/to/some/branch/ from revision @505859 into refs/remotes/p4/master fast-import failed: warning: Not updating refs/remotes/p4/master (new tip 29ef6ff25f1448fa2f907d22fd704594dc8769bd does not contain d477672be5ac6a00cc9175ba2713d5395660e840) git-fast-import statistics: - Alloc'd objects: 165000 Total objects: 69 (232434 duplicates ) blobs : 45 (209904 duplicates 40 deltas of 42 attempts) trees : 23 ( 22530 duplicates 0 deltas of 23 attempts) commits:1 ( 0 duplicates 0 deltas of 0 attempts) tags :0 ( 0 duplicates 0 deltas of 0 attempts) Total branches: 1 ( 1 loads ) marks: 1024 ( 0 unique) atoms: 105170 Memory total: 24421 KiB pools: 17976 KiB objects: 6445 KiB - pack_report: getpagesize()= 4096 pack_report: core.packedGitWindowSize = 33554432 pack_report: core.packedGitLimit = 268435456 pack_report: pack_used_ctr= 4371 pack_report: pack_mmap_calls =124 pack_report: pack_open_windows= 8 / 9 pack_report: pack_mapped = 268435456 / 268435456 - -- To unsubscribe from this list: send the line 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] Add testcase for needless objects during a shallow fetch
This is a testcase that checks for a problem where, during a specific shallow fetch where the client does not have any commits that are a successor of the new shallow root (i.e., the fetch creates a new detached piece of history), the server would simply send over _all_ objects, instead of taking into account the objects already present in the client. The actual problem was fixed by a recent patch series by Nguyễn Thái Ngọc Duy already. Signed-off-by: Matthijs Kooijman matth...@stdin.nl --- t/t5500-fetch-pack.sh | 11 +++ 1 file changed, 11 insertions(+) diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh index fd2598e..a022d65 100755 --- a/t/t5500-fetch-pack.sh +++ b/t/t5500-fetch-pack.sh @@ -393,6 +393,17 @@ test_expect_success 'fetch in shallow repo unreachable shallow objects' ' git fsck --no-dangling ) ' +test_expect_success 'fetch creating new shallow root' ' + ( + git clone file://$(pwd)/. shallow10 + git commit --allow-empty -m empty + cd shallow10 + git fetch --depth=1 --progress 2 actual + # This should fetch only the empty commit, no tree or + # blob objects + grep remote: Total 1 actual + ) +' test_expect_success 'setup tests for the --stdin parameter' ' for head in C D E F -- 1.8.4.rc1 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: the pager
From: Junio C Hamano gits...@pobox.com I've noticed that Git by default puts long output through less as a pager. I don't like that, but this is not the time to change established behavior. But while tracking that down, I noticed that the paging behavior is controlled by at least 5 things: the -p/--paginate/--no-pager options the GIT_PAGER environment variable the PAGER environment variable the core.pager Git configuration variable the build-in default (which seems to usually be less) ... What is the (intended) order of precedence of specifiers of paging behavior? My guess is that it should be the order I've given above. I think that sounds about right (I didn't check the code, though). The most specific to the command line invocation (i.e. option) trumps the environment, which trumps the configured default, and the hard wired stuff is used as the fallback default. I am not sure about PAGER environment and core.pager, though. People want Git specific pager that applies only to Git process specified to core.pager, and still want to use their own generic PAGER to other programs, so my gut feeling is that it would make sense to consider core.pager a way to specify GIT_PAGER without contaminating the environment, and use both to override the generic PAGER (in other words, core.pager should take precedence over PAGER as far as Git is concerned). I've just discovered this bit of documentation. Within the git-var manual page is this entry: GIT_PAGER Text viewer for use by git commands (e.g., less). The value is meant to be interpreted by the shell. The order of preference is the $GIT_PAGER environment variable, then core.pager configuration, then $PAGER, and then finally less. This suggests that the ordering is GIT_PAGER core.pager PAGER default. Dale -- To unsubscribe from this list: send the line 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] builtin/fetch.c: Fix a sparse warning
Sparse issues an 'prepare_transport' was not declared. Should it
be static? warning. In order to suppress the warning, since this
symbol only requires file scope, we simply add the static modifier
to it's declaration.
Signed-off-by: Ramsay Jones ram...@ramsay1.demon.co.uk
---
Hi Junio,
When you re-build the next branch, could you please squash this into
commit db5723c6 (fetch: refactor code that prepares a transport,
07-08-2013). [from the 'jc/transport-do-not-use-connect-twice-in-fetch'
branch]
Thanks!
ATB,
Ramsay Jones
builtin/fetch.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index e880116..9e654ef 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -747,7 +747,7 @@ static void set_option(struct transport *transport, const
char *name, const char
name, transport-url);
}
-struct transport *prepare_transport(struct remote *remote)
+static struct transport *prepare_transport(struct remote *remote)
{
struct transport *transport;
transport = transport_get(remote, NULL);
--
1.8.4
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/8] remote-{hg,bzr}: updates
Felipe Contreras (8): remote-bzr: fix export of utf-8 authors remote-bzr: make bzr branches configurable per-repo remote-hg: fix test remote-hg: add missing s in the test remote-hg: improve basic test remote-helpers: trivial style fixes remote-helpers: cleanup more global variables remote-hg: support for notes contrib/remote-helpers/git-remote-bzr | 47 +++-- contrib/remote-helpers/git-remote-hg | 65 +++ contrib/remote-helpers/test-bzr.sh| 30 contrib/remote-helpers/test-hg.sh | 10 +++--- 4 files changed, 82 insertions(+), 70 deletions(-) -- 1.8.4-fc -- To unsubscribe from this list: send the line 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/8] remote-bzr: make bzr branches configurable per-repo
Different repositories have different branches, some are are even
branches themselves.
Reported-by: Peter Niederlag netserv...@niekom.de
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
contrib/remote-helpers/git-remote-bzr | 13 ++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/contrib/remote-helpers/git-remote-bzr
b/contrib/remote-helpers/git-remote-bzr
index 08b0b61..a7d2ac9 100755
--- a/contrib/remote-helpers/git-remote-bzr
+++ b/contrib/remote-helpers/git-remote-bzr
@@ -13,8 +13,11 @@
# or
# % git clone bzr::lp:myrepo
#
-# If you want to specify which branches you want track (per repo):
-# git config remote-bzr.branches 'trunk, devel, test'
+# If you want to specify which branches you want to track (per repo):
+# % git config remote.origin.bzr-branches 'trunk, devel, test'
+#
+# Where 'origin' is the name of the repository you want to specify the
+# branches.
#
import sys
@@ -852,9 +855,13 @@ def get_repo(url, alias):
except bzrlib.errors.NoRepositoryPresent:
pass
-wanted = get_config('remote-bzr.branches').rstrip().split(', ')
+wanted = get_config('remote.%s.bzr-branches' % alias).rstrip().split(', ')
# stupid python
wanted = [e for e in wanted if e]
+if not wanted:
+wanted = get_config('remote-bzr.branches').rstrip().split(', ')
+# stupid python
+wanted = [e for e in wanted if e]
if not wanted:
try:
--
1.8.4-fc
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/8] remote-hg: fix test
It wasn't being checked properly before; those refs never existed. Signed-off-by: Felipe Contreras felipe.contre...@gmail.com --- contrib/remote-helpers/test-hg.sh | 2 -- 1 file changed, 2 deletions(-) diff --git a/contrib/remote-helpers/test-hg.sh b/contrib/remote-helpers/test-hg.sh index f7ce8aa..cbf8617 100755 --- a/contrib/remote-helpers/test-hg.sh +++ b/contrib/remote-helpers/test-hg.sh @@ -580,8 +580,6 @@ test_expect_success 'remote big push fetch first' ' check_push 1 --all -EOF master good_bmark - new_bmark:new - new_branch:new bad_bmark:fetch-first branches/bad_branch:festch-first EOF -- 1.8.4-fc -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/8] remote-hg: add missing s in the test
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com --- contrib/remote-helpers/test-hg.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/contrib/remote-helpers/test-hg.sh b/contrib/remote-helpers/test-hg.sh index cbf8617..94b0bba 100755 --- a/contrib/remote-helpers/test-hg.sh +++ b/contrib/remote-helpers/test-hg.sh @@ -577,7 +577,7 @@ test_expect_success 'remote big push fetch first' ' echo five content git commit -q -a -m five - check_push 1 --all -EOF + check_push 1 --all -EOF master good_bmark bad_bmark:fetch-first @@ -633,7 +633,7 @@ test_expect_failure 'remote big push dry-run' ' ( cd gitrepo - check_push 0 --dry-run --all -EOF + check_push 1 --dry-run --all -EOF master good_bmark branches/good_branch -- 1.8.4-fc -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 6/8] remote-helpers: trivial style fixes
In accordance with pep8. Signed-off-by: Felipe Contreras felipe.contre...@gmail.com --- contrib/remote-helpers/git-remote-bzr | 4 ++-- contrib/remote-helpers/git-remote-hg | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/contrib/remote-helpers/git-remote-bzr b/contrib/remote-helpers/git-remote-bzr index a7d2ac9..e4fdfb0 100755 --- a/contrib/remote-helpers/git-remote-bzr +++ b/contrib/remote-helpers/git-remote-bzr @@ -488,7 +488,7 @@ class CustomTree(): add_entry(fid, dirname, 'directory') return fid -def add_entry(fid, path, kind, mode = None): +def add_entry(fid, path, kind, mode=None): dirname, basename = os.path.split(path) parent_fid = get_parent(dirname, basename) @@ -509,7 +509,7 @@ class CustomTree(): self.files[path] = [change[0], None] changes.append(change) -def update_entry(fid, path, kind, mode = None): +def update_entry(fid, path, kind, mode=None): dirname, basename = os.path.split(path) parent_fid = get_parent(dirname, basename) diff --git a/contrib/remote-helpers/git-remote-hg b/contrib/remote-helpers/git-remote-hg index d2ff0e2..178a5a5 100755 --- a/contrib/remote-helpers/git-remote-hg +++ b/contrib/remote-helpers/git-remote-hg @@ -1079,7 +1079,7 @@ def do_export(parser): author, msg = parsed_tags.get(tag, (None, None)) if mode == 'git': if not msg: -msg = 'Added tag %s for changeset %s' % (tag, node[:12]); +msg = 'Added tag %s for changeset %s' % (tag, node[:12]) tagnode, branch = write_tag(parser.repo, tag, node, msg, author) p_revs[tagnode] = 'refs/heads/branches/' + gitref(branch) else: -- 1.8.4-fc -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 5/8] remote-hg: improve basic test
It appears 'let' is not present in all shells.
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
contrib/remote-helpers/test-hg.sh | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/contrib/remote-helpers/test-hg.sh
b/contrib/remote-helpers/test-hg.sh
index 94b0bba..5a6f745 100755
--- a/contrib/remote-helpers/test-hg.sh
+++ b/contrib/remote-helpers/test-hg.sh
@@ -75,10 +75,10 @@ check_push () {
grep ^ [a-f0-9]*\.\.[a-f0-9]* *${branch} -
${branch}$ error || ref_ret=1
;;
esac
- let 'ref_ret' echo match for '$branch' failed break
+ test $ref_ret -ne 0 echo match for '$branch' failed
break
done
- if let 'expected_ret != ret || ref_ret'
+ if test $expected_ret -ne $ret -o $ref_ret -ne 0
then
return 1
fi
--
1.8.4-fc
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 8/8] remote-hg: support for notes
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
contrib/remote-helpers/git-remote-hg | 33 -
1 file changed, 32 insertions(+), 1 deletion(-)
diff --git a/contrib/remote-helpers/git-remote-hg
b/contrib/remote-helpers/git-remote-hg
index 307d82c..0dbda75 100755
--- a/contrib/remote-helpers/git-remote-hg
+++ b/contrib/remote-helpers/git-remote-hg
@@ -23,6 +23,7 @@ import subprocess
import urllib
import atexit
import urlparse, hashlib
+import time as ptime
#
# If you are not in hg-git-compat mode and want to disable the tracking of
@@ -126,6 +127,7 @@ class Marks:
self.rev_marks = {}
self.last_mark = 0
self.version = 0
+self.last_note = 0
def load(self):
if not os.path.exists(self.path):
@@ -137,6 +139,7 @@ class Marks:
self.marks = tmp['marks']
self.last_mark = tmp['last-mark']
self.version = tmp.get('version', 1)
+self.last_note = tmp.get('last-note', 0)
for rev, mark in self.marks.iteritems():
self.rev_marks[mark] = rev
@@ -150,7 +153,7 @@ class Marks:
self.version = 2
def dict(self):
-return { 'tips': self.tips, 'marks': self.marks, 'last-mark' :
self.last_mark, 'version' : self.version }
+return { 'tips': self.tips, 'marks': self.marks, 'last-mark' :
self.last_mark, 'version' : self.version, 'last-note' : self.last_note }
def store(self):
json.dump(self.dict(), open(self.path, 'w'))
@@ -525,6 +528,31 @@ def export_ref(repo, name, kind, head):
print from :%u % rev_to_mark(head)
print
+pending_revs = set(revs) - notes
+if pending_revs:
+note_mark = marks.next_mark()
+ref = refs/notes/hg
+
+print commit %s % ref
+print mark :%d % (note_mark)
+print committer remote-hg %s % (ptime.strftime('%s %z'))
+desc = Notes for %s\n % (name)
+print data %d % (len(desc))
+print desc
+if marks.last_note:
+print from :%u % marks.last_note
+
+for rev in pending_revs:
+notes.add(rev)
+c = repo[rev]
+print N inline :%u % rev_to_mark(c)
+msg = c.hex()
+print data %d % (len(msg))
+print msg
+print
+
+marks.last_note = note_mark
+
marks.set_tip(ename, head.hex())
def export_tag(repo, tag):
@@ -629,6 +657,7 @@ def do_import(parser):
print feature import-marks=%s % path
print feature export-marks=%s % path
print feature force
+print feature notes
sys.stdout.flush()
tmp = encoding.encoding
@@ -1126,6 +1155,7 @@ def main(args):
global filenodes
global fake_bmark, hg_version
global dry_run
+global notes, alias
alias = args[1]
url = args[2]
@@ -1165,6 +1195,7 @@ def main(args):
except:
hg_version = None
dry_run = False
+notes = set()
repo = get_repo(url, alias)
prefix = 'refs/hg/%s' % alias
--
1.8.4-fc
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 7/8] remote-helpers: cleanup more global variables
They don't need to be specified if they are not going to be set.
Suggested-by: Dusty Phillips du...@linux.ca
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
contrib/remote-helpers/git-remote-bzr | 29 -
contrib/remote-helpers/git-remote-hg | 30 ++
2 files changed, 2 insertions(+), 57 deletions(-)
diff --git a/contrib/remote-helpers/git-remote-bzr
b/contrib/remote-helpers/git-remote-bzr
index e4fdfb0..42cec58 100755
--- a/contrib/remote-helpers/git-remote-bzr
+++ b/contrib/remote-helpers/git-remote-bzr
@@ -178,11 +178,9 @@ class Parser:
return (committer, int(date), tz)
def rev_to_mark(rev):
-global marks
return marks.from_rev(rev)
def mark_to_rev(mark):
-global marks
return marks.to_rev(mark)
def fixup_user(user):
@@ -237,8 +235,6 @@ def get_filechanges(cur, prev):
return modified, removed
def export_files(tree, files):
-global marks, filenodes
-
final = []
for path, fid in files.iteritems():
kind = tree.kind(fid)
@@ -280,8 +276,6 @@ def export_files(tree, files):
return final
def export_branch(repo, name):
-global prefix
-
ref = '%s/heads/%s' % (prefix, name)
tip = marks.get_tip(name)
@@ -382,16 +376,12 @@ def export_branch(repo, name):
marks.set_tip(name, revid)
def export_tag(repo, name):
-global tags, prefix
-
ref = '%s/tags/%s' % (prefix, name)
print reset %s % ref
print from :%u % rev_to_mark(tags[name])
print
def do_import(parser):
-global dirname
-
repo = parser.repo
path = os.path.join(dirname, 'marks-git')
@@ -417,8 +407,6 @@ def do_import(parser):
sys.stdout.flush()
def parse_blob(parser):
-global blob_marks
-
parser.next()
mark = parser.get_mark()
parser.next()
@@ -429,8 +417,6 @@ def parse_blob(parser):
class CustomTree():
def __init__(self, branch, revid, parents, files):
-global files_cache
-
self.updates = {}
self.branch = branch
@@ -587,9 +573,6 @@ def c_style_unescape(string):
return string
def parse_commit(parser):
-global marks, blob_marks, parsed_refs
-global mode
-
parents = []
ref = parser[1]
@@ -661,8 +644,6 @@ def parse_commit(parser):
marks.new_mark(revid, commit_mark)
def parse_reset(parser):
-global parsed_refs
-
ref = parser[1]
parser.next()
@@ -678,8 +659,6 @@ def parse_reset(parser):
parsed_refs[ref] = mark_to_rev(from_mark)
def do_export(parser):
-global parsed_refs, dirname
-
parser.next()
for line in parser.each_block('done'):
@@ -728,8 +707,6 @@ def do_export(parser):
print
def do_capabilities(parser):
-global dirname
-
print import
print export
print refspec refs/heads/*:%s/heads/* % prefix
@@ -747,8 +724,6 @@ def ref_is_valid(name):
return not True in [c in name for c in '~^: \\']
def do_list(parser):
-global tags
-
master_branch = None
for name in branches:
@@ -782,8 +757,6 @@ def clone(path, remote_branch):
return remote_branch.sprout(bdir, repository=repo)
def get_remote_branch(name):
-global dirname, branches
-
remote_branch = bzrlib.branch.Branch.open(branches[name])
if isinstance(remote_branch.user_transport,
bzrlib.transport.local.LocalTransport):
return remote_branch
@@ -825,8 +798,6 @@ def find_branches(repo):
yield name, branch.base
def get_repo(url, alias):
-global dirname, peer, branches
-
normal_url = bzrlib.urlutils.normalize_url(url)
origin = bzrlib.bzrdir.BzrDir.open(url)
is_local = isinstance(origin.transport,
bzrlib.transport.local.LocalTransport)
diff --git a/contrib/remote-helpers/git-remote-hg
b/contrib/remote-helpers/git-remote-hg
index 178a5a5..307d82c 100755
--- a/contrib/remote-helpers/git-remote-hg
+++ b/contrib/remote-helpers/git-remote-hg
@@ -227,8 +227,6 @@ class Parser:
return sys.stdin.read(size)
def get_author(self):
-global bad_mail
-
ex = None
m = RAW_AUTHOR_RE.match(self.line)
if not m:
@@ -261,8 +259,6 @@ def fix_file_path(path):
return os.path.relpath(path, '/')
def export_files(files):
-global marks, filenodes
-
final = []
for f in files:
fid = node.hex(f.filenode())
@@ -344,8 +340,6 @@ def fixup_user_hg(user):
return (name, mail)
def fixup_user(user):
-global mode, bad_mail
-
if mode == 'git':
name, mail = fixup_user_git(user)
else:
@@ -374,7 +368,7 @@ def updatebookmarks(repo, peer):
bookmarks.write(repo)
def get_repo(url, alias):
-global dirname, peer
+global peer
myui = ui.ui()
myui.setconfig('ui', 'interactive', 'off')
@@ -429,16 +423,12 @@ def get_repo(url, alias):
return repo
def rev_to_mark(rev):
-global marks
return marks.from_rev(rev.hex())
def mark_to_rev(mark):
-global marks
[PATCH 1/8] remote-bzr: fix export of utf-8 authors
Reported-by: Joakim Verona joa...@verona.se
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
contrib/remote-helpers/git-remote-bzr | 1 +
contrib/remote-helpers/test-bzr.sh| 30 ++
2 files changed, 31 insertions(+)
diff --git a/contrib/remote-helpers/git-remote-bzr
b/contrib/remote-helpers/git-remote-bzr
index c3a3cac..08b0b61 100755
--- a/contrib/remote-helpers/git-remote-bzr
+++ b/contrib/remote-helpers/git-remote-bzr
@@ -168,6 +168,7 @@ class Parser:
if not m:
return None
_, name, email, date, tz = m.groups()
+name = name.decode('utf-8')
committer = '%s %s' % (name, email)
tz = int(tz)
tz = ((tz / 100) * 3600) + ((tz % 100) * 60)
diff --git a/contrib/remote-helpers/test-bzr.sh
b/contrib/remote-helpers/test-bzr.sh
index dce281f..ef8992a 100755
--- a/contrib/remote-helpers/test-bzr.sh
+++ b/contrib/remote-helpers/test-bzr.sh
@@ -358,4 +358,34 @@ test_expect_success 'strip' '
test_cmp expected actual
'
+test_expect_success 'export utf-8 authors' '
+ test_when_finished rm -rf bzrrepo gitrepo LC_ALL=C unset
GIT_COMMITTER_NAME
+
+ LC_ALL=en_US.UTF-8
+ export LC_ALL
+
+ GIT_COMMITTER_NAME=Grégoire
+ export GIT_COMMITTER_NAME
+
+ bzr init bzrrepo
+
+ (
+ git init gitrepo
+ cd gitrepo
+ echo greg content
+ git add content
+ git commit -m one
+ git remote add bzr bzr::../bzrrepo
+ git push bzr
+ )
+
+ (
+ cd bzrrepo
+ bzr log | grep ^committer: ../actual
+ )
+
+ echo committer: Grégoire commit...@example.com expected
+ test_cmp expected actual
+'
+
test_done
--
1.8.4-fc
--
To unsubscribe from this list: send the line unsubscribe 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/5] replace: forbid replacing an object with one of a different type
Thomas Rast tr...@inf.ethz.ch writes: Hrm, you're right, that's a flaw in my logic. You could do the same in all other cases too, e.g. replace a tree so that an entry is of a different type and at the same time change the type of the object itself. You however have to carefully go through all objects that refer to the one that was replaced, and fix the type in all of them. It still seems an extremely unsafe thing to do with trees... ... Should we add a --force flag of some sort to allow the user to do this, while keeping the normal safety checks? As long as we do not forbid such an unusual replacement on the reading side, we won't break people who are more inventive than we are (I am not convinced that we know people's workflow well enough to definitively say that no sane workflow, which benefits from being able to replace an object with another from a different type, exists). Preventing git replace wrapper from creating such a replacement by default will make it harder to do and may reduce mistakes, without breaking them too much, I think. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2] mailmap: handle mailmap blobs without trailing newlines
Jeff King p...@peff.net writes: This makes the code simpler and shorter (because we don't have to correlate strchrnul with the length calculation), correct (because the code with the off-by-one just goes away), and more efficient (we can drop the extra allocation we needed to create NUL-terminated strings for each line, and just terminate in place). Nice. 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: [RFC/PATCH v2 3/3] status: introduce status.displayCommentChar to disable display of #
Matthieu Moy matthieu@imag.fr writes:
diff --git a/Documentation/config.txt b/Documentation/config.txt
index ec57a15..dacf4b9 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2118,6 +2118,11 @@ status.branch::
Set to true to enable --branch by default in linkgit:git-status[1].
The option --no-branch takes precedence over this variable.
+status.displayCommentChar::
+ If set to false, linkgit:git-status[1] will not prefix each
+ line with the comment char (`core.commentchar`, i.e. `#` by
+ default). Defaults to true.
We prefix core.commentchar followed by a single space for non-empty
lines; is that worth mentioning, I wonder?
Also I envision that we would extend core.commentchar to be more
than a single character. Is displayCommentChar still a good name
for this setting?
status.omitCommentPrefix that defaults to false might be a better
setting, I suspect. I further suspect that in the longer term, we
may want to consider flipping its default to true (for Git 2.0, it
is too late). I do not think we need these comment prefix in the
human readable status output---after all, there is no line that is
not commented (other than nothing added to commit at the end) in
the current output, so the comment prefix only wastes the screen
real estate.
What are our plans to help existing scripts people have written over
time, especially before status -s was invented, that will be
broken by use of this?
@@ -663,17 +666,18 @@ static void wt_status_print_submodule_summary(struct
wt_status *s, int uncommitt
char summary_limit[64];
char index[PATH_MAX];
const char *env[] = { NULL, NULL };
- const char *argv[8];
+ const char *argv[9];
env[0] =index;
argv[0] = submodule;
argv[1] = summary;
argv[2] = uncommitted ? --files : --cached;
argv[3] = --for-status;
- argv[4] = --summary-limit;
- argv[5] = summary_limit;
- argv[6] = uncommitted ? NULL : (s-amend ? HEAD^ : HEAD);
- argv[7] = NULL;
+ argv[4] = s-display_comment_char ? --display-comment-char :
--no-display-comment-char;
+ argv[5] = --summary-limit;
+ argv[6] = summary_limit;
+ argv[7] = uncommitted ? NULL : (s-amend ? HEAD^ : HEAD);
+ argv[8] = NULL;
Not strictly your fault, but we should lose these funny indent after
= and use argv_array API here.
--
To unsubscribe from this list: send the line unsubscribe 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/8] remote-bzr: fix export of utf-8 authors
Felipe Contreras felipe.contre...@gmail.com writes: + echo greg content + git add content + git commit -m one test_commit would make it shorter. + bzr log | grep ^committer: ../actual + ) + + echo committer: Grégoire commit...@example.com expected Git's source code usually says ../actual and expected, without space after ''. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH v2 3/3] status: introduce status.displayCommentChar to disable display of #
Junio C Hamano gits...@pobox.com writes: status.omitCommentPrefix that defaults to false might be a better setting, I suspect. Or status.showCommentPrefix that defaults to true; I didn't mean to say a setting that defaults to false is preferred over one that defaults to true in my message. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: the pager
wor...@alum.mit.edu (Dale R. Worley) writes: From: Junio C Hamano gits...@pobox.com I've noticed that Git by default puts long output through less as a pager. I don't like that, but this is not the time to change established behavior. But while tracking that down, I noticed that the paging behavior is controlled by at least 5 things: the -p/--paginate/--no-pager options the GIT_PAGER environment variable the PAGER environment variable the core.pager Git configuration variable the build-in default (which seems to usually be less) ... What is the (intended) order of precedence of specifiers of paging behavior? My guess is that it should be the order I've given above. I think that sounds about right (I didn't check the code, though). The most specific to the command line invocation (i.e. option) trumps the environment, which trumps the configured default, and the hard wired stuff is used as the fallback default. I am not sure about PAGER environment and core.pager, though. People want Git specific pager that applies only to Git process specified to core.pager, and still want to use their own generic PAGER to other programs, so my gut feeling is that it would make sense to consider core.pager a way to specify GIT_PAGER without contaminating the environment, and use both to override the generic PAGER (in other words, core.pager should take precedence over PAGER as far as Git is concerned). I've just discovered this bit of documentation. Within the git-var manual page is this entry: GIT_PAGER Text viewer for use by git commands (e.g., less). The value is meant to be interpreted by the shell. The order of preference is the $GIT_PAGER environment variable, then core.pager configuration, then $PAGER, and then finally less. This suggests that the ordering is GIT_PAGER core.pager PAGER default. OK, that means that my gut feeling was right, we do the right thing, and we do document it. But your original documentation in git.1 and git-config.1, and the two are not coordinated to make it clear what happens in all cases. still stands. How can we improve the documentation to make the above paragraph easier to discover? Perhaps use the above wording to update git-config.1 that already mentions GIT_PAGER in the section for core.pager? The description over there is so incoherent that I needed to read it three times to see what points are mentioned. How about doing this? -- 8 -- config: rewrite core.pager documentation The text mentions core.pager and GIT_PAGER without giving the overall picture of precedences. Borrow a better description from the git-var(1) documentation. The use of the mechanism to allow system-wide, global and per-repository configuration files is not limited to this particular variable. Remove it to clarify the paragraph. Rewrite the part that explains how the environment variable LESS is set to Git's default value, and how to selectively customize it. Signed-off-by: Junio C Hamano gits...@pobox.com --- Documentation/config.txt | 28 1 file changed, 12 insertions(+), 16 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index ec57a15..7f9bc38 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -553,22 +553,18 @@ sequence.editor:: When not configured the default commit message editor is used instead. core.pager:: - The command that Git will use to paginate output. Can - be overridden with the `GIT_PAGER` environment - variable. Note that Git sets the `LESS` environment - variable to `FRSX` if it is unset when it runs the - pager. One can change these settings by setting the - `LESS` variable to some other value. Alternately, - these settings can be overridden on a project or - global basis by setting the `core.pager` option. - Setting `core.pager` has no effect on the `LESS` - environment variable behaviour above, so if you want - to override Git's default settings this way, you need - to be explicit. For example, to disable the S option - in a backward compatible manner, set `core.pager` - to `less -+S`. This will be passed to the shell by - Git, which will translate the final command to - `LESS=FRSX less -+S`. + Text viewer for use by Git commands (e.g., 'less'). The value + is meant to be interpreted by the shell. The order of preference + is the `$GIT_PAGER` environment variable, then `core.pager` + configuration, then `$PAGER`, and then the default chosen at + compile time (usually 'less'). ++ +When the `LESS` environment variable is unset, Git sets it to `FRSX` +(if `LESS` environment variable is set, Git does not change it at +all). If you want to override Git's default setting for `LESS`, you +can set `core.pager` to `less -+S`. This will be passed to
Re: [RFC/PATCH v2 3/3] status: introduce status.displayCommentChar to disable display of #
On Wed, Aug 28, 2013 at 01:05:38PM -0700, Junio C Hamano wrote: What are our plans to help existing scripts people have written over time, especially before status -s was invented, that will be broken by use of this? I thought that our response to parsing the long output of git status was always you are doing it wrong. The right way has always been to run the plumbing tools yourself, followed eventually by the --porcelain mode to git status being blessed as a convenient plumbing. I will not say that people might not do it anyway, but at what point do we say you were warned? -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/8] remote-bzr: fix export of utf-8 authors
On Wed, Aug 28, 2013 at 3:05 PM, Matthieu Moy matthieu@grenoble-inp.fr wrote: Felipe Contreras felipe.contre...@gmail.com writes: + echo greg content + git add content + git commit -m one test_commit would make it shorter. And it would make it inconsistent with the rest of the script. + bzr log | grep ^committer: ../actual + ) + + echo committer: Grégoire commit...@example.com expected Git's source code usually says ../actual and expected, without space after ''. Not that usually: % git grep ' ' t/*.sh | wc -l 1943 -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe 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 v2 3/3] status: introduce status.displayCommentChar to disable display of #
Junio C Hamano gits...@pobox.com writes:
Matthieu Moy matthieu@imag.fr writes:
diff --git a/Documentation/config.txt b/Documentation/config.txt
index ec57a15..dacf4b9 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2118,6 +2118,11 @@ status.branch::
Set to true to enable --branch by default in linkgit:git-status[1].
The option --no-branch takes precedence over this variable.
+status.displayCommentChar::
+If set to false, linkgit:git-status[1] will not prefix each
+line with the comment char (`core.commentchar`, i.e. `#` by
+default). Defaults to true.
We prefix core.commentchar followed by a single space for non-empty
lines; is that worth mentioning, I wonder?
(and sometimes # is followed by a tab)
I don't think it's worth the trouble. The goal is not to document the
format precisely, but to explain the user how to use the variable.
Also I envision that we would extend core.commentchar to be more
than a single character. Is displayCommentChar still a good name
for this setting?
It is as good as core.commentChar ;-).
I chose the variable name to remind commentChar (after finding
commentChar, one can grep it and find status.displayCommentChar). I tend
to think that it's better than omitCommentPrefix, but I can change is
people think it's better.
What are our plans to help existing scripts people have written over
time, especially before status -s was invented, that will be
broken by use of this?
I don't know what's the best plan, but I think any sensible plan starts
by waiting for a few releases, so that Git version not implementing
status.displayCommentChar become rare enough. So we can delay the actual
plan until next year I guess.
That said, I had an idea that may help the transition: allow auto as a
value, just like we did for colors. A patch implementing this (on top of
the previous series) is below. Good point: auto is a safe value, as it
won't break scripts. There is one drawback, though: auto is not a
really good default value in the long run, since newcommers may wonder
why there are differences between git status and git status | cat.
I still find this tempting, and it would make the transition so much
easier. I would even argue to flip the default to auto for Git 2.0
then (after all, we didn't even wait for this to change color.ui).
Maybe, later, a transition from auto to never could be done. Or
perhaps it's not so terrible to keep auto (my ls and ls | cat
produce very different outputs, and it never bugged me).
diff --git a/builtin/commit.c b/builtin/commit.c
index d4cfced..00e9487 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -163,6 +163,21 @@ static void determine_whence(struct wt_status *s)
s-whence = whence;
}
+static void determine_comment_mode(struct wt_status *s)
+{
+ if (s-display_comment_char == COMMENT_AUTO) {
+ /*
+* determine_comment_mode is used only for cmd_status,
+* which always prints to stdout.
+*/
+ if (isatty(1) || pager_in_use()) {
+ s-display_comment_char = COMMENT_NEVER;
+ } else {
+ s-display_comment_char = COMMENT_ALWAYS;
+ }
+ }
+}
+
static void rollback_index_files(void)
{
switch (commit_style) {
@@ -1183,6 +1198,20 @@ static int git_status_config(const char *k, const char
*v, void *cb)
return 0;
}
if (!strcmp(k, status.displaycommentchar)) {
+ if (v) {
+ if (!strcasecmp(v, never)) {
+ s-display_comment_char = COMMENT_NEVER;
+ return 0;
+ }
+ if (!strcasecmp(v, always)) {
+ s-display_comment_char = COMMENT_ALWAYS;
+ return 0;
+ }
+ if (!strcasecmp(v, auto)) {
+ s-display_comment_char = COMMENT_AUTO;
+ return 0;
+ }
+ }
s-display_comment_char = git_config_bool(k, v);
return 0;
}
@@ -1254,6 +1283,7 @@ int cmd_status(int argc, const char **argv, const char
*prefix)
gitmodules_config();
git_config(git_status_config, s);
determine_whence(s);
+ determine_comment_mode(s);
argc = parse_options(argc, argv, prefix,
builtin_status_options,
builtin_status_usage, 0);
@@ -1504,7 +1534,7 @@ int cmd_commit(int argc, const char **argv, const char
*prefix)
* Ignore status.displayCommentChar: we do need comments in
* COMMIT_EDITMSG.
*/
- s.display_comment_char = 1;
+ s.display_comment_char = COMMENT_ALWAYS;
determine_whence(s);
s.colopts = 0;
diff
Re: [PATCH 1/8] remote-bzr: fix export of utf-8 authors
Felipe Contreras felipe.contre...@gmail.com writes: On Wed, Aug 28, 2013 at 3:05 PM, Matthieu Moy matthieu@grenoble-inp.fr wrote: + bzr log | grep ^committer: ../actual + ) + + echo committer: Grégoire commit...@example.com expected Git's source code usually says ../actual and expected, without space after ''. Not that usually: % git grep ' ' t/*.sh | wc -l 1943 Do I really need to quote the paragraph in CodingGuidelines? -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH v2 2/3] submodule: introduce --[no-]display-comment-char
Am 28.08.2013 14:47, schrieb Matthieu Moy:
Signed-off-by: Matthieu Moy matthieu@imag.fr
---
(--for-status was undocumented, so I didn't bother documenting this either)
That's fine, as both if these options will - hopefully - only be used
by wt-status.c internally.
But I'm not terribly happy about having the --for-status option in the
submodule script in the first place, as I believe it should rather be
handled by wt-status.c itself (reading the output of submodule summary
using the start_command()/finish_command() combo instead of the simple
run_command() currently used and prepending the comment char by using
status_printf() friends for each line).
Having said that (and currently not having the time to do the above
myself), if we go along the route you are currently taking I'd prefer
to only add one other to-be-obsoleted-some-time-later option to the
submodule script. So what about only adding --no-display-comment-char
(and maybe call it --drop-status-comment-char or such to make meaning
and context a bit more obvious ;-)?
git-submodule.sh | 20
1 file changed, 16 insertions(+), 4 deletions(-)
diff --git a/git-submodule.sh b/git-submodule.sh
index 2979197..ac0fdad 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -966,6 +966,7 @@ set_name_rev () {
cmd_summary() {
summary_limit=-1
for_status=
+ display_comment_char=t
diff_cmd=diff-index
# parse $args after submodule ... summary.
@@ -981,6 +982,12 @@ cmd_summary() {
--for-status)
for_status=$1
;;
+ --display-comment-char)
+ display_comment_char=t
+ ;;
+ --no-display-comment-char)
+ display_comment_char=
+ ;;
-n|--summary-limit)
summary_limit=$2
isnumber $summary_limit || usage
@@ -1151,13 +1158,18 @@ cmd_summary() {
echo
done |
if test -n $for_status; then
+ if test -n $display_comment_char; then
+ filter_cmd='git stripspace -c'
+ else
+ filter_cmd=cat
+ fi
if [ -n $files ]; then
- gettextln Submodules changed but not updated: | git
stripspace -c
+ gettextln Submodules changed but not updated: |
$filter_cmd
else
- gettextln Submodule changes to be committed: | git
stripspace -c
+ gettextln Submodule changes to be committed: |
$filter_cmd
fi
- printf \n | git stripspace -c
- git stripspace -c
+ printf \n | $filter_cmd
+ $filter_cmd
else
cat
fi
--
To unsubscribe from this list: send the line 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 v2] remote-bzr: fix export of utf-8 authors
Reported-by: Joakim Verona joa...@verona.se
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
contrib/remote-helpers/git-remote-bzr | 1 +
contrib/remote-helpers/test-bzr.sh| 30 ++
2 files changed, 31 insertions(+)
diff --git a/contrib/remote-helpers/git-remote-bzr
b/contrib/remote-helpers/git-remote-bzr
index c3a3cac..08b0b61 100755
--- a/contrib/remote-helpers/git-remote-bzr
+++ b/contrib/remote-helpers/git-remote-bzr
@@ -168,6 +168,7 @@ class Parser:
if not m:
return None
_, name, email, date, tz = m.groups()
+name = name.decode('utf-8')
committer = '%s %s' % (name, email)
tz = int(tz)
tz = ((tz / 100) * 3600) + ((tz % 100) * 60)
diff --git a/contrib/remote-helpers/test-bzr.sh
b/contrib/remote-helpers/test-bzr.sh
index dce281f..b0d70fd 100755
--- a/contrib/remote-helpers/test-bzr.sh
+++ b/contrib/remote-helpers/test-bzr.sh
@@ -358,4 +358,34 @@ test_expect_success 'strip' '
test_cmp expected actual
'
+test_expect_success 'export utf-8 authors' '
+ test_when_finished rm -rf bzrrepo gitrepo LC_ALL=C unset
GIT_COMMITTER_NAME
+
+ LC_ALL=en_US.UTF-8
+ export LC_ALL
+
+ GIT_COMMITTER_NAME=Grégoire
+ export GIT_COMMITTER_NAME
+
+ bzr init bzrrepo
+
+ (
+ git init gitrepo
+ cd gitrepo
+ echo greg content
+ git add content
+ git commit -m one
+ git remote add bzr bzr::../bzrrepo
+ git push bzr
+ )
+
+ (
+ cd bzrrepo
+ bzr log | grep ^committer: ../actual
+ )
+
+ echo committer: Grégoire commit...@example.com expected
+ test_cmp expected actual
+'
+
test_done
--
1.8.4-fc-3-g64bc480
--
To unsubscribe from this list: send the line unsubscribe 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/8] remote-bzr: fix export of utf-8 authors
On Wed, Aug 28, 2013 at 3:54 PM, Antoine Pelisse apeli...@gmail.com wrote: On Wed, Aug 28, 2013 at 10:48 PM, Felipe Contreras felipe.contre...@gmail.com wrote: On Wed, Aug 28, 2013 at 3:05 PM, Matthieu Moy matthieu@grenoble-inp.fr wrote: Felipe Contreras felipe.contre...@gmail.com writes: + echo greg content + git add content + git commit -m one test_commit would make it shorter. And it would make it inconsistent with the rest of the script. + bzr log | grep ^committer: ../actual + ) + + echo committer: Grégoire commit...@example.com expected Git's source code usually says ../actual and expected, without space after ''. Not that usually: % git grep ' ' t/*.sh | wc -l 1943 There are many false positive in that count. As in this one: $ git grep ' [^ ]' -- t/*.sh | wc -l 10354 Ten thousand is still a lot, and the claim remains: it's far from unusual to follow this style. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe 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/8] remote-bzr: fix export of utf-8 authors
On Wed, Aug 28, 2013 at 4:05 PM, Matthieu Moy matthieu@grenoble-inp.fr wrote: Felipe Contreras felipe.contre...@gmail.com writes: On Wed, Aug 28, 2013 at 3:05 PM, Matthieu Moy matthieu@grenoble-inp.fr wrote: + bzr log | grep ^committer: ../actual + ) + + echo committer: Grégoire commit...@example.com expected Git's source code usually says ../actual and expected, without space after ''. Not that usually: % git grep ' ' t/*.sh | wc -l 1943 Do I really need to quote the paragraph in CodingGuidelines? Do I need to explain what a guideline is? -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe 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 v2 3/3] status: introduce status.displayCommentChar to disable display of #
Matthieu Moy matthieu@imag.fr writes: diff --git a/wt-status.c b/wt-status.c index cb24f1f..97068d5 100644 --- a/wt-status.c +++ b/wt-status.c @@ -774,12 +778,21 @@ static void wt_status_print_tracking(struct wt_status *s) if (!format_tracking_info(branch, sb)) return; + char comment_line_string[3]; + int i = 0; decl-after-statement; queued with a fix to be squashed in. -- To unsubscribe from this list: send the line unsubscribe 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 v2 3/3] status: introduce status.displayCommentChar to disable display of #
Matthieu Moy matthieu@grenoble-inp.fr writes: Junio C Hamano gits...@pobox.com writes: Matthieu Moy matthieu@imag.fr writes: diff --git a/Documentation/config.txt b/Documentation/config.txt index ec57a15..dacf4b9 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -2118,6 +2118,11 @@ status.branch:: Set to true to enable --branch by default in linkgit:git-status[1]. The option --no-branch takes precedence over this variable. +status.displayCommentChar:: + If set to false, linkgit:git-status[1] will not prefix each + line with the comment char (`core.commentchar`, i.e. `#` by + default). Defaults to true. We prefix core.commentchar followed by a single space for non-empty lines; is that worth mentioning, I wonder? (and sometimes # is followed by a tab) I don't think it's worth the trouble. The goal is not to document the format precisely, but to explain the user how to use the variable. Also I envision that we would extend core.commentchar to be more than a single character. Is displayCommentChar still a good name for this setting? It is as good as core.commentChar ;-). Not really; taken together with # has a space after it, calling it prefix in the mechanism to omit it makes tons of sense. What are our plans to help existing scripts people have written over time, especially before status -s was invented, that will be broken by use of this? I don't know what's the best plan, but I think any sensible plan starts by waiting for a few releases, so that Git version not implementing status.displayCommentChar become rare enough. So we can delay the actual plan until next year I guess. Actually as Peff pointed out, we've already told number of times that status output is for human consumption and scripts should not attempt to parse it, long before we gave them -s/--porcelain options, so we are good without auto. -- To unsubscribe from this list: send the line unsubscribe 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/8] remote-bzr: fix export of utf-8 authors
Matthieu Moy matthieu@grenoble-inp.fr writes: Felipe Contreras felipe.contre...@gmail.com writes: On Wed, Aug 28, 2013 at 3:05 PM, Matthieu Moy matthieu@grenoble-inp.fr wrote: + bzr log | grep ^committer: ../actual + ) + + echo committer: Grégoire commit...@example.com expected Git's source code usually says ../actual and expected, without space after ''. Not that usually: % git grep ' ' t/*.sh | wc -l 1943 Do I really need to quote the paragraph in CodingGuidelines? Existing violations are not an excuse to make things worse by adding more. I think with these comments we can expect a reroll coming, and it should be trivial for any contributor to fix it while at 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: [PATCHv2] git-diff: Clarify operation when not inside a repository.
wor...@alum.mit.edu (Dale R. Worley) writes: For now, I'll queue your version as-is modulo style fixes, while waiting for others to help polishing the documentation better. It'd difficult to figure out how to describe it well. In my opinion, the problem here is the DWIM nature of the command, ... My preference is ... But that's not how git-diff works. So given the constraints, I think this is the best we can do. As nobody seems to be helping to polish the text, here is my attempt, on top of your patch. Documentation/git-diff.txt | 14 +- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/Documentation/git-diff.txt b/Documentation/git-diff.txt index b1630ba..33fbd8c 100644 --- a/Documentation/git-diff.txt +++ b/Documentation/git-diff.txt @@ -28,11 +28,15 @@ two blob objects, or changes between two files on disk. words, the differences are what you _could_ tell Git to further add to the index but you still haven't. You can stage these changes by using linkgit:git-add[1]. -+ -If exactly two paths are given and at least one points outside -the current repository, 'git diff' will compare the two files / -directories. This behavior can be forced by --no-index or by -executing 'git diff' outside of a working tree. + +'git diff' --no-index [--options] [--] [path...]:: + + This form is to compare the given two paths on the + filesystem. You can omit the `--no-index` option when + running the command in a working tree controlled by Git and + at least one of the paths points outside the working tree, + or when running the command outside a working tree + controlled by Git. 'git diff' [--options] --cached [commit] [--] [path...]:: -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/8] remote-bzr: fix export of utf-8 authors
On Wed, Aug 28, 2013 at 4:58 PM, Junio C Hamano gits...@pobox.com wrote:
Matthieu Moy matthieu@grenoble-inp.fr writes:
Felipe Contreras felipe.contre...@gmail.com writes:
On Wed, Aug 28, 2013 at 3:05 PM, Matthieu Moy
matthieu@grenoble-inp.fr wrote:
+ bzr log | grep ^committer: ../actual
+ )
+
+ echo committer: Grégoire commit...@example.com expected
Git's source code usually says ../actual and expected, without space
after ''.
Not that usually:
% git grep ' ' t/*.sh | wc -l
1943
Do I really need to quote the paragraph in CodingGuidelines?
Existing violations are not an excuse to make things worse by adding
more.
1)
By definition following a guideline is not mandatory:
https://en.wikipedia.org/wiki/Guideline
Violation is a too strong word for not following something that is not
mandatory in the first place; deviation is a more appropriate word.
2)
This code is part of the 'contrib' area, and part of the tests, most
of the code in this area doesn't even have tests.
3)
Blindly following guidelines is not a good idea.
cmd out
cmd out 2err
if (n 1)
cmd1 | cmd2
Clearly, those are inconsistent.
cmd out
cmd out 2 err
if (n 1)
cmd1 | cmd2
Those are not.
Fortunately since these rules are only guidelines, there's no need to
engage in bike-sheding argumentation if the code deviates from
guideline, specially for this insignificantly trivial issue.
Blocking a patch that would be obviously advantageous for the users on
the basis of a single white-space on the tests which aren't even run
by default is simply stupid.
I think with these comments we can expect a reroll coming,
and it should be trivial for any contributor to fix it while at it.
I already send the update, I do not see the need for any more changes,
so no more changes will come from me unless there is valid feedback.
If it makes you happier, apply this:
From 7db40fa6b2780f9b5787c7d38f0d2dac01b175d3 Mon Sep 17 00:00:00 2001
From: Felipe Contreras felipe.contre...@gmail.com
Date: Sun, 4 Aug 2013 19:27:51 -0500
Subject: [PATCH] remote-bzr: fix export of utf-8 authors
Reported-by: Joakim Verona joa...@verona.se
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
contrib/remote-helpers/git-remote-bzr | 1 +
1 file changed, 1 insertion(+)
diff --git a/contrib/remote-helpers/git-remote-bzr
b/contrib/remote-helpers/git-remote-bzr
index c3a3cac..08b0b61 100755
--- a/contrib/remote-helpers/git-remote-bzr
+++ b/contrib/remote-helpers/git-remote-bzr
@@ -168,6 +168,7 @@ class Parser:
if not m:
return None
_, name, email, date, tz = m.groups()
+name = name.decode('utf-8')
committer = '%s %s' % (name, email)
tz = int(tz)
tz = ((tz / 100) * 3600) + ((tz % 100) * 60)
--
1.8.4-fc-3-g64bc480
There, there is no more test GUIDEline violation.
--
Felipe Contreras
--
To unsubscribe from this list: send the line unsubscribe 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 v2 3/3] status: introduce status.displayCommentChar to disable display of #
David Aguilar dav...@gmail.com writes: On Wed, Aug 28, 2013 at 2:39 PM, Junio C Hamano gits...@pobox.com wrote: Jeff King p...@peff.net writes: On Wed, Aug 28, 2013 at 01:05:38PM -0700, Junio C Hamano wrote: What are our plans to help existing scripts people have written over time, especially before status -s was invented, that will be broken by use of this? I thought that our response to parsing the long output of git status was always you are doing it wrong. The right way has always been to run the plumbing tools yourself, followed eventually by the --porcelain mode to git status being blessed as a convenient plumbing. I will not say that people might not do it anyway, but at what point do we say you were warned? OK, sounds good enough. I personally think it's a little strange for this to be configurable. I have a poor imagination and cannot imagine why it needs to be switchable. If someone switches it to a does that mean that any commit message line that starts with the letter a will be filtered out? Specifically, core.commentchar seems really unnecessary to me. What is the benefit? I do see downsides -- folks do parse the output, they don't read the release notes, they don't know any better, but, hey, it works, so they'll be broken just because someone doesn't like #? What about hooks that write custom commit messages? Do they need to start caring about core.commentchar? They are valid questions, I think, but are best asked to those who wanted core.commentchar configuration, not to people involved in this thread. Also unfortunately it is too late by two releases to ask that question. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
http.postBuffer set at the server side?
We have systems hosting git which are behind proxies, and unless the client sets the http.postBuffer to a large size they connections fails. Is there a way to set this on the server side? If not would a patch be possible to fix this? jason.pyeron@hostname /home/jason.pyeron/desktop/projectname $ git push remote --all Username for 'https://server.fqdn': Password for 'https://jpye...@server.fqdn': Counting objects: 1820, done. Delta compression using up to 4 threads. Compressing objects: 100% (1276/1276), done. error: RPC failed; result=22, HTTP code = 411 fatal: The remote end hung up unexpectedly Writing objects: 100% (1820/1820), 17.72 MiB | 5.50 MiB/s, done. Total 1820 (delta 527), reused 26 (delta 6) fatal: The remote end hung up unexpectedly jason.pyeron@hostname /home/jason.pyeron/desktop/projectname $ git config http.postBuffer 524288000 jason.pyeron@hostname /home/jason.pyeron/desktop/projectname $ git push remote --all Username for 'https://server.fqdn': Password for 'https://jpye...@server.fqdn': Counting objects: 1820, done. Delta compression using up to 4 threads. Compressing objects: 100% (1276/1276), done. Writing objects: 100% (1820/1820), 17.72 MiB | 11.31 MiB/s, done. Total 1820 (delta 519), reused 26 (delta 6) To https://server.fqdn/git/netasset-portal/ * [new branch] master - master smime.p7s Description: S/MIME cryptographic signature
Re: [RFC/PATCH v2 3/3] status: introduce status.displayCommentChar to disable display of #
Matthieu Moy wrote: Historically, git status needed to prefix each output line with '#' so that the output could be added as comment to the commit message. This prefix comment has no real purpose when git status is ran from the command-line, and this may distract users from the real content. Allow the user to disable this prefix comment. Why not do this unconditionally? In the long run, if users like the non-prefix output, it may make sense to flip the default value to true. Hmm, do you mean that the configuration is to give the change-averse some time to get used to the new output? I think it would make sense to do it all at once (and even think that that would be less confusing). The rest of the idea and execution seem sane. Thanks, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] pull: require choice between rebase/merge on non-fast-forward pull
On Fri, Jun 28, 2013 at 5:41 PM, Junio C Hamano gits...@pobox.com wrote: John Keeping j...@keeping.me.uk writes: I don't think git pull remote branch falls into the same category as plain git pull so I'm not convinced that defaulting to merge there is unreasonable. The original message about this [1] did talk about only git pull with no arguments. If you want to limit the scope to only git pull (without any command line argument), I actually do not have strong preference for or against it either way. Perhaps a follow-up patch to be squashed? I do. Whether the user does 'git pull' or 'git pull origin' doesn't matter, we still want to reject non-fast-forward merges. -- Felipe Contreras -- To unsubscribe from this list: send the line 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 (Aug 2013, #07; Wed, 28)
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 'next' has been rewound. I ejected a handful of topics that have been cooking there while rebuilding it, but it is not because I found anything in them problematic, but merely because they were young and I wanted to give their authors a chance to tweak with a reroll instead of a set of follow-up patches. Unless I hear otherwise in a few days, they will be merged back to 'next'. By the way, the push that overrides the usual must fast-forward was done using the force-with-lease option that has been cooking in next, like so: $ git fetch ko next $ anchor=$(git rev-parse --verify FETCH_HEAD) $ for remote in ko repo gph github2 do git push --force-with-lease=refs/heads/next:$anchor $remote next done I used to do this with just --force. 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] * cc/replace-with-the-same-type (2013-08-27) 5 commits - Documentation/replace: add Creating Replacement Objects section - t6050-replace: add test to clean up all the replace refs - t6050-replace: test that objects are of the same type - Documentation/replace: state that objects must be of the same type - replace: forbid replacing an object with one of a different type Using the replace mechanism to swap an object with another object of a different type can introduce inconsistency (e.g. a tree expects an object name to refer to a blob, but the blob object can be mistakenly or maliciously replaced with an object with a different type). Attempt to forbid such. This may need to be given an escape hatch --force, though. * jx/clean-interactive (2013-08-28) 1 commit - documentation: clarify notes for clean.requireForce Finishing touches to update the document to adjust to a new option git clean learned recently. Will merge to 'next'. * mm/status-without-comment-char (2013-08-28) 3 commits - SQUASH??? wt-status.c decl-after-stmt - status: introduce status.displayCommentChar to disable display of # - submodule: introduce --[no-]display-comment-char Allow git status to omit the prefix to make its output a comment in a commit log editor, which is not necessary for human consumption. * xx/cleanup (2013-08-28) 1 commit - builtin/stripspace.c: fix broken indentation Will probably directly cherry-pick to 'maint' or 'master' and kill the branch. -- [Stalled] * rv/send-email-cache-generated-mid (2013-08-21) 2 commits - git-send-email: Cache generated message-ids, use them when prompting - git-send-email: add optional 'choices' parameter to the ask sub * rj/read-default-config-in-show-ref-pack-refs (2013-06-17) 3 commits - ### DONTMERGE: needs better explanation on what config they need - pack-refs.c: Add missing call to git_config() - show-ref.c: Add missing call to git_config() The changes themselves are probably good, but it is unclear what basic setting needs to be read for which exact operation. Waiting for clarification. $gmane/228294 * jh/shorten-refname (2013-05-07) 4 commits - t1514: refname shortening is done after dereferencing symbolic refs - shorten_unambiguous_ref(): Fix shortening refs/remotes/origin/HEAD to origin - t1514: Demonstrate failure to correctly shorten refs/remotes/origin/HEAD - t1514: Add tests of shortening refnames in strict/loose mode When remotes/origin/HEAD is not a symbolic ref, rev-parse --abbrev-ref remotes/origin/HEAD ought to show origin, not origin/HEAD, which is fixed with this series (if it is a symbolic ref that points at remotes/origin/something, then it should show origin/something and it already does). Expecting a reroll, as an early part of a larger series. $gmane/225137 * jk/list-objects-sans-blobs (2013-06-06) 4 commits . archive: ignore blob objects when checking reachability . list-objects: optimize revs-blob_objects = 0 case . upload-archive: restrict remote objects with reachability check . clear parsed flag when we free tree buffers Attempt to allow archive --remote=$there $arbitrary_sha1 while keeping the reachability safety. Seems to break some tests in a trivial and obvious way. * mg/more-textconv (2013-05-10) 7 commits - grep: honor --textconv for the case rev:path - grep: allow to use textconv filters - t7008: demonstrate behavior of grep with textconv - cat-file: do not die on --textconv without textconv filters - show: honor --textconv for blobs - diff_opt: track whether flags have been set explicitly - t4030: demonstrate behavior of show with textconv Make git grep and git show pay attention to --textconv when dealing with blob objects. I thought this was pretty well designed
Re: What's cooking in git.git (Aug 2013, #07; Wed, 28)
On Aug 28, 2013, at 16:24, Junio C Hamano wrote: * km/svn-1.8-serf-only (2013-07-18) 3 commits (merged to 'next' on 2013-08-28 at 1119134) + Git.pm: revert _temp_cache use of temp_is_locked + git-svn: allow git-svn fetching to work using serf + Git.pm: add new temp_is_locked function Originally merged to 'next' on 2013-07-19 Subversion 1.8.0 that was recently released breaks older subversion clients coming over http/https in various ways. Will merge to 'master'. This series could be cleaned up by squashing Git.pm: revert _temp_cache use of temp_is_locked into Git.pm: add new temp_is_locked function if desired since that Git.pm: add new temp_is_locked function commit contains the one-line change that Git.pm: revert _temp_cache use of temp_is_locked reverts. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: http.postBuffer set at the server side?
On Wed, Aug 28, 2013 at 11:08:02PM +, Pyeron, Jason J CTR (US) wrote: We have systems hosting git which are behind proxies, and unless the client sets the http.postBuffer to a large size they connections fails. Is there a way to set this on the server side? If not would a patch be possible to fix this? What would it mean to set it on the server? It is the size at which the client decides to use a chunked transfer-encoding rather than buffering the whole output to send at once. So you'd want to figure out why the server is upset about the chunked encoding. jason.pyeron@hostname /home/jason.pyeron/desktop/projectname $ git push remote --all Username for 'https://server.fqdn': Password for 'https://jpye...@server.fqdn': Counting objects: 1820, done. Delta compression using up to 4 threads. Compressing objects: 100% (1276/1276), done. error: RPC failed; result=22, HTTP code = 411 fatal: The remote end hung up unexpectedly Writing objects: 100% (1820/1820), 17.72 MiB | 5.50 MiB/s, done. Total 1820 (delta 527), reused 26 (delta 6) fatal: The remote end hung up unexpectedly The server (or the proxy) returns 411, complaining that it didn't get a Content-Length header. That's because the git http client doesn't know how big the content is ahead of time (and that's kind of the point of chunked encoding; the content is streamed). jason.pyeron@hostname /home/jason.pyeron/desktop/projectname $ git config http.postBuffer 524288000 jason.pyeron@hostname /home/jason.pyeron/desktop/projectname $ git push remote --all Username for 'https://server.fqdn': Password for 'https://jpye...@server.fqdn': Counting objects: 1820, done. Delta compression using up to 4 threads. Compressing objects: 100% (1276/1276), done. Writing objects: 100% (1820/1820), 17.72 MiB | 11.31 MiB/s, done. Total 1820 (delta 519), reused 26 (delta 6) To https://server.fqdn/git/netasset-portal/ * [new branch] master - master And here you've bumped the buffer to 500MB, so git will potentially buffer that much in memory before sending anything. Which works for your 17MB packfile, as we buffer the whole thing and then send the exact size ahead of time, appeasing the proxy. But there are two problems I see with just bumping the postBuffer value: 1. You've just postponed the problem. The first 501MB push will fail again. You can bump it higher, but you may eventually hit a point where your buffer is too big to fit in RAM. 2. You've lost the pipelining. With a small postBuffer, we are streaming content up to the server as pack-objects generates it. But with a large buffer, we generate all of the content, then start sending the first byte (notice how the progress meter, which is generated by pack-objects, shows twice as fast in the second case. It is not measuring the network at all, but is streaming into git-remote-https's buffer). If the server really insists on a content-length header, then we can't ever fix (2). But we could fix (1) by spooling the packfile to disk and then sending from there (under the assumption that you have way more temporary disk space than RAM). However, if you have control of the proxies, the best thing would be to tweak its config to stop complaining about a lack of content-length header (at least in cases where you're getting a chunked content-transfer-encoding). That would solve both issues (and without clients having to change anything). -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: http.postBuffer set at the server side?
-Original Message- From: Jeff King Sent: Wednesday, August 28, 2013 23:52 To: Pyeron, Jason J CTR (US) Cc: git@vger.kernel.org Subject: Re: http.postBuffer set at the server side? On Wed, Aug 28, 2013 at 11:08:02PM +, Pyeron, Jason J CTR (US) wrote: We have systems hosting git which are behind proxies, and unless the client sets the http.postBuffer to a large size they connections fails. Is there a way to set this on the server side? If not would a patch be possible to fix this? What would it mean to set it on the server? It is the size at which the client decides to use a chunked To tell the client... transfer-encoding rather than buffering the whole output to send at once. So you'd want to figure out why the server is upset about the chunked encoding. Unchangable settings in a specific case here. jason.pyeron@hostname /home/jason.pyeron/desktop/projectname $ git push remote --all Username for 'https://server.fqdn': Password for 'https://jpye...@server.fqdn': Counting objects: 1820, done. Delta compression using up to 4 threads. Compressing objects: 100% (1276/1276), done. error: RPC failed; result=22, HTTP code = 411 fatal: The remote end hung up unexpectedly Writing objects: 100% (1820/1820), 17.72 MiB | 5.50 MiB/s, done. Total 1820 (delta 527), reused 26 (delta 6) fatal: The remote end hung up unexpectedly The server (or the proxy) returns 411, complaining that it didn't get a Content-Length header. That's because the git http client doesn't know how big the content is ahead of time (and that's kind of the point of chunked encoding; the content is streamed). jason.pyeron@hostname /home/jason.pyeron/desktop/projectname $ git config http.postBuffer 524288000 jason.pyeron@hostname /home/jason.pyeron/desktop/projectname $ git push remote --all Username for 'https://server.fqdn': Password for 'https://jpye...@server.fqdn': Counting objects: 1820, done. Delta compression using up to 4 threads. Compressing objects: 100% (1276/1276), done. Writing objects: 100% (1820/1820), 17.72 MiB | 11.31 MiB/s, done. Total 1820 (delta 519), reused 26 (delta 6) To https://server.fqdn/git/netasset-portal/ * [new branch] master - master And here you've bumped the buffer to 500MB, so git will potentially buffer that much in memory before sending anything. Which works for your 17MB packfile, as we buffer the whole thing and then send the exact size ahead of time, appeasing the proxy. But there are two problems I see with just bumping the postBuffer value: 1. You've just postponed the problem. The first 501MB push will fail again. You can bump it higher, but you may eventually hit a point where your buffer is too big to fit in RAM. Agreed. By then I hope to get our infrastructure team to address the proxies. Liking the spool idea below. The other idea would be restrict the size of any one transfer. (would that work if a given commit is large than the threshold???) 2. You've lost the pipelining. With a small postBuffer, we are streaming content up to the server as pack-objects generates it. But with a large buffer, we generate all of the content, then start sending the first byte (notice how the progress meter, which is generated by pack-objects, shows twice as fast in the second case. It is not measuring the network at all, but is streaming into git-remote-https's buffer). If the server really insists on a content-length header, then we can't ever fix (2). But we could fix (1) by spooling the packfile to disk and then sending from there (under the assumption that you have way more temporary disk space than RAM). Hmmm. So if the server says, hey I have a borked infrastructure, please send me a content length git could spool then. However, if you have control of the proxies, the best thing would be to tweak its config to stop complaining about a lack of content-length header (at least in cases where you're getting a chunked content-transfer-encoding). That would solve both issues (and without clients having to change anything). One of the many is under my control, I will get that one addressed but the others there is no hope. -Jason -- -=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=- - - - Jason Pyeron PD Inc. http://www.pdinc.us - - Principal Consultant 10 West 24th Street #100- - +1 (443) 269-1555 x333Baltimore, Maryland 21218 - - - -=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=- This message is copyright PD Inc, subject to license 20080407P00. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo
