Re: [PATCH v4 1/2] for-each-repo: new command used for multi-repo operations
On Sun, Jan 27, 2013 at 8:04 PM, Junio C Hamano wrote:
> Lars Hjemli writes:
>
>> The command also honours the option '--clean' which restricts the set of
>> repos to those which '--dirty' would skip, and '-x' which is used to
>> execute non-git commands.
>
> It might make sense to internally use RUN_GIT_CMD flag when the
> first word of the command line is 'git' as an optimization, but
> I am not sure it is a good idea to force the end users to think
> when to use -x and when not to is a good idea.
>
> In other words, I think
>
> git for-each-repo -d diff --name-only
> git for-each-repo -d -x ls '*.c'
>
> is less nice than letting the user say
>
> git for-each-repo -d git diff --name-only
> git for-each-repo -d ls '*.c'
>
The 'git-for-each-repo' command was made to allow any git command to
be executed in all discovered repositories, and I've used it that way
for two years (in the form of a shell-script called 'git-all'). During
this time, I've occasionally thought about forking non-git commands
but the itch hasn't been strong enough for me to scratch. The point
I'm trying to make is that to me, this command acts as a modifier for
other git commands[1]. Having the possibility to execute non-git
commands would be nice, but it is not the main objective of this
command.
[1] The 'git -a' rewrite patch shows how I think about this command -
it's just an option to the 'git' command, modifying the way any
subcommand is invoked (btw: I don't expect that patch to be applied
since 'git-all' was deemed to generic, so I'll just carry the patch in
my own tree).
>> Finally, the command to execute within each repo is optional. If none is
>> given, git-for-each-repo will just print the path to each repo found. And
>> since the command supports -z, this can be used for more advanced scripting
>> needs.
>
> It amounts to the same thing, but I would rather describe it as:
>
> To allow scripts to handle paths with shell-unsafe characters,
> support "-z" to show paths with NUL termination. Otherwise,
> such paths are shown with the usual c-quoting.
>
Much better, thanks.
> One more thing that nobody brought up during the previous reviews is
> if we want to support subset of repositories by allowing the
> standard pathspec match mechanism. For example,
>
> git for-each-repo -d git diff --name-only -- foo/ bar/b\*z
>
> might be a way to ask "please find repositories match the given
> pathspecs (i.e. foo/ bar/b\*z) and run the command in the ones that
> are dirty". We would need to think about how to mark the end of the
> command though---we could borrow \; from find(1), even though find
> is not the best example of the UI design. I.e.
>
> git for-each-repo -d git diff --name-only \; [--] foo/ bar/b\*z
>
> with or without "--".
I don't think this would be very nice to end users, and would prefer
--include and --exclude options (the latter is actually already a part
of git-all, added by one of my coworkers).
>> +NOTES
>> +-
>> +
>> +For the purpose of `git-for-each-repo`, a dirty worktree is defined as a
>> +worktree with uncommitted changes.
>
> Is it a definition that is different from usual? If so why does it
> need to be inconsistent with the rest of the system?
I just wanted to clarify what condition --dirty and --clean will
check. In particular, the lack of checking for untracked files (which
could be added as yet another option).
>> +static void print_repo_path(const char *path, unsigned pretty)
>> +{
>> + if (path[0] == '.' && path[1] == '/')
>> + path += 2;
>> + if (pretty)
>> + color_fprintf_ln(stdout, color, "[%s]", path);
>
> This is shown before running a command in that repository. I am of
> two minds. It certainly is nice to be able to tell which repository
> each block of output lines comes from, and not requiring the command
> to do this themselves is a good default. However, I wonder if people
> would want to do something like this:
>
> git for-each-repo sh -c '
> git diff --name-only |
> sed -e "s|^|$path/|"
> '
>
> to get a consolidated view, in a way similar to how "submodule
> foreach" can be used. This unconditional output will get in the way
> for such a use case.
I guess -q/--quiet could be useful.
>> +static int walk(struct strbuf *path, int argc, const char **argv)
>> +{
>> + DIR *dir;
>> + struct dirent *ent;
>> + struct stat st;
>> + size_t len;
>> + int has_dotgit = 0;
>> + struct string_list list = STRING_LIST_INIT_DUP;
>> + struct string_list_item *item;
>> +
>> + dir = opendir(path->buf);
>> + if (!dir)
>> + return errno;
>> + strbuf_addstr(path, "/");
>> + len = path->len;
>> + while ((ent = readdir(dir))) {
>> + if (!strcmp(ent->d_name, ".") || !strcmp(ent->d_name, ".."))
>> + continue;
>> + if (!strcmp(ent->d_name, ".git")) {
>> +
Re: [PATCH 6/7] read-cache: refuse to create index referring to external objects
On Mon, Jan 28, 2013 at 1:36 PM, Junio C Hamano wrote: > Duy Nguyen writes: > >> On Mon, Jan 28, 2013 at 12:48 PM, Duy Nguyen wrote: >>> Regardless the submodule odb issue, I think we should prefer >>> reading local loose objects over alternate packed ones. >> >> I think I went from one problem to another and did not make it clear. >> The reason behind this preference is security. > > Reading from ours first would not help you at all if you are lacking > some that you do need from your local repository and the only > solution is not to borrow from untrustworthy sources, I think. > > You borrow only from a trusted source in the first place, no? Hmm.. yeah. -- 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
What's cooking in git.git (Jan 2013, #10; Sun, 27)
Here are the topics that have been cooking. Commits prefixed with '-' are only in 'pu' (proposed updates) while commits prefixed with '+' are in 'next'. As usual, this cycle is expected to last for 8 to 10 weeks, with a preview -rc0 sometime in the middle of next month. You can find the changes described here in the integration branches of the repositories listed at http://git-blame.blogspot.com/p/git-public-repositories.html -- [New Topics] * bc/git-p4-for-python-2.4 (2013-01-26) 2 commits - git-p4.py: support Python 2.4 - git-p4.py: support Python 2.5 With small updates to remove dependency on newer features of Python, keep git-p4 usable with older Python. Will merge to 'next'. * jk/gc-auto-after-fetch (2013-01-26) 1 commit - Merge branch 'jk/maint-gc-auto-after-fetch' into jk/gc-auto-after-fetch (this branch uses jk/maint-gc-auto-after-fetch.) This is to resolve merge conflicts early for the same topic to recent codebase. Will merge to 'next'. * jk/maint-gc-auto-after-fetch (2013-01-26) 2 commits - fetch-pack: avoid repeatedly re-scanning pack directory - fetch: run gc --auto after fetching (this branch is used by jk/gc-auto-after-fetch.) Help "fetch only" repositories that does not trigger "gc --auto" often enough. Will merge to 'next' via jk/gc-auto-after-fetch. * jk/read-commit-buffer-data-after-free (2013-01-26) 3 commits - logmsg_reencode: lazily load missing commit buffers - logmsg_reencode: never return NULL - commit: drop useless xstrdup of commit message Clarify the ownership rule for commit->buffer field, which some callers incorrectly accessed without making sure the data is available there. Will merge to 'next'. * pw/git-p4-on-cygwin (2013-01-26) 21 commits - git p4: introduce gitConfigBool - git p4: avoid shell when calling git config - git p4: avoid shell when invoking git config --get-all - git p4: avoid shell when invoking git rev-list - git p4: avoid shell when mapping users - git p4: disable read-only attribute before deleting - git p4 test: use test_chmod for cygwin - git p4: cygwin p4 client does not mark read-only - git p4 test: avoid wildcard * in windows - git p4 test: use LineEnd unix in windows tests too - git p4 test: newline handling - git p4: scrub crlf for utf16 files on windows - git p4: remove unreachable windows \r\n conversion code - git p4 test: translate windows paths for cygwin - git p4 test: start p4d inside its db dir - git p4 test: use client_view in t9806 - git p4 test: avoid loop in client_view - git p4 test: use client_view to build the initial client - git p4: generate better error message for bad depot path - git p4: remove unused imports - git p4: temp branch name should use / even on windows Improve "git p4" on Cygwin. The cover letter said it is not yet ready for full Windows support so I won't move this to 'next' until told by the author (the area maintainer) otherwise. * ss/mergetools-tortoise (2013-01-26) 2 commits - mergetools: allow passing pathnames with SP in them to TortoiseGitMerge - mergetools: support TortoiseGitMerge Update mergetools to work better with newer merge helper tortoise ships. Will merge to 'next'. * da/mergetool-docs (2013-01-27) 4 commits - doc: generate a list of valid merge tools - mergetool--lib: add functions for finding available tools - mergetool--lib: improve the help text in guess_merge_tool() - mergetool--lib: simplify command expressions (this branch uses jk/mergetool.) Build on top of the clean-up done by jk/mergetool and automatically generate the list of mergetool and difftool backends the build supports to be included in the documentation. This may still need to be fixed up at minor details; I'd like to see a review from John Keeping on these. * nd/branch-error-cases (2013-01-27) 4 commits - branch: mark more strings for translation - branch: give a more helpful message on redundant arguments - branch: reject -D/-d without branch name - branch: no detached HEAD check when editing another branch's description Fix various error messages and conditions in "git branch", e.g. we advertised "branch -d/-D" to remove one or more branches but actually implemented removal of zero or more branches---request to remove no branches was not rejected. Will merge to 'next', perhaps after rebasing on an older base so that this can later be merged to the maintenance track. -- [Stalled] * mp/complete-paths (2013-01-11) 1 commit - git-completion.bash: add support for path completion The completion script used to let the default completer to suggest pathnames, which gave too many irrelevant choices (e.g. "git add" would not want to add an unmodified path). Teach it to use a more git-aware logic to enumerate only relevant ones. Waiting for area-experts' help and review. * jl/submodule-deinit (2012-12-04) 1 commit - submodul
Re: [PATCH 6/7] read-cache: refuse to create index referring to external objects
Duy Nguyen writes: > On Mon, Jan 28, 2013 at 12:48 PM, Duy Nguyen wrote: >> Regardless the submodule odb issue, I think we should prefer >> reading local loose objects over alternate packed ones. > > I think I went from one problem to another and did not make it clear. > The reason behind this preference is security. Reading from ours first would not help you at all if you are lacking some that you do need from your local repository and the only solution is not to borrow from untrustworthy sources, I think. You borrow only from a trusted source in the first place, no? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/7] read-cache: refuse to create index referring to external objects
On Mon, Jan 28, 2013 at 12:48 PM, Duy Nguyen wrote: > Regardless the submodule odb issue, I think we should prefer > reading local loose objects over alternate packed ones. I think I went from one problem to another and did not make it clear. The reason behind this preference is security. With "all packs first" reading order, someone can create a pack in the alternate source and that pack will override the same local loose objects (local packed ones are safe). If someone can create a malicious version with the same SHA-1, we (well, the user) are in trouble. If the user uses this repository directly then the malicious object can be used without detected, even if it does not match the original SHA-1, as we do not always verify content against its SHA-1 for common commands. -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] l10n: de.po: translate 11 new messages
Translate 11 new messages came from git.pot update
in 46bc403 (l10n: Update git.pot (11 new, 7 removed
messages)).
Signed-off-by: Ralf Thielow
---
po/de.po | 37 ++---
1 file changed, 18 insertions(+), 19 deletions(-)
diff --git a/po/de.po b/po/de.po
index 3779f4c..ed8330a 100644
--- a/po/de.po
+++ b/po/de.po
@@ -5,7 +5,7 @@
#
msgid ""
msgstr ""
-"Project-Id-Version: git 1.8.1\n"
+"Project-Id-Version: git 1.8.2\n"
"Report-Msgid-Bugs-To: Git Mailing List \n"
"POT-Creation-Date: 2013-01-25 12:33+0800\n"
"PO-Revision-Date: 2012-10-02 19:35+0200\n"
@@ -1033,9 +1033,9 @@ msgid "unable to access '%s': %s"
msgstr "konnte nicht auf '%s' zugreifen: %s"
#: wrapper.c:423
-#, fuzzy, c-format
+#, c-format
msgid "unable to access '%s'"
-msgstr "konnte nicht auf '%s' zugreifen: %s"
+msgstr "konnte nicht auf '%s' zugreifen"
#: wrapper.c:434
#, c-format
@@ -2997,14 +2997,14 @@ msgid "Would remove %s\n"
msgstr "Würde %s löschen\n"
#: builtin/clean.c:26
-#, fuzzy, c-format
+#, c-format
msgid "Skipping repository %s\n"
-msgstr "ungültiges Projektarchiv '%s'"
+msgstr "Überspringe Projektarchiv %s\n"
#: builtin/clean.c:27
-#, fuzzy, c-format
+#, c-format
msgid "Would skip repository %s\n"
-msgstr "ungültiges Projektarchiv '%s'"
+msgstr "Würde Projektarchiv %s überspringen\n"
#: builtin/clean.c:28
#, c-format
@@ -3223,9 +3223,8 @@ msgid "--bare and --origin %s options are incompatible."
msgstr "Die Optionen --bare und --origin %s sind inkompatibel."
#: builtin/clone.c:708
-#, fuzzy
msgid "--bare and --separate-git-dir are incompatible."
-msgstr "Die Optionen --bare und --origin %s sind inkompatibel."
+msgstr "Die Optionen --bare und --separate-git-dir sind inkompatibel."
#: builtin/clone.c:721
#, c-format
@@ -5449,7 +5448,7 @@ msgstr "zeigt Quelle"
#: builtin/log.c:104
msgid "Use mail map file"
-msgstr ""
+msgstr "verwendet \"mailmap\"-Datei"
#: builtin/log.c:105
msgid "decorate options"
@@ -5542,7 +5541,7 @@ msgstr "beginnt die Nummerierung der Patches bei
anstatt bei 1"
#: builtin/log.c:1114
msgid "mark the series as Nth re-roll"
-msgstr ""
+msgstr "kennzeichnet die Serie als n-te Fassung"
#: builtin/log.c:1116
msgid "Use [] instead of [PATCH]"
@@ -7099,6 +7098,8 @@ msgid ""
"Updates were rejected because the destination reference already exists\n"
"in the remote."
msgstr ""
+"Aktualisierungen wurden zurückgewiesen, weil die Zielreferenz bereits\n"
+"im Fernarchiv existiert."
#: builtin/push.c:269
#, c-format
@@ -7841,14 +7842,12 @@ msgstr ""
"git reset [--mixed | --soft | --hard | --merge | --keep] [-q] []"
#: builtin/reset.c:26
-#, fuzzy
msgid "git reset [-q] [--] ..."
-msgstr "git reset [-q] [--] ..."
+msgstr "git reset [-q] [--] ..."
#: builtin/reset.c:27
-#, fuzzy
msgid "git reset --patch [] [--] [...]"
-msgstr "git reset --patch [] [--] [...]"
+msgstr "git reset --patch [] [--] [...]"
#: builtin/reset.c:33
msgid "mixed"
@@ -7916,9 +7915,9 @@ msgid "reset HEAD but keep local changes"
msgstr "setzt Zweigspitze (HEAD) zurück, behält aber lokale Änderungen"
#: builtin/reset.c:275
-#, fuzzy, c-format
+#, c-format
msgid "Failed to resolve '%s' as a valid revision."
-msgstr "Konnte '%s' nicht als gültige Referenz auflösen."
+msgstr "Konnte '%s' nicht als gültige Revision auflösen."
#: builtin/reset.c:278 builtin/reset.c:286
#, c-format
@@ -7926,9 +7925,9 @@ msgid "Could not parse object '%s'."
msgstr "Konnte Objekt '%s' nicht parsen."
#: builtin/reset.c:283
-#, fuzzy, c-format
+#, c-format
msgid "Failed to resolve '%s' as a valid tree."
-msgstr "Konnte '%s' nicht als gültige Referenz auflösen."
+msgstr "Konnte '%s' nicht als gültigen Baum auflösen."
#: builtin/reset.c:292
msgid "--patch is incompatible with --{hard,mixed,soft}"
--
1.8.1.1.439.g50a6b54
--
To unsubscribe from this list: send the line "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/7] read-cache: refuse to create index referring to external objects
Duy Nguyen writes: >>> Another thing needs to be done for this to work. The current reading >> >> For *what* to work??? > > The "forbid making our repository depend on objects we do not have but > we know about afterwe peek submodule odb" With your "when our object database is contaminated, check objects we base our new object on are available in local or our alternates" together with the "when we try write_object(), do not bypass it with has_sha1_file() check because that may find the object in submodule odb we should *not* have access to; instead check with the same 'local or our alternates' test" I brought up in the message you were responding to, I do not think object read order does not make a difference to our effort to prevent the object database breakage due to temporary contamination by submodule objects. >>> Regardless the submodule odb issue, I think we should prefer >>> reading local loose objects over alternate packed ones. I suspect you are alluding to make write_object() check with has_sha1_file_locally() so that we can wean our repository from existing alternates, but I do not think it is a right approach (instead, you just fully repack locally if you want to dissociate yourself from your alternates). What I was suggesting was to change it to check with has_sha1_file_proper(), to make sure we do not omit writing an object we need to access to, when we know it will vanish once we stop temporarily borrowing from the submodule object store. -- To unsubscribe from this list: send the line "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/7] read-cache: refuse to create index referring to external objects
On Mon, Jan 28, 2013 at 12:54 PM, Junio C Hamano wrote: > Duy Nguyen writes: > >> On Sat, Jan 26, 2013 at 2:00 AM, Junio C Hamano wrote: >>> This is not a tangent, but if you want to go this "forbid making our >>> repository depend on objects we do not have but we know about after >>> we peek submodule odb" route [*1*], write_sha1_file() needs to be >>> told about has_sha1_file_proper(). We may "git add" a new blob in >>> the superproject, the blob may not yet exist in *our* repository, >>> but may happen to already exist in the submodue odb. In such a >>> case, write_sha1_file() has to write that blob in our repository, >>> without the existing has_sha1_file() check bypassing it. Otherwise >>> our attempt to create a tree that contains that blob will fail, >>> saying that the blob only seems to exist to us via submodule odb but >>> not in our repository. >> >> Another thing needs to be done for this to work. The current reading > > For *what* to work??? The "forbid making our repository depend on objects we do not have but we know about afterwe peek submodule odb" > I think the performance consideration is the > only thing that should drive the read-order; correctness should not > be affected. > >> order is packs first, loose objects next. If we create a local loose >> duplicate of an alternate packed object, our local version will never >> be read. Regardless the submodule odb issue, I think we should prefer >> reading local loose objects over alternate packed ones. -- 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 6/7] read-cache: refuse to create index referring to external objects
Duy Nguyen writes: > On Sat, Jan 26, 2013 at 2:00 AM, Junio C Hamano wrote: >> This is not a tangent, but if you want to go this "forbid making our >> repository depend on objects we do not have but we know about after >> we peek submodule odb" route [*1*], write_sha1_file() needs to be >> told about has_sha1_file_proper(). We may "git add" a new blob in >> the superproject, the blob may not yet exist in *our* repository, >> but may happen to already exist in the submodue odb. In such a >> case, write_sha1_file() has to write that blob in our repository, >> without the existing has_sha1_file() check bypassing it. Otherwise >> our attempt to create a tree that contains that blob will fail, >> saying that the blob only seems to exist to us via submodule odb but >> not in our repository. > > Another thing needs to be done for this to work. The current reading For *what* to work??? I think the performance consideration is the only thing that should drive the read-order; correctness should not be affected. > order is packs first, loose objects next. If we create a local loose > duplicate of an alternate packed object, our local version will never > be read. Regardless the submodule odb issue, I think we should prefer > reading local loose objects over alternate packed ones. -- To unsubscribe from this list: send the line "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 09/11] t4014: more tests about appending s-o-b lines
Junio C Hamano writes: >> Is "grep -n" portable? I didn't find any uses of it elsewhere in the >> testsuite. > > Yes, "-n" is in POSIX. Even though we use it ourselves, "git grep" > supports it, too. Ehh even though we *DONT* use it ourselves, ... that is. I do not think we mind, if its use helps our test. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 00/11] unify appending of sob
Jonathan Nieder writes: > Brandon Casey wrote: > >> Round 3. > > Thanks for a pleasant read. My only remaining observations are > cosmetic, except for a portability question in Duy's test script, a > small behavior change when the commit message ends with an > RFC2822-style header with no trailing newline and the possibility of > tightening the pattern in sequencer.c to match the strictness of > format-patch (which could easily wait for a later patch). > > Jonathan Thanks for a quick review. I agree that this series is getting very close with your help. -- To unsubscribe from this list: send the line "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/7] read-cache: refuse to create index referring to external objects
On Sat, Jan 26, 2013 at 2:00 AM, Junio C Hamano wrote: > This is not a tangent, but if you want to go this "forbid making our > repository depend on objects we do not have but we know about after > we peek submodule odb" route [*1*], write_sha1_file() needs to be > told about has_sha1_file_proper(). We may "git add" a new blob in > the superproject, the blob may not yet exist in *our* repository, > but may happen to already exist in the submodue odb. In such a > case, write_sha1_file() has to write that blob in our repository, > without the existing has_sha1_file() check bypassing it. Otherwise > our attempt to create a tree that contains that blob will fail, > saying that the blob only seems to exist to us via submodule odb but > not in our repository. Another thing needs to be done for this to work. The current reading order is packs first, loose objects next. If we create a local loose duplicate of an alternate packed object, our local version will never be read. Regardless the submodule odb issue, I think we should prefer reading local loose objects over alternate packed ones. -- 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 v3 09/11] t4014: more tests about appending s-o-b lines
Jonathan Nieder writes:
> Brandon Casey wrote:
>
> [...]
>> --- a/t/t4014-format-patch.sh
>> +++ b/t/t4014-format-patch.sh
>> @@ -1021,4 +1021,246 @@ test_expect_success 'cover letter using branch
>> description (6)' '
>> grep hello actual >/dev/null
>> '
>>
>> +append_signoff()
>> +{
>> +C=`git commit-tree HEAD^^{tree} -p HEAD` &&
>> +git format-patch --stdout --signoff ${C}^..${C} |
>> +tee append_signoff.patch |
>> +sed -n "1,/^---$/p" |
>> +grep -n -E "^Subject|Sign|^$"
>> +}
>
> Is "grep -n" portable? I didn't find any uses of it elsewhere in the
> testsuite.
Yes, "-n" is in POSIX. Even though we use it ourselves, "git grep"
supports it, too.
Any Emacs user would scream if their platform "grep" does not
support it, as it will make M-x grep (or grep-find) useless.
> Style: checking exit status from format-patch, avoiding sed|grep pipeline:
>
> C=$(git commit-tree HEAD^ -p HEAD) &&
> git format-patch --stdout --signoff $C^..$C >append_signoff.patch &&
> awk '
> /^---$/ { exit; }
> /^Subject/ || /^Sign/ || /^$/ { print NR ":" $0 }
> ' actual
Yeah, awk/perl would be fine, too, and it is good that you pointed
out that the original was losing the exit status from format-patch.
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 v3 05/11] sequencer.c: recognize "(cherry picked from ..." as part of s-o-b footer
On Sun, Jan 27, 2013 at 6:51 PM, Jonathan Nieder wrote:
> Jonathan Nieder wrote:
>
>> Here's the tweak I suggested last time. I think its behavior is
>> slightly better in the "ends with incomplete line" case because it
>> limits the characters examined by is_rfc2822_line() and
>> is_cherry_picked_from_line() not to include buf[len] (which would
>> presumably sometimes be '\0').
>
> Whoops, that revealed a subtlety --- the '\n' or '\0' is what prevents
> exiting the loop in is_rfc2822_line when the line does not contain a
> colon. Here's a corrected version of the tweak, that should actually
> work. :)
>
> diff --git i/sequencer.c w/sequencer.c
> index 0b5cd18c..108ea27b 100644
> --- i/sequencer.c
> +++ w/sequencer.c
> @@ -1029,13 +1029,11 @@ static int is_rfc2822_line(const char *buf, int len)
> for (i = 0; i < len; i++) {
> int ch = buf[i];
> if (ch == ':')
> + return 1;
> + if (!isalnum(ch) && ch != '-')
> break;
> - if (isalnum(ch) || (ch == '-'))
> - continue;
> - return 0;
> }
> -
> - return 1;
> + return 0;
> }
>
> static int is_cherry_picked_from_line(const char *buf, int len)
> @@ -1043,9 +1041,7 @@ static int is_cherry_picked_from_line(const char *buf,
> int len)
> /*
> * We only care that it looks roughly like (cherry picked from ...)
> */
> - return !prefixcmp(buf, cherry_picked_prefix) &&
> - (buf[len - 1] == ')' ||
> -(buf[len - 1] == '\n' && buf[len - 2] == ')'));
> + return !prefixcmp(buf, cherry_picked_prefix) && buf[len - 1] == ')';
> }
>
> static int has_conforming_footer(struct strbuf *sb, int ignore_footer)
> @@ -1072,8 +1068,8 @@ static int has_conforming_footer(struct strbuf *sb, int
> ignore_footer)
> ; /* do nothing */
> k++;
>
> - if (!(is_rfc2822_line(buf + i, k - i) ||
> - is_cherry_picked_from_line(buf + i, k - i)))
> + if (!is_rfc2822_line(buf + i, k - i - 1) &&
> + !is_cherry_picked_from_line(buf + i, k - i - 1))
> return 0;
> }
> return 1;
Looks good to me.
-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
Updating shared ref from remote helper, or fetch hook
I'm working on an hg remote helper that uses git notes for the sha1 revision, so that git users can more easily refer to specific commits when communicating with hg users. Since there may be multiple concurrent fast-import streams, I write the notes to a private ref (refs/notes/hg-REMOTE), to be merged eventually using git notes --ref hg merge hg-REMOTE* There will never be conflicts because each hg commit translates to a unique git commit, thus even if multiple concurrent remotes process the same commit, the corresponding note will match. Unfortunately, I couldn't find a safe way to get this run after a fetch or clone. Of course I can ask the user to arrange to have this command run, but it would be a better interface to have it run automatically since it is a natural responsibility of the remote helper. Am I missing a way to do this or a viable alternative approach? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Fw:รู้เท่าทันกับสถานการณ์ใต้
รู้เท่าทันกับสถานการณ์ใต้ ต้องเรียกว่า“ยอม”กับการแผนการของกลุ่มก่อเหตุรุนแรงที่พยายามทำให้คนไทยพุทธกับมุสลิมแตกแยก เข่นฆ่าล้างแค้นเนื่องจากเข้าใจผิด ซึ่งถ้าหากทั้งสองฝ่ายหลงกลและกระทำการให้เป็นไปตามแผน ที่วางไว้จะกลายเป็นการ ล้างแค้นที่ไม่มีวันสิ้นสุด สุดท้ายจังหวัดชายแดนภาคใต้ก็จะลุกเป็นไฟ แผนการดังกล่าวถูกนำมาใช้นานมาแล้วพร้อมกับแผนการอื่นๆ ซึ่งเริ่มตั้งแต่ปี 2547 ซึ่งเป็นปีแรกที่เกิดเหตุการณ์ และปัจจุบันยังคงนำมาใช้อย่างต่อเนื่องเป็นระยะ ๆ สลับการก่อเหตุอื่น ๆ โดยกลุ่มผู้ก่อเหตุ จะใช้วิธีแต่งกายเลียนแบบเจ้าหน้าที่ เริ่มก่อเหตุด้วยการฆ่าผู้บริสุทธิ์ที่เป็นมุสลิม หรือผู้นำชุมชน ผู้นำศาสนา และปล่อยข่าวลือว่าเจ้าหน้าที่เป็นผู้กระทำ หลังจากนั้นไม่กี่วันกลุ่มผู้ก่อเหตุกลุ่มเดียวกันจะแต่งกายคล้ายคนดาวะห์ ซุกซ่อนอาวุธแล้วไปยิงประชาชนที่เป็นไทยพุทธ พร้อมกับปล่อยข่าวว่าเป็นการกระทำเพื่อล้างแค้นที่เจ้าหน้าที่ทำกับคนมุสลิม เพราะฉะนั้นถึงเวลาหรือยังที่ทั้งคนไทยพุทธและมุสลิมต้องร่วมตระหนัก อย่าไปหลงเชื่อและตกเป็นเหยื่อของการกระทำที่มุ่งร้ายเหล่านั้น และต้องหันมาสร้างความเข้าใจและร่วมมืออย่างจริงจัง เพื่อขจัด กลุ่มคนเหล่านี้ให้หมดไปจากแผ่นดิน เพื่อสันติสุข -- To unsubscribe from this list: send the line "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 00/11] unify appending of sob
Brandon Casey wrote: > Round 3. Thanks for a pleasant read. My only remaining observations are cosmetic, except for a portability question in Duy's test script, a small behavior change when the commit message ends with an RFC2822-style header with no trailing newline and the possibility of tightening the pattern in sequencer.c to match the strictness of format-patch (which could easily wait for a later patch). Jonathan -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 11/11] Unify appending signoff in format-patch, commit and sequencer
Brandon Casey wrote:
> --- a/log-tree.c
> +++ b/log-tree.c
[...]
> @@ -208,94 +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.
> - */
That's stricter than the test from sequencer.c. Maybe it's worth
stealing to avoid false positives?
--
To unsubscribe from this list: send the line "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 10/11] format-patch: update append_signoff prototype
Brandon Casey wrote:
> From: Nguyễn Thái Ngọc Duy
>
> This is a preparation step for merging with append_signoff from
> sequencer.c
Avoids a small unfreed allocation, too. Neat.
Reviewed-by: Jonathan Nieder
(On the other hand, this means more malloc churn when running
"format-patch -s" on a long series of patches, but I don't think
anyone will mind.)
[...]
> --- a/builtin/log.c
> +++ b/builtin/log.c
> @@ -1193,16 +1192,6 @@ int cmd_format_patch(int argc, const char **argv,
> const char *prefix)
> rev.subject_prefix = strbuf_detach(&sprefix, NULL);
> }
>
> - if (do_signoff) {
> - const char *committer;
> - const char *endpos;
> - committer = git_committer_info(IDENT_STRICT);
sequencer.c uses fmt_name() which uses IDENT_STRICT, too. Phew.
--
To unsubscribe from this list: send the line "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 09/11] t4014: more tests about appending s-o-b lines
Brandon Casey wrote:
[...]
> --- a/t/t4014-format-patch.sh
> +++ b/t/t4014-format-patch.sh
> @@ -1021,4 +1021,246 @@ test_expect_success 'cover letter using branch
> description (6)' '
> grep hello actual >/dev/null
> '
>
> +append_signoff()
> +{
> + C=`git commit-tree HEAD^^{tree} -p HEAD` &&
> + git format-patch --stdout --signoff ${C}^..${C} |
> + tee append_signoff.patch |
> + sed -n "1,/^---$/p" |
> + grep -n -E "^Subject|Sign|^$"
> +}
Is "grep -n" portable? I didn't find any uses of it elsewhere in the
testsuite.
Style: checking exit status from format-patch, avoiding sed|grep pipeline:
C=$(git commit-tree HEAD^ -p HEAD) &&
git format-patch --stdout --signoff $C^..$C >append_signoff.patch &&
awk '
/^---$/ { exit; }
/^Subject/ || /^Sign/ || /^$/ { print NR ":" $0 }
' actual
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 08/11] sequencer.c: teach append_signoff to avoid adding a duplicate newline
Brandon Casey wrote:
> Teach append_signoff to detect whether a blank line exists at the position
> that the signed-off-by line will be added, and avoid adding an additional
> one if one already exists. This is necessary to allow format-patch to add a
> s-o-b to a patch with no commit message without adding an extra newline.
I assume this means you're preserving historical behavior when adding
a sign-off to a commit with empty description (which is a good thing),
but what is that behavior? Is it a deliberate choice or something
that developed by default?
[...]
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -1118,11 +1118,15 @@ void append_signoff(struct strbuf *msgbuf, int
> ignore_footer, unsigned flag)
> for (i = msgbuf->len - 1 - ignore_footer; i > 0 && msgbuf->buf[i - 1]
> != '\n'; i--)
> ; /* do nothing */
>
> - if (i)
> - has_footer = has_conforming_footer(msgbuf, &sob, ignore_footer);
> -
> - if (!has_footer)
> - strbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0, "\n", 1);
> + if (msgbuf->buf[i] != '\n') {
> + if (i)
> + has_footer = has_conforming_footer(msgbuf, &sob,
> + ignore_footer);
> +
> + if (!has_footer)
> + strbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0,
> + "\n", 1);
> + }
>
> if (has_footer != 3 && (!no_dup_sob || has_footer != 2))
This is too much nesting for my small mind to keep track of. Am I
correct in imagining the effect is the same as the following?
has_footer = has_conforming_footer(...)
if (!has_footer && msgbuf->buf[i] != '\n')
strbuf_splice(...); /* add blank line */
if (has_footer != 3 && ...
--
To unsubscribe from this list: send the line "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 07/11] sequencer.c: teach append_signoff how to detect duplicate s-o-b
Brandon Casey wrote:
> --- a/sequencer.c
> +++ b/sequencer.c
[...]
> @@ -1096,10 +1117,16 @@ 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 || !has_conforming_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 = has_conforming_footer(msgbuf, &sob, ignore_footer);
Is this "if (i)" test needed? I'd expect that if the message has no newlines,
has_conforming_footer() would notice that and 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
Re: [PATCH v3 07/11] sequencer.c: teach append_signoff how to detect duplicate s-o-b
Brandon Casey wrote: > Teach append_signoff how to detect a duplicate s-o-b in the commit footer. This replaces the previous (slightly broken) logic that checked whether the sign-off to be appended would be redundant and puts the fixed logic further down the call-chain next to the rest of footer parsing. I am not thrilled with the 0 = no rfc-style footer 1 = rfc-style footer, no sign-off found 2 = rfc-style footer, sign-off found but not as last field 3 = rfc-style footer, sign-off found as last field convention but since it's local to sequencer.c and this logic to scan all lines for the sign-off can probably be ripped out soon I don't mind. The general direction is good and the execution looks obviously correct. 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: [PATCH v2 0/4] Auto-generate mergetool lists
David Aguilar writes: > Okay, cool, so no need to reroll, ya? It was more like "please don't switch to incremental yet"; I tweaked the mode_ok in your v2 and pushed out the result on 'pu' again. There may later be comments from others that make us realize some patches need to be rerolled, but nothing from me for now. 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 v3 05/11] sequencer.c: recognize "(cherry picked from ..." as part of s-o-b footer
Jonathan Nieder wrote:
> Here's the tweak I suggested last time. I think its behavior is
> slightly better in the "ends with incomplete line" case because it
> limits the characters examined by is_rfc2822_line() and
> is_cherry_picked_from_line() not to include buf[len] (which would
> presumably sometimes be '\0').
Whoops, that revealed a subtlety --- the '\n' or '\0' is what prevents
exiting the loop in is_rfc2822_line when the line does not contain a
colon. Here's a corrected version of the tweak, that should actually
work. :)
diff --git i/sequencer.c w/sequencer.c
index 0b5cd18c..108ea27b 100644
--- i/sequencer.c
+++ w/sequencer.c
@@ -1029,13 +1029,11 @@ static int is_rfc2822_line(const char *buf, int len)
for (i = 0; i < len; i++) {
int ch = buf[i];
if (ch == ':')
+ return 1;
+ if (!isalnum(ch) && ch != '-')
break;
- if (isalnum(ch) || (ch == '-'))
- continue;
- return 0;
}
-
- return 1;
+ return 0;
}
static int is_cherry_picked_from_line(const char *buf, int len)
@@ -1043,9 +1041,7 @@ static int is_cherry_picked_from_line(const char *buf,
int len)
/*
* We only care that it looks roughly like (cherry picked from ...)
*/
- return !prefixcmp(buf, cherry_picked_prefix) &&
- (buf[len - 1] == ')' ||
-(buf[len - 1] == '\n' && buf[len - 2] == ')'));
+ return !prefixcmp(buf, cherry_picked_prefix) && buf[len - 1] == ')';
}
static int has_conforming_footer(struct strbuf *sb, int ignore_footer)
@@ -1072,8 +1068,8 @@ static int has_conforming_footer(struct strbuf *sb, int
ignore_footer)
; /* do nothing */
k++;
- if (!(is_rfc2822_line(buf + i, k - i) ||
- is_cherry_picked_from_line(buf + i, k - i)))
+ if (!is_rfc2822_line(buf + i, k - i - 1) &&
+ !is_cherry_picked_from_line(buf + i, k - i - 1))
return 0;
}
return 1;
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/4] Auto-generate mergetool lists
On Sun, Jan 27, 2013 at 6:27 PM, Junio C Hamano wrote: > David Aguilar writes: > >> On Sun, Jan 27, 2013 at 6:08 PM, Junio C Hamano wrote: >>> I think our works crossed, while I was tweaking the previous series >>> to push out as part of 'pu' you were already rerolling. Could you >>> compare this series with what I pushed out and see if anything you >>> missed? I think I fixed the (a && b || c && d) issue in the version >>> I pushed out, but it is still there in this series. >> >> Ah, I see. >> >> I can add the addition of preamble for use by show_tool_help() >> as a follow up along with using a here-doc when printing. > > I think the progression of the series is just fine as-is with the > new series you posted (I didn't amend the old one with all the > suggestions I made in the review, just only with the more important > ones that would affect correctness, so please consider that the > changes you have in this new round that I didn't have in 'pu' are > good ones to keep. Okay, cool, so no need to reroll, ya? The one thing missing in my latest series was the fixed mode_ok() which you corrected in cfb611b34089a0b5794f4ec453289a4764d94050. Let me know if there's anything else I should send out or splice together. John, I didn't completely address your question about keeping the sort and prefix in show_tool_help() but I can stop poking at it now in case you want to start looking at what it would take to get custom tools listed in the --tool-help output. -- David -- To unsubscribe from this list: send the line "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 06/11] sequencer.c: always separate "(cherry picked from" from commit body
Brandon Casey wrote:
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -20,6 +20,67 @@
[...]
> static int has_conforming_footer(struct strbuf *sb, int ignore_footer)
[...]
> + /* require at least one blank line */
> + if (!last_char_was_nl || buf[i] != '\n')
> + return 0;
Makes sense. append_signoff already added a blank line after a
one-line message
foo: bar
because of e5138436 (builtin-commit.c: fix logic to omit empty line
before existing footers, 2009-11-06) but the logic to do so was in the
wrong place and it didn't handle its ill-formatted cousin
foo: bar
baz: qux
[...]
> @@ -497,6 +558,8 @@ static int do_pick_commit(struct commit *commit, struct
> replay_opts *opts)
> }
>
> if (opts->record_origin) {
> + if (!has_conforming_footer(&msgbuf, 0))
> + strbuf_addch(&msgbuf, '\n');
What should this do in the case of an entirely blank message?
(Maybe that's too insane a case to worry about.)
[...]
> --- a/t/t3511-cherry-pick-x.sh
> +++ b/t/t3511-cherry-pick-x.sh
> @@ -81,6 +94,19 @@ test_expect_failure 'cherry-pick -s inserts blank line
> after non-conforming foot
> test_cmp expect actual
> '
>
> +test_expect_success 'cherry-pick -x inserts blank line when conforming
> footer not found' '
Yay. :)
Thanks for a clear and well thought-out patch with thorough tests.
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: [PATCH v2 0/4] Auto-generate mergetool lists
David Aguilar writes: > On Sun, Jan 27, 2013 at 6:08 PM, Junio C Hamano wrote: >> I think our works crossed, while I was tweaking the previous series >> to push out as part of 'pu' you were already rerolling. Could you >> compare this series with what I pushed out and see if anything you >> missed? I think I fixed the (a && b || c && d) issue in the version >> I pushed out, but it is still there in this series. > > Ah, I see. > > I can add the addition of preamble for use by show_tool_help() > as a follow up along with using a here-doc when printing. I think the progression of the series is just fine as-is with the new series you posted (I didn't amend the old one with all the suggestions I made in the review, just only with the more important ones that would affect correctness, so please consider that the changes you have in this new round that I didn't have in 'pu' are good ones to keep. -- To unsubscribe from this list: send the line "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 05/11] sequencer.c: recognize "(cherry picked from ..." as part of s-o-b footer
Brandon Casey wrote: > Let's detect "(cherry picked from...)" as part of the footer so that we > will produce this: > >Signed-off-by: A U Thor >(cherry picked from da39a3ee5e6b4b0d3255bfef95601890afd80709) >Signed-off-by: C O Mmitter > > instead of this: > >Signed-off-by: A U Thor >(cherry picked from da39a3ee5e6b4b0d3255bfef95601890afd80709) > >Signed-off-by: C O Mmitter Here's the tweak I suggested last time. I think its behavior is slightly better in the "ends with incomplete line" case because it limits the characters examined by is_rfc2822_line() and is_cherry_picked_from_line() not to include buf[len] (which would presumably sometimes be '\0'). diff --git i/sequencer.c w/sequencer.c index 0b5cd18c..fa29c7c5 100644 --- i/sequencer.c +++ w/sequencer.c @@ -1043,9 +1043,7 @@ static int is_cherry_picked_from_line(const char *buf, int len) /* * We only care that it looks roughly like (cherry picked from ...) */ - return !prefixcmp(buf, cherry_picked_prefix) && - (buf[len - 1] == ')' || -(buf[len - 1] == '\n' && buf[len - 2] == ')')); + return !prefixcmp(buf, cherry_picked_prefix) && buf[len - 1] == ')'; } static int has_conforming_footer(struct strbuf *sb, int ignore_footer) @@ -1072,8 +1070,8 @@ static int has_conforming_footer(struct strbuf *sb, int ignore_footer) ; /* do nothing */ k++; - if (!(is_rfc2822_line(buf + i, k - i) || - is_cherry_picked_from_line(buf + i, k - i))) + if (!is_rfc2822_line(buf + i, k - i - 1) && + !is_cherry_picked_from_line(buf + i, k - i - 1)) return 0; } return 1; -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/4] Auto-generate mergetool lists
On Sun, Jan 27, 2013 at 6:08 PM, Junio C Hamano wrote: > I think our works crossed, while I was tweaking the previous series > to push out as part of 'pu' you were already rerolling. Could you > compare this series with what I pushed out and see if anything you > missed? I think I fixed the (a && b || c && d) issue in the version > I pushed out, but it is still there in this series. Ah, I see. I can add the addition of preamble for use by show_tool_help() as a follow up along with using a here-doc when printing. The other diff is the Makefile dependencies. I currently have this diff against da/mergetool-docs: diff --git a/Documentation/Makefile b/Documentation/Makefile index f595d26..834ec25 100644 --- a/Documentation/Makefile +++ b/Documentation/Makefile @@ -202,7 +202,11 @@ install-html: html # # Determine "include::" file references in asciidoc files. # -doc.dep : $(wildcard *.txt) build-docdep.perl +docdep_prereqs = \ + mergetools-list.made $(mergetools_txt) \ + cmd-list.made $(cmds_txt) + +doc.dep : $(docdep_prereqs) $(wildcard *.txt) build-docdep.perl $(QUIET_GEN)$(RM) $@+ $@ && \ $(PERL_PATH) ./build-docdep.perl >$@+ $(QUIET_STDERR) && \ mv $@+ $@ I'll send three follow-up patches. - here-doc, preamble stuff, and Makefile deps, based on what's currently in pu. -- David -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 4/4] doc: Generate a list of valid merge tools
David Aguilar writes: > Use the show_tool_names() function to build lists of all > the built-in tools supported by difftool and mergetool. > This frees us from needing to update the documentation > whenever a new tool is added. > > Signed-off-by: David Aguilar > --- > Adjusted to use show_tool_names() and reworked the makefile dependencies. > I could use another set of eyes on the Makefile.. Looks good from a quick scan and comparison with the previous round. Thanks for a quick reroll. > Documentation/.gitignore | 1 + > Documentation/Makefile | 22 -- > Documentation/diff-config.txt | 13 +++-- > Documentation/merge-config.txt | 12 ++-- > git-mergetool--lib.sh | 3 ++- > 5 files changed, 36 insertions(+), 15 deletions(-) > > diff --git a/Documentation/.gitignore b/Documentation/.gitignore > index d62aebd..2c8b2d6 100644 > --- a/Documentation/.gitignore > +++ b/Documentation/.gitignore > @@ -9,4 +9,5 @@ gitman.info > howto-index.txt > doc.dep > cmds-*.txt > +mergetools-*.txt > manpage-base-url.xsl > diff --git a/Documentation/Makefile b/Documentation/Makefile > index 267dfe1..834ec25 100644 > --- a/Documentation/Makefile > +++ b/Documentation/Makefile > @@ -202,7 +202,11 @@ install-html: html > # > # Determine "include::" file references in asciidoc files. > # > -doc.dep : $(wildcard *.txt) build-docdep.perl > +docdep_prereqs = \ > + mergetools-list.made $(mergetools_txt) \ > + cmd-list.made $(cmds_txt) > + > +doc.dep : $(docdep_prereqs) $(wildcard *.txt) build-docdep.perl > $(QUIET_GEN)$(RM) $@+ $@ && \ > $(PERL_PATH) ./build-docdep.perl >$@+ $(QUIET_STDERR) && \ > mv $@+ $@ > @@ -226,13 +230,27 @@ cmd-list.made: cmd-list.perl ../command-list.txt > $(MAN1_TXT) > $(PERL_PATH) ./cmd-list.perl ../command-list.txt $(QUIET_STDERR) && \ > date >$@ > > +mergetools_txt = mergetools-diff.txt mergetools-merge.txt > + > +$(mergetools_txt): mergetools-list.made > + > +mergetools-list.made: ../git-mergetool--lib.sh $(wildcard ../mergetools/*) > + $(QUIET_GEN)$(RM) $@ && \ > + $(SHELL_PATH) -c 'MERGE_TOOLS_DIR=../mergetools && \ > + . ../git-mergetool--lib.sh && \ > + show_tool_names can_diff "* "' > mergetools-diff.txt && \ > + $(SHELL_PATH) -c 'MERGE_TOOLS_DIR=../mergetools && \ > + . ../git-mergetool--lib.sh && \ > + show_tool_names can_merge "* "' > mergetools-merge.txt && \ > + date > $@ > + > clean: > $(RM) *.xml *.xml+ *.html *.html+ *.1 *.5 *.7 > $(RM) *.texi *.texi+ *.texi++ git.info gitman.info > $(RM) *.pdf > $(RM) howto-index.txt howto/*.html doc.dep > $(RM) technical/api-*.html technical/api-index.txt > - $(RM) $(cmds_txt) *.made > + $(RM) $(cmds_txt) $(mergetools_txt) *.made > $(RM) manpage-base-url.xsl > > $(MAN_HTML): %.html : %.txt > diff --git a/Documentation/diff-config.txt b/Documentation/diff-config.txt > index 67a90a8..7c968d1 100644 > --- a/Documentation/diff-config.txt > +++ b/Documentation/diff-config.txt > @@ -132,9 +132,10 @@ diff..cachetextconv:: > conversion outputs. See linkgit:gitattributes[5] for details. > > diff.tool:: > - The diff tool to be used by linkgit:git-difftool[1]. This > - option overrides `merge.tool`, and has the same valid built-in > - values as `merge.tool` minus "tortoisemerge" and plus > - "kompare". Any other value is treated as a custom diff tool, > - and there must be a corresponding `difftool..cmd` > - option. > + Controls which diff tool is used by linkgit:git-difftool[1]. > + This variable overrides the value configured in `merge.tool`. > + The list below shows the valid built-in values. > + Any other value is treated as a custom diff tool and requires > + that a corresponding difftool..cmd variable is defined. > + > +include::mergetools-diff.txt[] > diff --git a/Documentation/merge-config.txt b/Documentation/merge-config.txt > index 861bd6f..5f40e71 100644 > --- a/Documentation/merge-config.txt > +++ b/Documentation/merge-config.txt > @@ -52,12 +52,12 @@ merge.stat:: > at the end of the merge. True by default. > > merge.tool:: > - Controls which merge resolution program is used by > - linkgit:git-mergetool[1]. Valid built-in values are: "araxis", > - "bc3", "diffuse", "ecmerge", "emerge", "gvimdiff", "kdiff3", "meld", > - "opendiff", "p4merge", "tkdiff", "tortoisemerge", "vimdiff" > - and "xxdiff". Any other value is treated is custom merge tool > - and there must be a corresponding mergetool..cmd option. > + Controls which merge tool is used by linkgit:git-mergetool[1]. > + The list below shows the valid built-in values. > + Any other value is treated as a custom merge tool and requires > + that a corresponding mergetool..cmd variable is defined. > + > +include::mergetools-merge.txt[] > > merge.verbosity:: > Controls th
Re: [PATCH v2 0/4] Auto-generate mergetool lists
I think our works crossed, while I was tweaking the previous series to push out as part of 'pu' you were already rerolling. Could you compare this series with what I pushed out and see if anything you missed? I think I fixed the (a && b || c && d) issue in the version I pushed out, but it is still there in this series. -- To unsubscribe from this list: send the line "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 04/11] t/t3511: add some tests of 'cherry-pick -s' functionality
Brandon Casey wrote:
> --- /dev/null
> +++ b/t/t3511-cherry-pick-x.sh
> @@ -0,0 +1,111 @@
> +#!/bin/sh
> +
> +test_description='Test cherry-pick -x and -s'
> +
> +. ./test-lib.sh
> +
> +pristine_detach () {
> + git cherry-pick --quit &&
> + git checkout -f "$1^0" &&
> + git read-tree -u --reset HEAD &&
> + git clean -d -f -f -q -x
> +}
Some day this should move to test-lib-functions.sh. Not relevant
for this patch, though.
[...]
> +test_expect_failure 'cherry-pick -s inserts blank line after non-conforming
> footer' '
> + pristine_detach initial &&
> + git cherry-pick -s mesg-broken-footer &&
> + cat <<-EOF >expect &&
> + $mesg_broken_footer
> +
> + Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>
This bug is old. When c1e01b0c (commit: More generous accepting of
RFC-2822 footer lines, 2009-10-28) added more precise parsing of the
RFC2822 footer when deciding whether to add a newline before a new
signoff, it forgot to change the "sign-off already present" case to
match.
Thanks for the test. The rest of the tests in this file also look
good, of course.
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: [PATCH v2 06/10] sequencer.c: teach append_signoff how to detect duplicate s-o-b
Brandon Casey writes: > On Tue, Jan 22, 2013 at 12:38 AM, Jonathan Nieder wrote: >> Brandon Casey wrote: >> >>> Teach append_signoff how to detect a duplicate s-o-b in the commit footer. >>> This is in preparation to unify the append_signoff implementations in >>> log-tree.c and sequencer.c. >> [...] >>> --- a/sequencer.c >>> +++ b/sequencer.c >>> @@ -1082,9 +1101,10 @@ int sequencer_pick_revisions(struct replay_opts >>> *opts) >>> return pick_commits(todo_list, opts); >>> } >>> >>> -void append_signoff(struct strbuf *msgbuf, int ignore_footer) >>> +void append_signoff(struct strbuf *msgbuf, int ignore_footer, int >>> no_dup_sob) >> >> Isn't the behavior of passing '1' here just a bug in "format-patch -s"? > > I think that is an open question. Yes. as I said in a previous review round, I think it was a mistake that format-patch chose to pay attention to any S-o-b in the patch trail, not only the last one, and we may want to correcct it once this series solidifies as a separate "bugfix" change on top. -- To unsubscribe from this list: send the line "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 02/11] commit, cherry-pick -s: remove broken support for multiline rfc2822 fields
Brandon Casey wrote: > sequencer.c | 6 -- > 1 file changed, 6 deletions(-) Nice simplification. 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: [PATCH v3 01/11] sequencer.c: rework search for start of footer to improve clarity
Brandon Casey wrote:
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -1024,16 +1024,19 @@ int sequencer_pick_revisions(struct replay_opts *opts)
> static int ends_rfc2822_footer(struct strbuf *sb, int ignore_footer)
> {
> int ch;
> - int hit = 0;
> + int last_char_was_nl, this_char_is_nl;
> int i, j, k;
> int len = sb->len - ignore_footer;
> int first = 1;
> const char *buf = sb->buf;
>
> + /* find start of last paragraph */
> + last_char_was_nl = 0;
> for (i = len - 1; i > 0; i--) {
> - if (hit && buf[i] == '\n')
> + this_char_is_nl = (buf[i] == '\n');
> + if (last_char_was_nl && this_char_is_nl)
> break;
> - hit = (buf[i] == '\n');
> + last_char_was_nl = this_char_is_nl;
I would have been tempted to write
char prev;
prev = 0;
for (i = len - 1; i > 0; i--) {
char ch = buf[i];
if (prev == '\n' && ch == '\n') /* paragraph break */
break;
prev = ch;
}
but your rewrite is just as clear. For what it's worth,
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: [PATCH v2 04/10] sequencer.c: recognize "(cherry picked from ..." as part of s-o-b footer
Hi,
Brandon Casey wrote:
> Let's detect "(cherry picked from...)" as part of the footer so that we
> will produce this:
>
>Signed-off-by: A U Thor
>(cherry picked from da39a3ee5e6b4b0d3255bfef95601890afd80709)
>Signed-off-by: C O Mmitter
>
> instead of this:
>
>Signed-off-by: A U Thor
>(cherry picked from da39a3ee5e6b4b0d3255bfef95601890afd80709)
>
>Signed-off-by: C O Mmitter
My last review was based on a misunderstanding of
ends_rfc2822_footer(). This time I can unreservedly say I like this.
[...]
> --- 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 ";
The static/nonstatic mismatch looks strange. Not about this patch,
but probably rest_is_empty() from builtin-commit.c should move here
some day.
[...]
> @@ -1021,11 +1022,36 @@ int sequencer_pick_revisions(struct replay_opts *opts)
[...]
> +static int is_cherry_picked_from_line(const char *buf, int len)
> +{
> + /*
> + * We only care that it looks roughly like (cherry picked from ...)
> + */
> + return !prefixcmp(buf, cherry_picked_prefix) &&
> + (buf[len - 1] == ')' ||
> + (buf[len - 1] == '\n' && buf[len - 2] == ')'));
These two cases are based on whether the commit message ended with
a '(cherry picked from' with no trailing newline, I guess?
I wonder if switching the convention for 'k' would make this simpler:
const char *line, *eol;
line = buf + i;
eol = memchr(buf, '\n', len - i);
if (!eol)
eol = buf + len;
if (!is_rfc2822_field(line, eol - line) &&
!is_cherry_picked_from_line(line, eol - line))
return 0;
I notice you just sent a new version of the series. I'll try this out
on top of that.
Thanks,
Jonathan
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 4/4] branch: mark more strings for translation
Reviewed-by: Jonathan Nieder
Signed-off-by: Nguyễn Thái Ngọc Duy
---
On Sun, Jan 27, 2013 at 6:55 PM, Jonathan Nieder wrote:
> For what it's worth, assuming this passes tests,
It passes the tests. Although I doubt the tests are written to catch
this.
builtin/branch.c | 11 ++-
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/builtin/branch.c b/builtin/branch.c
index ca61c5b..597b578 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -466,7 +466,7 @@ static void add_verbose_info(struct strbuf *out, struct
ref_item *item,
int verbose, int abbrev)
{
struct strbuf subject = STRBUF_INIT, stat = STRBUF_INIT;
- const char *sub = " invalid ref ";
+ const char *sub = _(" invalid ref ");
struct commit *commit = item->commit;
if (commit && !parse_commit(commit)) {
@@ -590,7 +590,7 @@ static int print_ref_list(int kinds, int detached, int
verbose, int abbrev, stru
struct commit *filter;
filter = lookup_commit_reference_gently(merge_filter_ref, 0);
if (!filter)
- die("object '%s' does not point to a commit",
+ die(_("object '%s' does not point to a commit"),
sha1_to_hex(merge_filter_ref));
filter->object.flags |= UNINTERESTING;
@@ -854,7 +854,7 @@ int cmd_branch(int argc, const char **argv, const char
*prefix)
if (!argc) {
if (detached)
- die("Cannot give description to detached HEAD");
+ die(_("Cannot give description to detached
HEAD"));
branch_name = head;
} else if (argc == 1)
branch_name = argv[0];
@@ -866,10 +866,11 @@ int cmd_branch(int argc, const char **argv, const char
*prefix)
strbuf_release(&branch_ref);
if (!argc)
- return error("No commit on branch '%s' yet.",
+ return error(_("No commit on branch '%s' yet."),
branch_name);
else
- return error("No such branch '%s'.",
branch_name);
+ return error(_("No branch named '%s'."),
+branch_name);
}
strbuf_release(&branch_ref);
--
1.8.1.1.459.g5970e58
--
To unsubscribe from this list: send the line "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 v3 1/4] branch: no detached HEAD check when editing another branch's description
Signed-off-by: Nguyễn Thái Ngọc Duy
---
Split out from v3's 2/3 as Jonathan suggested.
builtin/branch.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/builtin/branch.c b/builtin/branch.c
index 873f624..ea6498b 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -850,11 +850,11 @@ int cmd_branch(int argc, const char **argv, const char
*prefix)
const char *branch_name;
struct strbuf branch_ref = STRBUF_INIT;
- if (detached)
- die("Cannot give description to detached HEAD");
- if (!argc)
+ if (!argc) {
+ if (detached)
+ die("Cannot give description to detached HEAD");
branch_name = head;
- else if (argc == 1)
+ } else if (argc == 1)
branch_name = argv[0];
else
usage_with_options(builtin_branch_usage, options);
--
1.8.1.1.459.g5970e58
--
To unsubscribe from this list: send the line "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 v3 2/4] branch: reject -D/-d without branch name
Signed-off-by: Nguyễn Thái Ngọc Duy
---
builtin/branch.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/builtin/branch.c b/builtin/branch.c
index ea6498b..30c4545 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -837,9 +837,11 @@ int cmd_branch(int argc, const char **argv, const char
*prefix)
colopts = 0;
}
- if (delete)
+ if (delete) {
+ if (!argc)
+ die(_("branch name required"));
return delete_branches(argc, argv, delete > 1, kinds, quiet);
- else if (list) {
+ } else if (list) {
int ret = print_ref_list(kinds, detached, verbose, abbrev,
with_commit, argv);
print_columns(&output, colopts, NULL);
--
1.8.1.1.459.g5970e58
--
To unsubscribe from this list: send the line "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 v3 3/4] branch: give a more helpful message on redundant arguments
Signed-off-by: Nguyễn Thái Ngọc Duy
---
builtin/branch.c | 4 ++--
t/t3200-branch.sh | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/builtin/branch.c b/builtin/branch.c
index 30c4545..ca61c5b 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -859,7 +859,7 @@ int cmd_branch(int argc, const char **argv, const char
*prefix)
} else if (argc == 1)
branch_name = argv[0];
else
- usage_with_options(builtin_branch_usage, options);
+ die(_("cannot edit description of more than one
branch"));
strbuf_addf(&branch_ref, "refs/heads/%s", branch_name);
if (!ref_exists(branch_ref.buf)) {
@@ -881,7 +881,7 @@ int cmd_branch(int argc, const char **argv, const char
*prefix)
else if (argc == 2)
rename_branch(argv[0], argv[1], rename > 1);
else
- usage_with_options(builtin_branch_usage, options);
+ die(_("too many branches for a rename operation"));
} else if (new_upstream) {
struct branch *branch = branch_get(argv[0]);
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index 80e6be3..f3e0e4a 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -73,8 +73,8 @@ test_expect_success \
test_expect_success \
'git branch -m dumps usage' \
- 'test_expect_code 129 git branch -m 2>err &&
- test_i18ngrep "[Uu]sage: git branch" err'
+ 'test_expect_code 128 git branch -m 2>err &&
+ test_i18ngrep "too many branches for a rename operation" err'
test_expect_success \
'git branch -m m m/m should work' \
--
1.8.1.1.459.g5970e58
--
To unsubscribe from this list: send the line "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 v3 10/11] format-patch: update append_signoff prototype
From: Nguyễn Thái Ngọc Duy
This is a preparation step for merging with append_signoff from
sequencer.c
Signed-off-by: Nguyễn Thái Ngọc Duy
Signed-off-by: Brandon Casey
---
builtin/log.c | 13 +
log-tree.c| 17 +
revision.h| 2 +-
3 files changed, 15 insertions(+), 17 deletions(-)
diff --git a/builtin/log.c b/builtin/log.c
index 8f0b2e8..59de484 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1086,7 +1086,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;
@@ -1193,16 +1192,6 @@ int cmd_format_patch(int argc, const char **argv, const
char *prefix)
rev.subject_prefix = strbuf_detach(&sprefix, NULL);
}
- 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');
@@ -1393,7 +1382,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 5dc45c4..ac1cd68 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -10,6 +10,8 @@
#include "color.h"
#include "gpg-interface.h"
+#define APPEND_SIGNOFF_DEDUP (1u <<0)
+
struct decoration name_decoration = { "object names" };
enum decoration_type {
@@ -253,9 +255,12 @@ static int detect_any_signoff(char *letter, int size)
return seen_head && seen_name;
}
-static void append_signoff(struct strbuf *sb, const char *signoff)
+static void append_signoff(struct strbuf *sb, int ignore_footer, unsigned flag)
{
+ unsigned no_dup_sob = flag & APPEND_SIGNOFF_DEDUP;
static const char signed_off_by[] = "Signed-off-by: ";
+ char *signoff = xstrdup(fmt_name(getenv("GIT_COMMITTER_NAME"),
+ getenv("GIT_COMMITTER_EMAIL")));
size_t signoff_len = strlen(signoff);
int has_signoff = 0;
char *cp;
@@ -275,6 +280,7 @@ static void append_signoff(struct strbuf *sb, const char
*signoff)
if (!isspace(cp[signoff_len]))
continue;
/* we already have him */
+ free(signoff);
return;
}
@@ -287,6 +293,7 @@ static void append_signoff(struct strbuf *sb, const char
*signoff)
strbuf_addstr(sb, signed_off_by);
strbuf_add(sb, signoff, signoff_len);
strbuf_addch(sb, '\n');
+ free(signoff);
}
static unsigned int digits_in_number(unsigned int number)
@@ -672,8 +679,10 @@ void show_log(struct rev_info *opt)
/*
* And then the pretty-printed message itself
*/
- if (ctx.need_8bit_cte >= 0)
- ctx.need_8bit_cte = has_non_ascii(opt->add_signoff);
+ if (ctx.need_8bit_cte >= 0 && opt->add_signoff)
+ ctx.need_8bit_cte =
+ has_non_ascii(fmt_name(getenv("GIT_COMMITTER_NAME"),
+ getenv("GIT_COMMITTER_EMAIL")));
ctx.date_mode = opt->date_mode;
ctx.date_mode_explicit = opt->date_mode_explicit;
ctx.abbrev = opt->diffopt.abbrev;
@@ -686,7 +695,7 @@ void show_log(struct rev_info *opt)
pretty_print_commit(&ctx, commit, &msgbuf);
if (opt->add_signoff)
- append_signoff(&msgbuf, opt->add_signoff);
+ append_signoff(&msgbuf, 0, APPEND_SIGNOFF_DEDUP);
if ((ctx.fmt != CMIT_FMT_USERFORMAT) &&
ctx.notes_message && *ctx.notes_message) {
diff --git a/revision.h b/revision.h
index 5da09ee..01bd2b7 100644
--- a/revision.h
+++ b/revision.h
@@ -138,7 +138,7 @@ struct rev_info {
int reroll_count;
char*message_id;
struct string_list *ref_message_ids;
- const char *add_signoff;
+ int add_signoff;
const char *extra_headers;
const char *log_reencode;
const char *subject_prefix;
--
1.8.1.1.450.g0327af3
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.h
[PATCH v3 11/11] 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. Unify on top of the
sequencer.c implementation.
Add a test in t4014 to demonstrate support for non-s-o-b elements in the
commit footer provided by sequence.c:append_sob. Mark tests fixed as
appropriate.
[Commit message mostly stolen from Nguyễn Thái Ngọc Duy's original
unification patch]
Signed-off-by: Brandon Casey
---
log-tree.c | 91 +
t/t4014-format-patch.sh | 31 ++---
2 files changed, 27 insertions(+), 95 deletions(-)
diff --git a/log-tree.c b/log-tree.c
index ac1cd68..c9d9a37 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -9,8 +9,7 @@
#include "string-list.h"
#include "color.h"
#include "gpg-interface.h"
-
-#define APPEND_SIGNOFF_DEDUP (1u <<0)
+#include "sequencer.h"
struct decoration name_decoration = { "object names" };
@@ -208,94 +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, int ignore_footer, unsigned flag)
-{
- unsigned no_dup_sob = flag & APPEND_SIGNOFF_DEDUP;
- static const char signed_off_by[] = "Signed-off-by: ";
- char *signoff = xstrdup(fmt_name(getenv("GIT_COMMITTER_NAME"),
- getenv("GIT_COMMITTER_EMAIL")));
- 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]))
- continue;
- /* we already have him */
- free(signoff);
- return;
- }
-
- if (!has_signoff)
- has_signoff = detect_any_signoff(sb->buf, sb->len);
-
- if (!has_signoff)
- strbuf_addch(sb, '\n');
-
- strbuf_addstr(sb, signed_off_by);
- strbuf_add(sb, signoff, signoff_len);
- strbuf_addch(sb, '\n');
- free(signoff);
-}
-
static unsigned int digits_in_number(unsigned int number)
{
unsigned int i = 10, result = 1;
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index 3868cef..d0ec097 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -1104,7 +1104,28 @@ EOF
test_cmp expected actual
'
-test_expect_failure 'signoff: some random signoff-alike' '
+test_expect_success 'signoff: misc conforming footer elements' '
+ append_signoff <<\EOF >actual &&
+subject
+
+body
+
+Signed-off-by: my@house
+(cherry picked from commit da39a3ee5e6b4b0d3255bfef95601890afd80709)
+Tested-by: Some One
+Bug: 1234
+EOF
+ cat >expected <<\EOF &&
+4:Subject: [PATCH] subject
+8:
+10:
+11:Signed-off-by: my@house
+15:Signed-off-by: C O Mitter
+EOF
+ test_cmp expected actual
+'
+
+test_expect_success 'signoff: some random signoff-alike' '
append_signoff <<\EOF >actual &&
subject
@@ -1120,7 +1141,7 @@ EOF
test_cmp expected actual
'
-test_expect_failure 'signoff: not really a signoff' '
+test_expect_success 'signoff: not really a signoff' '
append_signoff <<\EOF >actual &&
subject
@@ -1136,7 +1157,7 @@ EOF
test_cm
[PATCH v3 08/11] sequencer.c: teach append_signoff to avoid adding a duplicate newline
Teach append_signoff to detect whether a blank line exists at the position
that the signed-off-by line will be added, and avoid adding an additional
one if one already exists. This is necessary to allow format-patch to add a
s-o-b to a patch with no commit message without adding an extra newline. A
following patch will make format-patch use this function rather than the
append_signoff implementation inside log-tree.c.
Signed-off-by: Brandon Casey
---
sequencer.c | 14 +-
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index 015cb58..404b786 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -1118,11 +1118,15 @@ void append_signoff(struct strbuf *msgbuf, int
ignore_footer, unsigned flag)
for (i = msgbuf->len - 1 - ignore_footer; i > 0 && msgbuf->buf[i - 1]
!= '\n'; i--)
; /* do nothing */
- if (i)
- has_footer = has_conforming_footer(msgbuf, &sob, ignore_footer);
-
- if (!has_footer)
- strbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0, "\n", 1);
+ if (msgbuf->buf[i] != '\n') {
+ if (i)
+ has_footer = has_conforming_footer(msgbuf, &sob,
+ ignore_footer);
+
+ if (!has_footer)
+ strbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0,
+ "\n", 1);
+ }
if (has_footer != 3 && (!no_dup_sob || has_footer != 2))
strbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0,
--
1.8.1.1.450.g0327af3
--
To unsubscribe from this list: send the line "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 v3 09/11] t4014: more tests about appending s-o-b lines
From: Nguyễn Thái Ngọc Duy
[bc: Squash the tests from Duy's original unify-appending-sob series.
Fix test 90 "signoff: some random signoff-alike" and mark as failing.
Correct behavior should insert a blank line after message body and
signed-off-by.
Add two additional tests:
1. failure to detect non-conforming elements in the footer when last
line matches committer's s-o-b.
2. ensure various s-o-b -like elements in the footer are handled as
conforming. e.g. "Change-id: I or Bug: 1234"
]
Signed-off-by: Nguyễn Thái Ngọc Duy
Signed-off-by: Brandon Casey
---
t/t4014-format-patch.sh | 242
1 file changed, 242 insertions(+)
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index 7fa3647..3868cef 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -1021,4 +1021,246 @@ test_expect_success 'cover letter using branch
description (6)' '
grep hello actual >/dev/null
'
+append_signoff()
+{
+ C=`git commit-tree HEAD^^{tree} -p HEAD` &&
+ git format-patch --stdout --signoff ${C}^..${C} |
+ tee append_signoff.patch |
+ sed -n "1,/^---$/p" |
+ grep -n -E "^Subject|Sign|^$"
+}
+
+test_expect_success 'signoff: commit with no body' '
+ append_signoff actual &&
+ cat <<\EOF | sed "s/EOL$//" >expected &&
+4:Subject: [PATCH] EOL
+8:
+9:Signed-off-by: C O Mitter
+EOF
+ test_cmp expected actual
+'
+
+test_expect_success 'signoff: commit with only subject' '
+ echo subject | append_signoff >actual &&
+ cat >expected <<\EOF &&
+4:Subject: [PATCH] subject
+8:
+9:Signed-off-by: C O Mitter
+EOF
+ test_cmp expected actual
+'
+
+test_expect_success 'signoff: commit with only subject that does not end with
NL' '
+ printf subject | append_signoff >actual &&
+ cat >expected <<\EOF &&
+4:Subject: [PATCH] subject
+8:
+9:Signed-off-by: C O Mitter
+EOF
+ test_cmp expected actual
+'
+
+test_expect_success 'signoff: no existing signoffs' '
+ append_signoff <<\EOF >actual &&
+subject
+
+body
+EOF
+ cat >expected <<\EOF &&
+4:Subject: [PATCH] subject
+8:
+10:
+11:Signed-off-by: C O Mitter
+EOF
+ test_cmp expected actual
+'
+
+test_expect_success 'signoff: no existing signoffs and no trailing NL' '
+ printf "subject\n\nbody" | append_signoff >actual &&
+ cat >expected <<\EOF &&
+4:Subject: [PATCH] subject
+8:
+10:
+11:Signed-off-by: C O Mitter
+EOF
+ test_cmp expected actual
+'
+
+test_expect_success 'signoff: some random signoff' '
+ append_signoff <<\EOF >actual &&
+subject
+
+body
+
+Signed-off-by: my@house
+EOF
+ cat >expected <<\EOF &&
+4:Subject: [PATCH] subject
+8:
+10:
+11:Signed-off-by: my@house
+12:Signed-off-by: C O Mitter
+EOF
+ test_cmp expected actual
+'
+
+test_expect_failure 'signoff: some random signoff-alike' '
+ append_signoff <<\EOF >actual &&
+subject
+
+body
+Fooled-by-me: my@house
+EOF
+ cat >expected <<\EOF &&
+4:Subject: [PATCH] subject
+8:
+11:
+12:Signed-off-by: C O Mitter
+EOF
+ test_cmp expected actual
+'
+
+test_expect_failure 'signoff: not really a signoff' '
+ append_signoff <<\EOF >actual &&
+subject
+
+I want to mention about Signed-off-by: here.
+EOF
+ cat >expected <<\EOF &&
+4:Subject: [PATCH] subject
+8:
+9:I want to mention about Signed-off-by: here.
+10:
+11:Signed-off-by: C O Mitter
+EOF
+ test_cmp expected actual
+'
+
+test_expect_failure 'signoff: not really a signoff (2)' '
+ append_signoff <<\EOF >actual &&
+subject
+
+My unfortunate
+Signed-off-by: example happens to be wrapped here.
+EOF
+ cat >expected <<\EOF &&
+4:Subject: [PATCH] subject
+8:
+10:Signed-off-by: example happens to be wrapped here.
+11:
+12:Signed-off-by: C O Mitter
+EOF
+ test_cmp expected actual
+'
+
+test_expect_failure 'signoff: valid S-o-b paragraph in the middle' '
+ append_signoff <<\EOF >actual &&
+subject
+
+Signed-off-by: my@house
+Signed-off-by: your@house
+
+A lot of houses.
+EOF
+ cat >expected <<\EOF &&
+4:Subject: [PATCH] subject
+8:
+9:Signed-off-by: my@house
+10:Signed-off-by: your@house
+11:
+13:
+14:Signed-off-by: C O Mitter
+EOF
+ test_cmp expected actual
+'
+
+test_expect_success 'signoff: the same signoff at the end' '
+ append_signoff <<\EOF >actual &&
+subject
+
+body
+
+Signed-off-by: C O Mitter
+EOF
+ cat >expected <<\EOF &&
+4:Subject: [PATCH] subject
+8:
+10:
+11:Signed-off-by: C O Mitter
+EOF
+ test_cmp expected actual
+'
+
+test_expect_success 'signoff: the same signoff at the end, no trailing NL' '
+ printf "subject\n\nSigned-off-by: C O Mitter " |
+ append_signoff >actual &&
+ cat >expected <<\EOF &&
+4:Subject: [PATCH] subject
+8:
+9:Signed-off-by: C O Mitter
+EOF
+ test_cmp expected actual
+'
+
+test_expect_success 'signoff: the same signoff NOT at the en
[PATCH v3 07/11] sequencer.c: teach append_signoff how to detect duplicate s-o-b
Teach append_signoff how to detect a duplicate s-o-b in the commit footer.
This is in preparation to unify the append_signoff implementations in
log-tree.c and sequencer.c.
Fixes test in t3511.
Signed-off-by: Brandon Casey
---
builtin/commit.c | 2 +-
sequencer.c | 47 +--
sequencer.h | 4 +++-
t/t3511-cherry-pick-x.sh | 2 +-
4 files changed, 42 insertions(+), 13 deletions(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index 38b9a9c..67ea9e7 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -700,7 +700,7 @@ static int prepare_to_commit(const char *index_file, const
char *prefix,
previous = eol;
}
- append_signoff(&sb, ignore_footer);
+ append_signoff(&sb, ignore_footer, 0);
}
if (fwrite(sb.buf, 1, sb.len, s->fp) < sb.len)
diff --git a/sequencer.c b/sequencer.c
index 46d51b2..015cb58 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -46,12 +46,20 @@ static int is_cherry_picked_from_line(const char *buf, int
len)
(buf[len - 1] == '\n' && buf[len - 2] == ')'));
}
-static int has_conforming_footer(struct strbuf *sb, int ignore_footer)
+/*
+ * Returns 0 for non-conforming footer
+ * Returns 1 for conforming footer
+ * Returns 2 when sob exists within conforming footer
+ * Returns 3 when sob exists within conforming footer as last entry
+ */
+static int has_conforming_footer(struct strbuf *sb, struct strbuf *sob,
+ int ignore_footer)
{
int last_char_was_nl, this_char_is_nl;
int i, k;
int len = sb->len - ignore_footer;
const char *buf = sb->buf;
+ int found_sob = 0;
/* find start of last paragraph */
last_char_was_nl = 0;
@@ -70,14 +78,25 @@ static int has_conforming_footer(struct strbuf *sb, int
ignore_footer)
i++;
for (; i < len; i = k) {
+ int found_rfc2822;
+
for (k = i; k < len && buf[k] != '\n'; k++)
; /* do nothing */
k++;
- if (!(is_rfc2822_line(buf + i, k - i) ||
+ found_rfc2822 = is_rfc2822_line(buf + i, k - i);
+ if (found_rfc2822 && sob &&
+ !strncmp(buf + i, sob->buf, sob->len))
+ found_sob = k;
+
+ if (!(found_rfc2822 ||
is_cherry_picked_from_line(buf + i, k - i)))
return 0;
}
+ if (found_sob == i)
+ return 3;
+ if (found_sob)
+ return 2;
return 1;
}
@@ -299,7 +318,7 @@ static int do_recursive_merge(struct commit *base, struct
commit *next,
rollback_lock_file(&index_lock);
if (opts->signoff)
- append_signoff(msgbuf, 0);
+ append_signoff(msgbuf, 0, 0);
if (!clean) {
int i;
@@ -558,7 +577,7 @@ static int do_pick_commit(struct commit *commit, struct
replay_opts *opts)
}
if (opts->record_origin) {
- if (!has_conforming_footer(&msgbuf, 0))
+ if (!has_conforming_footer(&msgbuf, NULL, 0))
strbuf_addch(&msgbuf, '\n');
strbuf_addstr(&msgbuf, cherry_picked_prefix);
strbuf_addstr(&msgbuf,
sha1_to_hex(commit->object.sha1));
@@ -1085,9 +1104,11 @@ int sequencer_pick_revisions(struct replay_opts *opts)
return pick_commits(todo_list, opts);
}
-void append_signoff(struct strbuf *msgbuf, int ignore_footer)
+void append_signoff(struct strbuf *msgbuf, int ignore_footer, unsigned flag)
{
+ unsigned no_dup_sob = flag & APPEND_SIGNOFF_DEDUP;
struct strbuf sob = STRBUF_INIT;
+ int has_footer = 0;
int i;
strbuf_addstr(&sob, sign_off_header);
@@ -1096,10 +1117,16 @@ 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 || !has_conforming_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 = has_conforming_footer(msgbuf, &sob, ignore_footer);
+
+ if (!has_footer)
+ strbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0, "\n", 1);
+
+ if (has_footer != 3 && (!no_dup_sob || has_footer != 2))
+ strbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0,
+ sob.buf, sob.len);
+
strbuf_release(&sob);
}
diff --git a/sequencer.h b/sequencer.h
index 9d57d57..1fc22dc 100644
---
[PATCH v3 06/11] sequencer.c: always separate "(cherry picked from" from commit body
Start treating the "(cherry picked from" line added by cherry-pick -x
the same way that the s-o-b lines are treated. Namely, separate them
from the main commit message body with an empty line.
This commit is mostly a code movement, but notice that
has_conforming_footer() was modified, in addition to being moved.
Introduce tests to test this functionality.
Signed-off-by: Brandon Casey
---
sequencer.c | 120 +--
t/t3511-cherry-pick-x.sh | 53 +
2 files changed, 116 insertions(+), 57 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index 0b5cd18..46d51b2 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -20,6 +20,67 @@
const char sign_off_header[] = "Signed-off-by: ";
static const char cherry_picked_prefix[] = "(cherry picked from commit ";
+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_picked_from_line(const char *buf, int len)
+{
+ /*
+* We only care that it looks roughly like (cherry picked from ...)
+*/
+ return !prefixcmp(buf, cherry_picked_prefix) &&
+ (buf[len - 1] == ')' ||
+(buf[len - 1] == '\n' && buf[len - 2] == ')'));
+}
+
+static int has_conforming_footer(struct strbuf *sb, int ignore_footer)
+{
+ int last_char_was_nl, this_char_is_nl;
+ int i, k;
+ int len = sb->len - ignore_footer;
+ const char *buf = sb->buf;
+
+ /* find start of last paragraph */
+ last_char_was_nl = 0;
+ for (i = len - 1; i > 0; i--) {
+ this_char_is_nl = (buf[i] == '\n');
+ if (last_char_was_nl && this_char_is_nl)
+ break;
+ last_char_was_nl = this_char_is_nl;
+ }
+
+ /* require at least one blank line */
+ if (!last_char_was_nl || buf[i] != '\n')
+ return 0;
+
+ while (i < len - 1 && buf[i] == '\n')
+ i++;
+
+ for (; i < len; i = k) {
+ for (k = i; k < len && buf[k] != '\n'; k++)
+ ; /* do nothing */
+ k++;
+
+ if (!(is_rfc2822_line(buf + i, k - i) ||
+ is_cherry_picked_from_line(buf + i, k - i)))
+ return 0;
+ }
+ return 1;
+}
+
static void remove_sequencer_state(void)
{
struct strbuf seq_dir = STRBUF_INIT;
@@ -497,6 +558,8 @@ static int do_pick_commit(struct commit *commit, struct
replay_opts *opts)
}
if (opts->record_origin) {
+ if (!has_conforming_footer(&msgbuf, 0))
+ strbuf_addch(&msgbuf, '\n');
strbuf_addstr(&msgbuf, cherry_picked_prefix);
strbuf_addstr(&msgbuf,
sha1_to_hex(commit->object.sha1));
strbuf_addstr(&msgbuf, ")\n");
@@ -1022,63 +1085,6 @@ 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_picked_from_line(const char *buf, int len)
-{
- /*
-* We only care that it looks roughly like (cherry picked from ...)
-*/
- return !prefixcmp(buf, cherry_picked_prefix) &&
- (buf[len - 1] == ')' ||
-(buf[len - 1] == '\n' && buf[len - 2] == ')'));
-}
-
-static int has_conforming_footer(struct strbuf *sb, int ignore_footer)
-{
- int last_char_was_nl, this_char_is_nl;
- int i, k;
- int len = sb->len - ignore_footer;
- const char *buf = sb->buf;
-
- /* find start of last paragraph */
- last_char_was_nl = 0;
- for (i = len - 1; i > 0; i--) {
- this_char_is_nl = (buf[i] == '\n');
- if (last_char_was_nl && this_char_is_nl)
- break;
- last_char_was_nl = this_char_is_nl;
- }
-
- while (i < len - 1 && buf[i] == '\n')
- i++;
-
- for (; i < len; i = k) {
- for (k = i; k < len && buf[k] != '\n'; k++)
- ; /* do nothing */
- k++;
-
- if (!(is_rfc2822_line(buf + i, k - i) ||
- is_cherry_picked_from_line(buf + i, k - i)))
- return 0;
- }
- return 1;
-}
-
void append_signoff(struct strbuf *msgbuf, int ign
[PATCH v3 04/11] t/t3511: add some tests of 'cherry-pick -s' functionality
Add some tests to ensure that 'cherry-pick -s' operates in the following
manner:
* Inserts a blank line before appending a s-o-b to a commit message that
does not contain a s-o-b footer
* Does not mistake first line "subject: description" as a s-o-b footer
* Does not mistake single word message body as conforming to rfc2822
* Appends a s-o-b when last s-o-b in footer does not match committer
s-o-b, even when committer's s-o-b exists elsewhere in footer.
* Does not append a s-o-b when last s-o-b matches committer s-o-b
* Correctly detects a non-conforming footer containing a mix of s-o-b
like elements and s-o-b elements. (marked "expect failure")
Signed-off-by: Brandon Casey
---
t/t3511-cherry-pick-x.sh | 111 +++
1 file changed, 111 insertions(+)
create mode 100755 t/t3511-cherry-pick-x.sh
diff --git a/t/t3511-cherry-pick-x.sh b/t/t3511-cherry-pick-x.sh
new file mode 100755
index 000..2a040b7
--- /dev/null
+++ b/t/t3511-cherry-pick-x.sh
@@ -0,0 +1,111 @@
+#!/bin/sh
+
+test_description='Test cherry-pick -x and -s'
+
+. ./test-lib.sh
+
+pristine_detach () {
+ git cherry-pick --quit &&
+ git checkout -f "$1^0" &&
+ git read-tree -u --reset HEAD &&
+ git clean -d -f -f -q -x
+}
+
+mesg_one_line='base: commit message'
+
+mesg_no_footer="$mesg_one_line
+
+OneWordBodyThatsNotA-S-o-B"
+
+mesg_with_footer="$mesg_no_footer
+
+Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>
+Signed-off-by: A.U. Thor
+Signed-off-by: B.U. Thor "
+
+mesg_broken_footer="$mesg_no_footer
+
+The signed-off-by string should begin with the words Signed-off-by followed
+by a colon and space, and then the signers name and email address. e.g.
+Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>"
+
+mesg_with_footer_sob="$mesg_with_footer
+Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>"
+
+
+test_expect_success setup '
+ git config advice.detachedhead false &&
+ echo unrelated >unrelated &&
+ git add unrelated &&
+ test_commit initial foo a &&
+ test_commit "$mesg_one_line" foo b mesg-one-line &&
+ git reset --hard initial &&
+ test_commit "$mesg_no_footer" foo b mesg-no-footer &&
+ git reset --hard initial &&
+ test_commit "$mesg_broken_footer" foo b mesg-broken-footer &&
+ git reset --hard initial &&
+ test_commit "$mesg_with_footer" foo b mesg-with-footer &&
+ git reset --hard initial &&
+ test_commit "$mesg_with_footer_sob" foo b mesg-with-footer-sob &&
+ pristine_detach initial &&
+ test_commit conflicting unrelated
+'
+
+test_expect_success 'cherry-pick -s inserts blank line after one line subject'
'
+ pristine_detach initial &&
+ git cherry-pick -s mesg-one-line &&
+ cat <<-EOF >expect &&
+ $mesg_one_line
+
+ Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>
+ EOF
+ git log -1 --pretty=format:%B >actual &&
+ test_cmp expect actual
+'
+
+test_expect_failure 'cherry-pick -s inserts blank line after non-conforming
footer' '
+ pristine_detach initial &&
+ git cherry-pick -s mesg-broken-footer &&
+ cat <<-EOF >expect &&
+ $mesg_broken_footer
+
+ Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>
+ EOF
+ git log -1 --pretty=format:%B >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'cherry-pick -s inserts blank line when conforming footer
not found' '
+ pristine_detach initial &&
+ git cherry-pick -s mesg-no-footer &&
+ cat <<-EOF >expect &&
+ $mesg_no_footer
+
+ Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>
+ EOF
+ git log -1 --pretty=format:%B >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'cherry-pick -s adds sob when last sob doesnt match
committer' '
+ pristine_detach initial &&
+ git cherry-pick -s mesg-with-footer &&
+ cat <<-EOF >expect &&
+ $mesg_with_footer
+ Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>
+ EOF
+ git log -1 --pretty=format:%B >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'cherry-pick -s refrains from adding duplicate trailing
sob' '
+ pristine_detach initial &&
+ git cherry-pick -s mesg-with-footer-sob &&
+ cat <<-EOF >expect &&
+ $mesg_with_footer_sob
+ EOF
+ git log -1 --pretty=format:%B >actual &&
+ test_cmp expect actual
+'
+
+test_done
--
1.8.1.1.450.g0327af3
--
To unsubscribe from this list: send the line "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 v3 05/11] sequencer.c: recognize "(cherry picked from ..." as part of s-o-b footer
When 'cherry-pick -s' is used to append a signed-off-by line to a cherry
picked commit, it does not currently detect the "(cherry picked from..."
that may have been appended by a previous 'cherry-pick -x' as part of the
s-o-b footer and it will insert a blank line before appending a new s-o-b.
Let's detect "(cherry picked from...)" as part of the footer so that we
will produce this:
Signed-off-by: A U Thor
(cherry picked from da39a3ee5e6b4b0d3255bfef95601890afd80709)
Signed-off-by: C O Mmitter
instead of this:
Signed-off-by: A U Thor
(cherry picked from da39a3ee5e6b4b0d3255bfef95601890afd80709)
Signed-off-by: C O Mmitter
Signed-off-by: Brandon Casey
Acked-by: Jonathan Nieder
---
sequencer.c | 46
t/t3511-cherry-pick-x.sh | 55
2 files changed, 88 insertions(+), 13 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index 39a752b..0b5cd18 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)
{
@@ -496,7 +497,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");
}
@@ -1021,11 +1022,36 @@ int sequencer_pick_revisions(struct replay_opts *opts)
return pick_commits(todo_list, opts);
}
-static int ends_rfc2822_footer(struct strbuf *sb, int ignore_footer)
+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_picked_from_line(const char *buf, int len)
+{
+ /*
+* We only care that it looks roughly like (cherry picked from ...)
+*/
+ return !prefixcmp(buf, cherry_picked_prefix) &&
+ (buf[len - 1] == ')' ||
+(buf[len - 1] == '\n' && buf[len - 2] == ')'));
+}
+
+static int has_conforming_footer(struct strbuf *sb, int ignore_footer)
{
- int ch;
int last_char_was_nl, this_char_is_nl;
- int i, j, k;
+ int i, k;
int len = sb->len - ignore_footer;
const char *buf = sb->buf;
@@ -1046,15 +1072,9 @@ static int ends_rfc2822_footer(struct strbuf *sb, int
ignore_footer)
; /* do nothing */
k++;
- for (j = 0; i + j < len; j++) {
- ch = buf[i + j];
- if (ch == ':')
- break;
- if (isalnum(ch) ||
- (ch == '-'))
- continue;
+ if (!(is_rfc2822_line(buf + i, k - i) ||
+ is_cherry_picked_from_line(buf + i, k - i)))
return 0;
- }
}
return 1;
}
@@ -1071,7 +1091,7 @@ void append_signoff(struct strbuf *msgbuf, int
ignore_footer)
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))
+ if (!i || !has_conforming_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);
}
diff --git a/t/t3511-cherry-pick-x.sh b/t/t3511-cherry-pick-x.sh
index 2a040b7..73da182 100755
--- a/t/t3511-cherry-pick-x.sh
+++ b/t/t3511-cherry-pick-x.sh
@@ -32,6 +32,10 @@ Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>"
mesg_with_footer_sob="$mesg_with_footer
Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>"
+mesg_with_cherry_footer="$mesg_with_footer_sob
+(cherry picked from commit da39a3ee5e6b4b0d3255bfef95601890afd80709)
+Tested-by: C.U. Thor "
+
test_expect_success setup '
git config advice.detachedhead false &&
@@ -47,6 +51,8 @@ test_expect_success setup '
test_commit "$mesg_with_footer" foo b mesg-with-footer &&
git reset --hard initial &&
test_commit "$mesg_with_footer_sob" foo b mesg-with-footer-sob &&
+ git reset --hard initial &&
+ test_commit "$mesg_with_cherry_footer" foo b
[PATCH v3 02/11] commit, cherry-pick -s: remove broken support for multiline rfc2822 fields
Starting with c1e01b0c (commit: More generous accepting of RFC-2822 footer
lines, 2009-10-28), "git commit -s" carefully parses the last paragraph of
each commit message to check if it consists only of RFC2822-style headers,
in which case the signoff will be added as a new line in the same list:
Reported-by: Reporter
Signed-off-by: Author
Acked-by: Lieutenant
It even included support for accepting indented continuation lines for
multiline fields. Unfortunately the multiline field support is broken
because it checks whether buf[k] (the first character of the *next* line)
instead of buf[i] is a whitespace character. The result is that any footer
with a continuation line is not accepted, since the last continuation line
neither starts with an RFC2822 field name nor is followed by a continuation
line.
That this has remained broken for so long is good evidence that nobody
actually needed multiline fields. Rip out the broken continuation support.
There should be no functional change.
[Thanks to Jonathan Nieder for the excellent commit message]
Signed-off-by: Brandon Casey
---
sequencer.c | 6 --
1 file changed, 6 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index cd211b2..39a752b 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -1027,7 +1027,6 @@ static int ends_rfc2822_footer(struct strbuf *sb, int
ignore_footer)
int last_char_was_nl, this_char_is_nl;
int i, j, k;
int len = sb->len - ignore_footer;
- int first = 1;
const char *buf = sb->buf;
/* find start of last paragraph */
@@ -1047,11 +1046,6 @@ static int ends_rfc2822_footer(struct strbuf *sb, int
ignore_footer)
; /* do nothing */
k++;
- if ((buf[k] == ' ' || buf[k] == '\t') && !first)
- continue;
-
- first = 0;
-
for (j = 0; i + j < len; j++) {
ch = buf[i + j];
if (ch == ':')
--
1.8.1.1.450.g0327af3
--
To unsubscribe from this list: send the line "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 v3 03/11] t/test-lib-functions.sh: allow to specify the tag name to test_commit
The part of test_commit() may not be appropriate for a tag name.
So let's allow test_commit to accept a fourth argument to specify the tag
name.
Signed-off-by: Brandon Casey
Reviewed-by: Jonathan Nieder
---
t/test-lib-functions.sh | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index fa62d01..61d0804 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -135,12 +135,12 @@ test_pause () {
fi
}
-# Call test_commit with the arguments " [ []]"
+# Call test_commit with the arguments " [ [ []]]"
#
# This will commit a file with the given contents and the given commit
-# message. It will also add a tag with as name.
+# message, and tag the resulting commit with the given tag name.
#
-# Both and default to .
+# , , and all default to .
test_commit () {
notick= &&
@@ -168,7 +168,7 @@ test_commit () {
test_tick
fi &&
git commit $signoff -m "$1" &&
- git tag "$1"
+ git tag "${4:-$1}"
}
# Call test_merge with the arguments " ", where
--
1.8.1.1.450.g0327af3
--
To unsubscribe from this list: send the line "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 v3 01/11] sequencer.c: rework search for start of footer to improve clarity
This code sequence is somewhat difficult to read. Let's rewrite it
using more descriptive variable names to try to make it easier to
understand.
Signed-off-by: Brandon Casey
---
sequencer.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index aef5e8a..cd211b2 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -1024,16 +1024,19 @@ int sequencer_pick_revisions(struct replay_opts *opts)
static int ends_rfc2822_footer(struct strbuf *sb, int ignore_footer)
{
int ch;
- int hit = 0;
+ int last_char_was_nl, this_char_is_nl;
int i, j, k;
int len = sb->len - ignore_footer;
int first = 1;
const char *buf = sb->buf;
+ /* find start of last paragraph */
+ last_char_was_nl = 0;
for (i = len - 1; i > 0; i--) {
- if (hit && buf[i] == '\n')
+ this_char_is_nl = (buf[i] == '\n');
+ if (last_char_was_nl && this_char_is_nl)
break;
- hit = (buf[i] == '\n');
+ last_char_was_nl = this_char_is_nl;
}
while (i < len - 1 && buf[i] == '\n')
--
1.8.1.1.450.g0327af3
--
To unsubscribe from this list: send the line "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 v3 00/11] unify appending of sob
Round 3. -Brandon Brandon Casey (9): sequencer.c: rework search for start of footer to improve clarity commit, cherry-pick -s: remove broken support for multiline rfc2822 fields t/test-lib-functions.sh: allow to specify the tag name to test_commit t/t3511: add some tests of 'cherry-pick -s' functionality sequencer.c: recognize "(cherry picked from ..." as part of s-o-b footer sequencer.c: always separate "(cherry picked from" from commit body sequencer.c: teach append_signoff how to detect duplicate s-o-b sequencer.c: teach append_signoff to avoid adding a duplicate newline Unify appending signoff in format-patch, commit and sequencer Nguyễn Thái Ngọc Duy (2): t4014: more tests about appending s-o-b lines format-patch: update append_signoff prototype builtin/commit.c | 2 +- builtin/log.c| 13 +-- log-tree.c | 92 ++--- revision.h | 2 +- sequencer.c | 150 ++- sequencer.h | 4 +- t/t3511-cherry-pick-x.sh | 219 +++ t/t4014-format-patch.sh | 263 +++ t/test-lib-functions.sh | 8 +- 9 files changed, 600 insertions(+), 153 deletions(-) create mode 100755 t/t3511-cherry-pick-x.sh -- 1.8.1.1.450.g0327af3 -- To unsubscribe from this list: send the line "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/4] doc: Generate a list of valid merge tools
Use the show_tool_names() function to build lists of all the built-in tools supported by difftool and mergetool. This frees us from needing to update the documentation whenever a new tool is added. Signed-off-by: David Aguilar --- Adjusted to use show_tool_names() and reworked the makefile dependencies. I could use another set of eyes on the Makefile.. Documentation/.gitignore | 1 + Documentation/Makefile | 22 -- Documentation/diff-config.txt | 13 +++-- Documentation/merge-config.txt | 12 ++-- git-mergetool--lib.sh | 3 ++- 5 files changed, 36 insertions(+), 15 deletions(-) diff --git a/Documentation/.gitignore b/Documentation/.gitignore index d62aebd..2c8b2d6 100644 --- a/Documentation/.gitignore +++ b/Documentation/.gitignore @@ -9,4 +9,5 @@ gitman.info howto-index.txt doc.dep cmds-*.txt +mergetools-*.txt manpage-base-url.xsl diff --git a/Documentation/Makefile b/Documentation/Makefile index 267dfe1..834ec25 100644 --- a/Documentation/Makefile +++ b/Documentation/Makefile @@ -202,7 +202,11 @@ install-html: html # # Determine "include::" file references in asciidoc files. # -doc.dep : $(wildcard *.txt) build-docdep.perl +docdep_prereqs = \ + mergetools-list.made $(mergetools_txt) \ + cmd-list.made $(cmds_txt) + +doc.dep : $(docdep_prereqs) $(wildcard *.txt) build-docdep.perl $(QUIET_GEN)$(RM) $@+ $@ && \ $(PERL_PATH) ./build-docdep.perl >$@+ $(QUIET_STDERR) && \ mv $@+ $@ @@ -226,13 +230,27 @@ cmd-list.made: cmd-list.perl ../command-list.txt $(MAN1_TXT) $(PERL_PATH) ./cmd-list.perl ../command-list.txt $(QUIET_STDERR) && \ date >$@ +mergetools_txt = mergetools-diff.txt mergetools-merge.txt + +$(mergetools_txt): mergetools-list.made + +mergetools-list.made: ../git-mergetool--lib.sh $(wildcard ../mergetools/*) + $(QUIET_GEN)$(RM) $@ && \ + $(SHELL_PATH) -c 'MERGE_TOOLS_DIR=../mergetools && \ + . ../git-mergetool--lib.sh && \ + show_tool_names can_diff "* "' > mergetools-diff.txt && \ + $(SHELL_PATH) -c 'MERGE_TOOLS_DIR=../mergetools && \ + . ../git-mergetool--lib.sh && \ + show_tool_names can_merge "* "' > mergetools-merge.txt && \ + date > $@ + clean: $(RM) *.xml *.xml+ *.html *.html+ *.1 *.5 *.7 $(RM) *.texi *.texi+ *.texi++ git.info gitman.info $(RM) *.pdf $(RM) howto-index.txt howto/*.html doc.dep $(RM) technical/api-*.html technical/api-index.txt - $(RM) $(cmds_txt) *.made + $(RM) $(cmds_txt) $(mergetools_txt) *.made $(RM) manpage-base-url.xsl $(MAN_HTML): %.html : %.txt diff --git a/Documentation/diff-config.txt b/Documentation/diff-config.txt index 67a90a8..7c968d1 100644 --- a/Documentation/diff-config.txt +++ b/Documentation/diff-config.txt @@ -132,9 +132,10 @@ diff..cachetextconv:: conversion outputs. See linkgit:gitattributes[5] for details. diff.tool:: - The diff tool to be used by linkgit:git-difftool[1]. This - option overrides `merge.tool`, and has the same valid built-in - values as `merge.tool` minus "tortoisemerge" and plus - "kompare". Any other value is treated as a custom diff tool, - and there must be a corresponding `difftool..cmd` - option. + Controls which diff tool is used by linkgit:git-difftool[1]. + This variable overrides the value configured in `merge.tool`. + The list below shows the valid built-in values. + Any other value is treated as a custom diff tool and requires + that a corresponding difftool..cmd variable is defined. + +include::mergetools-diff.txt[] diff --git a/Documentation/merge-config.txt b/Documentation/merge-config.txt index 861bd6f..5f40e71 100644 --- a/Documentation/merge-config.txt +++ b/Documentation/merge-config.txt @@ -52,12 +52,12 @@ merge.stat:: at the end of the merge. True by default. merge.tool:: - Controls which merge resolution program is used by - linkgit:git-mergetool[1]. Valid built-in values are: "araxis", - "bc3", "diffuse", "ecmerge", "emerge", "gvimdiff", "kdiff3", "meld", - "opendiff", "p4merge", "tkdiff", "tortoisemerge", "vimdiff" - and "xxdiff". Any other value is treated is custom merge tool - and there must be a corresponding mergetool..cmd option. + Controls which merge tool is used by linkgit:git-mergetool[1]. + The list below shows the valid built-in values. + Any other value is treated as a custom merge tool and requires + that a corresponding mergetool..cmd variable is defined. + +include::mergetools-merge.txt[] merge.verbosity:: Controls the amount of output shown by the recursive merge diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh index fe068f6..f665bee 100644 --- a/git-mergetool--lib.sh +++ b/git-mergetool--lib.sh @@ -1,6 +1,7 @@ #!/bin/sh # git-mergetool--lib is a library for co
[PATCH v2 3/4] mergetool--lib: Add functions for finding available tools
Refactor show_tool_help() so that the tool-finding logic is broken out
into a separate show_tool_names() function.
Signed-off-by: David Aguilar
---
filter_tools renamed to show_tool_names() and simplfied
to use ls -1. show_tool_names() now has a preamble as discussed.
git-mergetool--lib.sh | 68 +--
1 file changed, 39 insertions(+), 29 deletions(-)
diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index db3eb58..fe068f6 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -2,6 +2,35 @@
# git-mergetool--lib is a library for common merge tool functions
MERGE_TOOLS_DIR=$(git --exec-path)/mergetools
+mode_ok () {
+ diff_mode && can_diff ||
+ merge_mode && can_merge
+}
+
+is_available () {
+ merge_tool_path=$(translate_merge_tool_path "$1") &&
+ type "$merge_tool_path" >/dev/null 2>&1
+}
+
+show_tool_names () {
+ condition=${1:-true} per_line_prefix=${2:-} preamble=${3:-}
+
+ ( cd "$MERGE_TOOLS_DIR" && ls -1 * ) |
+ while read toolname
+ do
+ if setup_tool "$toolname" 2>/dev/null &&
+ (eval "$condition" "$toolname")
+ then
+ if test -n "$preamble"
+ then
+ echo "$preamble"
+ preamble=
+ fi
+ printf "%s%s\n" "$per_line_prefix" "$tool"
+ fi
+ done
+}
+
diff_mode() {
test "$TOOL_MODE" = diff
}
@@ -199,35 +228,21 @@ list_merge_tool_candidates () {
}
show_tool_help () {
- unavailable= available= LF='
-'
- for i in "$MERGE_TOOLS_DIR"/*
- do
- tool=$(basename "$i")
- setup_tool "$tool" 2>/dev/null || continue
-
- merge_tool_path=$(translate_merge_tool_path "$tool")
- if type "$merge_tool_path" >/dev/null 2>&1
- then
- available="$available$tool$LF"
- else
- unavailable="$unavailable$tool$LF"
- fi
- done
-
- cmd_name=${TOOL_MODE}tool
+ tool_opt="'git ${TOOL_MODE}tool --tool-'"
+ available=$(show_tool_names 'mode_ok && is_available' '\t\t' \
+ "$tool_opt may be set to one of the following:")
+ unavailable=$(show_tool_names 'mode_ok && ! is_available' '\t\t' \
+ "The following tools are valid, but not currently available:")
if test -n "$available"
then
- echo "'git $cmd_name --tool=' may be set to one of the
following:"
- echo "$available" | sort | sed -e 's/^/ /'
+ echo "$available"
else
echo "No suitable tool for 'git $cmd_name --tool=' found."
fi
if test -n "$unavailable"
then
echo
- echo 'The following tools are valid, but not currently
available:'
- echo "$unavailable" | sort | sed -e 's/^/ /'
+ echo "$unavailable"
fi
if test -n "$unavailable$available"
then
@@ -248,17 +263,12 @@ See 'git ${TOOL_MODE}tool --tool-help' or 'git help
config' for more details.
$tools
EOF
# Loop over each candidate and stop when a valid merge tool is found.
- for i in $tools
+ for tool in $tools
do
- merge_tool_path=$(translate_merge_tool_path "$i")
- if type "$merge_tool_path" >/dev/null 2>&1
- then
- echo "$i"
- return 0
- fi
+ is_available "$tool" && echo "$tool" && return 0
done
- echo >&2 "No known merge resolution program available."
+ echo >&2 "No known ${TOOL_MODE} tool is available."
return 1
}
--
1.8.0.13.g3ff16bb
--
To unsubscribe from this list: send the line "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 2/4] mergetool--lib: Improve the help text in guess_merge_tool()
This code path is only activated when the user does not have a valid
configured tool. Add a message to guide new users towards configuring a
default tool.
Signed-off-by: David Aguilar
---
This now uses a cat << here-doc.
git-mergetool--lib.sh | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index 9a5aae9..db3eb58 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -240,8 +240,13 @@ show_tool_help () {
guess_merge_tool () {
list_merge_tool_candidates
- echo >&2 "merge tool candidates: $tools"
+ cat >&2 <<-EOF
+This message is displayed because '$TOOL_MODE.tool' is not configured.
+See 'git ${TOOL_MODE}tool --tool-help' or 'git help config' for more details.
+'git ${TOOL_MODE}tool' will now attempt to use one of the following tools:
+$tools
+EOF
# Loop over each candidate and stop when a valid merge tool is found.
for i in $tools
do
--
1.8.0.13.g3ff16bb
--
To unsubscribe from this list: send the line "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 0/4] Auto-generate mergetool lists
This is round two of this series. I think this touched on everything brought up in the code review. 4/4 could use a review as I'm not completely familiar with the makefile dependencies, though it seems to work correctly. David Aguilar (4): mergetool--lib: Simplify command expressions mergetool--lib: Improve the help text in guess_merge_tool() mergetool--lib: Add functions for finding available tools doc: Generate a list of valid merge tools Documentation/.gitignore | 1 + Documentation/Makefile | 22 +++- Documentation/diff-config.txt | 13 ++--- Documentation/merge-config.txt | 12 ++--- git-mergetool--lib.sh | 116 ++--- 5 files changed, 96 insertions(+), 68 deletions(-) -- 1.8.0.13.g3ff16bb -- To unsubscribe from this list: send the line "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 1/4] mergetool--lib: Simplify command expressions
Update variable assignments to always use $(command "$arg")
in their RHS instead of "$(command "$arg")" as the latter
is harder to read. Make get_merge_tool_cmd() simpler by
avoiding "echo" and $(command) substitutions completely.
Signed-off-by: David Aguilar
---
I reworded the commit message to be more clear.
git-mergetool--lib.sh | 40
1 file changed, 16 insertions(+), 24 deletions(-)
diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index 1d0fb12..9a5aae9 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -32,17 +32,10 @@ check_unchanged () {
fi
}
-valid_tool_config () {
- if test -n "$(get_merge_tool_cmd "$1")"
- then
- return 0
- else
- return 1
- fi
-}
-
valid_tool () {
- setup_tool "$1" || valid_tool_config "$1"
+ setup_tool "$1" && return 0
+ cmd=$(get_merge_tool_cmd "$1")
+ test -n "$cmd"
}
setup_tool () {
@@ -96,14 +89,13 @@ setup_tool () {
}
get_merge_tool_cmd () {
- # Prints the custom command for a merge tool
merge_tool="$1"
if diff_mode
then
- echo "$(git config difftool.$merge_tool.cmd ||
- git config mergetool.$merge_tool.cmd)"
+ git config "difftool.$merge_tool.cmd" ||
+ git config "mergetool.$merge_tool.cmd"
else
- echo "$(git config mergetool.$merge_tool.cmd)"
+ git config "mergetool.$merge_tool.cmd"
fi
}
@@ -114,7 +106,7 @@ run_merge_tool () {
GIT_PREFIX=${GIT_PREFIX:-.}
export GIT_PREFIX
- merge_tool_path="$(get_merge_tool_path "$1")" || exit
+ merge_tool_path=$(get_merge_tool_path "$1") || exit
base_present="$2"
status=0
@@ -145,7 +137,7 @@ run_merge_tool () {
# Run a either a configured or built-in diff tool
run_diff_cmd () {
- merge_tool_cmd="$(get_merge_tool_cmd "$1")"
+ merge_tool_cmd=$(get_merge_tool_cmd "$1")
if test -n "$merge_tool_cmd"
then
( eval $merge_tool_cmd )
@@ -158,11 +150,11 @@ run_diff_cmd () {
# Run a either a configured or built-in merge tool
run_merge_cmd () {
- merge_tool_cmd="$(get_merge_tool_cmd "$1")"
+ merge_tool_cmd=$(get_merge_tool_cmd "$1")
if test -n "$merge_tool_cmd"
then
- trust_exit_code="$(git config --bool \
- mergetool."$1".trustExitCode || echo false)"
+ trust_exit_code=$(git config --bool \
+ "mergetool.$1.trustExitCode" || echo false)
if test "$trust_exit_code" = "false"
then
touch "$BACKUP"
@@ -253,7 +245,7 @@ guess_merge_tool () {
# Loop over each candidate and stop when a valid merge tool is found.
for i in $tools
do
- merge_tool_path="$(translate_merge_tool_path "$i")"
+ merge_tool_path=$(translate_merge_tool_path "$i")
if type "$merge_tool_path" >/dev/null 2>&1
then
echo "$i"
@@ -300,9 +292,9 @@ get_merge_tool_path () {
fi
if test -z "$merge_tool_path"
then
- merge_tool_path="$(translate_merge_tool_path "$merge_tool")"
+ merge_tool_path=$(translate_merge_tool_path "$merge_tool")
fi
- if test -z "$(get_merge_tool_cmd "$merge_tool")" &&
+ if test -z $(get_merge_tool_cmd "$merge_tool") &&
! type "$merge_tool_path" >/dev/null 2>&1
then
echo >&2 "The $TOOL_MODE tool $merge_tool is not available as"\
@@ -314,11 +306,11 @@ get_merge_tool_path () {
get_merge_tool () {
# Check if a merge tool has been configured
- merge_tool="$(get_configured_merge_tool)"
+ merge_tool=$(get_configured_merge_tool)
# Try to guess an appropriate merge tool if no tool has been set.
if test -z "$merge_tool"
then
- merge_tool="$(guess_merge_tool)" || exit
+ merge_tool=$(guess_merge_tool) || exit
fi
echo "$merge_tool"
}
--
1.8.0.13.g3ff16bb
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 06/10] sequencer.c: teach append_signoff how to detect duplicate s-o-b
On Tue, Jan 22, 2013 at 12:38 AM, Jonathan Nieder wrote: > Brandon Casey wrote: > >> Teach append_signoff how to detect a duplicate s-o-b in the commit footer. >> This is in preparation to unify the append_signoff implementations in >> log-tree.c and sequencer.c. > [...] >> --- a/sequencer.c >> +++ b/sequencer.c >> @@ -1082,9 +1101,10 @@ int sequencer_pick_revisions(struct replay_opts *opts) >> return pick_commits(todo_list, opts); >> } >> >> -void append_signoff(struct strbuf *msgbuf, int ignore_footer) >> +void append_signoff(struct strbuf *msgbuf, int ignore_footer, int >> no_dup_sob) > > Isn't the behavior of passing '1' here just a bug in "format-patch -s"? I think that is an open question. > Style: callers will be easier to read if the function takes a flag > word with named bits, as in: > > #define APPEND_SIGNOFF_DEDUP (1 << 0) > void append_signoff(..., int ignore_footer, unsigned flag) Can't I just be lazy!!! Ok, you win. :b -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 4/4] doc: Generate a list of valid merge tools
David Aguilar writes: > diff --git a/Documentation/Makefile b/Documentation/Makefile > index 267dfe1..f595d26 100644 > --- a/Documentation/Makefile > +++ b/Documentation/Makefile > @@ -226,13 +226,27 @@ cmd-list.made: cmd-list.perl ../command-list.txt > $(MAN1_TXT) > $(PERL_PATH) ./cmd-list.perl ../command-list.txt $(QUIET_STDERR) && \ > date >$@ > > +mergetools_txt = mergetools-diff.txt mergetools-merge.txt > + > +$(mergetools_txt): mergetools-list.made The product of files that include these two and doc.dep target have to depend on these two files being up to date. doc.dep depends on $(wildcard *.txt) so you may be lucky and $(mergetools_txt) have been built once in which case they would depend on it, or unlukcy and end up doc.dep that does not know about them, no? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] mergetool--lib: Add functions for finding available tools
On Sun, Jan 27, 2013 at 3:32 PM, Junio C Hamano wrote:
> David Aguilar writes:
>
>> +filter_tools () {
>> + filter="$1"
>> + prefix="$2"
>> + (
>> + cd "$MERGE_TOOLS_DIR" &&
>> + for i in *
>> + do
>> + echo "$i"
>> + done
>> + ) | sort | while read tool
>> + do
>> + setup_tool "$tool" 2>/dev/null &&
>> + (eval "$filter" "$tool") &&
>> + printf "$prefix$tool\n"
>> + done
>> +}
>> +
>> diff_mode() {
>> test "$TOOL_MODE" = diff
>> }
>> @@ -199,27 +226,13 @@ list_merge_tool_candidates () {
>> }
>>
>> show_tool_help () {
>> - unavailable= available= LF='
>> -'
>> - for i in "$MERGE_TOOLS_DIR"/*
>> - do
>> - tool=$(basename "$i")
>> - setup_tool "$tool" 2>/dev/null || continue
>> -
>> - merge_tool_path=$(translate_merge_tool_path "$tool")
>> - if type "$merge_tool_path" >/dev/null 2>&1
>> - then
>> - available="$available$tool$LF"
>> - else
>> - unavailable="$unavailable$tool$LF"
>> - fi
>> - done
>> -
>> cmd_name=${TOOL_MODE}tool
>> + available=$(filter_tools 'mode_ok && is_available' '\t\t')
>> + unavailable=$(filter_tools 'mode_ok && ! is_available' '\t\t')
>> if test -n "$available"
>> then
>> echo "'git $cmd_name --tool=' may be set to one of the
>> following:"
>> - echo "$available" | sort | sed -e 's/^/ /'
>> + printf "$available"
>
> This may happen to be safe as available will not have anything with
> a per-cent % in it, but is not a good discipline in general.
>
> printf "%s" "$available"
>
> If you are giving filter_tools an optional "prefix-per-line", I do
> not think it is too much of a stretch to introduce another optional
> "perfix for the whole thing" and let this call site say something
> like this:
>
> cmd_name=${TOOL_MODE}tool
> show_tool_names 'mode_ok && is_available' '\t\t' \
> "'git $cmd_name --tool=' may be set to one of these:"
> show_tool_names 'mode_ok && !is_available' '\t\t' \
> "These are valid but not available:"
>
> without any of the above logic (and the same for unav). It may look like
> this:
>
> show_tool_names () {
> condition=${1?condition} per_line_prefix=${2:-} preamble=${3:-}
>
> ( cd "$MERGE_TOOLS_DIR && ls -1 * ) |
> while read toolname
> do
> if setup_tool "$toolname" 2>/dev/null && (eval "$condition")
> then
> if test -n "$preamble"
> then
> echo "$preamble"
> preamble=
> fi
> printf "%s%s\n" "$prefix" "$toolname"
> fi
> done
> }
Thanks guys. I'll re-roll this soon.
--
David
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 03/10] t/t3511: add some tests of 'cherry-pick -s' functionality
On Tue, Jan 22, 2013 at 12:17 AM, Jonathan Nieder wrote: > Brandon Casey wrote: > >> --- /dev/null >> +++ b/t/t3511-cherry-pick-x.sh > [...] >> +test_expect_failure 'cherry-pick -s inserts blank line after non-conforming >> footer' ' > > IIUC this is an illustration of false-positives from messages like > this one: > > base: do something great without a sign-off > > If he does that, it will be the best thing in the > world: or so I think. > > A worthy cause. Could the example broken message be tweaked to > emphasize that use case? With the current example, I'd consider > either result (blank line or no blank line) to be ok behavior by git. The primary motivation for this test was to exercise an existing behavior which fails to append a newline and sob if the last line of the last paragraph matches the sob of the committer regardless of whether the entire paragraph would be interpreted as a conforming footer. Your example is tested as a side-effect of that. I'll tweak the string so it looks like this: The signed-off-by string should begin with the words Signed-off-by followed by a colon and space, and then the signers name and email address. e.g. Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> > [...] >> +test_expect_success 'cherry-pick -s refrains from adding duplicate trailing >> sob' ' > > And the other side of basic "-s" functionality. > > One more test would be interesting: what does "-s" do when asked to > produce a duplicate signoff with an interspersed signoff by someone else? > > test: a patch with a more complicated life > > This patch bounced from $GIT_COMMITTER_NAME to Ms. Thor for > tweaking, then back to $GIT_COMMITTER_NAME who will be > recording it in permanent history. > > Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>" > Signed-off-by: A U Thor This one exists as "adds sob when last sob doesn't match committer". In this case an additional sob should be appended to the footer. -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 v2 03/10] t/t3511: add some tests of 'cherry-pick -s' functionality
Brandon Casey wrote: >I'll tweak > the string so it looks like this: > > The signed-off-by string should begin with the words Signed-off-by followed > by a colon and space, and then the signers name and email address. e.g. > Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> Yep, that looks better than the example I suggested. :) [...] > On Tue, Jan 22, 2013 at 12:17 AM, Jonathan Nieder wrote: >> One more test would be interesting: what does "-s" do when asked to >> produce a duplicate signoff with an interspersed signoff by someone else? [...] > This one exists as "adds sob when last sob doesn't match committer". So it does. Thanks for the sanity check. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] mergetool--lib: Add functions for finding available tools
David Aguilar writes:
> +filter_tools () {
> + filter="$1"
> + prefix="$2"
> + (
> + cd "$MERGE_TOOLS_DIR" &&
> + for i in *
> + do
> + echo "$i"
> + done
> + ) | sort | while read tool
> + do
> + setup_tool "$tool" 2>/dev/null &&
> + (eval "$filter" "$tool") &&
> + printf "$prefix$tool\n"
> + done
> +}
> +
> diff_mode() {
> test "$TOOL_MODE" = diff
> }
> @@ -199,27 +226,13 @@ list_merge_tool_candidates () {
> }
>
> show_tool_help () {
> - unavailable= available= LF='
> -'
> - for i in "$MERGE_TOOLS_DIR"/*
> - do
> - tool=$(basename "$i")
> - setup_tool "$tool" 2>/dev/null || continue
> -
> - merge_tool_path=$(translate_merge_tool_path "$tool")
> - if type "$merge_tool_path" >/dev/null 2>&1
> - then
> - available="$available$tool$LF"
> - else
> - unavailable="$unavailable$tool$LF"
> - fi
> - done
> -
> cmd_name=${TOOL_MODE}tool
> + available=$(filter_tools 'mode_ok && is_available' '\t\t')
> + unavailable=$(filter_tools 'mode_ok && ! is_available' '\t\t')
> if test -n "$available"
> then
> echo "'git $cmd_name --tool=' may be set to one of the
> following:"
> - echo "$available" | sort | sed -e 's/^/ /'
> + printf "$available"
This may happen to be safe as available will not have anything with
a per-cent % in it, but is not a good discipline in general.
printf "%s" "$available"
If you are giving filter_tools an optional "prefix-per-line", I do
not think it is too much of a stretch to introduce another optional
"perfix for the whole thing" and let this call site say something
like this:
cmd_name=${TOOL_MODE}tool
show_tool_names 'mode_ok && is_available' '\t\t' \
"'git $cmd_name --tool=' may be set to one of these:"
show_tool_names 'mode_ok && !is_available' '\t\t' \
"These are valid but not available:"
without any of the above logic (and the same for unav). It may look like this:
show_tool_names () {
condition=${1?condition} per_line_prefix=${2:-} preamble=${3:-}
( cd "$MERGE_TOOLS_DIR && ls -1 * ) |
while read toolname
do
if setup_tool "$toolname" 2>/dev/null && (eval "$condition")
then
if test -n "$preamble"
then
echo "$preamble"
preamble=
fi
printf "%s%s\n" "$prefix" "$toolname"
fi
done
}
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] fetch-pack: avoid repeatedly re-scanning pack directory
Junio C Hamano wrote:
> It is not about a rough estimate nor common commits, though. The
> "everything local" check in question is interested in only one
> thing: are we _clearly_ up to date without fetching anything from
> them?
[...]
> Jonathan Nieder writes:
>> * Why is 49bb805e ("Do not ask for objects known to be complete",
>>2005-10-19) trying to do? Are we hurting that in any way?
>
> An earlier fetch may have acquired all the necessary objects but may
> not have updated our refs for some reason (e.g. fast-forward check
> may have fired). In such a case, we may already have a history that
> is good (i.e. not missing paths down to the common history) in our
> repository that is not connected to any of our refs, and we can
> update our refs (or write to FETCH_HEAD) without asking the remote
> end to do any common ancestor computation or object transfer.
>
> That was the primary thing the patch wanted to do.
Interesting.
After I fetch objects for branches a, b, and c which all have different
commit times in a fetch that fails due to the fast-forward check, I
have the following objects:
a commit date = 25 January 2013
b commit date = 26 January 2013
c commit date = 27 January 2013
When I try to fetch again (forcibly this time), git notices that the
objects are available locally and sets "cutoff" to 27 January 2013 as
a hint about the last time I fetched.
mark_recent_complete_commits() is called, and since these objects are
not reachable from any local refs, none are visited in the walk, and
49bb805e does not affect the outcome of the fetch positively or
negatively.
On the other hand, if I fetched a, b, and c to local branches and then
built on top of them, 49bb805e ensures that a second fetch of the same
refs will know right away not to request objects for c. So it brings
some of the benefits of 2759cbc7 (git-fetch-pack: avoid unnecessary
zero packing, 2005-10-18) when the receiving side has either (A)
renamed refs or (B) built new history on top of them.
Correct?
It is only in case (B) that the cutoff matters. If we miscalculate
the cutoff, that means either (i) cutoff == 0, and we would lose the
benefit of the walk that finds complete commits, or (ii) cutoff is a
little earlier, and the walk to find complete commits would take a
little longer. Neither effects the result of the fetch, and neither
is likely to be a significant enough performance difference to offset
the benefit of Jeff's patch.
Thanks for explaining.
Jonathan
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-remote-testpy: fix patch hashing on Python 3
John Keeping writes: > On Sun, Jan 27, 2013 at 12:47:09PM -0800, Junio C Hamano wrote: >> I remember that I earlier asked somewhere if we want to say "Python >> 3.x that is older than 3.y is unsupported" >> >> >> http://thread.gmane.org/gmane.comp.version-control.git/213920/focus=213926 >> >> but I was told that we will support all versions in 3.x series, IIRC. >> >> Does this patch contradict with that? If so I think we would need >> to revisit the update to CodingGuidelines in that thread. > > Yes. I'll send an update to that over the next couple of days. > > I think 3.1 and later is fine, when I said "Python 3.0 is unsupported" > in the commit message below, I meant "unsupported by the Python > developers". Yeah, I knew what you meant. I do not think it is so wrong to write 3.0 off as an early 0.x release of Python3 that was not yet usable for that exact reason. -- To unsubscribe from this list: send the line "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/4] mergetool--lib: Improve the help text in guess_merge_tool()
David Aguilar writes:
> This code path is only activated when the user does not have a valid
> configured tool. Add a message to guide new users towards configuring a
> default tool.
>
> Signed-off-by: David Aguilar
> ---
> git-mergetool--lib.sh | 9 -
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
> index 9a5aae9..cf52423 100644
> --- a/git-mergetool--lib.sh
> +++ b/git-mergetool--lib.sh
> @@ -240,7 +240,14 @@ show_tool_help () {
>
> guess_merge_tool () {
> list_merge_tool_candidates
> - echo >&2 "merge tool candidates: $tools"
> + msg="\
> +
> +This message is displayed because '$TOOL_MODE.tool' is not configured.
> +See 'git ${TOOL_MODE}tool --tool-help' or 'git help config' for more details.
> +'git ${TOOL_MODE}tool' will now attempt to use one of the following tools:
> +$tools
> +"
> + printf "$msg" >&2
This is not wrong per-se, but wouldn't it be much easier to read to
use an indented HERE-text like this?
cat >&2 <<-EOF
... a long message with $var substitution ...
... comes here ...
EOF
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] mergetool--lib: Add functions for finding available tools
On Sun, Jan 27, 2013 at 01:24:45PM -0800, David Aguilar wrote:
> +filter_tools () {
> + filter="$1"
> + prefix="$2"
> + (
> + cd "$MERGE_TOOLS_DIR" &&
> + for i in *
> + do
> + echo "$i"
> + done
> + ) | sort | while read tool
> + do
> + setup_tool "$tool" 2>/dev/null &&
> + (eval "$filter" "$tool") &&
> + printf "$prefix$tool\n"
> + done
> +}
Can we change this so that it does this:
filter_tools () {
filter="$1"
(
cd "$MERGE_TOOLS_DIR" &&
for i in *
do
echo "$i"
done
) |
while read tool
do
setup_tool "$tool" 2>/dev/null &&
(eval "$filter" "$tool") &&
echo "$tool"
done
}
and keep the sorting and prefix in show_tool_help? This will make it
easier to integrate the user-configured tools from git-config.
John
--
To unsubscribe from this list: send the line "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: [feature request] git add completion should exclude staged content
wookietreiber writes: > I have a feature request for `git add` auto completion: > > `git add` auto completion suggests all files / directories, > filtered by nothing. I guess it would be much nicer (as in > increasing productivity) if it would only suggest unstaged > content, as reported by `git status`, because that would be the > only content one would be able to add. I think that is what Manlio Perillo tried to do with the stalled mp/complete-paths topic that is queued in 'pu'. Manlio, any progress? -- To unsubscribe from this list: send the line "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 4/4] doc: Generate a list of valid merge tools
On Sun, Jan 27, 2013 at 01:24:46PM -0800, David Aguilar wrote:
> --- a/git-mergetool--lib.sh
> +++ b/git-mergetool--lib.sh
> @@ -1,5 +1,6 @@
> #!/bin/sh
> # git-mergetool--lib is a library for common merge tool functions
> +test -z "$MERGE_TOOLS_DIR" &&
> MERGE_TOOLS_DIR=$(git --exec-path)/mergetools
The preferred pattern in Git seems to be this:
: ${MERGE_TOOLS_DIR=$(git --exec-path)/mergetools}
John
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-remote-testpy: fix patch hashing on Python 3
On Sun, Jan 27, 2013 at 12:47:09PM -0800, Junio C Hamano wrote: > I remember that I earlier asked somewhere if we want to say "Python > 3.x that is older than 3.y is unsupported" > > http://thread.gmane.org/gmane.comp.version-control.git/213920/focus=213926 > > but I was told that we will support all versions in 3.x series, IIRC. > > Does this patch contradict with that? If so I think we would need > to revisit the update to CodingGuidelines in that thread. Yes. I'll send an update to that over the next couple of days. I think 3.1 and later is fine, when I said "Python 3.0 is unsupported" in the commit message below, I meant "unsupported by the Python developers". Support ended at least 3 months ago: http://hg.python.org/peps/rev/6d2e9d41dfaa John -- To unsubscribe from this list: send the line "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 4/4] doc: Generate a list of valid merge tools
David Aguilar writes: > +mergetools_txt = mergetools-diff.txt mergetools-merge.txt > + > +$(mergetools_txt): mergetools-list.made > + > +mergetools-list.made: ../git-mergetool--lib.sh $(wildcard ../mergetools/*) > + $(QUIET_GEN)$(RM) $@ && \ > + $(SHELL_PATH) -c 'MERGE_TOOLS_DIR=../mergetools && \ > + . ../git-mergetool--lib.sh && \ > + filter_tools can_diff "* "' > mergetools-diff.txt && \ > + $(SHELL_PATH) -c 'MERGE_TOOLS_DIR=../mergetools && \ > + . ../git-mergetool--lib.sh && \ > + filter_tools can_merge "* "' > mergetools-merge.txt && \ > + date > $@ Nicely done. By omitting is_available check and only checking can_*, we would get the same result on all platforms and I'd see tortoise appear in the output even I do not do Windows. 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 1/4] mergetool--lib: Simplify command expressions
David Aguilar writes: > Definitely. I learned this the hard way when the tests broke on me while > working it ;-) My patch rewrites things to always use var=$(command) > expressions with separate test "$var" evaluating them. OK; that wasn't clear from the log 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 3/4] mergetool--lib: Add functions for finding available tools
David Aguilar writes:
> Refactor show_tool_help() so that the tool-finding logic is broken out
> into separate functions.
>
> Signed-off-by: David Aguilar
> ---
> git-mergetool--lib.sh | 60
> +--
> 1 file changed, 34 insertions(+), 26 deletions(-)
>
> diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
> index cf52423..894b849 100644
> --- a/git-mergetool--lib.sh
> +++ b/git-mergetool--lib.sh
> @@ -2,6 +2,33 @@
> # git-mergetool--lib is a library for common merge tool functions
> MERGE_TOOLS_DIR=$(git --exec-path)/mergetools
>
> +mode_ok () {
> + diff_mode && can_diff ||
> + merge_mode && can_merge
> +}
I think you got the operator precedence mixed up.
What happens when diff_mode=true, can_diff=true, merge_mode=true and
can_merge=false?
if true && true || true && false
then
echo OK
else
echo NO
fi
if (true && true) || (true && false)
then
echo OK
else
echo NO
fi
> +is_available () {
> + merge_tool_path=$(translate_merge_tool_path "$1") &&
> + type "$merge_tool_path" >/dev/null 2>&1
> +}
> +
> +filter_tools () {
> + filter="$1"
> + prefix="$2"
> + (
> + cd "$MERGE_TOOLS_DIR" &&
> + for i in *
> + do
> + echo "$i"
> + done
> + ) | sort | while read tool
> + do
Please start a new line before keywords that define the syntactic
structure, like "while", i.e.
... piped | commands |
while read tool
do
...
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] mergetool--lib: Add functions for finding available tools
Am 27.01.2013 22:24, schrieb David Aguilar:
> Refactor show_tool_help() so that the tool-finding logic is broken out
> into separate functions.
>
> Signed-off-by: David Aguilar
> ---
> git-mergetool--lib.sh | 60
> +--
> 1 file changed, 34 insertions(+), 26 deletions(-)
>
> diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
> index cf52423..894b849 100644
> --- a/git-mergetool--lib.sh
> +++ b/git-mergetool--lib.sh
> @@ -2,6 +2,33 @@
> # git-mergetool--lib is a library for common merge tool functions
> MERGE_TOOLS_DIR=$(git --exec-path)/mergetools
>
> +mode_ok () {
> + diff_mode && can_diff ||
> + merge_mode && can_merge
> +}
&& and || have the same precedence: if diff_mode and can_diff both are
"true", then the result of the function is that of can_merge. I don't
think that is what is intended.
> +filter_tools () {
> + filter="$1"
> + prefix="$2"
> + (
> + cd "$MERGE_TOOLS_DIR" &&
> + for i in *
> + do
> + echo "$i"
> + done
cd "$MERGE_TOOLS_DIR" &&
printf "%s\n" *
But what's wrong with "ls -1"? It would save the explicit sort.
> + ) | sort | while read tool
> + do
> + setup_tool "$tool" 2>/dev/null &&
> + (eval "$filter" "$tool") &&
> + printf "$prefix$tool\n"
> + done
> +}
-- Hannes
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] mergetool--lib: Simplify command expressions
David Aguilar writes: > Use $(command "$arg") instead of "$(command "$arg")" as the latter is > harder to read. Did you miss my comment that this is about RHS of an assignment? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] mergetool--lib: Simplify command expressions
On Sun, Jan 27, 2013 at 2:08 PM, Johannes Sixt wrote: > Am 27.01.2013 22:24, schrieb David Aguilar: >> Use $(command "$arg") instead of "$(command "$arg")" as the latter is >> harder to read. > > If at all, you should restrict yourself to simplify only variable > assignments. Because this case: > >> - if test -z "$(get_merge_tool_cmd "$merge_tool")" && >> + if test -z $(get_merge_tool_cmd "$merge_tool") && > > cannot work as intended: If the output of $() is empty, then without the > outer quotes this becomes > > test -z > > without an operand for -z, which is a syntax error (of the test command). Definitely. I learned this the hard way when the tests broke on me while working it ;-) My patch rewrites things to always use var=$(command) expressions with separate test "$var" evaluating them. Thanks for the tip, -- David -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] mergetool--lib: Simplify command expressions
Am 27.01.2013 22:24, schrieb David Aguilar: > Use $(command "$arg") instead of "$(command "$arg")" as the latter is > harder to read. If at all, you should restrict yourself to simplify only variable assignments. Because this case: > - if test -z "$(get_merge_tool_cmd "$merge_tool")" && > + if test -z $(get_merge_tool_cmd "$merge_tool") && cannot work as intended: If the output of $() is empty, then without the outer quotes this becomes test -z without an operand for -z, which is a syntax error (of the test command). -- Hannes -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/4] mergetool--lib: Add functions for finding available tools
Refactor show_tool_help() so that the tool-finding logic is broken out
into separate functions.
Signed-off-by: David Aguilar
---
git-mergetool--lib.sh | 60 +--
1 file changed, 34 insertions(+), 26 deletions(-)
diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index cf52423..894b849 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -2,6 +2,33 @@
# git-mergetool--lib is a library for common merge tool functions
MERGE_TOOLS_DIR=$(git --exec-path)/mergetools
+mode_ok () {
+ diff_mode && can_diff ||
+ merge_mode && can_merge
+}
+
+is_available () {
+ merge_tool_path=$(translate_merge_tool_path "$1") &&
+ type "$merge_tool_path" >/dev/null 2>&1
+}
+
+filter_tools () {
+ filter="$1"
+ prefix="$2"
+ (
+ cd "$MERGE_TOOLS_DIR" &&
+ for i in *
+ do
+ echo "$i"
+ done
+ ) | sort | while read tool
+ do
+ setup_tool "$tool" 2>/dev/null &&
+ (eval "$filter" "$tool") &&
+ printf "$prefix$tool\n"
+ done
+}
+
diff_mode() {
test "$TOOL_MODE" = diff
}
@@ -199,27 +226,13 @@ list_merge_tool_candidates () {
}
show_tool_help () {
- unavailable= available= LF='
-'
- for i in "$MERGE_TOOLS_DIR"/*
- do
- tool=$(basename "$i")
- setup_tool "$tool" 2>/dev/null || continue
-
- merge_tool_path=$(translate_merge_tool_path "$tool")
- if type "$merge_tool_path" >/dev/null 2>&1
- then
- available="$available$tool$LF"
- else
- unavailable="$unavailable$tool$LF"
- fi
- done
-
cmd_name=${TOOL_MODE}tool
+ available=$(filter_tools 'mode_ok && is_available' '\t\t')
+ unavailable=$(filter_tools 'mode_ok && ! is_available' '\t\t')
if test -n "$available"
then
echo "'git $cmd_name --tool=' may be set to one of the
following:"
- echo "$available" | sort | sed -e 's/^/ /'
+ printf "$available"
else
echo "No suitable tool for 'git $cmd_name --tool=' found."
fi
@@ -227,7 +240,7 @@ show_tool_help () {
then
echo
echo 'The following tools are valid, but not currently
available:'
- echo "$unavailable" | sort | sed -e 's/^/ /'
+ printf "$unavailable"
fi
if test -n "$unavailable$available"
then
@@ -250,17 +263,12 @@ $tools
printf "$msg" >&2
# Loop over each candidate and stop when a valid merge tool is found.
- for i in $tools
+ for tool in $tools
do
- merge_tool_path=$(translate_merge_tool_path "$i")
- if type "$merge_tool_path" >/dev/null 2>&1
- then
- echo "$i"
- return 0
- fi
+ is_available "$tool" && echo "$tool" && return 0
done
- echo >&2 "No known merge resolution program available."
+ echo >&2 "No known ${TOOL_MODE} tool is available."
return 1
}
--
1.8.0.13.gf25ae33
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/4] doc: Generate a list of valid merge tools
Use the new filter_tools() function to build lists of all
the built-in tools supported by difftool and mergetool.
This frees us from needing to update the documentation
whenever a new tool is added.
Signed-off-by: David Aguilar
---
Documentation/.gitignore | 1 +
Documentation/Makefile | 16 +++-
Documentation/diff-config.txt | 13 +++--
Documentation/merge-config.txt | 12 ++--
git-mergetool--lib.sh | 1 +
5 files changed, 30 insertions(+), 13 deletions(-)
diff --git a/Documentation/.gitignore b/Documentation/.gitignore
index d62aebd..2c8b2d6 100644
--- a/Documentation/.gitignore
+++ b/Documentation/.gitignore
@@ -9,4 +9,5 @@ gitman.info
howto-index.txt
doc.dep
cmds-*.txt
+mergetools-*.txt
manpage-base-url.xsl
diff --git a/Documentation/Makefile b/Documentation/Makefile
index 267dfe1..f595d26 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -226,13 +226,27 @@ cmd-list.made: cmd-list.perl ../command-list.txt
$(MAN1_TXT)
$(PERL_PATH) ./cmd-list.perl ../command-list.txt $(QUIET_STDERR) && \
date >$@
+mergetools_txt = mergetools-diff.txt mergetools-merge.txt
+
+$(mergetools_txt): mergetools-list.made
+
+mergetools-list.made: ../git-mergetool--lib.sh $(wildcard ../mergetools/*)
+ $(QUIET_GEN)$(RM) $@ && \
+ $(SHELL_PATH) -c 'MERGE_TOOLS_DIR=../mergetools && \
+ . ../git-mergetool--lib.sh && \
+ filter_tools can_diff "* "' > mergetools-diff.txt && \
+ $(SHELL_PATH) -c 'MERGE_TOOLS_DIR=../mergetools && \
+ . ../git-mergetool--lib.sh && \
+ filter_tools can_merge "* "' > mergetools-merge.txt && \
+ date > $@
+
clean:
$(RM) *.xml *.xml+ *.html *.html+ *.1 *.5 *.7
$(RM) *.texi *.texi+ *.texi++ git.info gitman.info
$(RM) *.pdf
$(RM) howto-index.txt howto/*.html doc.dep
$(RM) technical/api-*.html technical/api-index.txt
- $(RM) $(cmds_txt) *.made
+ $(RM) $(cmds_txt) $(mergetools_txt) *.made
$(RM) manpage-base-url.xsl
$(MAN_HTML): %.html : %.txt
diff --git a/Documentation/diff-config.txt b/Documentation/diff-config.txt
index 67a90a8..7c968d1 100644
--- a/Documentation/diff-config.txt
+++ b/Documentation/diff-config.txt
@@ -132,9 +132,10 @@ diff..cachetextconv::
conversion outputs. See linkgit:gitattributes[5] for details.
diff.tool::
- The diff tool to be used by linkgit:git-difftool[1]. This
- option overrides `merge.tool`, and has the same valid built-in
- values as `merge.tool` minus "tortoisemerge" and plus
- "kompare". Any other value is treated as a custom diff tool,
- and there must be a corresponding `difftool..cmd`
- option.
+ Controls which diff tool is used by linkgit:git-difftool[1].
+ This variable overrides the value configured in `merge.tool`.
+ The list below shows the valid built-in values.
+ Any other value is treated as a custom diff tool and requires
+ that a corresponding difftool..cmd variable is defined.
+
+include::mergetools-diff.txt[]
diff --git a/Documentation/merge-config.txt b/Documentation/merge-config.txt
index 861bd6f..5f40e71 100644
--- a/Documentation/merge-config.txt
+++ b/Documentation/merge-config.txt
@@ -52,12 +52,12 @@ merge.stat::
at the end of the merge. True by default.
merge.tool::
- Controls which merge resolution program is used by
- linkgit:git-mergetool[1]. Valid built-in values are: "araxis",
- "bc3", "diffuse", "ecmerge", "emerge", "gvimdiff", "kdiff3", "meld",
- "opendiff", "p4merge", "tkdiff", "tortoisemerge", "vimdiff"
- and "xxdiff". Any other value is treated is custom merge tool
- and there must be a corresponding mergetool..cmd option.
+ Controls which merge tool is used by linkgit:git-mergetool[1].
+ The list below shows the valid built-in values.
+ Any other value is treated as a custom merge tool and requires
+ that a corresponding mergetool..cmd variable is defined.
+
+include::mergetools-merge.txt[]
merge.verbosity::
Controls the amount of output shown by the recursive merge
diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index 894b849..59e1650 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -1,5 +1,6 @@
#!/bin/sh
# git-mergetool--lib is a library for common merge tool functions
+test -z "$MERGE_TOOLS_DIR" &&
MERGE_TOOLS_DIR=$(git --exec-path)/mergetools
mode_ok () {
--
1.8.0.13.gf25ae33
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/4] mergetool--lib: Improve the help text in guess_merge_tool()
This code path is only activated when the user does not have a valid
configured tool. Add a message to guide new users towards configuring a
default tool.
Signed-off-by: David Aguilar
---
git-mergetool--lib.sh | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index 9a5aae9..cf52423 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -240,7 +240,14 @@ show_tool_help () {
guess_merge_tool () {
list_merge_tool_candidates
- echo >&2 "merge tool candidates: $tools"
+ msg="\
+
+This message is displayed because '$TOOL_MODE.tool' is not configured.
+See 'git ${TOOL_MODE}tool --tool-help' or 'git help config' for more details.
+'git ${TOOL_MODE}tool' will now attempt to use one of the following tools:
+$tools
+"
+ printf "$msg" >&2
# Loop over each candidate and stop when a valid merge tool is found.
for i in $tools
--
1.8.0.13.gf25ae33
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/4] mergetool--lib: Simplify command expressions
Use $(command "$arg") instead of "$(command "$arg")" as the latter is
harder to read. Make the expression in get_merge_tool_cmd() even
simpler by avoiding "echo" completely.
Signed-off-by: David Aguilar
---
git-mergetool--lib.sh | 40
1 file changed, 16 insertions(+), 24 deletions(-)
diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index 1d0fb12..9a5aae9 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -32,17 +32,10 @@ check_unchanged () {
fi
}
-valid_tool_config () {
- if test -n "$(get_merge_tool_cmd "$1")"
- then
- return 0
- else
- return 1
- fi
-}
-
valid_tool () {
- setup_tool "$1" || valid_tool_config "$1"
+ setup_tool "$1" && return 0
+ cmd=$(get_merge_tool_cmd "$1")
+ test -n "$cmd"
}
setup_tool () {
@@ -96,14 +89,13 @@ setup_tool () {
}
get_merge_tool_cmd () {
- # Prints the custom command for a merge tool
merge_tool="$1"
if diff_mode
then
- echo "$(git config difftool.$merge_tool.cmd ||
- git config mergetool.$merge_tool.cmd)"
+ git config "difftool.$merge_tool.cmd" ||
+ git config "mergetool.$merge_tool.cmd"
else
- echo "$(git config mergetool.$merge_tool.cmd)"
+ git config "mergetool.$merge_tool.cmd"
fi
}
@@ -114,7 +106,7 @@ run_merge_tool () {
GIT_PREFIX=${GIT_PREFIX:-.}
export GIT_PREFIX
- merge_tool_path="$(get_merge_tool_path "$1")" || exit
+ merge_tool_path=$(get_merge_tool_path "$1") || exit
base_present="$2"
status=0
@@ -145,7 +137,7 @@ run_merge_tool () {
# Run a either a configured or built-in diff tool
run_diff_cmd () {
- merge_tool_cmd="$(get_merge_tool_cmd "$1")"
+ merge_tool_cmd=$(get_merge_tool_cmd "$1")
if test -n "$merge_tool_cmd"
then
( eval $merge_tool_cmd )
@@ -158,11 +150,11 @@ run_diff_cmd () {
# Run a either a configured or built-in merge tool
run_merge_cmd () {
- merge_tool_cmd="$(get_merge_tool_cmd "$1")"
+ merge_tool_cmd=$(get_merge_tool_cmd "$1")
if test -n "$merge_tool_cmd"
then
- trust_exit_code="$(git config --bool \
- mergetool."$1".trustExitCode || echo false)"
+ trust_exit_code=$(git config --bool \
+ "mergetool.$1.trustExitCode" || echo false)
if test "$trust_exit_code" = "false"
then
touch "$BACKUP"
@@ -253,7 +245,7 @@ guess_merge_tool () {
# Loop over each candidate and stop when a valid merge tool is found.
for i in $tools
do
- merge_tool_path="$(translate_merge_tool_path "$i")"
+ merge_tool_path=$(translate_merge_tool_path "$i")
if type "$merge_tool_path" >/dev/null 2>&1
then
echo "$i"
@@ -300,9 +292,9 @@ get_merge_tool_path () {
fi
if test -z "$merge_tool_path"
then
- merge_tool_path="$(translate_merge_tool_path "$merge_tool")"
+ merge_tool_path=$(translate_merge_tool_path "$merge_tool")
fi
- if test -z "$(get_merge_tool_cmd "$merge_tool")" &&
+ if test -z $(get_merge_tool_cmd "$merge_tool") &&
! type "$merge_tool_path" >/dev/null 2>&1
then
echo >&2 "The $TOOL_MODE tool $merge_tool is not available as"\
@@ -314,11 +306,11 @@ get_merge_tool_path () {
get_merge_tool () {
# Check if a merge tool has been configured
- merge_tool="$(get_configured_merge_tool)"
+ merge_tool=$(get_configured_merge_tool)
# Try to guess an appropriate merge tool if no tool has been set.
if test -z "$merge_tool"
then
- merge_tool="$(guess_merge_tool)" || exit
+ merge_tool=$(guess_merge_tool) || exit
fi
echo "$merge_tool"
}
--
1.8.0.13.gf25ae33
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/4] Documentation: Auto-generate merge tool lists
Refactor the mergetool-lib so that we can reuse it in Documentation/Makefile. The end result is that the diff.tool and merge.tool documentation now includes an auto-generated list of all available tools. This applies on top of jk/mergetool in pu. David Aguilar (4): mergetool--lib: Simplify command expressions mergetool--lib: Improve the help text in guess_merge_tool() mergetool--lib: Add functions for finding available tools doc: Generate a list of valid merge tools Documentation/.gitignore | 1 + Documentation/Makefile | 16 +- Documentation/diff-config.txt | 13 ++--- Documentation/merge-config.txt | 12 ++--- git-mergetool--lib.sh | 108 ++--- 5 files changed, 87 insertions(+), 63 deletions(-) -- 1.8.0.13.gf25ae33 -- To unsubscribe from this list: send the line "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: mergetool: include custom tools in '--tool-help'
On Sun, Jan 27, 2013 at 12:13 PM, Junio C Hamano wrote: > John Keeping writes: > >> I think I'd want to do this with a suffix if at all, so the output would >> be like this: >> >> 'git mergetool --tool=' may be set to one of the following: >> >> araxis >> gvimdiff >> gvimdiff2 >> mytool(user-defined) >> vimdiff >> vimdiff2 > > That is fine by me, but the real users of mergetool please feel free > to raise objections. This seems pretty useful. I did a bit of refactoring last night that I'd like to post here, the end result being something that's plugged into Documentation/. I think what I did may also help add this functionality, and could be useful to build upon. I'll send my patches shortly so you can take a look. Basically, I added a simple way to loop over the tools and filter them. This is reused in show_tool_help() and Documentation/Makefile. The refactoring changes how show_tool_help() works, so I'd like you to take a look before we add a new feature since it might make it easier to do. -- David -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-remote-testpy: fix patch hashing on Python 3
Junio C Hamano writes:
> John Keeping writes:
>
>> So I think the answer is "habit, but I probably shouldn't have put it
>> in in this case".
>
> OK, then I'll queue with a local amend to drop the leading
> underscore.
So this is what I will be queuing (I'd appreciate the second set of
eyes, though), with the leading-underscore removal and log message
typofixes.
I remember that I earlier asked somewhere if we want to say "Python
3.x that is older than 3.y is unsupported"
http://thread.gmane.org/gmane.comp.version-control.git/213920/focus=213926
but I was told that we will support all versions in 3.x series, IIRC.
Does this patch contradict with that? If so I think we would need
to revisit the update to CodingGuidelines in that thread.
I am perfectly fine with discarding early 3.x as "0.x releases of
Python3", but I would want to see our document say so if that is
what we do.
-- >8 --
From: John Keeping
Date: Sun, 27 Jan 2013 14:50:56 +
Subject: [PATCH] git-remote-testpy: fix path hashing on Python 3
When this change was originally made (0846b0c - git-remote-testpy:
hash bytes explicitly , I didn't realise that the "hex" encoding we
chose is a "bytes to bytes" encoding so it just fails with an error
on Python 3 in the same way as the original code.
It is not possible to provide a single code path that works on
Python 2 and Python 3 since Python 2.x will attempt to decode the
string before encoding it, which fails for strings that are not
valid in the default encoding. Python 3.1 introduced the
"surrogateescape" error handler which handles this correctly and
permits a bytes -> unicode -> bytes round-trip to be lossless.
At this point Python 3.0 is unsupported so we don't go out of our
way to try to support it.
Helped-by: Michael Haggerty
Signed-off-by: John Keeping
Signed-off-by: Junio C Hamano
---
git-remote-testpy.py | 18 +-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/git-remote-testpy.py b/git-remote-testpy.py
index c7a04ec..6098bdd 100644
--- a/git-remote-testpy.py
+++ b/git-remote-testpy.py
@@ -36,6 +36,22 @@ if sys.hexversion < 0x0200:
sys.stderr.write("git-remote-testgit: requires Python 2.0 or later.\n")
sys.exit(1)
+
+def encode_filepath(path):
+"""Encodes a Unicode file path to a byte string.
+
+On Python 2 this is a no-op; on Python 3 we encode the string as
+suggested by [1] which allows an exact round-trip from the command line
+to the filesystem.
+
+[1] http://docs.python.org/3/c-api/unicode.html#file-system-encoding
+
+"""
+if sys.hexversion < 0x0300:
+return path
+return path.encode('utf-8', 'surrogateescape')
+
+
def get_repo(alias, url):
"""Returns a git repository object initialized for usage.
"""
@@ -45,7 +61,7 @@ def get_repo(alias, url):
repo.get_head()
hasher = _digest()
-hasher.update(repo.path.encode('hex'))
+hasher.update(encode_filepath(repo.path))
repo.hash = hasher.hexdigest()
repo.get_base_path = lambda base: os.path.join(
--
1.8.1.1.550.g40037fd
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-remote-testpy: fix patch hashing on Python 3
John Keeping writes: > On Sun, Jan 27, 2013 at 12:11:20PM -0800, Junio C Hamano wrote: >> John Keeping writes: >> >> >> Thanks; will queue and wait for an Ack from Michael. >> >> >> >> Does the helper function need to be named with leading underscore, >> >> though? >> > >> > ... Since this is a script >> > not a library module I don't feel strongly about it in this case. >> >> That is exactly why I asked. > > So I think the answer is "habit, but I probably shouldn't have put it > in in this case". OK, then I'll queue with a local amend to drop the leading underscore. 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 v2] add: warn when -u or -A is used without filepattern
Matthieu Moy writes:
> Plus, option_with_implicit_dot is used in cut-and-paste ready commands
> below.
I do not think we should aim for easy cut-and-paste, especially when
the real purpose of the change is to train people's fingers; the
message should discouraging cut-and-paste in a case like this, if
anything.
But we could obviously do this, if you really want to cut-and-paste.
builtin/add.c | 29 ++---
1 file changed, 18 insertions(+), 11 deletions(-)
diff --git a/builtin/add.c b/builtin/add.c
index 7552f7f..ba72a57 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -363,7 +363,7 @@ static int add_files(struct dir_struct *dir, int flags)
return exit_status;
}
-static void warn_pathless_add(const char *option_name) {
+static void warn_pathless_add(const char *option_name, const char *short_name)
{
/*
* To be consistent with "git add -p" and most Git
* commands, we should default to being tree-wide, but
@@ -374,20 +374,21 @@ static void warn_pathless_add(const char *option_name) {
* turned into a die(...), and eventually we may
* reallow the command with a new behavior.
*/
- warning(_("The behavior of 'git add %s' with no path argument from a
subdirectory of the\n"
- "tree will change in Git 2.0 and shouldn't be used anymore.\n"
+ warning(_("The behavior of 'git add %s (or %s)' with no path argument
from a\n"
+ "subdirectory of the tree will change in Git 2.0 and should
not be\n"
+ "used anymore.\n"
"To add content for the whole tree, run:\n"
"\n"
- " git add %s :/\n"
+ " git add %s :/ ;# or git add %s :/\n"
"\n"
"To restrict the command to the current directory, run:\n"
"\n"
- " git add %s .\n"
+ " git add %s . ;# or git add %s .\n"
"\n"
"With the current Git version, the command is restricted to
the current directory."),
- option_name,
- option_name,
- option_name);
+ option_name, short_name,
+ option_name, short_name,
+ option_name, short_name);
}
int cmd_add(int argc, const char **argv, const char *prefix)
@@ -401,6 +402,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
int require_pathspec;
char *seen = NULL;
const char *option_with_implicit_dot = NULL;
+ const char *short_option_with_implicit_dot = NULL;
git_config(add_config, NULL);
@@ -420,14 +422,19 @@ int cmd_add(int argc, const char **argv, const char
*prefix)
die(_("-A and -u are mutually incompatible"));
if (!show_only && ignore_missing)
die(_("Option --ignore-missing can only be used together with
--dry-run"));
- if (addremove)
+ if (addremove) {
option_with_implicit_dot = "--all";
- if (take_worktree_changes)
+ short_option_with_implicit_dot = "-A";
+ }
+ if (take_worktree_changes) {
option_with_implicit_dot = "--update";
+ short_option_with_implicit_dot = "-u";
+ }
if (option_with_implicit_dot && !argc) {
static const char *here[2] = { ".", NULL };
if (prefix)
- warn_pathless_add(option_with_implicit_dot);
+ warn_pathless_add(option_with_implicit_dot,
+ short_option_with_implicit_dot);
argc = 1;
argv = here;
}
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] tests: turn on test-lint-shell-syntax by default
Junio C Hamano writes:
> If we did not care about incurring runtime performance cost, we
> could arrange:
> ...
> Then you can wrap commands whose use we want to limit, perhaps like
> this, in the test framework:
> ...
> sed () {
> ...
> done
> if test -z "$must_abort"
> sed "$@"
> fi
> }
Of course, aside from missing "then", this needs to use the
real "sed", so this has to be
if test -z "$must_abort"
then
command sed "$@"
fi
or something like that.
An approach along this line may reduce both the false negatives and
false positives down to an acceptable level, but I doubt the result
would be efficient enough for us to tolerate the runtime penalty.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-remote-testpy: fix patch hashing on Python 3
On Sun, Jan 27, 2013 at 12:11:20PM -0800, Junio C Hamano wrote: > John Keeping writes: > > >> Thanks; will queue and wait for an Ack from Michael. > >> > >> Does the helper function need to be named with leading underscore, > >> though? > > > > ... Since this is a script > > not a library module I don't feel strongly about it in this case. > > That is exactly why I asked. So I think the answer is "habit, but I probably shouldn't have put it in in this case". John -- To unsubscribe from this list: send the line "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: mergetool: include custom tools in '--tool-help'
John Keeping writes: > I think I'd want to do this with a suffix if at all, so the output would > be like this: > > 'git mergetool --tool=' may be set to one of the following: > > araxis > gvimdiff > gvimdiff2 > mytool(user-defined) > vimdiff > vimdiff2 That is fine by me, but the real users of mergetool please feel free to raise objections. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-remote-testpy: fix patch hashing on Python 3
John Keeping writes: >> Thanks; will queue and wait for an Ack from Michael. >> >> Does the helper function need to be named with leading underscore, >> though? > > ... Since this is a script > not a library module I don't feel strongly about it in this case. That is exactly why I asked. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] fetch-pack: avoid repeatedly re-scanning pack directory
Jonathan Nieder writes:
> Jeff King wrote:
>
>> When we look up a sha1 object for reading, we first check
>> packfiles, and then loose objects. If we still haven't found
>> it, we re-scan the list of packfiles in `objects/pack`. This
>> final step ensures that we can co-exist with a simultaneous
>> repack process which creates a new pack and then prunes the
>> old object.
>
> I like the context above and what follows it, but I think you forgot
> to mention what the patch actually does. :)
>
> I guess it is:
>
> However, in the first scan over refs in fetch-pack.c::everything_local,
> this double-check of packfiles is not necessary since we are only
> trying to get a rough estimate of the last time we fetched from this
> remote repository in order to find good candidate common commits ---
> a missed object would only result in a slightly slower fetch.
It is not about a rough estimate nor common commits, though. The
"everything local" check in question is interested in only one
thing: are we _clearly_ up to date without fetching anything from
them?
Loosening the check may miss the rare case where we race against a
simultaneous repack and will cause us to go to the network when we
do not have to, and it becomes a trade off between the common unracy
case going faster by allowing the "Are we clearly up to date" check
to cheat, at the expense of rare racy cases suffering unnecessary
object transfer overhead.
> Avoid that slow second scan in the common case by guarding the object
> lookup with has_sha1_file().
This conclusion is correct.
> I had not read this codepath before. I'm left with a few questions:
>
> * Why is 49bb805e ("Do not ask for objects known to be complete",
>2005-10-19) trying to do? Are we hurting that in any way?
An earlier fetch may have acquired all the necessary objects but may
not have updated our refs for some reason (e.g. fast-forward check
may have fired). In such a case, we may already have a history that
is good (i.e. not missing paths down to the common history) in our
repository that is not connected to any of our refs, and we can
update our refs (or write to FETCH_HEAD) without asking the remote
end to do any common ancestor computation or object transfer.
That was the primary thing the patch wanted to do.
As a side-effect, we know more objects than just the objects at the
tips of our refs are complete and that may help the later common
history discovery step, but obviously we do not want to dig the
history down to root. The cutoff value is merely a heuristics
chosen without any deep thought.
> * Is has_sha1_file() generally succeptible to the race against repack
>you mentioned? How is that normally dealt with?
By failing to find, so that the user will restart. When the caller
really wants to use the object, parse_objects() => read_sha1_file()
=> read_object() is used and we will see the retry.
> * Can a slow operation get confused if an object is incorporated into
>a pack and then expelled again by two repacks in sequence?
If it checks "the object should be there" first, wait for a long
time, and then tries to find that object's data, the later access
will go to the parse_objects() callpath and I think it should do the
right thing. If that slow opearation stops inside read_object(), it
could find it unable to map the loose object file and then unable to
find it in the pack, either. Is that what you are worried about?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-remote-testpy: fix patch hashing on Python 3
On Sun, Jan 27, 2013 at 11:49:39AM -0800, Junio C Hamano wrote: > John Keeping writes: > > > When this change was originally made (0846b0c - git-remote-testpy: hash > > bytes explicitly , I didn't realised that the "hex" encoding we chose is > > a "bytes to bytes" encoding so it just fails with an error on Python 3 > > in the same way as the original code. > > > > It is not possible to provide a single code path that works on Python 2 > > and Python 3 since Python 2.x will attempt to decode the string before > > encoding it, which fails for strings that are not valid in the default > > encoding. Python 3.1 introduced the "surrogateescape" error handler > > which handles this correctly and permits a bytes -> unicode -> bytes > > round-trip to be lossless. > > > > At this point Python 3.0 is unsupported so we don't go out of our way to > > try to support it. > > > > Helped-by: Michael Haggerty > > Signed-off-by: John Keeping > > --- > > Thanks; will queue and wait for an Ack from Michael. > > Does the helper function need to be named with leading underscore, > though? It's a Python convention for internal functions. Since this is a script not a library module I don't feel strongly about it in this case. John -- To unsubscribe from this list: send the line "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: mergetool: include custom tools in '--tool-help'
On Sun, Jan 27, 2013 at 10:03:19AM -0800, Junio C Hamano wrote: > John Keeping writes: > > > 'git mergetool --tool-help' only lists builtin tools, not those that the > > user has configured via a 'mergetool..cmd' config value. Fix this > > by inspecting the tools configured in this way and adding them to the > > available and unavailable lists before displaying them. > > Although I am not a mergetool user, I would imagine that it would > make sense to show it as available. > > Just like "git help -a" lists subcommands in a way that can be easy > to tell which ones are the standard ones and which ones are user > customizations, this may want to give a similar distinction, though. > I dunno. I think I'd want to do this with a suffix if at all, so the output would be like this: 'git mergetool --tool=' may be set to one of the following: araxis gvimdiff gvimdiff2 mytool (user-defined) vimdiff vimdiff2 The following tools are valid, but not currently available: bc3 codecompare deltawalker diffuse ecmerge emerge kdiff3 meld opendiff p4merge tkdiff tortoisemerge xxdiff Some of the tools listed above only work in a windowed environment. If run in a terminal-only session, they will fail. Adding more sections for the user-defined tools feels like the output would be too imposing and would make it hard to immediately identify the valid option. John -- To unsubscribe from this list: send the line "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: Behavior of stash apply vs merge
- Ursprungligt meddelande - > Robin Rosenberg writes: > > > Thanks. Feeling a bit studid now. > > > > I was actually thinking about using merge to implement stash apply > > in JGit. What we have is broken so I tried using merge to implement > > it and them compared to git merge --no-commit.. FAIL. > > Do you have "cherry-pick"? > > In short, "stash apply" is a "cherry-pick" in disguise. Yes, that's what I did. Thanks for confirming this. One for the working tree and if that succeeds I do another one to restore the index if requested. -- robin -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-remote-testpy: fix patch hashing on Python 3
John Keeping writes:
> When this change was originally made (0846b0c - git-remote-testpy: hash
> bytes explicitly , I didn't realised that the "hex" encoding we chose is
> a "bytes to bytes" encoding so it just fails with an error on Python 3
> in the same way as the original code.
>
> It is not possible to provide a single code path that works on Python 2
> and Python 3 since Python 2.x will attempt to decode the string before
> encoding it, which fails for strings that are not valid in the default
> encoding. Python 3.1 introduced the "surrogateescape" error handler
> which handles this correctly and permits a bytes -> unicode -> bytes
> round-trip to be lossless.
>
> At this point Python 3.0 is unsupported so we don't go out of our way to
> try to support it.
>
> Helped-by: Michael Haggerty
> Signed-off-by: John Keeping
> ---
Thanks; will queue and wait for an Ack from Michael.
Does the helper function need to be named with leading underscore,
though?
> On Sun, Jan 27, 2013 at 02:13:29PM +, John Keeping wrote:
>> On Sun, Jan 27, 2013 at 05:44:37AM +0100, Michael Haggerty wrote:
>> > So to handle all of the cases across Python versions as closely as
>> > possible to the old 2.x code, it might be necessary to make the code
>> > explicitly depend on the Python version number, like:
>> >
>> > hasher = _digest()
>> > if sys.hexversion < 0x0300:
>> > pathbytes = repo.path
>> > elif sys.hexversion < 0x0301:
>> > # If support for Python 3.0.x is desired (note: result can
>> > # be different in this case than under 2.x or 3.1+):
>> > pathbytes = repo.path.encode(sys.getfilesystemencoding(),
>> > 'backslashreplace')
>> > else
>> > pathbytes = repo.path.encode(sys.getfilesystemencoding(),
>> > 'surrogateescape')
>> > hasher.update(pathbytes)
>> > repo.hash = hasher.hexdigest()
>
> How about this?
>
> git-remote-testpy.py | 18 +-
> 1 file changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/git-remote-testpy.py b/git-remote-testpy.py
> index c7a04ec..16b0c52 100644
> --- a/git-remote-testpy.py
> +++ b/git-remote-testpy.py
> @@ -36,6 +36,22 @@ if sys.hexversion < 0x0200:
> sys.stderr.write("git-remote-testgit: requires Python 2.0 or later.\n")
> sys.exit(1)
>
> +
> +def _encode_filepath(path):
> +"""Encodes a Unicode file path to a byte string.
> +
> +On Python 2 this is a no-op; on Python 3 we encode the string as
> +suggested by [1] which allows an exact round-trip from the command line
> +to the filesystem.
> +
> +[1] http://docs.python.org/3/c-api/unicode.html#file-system-encoding
> +
> +"""
> +if sys.hexversion < 0x0300:
> +return path
> +return path.encode('utf-8', 'surrogateescape')
> +
> +
> def get_repo(alias, url):
> """Returns a git repository object initialized for usage.
> """
> @@ -45,7 +61,7 @@ def get_repo(alias, url):
> repo.get_head()
>
> hasher = _digest()
> -hasher.update(repo.path.encode('hex'))
> +hasher.update(_encode_filepath(repo.path))
> repo.hash = hasher.hexdigest()
>
> repo.get_base_path = lambda base: os.path.join(
--
To unsubscribe from this list: send the line "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 1/2] for-each-repo: new command used for multi-repo operations
John Keeping writes: > On Sun, Jan 27, 2013 at 11:04:08AM -0800, Junio C Hamano wrote: >> One more thing that nobody brought up during the previous reviews is >> if we want to support subset of repositories by allowing the >> standard pathspec match mechanism. For example, >> >> git for-each-repo -d git diff --name-only -- foo/ bar/b\*z >> >> might be a way to ask "please find repositories match the given >> pathspecs (i.e. foo/ bar/b\*z) and run the command in the ones that >> are dirty". We would need to think about how to mark the end of the >> command though---we could borrow \; from find(1), even though find >> is not the best example of the UI design. I.e. >> >> git for-each-repo -d git diff --name-only \; [--] foo/ bar/b\*z >> >> with or without "--". > > Would it be better to make this a (multi-valued) option? > > git for-each-repo -d --filter=foo/ --filter=bar/b\*z git diff --name-only The standard way to use filtering based on paths we have is to use the pathspec parameters at the end of the commmand line. I see no reason for such an inconsistency with an option like --filter. >> Oh, that reminds me of another thing. Perhaps we would want to >> export the (relative) path to the found repository in some way to >> allow the commands to do this kind of thing in the first place? >> "submodule foreach" does this with $path, I think. > > I think $path is the only variable exported by "submodule foreach" which > is applicable here, but it doesn't work on Windows, where environment > variables are case-insensitive. > > Commit 64394e3 (git-submodule.sh: Don't use $path variable in > eval_gettext string) changed "submodule foreach" to use $sm_path > internally although I notice that the documentation still uses $path. > > Perhaps $repo_path in this case? I do not care too deeply about the name, as long as the names used by both mechanisms are the same. -- To unsubscribe from this list: send the line "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 1/2] for-each-repo: new command used for multi-repo operations
On Sun, Jan 27, 2013 at 11:04:08AM -0800, Junio C Hamano wrote: > One more thing that nobody brought up during the previous reviews is > if we want to support subset of repositories by allowing the > standard pathspec match mechanism. For example, > > git for-each-repo -d git diff --name-only -- foo/ bar/b\*z > > might be a way to ask "please find repositories match the given > pathspecs (i.e. foo/ bar/b\*z) and run the command in the ones that > are dirty". We would need to think about how to mark the end of the > command though---we could borrow \; from find(1), even though find > is not the best example of the UI design. I.e. > > git for-each-repo -d git diff --name-only \; [--] foo/ bar/b\*z > > with or without "--". Would it be better to make this a (multi-valued) option? git for-each-repo -d --filter=foo/ --filter=bar/b\*z git diff --name-only It seems a lot simpler than trying to figure out how the command is going to handle '--' arguments. > Oh, that reminds me of another thing. Perhaps we would want to > export the (relative) path to the found repository in some way to > allow the commands to do this kind of thing in the first place? > "submodule foreach" does this with $path, I think. I think $path is the only variable exported by "submodule foreach" which is applicable here, but it doesn't work on Windows, where environment variables are case-insensitive. Commit 64394e3 (git-submodule.sh: Don't use $path variable in eval_gettext string) changed "submodule foreach" to use $sm_path internally although I notice that the documentation still uses $path. Perhaps $repo_path in this case? John -- To unsubscribe from this list: send the line "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: Behavior of stash apply vs merge
Robin Rosenberg writes: > Thanks. Feeling a bit studid now. > > I was actually thinking about using merge to implement stash apply > in JGit. What we have is broken so I tried using merge to implement > it and them compared to git merge --no-commit.. FAIL. Do you have "cherry-pick"? In short, "stash apply" is a "cherry-pick" in disguise. -- To unsubscribe from this list: send the line "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: Behavior of stash apply vs merge
Thanks. Feeling a bit studid now. I was actually thinking about using merge to implement stash apply in JGit. What we have is broken so I tried using merge to implement it and them compared to git merge --no-commit.. FAIL. The main difference is of course that I set the merge base to stash^1, which is obviously not what my question was about. -- robin -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
