Re: `git mv` has ambiguous error message for non-existing target
But just because mv's error essage isnt very good, does that mean git mv's error message mustn't be better? That would strike me as an odd bit of reasoning. On Fr 16 Nov 2012 02:34:32 CET, Junio C Hamano wrote: Patrick Lehner writes: To reproduce: - cd into a git repo - assuming "filea.txt" is an existing file in the CWD, and "dirb" is neither a file nor a directory in the CWD, use the command "git mv filea.txt dirb/filea.txt" - this will produce an error message like `fatal: renaming 'filea.sh' failed: No such file or directory` It does not mention that the problem is, in fact, the target directory not existing. This seems to be mostly a problem for users unfamiliar with bash/*nix console commands. Although it is documented that git mv will not create intermediate folders (which is fine, because neither does mv), the error message might lead to believe a problem exists with the source file. $ rm -fr xxx $ >yyy $ mv yyy xxx/yyy mv: cannot move `yyy' to `xxx/yyy': No such file or directory It doesn't mention that the problem is with 'xxx' and not 'yyy' either. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Crash when pushing large binary file
Thanks for the quick reply! On Fri, Nov 16, 2012 at 3:25 PM, Nguyen Thai Ngoc Duy wrote: > On Fri, Nov 16, 2012 at 12:44 PM, Thomas Gay wrote: >> Using Git 1.8 on Mac OS X 10.7.5. I just added a large binary file to > > How large exactly? 2.46 GB > If you set receive.unpacklimit to 1 on the receiving end, does it still crash? Yes. The crash log looks the same too. -Tom -- To unsubscribe from this list: send the line "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: Crash when pushing large binary file
On Fri, Nov 16, 2012 at 12:44 PM, Thomas Gay wrote: > Using Git 1.8 on Mac OS X 10.7.5. I just added a large binary file to How large exactly? > my repo, and each time I try to push it, Git crashes. I've attached > the crash log to this email and pasted the console output below. > > Counting objects: 27, done. > Delta compression using up to 4 threads. > Compressing objects: 100% (16/16), done. > error: unpack-objects died of signal 11 | 76.91 MiB/s > error: pack-objects died of signal 13 > error: failed to push some refs to 'ssh://...' If you set receive.unpacklimit to 1 on the receiving end, does it still crash? -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Crash when pushing large binary file
Using Git 1.8 on Mac OS X 10.7.5. I just added a large binary file to my repo, and each time I try to push it, Git crashes. I've attached the crash log to this email and pasted the console output below. Counting objects: 27, done. Delta compression using up to 4 threads. Compressing objects: 100% (16/16), done. error: unpack-objects died of signal 11 | 76.91 MiB/s error: pack-objects died of signal 13 error: failed to push some refs to 'ssh://...' -Tom git_2012-11-16-143138_localhost.crash Description: Binary data
Re: [PATCHv2 8/8] send-email: do not prompt for explicit repo ident
Jeff King writes: > That is a good question. That confirmation step does come after they > have typed their cover letter. However, if they are using --compose, > they are dumped in their editor with something like: > > From Jeff King # This line is ignored. > GIT: Lines beginning in "GIT:" will be removed. > GIT: Consider including an overall diffstat or table of contents > GIT: for the patch you are writing. > GIT: > GIT: Clear the body content if you don't wish to send a summary. > From: Jeff King > Subject: > In-Reply-To: > > which I think would count as sufficient notice of the address being > used. OK. Tentatively I replaced your old series with these 8 patches including the last one, as I tend to agree with the value the earlier clean-up in the series gives us in the longer term. As you and Felipe discussed, we may want to replace the last one with a simpler "don't bother asking" patch, but I think that is more or less an orthogonal issue. Thanks. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] wildmatch: correct isprint and isspace
On Fri, Nov 16, 2012 at 12:13 AM, "Jan H. Schönherr" wrote: >> #define isprint(x) (sane_istest(x, GIT_ALPHA | GIT_DIGIT | GIT_SPACE | \ >> GIT_PUNCT | GIT_REGEX_SPECIAL | GIT_GLOB_SPECIAL | \ >> - GIT_PATHSPEC_MAGIC)) >> + GIT_PATHSPEC_MAGIC) && \ >> + (x) >= 32) > > May I suggest the current is_print() implementation in master: > > #define isprint(x) ((x) >= 0x20 && (x) <= 0x7e) > > > To summarize my opinion: > > I no longer see a reason to correct isspace() (unless somebody with an actual > use case complains), and a more POSIXly isprint() is already in master. > > => Nothing to do. :) Yeah. I remember to remind myself to check "the implementation in master" you mentioned but I probably failed at that. Just checked that isprint() is already in master, and your comment about isspace() use in wildmatch.c makes sense too. So I'm all for doing nothing. -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Unify appending signoff in format-patch, commit and sequencer
On Fri, Nov 16, 2012 at 3:42 AM, Brandon Casey wrote: >> Interestingly this patch triggers the fault that it fixes. >> I was surprised that there was no blank line before my S-o-b >> and thought I broke something. It turns out I used unmodified >> format-patch and it mistook the S-o-b quote as true S-o-b line. >> The modified one puts the blank line back. > > Heh, yeah I noticed this bug yesterday when I submitted my changes > affecting the same area of code (sequence.c). Glad I didn't waste too > much time fixing it. > > Have you looked at this: > >http://thread.gmane.org/gmane.comp.version-control.git/209781 > > That series doesn't duplicate your work, but the two series's should > be resolved. Thanks I'm watching that discussion now and probably will rebase on top of yours once it's finalized. I actually wrote this patch a while ago, just waiting for nd/builtin-to-libgit to graduate because of some linking issues this patch causes. I can wait a bit longer until yours graduates. -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/5] t/t3511: demonstrate breakage in cherry-pick -s
On Thu, Nov 15, 2012 at 5:58 PM, Junio C Hamano wrote: > Brandon Casey writes: > >> The cherry-pick -s functionality is currently broken in two ways. >> >>1. handling of rfc2822 continuation lines has a bug, and the >> continuation lines are not handled correctly. > > This is not limited to you, but people should think twice when > writing "has a bug" and "are not handled correctly" in their log > message. Did you write what the expected and actual behaviours? Yeah, I wasn't clear here. The "bug" was that the incorrect index variable was being used, which caused the wrong line to be examined to see if it was an rfc2822 continuation line. I could have mentioned that. >> +rfc2822_mesg="$non_rfc2822_mesg >> + >> +Signed-off-by: A.U. Thor >> + >> +Signed-off-by: B.U. Thor " > > The S-o-b: lines are meant to record people's contact info in human > readable forms, and folding the lines like the above makes it a lot > harder to read. They typically do not have to be folded. Well, I wasn't adding functionality here, I was only fixing what I noticed was broken when I started touching this code. > Besides, the footer lines are *not* RFC2822 headers (and are not > used as such when send-email comes up with Cc: list) in the first > place; have we ever said anything about supporting the RFC2822 line > folding in the commit footer? If not (and I am reasonably sure we > never have), I personally think we should actively *discourage* line > folding there. That's fine with me. I can't think of a reason that would make it necessary to support line continuation. >> i.e. we should produce this: >> >> Signed-off-by: A.U. Thor >> (cherry picked from ) >> Signed-off-by: C O Mmitter >> >> not >> >> Signed-off-by: A.U. Thor >> (cherry picked from da39a3ee5e6b4b0d3255bfef95601890afd80709) >> >> Signed-off-by: C O Mmitter > > I can buy that, but then this makes it very clear that these footer > lines are not shaped like RFC2822 headers, no? The lines that are _not_ "(cherry picked from ...)" lines do follow the format defined by rfc2822 for header lines (mostly). That's probably why the author of the function in sequencer.c that checks for a s-o-b footer named it "ends_rfc2822_footer". I'll remove the *broken* existing code that was intended to support continuation lines and submit a new patch. -Brandon -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/5] sequencer.c: refrain from adding duplicate s-o-b lines
Brandon Casey writes: > Detect whether the s-o-b already exists in the commit footer and refrain > from adding a duplicate. If you are trying to forbid git cherry-pick -s $other from adding s-o-b: A when $other ends with these two existing s-o-b: s-o-b: A s-o-b: B then I think that is a wrong thing to do. In such a case, the resulting commit should gain another s-o-b from A to record the provenance as a chain of events. A originally wrote the patch, B forwarded it (possibly with his own tweaks), and then A picked it up and recorded the result to the history, possibly with a final tweak or two. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/5] t/t3511: demonstrate breakage in cherry-pick -s
Brandon Casey writes: > The cherry-pick -s functionality is currently broken in two ways. > >1. handling of rfc2822 continuation lines has a bug, and the > continuation lines are not handled correctly. This is not limited to you, but people should think twice when writing "has a bug" and "are not handled correctly" in their log message. Did you write what the expected and actual behaviours? > +rfc2822_mesg="$non_rfc2822_mesg > + > +Signed-off-by: A.U. Thor > + > +Signed-off-by: B.U. Thor " The S-o-b: lines are meant to record people's contact info in human readable forms, and folding the lines like the above makes it a lot harder to read. They typically do not have to be folded. Besides, the footer lines are *not* RFC2822 headers (and are not used as such when send-email comes up with Cc: list) in the first place; have we ever said anything about supporting the RFC2822 line folding in the commit footer? If not (and I am reasonably sure we never have), I personally think we should actively *discourage* line folding there. > i.e. we should produce this: > > Signed-off-by: A.U. Thor > (cherry picked from ) > Signed-off-by: C O Mmitter > > not > > Signed-off-by: A.U. Thor > (cherry picked from da39a3ee5e6b4b0d3255bfef95601890afd80709) > > Signed-off-by: C O Mmitter I can buy that, but then this makes it very clear that these footer lines are not shaped like RFC2822 headers, no? Thanks. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] tcsh-completion re-using git-completion.bash
On Thu, Nov 15, 2012 at 12:51 PM, Marc Khouzam wrote:
> The current tcsh-completion support for Git, as can be found on the
> Internet, takes the approach of defining the possible completions
> explicitly. This has the obvious draw-back to require constant
> updating as the Git code base evolves.
>
> The approach taken by this commit is to to re-use the advanced bash
> completion script and use its result for tcsh completion. This is
> achieved by executing (versus sourcing) the bash script and
> outputting the completion result for tcsh consumption.
>
> Three solutions were looked at to implement this approach with (A)
> being retained:
>
> A) Modifications:
> git-completion.bash and new git-completion.tcsh
As I said, I don't think this is needed. It can be done in a single
stand-alone script without modifications to git-completion.bash.
This works:
set called = ($_)
set script = "${called[2]}.tmp"
cat <<\EOF > $script
source "$HOME/.git-completion.sh"
# Set COMP_WORDS in a way that can be handled by the bash script.
COMP_WORDS=($1)
# Set COMP_CWORD to the cursor location as bash would.
if [ -n "${2-}" ]; then
COMP_CWORD=$2
else
# Assume the cursor is at the end of parameter #1.
# We must check for a space as the last character which will
# tell us that the previous word is complete and the cursor
# is on the next word.
if [ "${1: -1}" == " " ]; then
# The last character is a space, so our location is at the end
# of the command-line array
COMP_CWORD=${#COMP_WORDS[@]}
else
# The last character is not a space, so our location is on the
# last word of the command-line array, so we must decrement the
# count by 1
COMP_CWORD=$((${#COMP_WORDS[@]}-1))
fi
fi
# Call _git() or _gitk() of the bash script, based on the first
# element of the command-line
_${COMP_WORDS[0]}
IFS=$'\n'
echo "${COMPREPLY[*]}"
\EOF
complete git 'p/*/`bash ${script} "${COMMAND_LINE}" | sort | uniq`/'
complete gitk 'p/*/`bash ${script} "${COMMAND_LINE}" | sort | uniq`/'
--
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: `git mv` has ambiguous error message for non-existing target
Patrick Lehner writes: > To reproduce: > - cd into a git repo > - assuming "filea.txt" is an existing file in the CWD, and "dirb" is > neither a file nor a directory in the CWD, use the command "git mv > filea.txt dirb/filea.txt" > - this will produce an error message like `fatal: renaming 'filea.sh' > failed: No such file or directory` > > It does not mention that the problem is, in fact, the target directory > not existing. This seems to be mostly a problem for users unfamiliar > with bash/*nix console commands. Although it is documented that git mv > will not create intermediate folders (which is fine, because neither > does mv), the error message might lead to believe a problem exists > with the source file. $ rm -fr xxx $ >yyy $ mv yyy xxx/yyy mv: cannot move `yyy' to `xxx/yyy': No such file or directory It doesn't mention that the problem is with 'xxx' and not 'yyy' either. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Nov 2012, #03; Tue, 13)
On 11/15/2012 02:35 PM, Torsten Bögershausen wrote: On 11/15/2012 08:05 PM, Ramsay Jones wrote: Did the cygwin project not bump an api version number somewhere? ATB, Ramsay Jones Ramsay, you can run uname -r to see the version number. I myself haven't fully understood all the consequences, somewhere between 1.7.7 and 1.7.17 the include files had been changed. If this has consequences for using e.g. winsock2.dll, I want to know myself ;-) /Torsten uname -r gives the version of the dll, not of any particular package, and all of the various components are versioned independently. The win32api is spread across several packages, and the recent update changed the names, There is no api number advertised. You could check the names of currently installed packages if you assume those names won't change, but any such method is going to be fragile (and very unsupported). Mark -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 6/5] sequencer.c: refrain from adding duplicate s-o-b lines
Detect whether the s-o-b already exists in the commit footer and refrain
from adding a duplicate.
Update t3511 to test new behavior.
Signed-off-by: Brandon Casey
---
Hi Duy,
How about this patch on top of my series as a base for your patch to
unify the code paths that append signoff in format-patch, commit, and
sequencer?
-Brandon
sequencer.c | 28 ++--
t/t3511-cherry-pick-x.sh | 20 ++--
2 files changed, 36 insertions(+), 12 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index 7ad1163..546dacb 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -42,13 +42,15 @@ static int is_cherry_pick_from_line(const char *buf, int
len)
!prefixcmp(buf, cherry_picked_prefix);
}
-static int ends_rfc2822_footer(struct strbuf *sb, int ignore_footer)
+static int ends_rfc2822_footer(struct strbuf *sb, struct strbuf *sob,
+ int ignore_footer)
{
int hit = 0;
int i, k;
int len = sb->len - ignore_footer;
int last_was_rfc2822 = 0;
const char *buf = sb->buf;
+ int found_sob = 0;
for (i = len - 1; i > 0; i--) {
if (hit && buf[i] == '\n')
@@ -66,12 +68,15 @@ static int ends_rfc2822_footer(struct strbuf *sb, int
ignore_footer)
if (last_was_rfc2822 && (buf[i] == ' ' || buf[i] == '\t'))
continue;
+ if ((last_was_rfc2822 = is_rfc2822_line(buf+i, k-i)) &&
+ sob && !found_sob &&
+ !strncasecmp(buf+i, sob->buf, sob->len))
+ found_sob = 1;
- if (!((last_was_rfc2822 = is_rfc2822_line(buf+i, k-i)) ||
- is_cherry_pick_from_line(buf+i, k-i)))
+ if (!(last_was_rfc2822 || is_cherry_pick_from_line(buf+i, k-i)))
return 0;
}
- return 1;
+ return 1 + found_sob;
}
static void remove_sequencer_state(void)
@@ -547,7 +552,7 @@ static int do_pick_commit(struct commit *commit, struct
replay_opts *opts)
}
if (opts->record_origin) {
- if (!ends_rfc2822_footer(&msgbuf, 0))
+ if (!ends_rfc2822_footer(&msgbuf, NULL, 0))
strbuf_addch(&msgbuf, '\n');
strbuf_addstr(&msgbuf, cherry_picked_prefix);
strbuf_addstr(&msgbuf,
sha1_to_hex(commit->object.sha1));
@@ -1077,6 +1082,7 @@ int sequencer_pick_revisions(struct replay_opts *opts)
void append_signoff(struct strbuf *msgbuf, int ignore_footer)
{
struct strbuf sob = STRBUF_INIT;
+ int has_footer = 0;
int i;
strbuf_addstr(&sob, sign_off_header);
@@ -1085,10 +1091,12 @@ void append_signoff(struct strbuf *msgbuf, int
ignore_footer)
strbuf_addch(&sob, '\n');
for (i = msgbuf->len - 1 - ignore_footer; i > 0 && msgbuf->buf[i - 1]
!= '\n'; i--)
; /* do nothing */
- if (prefixcmp(msgbuf->buf + i, sob.buf)) {
- if (!i || !ends_rfc2822_footer(msgbuf, ignore_footer))
- strbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0,
"\n", 1);
- strbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0, sob.buf,
sob.len);
- }
+ if (!i || !(has_footer =
+ ends_rfc2822_footer(msgbuf, &sob, ignore_footer)))
+ strbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0,
+ "\n", 1);
+ if (has_footer != 2)
+ strbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0, sob.buf,
+ sob.len);
strbuf_release(&sob);
}
diff --git a/t/t3511-cherry-pick-x.sh b/t/t3511-cherry-pick-x.sh
index af7a87c..a15b199 100755
--- a/t/t3511-cherry-pick-x.sh
+++ b/t/t3511-cherry-pick-x.sh
@@ -11,9 +11,10 @@ pristine_detach () {
git clean -d -f -f -q -x
}
-non_rfc2822_mesg='base with footer
+non_rfc2822_mesg="base with footer
-Commit message body is here.'
+Commit message body is here.
+Not an s-o-b Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>"
rfc2822_mesg="$non_rfc2822_mesg
@@ -25,6 +26,9 @@ rfc2822_cherry_mesg="$rfc2822_mesg
(cherry picked from commit da39a3ee5e6b4b0d3255bfef95601890afd80709)
Tested-by: C.U. Thor "
+rfc2822_cherry_sob_mesg="$rfc2822_cherry_mesg
+Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>
+Signed-off-by: C.U. Thor "
test_expect_success setup '
git config advice.detachedhead false &&
@@ -36,6 +40,8 @@ test_expect_success setup '
test_commit "$rfc2822_mesg" foo b rfc2822-base &&
git reset --hard initial &&
test_commit "$rfc2822_cherry_mesg" foo b rfc2822-cherry-base &&
+ git reset --hard initial &&
+ test_commit "$rfc2822_cherry_sob_mesg" foo b rfc2822-cherry-sob-base &&
pristine_detach initial &&
test_commit conflicting unrelated
'
@@ -151,4 +157,14 @@ test_expect_success 'cher
Re: gitpacker progress report and a question
Max Horn : > > I'm still looking for a better name for it and would welcome suggestions. > > Isn't "gitar" the kind of natural choice? ;) At least for a stand-alone tool, > not for a git subcommand. I just renamed it git-weave. I keep talking about tarballs because I keep thinking about using it archeologically on projects that only exist as tarball sequences, but the tool actually oacks and unpacks *file tree* sequences. -- http://www.catb.org/~esr/";>Eric S. Raymond -- To unsubscribe from this list: send the line "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: gitpacker progress report and a question
On 15.11.2012, at 22:28, Eric S. Raymond wrote: > Some days ago I reported that I was attempting to write a tool that could > (a) take a git repo and unpack it into a tarball sequence plus a metadata log, > (b) reverse that operation, packing a tarball and log sequence into a repo. Ah, I could have used such a tool a year or so ago. Sounds useful to me, anyway :) > > Thanks in part to advice by Andreas Schwab and in part to looking at the > text of the p4 import script, this effort has succeeded. A proof of > concept is enclosed. It isn't documented yet, and has not been tested > on a repository with branches or merges in the history, but I am confident > that the distance from here to a finished and tested tool is short. > > The immediate intended use is for importing older projects that are > available only as sequences of release tarballs, but there are other > sorts of repository surgery that would become easier using it. > > I'm still looking for a better name for it and would welcome suggestions. Isn't "gitar" the kind of natural choice? ;) At least for a stand-alone tool, not for a git subcommand. Cheers, Max > > Before I do much further work, I need to determine how this will be shipped. > I see two possibilities: either I ship it as a small standalone project, > or it becomes a git subcommand shipped with the git suite. How I document > it and set up its tests would differ between these two cases. > > Is there a process for submitting new subcommands? What are the > test-suite and documentation requirements? > -- > http://www.catb.org/~esr/";>Eric S. Raymond > -- To unsubscribe from this list: send the line "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: What's cooking in git.git (Nov 2012, #04; Wed, 14)
Felipe Contreras writes: > On Wed, Nov 14, 2012 at 11:42 PM, Junio C Hamano wrote: > >> * fc/completion-test-simplification (2012-10-29) 2 commits >> - completion: simplify __gitcomp test helper >> - completion: refactor __gitcomp related tests >> >> Clean up completion tests. >> >> There were some comments on the list. >> >> Expecting a re-roll. > > This was already re-rolled > http://article.gmane.org/gmane.comp.version-control.git/209382 Thanks --- will take a look and replace but not today (I was spending most of my time on the 'master' integration front). -- To unsubscribe from this list: send the line "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] status: add advice on how to push/pull to tracking branch
Matthieu Moy writes: > I don't understand what you mean by "non-current". If you mean a local > branch not pointed to by HEAD, then I don't understand the remark, as > the message is shown by "git status" (looking more closely, it is also > shown by "git checkout", but after switching branch so also showing a > message about the current branch) and precisely talks about the current > branch. Ah, Ok, I somehow thought that "branch -v" would also use this information, and/or during my absense this function from remote.c got linked into "git remote show" ;-) So it is not an issue right now, but we will have to worry about the messaging when we start using this to describe a branch that is not currently checked out. -- To unsubscribe from this list: send the line "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] Unify appending signoff in format-patch, commit and sequencer
On Thu, Nov 15, 2012 at 4:32 AM, Nguyễn Thái Ngọc Duy wrote: > There are two implementations of append_signoff in log-tree.c and > sequencer.c, which do more or less the same thing. This patch > > - teaches sequencer.c's append_signoff() not to append the signoff if >it's already there but not at the bottom > > - removes log-tree.c's > > - make sure "Signed-off-by: foo" in the middle of a line is not >counted as a sign off > > Signed-off-by: Nguyễn Thái Ngọc Duy > --- > Interestingly this patch triggers the fault that it fixes. > I was surprised that there was no blank line before my S-o-b > and thought I broke something. It turns out I used unmodified > format-patch and it mistook the S-o-b quote as true S-o-b line. > The modified one puts the blank line back. Heh, yeah I noticed this bug yesterday when I submitted my changes affecting the same area of code (sequence.c). Glad I didn't waste too much time fixing it. Have you looked at this: http://thread.gmane.org/gmane.comp.version-control.git/209781 That series doesn't duplicate your work, but the two series's should be resolved. -Brandon -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
gitpacker progress report and a question
Some days ago I reported that I was attempting to write a tool that could
(a) take a git repo and unpack it into a tarball sequence plus a metadata log,
(b) reverse that operation, packing a tarball and log sequence into a repo.
Thanks in part to advice by Andreas Schwab and in part to looking at the
text of the p4 import script, this effort has succeeded. A proof of
concept is enclosed. It isn't documented yet, and has not been tested
on a repository with branches or merges in the history, but I am confident
that the distance from here to a finished and tested tool is short.
The immediate intended use is for importing older projects that are
available only as sequences of release tarballs, but there are other
sorts of repository surgery that would become easier using it.
I'm still looking for a better name for it and would welcome suggestions.
Before I do much further work, I need to determine how this will be shipped.
I see two possibilities: either I ship it as a small standalone project,
or it becomes a git subcommand shipped with the git suite. How I document
it and set up its tests would differ between these two cases.
Is there a process for submitting new subcommands? What are the
test-suite and documentation requirements?
--
http://www.catb.org/~esr/";>Eric S. Raymond
#!/usr/bin/env python
"""
gitpacker - assemble tree sequences into repository histories
Requires git and cpio.
"""
import sys, os, getopt, subprocess, time, tempfile
DEBUG_GENERAL = 1
DEBUG_PROGRESS = 2
DEBUG_COMMANDS = 3
class Fatal(Exception):
"Unrecoverable error."
def __init__(self, msg):
Exception.__init__(self)
self.msg = msg
class Baton:
"Ship progress indications to stdout."
def __init__(self, prompt, endmsg='done', enable=False):
self.prompt = prompt
self.endmsg = endmsg
self.countfmt = None
self.counter = 0
if enable:
self.stream = sys.stdout
else:
self.stream = None
self.count = 0
self.time = 0
def __enter__(self):
if self.stream:
self.stream.write(self.prompt + "...")
if os.isatty(self.stream.fileno()):
self.stream.write(" \010")
self.stream.flush()
self.count = 0
self.time = time.time()
return self
def startcounter(self, countfmt, initial=1):
self.countfmt = countfmt
self.counter = initial
def bumpcounter(self):
if self.stream is None:
return
if os.isatty(self.stream.fileno()):
if self.countfmt:
update = self.countfmt % self.counter
self.stream.write(update + ("\010" * len(update)))
self.stream.flush()
else:
self.twirl()
self.counter = self.counter + 1
def endcounter(self):
if self.stream:
w = len(self.countfmt % self.count)
self.stream.write((" " * w) + ("\010" * w))
self.stream.flush()
self.countfmt = None
def twirl(self, ch=None):
"One twirl of the baton."
if self.stream is None:
return
if os.isatty(self.stream.fileno()):
if ch:
self.stream.write(ch)
self.stream.flush()
return
else:
update = "-/|\\"[self.count % 4]
self.stream.write(update + ("\010" * len(update)))
self.stream.flush()
self.count = self.count + 1
def __exit__(self, extype, value_unused, traceback_unused):
if extype == KeyboardInterrupt:
self.endmsg = "interrupted"
if extype == Fatal:
self.endmsg = "aborted by error"
if self.stream:
self.stream.write("...(%2.2f sec) %s.\n" \
% (time.time() - self.time, self.endmsg))
return False
def do_or_die(dcmd, legend=""):
"Either execute a command or raise a fatal exception."
if legend:
legend = " " + legend
if verbose >= DEBUG_COMMANDS:
sys.stdout.write("executing '%s'%s\n" % (dcmd, legend))
try:
retcode = subprocess.call(dcmd, shell=True)
if retcode < 0:
raise Fatal("child was terminated by signal %d." % -retcode)
elif retcode != 0:
raise Fatal("child returned %d." % retcode)
except (OSError, IOError) as e:
raise Fatal("execution of %s%s failed: %s" % (dcmd, legend, e))
def capture_or_die(dcmd, legend=""):
"Either execute a command and capture its output or die."
if legend:
legend = " " + legend
if verbose >= DEBUG_COMMANDS:
sys.stdout.write("executing '%s'%s\n" % (dcmd, legend))
try:
return subprocess.check_output(dcmd, shell=True)
except subprocess.CalledProcessError as e:
if e.returncode < 0:
raise Fatal("child was terminated by signal %d." % -e.returnc
Re: [PATCH] status: add advice on how to push/pull to tracking branch
Junio C Hamano writes:
>> -if (!num_theirs)
>> +if (!num_theirs) {
>> strbuf_addf(sb,
>> Q_("Your branch is ahead of '%s' by %d commit.\n",
>> "Your branch is ahead of '%s' by %d commits.\n",
>> num_ours),
>> base, num_ours);
>> -else if (!num_ours)
>> +strbuf_addf(sb,
>> +_(" (use \"git push\" to publish your local
>> commits)\n"));
>> +} else if (!num_ours) {
>
> The message should make it clear that the two words in double quotes
> only hint what command is used to "publish your local commits" and
> not to be taken as literal "here is what you exactly would type",
> but I do not think that is what I would get from this if I were a
> total newbie who would need this advise.
>
> It is even more true given that this is showing an arbitrary, and
> more likely than not a non-current branch, especially with the
> recent move from "matching" to "simple" where a naive use of "git
> push" is to push the branch that is currently checked out and no
> other branches.
I don't understand what you mean by "non-current". If you mean a local
branch not pointed to by HEAD, then I don't understand the remark, as
the message is shown by "git status" (looking more closely, it is also
shown by "git checkout", but after switching branch so also showing a
message about the current branch) and precisely talks about the current
branch. If you mean that the upsteam branch has a name different from
the local one, then with "push.default=simple", argumentless "git push"
will fail and show a detailed explanation to the user, which I find
acceptable.
I can tweak the advice to show the full "git push" command with
push.default=matching/current, but first, I'd like to understand your
remark.
>> +strbuf_addf(sb,
>> +_(" (use \"git pull\" to update your local
>> branch)\n"));
>> +} else {
>
> Likewise, and the non-currentness of the branch being described is
> even worse in here, as unlike "git push" that can still be used to
> push a non-current branch, "git pull" is never to be used to update
> local branch that is not current, which means the advice must mention
> "git checkout" somewhere.
I understand this remark even less. We're showing a message about the
current branch and its upstream branch. In which case would "git pull"
not do the right thing?
--
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: What's cooking in git.git (Nov 2012, #03; Tue, 13)
On 11/15/2012 08:05 PM, Ramsay Jones wrote: Torsten Bögershausen wrote: * ml/cygwin-mingw-headers (2012-11-12) 1 commit - Update cygwin.c for new mingw-64 win32 api headers Make git work on newer cygwin. Will merge to 'next'. (Sorry for late answer, I managed to test the original patch minutes before Peff merged it to pu) (And thanks for maintaining git) Is everybody using cygwin happy with this? I am still on cygwin 1.5.22 and quite happy that this patch does not (seem) to cause any problems. ;-P I managed to compile on a fresh installed cygwin, but failed to compile under 1.7.7, see below. Is there a way we can achieve to compile git both under "old" and "new" cygwin 1.7 ? Or is this not worth the effort? Did the cygwin project not bump an api version number somewhere? ATB, Ramsay Jones Ramsay, you can run uname -r to see the version number. I myself haven't fully understood all the consequences, somewhere between 1.7.7 and 1.7.17 the include files had been changed. If this has consequences for using e.g. winsock2.dll, I want to know myself ;-) /Torsten -- To unsubscribe from this list: send the line "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 does not understand absolute Win'dos' path
Johannes Sixt wrote: > Am 11/14/2012 10:12, schrieb Martin Lichtin: >> Maven's release plugin prepares a call Git like in this example: >> >> cmd.exe /X /C "git commit --verbose -F >> C:\cygwin\tmp\maven-scm-915771020.commit pom.xml" >> >> Git doesn't seem to understand the -F argument and treats it like a >> relative path (relative to the repository root): >> >> $ cmd.exe /X /C "git commit --verbose -F C:\cygwin\tmp\commit pom.xml" >> fatal: could not read log file 'mytestdir/C:\cygwin\tmp\commit': No >> such file or directory > > According to the code, this should not happen if you are using msysgit. > For this reason, I guess you are using Cygwin git. Right? > > I don't know what Cygwin programs are supposed to do if they receive an > argument that looks like a Windows style absolute path. > > OTOH, it could be argued that Maven should not treat a Cygwin program like > a DOS program, and it should pass the path in the POSIXy form > /c/cygwin/tmp/commit or /tmp/commit. I would argue precisely this! :-D ATB, Ramsay Jones -- To unsubscribe from this list: send the line "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: [regression] Newer gits cannot clone any remote repos
Torsten Bögershausen wrote:
> On 13.11.12 19:55, Ramsay Jones wrote:
>> Douglas Mencken wrote:
>>> *Any* git clone fails with:
>>>
>>> fatal: premature end of pack file, 106 bytes missing
>>> fatal: index-pack failed
>>>
>>> At first, I tried 1.8.0, and it failed. Then I tried to build 1.7.10.5
>>> then, and it worked. Then I tried 1.7.12.2, but it fails the same way
>>> as 1.8.0.
>>> So I decided to git bisect.
>>>
>>> b8a2486f1524947f232f657e9f2ebf44e3e7a243 is the first bad commit
>>> ``index-pack: support multithreaded delta resolving''
>>
>> This looks like the same problem I had on cygwin, which lead to
>> commit c0f86547c ("index-pack: Disable threading on cygwin", 26-06-2012).
>>
>> I didn't notice which platform you are on, but maybe you also have a
>> thread-unsafe pread()? Could you try re-building git with the
>> NO_THREAD_SAFE_PREAD build variable set?
>>
>> HTH.
>>
>> ATB,
>> Ramsay Jones
>
> This is interesting.
> I had the same problem on a PowerPC
> (Old PowerBook G4 running Linux).
>
> Using NO_THREAD_SAFE_PREAD helped, thanks for the hint.
> (After recompiling without NO_THREAD_SAFE_PREAD I could clone
> from this machine again, so the problem is not really reproducable)
Yes, the failures would be intermittent (and often not easily
reproducible). The threaded index-pack code did not fail for
me on cygwin at all during development, including tests, but failed
immediately I installed v1.7.11. On real repositories, it failed
intermittently. On some repos it always failed, on some it never
failed and on some others it would sometimes fail, sometimes not.
ATB,
Ramsay Jones
--
To unsubscribe from this list: send the line "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: What's cooking in git.git (Nov 2012, #03; Tue, 13)
Torsten Bögershausen wrote: >> * ml/cygwin-mingw-headers (2012-11-12) 1 commit >> - Update cygwin.c for new mingw-64 win32 api headers >> >> Make git work on newer cygwin. >> >> Will merge to 'next'. > > (Sorry for late answer, I managed to test the original patch minutes before > Peff merged it to pu) > (And thanks for maintaining git) > > Is everybody using cygwin happy with this? I am still on cygwin 1.5.22 and quite happy that this patch does not (seem) to cause any problems. ;-P > I managed to compile on a fresh installed cygwin, > but failed to compile under 1.7.7, see below. > Is there a way we can achieve to compile git both under "old" and "new" > cygwin 1.7 ? > Or is this not worth the effort? Did the cygwin project not bump an api version number somewhere? ATB, Ramsay Jones -- To unsubscribe from this list: send the line "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: [regression] Newer gits cannot clone any remote repos
Douglas Mencken wrote:
>> Could you try re-building git with the
>> NO_THREAD_SAFE_PREAD build variable set?
>
> Yeah! It works!!!
>
> --- evil/Makefile
> +++ good/Makefile
> @@ -957,6 +957,7 @@
> HAVE_PATHS_H = YesPlease
> LIBC_CONTAINS_LIBINTL = YesPlease
> HAVE_DEV_TTY = YesPlease
> + NO_THREAD_SAFE_PREAD = YesPlease
> endif
> ifeq ($(uname_S),GNU/kFreeBSD)
> NO_STRLCPY = YesPlease
>
> With this, I do have correctly working git clone.
OK, good.
You didn't mention which platform you are on; from the above Makefile
hunk, however, I can deduce that you are on *some* version of Linux.
Hmm, it doesn't seem too likely that your pread() is thread-unsafe
(possible, just unlikely), so it could be a more fundamental problem
with the threaded index-pack code (ie commit b8a2486f1 et. seq.).
However, as a first step could you try running the test program
(given below) on your system to determine if your pread() is thread-safe
or not. (gcc -I. -o test-pread test-pread.c; ./test-pread)
Also, what is the output of "uname -a".
ATB,
Ramsay Jones
-- >8 --
#include "git-compat-util.h"
#include "thread-utils.h"
#define DATA_FILE "junk.data"
#define MAX_DATA 256 * 1024
#define NUM_THREADS 3
#define TRIALS 50
struct thread_data {
pthread_t t;
int fd;
int cnt;
int fails;
unsigned long n;
};
static struct thread_data t[NUM_THREADS+1];
int create_data_file(void)
{
int i, fd = open(DATA_FILE, O_CREAT | O_TRUNC | O_WRONLY, 0600);
if (fd < 0)
return -1;
for (i = 0; i < MAX_DATA; i++)
if (write(fd, &i, sizeof(int)) < 0) {
close(fd);
unlink(DATA_FILE);
return -1;
}
close(fd);
return 0;
}
void *read_thread(void *data)
{
struct thread_data *d = (struct thread_data *)data;
int i, j, rd;
for (i = 0; i < TRIALS; i += MAX_DATA) {
for (j = 0; j < MAX_DATA; j++) {
ssize_t sz = read(d->fd, &rd, sizeof(int));
if (sz < 0 || rd != j)
d->fails++;
d->cnt++;
}
lseek(d->fd, 0, SEEK_SET);
}
return NULL;
}
void *pread_thread(void *data)
{
struct thread_data *d = (struct thread_data *)data;
int i, j, rd;
for (i = 0; i < TRIALS; i++) {
ssize_t sz;
d->n = d->n * 1103515245 + 12345;
j = d->n % MAX_DATA;
sz = pread(d->fd, &rd, sizeof(int), j * sizeof(int));
if (sz < 0 || rd != j)
d->fails++;
d->cnt++;
}
return NULL;
}
int main(int argc, char *argv[])
{
int fd, i;
if (create_data_file() < 0) {
printf("can't create data file\n");
return 1;
}
if ((fd = open(DATA_FILE, O_RDONLY)) < 0) {
printf("can't open data file\n");
unlink(DATA_FILE);
return 1;
}
for (i = 0; i < NUM_THREADS+1; i++) {
int ret;
t[i].fd = fd;
t[i].cnt = 0;
t[i].fails = 0;
t[i].n = i * 16381;
ret = pthread_create(&t[i].t, NULL,
(i == 0) ? read_thread : pread_thread,
&t[i]);
if (ret) {
printf("can't create thread %d (%s)\n", i,
strerror(ret));
unlink(DATA_FILE);
return 1;
}
}
for (i = 0; i < NUM_THREADS+1; i++)
pthread_join(t[i].t, NULL);
close(fd);
for (i = 0; i < NUM_THREADS+1; i++)
printf("%2d: trials %d, failed %d\n", i, t[i].cnt, t[i].fails);
unlink(DATA_FILE);
return 0;
}
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
`git mv` has ambiguous error message for non-existing target
Hey guys, as was brought up on #github today, the "git mv" command has a bit of a little-helping output message when the target directory (or any intermediate directories) dont exist. To reproduce: - cd into a git repo - assuming "filea.txt" is an existing file in the CWD, and "dirb" is neither a file nor a directory in the CWD, use the command "git mv filea.txt dirb/filea.txt" - this will produce an error message like `fatal: renaming 'filea.sh' failed: No such file or directory` It does not mention that the problem is, in fact, the target directory not existing. This seems to be mostly a problem for users unfamiliar with bash/*nix console commands. Although it is documented that git mv will not create intermediate folders (which is fine, because neither does mv), the error message might lead to believe a problem exists with the source file. Expanding the error message to "No such file or directory: 'dirb/' " would probably clear this up. Best regards and thanks to anyone who could improve this, Patrick -- To unsubscribe from this list: send the line "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] config: don't segfault when given --path with a missing value
On Thu, Nov 15, 2012 at 10:10:01AM -0800, Carlos Martín Nieto wrote: > When given a variable without a value, such as '[section] var' and > asking git-config to treat it as a path, git_config_pathname returns > an error and doesn't modify its output parameter. show_config assumes > that the call is always successful and sets a variable to indicate > that vptr should be freed. In case of an error however, trying to do > this will cause the program to be killed, as it's pointing to memory > in the stack. > > Detect the error and return immediately to avoid freeing or accessing > the uninitialed memory in the stack. > > Signed-off-by: Carlos Martín Nieto Acked-by: Jeff King > Yeah, that's more sensible. I didn't notice that the buffer never gets > written to in this codepath, and the trying to print it out is silly > when we know that there is nothing valid to print. > Thanks for the review. I've included your test as well, which really > makes all of this your code. Eh, I guess so. You did the hard part of finding it, though. ;) > Do we have some equivalent of a Basically-writen-by line? Nothing structured. But I am comfortable enough with the number of times I am mentioned in "git log" already, so don't worry about it. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] config: don't segfault when given --path with a missing value
When given a variable without a value, such as '[section] var' and
asking git-config to treat it as a path, git_config_pathname returns
an error and doesn't modify its output parameter. show_config assumes
that the call is always successful and sets a variable to indicate
that vptr should be freed. In case of an error however, trying to do
this will cause the program to be killed, as it's pointing to memory
in the stack.
Detect the error and return immediately to avoid freeing or accessing
the uninitialed memory in the stack.
Signed-off-by: Carlos Martín Nieto
---
On Thu, Nov 15, 2012 at 08:11:50AM -0800, Jeff King wrote:
> Hmm, actually, we should probably propagate the error (I was thinking
> for some reason this was in the listing code, but it is really about
> getting a specific variable, and that variable does not have a sane
> format. We'll already have printed the non-bool error, so we should
> probably die. So more like:
>
> if (git_config_pathname(&vptr, key_, value_) < 0)
> return -1;
> must_free_vptr = 1;
Yeah, that's more sensible. I didn't notice that the buffer never gets
written to in this codepath, and the trying to print it out is silly
when we know that there is nothing valid to print. Thanks for the
review. I've included your test as well, which really makes all of
this your code. Do we have some equivalent of a Basically-writen-by
line?
builtin/config.c | 3 ++-
t/t1300-repo-config.sh | 5 +
2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/builtin/config.c b/builtin/config.c
index 442ccc2..4dc5ffa 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -129,7 +129,8 @@ static int show_config(const char *key_, const char
*value_, void *cb)
else
sprintf(value, "%d", v);
} else if (types == TYPE_PATH) {
- git_config_pathname(&vptr, key_, value_);
+ if (git_config_pathname(&vptr, key_, value_) < 0)
+ return -1;
must_free_vptr = 1;
} else if (value_) {
vptr = value_;
diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index a477453..17272e0 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -803,6 +803,11 @@ test_expect_success NOT_MINGW 'get --path copes with unset
$HOME' '
test_cmp expect result
'
+test_expect_success 'get --path barfs on boolean variable' '
+ echo "[path]bool" >.git/config &&
+ test_must_fail git config --get --path path.bool
+'
+
cat > expect << EOF
[quote]
leading = " test"
--
1.8.0.316.g291341c
--
To unsubscribe from this list: send the line "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 4/5] sequencer.c: recognize "(cherry picked from ..." as part of s-o-b footer
Currently, if the s-o-b footer of a commit message contains a
"(cherry picked from ..." line that was added by a previous cherry-pick -x,
it is not recognized as a s-o-b footer and will cause a newline to be
inserted before an additional s-o-b is added.
So, rework ends_rfc2822_footer to recognize the "(cherry picked from ..."
string as part of the footer. Plus mark the test in t3511 as fixed.
Signed-off-by: Brandon Casey
---
Declare cherry_picked_prefix variable as static. This is the only change
with respect to v1.
-Brandon
sequencer.c | 44 +---
t/t3511-cherry-pick-x.sh | 2 +-
2 files changed, 30 insertions(+), 16 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index 01edec2..213fa4f 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -18,6 +18,7 @@
#define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION"
const char sign_off_header[] = "Signed-off-by: ";
+static const char cherry_picked_prefix[] = "(cherry picked from commit ";
static void remove_sequencer_state(void)
{
@@ -492,7 +493,7 @@ static int do_pick_commit(struct commit *commit, struct
replay_opts *opts)
}
if (opts->record_origin) {
- strbuf_addstr(&msgbuf, "(cherry picked from commit ");
+ strbuf_addstr(&msgbuf, cherry_picked_prefix);
strbuf_addstr(&msgbuf,
sha1_to_hex(commit->object.sha1));
strbuf_addstr(&msgbuf, ")\n");
}
@@ -1017,13 +1018,34 @@ int sequencer_pick_revisions(struct replay_opts *opts)
return pick_commits(todo_list, opts);
}
+static int is_rfc2822_line(const char *buf, int len)
+{
+ int i;
+
+ for (i = 0; i < len; i++) {
+ int ch = buf[i];
+ if (ch == ':')
+ break;
+ if (isalnum(ch) || (ch == '-'))
+ continue;
+ return 0;
+ }
+
+ return 1;
+}
+
+static int is_cherry_pick_from_line(const char *buf, int len)
+{
+ return (strlen(cherry_picked_prefix) + 41) <= len &&
+ !prefixcmp(buf, cherry_picked_prefix);
+}
+
static int ends_rfc2822_footer(struct strbuf *sb, int ignore_footer)
{
- int ch;
int hit = 0;
- int i, j, k;
+ int i, k;
int len = sb->len - ignore_footer;
- int first = 1;
+ int last_was_rfc2822 = 0;
const char *buf = sb->buf;
for (i = len - 1; i > 0; i--) {
@@ -1040,20 +1062,12 @@ static int ends_rfc2822_footer(struct strbuf *sb, int
ignore_footer)
; /* do nothing */
k++;
- if ((buf[i] == ' ' || buf[i] == '\t') && !first)
+ if (last_was_rfc2822 && (buf[i] == ' ' || buf[i] == '\t'))
continue;
- first = 0;
-
- for (j = 0; i + j < len; j++) {
- ch = buf[i + j];
- if (ch == ':')
- break;
- if (isalnum(ch) ||
- (ch == '-'))
- continue;
+ if (!((last_was_rfc2822 = is_rfc2822_line(buf+i, k-i)) ||
+ is_cherry_pick_from_line(buf+i, k-i)))
return 0;
- }
}
return 1;
}
diff --git a/t/t3511-cherry-pick-x.sh b/t/t3511-cherry-pick-x.sh
index b2098e0..785486e 100755
--- a/t/t3511-cherry-pick-x.sh
+++ b/t/t3511-cherry-pick-x.sh
@@ -63,7 +63,7 @@ test_expect_success 'cherry-pick -s not confused by rfc2822
continuation line' '
test_cmp expect actual
'
-test_expect_failure 'cherry-pick treats -s "(cherry picked from..." line as
part of footer' '
+test_expect_success 'cherry-pick treats -s "(cherry picked from..." line as
part of footer' '
pristine_detach initial &&
git cherry-pick -s rfc2822-cherry-base &&
cat <<-EOF >expect &&
--
1.8.0
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] status: add advice on how to push/pull to tracking branch
Matthieu Moy writes:
> Signed-off-by: Matthieu Moy
> ---
> I thought this was obvious enough not to deserve an advice, but a
> colleague of mine had troubles with "commited but not pushed" changes.
> Maybe an additional advice would have helped. After all, it's an
> advice, and can be deactivated ...
>
> remote.c | 13 ++---
> t/t2020-checkout-detach.sh | 1 +
> 2 files changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/remote.c b/remote.c
> index 04fd9ea..9c19689 100644
> --- a/remote.c
> +++ b/remote.c
> @@ -1627,13 +1627,15 @@ int format_tracking_info(struct branch *branch,
> struct strbuf *sb)
>
> base = branch->merge[0]->dst;
> base = shorten_unambiguous_ref(base, 0);
> - if (!num_theirs)
> + if (!num_theirs) {
> strbuf_addf(sb,
> Q_("Your branch is ahead of '%s' by %d commit.\n",
> "Your branch is ahead of '%s' by %d commits.\n",
> num_ours),
> base, num_ours);
> - else if (!num_ours)
> + strbuf_addf(sb,
> + _(" (use \"git push\" to publish your local
> commits)\n"));
> + } else if (!num_ours) {
The message should make it clear that the two words in double quotes
only hint what command is used to "publish your local commits" and
not to be taken as literal "here is what you exactly would type",
but I do not think that is what I would get from this if I were a
total newbie who would need this advise.
It is even more true given that this is showing an arbitrary, and
more likely than not a non-current branch, especially with the
recent move from "matching" to "simple" where a naive use of "git
push" is to push the branch that is currently checked out and no
other branches.
see 'git push --help' to learn how to publish your local commits
might be more appropriate.
> + strbuf_addf(sb,
> + _(" (use \"git pull\" to update your local
> branch)\n"));
> + } else {
Likewise, and the non-currentness of the branch being described is
even worse in here, as unlike "git push" that can still be used to
push a non-current branch, "git pull" is never to be used to update
local branch that is not current, which means the advice must mention
"git checkout" somewhere.
--
To unsubscribe from this list: send the line "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 8/8] send-email: do not prompt for explicit repo ident
On Thu, Nov 15, 2012 at 08:56:37AM -0800, Junio C Hamano wrote: > > I think a much more compelling argument/commit message for your > > suggested patch would be: > > > > We currently prompt the user for the "From" address. This is an > > inconvenience in the common case that the user has configured their > > identity in the environment, but is meant as a safety check for when > > git falls back to an implicitly generated identity (which may or may > > not be valid). > > > > That safety check is not really necessary, though, as by default > > send-email will prompt the user for a final confirmation before > > sending out any message. The likelihood that a user has both bothered > > to turn off this default _and_ not configured any identity (nor > > checked that the automatic identity is valid) is rather low. > > This somehow reminds me of the first paragraph of f20f387 (commit: > check committer identity more strictly, 2012-07-23). > > I never use "send-email driving format-patch" workflow myself, but I > suspect there are people among who do so who are using --compose to > do the cover letter of their series. Does the "confirmation as the > last step" help them, or would they have to retype their message? That is a good question. That confirmation step does come after they have typed their cover letter. However, if they are using --compose, they are dumped in their editor with something like: From Jeff King # This line is ignored. GIT: Lines beginning in "GIT:" will be removed. GIT: Consider including an overall diffstat or table of contents GIT: for the patch you are writing. GIT: GIT: Clear the body content if you don't wish to send a summary. From: Jeff King Subject: In-Reply-To: which I think would count as sufficient notice of the address being used. -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] wildmatch: correct isprint and isspace
Am 15.11.2012 13:19, schrieb Nguyễn Thái Ngọc Duy: > On Thu, Nov 15, 2012 at 2:30 AM, René Scharfe > wrote: > > Nevertheless, it's unfortunate that we have an isspace() that *almost* does > > what the widely known thing of the same name does. I'd shy away from > > changing git's version directly, because it's used more than a hundred > times > > in the code, and estimating the impact of adding \v and \f to it. > > Perhaps renaming it to isgitspace() is a good first step, followed by > > adding a "standard" version of isspace() for wildmatch? > > There are just too many call sites of isspace() and there is a risk > of new call sites coming in independently. So I think keeping isspace() > as-is and using a different name for the standard version is probably > a better choice. After having a closer look, where wildmatch is actually used -- matching filenames -- and I've not yet seen \v or \f in a filename, it's possibly unnecessary to do anything about isspace() right now. (It's probably more an issue that filenames can be localized, and we only support unlocalized character classes.) > diff --git a/git-compat-util.h b/git-compat-util.h > index 02f48f6..d4c3fda 100644 > --- a/git-compat-util.h > +++ b/git-compat-util.h > @@ -486,6 +486,7 @@ extern const unsigned char sane_ctype[256]; > #define sane_istest(x,mask) ((sane_ctype[(unsigned char)(x)] & (mask)) != 0) > #define isascii(x) (((x) & ~0x7f) == 0) > #define isspace(x) sane_istest(x,GIT_SPACE) > +#define isspace_posix(x) (((x) >= 9 && (x) <= 13) || (x) == 32) > #define isdigit(x) sane_istest(x,GIT_DIGIT) > #define isalpha(x) sane_istest(x,GIT_ALPHA) > #define isalnum(x) sane_istest(x,GIT_ALPHA | GIT_DIGIT) > @@ -499,7 +500,8 @@ extern const unsigned char sane_ctype[256]; > #define isxdigit(x) (hexval_table[x] != -1) This was from a previous patch, but maybe: "hexval_table[(unsigned char)x]" > #define isprint(x) (sane_istest(x, GIT_ALPHA | GIT_DIGIT | GIT_SPACE | \ > GIT_PUNCT | GIT_REGEX_SPECIAL | GIT_GLOB_SPECIAL | \ > - GIT_PATHSPEC_MAGIC)) > + GIT_PATHSPEC_MAGIC) && \ > + (x) >= 32) May I suggest the current is_print() implementation in master: #define isprint(x) ((x) >= 0x20 && (x) <= 0x7e) To summarize my opinion: I no longer see a reason to correct isspace() (unless somebody with an actual use case complains), and a more POSIXly isprint() is already in master. => Nothing to do. :) Regards Jan -- To unsubscribe from this list: send the line "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: creation of empty branches
On Wed, Nov 14, 2012 at 01:27:33PM -0800, Junio C Hamano wrote:
> > Instead of
> > fatal: Not a valid object name: 'master'.
> > perhaps
> > fatal: Cannot create branch 'foo' from empty branch 'master'. To
> > rename 'master' use 'git branch -m master foo'.
>
> The first new sentence is a definite improvement, but I do not think
> the advice in the second sentence is necessarily a good idea,
> because it is dubious that the user is likely to have wanted to
> rename 'master' to something else. "git branch foo master" (or its
> moral equivalent "git checkout -b foo" while on master) is a wish to
> have a history that ends in 'foo' *forked* from history of 'master',
> but because you do not even have anything on 'master' yet, you
> cannot fork the history, as you explained earlier (snipped). In
> that sense, 'empty branch' is a slight misnomer---as far as "git
> branch foo master" is concerned, the 'master' branch does not yet
> exist (and that is why we often call it an "unborn branch", not
> "empty").
>
> fatal: cannot fork master's history that does not exist yet.
>
> would be more accurate description of the situation.
I agree with most of your reasoning. I think simply using Andrew's first
sentence is a little more clear to a new user, even though it may be
less technically precise, but I don't have a strong opinion.
That still leaves "checkout -b". I do not see it as a problem that "git
branch foo" does not work whereas "git checkout -b foo" does; we
special-case the latter because it can do something sensible, whereas
there is nothing sensible for "git branch foo" to do. However, I think
it is missing one piece:
-- >8 --
Subject: checkout: print a message when switching unborn branches
When we switch to a new branch using checkout, we usually output a
message indicating what happened. However, when we switch from an unborn
branch to a new branch, we do not print anything, which may leave the
user wondering what happened.
The reason is that the unborn branch is a special case (see abe1998),
and does not follow the usual switch_branches code path. Let's add a
similar informational message to the special case to match the usual
code path.
Signed-off-by: Jeff King
---
Two possible tweaks:
1. The message is the same as "git checkout -b" when we are actually
moving to a new branch. We could optionally mention that the branch
is empty or unborn.
2. We do not check whether the old unborn branch has the same name as
the new one. So you get "Switched to a new branch..." if you try to
checkout the same branch. We'd have to pass more information into
the special case to detect this. I don't know if we care.
builtin/checkout.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 781295b..a9c1b5a 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -951,6 +951,9 @@ static int switch_unborn_to_new_branch(const struct
checkout_opts *opts)
strbuf_addf(&branch_ref, "refs/heads/%s", opts->new_branch);
status = create_symref("HEAD", branch_ref.buf, "checkout -b");
strbuf_release(&branch_ref);
+ if (!opts->quiet)
+ fprintf(stderr, _("Switched to a new branch '%s'\n"),
+ opts->new_branch);
return status;
}
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 8/8] send-email: do not prompt for explicit repo ident
Jeff King writes: > I think a much more compelling argument/commit message for your > suggested patch would be: > > We currently prompt the user for the "From" address. This is an > inconvenience in the common case that the user has configured their > identity in the environment, but is meant as a safety check for when > git falls back to an implicitly generated identity (which may or may > not be valid). > > That safety check is not really necessary, though, as by default > send-email will prompt the user for a final confirmation before > sending out any message. The likelihood that a user has both bothered > to turn off this default _and_ not configured any identity (nor > checked that the automatic identity is valid) is rather low. This somehow reminds me of the first paragraph of f20f387 (commit: check committer identity more strictly, 2012-07-23). I never use "send-email driving format-patch" workflow myself, but I suspect there are people among who do so who are using --compose to do the cover letter of their series. Does the "confirmation as the last step" help them, or would they have to retype their 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: [PATCH v3 0/5] push: update remote tags only with force
Angelo Borsotti writes: >> I am *not* convinced that the "refs/tags/ is the only special >> hierarchy whose contents should not move" is a bad limitation we >> should avoid, but if it indeed is a bad limitation, the above is one >> possible way to think about avoiding it. > > What other hierarchy besides branches and tags is there? Do you have > in mind some other that should not move? People use their own hierarchies for various purposes that are not pre-defined by git-core, e.g. refs/changes/, refs/pull/, etc. Depending on the semantics the projects want out of these hierarchies, some of them may well be considered "create-only". -- To unsubscribe from this list: send the line "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 1/8] test-lib: allow negation of prerequisites
Jeff King wrote: > Yes. You can test it yourself with "bash t-basic.sh". The reason is > that the "!" is part of history expansion, which is only enabled by > default for interactive shells. Nice to hear. Thanks much for looking into it. > On Wed, Nov 14, 2012 at 11:46:58PM -0800, Jonathan Nieder wrote: >> If it works everywhere, this patch would help me conquer my fear of >> exclamation points in git's tests, which would be a comfort to me and >> a very good thing. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 1/8] test-lib: allow negation of prerequisites
On Wed, Nov 14, 2012 at 11:46:58PM -0800, Jonathan Nieder wrote: > > +test_expect_success !LAZY_TRUE 'missing lazy prereqs skip tests' ' > > I have a visceral nervousness when reading this code, from too much > unpleasant experience of bash's csh-style !history expansion. Luckily > bash does not treat ! specially in the '-o sh' mode used by tests. I can understand, having been bitten by it many times myself (not only is it counter-intuitively expanded inside double quotes, and not only does it not do what you wanted, but it does not insert the command into the history, so you cannot even go back to fix it easily). > Does this feature work when running a test explicitly using > "bash "? That's something I do from time to time to > figure out whether a weird behavior is shell-specific. Yes. You can test it yourself with "bash t-basic.sh". The reason is that the "!" is part of history expansion, which is only enabled by default for interactive shells. > If it works everywhere, this patch would help me conquer my fear of > exclamation points in git's tests, which would be a comfort to me and > a very good thing. As far as I know, "!" should be safe in a non-interactive shell script. -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 v4 0/4] Introduce diff.submodule
On Thu, Nov 15, 2012 at 08:25:26AM -0800, Jeff King wrote: > > Thanks, this version looks good to me. > > Oh wait. I did not look closely enough. The point was to move the option > parser _out_ of git_diff_ui_config into git_diff_basic_config, so that > it only triggers for porcelain, not plumbing. Oh no, I am having a muddled morning. It _is_ right. We want to move it out of basic into ui. Sorry for the noise. I just woke up and am thinking backwards. It may be worth squashing this test into patch 3: diff --git a/t/t4041-diff-submodule-option.sh b/t/t4041-diff-submodule-option.sh index e401814..023439f 100755 --- a/t/t4041-diff-submodule-option.sh +++ b/t/t4041-diff-submodule-option.sh @@ -72,6 +72,21 @@ EOF test_cmp expected actual " +test_expect_success 'diff.submodule does not affect plumbing' ' + test_config diff.submodule log && + git diff-index -p HEAD >actual && + cat >expected <<-EOF && + diff --git a/sm1 b/sm1 + new file mode 16 + index 000..a2c4dab + --- /dev/null + +++ b/sm1 + @@ -0,0 +1 @@ + +Subproject commit $fullhead1 + EOF + test_cmp expected actual +' + commit_file sm1 && head2=$(add_file sm1 foo3) BTW, while writing the test, I noticed two minor nits with your tests: 1. They can use test_config, which is simpler (you do not need to unset yourself after the test) and safer (the unset happens via test_when_finished, so it works even if the test fails). 2. You can still indent expected output when using <<-. I don't know if it is worth re-rolling for them. -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 v4 0/4] Introduce diff.submodule
On Thu, Nov 15, 2012 at 08:23:34AM -0800, Jeff King wrote: > On Tue, Nov 13, 2012 at 09:12:43PM +0530, Ramkumar Ramachandra wrote: > > > v1 is here: > > http://mid.gmane.org/1349196670-2844-1-git-send-email-artag...@gmail.com > > v2 is here: > > http://mid.gmane.org/1351766630-4837-1-git-send-email-artag...@gmail.com > > v3 is here: > > http://mid.gmane.org/1352653146-3932-1-git-send-email-artag...@gmail.com > > > > This version was prepared in response to Peff's review of v3. > > What changed: > > * Functional code simplified and moved to git_diff_ui_config. > > * Peff contributed one additional patch to the series. > > Thanks, this version looks good to me. Oh wait. I did not look closely enough. The point was to move the option parser _out_ of git_diff_ui_config into git_diff_basic_config, so that it only triggers for porcelain, not plumbing. -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 v4 0/4] Introduce diff.submodule
On Tue, Nov 13, 2012 at 09:12:43PM +0530, Ramkumar Ramachandra wrote: > v1 is here: > http://mid.gmane.org/1349196670-2844-1-git-send-email-artag...@gmail.com > v2 is here: > http://mid.gmane.org/1351766630-4837-1-git-send-email-artag...@gmail.com > v3 is here: > http://mid.gmane.org/1352653146-3932-1-git-send-email-artag...@gmail.com > > This version was prepared in response to Peff's review of v3. > What changed: > * Functional code simplified and moved to git_diff_ui_config. > * Peff contributed one additional patch to the series. Thanks, this version looks good to me. -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] config: don't segfault when given --path with a missing value
On Thu, Nov 15, 2012 at 08:11:50AM -0800, Jeff King wrote: > Hmm, actually, we should probably propagate the error (I was thinking > for some reason this was in the listing code, but it is really about > getting a specific variable, and that variable does not have a sane > format. We'll already have printed the non-bool error, so we should > probably die. So more like: > > if (git_config_pathname(&vptr, key_, value_) < 0) > return -1; > must_free_vptr = 1; You may want to squash in this test, which triggers your original problem, but also demonstrates the use of uninitialized memory (although you need to run under valgrind or similar to reliably notice it). diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh index e127f35..7c4c372 100755 --- a/t/t1300-repo-config.sh +++ b/t/t1300-repo-config.sh @@ -803,6 +803,11 @@ test_expect_success NOT_MINGW 'get --path copes with unset $HOME' ' test_cmp expect result ' +test_expect_success 'get --path barfs on boolean variable' ' + echo "[path]bool" >.git/config && + test_must_fail git config --get --path path.bool +' + cat > expect << EOF [quote] leading = " test" -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] config: don't segfault when given --path with a missing value
On Thu, Nov 15, 2012 at 08:08:49AM -0800, Jeff King wrote:
> That is definitely the right thing to do. But do we also need to take
> note of the error for later? After this code:
>
> > } else if (types == TYPE_PATH) {
> > - git_config_pathname(&vptr, key_, value_);
> > - must_free_vptr = 1;
> > + must_free_vptr = !git_config_pathname(&vptr, key_, value_);
>
> We don't have any clue that nothing got written into vptr. Which means
> it still points at the stack buffer "value", which contains
> uninitialized bytes. We will later try to print it, thinking it has the
> expanded path in it.
>
> Do we need something like:
>
> if (!git_config_pathname(&vptr, key_, value_))
> must_free_vptr = 1;
> else
> vptr = "";
Hmm, actually, we should probably propagate the error (I was thinking
for some reason this was in the listing code, but it is really about
getting a specific variable, and that variable does not have a sane
format. We'll already have printed the non-bool error, so we should
probably die. So more like:
if (git_config_pathname(&vptr, key_, value_) < 0)
return -1;
must_free_vptr = 1;
-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] config: don't segfault when given --path with a missing value
On Tue, Nov 13, 2012 at 08:50:04PM -0800, Carlos Martín Nieto wrote:
> When given a variable without a value, such as '[section] var' and
> asking git-config to treat it as a path, git_config_pathname returns
> an error and doesn't modify its output parameter. show_config assumes
> that the call is always successful and sets a variable to indicate
> that vptr should be freed. In case of an error however, trying to do
> this will cause the program to be killed, as it's pointing to memory
> in the stack.
Whoops.
> Set the must_free_vptr flag depending on the return value of
> git_config_pathname so it's accurate.
That is definitely the right thing to do. But do we also need to take
note of the error for later? After this code:
> } else if (types == TYPE_PATH) {
> - git_config_pathname(&vptr, key_, value_);
> - must_free_vptr = 1;
> + must_free_vptr = !git_config_pathname(&vptr, key_, value_);
We don't have any clue that nothing got written into vptr. Which means
it still points at the stack buffer "value", which contains
uninitialized bytes. We will later try to print it, thinking it has the
expanded path in it.
Do we need something like:
if (!git_config_pathname(&vptr, key_, value_))
must_free_vptr = 1;
else
vptr = "";
?
-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: [regression] Newer gits cannot clone any remote repos
On Wed, Nov 14, 2012 at 2:55 AM, Douglas Mencken wrote: >> Could you try re-building git with the >> NO_THREAD_SAFE_PREAD build variable set? > > Yeah! It works!!! > > --- evil/Makefile > +++ good/Makefile > @@ -957,6 +957,7 @@ > HAVE_PATHS_H = YesPlease > LIBC_CONTAINS_LIBINTL = YesPlease > HAVE_DEV_TTY = YesPlease > + NO_THREAD_SAFE_PREAD = YesPlease > endif > ifeq ($(uname_S),GNU/kFreeBSD) > NO_STRLCPY = YesPlease > > With this, I do have correctly working git clone. Sorry you had to figure that out the hard way. Could you make it a proper patch? I'm surprised that Linux pread does not behave the same way across platforms though. Or maybe it only happens with certain Linux versions. What version are you using? -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: use cases for git namespaces
On Thu, Nov 15, 2012 at 2:03 PM, Sitaram Chamarty wrote: > Hi, > > It seems to me that whatever namespaces can do, can functionally be > done using just a subdirectory of branches. The only real > differences I can see are (a) a client sees less branch clutter, and > (b) a fetch/clone pulls down less if the big stuff is in another > namespace. > > I would like to understand what other uses/reasons were thought of. (I should mention that I am asking from the client perspective. I know that on the server this has potential to save a lot of disk space). > I looked for discussion on the ml archives. I found the patch series > but could not easily find much *discussion* of the feature and its > design. I found one post [1] that indicated that "part of the > rationale..." (being what I described above), but I would like to > understand the *rest* of the rationale. > > Pointers to gmane are also fine, or brief descriptions of uses [being] > made of this. > > [1]: > http://article.gmane.org/gmane.comp.version-control.git/175832/match=namespace > > Thanks > > -- > Sitaram -- Sitaram -- To unsubscribe from this list: send the line "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] Unify appending signoff in format-patch, commit and sequencer
There are two implementations of append_signoff in log-tree.c and
sequencer.c, which do more or less the same thing. This patch
- teaches sequencer.c's append_signoff() not to append the signoff if
it's already there but not at the bottom
- removes log-tree.c's
- make sure "Signed-off-by: foo" in the middle of a line is not
counted as a sign off
Signed-off-by: Nguyễn Thái Ngọc Duy
---
Interestingly this patch triggers the fault that it fixes.
I was surprised that there was no blank line before my S-o-b
and thought I broke something. It turns out I used unmodified
format-patch and it mistook the S-o-b quote as true S-o-b line.
The modified one puts the blank line back.
builtin/log.c | 13 +
log-tree.c| 92 ---
revision.h| 2 +-
sequencer.c | 86 +++
4 files changed, 88 insertions(+), 105 deletions(-)
diff --git a/builtin/log.c b/builtin/log.c
index e7b7db1..bb48344 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1058,7 +1058,6 @@ int cmd_format_patch(int argc, const char **argv, const
char *prefix)
struct commit *origin = NULL, *head = NULL;
const char *in_reply_to = NULL;
struct patch_ids ids;
- char *add_signoff = NULL;
struct strbuf buf = STRBUF_INIT;
int use_patch_format = 0;
int quiet = 0;
@@ -1154,16 +1153,6 @@ int cmd_format_patch(int argc, const char **argv, const
char *prefix)
PARSE_OPT_KEEP_ARGV0 | PARSE_OPT_KEEP_UNKNOWN |
PARSE_OPT_KEEP_DASHDASH);
- if (do_signoff) {
- const char *committer;
- const char *endpos;
- committer = git_committer_info(IDENT_STRICT);
- endpos = strchr(committer, '>');
- if (!endpos)
- die(_("bogus committer info %s"), committer);
- add_signoff = xmemdupz(committer, endpos - committer + 1);
- }
-
for (i = 0; i < extra_hdr.nr; i++) {
strbuf_addstr(&buf, extra_hdr.items[i].string);
strbuf_addch(&buf, '\n');
@@ -1354,7 +1343,7 @@ int cmd_format_patch(int argc, const char **argv, const
char *prefix)
total++;
start_number--;
}
- rev.add_signoff = add_signoff;
+ rev.add_signoff = do_signoff;
while (0 <= --nr) {
int shown;
commit = list[nr];
diff --git a/log-tree.c b/log-tree.c
index c894930..18cf006 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -9,6 +9,7 @@
#include "string-list.h"
#include "color.h"
#include "gpg-interface.h"
+#include "sequencer.h"
struct decoration name_decoration = { "object names" };
@@ -206,89 +207,6 @@ void show_decorations(struct rev_info *opt, struct commit
*commit)
putchar(')');
}
-/*
- * Search for "^[-A-Za-z]+: [^@]+@" pattern. It usually matches
- * Signed-off-by: and Acked-by: lines.
- */
-static int detect_any_signoff(char *letter, int size)
-{
- char *cp;
- int seen_colon = 0;
- int seen_at = 0;
- int seen_name = 0;
- int seen_head = 0;
-
- cp = letter + size;
- while (letter <= --cp && *cp == '\n')
- continue;
-
- while (letter <= cp) {
- char ch = *cp--;
- if (ch == '\n')
- break;
-
- if (!seen_at) {
- if (ch == '@')
- seen_at = 1;
- continue;
- }
- if (!seen_colon) {
- if (ch == '@')
- return 0;
- else if (ch == ':')
- seen_colon = 1;
- else
- seen_name = 1;
- continue;
- }
- if (('A' <= ch && ch <= 'Z') ||
- ('a' <= ch && ch <= 'z') ||
- ch == '-') {
- seen_head = 1;
- continue;
- }
- /* no empty last line doesn't match */
- return 0;
- }
- return seen_head && seen_name;
-}
-
-static void append_signoff(struct strbuf *sb, const char *signoff)
-{
- static const char signed_off_by[] = "Signed-off-by: ";
- size_t signoff_len = strlen(signoff);
- int has_signoff = 0;
- char *cp;
-
- cp = sb->buf;
-
- /* First see if we already have the sign-off by the signer */
- while ((cp = strstr(cp, signed_off_by))) {
-
- has_signoff = 1;
-
- cp += strlen(signed_off_by);
- if (cp + signoff_len >= sb->buf + sb->len)
- break;
- if (strncmp(cp, signoff, signoff_len))
- continue;
- if (!isspace(cp[signoff_len]))
-
[PATCH] wildmatch: correct isprint and isspace
Current isprint() incorrectly includes control characters 9, 10 and 13, which is fixed by this patch. Current isspace() lacks 11 and 12. But Git's isspace() has been designed this way since the beginning and has over 100 call sites relying on this. Instead of updating isspace() behavior (which could be tricky as patches from other topics may come in parallel that assume the old isspace()), a new isspace_posix() is introduced and used by wildmatch.c. Other part of Git can be converted to use this new function if it seems appropriate. Signed-off-by: Nguyễn Thái Ngọc Duy --- Sorry for the late response. I'll reply to everybody in one mail. On Wed, Nov 14, 2012 at 1:58 AM, "Jan H. Schönherr" wrote: > An alternative to switching from 1-byte to 4-byte values (don't we have > a 2-byte datatype?), would be to free up GIT_CNTRL and simply do: > > #define iscntrl(x) ((x) < 0x20) No. 127 is also a control character. On Wed, Nov 14, 2012 at 2:41 AM, Johannes Sixt wrote: > So we have two properties that overlap: > > SS > > > You seem to generate partions: > >XXXYZ > > then assign individual bits to each partition. Now each entry in the > lookup table has only one bit set. Then you define isxxx() to check for > one of the two possible bits: > >iscntrl is X or Y >isspace is Y or Z > > But shouldn't you just assign one bit for S and another one for C, have > entries in the lookup table with more than one bit set, and check for > only one bit in the isxxx macro? > > That way you don't run out of bits as easily as you do with this patch. I need three sets of characters actually: control, spaces and printable (which contains non-control spaces). Making it (isspace(x) && (x) >= 32) is simpler and because isprint() is only used in wildmatch, I don't need to think about performance penalty (yet). On Thu, Nov 15, 2012 at 2:30 AM, René Scharfe wrote: > Nevertheless, it's unfortunate that we have an isspace() that *almost* does > what the widely known thing of the same name does. I'd shy away from > changing git's version directly, because it's used more than a hundred times > in the code, and estimating the impact of adding \v and \f to it. > Perhaps renaming it to isgitspace() is a good first step, followed by > adding a "standard" version of isspace() for wildmatch? There are just too many call sites of isspace() and there is a risk of new call sites coming in independently. So I think keeping isspace() as-is and using a different name for the standard version is probably a better choice. As the new isspace_posix() is only used by wildmatch, its performance as of now is not critical and a simple macro like in this patch is probably enough. We can optimize it later if we need to. git-compat-util.h | 4 +++- wildmatch.c | 8 ++-- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/git-compat-util.h b/git-compat-util.h index 02f48f6..d4c3fda 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -486,6 +486,7 @@ extern const unsigned char sane_ctype[256]; #define sane_istest(x,mask) ((sane_ctype[(unsigned char)(x)] & (mask)) != 0) #define isascii(x) (((x) & ~0x7f) == 0) #define isspace(x) sane_istest(x,GIT_SPACE) +#define isspace_posix(x) (((x) >= 9 && (x) <= 13) || (x) == 32) #define isdigit(x) sane_istest(x,GIT_DIGIT) #define isalpha(x) sane_istest(x,GIT_ALPHA) #define isalnum(x) sane_istest(x,GIT_ALPHA | GIT_DIGIT) @@ -499,7 +500,8 @@ extern const unsigned char sane_ctype[256]; #define isxdigit(x) (hexval_table[x] != -1) #define isprint(x) (sane_istest(x, GIT_ALPHA | GIT_DIGIT | GIT_SPACE | \ GIT_PUNCT | GIT_REGEX_SPECIAL | GIT_GLOB_SPECIAL | \ - GIT_PATHSPEC_MAGIC)) + GIT_PATHSPEC_MAGIC) && \ + (x) >= 32) #define tolower(x) sane_case((unsigned char)(x), 0x20) #define toupper(x) sane_case((unsigned char)(x), 0) #define is_pathspec_magic(x) sane_istest(x,GIT_PATHSPEC_MAGIC) diff --git a/wildmatch.c b/wildmatch.c index 3972e26..fd74efd 100644 --- a/wildmatch.c +++ b/wildmatch.c @@ -37,11 +37,7 @@ typedef unsigned char uchar; # define ISBLANK(c) ((c) == ' ' || (c) == '\t') #endif -#ifdef isgraph -# define ISGRAPH(c) (ISASCII(c) && isgraph(c)) -#else -# define ISGRAPH(c) (ISASCII(c) && isprint(c) && !isspace(c)) -#endif +#define ISGRAPH(c) (ISASCII(c) && isprint(c) && !isspace_posix(c)) #define ISPRINT(c) (ISASCII(c) && isprint(c)) #define ISDIGIT(c) (ISASCII(c) && isdigit(c)) @@ -50,7 +46,7 @@ typedef unsigned char uchar; #define ISCNTRL(c) (ISASCII(c) && iscntrl(c)) #define ISLOWER(c) (ISASCII(c) && islower(c)) #define ISPUNCT(c) (ISASCII(c) && ispunct(c)) -#define ISSPACE(c) (ISASCII(c) && isspace(c)) +#define ISSPACE(c) (ISASCII(c) && isspace_posix(c)) #define ISUPPER(c) (ISASCII(c) && isupper(c)) #define ISXDIGIT(c) (ISASCII(c) && isxdigit(c)) -- 1.8.0.rc2.23.g1fb49df -- To unsubscribe from this list: send th
[PATCH] tcsh-completion re-using git-completion.bash
The current tcsh-completion support for Git, as can be found on the
Internet, takes the approach of defining the possible completions
explicitly. This has the obvious draw-back to require constant
updating as the Git code base evolves.
The approach taken by this commit is to to re-use the advanced bash
completion script and use its result for tcsh completion. This is
achieved by executing (versus sourcing) the bash script and
outputting the completion result for tcsh consumption.
Three solutions were looked at to implement this approach with (A)
being retained:
A) Modifications:
git-completion.bash and new git-completion.tcsh
Modify the existing git-completion.bash script to support
being sourced using bash (as now), but also executed using bash.
When being executed, the script will output the result of the
computed completion to be re-used elsewhere (e.g., in tcsh).
The modification to git-completion.bash is made not to be
tcsh-specific, but to allow future users to also re-use its
output. Therefore, to be general, git-completion.bash accepts a
second optional parameter, which is not used by tcsh, but could
prove useful for other users.
Pros:
1- allows the git-completion.bash script to easily be re-used
2- tcsh support is mostly isolated in git-completion.tcsh
Cons (for tcsh users only):
1- requires the user to copy both git-completion.tcsh and
git-completion.bash to ${HOME}
2- requires bash script to have a fixed name and location:
${HOME}/.git-completion.bash
B) Modifications:
git-completion.bash
Modify the existing git-completion.bash script to support
being sourced using bash (as now), but also executed using bash,
and sourced using tcsh.
Pros:
1- only requires the user to deal with a single file
2- maintenance more obvious for tcsh since it is entirely part
of the same git-completion.bash script.
Cons:
1- tcsh support could affect bash support as they share the
same script
2- small tcsh section must use syntax suitable for both tcsh
and bash and must be at the beginning of the script
3- requires script to have a fixed name and location:
${HOME}/.git-completion.sh (for tcsh users only)
C) Modifications:
New git-completion.tcsh
Provide a short tcsh script that converts git-completion.bash
into an executable script suitable to be used by tcsh.
Pros:
1- tcsh support is entirely isolated in git-completion.tcsh
2- new tcsh script can be as complex as needed
Cons (for tcsh users only):
1- requires the user to copy both git-completion.tcsh and
git-completion.bash to ${HOME}
2- requires bash script to have a fixed name and location:
${HOME}/.git-completion.bash
3- sourcing the new script will generate a third script
Approach (A) was selected to keep the tcsh completion support well
isolated without introducing excessive complexity.
Signed-off-by: Marc Khouzam
---
Here is the updated version of the patch.
I got git send-email to work, so I hope the formatting will be correct.
Thanks in advance.
Marc
contrib/completion/git-completion.bash | 47
contrib/completion/git-completion.tcsh | 29 +++
2 files changed, 76 insertions(+), 0 deletions(-)
create mode 100644 contrib/completion/git-completion.tcsh
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index be800e0..d71a016 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2481,3 +2481,50 @@ __git_complete gitk __gitk_main
if [ Cygwin = "$(uname -o 2>/dev/null)" ]; then
__git_complete git.exe __git_main
fi
+
+# Method that will output the result of the completion done by
+# the bash completion script, so that it can be re-used in another
+# context than the bash complete command.
+# It accepts 1 to 2 arguments:
+# 1: The command-line to complete
+# 2: The index of the word within argument #1 in which the cursor is
+#located (optional). If parameter 2 is not provided, it will be
+#determined as best possible using parameter 1.
+__git_complete_with_output ()
+{
+ # Set COMP_WORDS in a way that can be handled by the bash script.
+ COMP_WORDS=($1)
+
+ # Set COMP_CWORD to the cursor location as bash would.
+ if [ -n "${2-}" ]; then
+ COMP_CWORD=$2
+ else
+ # Assume the cursor is at the end of parameter #1.
+ # We must check for a space as the last character which will
+ # tell us that the previous word is complete and the cursor
+ # is on the next word.
+ if [ "${1: -1}" == " " ]; then
+ # The last character is a space, so our location is at
the e
Re: [PATCHv2 8/8] send-email: do not prompt for explicit repo ident
On Thu, Nov 15, 2012 at 03:13:47AM -0800, Jeff King wrote: > I think a much more compelling argument/commit message for your > suggested patch would be: > > We currently prompt the user for the "From" address. This is an > inconvenience in the common case that the user has configured their > identity in the environment, but is meant as a safety check for when > git falls back to an implicitly generated identity (which may or may > not be valid). > > That safety check is not really necessary, though, as by default > send-email will prompt the user for a final confirmation before > sending out any message. The likelihood that a user has both bothered > to turn off this default _and_ not configured any identity (nor > checked that the automatic identity is valid) is rather low. If that is the route we want to go, then we should obviously drop my series in favor of your final patch. I think it would also need a test update, no? I think a more concise commit message would help, too. I disagree with a great deal of the reasoning in your existing message, but those parts turn out not to be relevant. The crux of the issue is that the safety check is not necessary because there is already one (i.e., point 8 of your list). Feel free to use any or all of my text above. >From my series, there were a few cleanups that might be worth salvaging. Here is a rundown by patch: [1/8]: test-lib: allow negation of prerequisites This stands on its own, and is something I have wanted a few times in the past. However, since there is no immediate user, I don't know if it is worth doing or not. [2/8]: t7502: factor out autoident prerequisite A minor cleanup and possible help to future tests, but since there are no other callers now, not sure if it is worth it. [3/8]: ident: make user_ident_explicitly_given static A cleanup that is worth doing, I think. [4/8]: ident: keep separate "explicit" flags for author and committer Another cleanup. This is "more correct", in that it handles the corner cases I mentioned in the commit message. But no current code cares about those corner cases, because the only real caller is git-commit, and this is a purely internal interface. I could take or leave it. [5/8]: var: accept multiple variables on the command line The tests for this can be split out; we currently don't have "git var" tests at all, so increasing our test coverage is reasonable. The multiple variables thing is potentially useful, but there are simply not that many callers of "git var", and nobody has been asking for such a feature (we could use it to save a process in git-send-email, but it is probably not worth the complexity). [6/8]: var: provide explicit/implicit ident information [7/8]: Git.pm: teach "ident" to query explicitness These two should probably be dropped. They would lock us into supporting the explicit/implicit variables in "git var", for no immediate benefit. [8/8]: send-email: do not prompt for explicit repo ident Obviously drop. > I could accept that line of reasoning. I see that this argument is > buried deep in your commit message, but I will admit to not reading your > 9-point list of conditions all that closely, as the first 7 points are, > in my opinion, not relevant (and I had already read and disagreed with > them in other messages). If it sounds like I am blaming you here, I am to some degree. But I am also blaming myself. I should have read your commit message more carefully, and I'm sorry for not doing so. I hope we can both try harder to avoid getting side-tracked on arguing about issues that turned out not to be important at all (of course, we cannot know which part of the discussion will turn out to be important, but I think there some obviously unproductive parts of this discussion). -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: [PATCHv2 8/8] send-email: do not prompt for explicit repo ident
On Thu, Nov 15, 2012 at 02:43:58AM -0800, Jeff King wrote: > > And doesn't have any of the following: > > > > * configured user.name/user.email > > * specified $EMAIL > > * configured sendemail.from > > * specified --from argument > > > > Very unlikely. > > That is certainly the opinion you have stated already. I'm not sure I > agree. Linus, for example, was an advocate of such a configuration early > on in git's history. I don't think he still runs that way, though. > > > And then, what would be the consequences of not receiving this prompt? > > An email would be sent with the generated identity. I suspect you did not need me to answer that question, but were setting it up as a rhetorical trap to mention the final confirmation, which I failed to note in my response. I think a much more compelling argument/commit message for your suggested patch would be: We currently prompt the user for the "From" address. This is an inconvenience in the common case that the user has configured their identity in the environment, but is meant as a safety check for when git falls back to an implicitly generated identity (which may or may not be valid). That safety check is not really necessary, though, as by default send-email will prompt the user for a final confirmation before sending out any message. The likelihood that a user has both bothered to turn off this default _and_ not configured any identity (nor checked that the automatic identity is valid) is rather low. I could accept that line of reasoning. I see that this argument is buried deep in your commit message, but I will admit to not reading your 9-point list of conditions all that closely, as the first 7 points are, in my opinion, not relevant (and I had already read and disagreed with them in other messages). -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] status: add advice on how to push/pull to tracking branch
Signed-off-by: Matthieu Moy
---
I thought this was obvious enough not to deserve an advice, but a
colleague of mine had troubles with "commited but not pushed" changes.
Maybe an additional advice would have helped. After all, it's an
advice, and can be deactivated ...
remote.c | 13 ++---
t/t2020-checkout-detach.sh | 1 +
2 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/remote.c b/remote.c
index 04fd9ea..9c19689 100644
--- a/remote.c
+++ b/remote.c
@@ -1627,13 +1627,15 @@ int format_tracking_info(struct branch *branch, struct
strbuf *sb)
base = branch->merge[0]->dst;
base = shorten_unambiguous_ref(base, 0);
- if (!num_theirs)
+ if (!num_theirs) {
strbuf_addf(sb,
Q_("Your branch is ahead of '%s' by %d commit.\n",
"Your branch is ahead of '%s' by %d commits.\n",
num_ours),
base, num_ours);
- else if (!num_ours)
+ strbuf_addf(sb,
+ _(" (use \"git push\" to publish your local
commits)\n"));
+ } else if (!num_ours) {
strbuf_addf(sb,
Q_("Your branch is behind '%s' by %d commit, "
"and can be fast-forwarded.\n",
@@ -1641,7 +1643,9 @@ int format_tracking_info(struct branch *branch, struct
strbuf *sb)
"and can be fast-forwarded.\n",
num_theirs),
base, num_theirs);
- else
+ strbuf_addf(sb,
+ _(" (use \"git pull\" to update your local
branch)\n"));
+ } else {
strbuf_addf(sb,
Q_("Your branch and '%s' have diverged,\n"
"and have %d and %d different commit each, "
@@ -1651,6 +1655,9 @@ int format_tracking_info(struct branch *branch, struct
strbuf *sb)
"respectively.\n",
num_theirs),
base, num_ours, num_theirs);
+ strbuf_addf(sb,
+ _(" (use \"git pull\" to merge the remote branch into
yours)\n"));
+ }
return 1;
}
diff --git a/t/t2020-checkout-detach.sh b/t/t2020-checkout-detach.sh
index 8100537..5d68729 100755
--- a/t/t2020-checkout-detach.sh
+++ b/t/t2020-checkout-detach.sh
@@ -151,6 +151,7 @@ test_expect_success 'checkout does not warn leaving
reachable commit' '
cat >expect <<'EOF'
Your branch is behind 'master' by 1 commit, and can be fast-forwarded.
+ (use "git pull" to update your local branch)
EOF
test_expect_success 'tracking count is accurate after orphan check' '
reset &&
--
1.8.0.319.g8abfee4
--
To unsubscribe from this list: send the line "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 8/8] send-email: do not prompt for explicit repo ident
On Thu, Nov 15, 2012 at 11:28:46AM +0100, Felipe Contreras wrote: > I tried both: > > ok 19 # skip implicit ident prompts for sender (missing AUTOIDENT of > PERL,AUTOIDENT) > ok 20 - broken implicit ident aborts send-email > > ok 19 - implicit ident prompts for sender > ok 20 # skip broken implicit ident aborts send-email (missing > !AUTOIDENT of PERL,!AUTOIDENT) > > However, it would be much easier if ident learned to check > GIT_TEST_FAKE_HOSTNAME, or something. Yes, it would be. It has two downsides: 1. The regular git code has to be instrumented to respect the variable, so it can potentially affect git in production use outside of the test suite. Since such code is simple, though, it is probably not a big risk. 2. We would not actually exercise the code paths for doing hostname and GECOS lookup. We do not test their resulting values, so the coverage is not great now, but we do at least run the code, which would let a run with "--valgrind" check it. I guess we could go through the motions of assembling the ident and then replace it at the end with the fake value. I don't have a strong opinion either way. > > One whose system is configured in such a way that git can produce an > > automatic ident (i.e., has a non-blank GECOS name and a FQDN). > > And doesn't have any of the following: > > * configured user.name/user.email > * specified $EMAIL > * configured sendemail.from > * specified --from argument > > Very unlikely. That is certainly the opinion you have stated already. I'm not sure I agree. Linus, for example, was an advocate of such a configuration early on in git's history. I don't think he still runs that way, though. > And then, what would be the consequences of not receiving this prompt? An email would be sent with the generated identity. -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: [PATCHv2 8/8] send-email: do not prompt for explicit repo ident
On Thu, Nov 15, 2012 at 9:33 AM, Jeff King wrote:
> On Thu, Nov 15, 2012 at 03:08:42AM +0100, Felipe Contreras wrote:
>
>> I don't think there's any need for all that, this does the trick:
>>
>> diff --git a/git-send-email.perl b/git-send-email.perl
>> index aea66a0..503e551 100755
>> --- a/git-send-email.perl
>> +++ b/git-send-email.perl
>> @@ -748,16 +748,11 @@ if (!$force) {
>> }
>> }
>>
>> -my $prompting = 0;
>> if (!defined $sender) {
>> $sender = $repoauthor || $repocommitter || '';
>> - $sender = ask("Who should the emails appear to be from? [$sender] ",
>> - default => $sender,
>> - valid_re => qr/\@.*\./, confirm_only => 1);
>> - print "Emails will be sent from: ", $sender, "\n";
>> - $prompting++;
>> }
>>
>> +my $prompting = 0;
>>
>> This passes all the current tests and the ones you added.
>
> It may pass on your system, but it will not on a system that meets the
> AUTOIDENT prerequisite (it fails the new t9001.19 on my system; I
> suspect your system config is such that we skip t9001.19 and run
> t9001.20, whereas mine is the opposite).
I tried both:
ok 19 # skip implicit ident prompts for sender (missing AUTOIDENT of
PERL,AUTOIDENT)
ok 20 - broken implicit ident aborts send-email
ok 19 - implicit ident prompts for sender
ok 20 # skip broken implicit ident aborts send-email (missing
!AUTOIDENT of PERL,!AUTOIDENT)
However, it would be much easier if ident learned to check
GIT_TEST_FAKE_HOSTNAME, or something.
But then I realized I had to run 'make' again. Yes, my patch breaks
test 19, but see below.
>> Which kind of user will get the prompt with your patch, that would
>> miss it with mine?
>
> One whose system is configured in such a way that git can produce an
> automatic ident (i.e., has a non-blank GECOS name and a FQDN).
And doesn't have any of the following:
* configured user.name/user.email
* specified $EMAIL
* configured sendemail.from
* specified --from argument
Very unlikely. And then, what would be the consequences of not
receiving this prompt?
Cheers.
--
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: [PATCHv2 8/8] send-email: do not prompt for explicit repo ident
On Thu, Nov 15, 2012 at 03:08:42AM +0100, Felipe Contreras wrote:
> I don't think there's any need for all that, this does the trick:
>
> diff --git a/git-send-email.perl b/git-send-email.perl
> index aea66a0..503e551 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -748,16 +748,11 @@ if (!$force) {
> }
> }
>
> -my $prompting = 0;
> if (!defined $sender) {
> $sender = $repoauthor || $repocommitter || '';
> - $sender = ask("Who should the emails appear to be from? [$sender] ",
> - default => $sender,
> - valid_re => qr/\@.*\./, confirm_only => 1);
> - print "Emails will be sent from: ", $sender, "\n";
> - $prompting++;
> }
>
> +my $prompting = 0;
>
> This passes all the current tests and the ones you added.
It may pass on your system, but it will not on a system that meets the
AUTOIDENT prerequisite (it fails the new t9001.19 on my system; I
suspect your system config is such that we skip t9001.19 and run
t9001.20, whereas mine is the opposite).
> Which kind of user will get the prompt with your patch, that would
> miss it with mine?
One whose system is configured in such a way that git can produce an
automatic ident (i.e., has a non-blank GECOS name and a FQDN).
-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: [PATCHv2 8/8] send-email: do not prompt for explicit repo ident
Jeff King wrote: > --- a/t/t9001-send-email.sh > +++ b/t/t9001-send-email.sh > @@ -191,15 +191,47 @@ test_expect_success $PREREQ 'Show all headers' ' > > test_expect_success $PREREQ 'Prompting works' ' > clean_fake_sendmail && > - (echo "Example " > - echo "t...@example.com" > + (echo "t...@example.com" >echo "" > ) | GIT_SEND_EMAIL_NOTTY=1 git send-email \ > --smtp-server="$(pwd)/fake.sendmail" \ > $patches \ > 2>errors && > + grep "^From: A U Thor \$" msgtxt1 && > + grep "^To: t...@example.com\$" msgtxt1 > +' The indentation seems strange here --- are the new "grep" lines continuations of the git send-email line? It's probably easier to change the structure completely: clean_fake_sendmail && echo t...@examples.com >prompt.input && echo >>prompt.input && GIT_SEND_EMAIL_NOTTY=1 \ git send-email --smtp-server=... $patches \$" msgtxt1 && grep "^To: t...@example.com\$" msgtxt1 > +test_expect_success $PREREQ,AUTOIDENT 'implicit ident prompts for sender' ' > + clean_fake_sendmail && > + (echo "Example " && > + echo "t...@example.com" && > + echo "" > + ) | > + (sane_unset GIT_AUTHOR_NAME && > + sane_unset GIT_AUTHOR_EMAIL && > + sane_unset GIT_COMMITTER_NAME && > + sane_unset GIT_COMMITTER_EMAIL && > + GIT_SEND_EMAIL_NOTTY=1 git send-email \ > + --smtp-server="$(pwd)/fake.sendmail" \ > + $patches \ > + 2>errors && > grep "^From: Example \$" msgtxt1 && > grep "^To: t...@example.com\$" msgtxt1 > + ) > +' Likewise: clean_fake_sendmail && echo "Example " >prompt.in && echo t...@example.com >>prompt.in echo >>prompt.in && ( sane_unset GIT_AUTHOR_NAME GIT_AUTHOR_EMAIL && sane_unset GIT_COMMITTER_NAME GIT_COMMITTER_EMAIL && GIT_SEND_EMAIL_NOTTY=1 \ git send-email --smtp-server=... $patches \$" msgtxt1 && grep "^To: t...@example.com\$" msgtxt1 > +test_expect_success $PREREQ,!AUTOIDENT 'broken implicit ident aborts > send-email' ' > + clean_fake_sendmail && > + (sane_unset GIT_AUTHOR_NAME && > + sane_unset GIT_AUTHOR_EMAIL && > + sane_unset GIT_COMMITTER_NAME && > + sane_unset GIT_COMMITTER_EMAIL && > + GIT_SEND_EMAIL_NOTTY=1 && export GIT_SEND_EMAIL_NOTTY && > + test_must_fail git send-email \ > + --smtp-server="$(pwd)/fake.sendmail" \ > + $patches errors.out && > + test_i18ngrep "tell me who you are" errors.out > + ) > ' Likewise: clean_fake_sendmail && ( sane_unset GIT_AUTHOR_NAME GIT_AUTHOR_EMAIL && sane_unset GIT_COMMITTER_NAME GIT_COMMITTER_EMAIL && GIT_SEND_EMAIL_NOTTY=1 \ git send-email --smtp-server=... $patches err ) && test_i18ngrep "[Tt]ell me who you are" err For what it's worth, with or without such changes, Acked-by: Jonathan Nieder -- To unsubscribe from this list: send the line "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 v3 0/8] Fix GIT_CEILING_DIRECTORIES that contain symlinks
On 11/13/2012 09:50 PM, David Aguilar wrote: > On Mon, Nov 12, 2012 at 9:47 AM, Junio C Hamano wrote: >> Michael Haggerty writes: >> >>> The log message of the original commit (0454dd93bf) described the >>> following scenario: a /home partition under which user home directories >>> are automounted, and setting GIT_CEILING_DIRECTORIES=/home to avoid >>> hitting /home/.git, /home/.git/objects, and /home/objects (which would >>> attempt to automount those directories). I believe that this scenario >>> would not be slowed down by my patches. >>> >>> How do you use GIT_CEILING_DIRECTORIES that the proposed changes cause a >>> slowdown? >> >> Yeah, I was also wondering about that. >> >> David? > > I double-checked our configuration and all the parent directories > of those listed in GIT_CEILING_DIRECTORIES are local, > so our particular configuration would not have a performance hit. > > We do have multiple directories listed there. Some of them share > a parent directory. I'm assuming the implementation is simple and > does not try and avoid repeating the check when the parent dir is > the same across multiple entries. > > In any case, it won't be a problem in practice based on my > reading of the current code. OK, so we're back to the following status: some people (including me) are nervous that this change could cause a performance regression, though it seems that the most sensible ways of using the GIT_CEILING_DIRECTORIES feature would not be affected. In favor: Currently, if a directory containing a symlink is added to GIT_CEILING_DIRECTORIES, then GIT_CEILING_DIRECTORIES will not work, git has no way of recognizing that there is a problem, and the only symptom observable by the user is that the hoped-for performance improvement from using GIT_CEILING_DIRECTORIES will not materialize (or will disappear after a filesystem reorg) [1]. Against: The change will cause a performance regression if a slow-to-stat directory is listed in GIT_CEILING_DIRECTORIES. The slowdown will occur whenever git is run outside of a true git-managed project, most nastily in the case of using __git_ps1 in a shell prompt. I don't have a preference either way about whether these patches should be merged. Michael [1] It is also conceivable that GIT_CEILING_DIRECTORIES is being used to *hide* an enclosing git project rather than to inform git that there are no enclosing projects, in which case the enclosing project would *not* be hidden. This is in fact the mechanism by which the problem causes failures in our test suite. But I don't expect that this is a common real-world scenario, and anyway such a failure would be obvious to the user and quickly fixed. -- Michael Haggerty mhag...@alum.mit.edu http://softwareswirl.blogspot.com/ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 7/8] Git.pm: teach "ident" to query explicitness
Jeff King wrote: > Signed-off-by: Jeff King For what it's worth, Acked-by: Jonathan Nieder -- To unsubscribe from this list: send the line "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 5/8] var: accept multiple variables on the command line
Jeff King wrote: > This patch lets callers specify multiple variables, and > prints one per line. [...] > Signed-off-by: Jeff King Very pleasantly done --- thanks. For what it's worth, assuming this is tested, I can't see any reason not to apply it. Reviewed-by: Jonathan Nieder -- To unsubscribe from this list: send the line "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 4/8] ident: keep separate "explicit" flags for author and committer
Jeff King wrote: > 1. GIT_COMMITTER_* is set explicitly, but we fallback for > GIT_AUTHOR. We claim the ident is explicit, even though > the author is not. > > 2. GIT_AUTHOR_* is set and we ask for author ident, but > not committer ident. We will claim the ident is > implicit, even though it is explicit. > > This patch uses two variables instead of one, updates both > when we set the "fallback" values, and updates them > individually when we read from the environment. Nice problem description. The fixed behavior makes sense to me, for what it's worth. Not about this patch, but: in case (1), shouldn't the author fall back to $GIT_COMMITER_NAME? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
