[PATCH v2] l10n: de.po: translate 99 new messages

2013-07-31 Thread Ralf Thielow
Translate 99 new messages came from git.pot update in
28b3cff (l10n: git.pot: v1.8.4 round 1 (99 new, 46 removed)).

Signed-off-by: Ralf Thielow 
---
Hi Thomas,

thanks for review!

>   -"Arbeitsverzeichnis von Submodul in '$sm_path' enthält lokale Änderungen; "
>   -"verwenden Sie '-f' um diese zu verwerfen"
>   +msgstr "Arbeitsverzeichnis von Submodul in '$displaypath' enthält
>   lokale Änderungen; verwenden Sie '-f' um diese zu verwerfen"
>
> Was it intended for application?  If so, can you fix your tooling to
> send correct patches?

Yes, there was a change in this message. I started using a po-plugin for
my editor. I'll remove it. Hope that will fix it.

 po/de.po | 366 +++
 1 file changed, 181 insertions(+), 185 deletions(-)

diff --git a/po/de.po b/po/de.po
index ee5e786..b01fbdf 100644
--- a/po/de.po
+++ b/po/de.po
@@ -8,7 +8,7 @@ msgstr ""
 "Project-Id-Version: git 1.8.4\n"
 "Report-Msgid-Bugs-To: Git Mailing List \n"
 "POT-Creation-Date: 2013-07-26 14:19+0800\n"
-"PO-Revision-Date: 2012-10-02 19:35+0200\n"
+"PO-Revision-Date: 2013-07-28 18:42+0200\n"
 "Last-Translator: Ralf Thielow \n"
 "Language-Team: German <>\n"
 "Language: de\n"
@@ -572,9 +572,9 @@ msgstr[1] ""
 "Haben Sie eines von diesen gemeint?"
 
 #: help.c:443
-#, fuzzy, c-format
+#, c-format
 msgid "%s: %s - %s"
-msgstr "%s: %s"
+msgstr "%s: %s - %s"
 
 #: merge.c:56
 msgid "failed to read the cache"
@@ -955,12 +955,12 @@ msgstr ""
 
 #: run-command.c:80
 msgid "open /dev/null failed"
-msgstr ""
+msgstr "Öffnen von /dev/null fehlgeschlagen"
 
 #: run-command.c:82
 #, c-format
 msgid "dup2(%d,%d) failed"
-msgstr ""
+msgstr "dup2(%d,%d) fehlgeschlagen"
 
 #: sequencer.c:206 builtin/merge.c:790 builtin/merge.c:903
 #: builtin/merge.c:1013 builtin/merge.c:1023
@@ -1223,6 +1223,18 @@ msgid ""
 "examine these refs and maybe delete them. Turn this message off by\n"
 "running \"git config advice.object_name_warning false\""
 msgstr ""
+"Git erzeugt normalerweise keine Referenzen die mit\n"
+"40 Hex-Zeichen enden, da diese ignoriert werden wenn\n"
+"Sie diese angeben. Diese Referenzen könnten aus Versehen\n"
+"erzeugt worden sein. Zum Beispiel,\n"
+"\n"
+"  git checkout -b $br $(git rev-parse ...)\n"
+"\n"
+"wobei \"$br\" leer ist und eine 40-Hex-Referenz erzeugt\n"
+"wurde. Bitte prüfen Sie diese Referenzen und löschen\n"
+"Sie sie gegebenenfalls. Unterdrücken Sie diese Meldung\n"
+"indem Sie \"git config advice.object_name_warning false\"\n"
+"ausführen."
 
 #: sha1_name.c:1097
 msgid "HEAD does not point to a branch"
@@ -1449,11 +1461,8 @@ msgid "The current patch is empty."
 msgstr "Der aktuelle Patch ist leer."
 
 #: wt-status.c:829
-#, fuzzy
 msgid "  (fix conflicts and then run \"git am --continue\")"
-msgstr ""
-"  (beheben Sie die Konflikte und führen Sie dann \"git rebase --continue\" "
-"aus)"
+msgstr "  (beheben Sie die Konflikte und führen Sie dann \"git am --continue\" 
aus)"
 
 #: wt-status.c:831
 msgid "  (use \"git am --skip\" to skip this patch)"
@@ -1540,22 +1549,16 @@ msgid "You are currently cherry-picking."
 msgstr "Sie führen gerade \"cherry-pick\" aus."
 
 #: wt-status.c:958
-#, fuzzy
 msgid "  (fix conflicts and run \"git cherry-pick --continue\")"
-msgstr ""
-"  (beheben Sie die Konflikte und führen Sie dann \"git revert --continue\" "
-"aus)"
+msgstr "  (beheben Sie die Konflikte und führen Sie dann \"git cherry-pick 
--continue\" aus)"
 
 #: wt-status.c:961
-#, fuzzy
 msgid "  (all conflicts fixed: run \"git cherry-pick --continue\")"
-msgstr "  (alle Konflikte behoben: führen Sie \"git revert --continue\" aus)"
+msgstr "  (alle Konflikte behoben: führen Sie \"git cherry-pick --continue\" 
aus)"
 
 #: wt-status.c:963
-#, fuzzy
 msgid "  (use \"git cherry-pick --abort\" to cancel the cherry-pick operation)"
-msgstr ""
-"  (benutzen Sie \"git revert --abort\" um die Revert-Operation abzubrechen)"
+msgstr "  (benutzen Sie \"git cherry-pick --abort\" um die 
Cherry-Pick-Operation abzubrechen)"
 
 #: wt-status.c:972
 #, c-format
@@ -1597,9 +1600,8 @@ msgid "On branch "
 msgstr "Auf Branch "
 
 #: wt-status.c:1180
-#, fuzzy
 msgid "rebase in progress; onto "
-msgstr "Kein Rebase im Gange?"
+msgstr "Rebase im Gange; auf "
 
 #: wt-status.c:1187
 msgid "HEAD detached at "
@@ -2223,7 +2225,7 @@ msgid "unable to remove %s from index"
 msgstr "konnte %s nicht aus der Staging-Area entfernen"
 
 #: builtin/apply.c:3851
-#, fuzzy, c-format
+#, c-format
 msgid "corrupt patch for submodule %s"
 msgstr "fehlerhafter Patch für Submodul %s"
 
@@ -3098,9 +3100,8 @@ msgid "suppress progress reporting"
 msgstr "unterdrückt Fortschrittsanzeige"
 
 #: builtin/check-ignore.c:26
-#, fuzzy
 msgid "show non-matching input paths"
-msgstr "zeigt Zeilen ohne Übereinstimmungen"
+msgstr "zeigt Eingabe-Pfade ohne Übereinstimmungen"
 
 #: builtin/check-ignore.c:143
 msgid "cannot specify pathnames with --stdin"
@@ -3124,29 +3125,25 @@ msgstr ""
 "Die Optionen --quiet und --verbose können nic

[PATCH] Scroll diff with arrow keys in log view

2013-07-31 Thread Kumar Appaiah
This commit introduces the VIEW_NO_PARENT_NAV flag and adds it to the
log view. This allows the scrolling commands to fall through from the
pager to the diff when the diff is viewed in the log mode.

Signed-Off-By: Kumar Appaiah 
---
 tig.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/tig.c b/tig.c
index 8301295..52831fd 100644
--- a/tig.c
+++ b/tig.c
@@ -1895,6 +1895,7 @@ enum view_flag {
VIEW_STDIN  = 1 << 8,
VIEW_SEND_CHILD_ENTER   = 1 << 9,
VIEW_FILE_FILTER= 1 << 10,
+   VIEW_NO_PARENT_NAV  = 1 << 11,
 };
 
 #define view_has_flags(view, flag) ((view)->ops->flags & (flag))
@@ -3765,7 +3766,7 @@ view_driver(struct view *view, enum request request)
 
case REQ_NEXT:
case REQ_PREVIOUS:
-   if (view->parent) {
+   if (view->parent && !view_has_flags(view->parent, 
VIEW_NO_PARENT_NAV)) {
int line;
 
view = view->parent;
@@ -4431,7 +4432,7 @@ log_request(struct view *view, enum request request, 
struct line *line)
 static struct view_ops log_ops = {
"line",
{ "log" },
-   VIEW_ADD_PAGER_REFS | VIEW_OPEN_DIFF | VIEW_SEND_CHILD_ENTER,
+   VIEW_ADD_PAGER_REFS | VIEW_OPEN_DIFF | VIEW_SEND_CHILD_ENTER | 
VIEW_NO_PARENT_NAV,
0,
log_open,
pager_read,
-- 
1.8.3.2

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Difficulty adding a symbolic link, part 3

2013-07-31 Thread Duy Nguyen
On Thu, Aug 1, 2013 at 3:29 AM, Dale R. Worley  wrote:
> I've run into a problem (with Git 1.8.3.3) where I cannot add a
> symbolic link (as such) to the repository *if* its path is given
> absolutely; instead Git adds the file the symbolic link points to.
> (If I give the path relatively, Git does what I expect, that is, adds
> the symbolic link.)
>
> I've written a test script that shows the problem and included it
> below.
>
> I would not expect *how* a path is presented to Git to change how Git
> processes the path.  In the test case, I would expect "/bin/awk" and
> "../../bin/awk" to produce the same effect when used as arguments to
> "git add".
>
> What is going on in the code is this:  In "git add", all paths are
> normalized by the function prefix_path_gently() in abspath.c.  That
> function removes symbolic links from the pathspec *only if* it is
> absolute, as shown in the first few lines of the function:
>
>  static char *prefix_path_gently(const char *prefix, int len, const char 
> *path)
>  {
>  const char *orig = path;
>  char *sanitized;
>  if (is_absolute_path(orig)) {
> -const char *temp = real_path(path);
> +const char *temp = absolute_path(path);
>  sanitized = xmalloc(len + strlen(temp) + 1);
>  strcpy(sanitized, temp);
>  } else {
>
> real_path() is specified to remove symbolic links.  As shown, I've
> replaced real_path() with absolute_path(), based on the comment at the
> top of real_path():
>
> /*
>  * Return the real path (i.e., absolute path, with symlinks resolved
>  * and extra slashes removed) equivalent to the specified path.  (If
>  * you want an absolute path but don't mind links, use
>  * absolute_path().)  The return value is a pointer to a static
>  * buffer.
>  *
>
> If I replace real_path() with absolute_path() as shown, the problem I
> am testing for disappears.

I think it also reverts 18e051a (setup: translate symlinks in filename
when using absolute paths - 2010-12-27). real_path() (or
make_absolute_path() back then) is added to resolve symlinks, at least
ones leading to the work tree, not ones inside the work tree, if I
understand the commit message correctly.

>
> With the above change, the test suite runs with zero failures, so it
> doesn't affect any common Git usage.

It means the test suite is incomplete. As you can see, the commit
introducing this change does not come with a test case to catch people
changing this.

>
> But I don't know enough about the internal architecture of Git to know
> that my change is correct in all cases.  I'm almost certain that the
> normalization process for pathspecs should *not* normalize a final
> component that is a symbolic link.  But I would expect it would be
> desirable to normalize non-final components that are symbolic links.
> On the other hand, that might not matter.
>
> Can someone give me advice on what this code *should* do?

It does as the function name says: given cwd, a prefix (i.e. a
relative path with no ".." components) and a path relative to
cwd+prefix, convert 'path' to something relative to cwd. In the
simplest case, prepending the prefix to 'path' is enough. cwd is also
get_git_work_tree().

I agree with you that this code should not resolve anything in the
components after 'cwd', after rebasing the path to 'cwd' (not just the
final component). Not sure how to do it correctly though because we do
need to resolve symlinks before cwd. Maybe a new variant of real_path
that stops at 'cwd'?

We may also have problems with resolve symlinks before cwd when 'path'
is relative, as normalize_path_copy() does not resolve symlinks. We
just found out emacs has this bug [1] but did not realize we also have
one :-P.

[1] http://thread.gmane.org/gmane.comp.version-control.git/231268

>
> I believe I can prepare a proper test for the test suite for this, so
> once I know what the code change should be, I can prepare a proper
> patch for it.
>
> Dale
-- 
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 v6 6/6] config: "git config --get-urlmatch" parses section..key

2013-07-31 Thread Junio C Hamano
Jeff King  writes:

>   1. Git-config expects pre-canonicalized variable names (so http.noepsv
>  instead of "http.noEPSV"). I think the "git config --get" code path
>  does this for the caller, so we should probably do the same for
>  "--get-urlmatch". And it is even easier here, because we know that
>  "http.noEPSV" does not contain a case-sensitive middle part. :)

I'll squash these in later, but here is from my working copy.
Thanks for spotting.

 builtin/config.c   | 32 +++-
 t/t1300-repo-config.sh |  4 ++--
 2 files changed, 21 insertions(+), 15 deletions(-)

diff --git a/builtin/config.c b/builtin/config.c
index c35c5be..c046f54 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -379,9 +379,22 @@ static int urlmatch_collect_fn(const char *var, const char 
*value, void *cb)
return 0;
 }
 
+static char *dup_downcase(const char *string)
+{
+   char *result;
+   size_t len, i;
+
+   len = strlen(string);
+   result = xmalloc(len + 1);
+   for (i = 0; i < len; i++)
+   result[i] = tolower(string[i]);
+   result[i] = '\0';
+   return result;
+}
+
 static int get_urlmatch(const char *var, const char *url)
 {
-   const char *section_tail;
+   char *section_tail;
struct string_list_item *item;
struct urlmatch_config config = { STRING_LIST_INIT_DUP };
struct string_list values = STRING_LIST_INIT_DUP;
@@ -393,13 +406,13 @@ static int get_urlmatch(const char *var, const char *url)
if (!url_normalize(url, &config.url))
die(config.url.err);
 
-   section_tail = strchr(var, '.');
+   config.section = dup_downcase(var);
+   section_tail = strchr(config.section, '.');
if (section_tail) {
-   config.section = xmemdupz(var, section_tail - var);
-   config.key = strrchr(var, '.') + 1;
+   *section_tail = '\0';
+   config.key = section_tail + 1;
show_keys = 0;
} else {
-   config.section = var;
config.key = NULL;
show_keys = 1;
}
@@ -425,14 +438,7 @@ static int get_urlmatch(const char *var, const char *url)
string_list_clear(&values, 1);
free(config.url.url);
 
-   /*
-* section name may have been copied to replace the dot, in which
-* case it needs to be freed.  key name is either NULL (e.g. 'http'
-* alone) or points into var (e.g. 'http.savecookies'), and we do
-* not own the storage.
-*/
-   if (config.section != var)
-   free((void *)config.section);
+   free((void *)config.section);
return 0;
 }
 
diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index 323e880..c23f478 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -1097,7 +1097,7 @@ test_expect_success 'urlmatch' '
EOF
 
echo true >expect &&
-   git config --bool --get-urlmatch http.sslverify 
https://good.example.com >actual &&
+   git config --bool --get-urlmatch http.SSLverify 
https://good.example.com >actual &&
test_cmp expect actual &&
 
echo false >expect &&
@@ -1108,7 +1108,7 @@ test_expect_success 'urlmatch' '
echo http.cookiefile /tmp/cookie.txt &&
echo http.sslverify false
} >expect &&
-   git config --get-urlmatch http https://weak.example.com >actual &&
+   git config --get-urlmatch HTTP https://weak.example.com >actual &&
test_cmp expect actual
 '
 
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v6 6/6] config: "git config --get-urlmatch" parses section..key

2013-07-31 Thread Jeff King
On Wed, Jul 31, 2013 at 04:03:01PM -0700, Kyle J. McKay wrote:

> > 1. Git-config expects pre-canonicalized variable names (so
> >http.noepsv
> >instead of "http.noEPSV"). I think the "git config --get" code
> >path
> >does this for the caller, so we should probably do the same for
> >"--get-urlmatch". And it is even easier here, because we know that
> >"http.noEPSV" does not contain a case-sensitive middle part. :)
> 
> The test was testing that too, which I think is a good thing.  Your
> replacement does not test that.  With a fix for --get-urlmatch as you
> mention above, the tests can check that again.

I do not think the existing tests were checking anything interesting in
that respect. The url-matching code does not do the canonicalization,
and nor should it (the internal callbacks for all variables should use
the canonical lowercase version). So we were only testing that
test-url-normalize lowercased them, which is not something we actually
care about (nobody but the test script should ever call it).

That being said, git-config _should_ be lowercasing to match the normal
--get code path. I think the fix (squashable on top of 6/6 + my earlier
patch) is just:

diff --git a/builtin/config.c b/builtin/config.c
index c35c5be..9328a90 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -589,7 +589,9 @@ int cmd_config(int argc, const char **argv, const char 
*prefix)
}
else if (actions == ACTION_GET_URLMATCH) {
check_argc(argc, 2, 2);
-   return get_urlmatch(argv[0], argv[1]);
+   if (git_config_parse_key(argv[0], &key, NULL))
+   return CONFIG_INVALID_KEY;
+   return get_urlmatch(key, argv[1]);
}
else if (actions == ACTION_UNSET) {
check_argc(argc, 1, 2);
diff --git a/t/t5200-url-normalize.sh b/t/t5200-url-normalize.sh
index a5190f7..7284dc6 100755
--- a/t/t5200-url-normalize.sh
+++ b/t/t5200-url-normalize.sh
@@ -190,14 +190,14 @@ check_url_config "$tc-2" example-agent http.useragent 
HTTPS://example.COM/p%61th
 
 check_url_config "$tc-1" other-agent http.useragent https://other.example.com/
 check_url_config "$tc-1" example-agent http.useragent https://example.com/
-check_url_config "$tc-1" false --bool http.sslverify https://example.com/
+check_url_config "$tc-1" false --bool http.sslVerify https://example.com/
 check_url_config "$tc-1" path-agent http.useragent https://example.com/path/sub
-check_url_config "$tc-1" false --bool http.sslverify 
https://example.com/path/sub
-check_url_config "$tc-1" true --bool http.noepsv https://elsewhere.com/
-check_url_config "$tc-1" true --bool http.noepsv https://example.com
-check_url_config "$tc-1" true --bool http.noepsv https://example.com/path
+check_url_config "$tc-1" false --bool http.sslVerify 
https://example.com/path/sub
+check_url_config "$tc-1" true --bool http.noEPSV https://elsewhere.com/
+check_url_config "$tc-1" true --bool http.noEPSV https://example.com
+check_url_config "$tc-1" true --bool http.noEPSV https://example.com/path
 check_url_config "$tc-2" example-agent http.useragent 
HTTPS://example.COM/p%61th
-check_url_config "$tc-2" false --bool http.sslverify HTTPS://example.COM/p%61th
+check_url_config "$tc-2" false --bool http.sslVerify HTTPS://example.COM/p%61th
 check_url_config "$tc-3" file-1 http.sslcainfo 
https://u...@example.com/path/name/here
 
 test_done

> >The wrapper is a little ugly. I do wonder if we actually need all
> >of these tests (i.e., it is not clear to me what different things
> >each is testing, and if it is not simply trying to exercise the
> >different variable names, which now all follow the same code path,
> >because git-config does not care about the particular names).
> 
> Each one tests a different item from the "$tc-n" config file to make
> sure that everything that's in each config file actually behaves as
> expected.

I guess I don't understand why we have so many items in each file. That
is, we have:

"$tc-1" "useragent" "https://other.example.com/"; = "other-agent"
"$tc-1" "useragent" "https://example.com/"; = "example-agent"

The first checks that we do not apply within a sub-domain (but fall back
to http.useragent), and the second checks that we do properly apply the
full domain.

"$tc-1" "sslVerify" "https://example.com/"; = "false"

This check seems redundant with the second one above.

"$tc-1" "useragent" "https://example.com/path/sub"; = "path-agent"
"$tc-1" "sslVerify" "https://example.com/path/sub"; = "false"

Here we make sure paths are preferred over non-paths (the first one),
but that config keys with non-paths are still used (the second).

"$tc-1" "noEPSV" "https://elsewhere.com/"; = "true"

This seems redundant with the first test (check that we do not match and
fallback to http.noepsv).

"$tc-1" "noEPSV" "https://example.com"; = "true"

Not sure what we are testing here; there is n

What's cooking in git.git (Jul 2013, #10; Wed, 31)

2013-07-31 Thread Junio C Hamano
Here are the topics that have been cooking.  Commits prefixed with
'-' are only in 'pu' (proposed updates) while commits prefixed with
'+' are in 'next'.

Hopefully the 1.8.4-rc1 tomorrow with topics marked for 'master'
in this issue of "What's cooking" report.

You can find the changes described here in the integration branches
of the repositories listed at

http://git-blame.blogspot.com/p/git-public-repositories.html

--
[Graduated to "master"]

* bc/completion-for-bash-3.0 (2013-07-24) 1 commit
  (merged to 'next' on 2013-07-24 at 913d89c)
 + git-completion.bash: replace zsh notation that breaks bash 3.X


* ds/doc-two-kinds-of-tags (2013-07-29) 1 commit
  (merged to 'next' on 2013-07-30 at 1201ca7)
 + docs/git-tag: explain lightweight versus annotated tags


* jk/capabilities-doc (2013-07-26) 5 commits
  (merged to 'next' on 2013-07-29 at cf3f07e)
 + document 'allow-tip-sha1-in-want' capability
  (merged to 'next' on 2013-07-24 at 5af9a33)
 + document 'quiet' receive-pack capability
 + document 'agent' protocol capability
 + docs: note that receive-pack knows side-band-64k capability
 + docs: fix 'report-status' protocol capability thinko


* jk/commit-how-to-abort-cherry-pick (2013-07-29) 1 commit
  (merged to 'next' on 2013-07-30 at 7b1c49c)
 + commit: tweak empty cherry pick advice for sequencer


* mh/packed-refs-do-one-ref-recursion (2013-07-17) 1 commit
  (merged to 'next' on 2013-07-30 at d8cc1ee)
 + do_one_ref(): save and restore value of current_ref

 Fix a NULL-pointer dereference during nested iterations over
 references (for example, when replace references are being used).


* rj/commit-slab-fix (2013-07-29) 1 commit
  (merged to 'next' on 2013-07-30 at 1914f37)
 + commit-slab.h: Fix memory allocation and addressing


* rr/maint-tilde-markup-in-doc (2013-07-26) 1 commit
  (merged to 'next' on 2013-07-30 at c8f83d5)
 + config doc: quote paths, fixing tilde-interpretation


* rr/rebase-autostash (2013-07-29) 1 commit
  (merged to 'next' on 2013-07-30 at 0095e2d)
 + git-rebase: fix typo


* sb/mailmap-updates (2013-07-24) 1 commit
  (merged to 'next' on 2013-07-24 at 9f8e681)
 + .mailmap: combine more (email, name) to individual persons

--
[New Topics]

* bc/unuse-packfile (2013-07-30) 1 commit
 - sha1_file: introduce close_one_pack() to close packs on fd pressure

 Will merge to and cook in 'next'.


* da/darwin (2013-07-30) 1 commit
 - imap-send: use Apple's Security framework for base64 encoding

 Will merge to and cook in 'next'.


* jc/rm-submodule-error-message (2013-07-25) 1 commit
 - builtin/rm.c: consolidate error reporting for removing submodules

 Consolidate two messages phrased subtly differently without a good
 reason.

 Will merge to 'next' and then to 'master', unless there is an objection.


* ms/subtree-install-fix (2013-07-30) 1 commit
 - contrib/subtree: Fix make install target

 Will merge to 'next' and then to 'master'.


* nd/sq-quote-buf (2013-07-30) 3 commits
 - quote: remove sq_quote_print()
 - tar-tree: remove dependency on sq_quote_print()
 - for-each-ref, quote: convert *_quote_print -> *_quote_buf

 Code simplification as a preparatory step to something larger.

 Will merge to and cook in 'next'.


* jc/url-match (2013-07-31) 6 commits
 - config: "git config --get-urlmatch" parses section..key
 - builtin/config: refactor collect_config()
 - config: parse http.. using urlmatch
 - config: add generic callback wrapper to parse section..key
 - config: add helper to normalize and match URLs
 - http.c: fix parsing of http.sslCertPasswordProtected variable

 Reroll of km/http-curl-config-per-url topic.

 Will merge to and cook in 'next'

--
[Stalled]

* tr/log-full-diff-keep-true-parents (2013-07-29) 2 commits
 - SQUASH??? free saved-parents slab when done
 - [PERHAPS LIKE THIS] log: use true parents for diff even when rewriting


* tf/gitweb-ss-tweak (2013-07-15) 4 commits
 - gitweb: make search help link less ugly
 - gitweb: omit the repository owner when it is unset
 - gitweb: vertically centre contents of page footer
 - gitweb: ensure OPML text fits inside its box

 Comments?


* rj/read-default-config-in-show-ref-pack-refs (2013-06-17) 3 commits
 - ### DONTMERGE: needs better explanation on what config they need
 - pack-refs.c: Add missing call to git_config()
 - show-ref.c: Add missing call to git_config()

 The changes themselves are probably good, but it is unclear what
 basic setting needs to be read for which exact operation.

 Waiting for clarification.
 $gmane/228294


* jh/shorten-refname (2013-05-07) 4 commits
 - t1514: refname shortening is done after dereferencing symbolic refs
 - shorten_unambiguous_ref(): Fix shortening refs/remotes/origin/HEAD to origin
 - t1514: Demonstrate failure to correctly shorten "refs/remotes/origin/HEAD"
 - t1514: Add tests of shortening refnames in strict/loose mode

 When remot

Re: [PATCH] tag: Use OPT_BOOL instead of OPT_BOOLEAN to allow one action multiple times

2013-07-31 Thread Junio C Hamano
Stefan Beller  writes:

> On 07/31/13 00:28, Junio C Hamano wrote:
>> 
>> we could just do
>> 
>> #define OPT_CMDMODE(s, l, v, h) \
>> { OPTION_CMDMODE, (s), (l), (v), NULL, \
>>   (h), PARSE_OPT_NOARG|PARSE_OPT_NONEG, NULL, (s) }
>> 
>
> I agree that's a better proposal than mine.

By the way, I haven't convinced myself that it is a good idea in
general to encourage more use of command mode options, so I am a bit
reluctant to add this before knowing which direction in the longer
term we are going.

 - Some large-ish Git subcommands, like "git submodule", use the
   mode word (e.g. "git submodule status") to specify the operation
   mode (youe could consider "status" a subsubcommand that
   "submodule" subcommand takes).  These commands typically began
   their life from day one with the mode words.

 - On the other hand, many Git subcommands, like "git tag", have
   "the primary operation mode" (e.g. "create a new one" is the
   primary operation mode for "git tag"), and use command mode
   options to specify other operation modes (e.g. "--delete").
   These commands started as single purpose commands (i.e. to
   perform their "primary operation") but have organically grown
   over time and acquired command mode options to invoke their
   secondary operations.

As an end user, you need to learn which style each command takes,
which is an unnecessary burden at the UI level.  In the longer term,
we may want to consider picking a single style, and migrating
everybody to it.  If I have to vote today, I would say we should
teach "git submodule" to also take command mode options (e.g. "git
submodule --status" will be understood the same way as "git
submodule status"), make them issue warnings when mode words are
used and encourage users to use command mode options instead, and
optionally remove the support of mode words at a large version bump
like 3.0.

One clear advantage mode words have over command mode options is
that there is no room for end user confusion.  The first word after
"git subcmd" is the mode word, and you will not even dream of asking
"what would 'git submodule add del foo' do?" as it is nonsensical.
The command mode options, on the other hand, gives too much useless
flexibility to ask for nonsense, e.g. "git tag --delete --verify",
"git tag --no-delete --delete", etc., and extra code needs to detect
and reject combinations.  But commands that took mode options cannot
be easily migrated to take mode words without hurting existing users
and scripts (e.g. "git tag delete master" can never be a request to
delete the tag 'master', as it is a request to create a tag whose
name is 'delete' that points at the same object as 'master' points
at).



--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v6 6/6] config: "git config --get-urlmatch" parses section..key

2013-07-31 Thread Kyle J. McKay

On Jul 31, 2013, at 15:45, Jeff King wrote:


On Wed, Jul 31, 2013 at 12:26:08PM -0700, Junio C Hamano wrote:


Using the same urlmatch_config_entry() infrastructure, add a new
mode "--get-urlmatch" to the "git config" command, to learn values
for the "virtual" two-level variables customized for the specific
URL.

   git config [--] --get-urlmatch [.] 


Do we want something like this on top, to convert the third form of
test-url-normalize into git-config calls?

It would be nicer squashed in, but we the tests are added earlier in  
the

series than "--get-urlmatch", so we would have to rip the tests out of
the earlier patches and have a "[PATCH 7/6] add tests for url
normalizing".

Two things to note about my test conversion:

 1. Git-config expects pre-canonicalized variable names (so  
http.noepsv
instead of "http.noEPSV"). I think the "git config --get" code  
path

does this for the caller, so we should probably do the same for
"--get-urlmatch". And it is even easier here, because we know that
"http.noEPSV" does not contain a case-sensitive middle part. :)


The test was testing that too, which I think is a good thing.  Your  
replacement does not test that.  With a fix for --get-urlmatch as you  
mention above, the tests can check that again.



 2. I turned the many 'test "$(git foo)" = "bar"' invocations into a
wrapper function that uses test_cmp. This helped immensely with
debugging (1).

The wrapper is a little ugly. I do wonder if we actually need all
of these tests (i.e., it is not clear to me what different things
each is testing, and if it is not simply trying to exercise the
different variable names, which now all follow the same code path,
because git-config does not care about the particular names).


Each one tests a different item from the "$tc-n" config file to make  
sure that everything that's in each config file actually behaves as  
expected.


If we do this (and I don't really have any objection except for the  
point noted above), then the tests really need to move out from t5200  
as they're not tied to the http operations anymore.  Also the Makefile  
rule for test-url-normalize.c needs to be simplified since it won't  
need the extra options to make it link since it's no longer including  
http.c.


The README has this:


First digit tells the family:

0 - the absolute basics and global stuff
1 - the basic commands concerning database
2 - the basic commands concerning the working tree
3 - the other basic commands (e.g. ls-files)
4 - the diff commands
5 - the pull and exporting commands
6 - the revision tree commands (even e.g. merge-base)
7 - the porcelainish commands concerning the working tree
8 - the porcelainish commands concerning forensics
9 - the git tools


But the best choice does not immediately jump out at me.  However,  
looking at the other tests that are there, I think perhaps 1307-config- 
url might be a reasonable choice.


--
To unsubscribe from this list: send the line "unsubscribe 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] log: use true parents for diff even when rewriting

2013-07-31 Thread Jeff King
On Wed, Jul 31, 2013 at 10:13:20PM +0200, Thomas Rast wrote:

> When using pathspec filtering in combination with diff-based log
> output, parent simplification happens before the diff is computed.
> The diff is therefore against the *simplified* parents.
> 
> This works okay, arguably by accident, in the normal case:
> simplification reduces to one parent as long as the commit is TREESAME
> to it.  So the simplified parent of any given commit must have the
> same tree contents on the filtered paths as its true (unfiltered)
> parent.
> 
> However, --full-diff breaks this guarantee, and indeed gives pretty
> spectacular results when comparing the output of
> 
>   git log --graph --stat ...
>   git log --graph --full-diff --stat ...
> 
> (--graph internally kicks in parent simplification, much like
> --parents).

Your description (and solution) make a lot of sense to me. Another code
path that has a similar problem is the "-g" reflog walker. It rewrites
the parents based on the reflog, and the diffs it produces are mostly
useless (e.g., try "git stash list -p").

Should we be applying the same technique there?

I guess it might bother people who really want to see the diff between
two lines in the reflog (e.g., comparing the results stored by "rebase"
with the previous version). I think that is rare enough that "git diff"
would be a better tool in that case, though.

And arguably, "log -g" should not be rewriting the parents at all,
because it makes "--graph" pointless (and indeed, we disallow the
combination). So potentially the solution is to stop the rewriting
entirely, not mask it for the diffs. But doing so might be a good
interim solution until somebody feels like rewriting the reflog walker.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: How to hierarchically merge from the root to the leaf of a branch tree? (Patch stack management)

2013-07-31 Thread Fredrik Gustafsson
On Thu, Aug 01, 2013 at 12:25:32AM +0200, Jens Müller wrote:
> Hi all!
> 
> I mainly use Git for version control, but have also tried out Mercurial.
> While I don't really like Mercurial in general, the idea of maintaining
> clearly separated patches with Mercurial Queues (MQ) is quite appealing.
> Therefore, I am looking for something similar (but easier to use, more
> "gitty" and maybe even more powerful) in Git.
> 
> So I will first explain what I have in mind:
> 
> As an example, let's say I am doing test-driven development. My master
> branch follows the main repository of the software. Branched out from
> that, I have a branch called "feature-test", and branched out from that,
> "feature-implementation":
> 
> master
> |_ feature-test
>|_ feature-implementation
> 
> For each branch, I remember the parent branch.
> 
> Implementation would then work like this: I checkout feature-test and
> write some test. Then I checkout feature-implementation, rebase it to
> the current status of feature-test and write the implemenation. And so on.
> 
> At some point, I update master, and then rebase both feature-test and
> feature-implementation.
> 
> As a side note: Instead of rebasing the branches, an alternative would
> be to merge the changes from the parent branch. This makes conflict
> resolution easier. The cascading merge through the chain of branches is
> like a rebase, anyway.
> 
> Of course, the process described above contains a lot of tedious manual
> work. So I am looking for tooling for tasks like the following:
> 
>  * While on a branch, pull master from a remote branch it tracks and
> merge the changes down the chain of branches. When a conflict is
> encountered, switch to the branch where it occured, allow the user to
> resolve the conflict, and then continue the cascading merge (similar to
> what git rebase does when it encounters a conflict).
>  * When checking out a branch, cascadingly merge from the ancestor
> branches automatically. Conflict handling should work as in the previous
> point.
> 
> The cascading merge should not check out master and the branches below
> it (unless necessary to resolve conflicts), in order to avoid rebuilds
> due to touched but unchanged files.
> 
> Do these requirements make sense? Is there some existing tool with a
> similar workflow?
> 
> BR - Jens
> 
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Since all commits in feature-test is in feature-implementation,
how about rebase feature-implementation on master and then move the
branch pointer for feature-test to the new commit (git reset)?

If it's still not trivial enough, a script for this would be fairly easy
to implement (if I don't miss anything big here).

-- 
Med vänliga hälsningar
Fredrik Gustafsson

tel: 0733-608274
e-post: iv...@iveqy.com
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v6 6/6] config: "git config --get-urlmatch" parses section..key

2013-07-31 Thread Jeff King
On Wed, Jul 31, 2013 at 12:26:08PM -0700, Junio C Hamano wrote:

> Using the same urlmatch_config_entry() infrastructure, add a new
> mode "--get-urlmatch" to the "git config" command, to learn values
> for the "virtual" two-level variables customized for the specific
> URL.
> 
> git config [--] --get-urlmatch [.] 

Do we want something like this on top, to convert the third form of
test-url-normalize into git-config calls?

It would be nicer squashed in, but we the tests are added earlier in the
series than "--get-urlmatch", so we would have to rip the tests out of
the earlier patches and have a "[PATCH 7/6] add tests for url
normalizing".

Two things to note about my test conversion:

  1. Git-config expects pre-canonicalized variable names (so http.noepsv
 instead of "http.noEPSV"). I think the "git config --get" code path
 does this for the caller, so we should probably do the same for
 "--get-urlmatch". And it is even easier here, because we know that
 "http.noEPSV" does not contain a case-sensitive middle part. :)

  2. I turned the many 'test "$(git foo)" = "bar"' invocations into a
 wrapper function that uses test_cmp. This helped immensely with
 debugging (1).

 The wrapper is a little ugly. I do wonder if we actually need all
 of these tests (i.e., it is not clear to me what different things
 each is testing, and if it is not simply trying to exercise the
 different variable names, which now all follow the same code path,
 because git-config does not care about the particular names).

---
 t/t5200-url-normalize.sh | 40 ---
 test-url-normalize.c | 93 ++-
 2 files changed, 27 insertions(+), 106 deletions(-)

diff --git a/t/t5200-url-normalize.sh b/t/t5200-url-normalize.sh
index f79bb0f..a5190f7 100755
--- a/t/t5200-url-normalize.sh
+++ b/t/t5200-url-normalize.sh
@@ -3,11 +3,6 @@ test_description='url normalization'
 test_description='url normalization'
 . ./test-lib.sh
 
-if test -n "$NO_CURL"; then
-   skip_all='skipping test, git built without http support'
-   test_done
-fi
-
 # The base name of the test url files
 tu="$TEST_DIRECTORY/t5200/url"
 
@@ -182,18 +177,27 @@ test_expect_success 'url equivalents' '
test-url-normalize "https://@x.y/^/.."; "httpS://@x.y:0443/"
 '
 
-test_expect_success 'url config normalization matching' '
-   test "$(test-url-normalize -c "$tc-1" "useragent" 
"https://other.example.com/";)" = "other-agent" &&
-   test "$(test-url-normalize -c "$tc-1" "useragent" 
"https://example.com/";)" = "example-agent" &&
-   test "$(test-url-normalize -c "$tc-1" "sslVerify" 
"https://example.com/";)" = "false" &&
-   test "$(test-url-normalize -c "$tc-1" "useragent" 
"https://example.com/path/sub";)" = "path-agent" &&
-   test "$(test-url-normalize -c "$tc-1" "sslVerify" 
"https://example.com/path/sub";)" = "false" &&
-   test "$(test-url-normalize -c "$tc-1" "noEPSV" 
"https://elsewhere.com/";)" = "true" &&
-   test "$(test-url-normalize -c "$tc-1" "noEPSV" "https://example.com";)" 
= "true" &&
-   test "$(test-url-normalize -c "$tc-1" "noEPSV" 
"https://example.com/path";)" = "true" &&
-   test "$(test-url-normalize -c "$tc-2" "useragent" 
"HTTPS://example.COM/p%61th")" = "example-agent" &&
-   test "$(test-url-normalize -c "$tc-2" "sslVerify" 
"HTTPS://example.COM/p%61th")" = "false" &&
-   test "$(test-url-normalize -c "$tc-3" "sslcainfo" 
"https://u...@example.com/path/name/here";)" = "file-1"
-'
+check_url_config() {
+   config=$1; shift
+   expect=$1; shift
+
+   test_expect_success "config normalization ($*)" "
+   echo '$expect' >expect &&
+   git config --file="\$config" --get-urlmatch $* >actual &&
+   test_cmp expect actual
+   "
+}
+
+check_url_config "$tc-1" other-agent http.useragent https://other.example.com/
+check_url_config "$tc-1" example-agent http.useragent https://example.com/
+check_url_config "$tc-1" false --bool http.sslverify https://example.com/
+check_url_config "$tc-1" path-agent http.useragent https://example.com/path/sub
+check_url_config "$tc-1" false --bool http.sslverify 
https://example.com/path/sub
+check_url_config "$tc-1" true --bool http.noepsv https://elsewhere.com/
+check_url_config "$tc-1" true --bool http.noepsv https://example.com
+check_url_config "$tc-1" true --bool http.noepsv https://example.com/path
+check_url_config "$tc-2" example-agent http.useragent 
HTTPS://example.COM/p%61th
+check_url_config "$tc-2" false --bool http.sslverify HTTPS://example.COM/p%61th
+check_url_config "$tc-3" file-1 http.sslcainfo 
https://u...@example.com/path/name/here
 
 test_done
diff --git a/test-url-normalize.c b/test-url-normalize.c
index 81d3da9..34a1a67 100644
--- a/test-url-normalize.c
+++ b/test-url-normalize.c
@@ -1,84 +1,11 @@ int main(int argc, char **argv)
-#ifdef NO_CURL
-
-int main()
-{
-   return 125;
-}
-
-#else /* !NO_CURL */
-

[PATCH ALTERNATIVE v6.v2 4/6] config: parse http.. using urlmatch

2013-07-31 Thread Kyle J. McKay
Use the urlmatch_config_entry() to wrap the underlying
http_options() two-level variable parser in order to set
http. to the value with the most specific URL in the
configuration.

Signed-off-by: Jeff King 
Signed-off-by: Kyle J. McKay 
Signed-off-by: Junio C Hamano 
---

Somehow I managed to screw up the previous version of 4/6 by
duplicating a line in the config.txt patch.  Arrggh!  Anyhow,
here's a corrected version of 4/6 with the duplicate line
removed.  I apologize for my clumsiness.

 .gitignore   |   1 +
 Documentation/config.txt |  46 +++
 Makefile |   7 ++
 http.c   |  13 +++-
 t/.gitattributes |   1 +
 t/t5200-url-normalize.sh | 199 +++
 t/t5200/README   |  18 +
 t/t5200/config-1 |   8 ++
 t/t5200/config-2 |   3 +
 t/t5200/config-3 |   4 +
 t/t5200/url-1| Bin 0 -> 20 bytes
 t/t5200/url-10   | Bin 0 -> 23 bytes
 t/t5200/url-11   | Bin 0 -> 25 bytes
 t/t5200/url-2| Bin 0 -> 20 bytes
 t/t5200/url-3| Bin 0 -> 23 bytes
 t/t5200/url-4| Bin 0 -> 23 bytes
 t/t5200/url-5| Bin 0 -> 23 bytes
 t/t5200/url-6| Bin 0 -> 23 bytes
 t/t5200/url-7| Bin 0 -> 23 bytes
 t/t5200/url-8| Bin 0 -> 23 bytes
 t/t5200/url-9| Bin 0 -> 23 bytes
 test-url-normalize.c | 139 +
 22 files changed, 438 insertions(+), 1 deletion(-)
 create mode 100755 t/t5200-url-normalize.sh
 create mode 100644 t/t5200/README
 create mode 100644 t/t5200/config-1
 create mode 100644 t/t5200/config-2
 create mode 100644 t/t5200/config-3
 create mode 100644 t/t5200/url-1
 create mode 100644 t/t5200/url-10
 create mode 100644 t/t5200/url-11
 create mode 100644 t/t5200/url-2
 create mode 100644 t/t5200/url-3
 create mode 100644 t/t5200/url-4
 create mode 100644 t/t5200/url-5
 create mode 100644 t/t5200/url-6
 create mode 100644 t/t5200/url-7
 create mode 100644 t/t5200/url-8
 create mode 100644 t/t5200/url-9
 create mode 100644 test-url-normalize.c

diff --git a/.gitignore b/.gitignore
index 6669bf0c..cd97e16a 100644
--- a/.gitignore
+++ b/.gitignore
@@ -198,6 +198,7 @@
 /test-string-list
 /test-subprocess
 /test-svn-fe
+/test-url-normalize
 /test-wildmatch
 /common-cmds.h
 *.tar.gz
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 6e53fc50..8cc0fd78 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1513,6 +1513,51 @@ http.useragent::
of common USER_AGENT strings (but not including those like git/1.7.1).
Can be overridden by the 'GIT_HTTP_USER_AGENT' environment variable.
 
+http..*::
+   Any of the http.* options above can be applied selectively to some urls.
+   For a config key to match a URL, each element of the config key is
+   compared to that of the URL, in the following order:
++
+--
+. Scheme (e.g., `https` in `https://example.com/`). This field
+  must match exactly between the config key and the URL.
+
+. Host/domain name (e.g., `example.com` in `https://example.com/`).
+  This field must match exactly between the config key and the URL.
+
+. Port number (e.g., `8080` in `http://example.com:8080/`).
+  This field must match exactly between the config key and the URL.
+  Omitted port numbers are automatically converted to the correct
+  default for the scheme before matching.
+
+. Path (e.g., `repo.git` in `https://example.com/repo.git`). The
+  path field of the config key must match the path field of the URL
+  either exactly or as a prefix of slash-delimited path elements.  This means
+  a config key with path `foo/` matches URL path `foo/bar`.  A prefix can only
+  match on a slash (`/`) boundary.  Longer matches take precedence (so a config
+  key with path `foo/bar` is a better match to URL path `foo/bar` than a config
+  key with just path `foo/`).
+
+. User name (e.g., `user` in `https://u...@example.com/repo.git`). If
+  the config key has a user name it must match the user name in the
+  URL exactly. If the config key does not have a user name, that
+  config key will match a URL with any user name (including none),
+  but at a lower precedence than a config key with a user name.
+--
++
+The list above is ordered by decreasing precedence; a URL that matches
+a config key's path is preferred to one that matches its user name. For 
example,
+if the URL is `https://u...@example.com/foo/bar` a config key match of
+`https://example.com/foo` will be preferred over a config key match of
+`https://u...@example.com`.
++
+All URLs are normalized before attempting any matching (the password part,
+if embedded in the URL, is always ignored for matching purposes) so that
+equivalent urls that are simply spelled differently will match properly.
+Environment variable settings always override any matches.  The urls that are
+matched against are those given directly to Git commands.  

How to hierarchically merge from the root to the leaf of a branch tree? (Patch stack management)

2013-07-31 Thread Jens Müller
Hi all!

I mainly use Git for version control, but have also tried out Mercurial.
While I don't really like Mercurial in general, the idea of maintaining
clearly separated patches with Mercurial Queues (MQ) is quite appealing.
Therefore, I am looking for something similar (but easier to use, more
"gitty" and maybe even more powerful) in Git.

So I will first explain what I have in mind:

As an example, let's say I am doing test-driven development. My master
branch follows the main repository of the software. Branched out from
that, I have a branch called "feature-test", and branched out from that,
"feature-implementation":

master
|_ feature-test
   |_ feature-implementation

For each branch, I remember the parent branch.

Implementation would then work like this: I checkout feature-test and
write some test. Then I checkout feature-implementation, rebase it to
the current status of feature-test and write the implemenation. And so on.

At some point, I update master, and then rebase both feature-test and
feature-implementation.

As a side note: Instead of rebasing the branches, an alternative would
be to merge the changes from the parent branch. This makes conflict
resolution easier. The cascading merge through the chain of branches is
like a rebase, anyway.

Of course, the process described above contains a lot of tedious manual
work. So I am looking for tooling for tasks like the following:

 * While on a branch, pull master from a remote branch it tracks and
merge the changes down the chain of branches. When a conflict is
encountered, switch to the branch where it occured, allow the user to
resolve the conflict, and then continue the cascading merge (similar to
what git rebase does when it encounters a conflict).
 * When checking out a branch, cascadingly merge from the ancestor
branches automatically. Conflict handling should work as in the previous
point.

The cascading merge should not check out master and the branches below
it (unless necessary to resolve conflicts), in order to avoid rebuilds
due to touched but unchanged files.

Do these requirements make sense? Is there some existing tool with a
similar workflow?

BR - Jens

--
To unsubscribe from this list: send the line "unsubscribe 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] Rename advice.object_name_warning to objectNameWarning

2013-07-31 Thread Junio C Hamano
Thomas Rast  writes:

> We spell config variables in camelCase instead of with_underscores.
>
> Signed-off-by: Thomas Rast 
> ---
>
> I figure since we don't have the variable in any release yet *and* the
> worst possible outcome is that someone sees the advice message again,
> the consistency is worth the change.

Thanks, I agree.
>
>  advice.c| 2 +-
>  sha1_name.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/advice.c b/advice.c
> index 2a52098..3eca9f5 100644
> --- a/advice.c
> +++ b/advice.c
> @@ -35,7 +35,7 @@
>   { "implicitidentity", &advice_implicit_identity },
>   { "detachedhead", &advice_detached_head },
>   { "setupstreamfailure", &advice_set_upstream_failure },
> - { "object_name_warning", &advice_object_name_warning },
> + { "objectnamewarning", &advice_object_name_warning },
>   { "rmhints", &advice_rm_hints },
>  
>   /* make this an alias for backward compatibility */
> diff --git a/sha1_name.c b/sha1_name.c
> index 1d210e3..852dd95 100644
> --- a/sha1_name.c
> +++ b/sha1_name.c
> @@ -445,7 +445,7 @@ static int get_sha1_basic(const char *str, int len, 
> unsigned char *sha1)
>   "\n"
>   "where \"$br\" is somehow empty and a 40-hex ref is created. Please\n"
>   "examine these refs and maybe delete them. Turn this message off by\n"
> - "running \"git config advice.object_name_warning false\"");
> + "running \"git config advice.objectNameWarning false\"");
>   unsigned char tmp_sha1[20];
>   char *real_ref = NULL;
>   int refs_found = 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 ALTERNATIVE v6 0/2] http.. and friends

2013-07-31 Thread Junio C Hamano
Thanks for a quick turnaround.  Will replace the two patches with
these and queue.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Making a patch: "git format-patch" does not produce the documented format

2013-07-31 Thread John Keeping
On Wed, Jul 31, 2013 at 05:31:47PM -0400, Dale R. Worley wrote:
> Notice that the whole commit message has been formatted as if it is
> part of the Subject line, and the line breaks in the commit message
> have been refilled.
> 
> The file Documentation/SubmittingPatches says that "git format-patch"
> produces patches in the best format, but the manual page shows an
> example more like this:
> 
> From 8f72bad1baf19a53459661343e21d6491c3908d3 Mon Sep 17 00:00:00 2001
> From: Tony Luck 
> Date: Tue, 13 Jul 2010 11:42:54 -0700
> Subject: [PATCH] Put ia64 config files on the
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
> 
> arch/arm config files were slimmed down using a python script
> (See commit c2330e286f68f1c408b4aa6515ba49d57f05beae comment)
> [...]
> 
> That is, the first line of the commit message is in the Subject and
> the remaining lines are in the message body.  As far as I can tell,
> that's what SubmittingPatches prescribes.  And that is what I see in
> the Git mailing list on vger.
> 
> (This is with git 1.8.3.3.)
> 
> Exactly how should the commit message be inserted into the patch
> e-mail?  What needs to be updated so the code is consistent with the
> documentation?

git-format-patch(1) says:

By default, the subject of a single patch is "[PATCH] " followed
by the concatenation of lines from the commit message up to the
first blank line (see the DISCUSSION section of git-commit(1)).

I think that accurately describes what you're seeing.  The referenced
DISCUSSION section describes how to write a commit message that is
formatted in a suitable way, with a short first subject line and then a
blank line before the body of the 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 2/2] Don't close pack fd when free'ing pack windows

2013-07-31 Thread Fredrik Gustafsson
On Wed, Jul 31, 2013 at 02:31:34PM -0700, Brandon Casey wrote:
> On Wed, Jul 31, 2013 at 2:21 PM, Fredrik Gustafsson  wrote:
> > On Wed, Jul 31, 2013 at 11:08:21PM +0200, Antoine Pelisse wrote:
> >> On Wed, Jul 31, 2013 at 9:51 PM, Brandon Casey  wrote:
> >> > ---
> >> > This email message is for the sole use of the intended recipient(s) and 
> >> > may contain
> >> > confidential information.  Any unauthorized review, use, disclosure or 
> >> > distribution
> >> > is prohibited.  If you are not the intended recipient, please contact 
> >> > the sender by
> >> > reply email and destroy all copies of the original message.
> >> > ---
> >>
> >> I'm certainly not a lawyer, and I'm sorry for not reviewing the
> >> content of the patch instead, but is that not a problem from a legal
> >> point of view ?
> >
> > Talking about legal, is it a problem if a commit isn't signed-off by
> > it's committer or author e-mail? Like in this case where the sign-off is
> > from gmail.com and the committer from nvidia.com?
> 
> It never has been.  My commits should have the author and committer
> set to my gmail address actually.

Oh, that's why the extra "From: " - field below the header is for.

> 
> Others have sometimes used the two fields to distinguish between a
> corporate identity (i.e. m...@somecompany.com) that represents the
> funder of the work and a canonical identity (m...@personalemail.com)
> that identifies the person that performed the work.
> 

In some contries your work when you're employed does not belong
to you but to your employer and when you're acting for your employer
you're representing the corporate legal person. Therefore two different
e-mails can be seen as two different (legal not physical) persons.

At least that's how I understand those "legal tips for developers" I've
got.

-- 
Med vänliga hälsningar
Fredrik Gustafsson

tel: 0733-608274
e-post: iv...@iveqy.com
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Making a patch: "git format-patch" does not produce the documented format

2013-07-31 Thread Dale R. Worley
I'm working on writing a patch, but I'm running into a problem.  The
patch itself is from this commit:

$ git log -1
commit 07a25537909dd277426818a39d9bc4235e755383
Author: Dale Worley 
Date:   Thu Jul 18 18:43:12 2013 -0400

open() returns -1 on failure, and indeed 0 is a possible success value
if the user closed stdin in our process.  Fix the test.
$ 

But the output of "git format-patch" is:

From 07a25537909dd277426818a39d9bc4235e755383 Mon Sep 17 00:00:00 2001
From: Dale Worley 
Date: Thu, 18 Jul 2013 18:43:12 -0400
Subject: [PATCH] open() returns -1 on failure, and indeed 0 is a possible
 success value if the user closed stdin in our process.  Fix
 the test.

---
 t/t0070-fundamental.sh |7 +++
 wrapper.c  |2 +-
 2 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/t/t0070-fundamental.sh b/t/t0070-fundamental.sh
index 986b2a8..d427f3a 100755
--- a/t/t0070-fundamental.sh
+++ b/t/t0070-fundamental.sh
@@ -25,6 +25,13 @@ test_expect_success POSIXPERM,SANITY 'mktemp to 
unwritable directory prints file
grep "cannotwrite/test" err
[...]

Notice that the whole commit message has been formatted as if it is
part of the Subject line, and the line breaks in the commit message
have been refilled.

The file Documentation/SubmittingPatches says that "git format-patch"
produces patches in the best format, but the manual page shows an
example more like this:

From 8f72bad1baf19a53459661343e21d6491c3908d3 Mon Sep 17 00:00:00 2001
From: Tony Luck 
Date: Tue, 13 Jul 2010 11:42:54 -0700
Subject: [PATCH] Put ia64 config files on the
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

arch/arm config files were slimmed down using a python script
(See commit c2330e286f68f1c408b4aa6515ba49d57f05beae comment)
[...]

That is, the first line of the commit message is in the Subject and
the remaining lines are in the message body.  As far as I can tell,
that's what SubmittingPatches prescribes.  And that is what I see in
the Git mailing list on vger.

(This is with git 1.8.3.3.)

Exactly how should the commit message be inserted into the patch
e-mail?  What needs to be updated so the code is consistent with the
documentation?

Dale
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] Don't close pack fd when free'ing pack windows

2013-07-31 Thread Brandon Casey
On Wed, Jul 31, 2013 at 2:21 PM, Fredrik Gustafsson  wrote:
> On Wed, Jul 31, 2013 at 11:08:21PM +0200, Antoine Pelisse wrote:
>> On Wed, Jul 31, 2013 at 9:51 PM, Brandon Casey  wrote:
>> > ---
>> > This email message is for the sole use of the intended recipient(s) and 
>> > may contain
>> > confidential information.  Any unauthorized review, use, disclosure or 
>> > distribution
>> > is prohibited.  If you are not the intended recipient, please contact the 
>> > sender by
>> > reply email and destroy all copies of the original message.
>> > ---
>>
>> I'm certainly not a lawyer, and I'm sorry for not reviewing the
>> content of the patch instead, but is that not a problem from a legal
>> point of view ?
>
> Talking about legal, is it a problem if a commit isn't signed-off by
> it's committer or author e-mail? Like in this case where the sign-off is
> from gmail.com and the committer from nvidia.com?

It never has been.  My commits should have the author and committer
set to my gmail address actually.

Others have sometimes used the two fields to distinguish between a
corporate identity (i.e. m...@somecompany.com) that represents the
funder of the work and a canonical identity (m...@personalemail.com)
that identifies the person that performed the work.

-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 2/2] Don't close pack fd when free'ing pack windows

2013-07-31 Thread Thomas Rast
Antoine Pelisse  writes:

> On Wed, Jul 31, 2013 at 9:51 PM, Brandon Casey  wrote:
>> ---
>> This email message is for the sole use of the intended recipient(s) and may 
>> contain
>> confidential information.  Any unauthorized review, use, disclosure or 
>> distribution
>> is prohibited.  If you are not the intended recipient, please contact the 
>> sender by
>> reply email and destroy all copies of the original message.
>> ---
>
> I'm certainly not a lawyer, and I'm sorry for not reviewing the
> content of the patch instead, but is that not a problem from a legal
> point of view ?
> I remember a video of Greg Kroah-Hartman where he talked about that
> (the video was posted by Junio on G+).

It's this video:

  http://www.youtube.com/watch?v=fMeH7wqOwXA

The comment starts at 13:55.

-- 
Thomas Rast
trast@{inf,student}.ethz.ch
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] Don't close pack fd when free'ing pack windows

2013-07-31 Thread Brandon Casey
On Wed, Jul 31, 2013 at 2:08 PM, Antoine Pelisse  wrote:
> On Wed, Jul 31, 2013 at 9:51 PM, Brandon Casey  wrote:
>> ---
>> This email message is for the sole use of the intended recipient(s) and may 
>> contain
>> confidential information.  Any unauthorized review, use, disclosure or 
>> distribution
>> is prohibited.  If you are not the intended recipient, please contact the 
>> sender by
>> reply email and destroy all copies of the original message.
>> ---
>
> I'm certainly not a lawyer, and I'm sorry for not reviewing the
> content of the patch instead, but is that not a problem from a legal
> point of view ?
> I remember a video of Greg Kroah-Hartman where he talked about that
> (the video was posted by Junio on G+).

Me either thank God.  Are those footers even enforceable?  I mean,
really, if someone mistakenly sends me their corporate financial
numbers am I supposed to be under some legal obligation not to share
it?  I always assumed it was a scare tactic that lawyers like to use.

To address the text of the footer, I'd say the "intended recipient(s)"
are those on the "to" line which includes git@vger.kernel.org and the
implicit use is for inclusion and distribution in the git source code.

Anyway, I doubt I would have any influence on getting the footer
removed.  If Junio would rather me not submit patches with that
footer, then I'd try to find a workaround.

-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 2/2] Don't close pack fd when free'ing pack windows

2013-07-31 Thread Fredrik Gustafsson
On Wed, Jul 31, 2013 at 11:08:21PM +0200, Antoine Pelisse wrote:
> On Wed, Jul 31, 2013 at 9:51 PM, Brandon Casey  wrote:
> > ---
> > This email message is for the sole use of the intended recipient(s) and may 
> > contain
> > confidential information.  Any unauthorized review, use, disclosure or 
> > distribution
> > is prohibited.  If you are not the intended recipient, please contact the 
> > sender by
> > reply email and destroy all copies of the original message.
> > ---
> 
> I'm certainly not a lawyer, and I'm sorry for not reviewing the
> content of the patch instead, but is that not a problem from a legal
> point of view ?

Talking about legal, is it a problem if a commit isn't signed-off by
it's committer or author e-mail? Like in this case where the sign-off is
from gmail.com and the committer from nvidia.com?

-- 
Med vänliga hälsningar
Fredrik Gustafsson

tel: 0733-608274
e-post: iv...@iveqy.com
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] Don't close pack fd when free'ing pack windows

2013-07-31 Thread Antoine Pelisse
On Wed, Jul 31, 2013 at 9:51 PM, Brandon Casey  wrote:
> ---
> This email message is for the sole use of the intended recipient(s) and may 
> contain
> confidential information.  Any unauthorized review, use, disclosure or 
> distribution
> is prohibited.  If you are not the intended recipient, please contact the 
> sender by
> reply email and destroy all copies of the original message.
> ---

I'm certainly not a lawyer, and I'm sorry for not reviewing the
content of the patch instead, but is that not a problem from a legal
point of view ?
I remember a video of Greg Kroah-Hartman where he talked about that
(the video was posted by Junio on G+).
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] l10n: de.po: translate 99 new messages

2013-07-31 Thread Thomas Rast
Hi Ralf

I made a feeble attempt to work directly on de.po so that I could send a
patch instead, but your patch was again linewrapping damaged at the last
line here at least:

  -"Arbeitsverzeichnis von Submodul in '$sm_path' enthält lokale Änderungen; "
  -"verwenden Sie '-f' um diese zu verwerfen"
  +msgstr "Arbeitsverzeichnis von Submodul in '$displaypath' enthält
  lokale Änderungen; verwenden Sie '-f' um diese zu verwerfen"
 
Was it intended for application?  If so, can you fix your tooling to
send correct patches?


Ralf Thielow  writes:

> -#, fuzzy, c-format
> +#, c-format
>  msgid "%s: %s - %s"
> -msgstr "%s: %s"
> +msgstr "%s: %s - %s"

Heh.  Curious that this is even translatable.

[...]
> @@ -1221,10 +1221,22 @@ msgid ""
>  "\n"
>  "where \"$br\" is somehow empty and a 40-hex ref is created. Please\n"
>  "examine these refs and maybe delete them. Turn this message off by\n"
>  "running \"git config advice.object_name_warning false\""
>  msgstr ""
> +"Git erzeugt normalerweise keine Referenzen die mit\n"
> +"40 Hex-Zeichen enden, da diese ignoriert werden wenn\n"
> +"Sie diese angeben. Diese Referenzen könnten ausversehen\n"

aus Versehen?

[...]
> @@ -3529,10 +3525,14 @@ msgid ""
>  "Prompt help:\n"
>  "1  - select a numbered item\n"
>  "foo- select item based on unique prefix\n"
>  "   - (empty) select nothing"
>  msgstr ""
> +"Eingabehilfe:\n"
> +"1  - numeriertes Element auswählen\n"

Isn't it "nummeriert" under the new spelling rules?

[...]
>  #: builtin/clean.c:634
>  #, c-format
>  msgid "Input ignore patterns>> "
> -msgstr ""
> +msgstr "Eingabe Ignorier-Muster>> "

"Input" here is an imperative, so it should be

  Ignorier-Muster eingeben>>

[...]
>  msgid "set From address to  (or committer ident if absent)"
>  msgstr ""
> +"setzt \"From\"-Adresse zu  (oder Identifikation\n"
> +"des Commit-Erstellers, wenn fehlend)"

  setzt \"From\"-Adresse auf 
 ^^^

or is that my Swiss German again?

In addition, I think  would sound smoother as Identität (in any
case that's what I think it expands to in English).  There are other
occurrences of "ident" that you would need to change to match.

[...]
>  msgid_plural ""
>  "the following submodules (or one of its nested submodules)\n"
>  "use a .git directory:"
>  msgstr[0] ""
> +"das folgende Submodul (oder ein geschachteltes Submodul hiervon)\n"
> +"benutzt ein .git-Verzeichnis:"
>  msgstr[1] ""
> +"die folgenden Submodule (oder ein geschachteltes Submodul hiervon)\n"
> +"benutzt ein .git-Verzeichnis:"

The sentence is not fully in plural; it should probably be

"die folgenden Submodule (oder geschachtelte Submodule hiervon)\n"
"benutzen ein .git-Verzeichnis:"


[...]
>  #: git-rebase.sh:160
>  msgid ""
>  "Applying autostash resulted in conflicts.\n"
>  "Your changes are safe in the stash.\n"
>  "You can run \"git stash pop\" or \"git stash drop\" it at any time.\n"
>  msgstr ""
> +"Anwendung von \"autostash\" resultierte in Konflikten.\n"
> +"Ihre Änderungen sind sicher im Stash.\n"

There would be no ambiguity in the intended meaning of "sicher" if you
instead said

  Ihre Änderungen sind im Stash sicher.

(Note that the ambiguity does not exist in English: the other meaning
would be "surely".)

> +"Sie können jederzeit \"git stash pop\" oder \"git stash drop\" ausführen.\n"
[...]

-- 
Thomas Rast
trast@{inf,student}.ethz.ch
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH ALTERNATIVE v6 4/4] config: parse http.. using urlmatch

2013-07-31 Thread Kyle J. McKay
Use the urlmatch_config_entry() to wrap the underlying
http_options() two-level variable parser in order to set
http. to the value with the most specific URL in the
configuration.

Signed-off-by: Jeff King 
Signed-off-by: Kyle J. McKay 
Signed-off-by: Junio C Hamano 
---
 .gitignore   |   1 +
 Documentation/config.txt |  46 +++
 Makefile |   7 ++
 http.c   |  13 +++-
 t/.gitattributes |   1 +
 t/t5200-url-normalize.sh | 199 +++
 t/t5200/README   |  18 +
 t/t5200/config-1 |   8 ++
 t/t5200/config-2 |   3 +
 t/t5200/config-3 |   4 +
 t/t5200/url-1| Bin 0 -> 20 bytes
 t/t5200/url-10   | Bin 0 -> 23 bytes
 t/t5200/url-11   | Bin 0 -> 25 bytes
 t/t5200/url-2| Bin 0 -> 20 bytes
 t/t5200/url-3| Bin 0 -> 23 bytes
 t/t5200/url-4| Bin 0 -> 23 bytes
 t/t5200/url-5| Bin 0 -> 23 bytes
 t/t5200/url-6| Bin 0 -> 23 bytes
 t/t5200/url-7| Bin 0 -> 23 bytes
 t/t5200/url-8| Bin 0 -> 23 bytes
 t/t5200/url-9| Bin 0 -> 23 bytes
 test-url-normalize.c | 139 +
 22 files changed, 438 insertions(+), 1 deletion(-)
 create mode 100755 t/t5200-url-normalize.sh
 create mode 100644 t/t5200/README
 create mode 100644 t/t5200/config-1
 create mode 100644 t/t5200/config-2
 create mode 100644 t/t5200/config-3
 create mode 100644 t/t5200/url-1
 create mode 100644 t/t5200/url-10
 create mode 100644 t/t5200/url-11
 create mode 100644 t/t5200/url-2
 create mode 100644 t/t5200/url-3
 create mode 100644 t/t5200/url-4
 create mode 100644 t/t5200/url-5
 create mode 100644 t/t5200/url-6
 create mode 100644 t/t5200/url-7
 create mode 100644 t/t5200/url-8
 create mode 100644 t/t5200/url-9
 create mode 100644 test-url-normalize.c

diff --git a/.gitignore b/.gitignore
index 6669bf0c..cd97e16a 100644
--- a/.gitignore
+++ b/.gitignore
@@ -198,6 +198,7 @@
 /test-string-list
 /test-subprocess
 /test-svn-fe
+/test-url-normalize
 /test-wildmatch
 /common-cmds.h
 *.tar.gz
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 6e53fc50..8cc0fd78 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1513,6 +1513,52 @@ http.useragent::
of common USER_AGENT strings (but not including those like git/1.7.1).
Can be overridden by the 'GIT_HTTP_USER_AGENT' environment variable.
 
+http..*::
+   Any of the http.* options above can be applied selectively to some urls.
+   For a config key to match a URL, each element of the config key is
+   compared to that of the URL, in the following order:
++
+--
+. Scheme (e.g., `https` in `https://example.com/`). This field
+  must match exactly between the config key and the URL.
+
+. Host/domain name (e.g., `example.com` in `https://example.com/`).
+  This field must match exactly between the config key and the URL.
+
+. Port number (e.g., `8080` in `http://example.com:8080/`).
+  This field must match exactly between the config key and the URL.
+  Omitted port numbers are automatically converted to the correct
+  default for the scheme before matching.
+
+. Path (e.g., `repo.git` in `https://example.com/repo.git`). The
+  path field of the config key must match the path field of the URL
+  either exactly or as a prefix of slash-delimited path elements.  This means
+  a config key with path `foo/` matches URL path `foo/bar`.  A prefix can only
+  match on a slash (`/`) boundary.  Longer matches take precedence (so a config
+  key with path `foo/bar` is a better match to URL path `foo/bar` than a config
+  key with just path `foo/`).
+
+. User name (e.g., `user` in `https://u...@example.com/repo.git`). If
+  the config key has a user name it must match the user name in the
+  URL exactly. If the config key does not have a user name, that
+  config key will match a URL with any user name (including none).
+  config key will match a URL with any user name (including none),
+  but at a lower precedence than a config key with a user name.
+--
++
+The list above is ordered by decreasing precedence; a URL that matches
+a config key's path is preferred to one that matches its user name. For 
example,
+if the URL is `https://u...@example.com/foo/bar` a config key match of
+`https://example.com/foo` will be preferred over a config key match of
+`https://u...@example.com`.
++
+All URLs are normalized before attempting any matching (the password part,
+if embedded in the URL, is always ignored for matching purposes) so that
+equivalent urls that are simply spelled differently will match properly.
+Environment variable settings always override any matches.  The urls that are
+matched against are those given directly to Git commands.  This means any URLs
+visited as a result of a redirection do not participate in matching.
+
 i18n.commitEncoding::
Character encoding the commit mes

[PATCH ALTERNATIVE v6 0/2] http.. and friends

2013-07-31 Thread Kyle J. McKay
This patch simply provides two alternative versions of the 2/6 and 4/6
patches previously sent as part of the:

  [PATCH v6 0/6] http.. and friends

series.  They are intended simply as complete alternatives to parts 2 and 4
that include the following changes:

2/4 - Include 1-line documentation update in log comment and rename static
  function from http_options_url_match_prefix to url_match_prefix.
  
4/4 - Include 1-line documentation update together with Peff's previously
  provided Signed-off-by for the copious amount of documentation text he
  has provided that has been included verbatim.  Also include the minor
  fixes from Ramsay Jones for compilation of test-url-normalize when
  NO_CURL is defined.

If simply squashing the patches in instead is desired, here's the patch for
part 2/4:

diff --git a/urlmatch.c b/urlmatch.c
index e1b03ee7..4f38cc7b 100644
--- a/urlmatch.c
+++ b/urlmatch.c
@@ -376,9 +376,9 @@ char *url_normalize(const char *url, struct url_info 
*out_info)
return result;
 }
 
-static size_t http_options_url_match_prefix(const char *url,
-   const char *url_prefix,
-   size_t url_prefix_len)
+static size_t url_match_prefix(const char *url,
+  const char *url_prefix,
+  size_t url_prefix_len)
 {
/*
 * url_prefix matches url if url_prefix is an exact match for url or it
@@ -457,7 +457,7 @@ int match_urls(const struct url_info *url,
return 0; /* host names and/or ports do not match */
 
/* check the path */
-   pathmatchlen = http_options_url_match_prefix(
+   pathmatchlen = url_match_prefix(
url->url + url->path_off,
url_prefix->url + url_prefix->path_off,
url_prefix->url_len - url_prefix->path_off);
---

And here's the patch for part 4/4:

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 60c140f1..8cc0fd78 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1542,6 +1542,8 @@ http..*::
   the config key has a user name it must match the user name in the
   URL exactly. If the config key does not have a user name, that
   config key will match a URL with any user name (including none).
+  config key will match a URL with any user name (including none),
+  but at a lower precedence than a config key with a user name.
 --
 +
 The list above is ordered by decreasing precedence; a URL that matches
diff --git a/test-url-normalize.c b/test-url-normalize.c
index 81d3da90..80437217 100644
--- a/test-url-normalize.c
+++ b/test-url-normalize.c
@@ -1,6 +1,6 @@
 #ifdef NO_CURL
 
-int main()
+int main(void)
 {
return 125;
 }
@@ -52,8 +52,10 @@ static int run_http_options(const char *file,
printf("%s\n", curl_ssl_try ? "true" : "false");
else if (!strcmp("minsessions", opt_lc.buf))
printf("%d\n", min_curl_sessions);
+#ifdef USE_CURL_MULTI
else if (!strcmp("maxrequests", opt_lc.buf))
printf("%d\n", max_requests);
+#endif
else if (!strcmp("lowspeedlimit", opt_lc.buf))
printf("%ld\n", curl_low_speed_limit);
else if (!strcmp("lowspeedtime", opt_lc.buf))
---
--
To unsubscribe from this list: send the line "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 ALTERNATIVE v6 2/4] config: add helper to normalize and match URLs

2013-07-31 Thread Kyle J. McKay
Some http.* configuration variables need to take values customized
for the URL we are talking to.  We may want to set http.sslVerify to
true in general but to false only for a certain site, for example,
with a configuration file like this:

[http]
sslVerify = true
[http "https://weak.example.com";]
sslVerify = false

and let the configuration machinery pick up the latter only when
talking to "https://weak.example.com";.  The latter needs to kick in
not only when the URL is exactly "https://weak.example.com";, but
also is anything that "match" it, e.g.

https://weak.example.com/test
https://m...@weak.example.com/test

The  in the configuration key consists of the following parts,
and is considered a match to the URL we are attempting to access
under certain conditions:

  . Scheme (e.g., `https` in `https://example.com/`). This field
must match exactly between the config key and the URL.

  . Host/domain name (e.g., `example.com` in `https://example.com/`).
This field must match exactly between the config key and the URL.

  . Port number (e.g., `8080` in `http://example.com:8080/`).  This
field must match exactly between the config key and the URL.
Omitted port numbers are automatically converted to the correct
default for the scheme before matching.

  . Path (e.g., `repo.git` in `https://example.com/repo.git`). The
path field of the config key must match the path field of the
URL either exactly or as a prefix of slash-delimited path
elements.  A config key with path `foo/` matches URL path
`foo/bar`.  A prefix can only match on a slash (`/`) boundary.
Longer matches take precedence (so a config key with path
`foo/bar` is a better match to URL path `foo/bar` than a config
key with just path `foo/`).

  . User name (e.g., `me` in `https://m...@example.com/repo.git`). If
the config key has a user name, it must match the user name in
the URL exactly. If the config key does not have a user name,
that config key will match a URL with any user name (including
none), but at a lower precedence than a config key with a user
name.

Longer matches take precedence over shorter matches.

This step adds two helper functions `url_normalize()` and
`match_urls()` to help implement the above semantics. The
normalization rules are based on RFC 3986 and should result in any
two equivalent urls being a match.

Signed-off-by: Kyle J. McKay 
Signed-off-by: Junio C Hamano 
---
 urlmatch.c | 468 +
 urlmatch.h |  36 +
 2 files changed, 504 insertions(+)
 create mode 100644 urlmatch.c
 create mode 100644 urlmatch.h

diff --git a/urlmatch.c b/urlmatch.c
new file mode 100644
index ..4f38cc7b
--- /dev/null
+++ b/urlmatch.c
@@ -0,0 +1,468 @@
+#include "cache.h"
+#include "urlmatch.h"
+
+#define URL_ALPHA "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz"
+#define URL_DIGIT "0123456789"
+#define URL_ALPHADIGIT URL_ALPHA URL_DIGIT
+#define URL_SCHEME_CHARS URL_ALPHADIGIT "+.-"
+#define URL_HOST_CHARS URL_ALPHADIGIT ".-[:]" /* IPv6 literals need [:] */
+#define URL_UNSAFE_CHARS " <>\"%{}|\\^`" /* plus 0x00-0x1F,0x7F-0xFF */
+#define URL_GEN_RESERVED ":/?#[]@"
+#define URL_SUB_RESERVED "!$&'()*+,;="
+#define URL_RESERVED URL_GEN_RESERVED URL_SUB_RESERVED /* only allowed delims 
*/
+
+static int append_normalized_escapes(struct strbuf *buf,
+const char *from,
+size_t from_len,
+const char *esc_extra,
+const char *esc_ok)
+{
+   /*
+* Append to strbuf 'buf' characters from string 'from' with length
+* 'from_len' while unescaping characters that do not need to be escaped
+* and escaping characters that do.  The set of characters to escape
+* (the complement of which is unescaped) starts out as the RFC 3986
+* unsafe characters (0x00-0x1F,0x7F-0xFF," <>\"#%{}|\\^`").  If
+* 'esc_extra' is not NULL, those additional characters will also always
+* be escaped.  If 'esc_ok' is not NULL, those characters will be left
+* escaped if found that way, but will not be unescaped otherwise (used
+* for delimiters).  If a %-escape sequence is encountered that is not
+* followed by 2 hexadecimal digits, the sequence is invalid and
+* false (0) will be returned.  Otherwise true (1) will be returned for
+* success.
+*
+* Note that all %-escape sequences will be normalized to UPPERCASE
+* as indicated in RFC 3986.  Unless included in esc_extra or esc_ok
+* alphanumerics and "-._~" will always be unescaped as per RFC 3986.
+*/
+
+   while (from_len) {
+   int ch = *from++;
+   int was_esc = 0;
+
+   from_len--;
+   if (ch == '%'

Re: [PATCH v6 4/6] config: parse http.. using urlmatch

2013-07-31 Thread Kyle J. McKay

On Jul 31, 2013, at 12:26, Junio C Hamano wrote:


From: "Kyle J. McKay" 

Use the urlmatch_config_entry() to wrap the underlying
http_options() two-level variable parser in order to set
http. to the value with the most specific URL in the
configuration.

Signed-off-by: Kyle J. McKay 
Signed-off-by: Junio C Hamano 


Needs Peff's Signed-off-by: for the copious amount of text he wrote  
that is included verbatim in the documentation part of the patch.  He  
previously gave it for this purpose.



diff --git a/Documentation/config.txt b/Documentation/config.txt
index 6e53fc5..60c140f 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1513,6 +1513,50 @@ http.useragent::
	of common USER_AGENT strings (but not including those like git/ 
1.7.1).

Can be overridden by the 'GIT_HTTP_USER_AGENT' environment variable.

+http..*::
+	Any of the http.* options above can be applied selectively to some  
urls.

+   For a config key to match a URL, each element of the config key is
+   compared to that of the URL, in the following order:
++
+--
+. Scheme (e.g., `https` in `https://example.com/`). This field
+  must match exactly between the config key and the URL.
+
+. Host/domain name (e.g., `example.com` in `https://example.com/`).
+  This field must match exactly between the config key and the URL.
+
+. Port number (e.g., `8080` in `http://example.com:8080/`).
+  This field must match exactly between the config key and the URL.
+  Omitted port numbers are automatically converted to the correct
+  default for the scheme before matching.
+
+. Path (e.g., `repo.git` in `https://example.com/repo.git`). The
+  path field of the config key must match the path field of the URL
+  either exactly or as a prefix of slash-delimited path elements.   
This means
+  a config key with path `foo/` matches URL path `foo/bar`.  A  
prefix can only
+  match on a slash (`/`) boundary.  Longer matches take precedence  
(so a config
+  key with path `foo/bar` is a better match to URL path `foo/bar`  
than a config

+  key with just path `foo/`).
+
+. User name (e.g., `user` in `https://u...@example.com/repo.git`). If
+  the config key has a user name it must match the user name in the
+  URL exactly. If the config key does not have a user name, that
+  config key will match a URL with any user name (including none).


Missing the single line follow-up patch:


diff --git a/Documentation/config.txt b/Documentation/config.txt
index 0dd5566..f2ed9ef 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1568,7 +1568,8 @@ http..*::
. User name (e.g., `user` in `https://u...@example.com/repo.git`). If
  the config key has a user name it must match the user name in the
  URL exactly. If the config key does not have a user name, that
-  config key will match a URL with any user name (including none).
+  config key will match a URL with any user name (including none),
+  but at a lower precedence than a config key with a user name.
--





diff --git a/test-url-normalize.c b/test-url-normalize.c
new file mode 100644
index 000..81d3da9
--- /dev/null
+++ b/test-url-normalize.c
@@ -0,0 +1,137 @@
+#ifdef NO_CURL
+
+int main()


Need's Ramsey's patch here:

-int main()
+int main(void)


+static int run_http_options(const char *file,
+   const char *opt,
+   const struct url_info *info)
+{
+   struct strbuf opt_lc;
+   size_t i, len;
+   struct urlmatch_config config = { STRING_LIST_INIT_DUP };
+
+   memcpy(&config.url, info, sizeof(*info));
+   config.section = "http";
+   config.collect_fn = http_options;
+   config.cascade_fn = git_default_config;
+   config.cb = NULL;
+
+	if (git_config_with_options(urlmatch_config_entry, &config, file,  
0))

+   return 1;
+
+   len = strlen(opt);
+   strbuf_init(&opt_lc, len);
+   for (i = 0; i < len; ++i) {
+   strbuf_addch(&opt_lc, tolower(opt[i]));
+   }
+
+   if (!strcmp("sslverify", opt_lc.buf))
+   printf("%s\n", curl_ssl_verify ? "true" : "false");
+   else if (!strcmp("sslcert", opt_lc.buf))
+   printf("%s\n", ssl_cert);
+#if LIBCURL_VERSION_NUM >= 0x070903
+   else if (!strcmp("sslkey", opt_lc.buf))
+   printf("%s\n", ssl_key);
+#endif
+#if LIBCURL_VERSION_NUM >= 0x070908
+   else if (!strcmp("sslcapath", opt_lc.buf))
+   printf("%s\n", ssl_capath);
+#endif
+   else if (!strcmp("sslcainfo", opt_lc.buf))
+   printf("%s\n", ssl_cainfo);
+   else if (!strcmp("sslcertpasswordprotected", opt_lc.buf))
+   printf("%s\n", ssl_cert_password_required ? "true" : "false");
+   else if (!strcmp("ssltry", opt_lc.buf))
+   printf("%s\n", curl_ssl_try ? "true" : "false");
+   else if (!strcmp("minsessions", opt_lc.buf))
+   printf("%d\n", min_curl_sessions);


And here

+#ifdef USE_CURL_MULTI

+   else if (!strcmp(

Re: [PATCH v6 2/6] config: add helper to normalize and match URLs

2013-07-31 Thread Kyle J. McKay

On Jul 31, 2013, at 12:26, Junio C Hamano wrote:


From: "Kyle J. McKay" 

Some http.* configuration variables need to take values customized
for the URL we are talking to.  We may want to set http.sslVerify to
true in general but to false only for a certain site, for example,
with a configuration file like this:

[...]
urlmatch.c | 468  
+

urlmatch.h |  36 +
2 files changed, 504 insertions(+)
create mode 100644 urlmatch.c
create mode 100644 urlmatch.h

diff --git a/urlmatch.c b/urlmatch.c
new file mode 100644
index 000..e1b03ee
--- /dev/null
+++ b/urlmatch.c

[...]

+
+static size_t http_options_url_match_prefix(const char *url,
+   const char *url_prefix,
+   size_t url_prefix_len)
+{
+   /*
+	 * url_prefix matches url if url_prefix is an exact match for url  
or it
+	 * is a prefix of url and the match ends on a path component  
boundary.
+	 * Both url and url_prefix are considered to have an implicit '/'  
on the

+* end for matching purposes if they do not already.


This function should probably be renamed to just url_match_prefix  
since it isn't part of nor does it depend on the http_options related  
files or functions anymore.


Otherwise looks good to me.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Difficulty adding a symbolic link, part 3

2013-07-31 Thread Dale R. Worley
I've run into a problem (with Git 1.8.3.3) where I cannot add a
symbolic link (as such) to the repository *if* its path is given
absolutely; instead Git adds the file the symbolic link points to.
(If I give the path relatively, Git does what I expect, that is, adds
the symbolic link.)

I've written a test script that shows the problem and included it
below.

I would not expect *how* a path is presented to Git to change how Git
processes the path.  In the test case, I would expect "/bin/awk" and
"../../bin/awk" to produce the same effect when used as arguments to
"git add".

What is going on in the code is this:  In "git add", all paths are
normalized by the function prefix_path_gently() in abspath.c.  That
function removes symbolic links from the pathspec *only if* it is
absolute, as shown in the first few lines of the function:

 static char *prefix_path_gently(const char *prefix, int len, const char *path)
 {
 const char *orig = path;
 char *sanitized;
 if (is_absolute_path(orig)) {
-const char *temp = real_path(path);
+const char *temp = absolute_path(path);
 sanitized = xmalloc(len + strlen(temp) + 1);
 strcpy(sanitized, temp);
 } else {

real_path() is specified to remove symbolic links.  As shown, I've
replaced real_path() with absolute_path(), based on the comment at the
top of real_path():

/*
 * Return the real path (i.e., absolute path, with symlinks resolved
 * and extra slashes removed) equivalent to the specified path.  (If
 * you want an absolute path but don't mind links, use
 * absolute_path().)  The return value is a pointer to a static
 * buffer.
 *

If I replace real_path() with absolute_path() as shown, the problem I
am testing for disappears.

With the above change, the test suite runs with zero failures, so it
doesn't affect any common Git usage.

But I don't know enough about the internal architecture of Git to know
that my change is correct in all cases.  I'm almost certain that the
normalization process for pathspecs should *not* normalize a final
component that is a symbolic link.  But I would expect it would be
desirable to normalize non-final components that are symbolic links.
On the other hand, that might not matter.

Can someone give me advice on what this code *should* do?

I believe I can prepare a proper test for the test suite for this, so
once I know what the code change should be, I can prepare a proper
patch for it.

Dale
--  
Here's a test case for adding a symbolic link.  This test exploits the
fact that on my system, /bin/awk is a symbolic link to "gawk".  As you
can see, the behavior of Git differs if the link's path is given to
"git add" as an absolute path or a relative path.

Here is the test script:
--
#! /bin/bash

# Illustrates a problem with applying "git add" to a symbolic link.

set -x

# To be run from a directory one step below the root directory.  E.g.,
# "/tmp".
# On this system, /bin/awk is a symbolic link to "gawk", which
# means /tmp/gawk.

# Show the Git version.
git version

# Make a test directory and cd to it.
DIR=temp.$$
mkdir $DIR
cd $DIR

# Create a Git repository.
git init
# Set the worktree to be /
git config core.worktree /
# Create an empty commit.
git commit --allow-empty -m Empty.

# Add the symbolic link using its absolute name.
ABSOLUTE=/bin/awk
ls -l $ABSOLUTE
git add $ABSOLUTE
# Notice that the target of the link is added, not the link itself.
git status -uno

# Reset the index.
git reset

# Add the symbolic link using its relative name.
# Remember that we are two directory levels below the root directory now.
RELATIVE=../..$ABSOLUTE
ls -l $RELATIVE
git add $RELATIVE
# Notice that now the link itself is added.
git status -uno
--
Here is sample output of the script:
--
+ git version
git version 1.8.3.3.756.g07a2553.dirty
+ DIR=temp.22366
+ mkdir temp.22366
+ cd temp.22366
+ git init
Initialized empty Git repository in /git-add-link/temp.22366/.git/
+ git config core.worktree /
+ git commit --allow-empty -m Empty.
[master (root-commit) fb232e5] Empty.
+ ABSOLUTE=/bin/awk
+ ls -l /bin/awk
lrwxrwxrwx. 1 root root 4 Nov  2  2012 /bin/awk -> gawk
+ git add /bin/awk
+ git status -uno
# On branch master
# Changes to be committed:
#   (use "git reset HEAD ..." to unstage)
#
#   new file:   ../../bin/gawk
#
# Untracked files not listed (use -u option to show untracked files)
+ git reset
+ RELATIVE=../../bin/awk
+ ls -l ../../bin/awk
lrwxrwxrwx. 1 root root 4 Nov  2  2012 ../../bin/awk -> gawk
+ git add ../../bin/awk
+ git status -uno
# On branch master
# Changes to be committed:
#   (use "git reset HEAD ..." to unstage)
#
#   new file:   ../../bin/awk
#
# Untracked 

[PATCH] Rename advice.object_name_warning to objectNameWarning

2013-07-31 Thread Thomas Rast
We spell config variables in camelCase instead of with_underscores.

Signed-off-by: Thomas Rast 
---

I figure since we don't have the variable in any release yet *and* the
worst possible outcome is that someone sees the advice message again,
the consistency is worth the change.


 advice.c| 2 +-
 sha1_name.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/advice.c b/advice.c
index 2a52098..3eca9f5 100644
--- a/advice.c
+++ b/advice.c
@@ -35,7 +35,7 @@
{ "implicitidentity", &advice_implicit_identity },
{ "detachedhead", &advice_detached_head },
{ "setupstreamfailure", &advice_set_upstream_failure },
-   { "object_name_warning", &advice_object_name_warning },
+   { "objectnamewarning", &advice_object_name_warning },
{ "rmhints", &advice_rm_hints },
 
/* make this an alias for backward compatibility */
diff --git a/sha1_name.c b/sha1_name.c
index 1d210e3..852dd95 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -445,7 +445,7 @@ static int get_sha1_basic(const char *str, int len, 
unsigned char *sha1)
"\n"
"where \"$br\" is somehow empty and a 40-hex ref is created. Please\n"
"examine these refs and maybe delete them. Turn this message off by\n"
-   "running \"git config advice.object_name_warning false\"");
+   "running \"git config advice.objectNameWarning false\"");
unsigned char tmp_sha1[20];
char *real_ref = NULL;
int refs_found = 0;
-- 
1.8.4.rc0.408.gad6868d

--
To unsubscribe from this list: send the line "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] log: use true parents for diff even when rewriting

2013-07-31 Thread Thomas Rast
When using pathspec filtering in combination with diff-based log
output, parent simplification happens before the diff is computed.
The diff is therefore against the *simplified* parents.

This works okay, arguably by accident, in the normal case:
simplification reduces to one parent as long as the commit is TREESAME
to it.  So the simplified parent of any given commit must have the
same tree contents on the filtered paths as its true (unfiltered)
parent.

However, --full-diff breaks this guarantee, and indeed gives pretty
spectacular results when comparing the output of

  git log --graph --stat ...
  git log --graph --full-diff --stat ...

(--graph internally kicks in parent simplification, much like
--parents).

To fix it, store a copy of the parent list before simplification (in a
slab) whenever --full-diff is in effect.  Then use the stored parents
instead of the simplified ones in the commit display code paths.  The
latter do not actually check for --full-diff to avoid duplicated code;
they just grab the original parents if save_parents() has not been
called for this revision walk.

For ordinary commits it should be obvious that this is the right thing
to do.

Merge commits are a bit subtle.  Observe that with default
simplification, merge simplification is an all-or-nothing decision:
either the merge is TREESAME to one parent and disappears, or it is
different from all parents and the parent list remains intact.
Redundant parents are not pruned, so the existing code also shows them
as a merge.

So if we do show a merge commit, the parent list just consists of the
rewrite result on each parent.  Running, e.g., --cc on this in
--full-diff mode is not very useful: if any commits were skipped, some
hunks will disagree with all sides of the merge (with one side,
because commits were skipped; with the others, because they didn't
have those changes in the first place).  This triggers --cc showing
these hunks spuriously.

Therefore I believe that even for merge commits it is better to show
the diffs wrt. the original parents.

Reported-by: Uwe Kleine-König 
Helped-by: Junio C Hamano 
Helped-by: Ramsay Jones 
Signed-off-by: Thomas Rast 
---

New in this version:

* Junio's and Ramsay's suggestions squashed

* Moved the slab variable from file-static to within the struct
  rev_info.  In practice there is no difference, because you cannot
  run two walks in parallel anyway (they would stomp over each others'
  parent lists!).  But it was easy, feels slightly cleaner, and avoids
  having yet another global variable.

* Wrote an actual commit message.  I hope the logic wrt. merge commits
  is correct.


 combine-diff.c   |  3 ++-
 commit.c | 16 
 commit.h |  3 +++
 log-tree.c   |  2 +-
 revision.c   | 43 ++-
 revision.h   | 20 
 t/t6012-rev-list-simplify.sh |  6 ++
 7 files changed, 90 insertions(+), 3 deletions(-)

diff --git a/combine-diff.c b/combine-diff.c
index 6dc0609..3d2aaf3 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -10,6 +10,7 @@
 #include "refs.h"
 #include "userdiff.h"
 #include "sha1-array.h"
+#include "revision.h"
 
 static struct combine_diff_path *intersect_paths(struct combine_diff_path 
*curr, int n, int num_parent)
 {
@@ -1383,7 +1384,7 @@ void diff_tree_combined(const unsigned char *sha1,
 void diff_tree_combined_merge(const struct commit *commit, int dense,
  struct rev_info *rev)
 {
-   struct commit_list *parent = commit->parents;
+   struct commit_list *parent = get_saved_parents(rev, commit);
struct sha1_array parents = SHA1_ARRAY_INIT;
 
while (parent) {
diff --git a/commit.c b/commit.c
index e5862f6..5ecdb38 100644
--- a/commit.c
+++ b/commit.c
@@ -377,6 +377,22 @@ unsigned commit_list_count(const struct commit_list *l)
return c;
 }
 
+struct commit_list *copy_commit_list(struct commit_list *list)
+{
+   struct commit_list *head = NULL;
+   struct commit_list **pp = &head;
+   while (list) {
+   struct commit_list *new;
+   new = xmalloc(sizeof(struct commit_list));
+   new->item = list->item;
+   new->next = NULL;
+   *pp = new;
+   pp = &new->next;
+   list = list->next;
+   }
+   return head;
+}
+
 void free_commit_list(struct commit_list *list)
 {
while (list) {
diff --git a/commit.h b/commit.h
index d912a9d..f9504f7 100644
--- a/commit.h
+++ b/commit.h
@@ -62,6 +62,9 @@ struct commit_list *commit_list_insert_by_date(struct commit 
*item,
struct commit_list **list);
 void commit_list_sort_by_date(struct commit_list **list);
 
+/* Shallow copy of the input list */
+struct commit_list *copy_commit_list(struct commit_list *list);
+
 void free_commit_list(struct commit_list *list);
 
 /* Commit f

[PATCH v2 1/2] sha1_file: introduce close_one_pack() to close packs on fd pressure

2013-07-31 Thread Brandon Casey
From: Brandon Casey 

When the number of open packs exceeds pack_max_fds, unuse_one_window()
is called repeatedly to attempt to release the least-recently-used
pack windows, which, as a side-effect, will also close a pack file
after closing its last open window.  If a pack file has been opened,
but no windows have been allocated into it, it will never be selected
by unuse_one_window() and hence its file descriptor will not be
closed.  When this happens, git may exceed the number of file
descriptors permitted by the system.

This latter situation can occur in show-ref or receive-pack during ref
advertisement.  During ref advertisement, receive-pack will iterate
over every ref in the repository and advertise it to the client after
ensuring that the ref exists in the local repository.  If the ref is
located inside a pack, then the pack is opened to ensure that it
exists, but since the object is not actually read from the pack, no
mmap windows are allocated.  When the number of open packs exceeds
pack_max_fds, unuse_one_window() will not be able to find any windows to
free and will not be able to close any packs.  Once the per-process
file descriptor limit is exceeded, receive-pack will produce a warning,
not an error, for each pack it cannot open, and will then most likely
fail with an error to spawn rev-list or index-pack like:

   error: cannot create standard input pipe for rev-list: Too many open files
   error: Could not run 'git rev-list'

This may also occur during upload-pack when refs are packed (in the
packed-refs file) and the number of packs that must be opened to
verify that these packed refs exist exceeds the file descriptor limit.
If the refs are loose, then upload-pack will read each ref from the
pack (allocating one or more mmap windows) so it can peel tags and
advertise the underlying object.  If the refs are packed and peeled,
then upload-pack will use the peeled sha1 in the packed-refs file and
will not need to read from the pack files, so no mmap windows will be
allocated and just like with receive-pack, unuse_one_window() will
never select these opened packs to close.

When we have file descriptor pressure, in contrast to memory pressure,
we need to free all windows and close the pack file descriptor so that
a new pack can be opened.  Let's introduce a new function
close_one_pack() designed specifically for this purpose to search
for and close the least-recently-used pack, where LRU is defined as

   * pack with oldest mtime and no allocated mmap windows or
   * pack with the least-recently-used windows, i.e. the pack
 with the oldest most-recently-used window

Signed-off-by: Brandon Casey 
---

The commit message was updated to fix the grammatical error that Eric
Sunshine pointed out, and to correct the paragraph about upload-pack.

-Brandon

 sha1_file.c | 63 -
 1 file changed, 62 insertions(+), 1 deletion(-)

diff --git a/sha1_file.c b/sha1_file.c
index 8e27db1..7731ab1 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -682,6 +682,67 @@ void close_pack_windows(struct packed_git *p)
}
 }
 
+/*
+ * The LRU pack is the one with the oldest MRU window or the oldest mtime
+ * if it has no windows allocated.
+ */
+static void find_lru_pack(struct packed_git *p, struct packed_git **lru_p, 
struct pack_window **mru_w)
+{
+   struct pack_window *w, *this_mru_w;
+
+   /*
+* Reject this pack if it has windows and the previously selected
+* one does not.  If this pack does not have windows, reject
+* it if the pack file is newer than the previously selected one.
+*/
+   if (*lru_p && !*mru_w && (p->windows || p->mtime > (*lru_p)->mtime))
+   return;
+
+   for (w = this_mru_w = p->windows; w; w = w->next) {
+   /* Reject this pack if any of its windows are in use */
+   if (w->inuse_cnt)
+   return;
+   /*
+* Reject this pack if it has windows that have been
+* used more recently than the previously selected pack.
+*/
+   if (*mru_w && w->last_used > (*mru_w)->last_used)
+   return;
+   if (w->last_used > this_mru_w->last_used)
+   this_mru_w = w;
+   }
+
+   /*
+* Select this pack.
+*/
+   *mru_w = this_mru_w;
+   *lru_p = p;
+}
+
+static int close_one_pack(void)
+{
+   struct packed_git *p, *lru_p = NULL;
+   struct pack_window *mru_w = NULL;
+
+   for (p = packed_git; p; p = p->next) {
+   if (p->pack_fd == -1)
+   continue;
+   find_lru_pack(p, &lru_p, &mru_w);
+   }
+
+   if (lru_p) {
+   close_pack_windows(lru_p);
+   close(lru_p->pack_fd);
+   pack_open_fds--;
+   lru_p->pack_fd = -1;
+   if (lru_p == last_found_pack)
+   last_f

[PATCH 2/2] Don't close pack fd when free'ing pack windows

2013-07-31 Thread Brandon Casey
From: Brandon Casey 

Now that close_one_pack() has been introduced to handle file
descriptor pressure, it is not strictly necessary to close the
pack file descriptor in unuse_one_window() when we're under memory
pressure.

Jeff King provided a justification for leaving the pack file open:

   If you close packfile descriptors, you can run into racy situations
   where somebody else is repacking and deleting packs, and they go away
   while you are trying to access them. If you keep a descriptor open,
   you're fine; they last to the end of the process. If you don't, then
   they disappear from under you.

   For normal object access, this isn't that big a deal; we just rescan
   the packs and retry. But if you are packing yourself (e.g., because
   you are a pack-objects started by upload-pack for a clone or fetch),
   it's much harder to recover (and we print some warnings).

Let's do so (or uh, not do so).

Signed-off-by: Brandon Casey 
---
 builtin/pack-objects.c |  2 +-
 git-compat-util.h  |  2 +-
 sha1_file.c| 21 +++--
 3 files changed, 9 insertions(+), 16 deletions(-)

diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index f069462..4eb0521 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -1809,7 +1809,7 @@ static void find_deltas(struct object_entry **list, 
unsigned *list_size,
 static void try_to_free_from_threads(size_t size)
 {
read_lock();
-   release_pack_memory(size, -1);
+   release_pack_memory(size);
read_unlock();
 }
 
diff --git a/git-compat-util.h b/git-compat-util.h
index cc4ba4d..29b1ee3 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -517,7 +517,7 @@ int inet_pton(int af, const char *src, void *dst);
 const char *inet_ntop(int af, const void *src, char *dst, size_t size);
 #endif
 
-extern void release_pack_memory(size_t, int);
+extern void release_pack_memory(size_t);
 
 typedef void (*try_to_free_t)(size_t);
 extern try_to_free_t set_try_to_free_routine(try_to_free_t);
diff --git a/sha1_file.c b/sha1_file.c
index 7731ab1..d26121a 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -614,7 +614,7 @@ static void scan_windows(struct packed_git *p,
}
 }
 
-static int unuse_one_window(struct packed_git *current, int keep_fd)
+static int unuse_one_window(struct packed_git *current)
 {
struct packed_git *p, *lru_p = NULL;
struct pack_window *lru_w = NULL, *lru_l = NULL;
@@ -628,15 +628,8 @@ static int unuse_one_window(struct packed_git *current, 
int keep_fd)
pack_mapped -= lru_w->len;
if (lru_l)
lru_l->next = lru_w->next;
-   else {
+   else
lru_p->windows = lru_w->next;
-   if (!lru_p->windows && lru_p->pack_fd != -1
-   && lru_p->pack_fd != keep_fd) {
-   close(lru_p->pack_fd);
-   pack_open_fds--;
-   lru_p->pack_fd = -1;
-   }
-   }
free(lru_w);
pack_open_windows--;
return 1;
@@ -644,10 +637,10 @@ static int unuse_one_window(struct packed_git *current, 
int keep_fd)
return 0;
 }
 
-void release_pack_memory(size_t need, int fd)
+void release_pack_memory(size_t need)
 {
size_t cur = pack_mapped;
-   while (need >= (cur - pack_mapped) && unuse_one_window(NULL, fd))
+   while (need >= (cur - pack_mapped) && unuse_one_window(NULL))
; /* nothing */
 }
 
@@ -658,7 +651,7 @@ void *xmmap(void *start, size_t length,
if (ret == MAP_FAILED) {
if (!length)
return NULL;
-   release_pack_memory(length, fd);
+   release_pack_memory(length);
ret = mmap(start, length, prot, flags, fd, offset);
if (ret == MAP_FAILED)
die_errno("Out of memory? mmap failed");
@@ -954,7 +947,7 @@ unsigned char *use_pack(struct packed_git *p,
win->len = (size_t)len;
pack_mapped += win->len;
while (packed_git_limit < pack_mapped
-   && unuse_one_window(p, p->pack_fd))
+   && unuse_one_window(p))
; /* nothing */
win->base = xmmap(NULL, win->len,
PROT_READ, MAP_PRIVATE,
@@ -1000,7 +993,7 @@ static struct packed_git *alloc_packed_git(int extra)
 
 static void try_to_free_pack_memory(size_t size)
 {
-   release_pack_memory(size, -1);
+   release_pack_memory(size);
 }
 
 struct packed_git *add_packed_git(const char *path, int path_len, int local)
-- 
1.8.4.rc0.2.g6cf5c31


---
This email message is for the sole use of the intended recipient(s) a

Re: [PATCH 0/3] "git config --get-urlmatch $section.$key $url"

2013-07-31 Thread Junio C Hamano
Ramsay Jones  writes:

> Junio C Hamano wrote:
>> So here is a bit of refactoring to extract the logic to find the
>> entry $section..$key from the configuration that best
>> matches the given $url to answer "what value $section.$key should be
>> for $url?" out of http.c (primarily because we never want to link
>> "git cofnig" with the cURL library), and use it from "git config" to
>> implement Peff's idea to extend "git config".
>> 
>> The first step is a pure code movement, plus some renaming of the
>> functions.
>> 
>> The second step is to factor out the code to handle --bool, --int, etc.
>> as a helper so that the new codepath can use it.
>> 
>> The last step currently duplicates the logic in http_options(), but
>> we might want to refactor it further so that the two functions can
>> share more code.  We hopefully can get rid of test-url-normalize and
>> instead use "git config --get-urlmatch" in the tests that protect
>> the http..config topic.
>
> I haven't been following this topic too closely and I don't have any
> feel for how long it will take to get to the end-game. However, unless
> the removal of test-url-normalize is coming soon, could I request that
> you apply my patch (or squash it into this series)? At present, I have
> to apply the patch before building the next and pu branches; OK it's not
> too onerous, but still ... :-P

Will squash in.  Thanks for a reminder.
--
To unsubscribe from this list: send the line "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 v6 0/6] http.. and friends

2013-07-31 Thread Junio C Hamano
This is my attempt to reroll Kyle's http.. series.

It adds a general .. support at the
infrastructure level and then rebuild http.. support on
top of it.  A useful side effect of doing it this way is that it
avoids having to touch the two-name parser http_options() at all.

The same infrastructure is used to add "--get-urlmatch" mode to "git
config", so that scripted Porcelains can use the same mechanism to
ask for the value for . variable with a URL, and learn
the value for .. whose  part best matches
the given URL.  In a sense, the infrastructure makes .
a "virtual" variable that is customized for URL.

 * Patch 1/6 is unchanged.

 * Patch 2/6 is to add only the two helpers url_normalize and
   match_urls from the original series by Kyle.

 * Patch 3/6 is the general .. support.  The
   urlmatch_config_entry() wrapper can use existing two-name parser
   to implement "virtual" . variables.

 * Patch 4/6 is the rest of Kyle's http.. ported on top of
   the infrastructure.

 * Patch 5/6 is unchanged from the previous round.

 * Patch 6/6 teaches "--get-urlmatch" to "git config"; this time it
   adds tests and docs.

Junio C Hamano (4):
  http.c: fix parsing of http.sslCertPasswordProtected variable
  config: add generic callback shim to parse section..key
  builtin/config: refactor collect_config()
  config: "git config --get-urlmatch" parses section..key

Kyle J. McKay (2):
  config: add helper to normalize and match URLs
  config: parse http.. using urlmatch

 .gitignore   |   1 +
 Documentation/config.txt |  44 
 Documentation/git-config.txt |  29 +++
 Makefile |   7 +
 builtin/config.c | 134 +--
 http.c   |  16 +-
 t/.gitattributes |   1 +
 t/t1300-repo-config.sh   |  25 ++
 t/t5200-url-normalize.sh | 199 
 t/t5200/README   | Bin 0 -> 644 bytes
 t/t5200/config-1 | Bin 0 -> 180 bytes
 t/t5200/config-2 | Bin 0 -> 80 bytes
 t/t5200/config-3 | Bin 0 -> 118 bytes
 t/t5200/url-1| Bin 0 -> 20 bytes
 t/t5200/url-10   | Bin 0 -> 23 bytes
 t/t5200/url-11   | Bin 0 -> 25 bytes
 t/t5200/url-2| Bin 0 -> 20 bytes
 t/t5200/url-3| Bin 0 -> 23 bytes
 t/t5200/url-4| Bin 0 -> 23 bytes
 t/t5200/url-5| Bin 0 -> 23 bytes
 t/t5200/url-6| Bin 0 -> 23 bytes
 t/t5200/url-7| Bin 0 -> 23 bytes
 t/t5200/url-8| Bin 0 -> 23 bytes
 t/t5200/url-9| Bin 0 -> 23 bytes
 test-url-normalize.c | 137 +++
 urlmatch.c   | 535 +++
 urlmatch.h   |  54 +
 27 files changed, 1158 insertions(+), 24 deletions(-)
 create mode 100755 t/t5200-url-normalize.sh
 create mode 100644 t/t5200/README
 create mode 100644 t/t5200/config-1
 create mode 100644 t/t5200/config-2
 create mode 100644 t/t5200/config-3
 create mode 100644 t/t5200/url-1
 create mode 100644 t/t5200/url-10
 create mode 100644 t/t5200/url-11
 create mode 100644 t/t5200/url-2
 create mode 100644 t/t5200/url-3
 create mode 100644 t/t5200/url-4
 create mode 100644 t/t5200/url-5
 create mode 100644 t/t5200/url-6
 create mode 100644 t/t5200/url-7
 create mode 100644 t/t5200/url-8
 create mode 100644 t/t5200/url-9
 create mode 100644 test-url-normalize.c
 create mode 100644 urlmatch.c
 create mode 100644 urlmatch.h

-- 
1.8.4-rc0-153-g9820077

--
To unsubscribe from this list: send the line "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 v6 6/6] config: "git config --get-urlmatch" parses section..key

2013-07-31 Thread Junio C Hamano
Using the same urlmatch_config_entry() infrastructure, add a new
mode "--get-urlmatch" to the "git config" command, to learn values
for the "virtual" two-level variables customized for the specific
URL.

git config [--] --get-urlmatch [.] 

With . fully specified, the configuration data for
.. for  that best matches the
given  is sought (and if not found, . is used)
and reported.  For example, with this configuration:

[http]
sslVerify
[http "https://weak.example.com";]
cookieFile = /tmp/cookie.txt
sslVerify = false

You would get

$ git config --bool --get-urlmatch http.sslVerify https://good.example.com
true
$ git config --bool --get-urlmatch http.sslVerify https://weak.example.com
false

With only  specified, you can get a list of all variables
in the section with their values that apply to the given URL.  E.g

$ git config --get-urlmatch http https://weak.example.com
http.cookiefile /tmp/cookie.txt
http.sslverify false

Signed-off-by: Junio C Hamano 
---
 Documentation/git-config.txt | 29 ++
 builtin/config.c | 92 
 t/t1300-repo-config.sh   | 25 
 3 files changed, 146 insertions(+)

diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
index d88a6fc..b48e2ec 100644
--- a/Documentation/git-config.txt
+++ b/Documentation/git-config.txt
@@ -15,6 +15,7 @@ SYNOPSIS
 'git config' [] [type] [-z|--null] --get name [value_regex]
 'git config' [] [type] [-z|--null] --get-all name [value_regex]
 'git config' [] [type] [-z|--null] --get-regexp name_regex 
[value_regex]
+'git config' [] [type] [-z|--null] --get-urlmatch name URL
 'git config' [] --unset name [value_regex]
 'git config' [] --unset-all name [value_regex]
 'git config' [] --rename-section old_name new_name
@@ -95,6 +96,14 @@ OPTIONS
in which section and variable names are lowercased, but subsection
names are not.
 
+--get-urlmatch name URL::
+   When given a two-part name section.key, the value for
+   section..key whose  part matches the best to the
+   given URL is returned (if no such key exists, the value for
+   section.key is used as a fallback).  When given just the
+   section as name, do so for all the keys in the section and
+   list them.
+
 --global::
For writing options: write to global ~/.gitconfig file rather than
the repository .git/config, write to $XDG_CONFIG_HOME/git/config file
@@ -273,6 +282,13 @@ Given a .git/config like this:
gitproxy=proxy-command for kernel.org
gitproxy=default-proxy ; for all the rest
 
+   ; HTTP
+   [http]
+   sslVerify
+   [http "https://weak.example.com";]
+   sslVerify = false
+   cookieFile = /tmp/cookie.txt
+
 you can set the filemode to true with
 
 
@@ -358,6 +374,19 @@ RESET=$(git config --get-color "" "reset")
 echo "${WS}your whitespace color or blue reverse${RESET}"
 
 
+For URLs in `https://weak.example.com`, `http.sslVerify` is set to
+false, while it is set to `true` for all others:
+
+
+% git config --bool --get-urlmatch http.sslverify https://good.example.com
+true
+% git config --bool --get-urlmatch http.sslverify https://weak.example.com
+false
+% git config --get-urlmatch http https://weak.example.com
+http.cookiefile /tmp/cookie.txt
+http.sslverify false
+
+
 include::config.txt[]
 
 GIT
diff --git a/builtin/config.c b/builtin/config.c
index 12c5073..c35c5be 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -2,6 +2,7 @@
 #include "cache.h"
 #include "color.h"
 #include "parse-options.h"
+#include "urlmatch.h"
 
 static const char *const builtin_config_usage[] = {
N_("git config [options]"),
@@ -41,6 +42,7 @@ static int respect_includes = -1;
 #define ACTION_SET_ALL (1<<12)
 #define ACTION_GET_COLOR (1<<13)
 #define ACTION_GET_COLORBOOL (1<<14)
+#define ACTION_GET_URLMATCH (1<<15)
 
 #define TYPE_BOOL (1<<0)
 #define TYPE_INT (1<<1)
@@ -57,6 +59,7 @@ static struct option builtin_config_options[] = {
OPT_BIT(0, "get", &actions, N_("get value: name [value-regex]"), 
ACTION_GET),
OPT_BIT(0, "get-all", &actions, N_("get all values: key 
[value-regex]"), ACTION_GET_ALL),
OPT_BIT(0, "get-regexp", &actions, N_("get values for regexp: 
name-regex [value-regex]"), ACTION_GET_REGEXP),
+   OPT_BIT(0, "get-urlmatch", &actions, N_("get value specific for the 
URL: section[.var] URL"), ACTION_GET_URLMATCH),
OPT_BIT(0, "replace-all", &actions, N_("replace all matching variables: 
name value [value_regex]"), ACTION_REPLACE_ALL),
OPT_BIT(0, "add", &actions, N_("add a new variable: name value"), 
ACTION_ADD),
OPT_BIT(0, "unset", &actions, N_("remove a variable: name 
[value-regex]"), ACTION_UNSET),
@@ -348,6 +351,91 @@ static int get_colorbool(int print)
return get_colorbool_found 

[PATCH v6 1/6] http.c: fix parsing of http.sslCertPasswordProtected variable

2013-07-31 Thread Junio C Hamano
The existing code triggers only when the configuration variable is
set to true.  Once the variable is set to true in a more generic
configuration file (e.g. ~/.gitconfig), it cannot be overriden to
false in the repository specific one (e.g. .git/config).

Signed-off-by: Junio C Hamano 
---
 http.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/http.c b/http.c
index 92aba59..37986f8 100644
--- a/http.c
+++ b/http.c
@@ -160,8 +160,7 @@ static int http_options(const char *var, const char *value, 
void *cb)
if (!strcmp("http.sslcainfo", var))
return git_config_string(&ssl_cainfo, var, value);
if (!strcmp("http.sslcertpasswordprotected", var)) {
-   if (git_config_bool(var, value))
-   ssl_cert_password_required = 1;
+   ssl_cert_password_required = git_config_bool(var, value);
return 0;
}
if (!strcmp("http.ssltry", var)) {
-- 
1.8.4-rc0-153-g9820077

--
To unsubscribe from this list: send the line "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 v6 3/6] config: add generic callback wrapper to parse section..key

2013-07-31 Thread Junio C Hamano
Existing configuration parsing functions (e.g. http_options() in
http.c) know how to parse two-level configuration variable names.
We would like to exploit them and parse something like this:

[http]
sslVerify = true
[http "https://weak.example.com";]
sslVerify = false

and pretend as if http.sslVerify were set to false when talking to
"https://weak.example.com/path";.

Introduce `urlmatch_config_entry()` wrapper that:

 - is called with the target URL (e.g. "https://weak.example.com/path";),
   and the two-level variable parser (e.g. `http_options`);

 - uses `url_normalize()` and `match_urls()` to see if configuration
   data matches the target URL; and

 - calls the traditional two-level configuration variable parser
   only for the configuration data whose  part matches the
   target URL (and if there are multiple matches, only do so if the
   current match is a better match than the ones previously seen).

Signed-off-by: Junio C Hamano 
---
 urlmatch.c | 67 ++
 urlmatch.h | 18 +
 2 files changed, 85 insertions(+)

diff --git a/urlmatch.c b/urlmatch.c
index e1b03ee..073fdd3 100644
--- a/urlmatch.c
+++ b/urlmatch.c
@@ -466,3 +466,70 @@ int match_urls(const struct url_info *url,
*exactusermatch = usermatched;
return pathmatchlen;
 }
+
+int urlmatch_config_entry(const char *var, const char *value, void *cb)
+{
+   struct string_list_item *item;
+   struct urlmatch_config *collect = cb;
+   struct urlmatch_item *matched;
+   struct url_info *url = &collect->url;
+   const char *key, *dot;
+   struct strbuf synthkey = STRBUF_INIT;
+   size_t matched_len = 0;
+   int user_matched = 0;
+   int retval;
+
+   key = skip_prefix(var, collect->section);
+   if (!key || *(key++) != '.') {
+   if (collect->cascade_fn)
+   return collect->cascade_fn(var, value, cb);
+   return 0; /* not interested */
+   }
+   dot = strrchr(key, '.');
+   if (dot) {
+   char *config_url, *norm_url;
+   struct url_info norm_info;
+
+   config_url = xmemdupz(key, dot - key);
+   norm_url = url_normalize(config_url, &norm_info);
+   free(config_url);
+   if (!norm_url)
+   return 0;
+   matched_len = match_urls(url, &norm_info, &user_matched);
+   free(norm_url);
+   if (!matched_len)
+   return 0;
+   key = dot + 1;
+   }
+
+   if (collect->key && strcmp(key, collect->key))
+   return 0;
+
+   item = string_list_insert(&collect->vars, key);
+   if (!item->util) {
+   matched = xcalloc(1, sizeof(*matched));
+   item->util = matched;
+   } else {
+   matched = item->util;
+   /*
+* Is our match shorter?  Is our match the same
+* length, and without user while the current
+* candidate is with user?  Then we cannot use it.
+*/
+   if (matched_len < matched->matched_len ||
+   ((matched_len == matched->matched_len) &&
+(!user_matched && matched->user_matched)))
+   return 0;
+   /* Otherwise, replace it with this one. */
+   }
+
+   matched->matched_len = matched_len;
+   matched->user_matched = user_matched;
+   strbuf_addstr(&synthkey, collect->section);
+   strbuf_addch(&synthkey, '.');
+   strbuf_addstr(&synthkey, key);
+   retval = collect->collect_fn(synthkey.buf, value, collect->cb);
+
+   strbuf_release(&synthkey);
+   return retval;
+}
diff --git a/urlmatch.h b/urlmatch.h
index b67f57f..b461dfd 100644
--- a/urlmatch.h
+++ b/urlmatch.h
@@ -33,4 +33,22 @@ struct url_info {
 extern char *url_normalize(const char *, struct url_info *);
 extern int match_urls(const struct url_info *url, const struct url_info 
*url_prefix, int *exactusermatch);
 
+struct urlmatch_item {
+   size_t matched_len;
+   char user_matched;
+};
+
+struct urlmatch_config {
+   struct string_list vars;
+   struct url_info url;
+   const char *section;
+   const char *key;
+
+   void *cb;
+   int (*collect_fn)(const char *var, const char *value, void *cb);
+   int (*cascade_fn)(const char *var, const char *value, void *cb);
+};
+
+extern int urlmatch_config_entry(const char *var, const char *value, void *cb);
+
 #endif /* URL_MATCH_H */
-- 
1.8.4-rc0-153-g9820077

--
To unsubscribe from this list: send the line "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 v6 4/6] config: parse http.. using urlmatch

2013-07-31 Thread Junio C Hamano
From: "Kyle J. McKay" 

Use the urlmatch_config_entry() to wrap the underlying
http_options() two-level variable parser in order to set
http. to the value with the most specific URL in the
configuration.

Signed-off-by: Kyle J. McKay 
Signed-off-by: Junio C Hamano 
---
 .gitignore   |   1 +
 Documentation/config.txt |  44 +++
 Makefile |   7 ++
 http.c   |  13 +++-
 t/.gitattributes |   1 +
 t/t5200-url-normalize.sh | 199 +++
 t/t5200/README   | Bin 0 -> 644 bytes
 t/t5200/config-1 | Bin 0 -> 180 bytes
 t/t5200/config-2 | Bin 0 -> 80 bytes
 t/t5200/config-3 | Bin 0 -> 118 bytes
 t/t5200/url-1| Bin 0 -> 20 bytes
 t/t5200/url-10   | Bin 0 -> 23 bytes
 t/t5200/url-11   | Bin 0 -> 25 bytes
 t/t5200/url-2| Bin 0 -> 20 bytes
 t/t5200/url-3| Bin 0 -> 23 bytes
 t/t5200/url-4| Bin 0 -> 23 bytes
 t/t5200/url-5| Bin 0 -> 23 bytes
 t/t5200/url-6| Bin 0 -> 23 bytes
 t/t5200/url-7| Bin 0 -> 23 bytes
 t/t5200/url-8| Bin 0 -> 23 bytes
 t/t5200/url-9| Bin 0 -> 23 bytes
 test-url-normalize.c | 137 
 22 files changed, 401 insertions(+), 1 deletion(-)
 create mode 100755 t/t5200-url-normalize.sh
 create mode 100644 t/t5200/README
 create mode 100644 t/t5200/config-1
 create mode 100644 t/t5200/config-2
 create mode 100644 t/t5200/config-3
 create mode 100644 t/t5200/url-1
 create mode 100644 t/t5200/url-10
 create mode 100644 t/t5200/url-11
 create mode 100644 t/t5200/url-2
 create mode 100644 t/t5200/url-3
 create mode 100644 t/t5200/url-4
 create mode 100644 t/t5200/url-5
 create mode 100644 t/t5200/url-6
 create mode 100644 t/t5200/url-7
 create mode 100644 t/t5200/url-8
 create mode 100644 t/t5200/url-9
 create mode 100644 test-url-normalize.c

diff --git a/.gitignore b/.gitignore
index 6669bf0..cd97e16 100644
--- a/.gitignore
+++ b/.gitignore
@@ -198,6 +198,7 @@
 /test-string-list
 /test-subprocess
 /test-svn-fe
+/test-url-normalize
 /test-wildmatch
 /common-cmds.h
 *.tar.gz
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 6e53fc5..60c140f 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1513,6 +1513,50 @@ http.useragent::
of common USER_AGENT strings (but not including those like git/1.7.1).
Can be overridden by the 'GIT_HTTP_USER_AGENT' environment variable.
 
+http..*::
+   Any of the http.* options above can be applied selectively to some urls.
+   For a config key to match a URL, each element of the config key is
+   compared to that of the URL, in the following order:
++
+--
+. Scheme (e.g., `https` in `https://example.com/`). This field
+  must match exactly between the config key and the URL.
+
+. Host/domain name (e.g., `example.com` in `https://example.com/`).
+  This field must match exactly between the config key and the URL.
+
+. Port number (e.g., `8080` in `http://example.com:8080/`).
+  This field must match exactly between the config key and the URL.
+  Omitted port numbers are automatically converted to the correct
+  default for the scheme before matching.
+
+. Path (e.g., `repo.git` in `https://example.com/repo.git`). The
+  path field of the config key must match the path field of the URL
+  either exactly or as a prefix of slash-delimited path elements.  This means
+  a config key with path `foo/` matches URL path `foo/bar`.  A prefix can only
+  match on a slash (`/`) boundary.  Longer matches take precedence (so a config
+  key with path `foo/bar` is a better match to URL path `foo/bar` than a config
+  key with just path `foo/`).
+
+. User name (e.g., `user` in `https://u...@example.com/repo.git`). If
+  the config key has a user name it must match the user name in the
+  URL exactly. If the config key does not have a user name, that
+  config key will match a URL with any user name (including none).
+--
++
+The list above is ordered by decreasing precedence; a URL that matches
+a config key's path is preferred to one that matches its user name. For 
example,
+if the URL is `https://u...@example.com/foo/bar` a config key match of
+`https://example.com/foo` will be preferred over a config key match of
+`https://u...@example.com`.
++
+All URLs are normalized before attempting any matching (the password part,
+if embedded in the URL, is always ignored for matching purposes) so that
+equivalent urls that are simply spelled differently will match properly.
+Environment variable settings always override any matches.  The urls that are
+matched against are those given directly to Git commands.  This means any URLs
+visited as a result of a redirection do not participate in matching.
+
 i18n.commitEncoding::
Character encoding the commit messages are stored in; Git itself
does not care per se, but this information is necessar

[PATCH v6 5/6] builtin/config: refactor collect_config()

2013-07-31 Thread Junio C Hamano
In order to reuse the logic to format the configuration value while
honouring the requested type, split this function into two.

Signed-off-by: Junio C Hamano 
---
 builtin/config.c | 42 +-
 1 file changed, 21 insertions(+), 21 deletions(-)

diff --git a/builtin/config.c b/builtin/config.c
index 33c9bf9..12c5073 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -100,25 +100,13 @@ struct strbuf_list {
int alloc;
 };
 
-static int collect_config(const char *key_, const char *value_, void *cb)
+static int format_config(struct strbuf *buf, const char *key_, const char 
*value_)
 {
-   struct strbuf_list *values = cb;
-   struct strbuf *buf;
-   char value[256];
-   const char *vptr = value;
int must_free_vptr = 0;
int must_print_delim = 0;
+   char value[256];
+   const char *vptr = value;
 
-   if (!use_key_regexp && strcmp(key_, key))
-   return 0;
-   if (use_key_regexp && regexec(key_regexp, key_, 0, NULL, 0))
-   return 0;
-   if (regexp != NULL &&
-   (do_not_match ^ !!regexec(regexp, (value_?value_:""), 0, NULL, 0)))
-   return 0;
-
-   ALLOC_GROW(values->items, values->nr + 1, values->alloc);
-   buf = &values->items[values->nr++];
strbuf_init(buf, 0);
 
if (show_keys) {
@@ -126,7 +114,7 @@ static int collect_config(const char *key_, const char 
*value_, void *cb)
must_print_delim = 1;
}
if (types == TYPE_INT)
-   sprintf(value, "%d", git_config_int(key_, value_?value_:""));
+   sprintf(value, "%d", git_config_int(key_, value_ ? value_ : 
""));
else if (types == TYPE_BOOL)
vptr = git_config_bool(key_, value_) ? "true" : "false";
else if (types == TYPE_BOOL_OR_INT) {
@@ -154,15 +142,27 @@ static int collect_config(const char *key_, const char 
*value_, void *cb)
strbuf_addch(buf, term);
 
if (must_free_vptr)
-   /* If vptr must be freed, it's a pointer to a
-* dynamically allocated buffer, it's safe to cast to
-* const.
-   */
free((char *)vptr);
-
return 0;
 }
 
+static int collect_config(const char *key_, const char *value_, void *cb)
+{
+   struct strbuf_list *values = cb;
+
+   if (!use_key_regexp && strcmp(key_, key))
+   return 0;
+   if (use_key_regexp && regexec(key_regexp, key_, 0, NULL, 0))
+   return 0;
+   if (regexp != NULL &&
+   (do_not_match ^ !!regexec(regexp, (value_?value_:""), 0, NULL, 0)))
+   return 0;
+
+   ALLOC_GROW(values->items, values->nr + 1, values->alloc);
+
+   return format_config(&values->items[values->nr++], key_, value_);
+}
+
 static int get_value(const char *key_, const char *regex_)
 {
int ret = CONFIG_GENERIC_ERROR;
-- 
1.8.4-rc0-153-g9820077

--
To unsubscribe from this list: send the line "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 v6 2/6] config: add helper to normalize and match URLs

2013-07-31 Thread Junio C Hamano
From: "Kyle J. McKay" 

Some http.* configuration variables need to take values customized
for the URL we are talking to.  We may want to set http.sslVerify to
true in general but to false only for a certain site, for example,
with a configuration file like this:

[http]
sslVerify = true
[http "https://weak.example.com";]
sslVerify = false

and let the configuration machinery pick up the latter only when
talking to "https://weak.example.com";.  The latter needs to kick in
not only when the URL is exactly "https://weak.example.com";, but
also is anything that "match" it, e.g.

https://weak.example.com/test
https://m...@weak.example.com/test

The  in the configuration key consists of the following parts,
and is considered a match to the URL we are attempting to access
under certain conditions:

  . Scheme (e.g., `https` in `https://example.com/`). This field
must match exactly between the config key and the URL.

  . Host/domain name (e.g., `example.com` in `https://example.com/`).
This field must match exactly between the config key and the URL.

  . Port number (e.g., `8080` in `http://example.com:8080/`).  This
field must match exactly between the config key and the URL.
Omitted port numbers are automatically converted to the correct
default for the scheme before matching.

  . Path (e.g., `repo.git` in `https://example.com/repo.git`). The
path field of the config key must match the path field of the
URL either exactly or as a prefix of slash-delimited path
elements.  A config key with path `foo/` matches URL path
`foo/bar`.  A prefix can only match on a slash (`/`) boundary.
Longer matches take precedence (so a config key with path
`foo/bar` is a better match to URL path `foo/bar` than a config
key with just path `foo/`).

  . User name (e.g., `me` in `https://m...@example.com/repo.git`). If
the config key has a user name, it must match the user name in
the URL exactly. If the config key does not have a user name,
that config key will match a URL with any user name (including
none).

Longer matches take precedence over shorter matches.

This step adds two helper functions `url_normalize()` and
`match_urls()` to help implement the above semantics. The
normalization rules are based on RFC 3986 and should result in any
two equivalent urls being a match.

Signed-off-by: Kyle J. McKay 
Signed-off-by: Junio C Hamano 
---
 urlmatch.c | 468 +
 urlmatch.h |  36 +
 2 files changed, 504 insertions(+)
 create mode 100644 urlmatch.c
 create mode 100644 urlmatch.h

diff --git a/urlmatch.c b/urlmatch.c
new file mode 100644
index 000..e1b03ee
--- /dev/null
+++ b/urlmatch.c
@@ -0,0 +1,468 @@
+#include "cache.h"
+#include "urlmatch.h"
+
+#define URL_ALPHA "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz"
+#define URL_DIGIT "0123456789"
+#define URL_ALPHADIGIT URL_ALPHA URL_DIGIT
+#define URL_SCHEME_CHARS URL_ALPHADIGIT "+.-"
+#define URL_HOST_CHARS URL_ALPHADIGIT ".-[:]" /* IPv6 literals need [:] */
+#define URL_UNSAFE_CHARS " <>\"%{}|\\^`" /* plus 0x00-0x1F,0x7F-0xFF */
+#define URL_GEN_RESERVED ":/?#[]@"
+#define URL_SUB_RESERVED "!$&'()*+,;="
+#define URL_RESERVED URL_GEN_RESERVED URL_SUB_RESERVED /* only allowed delims 
*/
+
+static int append_normalized_escapes(struct strbuf *buf,
+const char *from,
+size_t from_len,
+const char *esc_extra,
+const char *esc_ok)
+{
+   /*
+* Append to strbuf 'buf' characters from string 'from' with length
+* 'from_len' while unescaping characters that do not need to be escaped
+* and escaping characters that do.  The set of characters to escape
+* (the complement of which is unescaped) starts out as the RFC 3986
+* unsafe characters (0x00-0x1F,0x7F-0xFF," <>\"#%{}|\\^`").  If
+* 'esc_extra' is not NULL, those additional characters will also always
+* be escaped.  If 'esc_ok' is not NULL, those characters will be left
+* escaped if found that way, but will not be unescaped otherwise (used
+* for delimiters).  If a %-escape sequence is encountered that is not
+* followed by 2 hexadecimal digits, the sequence is invalid and
+* false (0) will be returned.  Otherwise true (1) will be returned for
+* success.
+*
+* Note that all %-escape sequences will be normalized to UPPERCASE
+* as indicated in RFC 3986.  Unless included in esc_extra or esc_ok
+* alphanumerics and "-._~" will always be unescaped as per RFC 3986.
+*/
+
+   while (from_len) {
+   int ch = *from++;
+   int was_esc = 0;
+
+   from_len--;
+   if (ch == '%') {
+   if (from_len < 2

Re: What's cooking in git.git (Jul 2013, #09; Mon, 29)

2013-07-31 Thread Junio C Hamano
Ramsay Jones  writes:

>>  I am personally in favor of this simpler solution.  Comments?
>
> I had expected this to me marked for 'master'.
>
> Has this simply been overlooked, or do you have reservations about
> applying this patch?

I am just being careful and do want to keep it cooking in 'next'
during the feature freeze.  The more users work with 'next' (not
"work *on* 'next'"), the more confidence we would be with, and
hopefully this can be one of the topis that graduate early after
the 1.8.4 release.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 0/3] "git config --get-urlmatch $section.$key $url"

2013-07-31 Thread Ramsay Jones
Junio C Hamano wrote:
> So here is a bit of refactoring to extract the logic to find the
> entry $section..$key from the configuration that best
> matches the given $url to answer "what value $section.$key should be
> for $url?" out of http.c (primarily because we never want to link
> "git cofnig" with the cURL library), and use it from "git config" to
> implement Peff's idea to extend "git config".
> 
> The first step is a pure code movement, plus some renaming of the
> functions.
> 
> The second step is to factor out the code to handle --bool, --int, etc.
> as a helper so that the new codepath can use it.
> 
> The last step currently duplicates the logic in http_options(), but
> we might want to refactor it further so that the two functions can
> share more code.  We hopefully can get rid of test-url-normalize and
> instead use "git config --get-urlmatch" in the tests that protect
> the http..config topic.

I haven't been following this topic too closely and I don't have any
feel for how long it will take to get to the end-game. However, unless
the removal of test-url-normalize is coming soon, could I request that
you apply my patch (or squash it into this series)? At present, I have
to apply the patch before building the next and pu branches; OK it's not
too onerous, but still ... :-P


ATB,
Ramsay Jones


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: What's cooking in git.git (Jul 2013, #09; Mon, 29)

2013-07-31 Thread Ramsay Jones
Junio C Hamano wrote:
> Here are the topics that have been cooking.  Commits prefixed with
> '-' are only in 'pu' (proposed updates) while commits prefixed with
> '+' are in 'next'.
> 
> The shape of the upcoming release is pretty much known by now; all
> the topics that are marked for 'master' in this issue will likely to
> be in the final, and others will cook during the pre-release freeze
> period.
> 
[ ... ]
> 
> * rj/cygwin-clarify-use-of-cheating-lstat (2013-07-18) 1 commit
>  - cygwin: Remove the Win32 l/stat() implementation
> 
>  Cygwin port added a "not quite correct but a lot faster and good
>  enough for many lstat() calls that are only used to see if the
>  working tree entity matches the index entry" lstat() emulation some
>  time ago, and it started biting us in places.  This removes it and
>  uses the standard lstat() that comes with Cygwin.
> 
>  I am personally in favor of this simpler solution.  Comments?

I had expected this to me marked for 'master'.

Has this simply been overlooked, or do you have reservations about
applying this patch? If so, then I need to find another solution
very quickly [1] (before v1.8.4).  At this time, despite passing the
test suite [2], the cygwin build is still very much broken.

ATB,
Ramsay Jones

[1] I do have another patch, patch #0 actually, which I said I didn't
want applied! :-P

[2] I ran the test suite on v1.8.4-rc0 + 1 patch, like so:

$ GIT_SKIP_TESTS='t0061.3' make test >test-outp16 2>&1

$ tail -17 test-outp16
[22:33:53] t9902-completion.sh  ok13650 ms
[22:34:26] t9903-bash-prompt.sh ... ok33468 ms
[22:34:26]
All tests successful.

Test Summary Report
---
t7008-grep-binary.sh (Wstat: 0 Tests: 23 Failed: 0)
  TODO passed:   12
Files=639, Tests=10291, 9581 wallclock secs ( 2.75 usr  1.41 sys + 9421.84 cusr
4551.31 csys = 13977.31 CPU)
Result: PASS
make clean-except-prove-cache
make[2]: Entering directory `/home/ramsay/git/t'
rm -f -r 'trash directory'.* 'test-results'
rm -f -r valgrind/bin
make[2]: Leaving directory `/home/ramsay/git/t'
make[1]: Leaving directory `/home/ramsay/git/t'
$

This was over 30 minutes faster than the last full test suite run, but it also
ran more tests (420 test files ran faster and 16 new test files were run).
I've *never* had the test suite run faster before! (I'm not going to celebrate
too much; it still took  9581s = 2h39m41s).


--
To unsubscribe from this list: send the line "unsubscribe 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] howto: Eliminate all tabs

2013-07-31 Thread Dirk Wallenstein
Junio C Hamano  wrote (Wed, Jul 31, 2013 at 10:03:15AM 
-0700):
> Dirk Wallenstein  writes:
> 
> > diff --git a/Documentation/howto/rebase-from-internal-branch.txt 
> > b/Documentation/howto/rebase-from-internal-branch.txt
> > index 19ab604..aefe5b1 100644
> > --- a/Documentation/howto/rebase-from-internal-branch.txt
> > +++ b/Documentation/howto/rebase-from-internal-branch.txt
> > @@ -1,8 +1,8 @@
> > -From:  Junio C Hamano 
> > -To:git@vger.kernel.org
> > -Cc:Petr Baudis , Linus Torvalds 
> > +From:Junio C Hamano 
> 
> Why does this patch have to break the e-mail headers like this?
> 
> These are copies of old e-mails; keep them as close to the original
> as they were.
> 
> Besides, the tab width of our source is 8, period.  Get over it.
> 

I will try.  It just doesn't make sense to me at all.

-- 
  Dirk
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCHv2 8/9] checkout-index: Fix negations of even numbers of -n

2013-07-31 Thread Eric Sunshine
On Wed, Jul 31, 2013 at 12:28 PM, Stefan Beller
 wrote:
> The --no-create was parsed with OPT_BOOLEAN, which has a counting up
> logic implemented. Since b04ba2bb (parse-options: deprecate OPT_BOOLEAN,
> 2011-09-27) the OPT_BOOLEAN is deprecated and is only a define:
> /* Deprecated synonym */
> #define OPTION_BOOLEAN OPTION_COUNTUP
>
> However the variable not_new, which can be counted up by giving multiple
> --no-create multiple times, is used to set a bit in the struct checkout

s/multiple --no-create multiple times/--no-create multiple times/

> bitfield (defined in cache.h:969, declared at builtin/checkout-index.c:19):
>
> state.not_new = not_new;
>
> When assigning a value other than 0 or 1 to a bit, all leading digits but
> the last are ignored and only the last bit is used for setting the bit
> variable.
>
> Hence the following:
> # in git.git:
> $ git status
> # working directory clean
> rm COPYING
> $ git status
> # deleted:COPYING
> $ git checkout-index -a -n
> # deleted:COPYING
> # which is expected as we're telling git to not restore or create
> # files, however:
> $ git checkout-index -a -n -n
> $ git status
> # working directory clean, COPYING is restored again!
> # That's the bug, we're fixing here.
>
> By restraining the variable not_new to a value being definitely 0 or 1
> by the macro OPT_BOOL the bug is fixed.
>
> Signed-off-by: Stefan Beller 
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCHv2 8/9] checkout-index: Fix negations of even numbers of -n

2013-07-31 Thread Eric Sunshine
On Wed, Jul 31, 2013 at 12:28 PM, Stefan Beller
 wrote:
> The --no-create was parsed with OPT_BOOLEAN, which has a counting up
> logic implemented. Since b04ba2bb (parse-options: deprecate OPT_BOOLEAN,
> 2011-09-27) the OPT_BOOLEAN is deprecated and is only a define:
> /* Deprecated synonym */
> #define OPTION_BOOLEAN OPTION_COUNTUP
>
> However the variable not_new, which can be counted up by giving multiple
> --no-create multiple times, is used to set a bit in the struct checkout
> bitfield (defined in cache.h:969, declared at builtin/checkout-index.c:19):
>
> state.not_new = not_new;
>
> When assigning a value other than 0 or 1 to a bit, all leading digits but
> the last are ignored and only the last bit is used for setting the bit
> variable.
>
> Hence the following:
> # in git.git:
> $ git status
> # working directory clean
> rm COPYING
> $ git status
> # deleted:COPYING
> $ git checkout-index -a -n

Missing "$ git status" at this point in example.

> # deleted:COPYING
> # which is expected as we're telling git to not restore or create
> # files, however:
> $ git checkout-index -a -n -n
> $ git status
> # working directory clean, COPYING is restored again!
> # That's the bug, we're fixing here.
>
> By restraining the variable not_new to a value being definitely 0 or 1
> by the macro OPT_BOOL the bug is fixed.
>
> Signed-off-by: Stefan Beller 
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCHv2 6/9] hash-object: Replace stdin parsing OPT_BOOLEAN by OPT_COUNTUP

2013-07-31 Thread Eric Sunshine
On Wed, Jul 31, 2013 at 12:28 PM, Stefan Beller
 wrote:
> This task emerged from b04ba2bb (parse-options: deprecate OPT_BOOLEAN,
> 2011-09-27). hash-object is a plumbing layer command, so better
> not change the input/output behavior for now.
>
> Unfortunately we have these lines relying on the count up mechanism of
> OPT_BOOLEAN:
>
> if (hashstdin > 1)
> errstr = "Multiple --stdin arguments are not supported";
>
> Maybe later, when the plumbing is refined (git 2.0?) we can drop that
> errormessage and replace the OPT_COUNTUP by OPT_BOOL.

s/errormessage/error message/

> Signed-off-by: Stefan Beller 
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCHv2 3/9] log, format-patch: parsing uses OPT__QUIET

2013-07-31 Thread Eric Sunshine
On Wed, Jul 31, 2013 at 12:28 PM, Stefan Beller
 wrote:
> This patch allows users to use the short form -q on
> log and format-patch, which was non possible before.

It would make sense for documentation updates to accompany these changes.

> Signed-off-by: Stefan Beller 
> ---
>  builtin/log.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/builtin/log.c b/builtin/log.c
> index 1dafbd0..ed4dec4 100644
> --- a/builtin/log.c
> +++ b/builtin/log.c
> @@ -121,7 +121,7 @@ static void cmd_log_init_finish(int argc, const char 
> **argv, const char *prefix,
> static struct line_opt_callback_data line_cb = {NULL, NULL, 
> STRING_LIST_INIT_DUP};
>
> const struct option builtin_log_options[] = {
> -   OPT_BOOL(0, "quiet", &quiet, N_("suppress diff output")),
> +   OPT__QUIET(&quiet, N_("suppress diff output")),
> OPT_BOOL(0, "source", &source, N_("show source")),
> OPT_BOOL(0, "use-mailmap", &mailmap, N_("Use mail map file")),
> { OPTION_CALLBACK, 0, "decorate", NULL, NULL, N_("decorate 
> options"),
> @@ -1210,8 +1210,7 @@ int cmd_format_patch(int argc, const char **argv, const 
> char *prefix)
> PARSE_OPT_OPTARG, thread_callback },
> OPT_STRING(0, "signature", &signature, N_("signature"),
> N_("add a signature")),
> -   OPT_BOOLEAN(0, "quiet", &quiet,
> -   N_("don't print the patch filenames")),
> +   OPT__QUIET(&quiet, N_("don't print the patch filenames")),
> OPT_END()
> };
>
> --
> 1.8.4.rc0.1.g8f6a3e5
>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe 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] howto: Eliminate all tabs

2013-07-31 Thread Junio C Hamano
Dirk Wallenstein  writes:

> diff --git a/Documentation/howto/rebase-from-internal-branch.txt 
> b/Documentation/howto/rebase-from-internal-branch.txt
> index 19ab604..aefe5b1 100644
> --- a/Documentation/howto/rebase-from-internal-branch.txt
> +++ b/Documentation/howto/rebase-from-internal-branch.txt
> @@ -1,8 +1,8 @@
> -From:Junio C Hamano 
> -To:  git@vger.kernel.org
> -Cc:  Petr Baudis , Linus Torvalds 
> +From:Junio C Hamano 

Why does this patch have to break the e-mail headers like this?

These are copies of old e-mails; keep them as close to the original
as they were.

Besides, the tab width of our source is 8, period.  Get over it.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 0/2] Remove tabs from howto documents

2013-07-31 Thread Fredrik Gustafsson
On Wed, Jul 31, 2013 at 06:54:07PM +0200, Dirk Wallenstein wrote:
> I really think that tabs are generally bad here.  So, this will
> remove all tabs from the howto folder and prevent indenting with tabs
> through gitattributes.
> 
> Dirk Wallenstein (2):
>   howto: Suppress indentation with tabs
>   howto: Eliminate all tabs
> 
>  Documentation/howto/.gitattributes |  1 +
>  .../howto/rebase-from-internal-branch.txt  |  8 ++--
>  Documentation/howto/rebuild-from-update-hook.txt   |  4 +-
>  .../howto/recover-corrupted-blob-object.txt| 50 
> +++---
>  Documentation/howto/revert-a-faulty-merge.txt  | 16 +++
>  Documentation/howto/revert-branch-rebase.txt   |  2 +-
>  Documentation/howto/update-hook-example.txt|  8 ++--
>  Documentation/howto/use-git-daemon.txt | 10 ++---
>  8 files changed, 50 insertions(+), 49 deletions(-)
>  create mode 100644 Documentation/howto/.gitattributes
> 

How about adding a line about this in Documentation/CodingGuidelines?

-- 
Med vänliga hälsningar
Fredrik Gustafsson

tel: 0733-608274
e-post: iv...@iveqy.com
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 1/2] howto: Suppress indentation with tabs

2013-07-31 Thread Dirk Wallenstein
The AsciiDoc files in the 'howto' folder are installed as documentation
and AsciiDoc files are meant to be read and printed as is.  To quote the
AsciiDoc Home Page:

AsciiDoc files are designed to be viewed, edited and printed
directly or translated to other presentation formats using the
asciidoc(1) command.

Tabs have the property of a configurable width and can thereby skew the
layout of a page and distort the meaning.  This is particularly a
problem when mixing lines with different indentation (space vs tab) in
code examples and ASCII art.

Prevent such a mix-up by prohibiting tab indentation entirely.

Signed-off-by: Dirk Wallenstein 
---
 Documentation/howto/.gitattributes | 1 +
 1 file changed, 1 insertion(+)
 create mode 100644 Documentation/howto/.gitattributes

diff --git a/Documentation/howto/.gitattributes 
b/Documentation/howto/.gitattributes
new file mode 100644
index 000..fecc113
--- /dev/null
+++ b/Documentation/howto/.gitattributes
@@ -0,0 +1 @@
+* whitespace=tab
-- 
1.8.3.3.2.g85103ba

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 2/2] howto: Eliminate all tabs

2013-07-31 Thread Dirk Wallenstein
Because tabs have a variable width, the layout can diverge from what the
author intended.  Replace all tabs with spaces to the next column that
is a multiple of 8.

This fixes several ascii art sketches and a code example where viewing
it with a tab-width other than 8 lead to wrong indentation.

Signed-off-by: Dirk Wallenstein 
---
 .../howto/rebase-from-internal-branch.txt  |  8 ++--
 Documentation/howto/rebuild-from-update-hook.txt   |  4 +-
 .../howto/recover-corrupted-blob-object.txt| 50 +++---
 Documentation/howto/revert-a-faulty-merge.txt  | 16 +++
 Documentation/howto/revert-branch-rebase.txt   |  2 +-
 Documentation/howto/update-hook-example.txt|  8 ++--
 Documentation/howto/use-git-daemon.txt | 10 ++---
 7 files changed, 49 insertions(+), 49 deletions(-)

diff --git a/Documentation/howto/rebase-from-internal-branch.txt 
b/Documentation/howto/rebase-from-internal-branch.txt
index 19ab604..aefe5b1 100644
--- a/Documentation/howto/rebase-from-internal-branch.txt
+++ b/Documentation/howto/rebase-from-internal-branch.txt
@@ -1,8 +1,8 @@
-From:  Junio C Hamano 
-To:git@vger.kernel.org
-Cc:Petr Baudis , Linus Torvalds 
+From:Junio C Hamano 
+To: git@vger.kernel.org
+Cc: Petr Baudis , Linus Torvalds 
 Subject: Re: sending changesets from the middle of a git tree
-Date:  Sun, 14 Aug 2005 18:37:39 -0700
+Date:   Sun, 14 Aug 2005 18:37:39 -0700
 Abstract: In this article, JC talks about how he rebases the
  public "pu" branch using the core Git tools when he updates
  the "master" branch, and how "rebase" works.  Also discussed
diff --git a/Documentation/howto/rebuild-from-update-hook.txt 
b/Documentation/howto/rebuild-from-update-hook.txt
index 25378f6..97365ff 100644
--- a/Documentation/howto/rebuild-from-update-hook.txt
+++ b/Documentation/howto/rebuild-from-update-hook.txt
@@ -19,8 +19,8 @@ when I took over Git maintainership from Linus.
 
 The directories relevant to this how-to are these two:
 
-/pub/scm/git/git.git/  The public Git repository.
-/pub/software/scm/git/docs/The HTML documentation page.
+/pub/scm/git/git.git/   The public Git repository.
+/pub/software/scm/git/docs/ The HTML documentation page.
 
 So I made a repository to generate the documentation under my
 home directory over there.
diff --git a/Documentation/howto/recover-corrupted-blob-object.txt 
b/Documentation/howto/recover-corrupted-blob-object.txt
index 1b3b188..6a8cc49 100644
--- a/Documentation/howto/recover-corrupted-blob-object.txt
+++ b/Documentation/howto/recover-corrupted-blob-object.txt
@@ -62,22 +62,22 @@ we now know which tree points to it!
 
 Now you can do
 
-   git ls-tree 2d9263c6d23595e7cb2a21e5ebbb53655278dff8
+git ls-tree 2d9263c6d23595e7cb2a21e5ebbb53655278dff8
 
 which will show something like
 
-   100644 blob 8d14531846b95bfa3564b58ccfb7913a034323b8.gitignore
-   100644 blob ebf9bf84da0aab5ed944264a5db2a65fe3a3e883.mailmap
-   100644 blob ca442d313d86dc67e0a2e5d584b465bd382cbf5cCOPYING
-   100644 blob ee909f2cc49e54f0799a4739d24c4cb9151ae453CREDITS
-   04 tree 0f5f709c17ad89e72bdbbef6ea221c69807009f6Documentation
-   100644 blob 1570d248ad9237e4fa6e4d079336b9da62d9ba32Kbuild
-   100644 blob 1c7c229a092665b11cd46a25dbd40feeb31661d9MAINTAINERS
-   ...
+100644 blob 8d14531846b95bfa3564b58ccfb7913a034323b8.gitignore
+100644 blob ebf9bf84da0aab5ed944264a5db2a65fe3a3e883.mailmap
+100644 blob ca442d313d86dc67e0a2e5d584b465bd382cbf5cCOPYING
+100644 blob ee909f2cc49e54f0799a4739d24c4cb9151ae453CREDITS
+04 tree 0f5f709c17ad89e72bdbbef6ea221c69807009f6Documentation
+100644 blob 1570d248ad9237e4fa6e4d079336b9da62d9ba32Kbuild
+100644 blob 1c7c229a092665b11cd46a25dbd40feeb31661d9MAINTAINERS
+...
 
 and you should now have a line that looks like
 
-   10064 blob 4b9458b3786228369c63936db65827de3cc06200 my-magic-file
+10064 blob 4b9458b3786228369c63936db65827de3cc06200 my-magic-file
 
 in the output. This already tells you a *lot* it tells you what file the
 corrupt blob came from!
@@ -87,7 +87,7 @@ Now, it doesn't tell you quite enough, though: it doesn't 
tell what
 lucky, and it may be the version that you already have checked out in your
 working tree, in which case fixing this problem is really simple, just do
 
-   git hash-object -w my-magic-file
+git hash-object -w my-magic-file
 
 again, and if it outputs the missing SHA-1 (4b945..) you're now all done!
 
@@ -96,26 +96,26 @@ version that was broken. How do you tell which version it 
was?
 
 The easiest way to do it is to do
 
-   git log --raw --all --full-history -- subdirectory/my-magic-file
+git log --raw --all --full-history -- subdirectory/my-magic-file
 
 and that will show you the whole log for that file (please realize that
 the tree y

[PATCH 0/2] Remove tabs from howto documents

2013-07-31 Thread Dirk Wallenstein
I really think that tabs are generally bad here.  So, this will
remove all tabs from the howto folder and prevent indenting with tabs
through gitattributes.

Dirk Wallenstein (2):
  howto: Suppress indentation with tabs
  howto: Eliminate all tabs

 Documentation/howto/.gitattributes |  1 +
 .../howto/rebase-from-internal-branch.txt  |  8 ++--
 Documentation/howto/rebuild-from-update-hook.txt   |  4 +-
 .../howto/recover-corrupted-blob-object.txt| 50 +++---
 Documentation/howto/revert-a-faulty-merge.txt  | 16 +++
 Documentation/howto/revert-branch-rebase.txt   |  2 +-
 Documentation/howto/update-hook-example.txt|  8 ++--
 Documentation/howto/use-git-daemon.txt | 10 ++---
 8 files changed, 50 insertions(+), 49 deletions(-)
 create mode 100644 Documentation/howto/.gitattributes

-- 
1.8.3.3.2.g85103ba

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCHv2 8/9] checkout-index: Fix negations of even numbers of -n

2013-07-31 Thread Stefan Beller
The --no-create was parsed with OPT_BOOLEAN, which has a counting up
logic implemented. Since b04ba2bb (parse-options: deprecate OPT_BOOLEAN,
2011-09-27) the OPT_BOOLEAN is deprecated and is only a define:
/* Deprecated synonym */
#define OPTION_BOOLEAN OPTION_COUNTUP

However the variable not_new, which can be counted up by giving multiple
--no-create multiple times, is used to set a bit in the struct checkout
bitfield (defined in cache.h:969, declared at builtin/checkout-index.c:19):

state.not_new = not_new;

When assigning a value other than 0 or 1 to a bit, all leading digits but
the last are ignored and only the last bit is used for setting the bit
variable.

Hence the following:
# in git.git:
$ git status
# working directory clean
rm COPYING
$ git status
# deleted:COPYING
$ git checkout-index -a -n
# deleted:COPYING
# which is expected as we're telling git to not restore or create
# files, however:
$ git checkout-index -a -n -n
$ git status
# working directory clean, COPYING is restored again!
# That's the bug, we're fixing here.

By restraining the variable not_new to a value being definitely 0 or 1
by the macro OPT_BOOL the bug is fixed.

Signed-off-by: Stefan Beller 
---
 builtin/checkout-index.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/checkout-index.c b/builtin/checkout-index.c
index aa922ed..69e167b 100644
--- a/builtin/checkout-index.c
+++ b/builtin/checkout-index.c
@@ -188,7 +188,7 @@ int cmd_checkout_index(int argc, const char **argv, const 
char *prefix)
OPT__FORCE(&force, N_("force overwrite of existing files")),
OPT__QUIET(&quiet,
N_("no warning for existing files and files not in 
index")),
-   OPT_BOOLEAN('n', "no-create", ¬_new,
+   OPT_BOOL('n', "no-create", ¬_new,
N_("don't checkout new files")),
{ OPTION_CALLBACK, 'u', "index", &newfd, NULL,
N_("update stat information in the index file"),
-- 
1.8.4.rc0.1.g8f6a3e5

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCHv2 6/9] hash-object: Replace stdin parsing OPT_BOOLEAN by OPT_COUNTUP

2013-07-31 Thread Stefan Beller
This task emerged from b04ba2bb (parse-options: deprecate OPT_BOOLEAN,
2011-09-27). hash-object is a plumbing layer command, so better
not change the input/output behavior for now.

Unfortunately we have these lines relying on the count up mechanism of
OPT_BOOLEAN:

if (hashstdin > 1)
errstr = "Multiple --stdin arguments are not supported";

Maybe later, when the plumbing is refined (git 2.0?) we can drop that
errormessage and replace the OPT_COUNTUP by OPT_BOOL.

Signed-off-by: Stefan Beller 
---
 builtin/hash-object.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/hash-object.c b/builtin/hash-object.c
index 4aea5bb..d7fcf4c 100644
--- a/builtin/hash-object.c
+++ b/builtin/hash-object.c
@@ -71,7 +71,7 @@ static const char *vpath;
 static const struct option hash_object_options[] = {
OPT_STRING('t', NULL, &type, N_("type"), N_("object type")),
OPT_BOOL('w', NULL, &write_object, N_("write the object into the object 
database")),
-   OPT_BOOLEAN( 0 , "stdin", &hashstdin, N_("read the object from stdin")),
+   OPT_COUNTUP( 0 , "stdin", &hashstdin, N_("read the object from stdin")),
OPT_BOOL( 0 , "stdin-paths", &stdin_paths, N_("read file names from 
stdin")),
OPT_BOOL( 0 , "no-filters", &no_filters, N_("store file as is without 
filters")),
OPT_STRING( 0 , "path", &vpath, N_("file"), N_("process file as it were 
from this path")),
-- 
1.8.4.rc0.1.g8f6a3e5

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCHv2 5/9] branch, commit, name-rev: ease up boolean conditions

2013-07-31 Thread Stefan Beller
Now that the variables are readin by OPT_BOOL, which makes sure
to have the values being 0 or 1 after reading, we do not need
the double negation to map any other value to 1 for integer
variables.

Signed-off-by: Stefan Beller 
---
 builtin/branch.c   | 3 ++-
 builtin/commit.c   | 2 +-
 builtin/name-rev.c | 2 +-
 3 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index 4daed0b..33ba1fb 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -872,7 +872,8 @@ int cmd_branch(int argc, const char **argv, const char 
*prefix)
if (with_commit || merge_filter != NO_FILTER)
list = 1;
 
-   if (!!delete + !!rename + !!force_create + !!list + !!new_upstream + 
!!unset_upstream > 1)
+   if (delete + rename + force_create + list + unset_upstream +
+   !!new_upstream > 1)
usage_with_options(builtin_branch_usage, options);
 
if (abbrev == -1)
diff --git a/builtin/commit.c b/builtin/commit.c
index c20426b..b0f86c8 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1072,7 +1072,7 @@ static int parse_and_validate_options(int argc, const 
char *argv[],
if (patch_interactive)
interactive = 1;
 
-   if (!!also + !!only + !!all + !!interactive > 1)
+   if (also + only + all + interactive > 1)
die(_("Only one of --include/--only/--all/--interactive/--patch 
can be used."));
if (argc == 0 && (also || (only && !amend)))
die(_("No paths with --include/--only does not make sense."));
diff --git a/builtin/name-rev.c b/builtin/name-rev.c
index a908a34..20fcf8c 100644
--- a/builtin/name-rev.c
+++ b/builtin/name-rev.c
@@ -331,7 +331,7 @@ int cmd_name_rev(int argc, const char **argv, const char 
*prefix)
 
git_config(git_default_config, NULL);
argc = parse_options(argc, argv, prefix, opts, name_rev_usage, 0);
-   if (!!all + !!transform_stdin + !!argc > 1) {
+   if (all + transform_stdin + !!argc > 1) {
error("Specify either a list, or --all, not both!");
usage_with_options(name_rev_usage, opts);
}
-- 
1.8.4.rc0.1.g8f6a3e5

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCHv2 3/9] log, format-patch: parsing uses OPT__QUIET

2013-07-31 Thread Stefan Beller
This patch allows users to use the short form -q on
log and format-patch, which was non possible before.

Signed-off-by: Stefan Beller 
---
 builtin/log.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/builtin/log.c b/builtin/log.c
index 1dafbd0..ed4dec4 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -121,7 +121,7 @@ static void cmd_log_init_finish(int argc, const char 
**argv, const char *prefix,
static struct line_opt_callback_data line_cb = {NULL, NULL, 
STRING_LIST_INIT_DUP};
 
const struct option builtin_log_options[] = {
-   OPT_BOOL(0, "quiet", &quiet, N_("suppress diff output")),
+   OPT__QUIET(&quiet, N_("suppress diff output")),
OPT_BOOL(0, "source", &source, N_("show source")),
OPT_BOOL(0, "use-mailmap", &mailmap, N_("Use mail map file")),
{ OPTION_CALLBACK, 0, "decorate", NULL, NULL, N_("decorate 
options"),
@@ -1210,8 +1210,7 @@ int cmd_format_patch(int argc, const char **argv, const 
char *prefix)
PARSE_OPT_OPTARG, thread_callback },
OPT_STRING(0, "signature", &signature, N_("signature"),
N_("add a signature")),
-   OPT_BOOLEAN(0, "quiet", &quiet,
-   N_("don't print the patch filenames")),
+   OPT__QUIET(&quiet, N_("don't print the patch filenames")),
OPT_END()
};
 
-- 
1.8.4.rc0.1.g8f6a3e5

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCHv2 7/9] config: parsing options: config file flag multiple times

2013-07-31 Thread Stefan Beller
This task emerged from b04ba2bb (parse-options: deprecate OPT_BOOLEAN,
2011-09-27).

This commit introduces a change for the users, after this patch
you can pass one of the config level flags multiple times:
Before:
$ git config --global --global --list
error: only one config file at a time.
usage: ...

Afterwards this will work. This is due to the following check in the code:
if (use_global_config + use_system_config + use_local_config +
!!given_config_file + !!given_config_blob > 1) {
error("only one config file at a time.");
usage_with_options(builtin_config_usage, 
builtin_config_options);
}

With OPT_BOOL instead of OPT_BOOLEAN the variables use_global_config,
use_system_config, use_local_config will only have the value 0 if the
command line option was not passed or 1 no matter how often the
respective command line option was passed.

Signed-off-by: Stefan Beller 
---
 builtin/config.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/builtin/config.c b/builtin/config.c
index da12fdb..4ab9e9a 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -50,9 +50,9 @@ static int respect_includes = -1;
 
 static struct option builtin_config_options[] = {
OPT_GROUP(N_("Config file location")),
-   OPT_BOOLEAN(0, "global", &use_global_config, N_("use global config 
file")),
-   OPT_BOOLEAN(0, "system", &use_system_config, N_("use system config 
file")),
-   OPT_BOOLEAN(0, "local", &use_local_config, N_("use repository config 
file")),
+   OPT_BOOL(0, "global", &use_global_config, N_("use global config file")),
+   OPT_BOOL(0, "system", &use_system_config, N_("use system config file")),
+   OPT_BOOL(0, "local", &use_local_config, N_("use repository config 
file")),
OPT_STRING('f', "file", &given_config_file, N_("file"), N_("use given 
config file")),
OPT_STRING(0, "blob", &given_config_blob, N_("blob-id"), N_("read 
config from given blob object")),
OPT_GROUP(N_("Action")),
-- 
1.8.4.rc0.1.g8f6a3e5

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCHv2 9/9] revert: use the OPT_CMDMODE for parsing, reducing code

2013-07-31 Thread Stefan Beller
The revert command comes with their own implementation of checking
for exclusiveness of parameters.
Now that the OPT_CMDMODE is in place, we can also rely on that macro
instead of cooking that solution for each command itself.

This commit also replaces OPT_BOOLEAN, which was deprecated by b04ba2bb
(parse-options: deprecate OPT_BOOLEAN, 2011-09-27). Instead OPT_BOOL is
used.

Signed-off-by: Stefan Beller 
---
 builtin/revert.c | 62 ++--
 1 file changed, 15 insertions(+), 47 deletions(-)

diff --git a/builtin/revert.c b/builtin/revert.c
index 1d2648b..8e87acd 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -71,44 +71,19 @@ static void verify_opt_compatible(const char *me, const 
char *base_opt, ...)
die(_("%s: %s cannot be used with %s"), me, this_opt, base_opt);
 }
 
-LAST_ARG_MUST_BE_NULL
-static void verify_opt_mutually_compatible(const char *me, ...)
-{
-   const char *opt1, *opt2 = NULL;
-   va_list ap;
-
-   va_start(ap, me);
-   while ((opt1 = va_arg(ap, const char *))) {
-   if (va_arg(ap, int))
-   break;
-   }
-   if (opt1) {
-   while ((opt2 = va_arg(ap, const char *))) {
-   if (va_arg(ap, int))
-   break;
-   }
-   }
-   va_end(ap);
-
-   if (opt1 && opt2)
-   die(_("%s: %s cannot be used with %s"), me, opt1, opt2);
-}
-
 static void parse_args(int argc, const char **argv, struct replay_opts *opts)
 {
const char * const * usage_str = revert_or_cherry_pick_usage(opts);
const char *me = action_name(opts);
-   int remove_state = 0;
-   int contin = 0;
-   int rollback = 0;
+   int cmd = 0;
struct option options[] = {
-   OPT_BOOLEAN(0, "quit", &remove_state, N_("end revert or 
cherry-pick sequence")),
-   OPT_BOOLEAN(0, "continue", &contin, N_("resume revert or 
cherry-pick sequence")),
-   OPT_BOOLEAN(0, "abort", &rollback, N_("cancel revert or 
cherry-pick sequence")),
-   OPT_BOOLEAN('n', "no-commit", &opts->no_commit, N_("don't 
automatically commit")),
-   OPT_BOOLEAN('e', "edit", &opts->edit, N_("edit the commit 
message")),
+   OPT_CMDMODE(0, "quit", &cmd, N_("end revert or cherry-pick 
sequence"), 'q'),
+   OPT_CMDMODE(0, "continue", &cmd, N_("resume revert or 
cherry-pick sequence"), 'c'),
+   OPT_CMDMODE(0, "abort", &cmd, N_("cancel revert or cherry-pick 
sequence"), 'a'),
+   OPT_BOOL('n', "no-commit", &opts->no_commit, N_("don't 
automatically commit")),
+   OPT_BOOL('e', "edit", &opts->edit, N_("edit the commit 
message")),
OPT_NOOP_NOARG('r', NULL),
-   OPT_BOOLEAN('s', "signoff", &opts->signoff, N_("add 
Signed-off-by:")),
+   OPT_BOOL('s', "signoff", &opts->signoff, N_("add 
Signed-off-by:")),
OPT_INTEGER('m', "mainline", &opts->mainline, N_("parent 
number")),
OPT_RERERE_AUTOUPDATE(&opts->allow_rerere_auto),
OPT_STRING(0, "strategy", &opts->strategy, N_("strategy"), 
N_("merge strategy")),
@@ -124,11 +99,11 @@ static void parse_args(int argc, const char **argv, struct 
replay_opts *opts)
 
if (opts->action == REPLAY_PICK) {
struct option cp_extra[] = {
-   OPT_BOOLEAN('x', NULL, &opts->record_origin, N_("append 
commit name")),
-   OPT_BOOLEAN(0, "ff", &opts->allow_ff, N_("allow 
fast-forward")),
-   OPT_BOOLEAN(0, "allow-empty", &opts->allow_empty, 
N_("preserve initially empty commits")),
-   OPT_BOOLEAN(0, "allow-empty-message", 
&opts->allow_empty_message, N_("allow commits with empty messages")),
-   OPT_BOOLEAN(0, "keep-redundant-commits", 
&opts->keep_redundant_commits, N_("keep redundant, empty commits")),
+   OPT_BOOL('x', NULL, &opts->record_origin, N_("append 
commit name")),
+   OPT_BOOL(0, "ff", &opts->allow_ff, N_("allow 
fast-forward")),
+   OPT_BOOL(0, "allow-empty", &opts->allow_empty, 
N_("preserve initially empty commits")),
+   OPT_BOOL(0, "allow-empty-message", 
&opts->allow_empty_message, N_("allow commits with empty messages")),
+   OPT_BOOL(0, "keep-redundant-commits", 
&opts->keep_redundant_commits, N_("keep redundant, empty commits")),
OPT_END(),
};
if (parse_options_concat(options, ARRAY_SIZE(options), 
cp_extra))
@@ -139,23 +114,16 @@ static void parse_args(int argc, const char **argv, 
struct replay_opts *opts)
PARSE_OPT_KEEP_ARGV0 |
PARSE_OPT_KEEP_UNKNOWN);
 
-   /* Check for incompatible subcommands */
-   verify_opt_mutually_compatible(me,
-

[PATCHv2 4/9] checkout: remove superfluous local variable

2013-07-31 Thread Stefan Beller
Signed-off-by: Stefan Beller 
---
 builtin/checkout.c | 12 +---
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index 8b48f4a..ed39cec 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -228,8 +228,6 @@ static int checkout_paths(const struct checkout_opts *opts,
int flag;
struct commit *head;
int errs = 0;
-   int stage = opts->writeout_stage;
-   int merge = opts->merge;
int newfd;
struct lock_file *lock_file;
 
@@ -327,8 +325,8 @@ static int checkout_paths(const struct checkout_opts *opts,
continue;
if (opts->force) {
warning(_("path '%s' is unmerged"), ce->name);
-   } else if (stage) {
-   errs |= check_stage(stage, ce, pos);
+   } else if (opts->writeout_stage) {
+   errs |= check_stage(opts->writeout_stage, ce, 
pos);
} else if (opts->merge) {
errs |= check_stages((1<<2) | (1<<3), ce, pos);
} else {
@@ -352,9 +350,9 @@ static int checkout_paths(const struct checkout_opts *opts,
errs |= checkout_entry(ce, &state, NULL);
continue;
}
-   if (stage)
-   errs |= checkout_stage(stage, ce, pos, &state);
-   else if (merge)
+   if (opts->writeout_stage)
+   errs |= checkout_stage(opts->writeout_stage, 
ce, pos, &state);
+   else if (opts->merge)
errs |= checkout_merged(pos, &state);
pos = skip_same_name(ce, pos) - 1;
}
-- 
1.8.4.rc0.1.g8f6a3e5

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCHv2 0/9] Removing deprecated parsing macros

2013-07-31 Thread Stefan Beller
I am rerolling "Removing deprecated parsing macros" series. 
Now with more patches! ;)
The patches apply on top of origin/jc/parseopt-command-modes

Within the builtin/ folder all occurrences of OPT_BOOLEAN have been removed.
Now we only need to review the usage of it in parse-options as used in
OPT__VERBOSE, OPT__QUIET, OPT__DRY_RUN and OPT__FORCE.
Most likely we could just use OPT_SET_INT there and then OPT_BOOLEAN is 
gone.

The patch 1 and 2 are not intended to change any semantics,
but were the most work, because of the checking for each place not
changing the semantics.

Patch 3 introduces -q the shortform of --quiet for log and format-patch 

Patch 4,5 are unspectacular, just improving readability.

Patch 6 is indeed the only occurence, where I needed to use OPT_COUNTUP
for OPT_BOOLEAN. Personally I'd change it there as well, but it's plumbing.

Patch 7 makes git config a little more flexible (allowing --global 
multiple times).

Patch 8 is a change in the plumbing layer, what I'd call a bugfix,
not urgent, but still.

Patch 9 introduces the new OPT_CMDMODE to revert. Junio suggested to 
change the OPT_CMDMODE a little and use the short parameter as the value
assigned to the command variable. This patch shows, why it might not be a
good idea, as the options there do not have short parameters.

Stefan

Stefan Beller (9):
  Remove deprecated OPTION_BOOLEAN for parsing arguments
  Replace deprecated OPT_BOOLEAN by OPT_BOOL
  log, format-patch: parsing uses OPT__QUIET
  checkout: remove superfluous local variable
  branch, commit, name-rev: ease up boolean conditions
  hash-object: Replace stdin parsing OPT_BOOLEAN by OPT_COUNTUP
  config: parsing options: config file flag multiple times
  checkout-index: Fix negations of odd numbers of -n
  revert: use the OPT_CMDMODE for parsing, reducing code

 builtin/apply.c  | 24 +--
 builtin/bisect--helper.c |  8 +++
 builtin/blame.c  |  8 +++
 builtin/branch.c | 13 +-
 builtin/check-attr.c |  8 +++
 builtin/check-ignore.c   | 12 +-
 builtin/checkout-index.c |  8 +++
 builtin/checkout.c   | 27 ++---
 builtin/clean.c  |  6 ++---
 builtin/clone.c  | 23 +-
 builtin/commit.c | 48 ++---
 builtin/config.c |  8 +++
 builtin/describe.c   | 20 
 builtin/fast-export.c| 10 
 builtin/fetch.c  | 24 +--
 builtin/fsck.c   | 16 ++---
 builtin/gc.c |  4 ++--
 builtin/grep.c   | 38 ++---
 builtin/hash-object.c|  8 +++
 builtin/log.c| 17 +++--
 builtin/ls-files.c   | 24 +--
 builtin/ls-tree.c|  6 ++---
 builtin/merge-base.c | 10 
 builtin/merge-file.c |  2 +-
 builtin/merge.c  | 12 +-
 builtin/mv.c |  2 +-
 builtin/name-rev.c   | 14 +--
 builtin/notes.c  | 12 +-
 builtin/push.c   |  6 ++---
 builtin/remote.c | 28 +++---
 builtin/replace.c|  6 ++---
 builtin/reset.c  |  2 +-
 builtin/rev-parse.c  |  4 ++--
 builtin/revert.c | 62 
 builtin/rm.c |  6 ++---
 builtin/shortlog.c   | 12 +-
 builtin/show-branch.c| 28 +++---
 builtin/show-ref.c   | 15 ++--
 builtin/tag.c|  4 ++--
 builtin/update-ref.c |  4 ++--
 parse-options.h  |  5 ++--
 41 files changed, 277 insertions(+), 317 deletions(-)

-- 
1.8.4.rc0.1.g8f6a3e5

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCHv2 1/9] Remove deprecated OPTION_BOOLEAN for parsing arguments

2013-07-31 Thread Stefan Beller
As of b04ba2bb4 OPTION_BOOLEAN was deprecated.
This commit removes all occurrences of OPTION_BOOLEAN.
In b04ba2bb4 Junio suggested to replace it with either
OPTION_SET_INT or OPTION_COUNTUP instead. However a pattern, which
occurred often with the OPTION_BOOLEAN was a hidden boolean parameter.
So I defined OPT_HIDDEN_BOOL as an additional possible parse option
in parse-options.h to make life easy.

The OPT_HIDDEN_BOOL was used in checkout, clone, commit, show-ref.
The only exception, where there was need to fiddle with OPTION_SET_INT
was log and notes. However in these two files there is also a pattern,
so we could think of introducing OPT_NONEG_BOOL.

Signed-off-by: Stefan Beller 
---
 builtin/checkout.c |  5 ++---
 builtin/clone.c|  7 +++
 builtin/commit.c   | 10 --
 builtin/log.c  |  4 ++--
 builtin/notes.c|  8 
 builtin/show-ref.c |  5 ++---
 parse-options.h|  5 ++---
 7 files changed, 19 insertions(+), 25 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index 7025938..646a475 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -1073,9 +1073,8 @@ int cmd_checkout(int argc, const char **argv, const char 
*prefix)
OPT_BOOLEAN('p', "patch", &opts.patch_mode, N_("select hunks 
interactively")),
OPT_BOOL(0, "ignore-skip-worktree-bits", 
&opts.ignore_skipworktree,
 N_("do not limit pathspecs to sparse entries only")),
-   { OPTION_BOOLEAN, 0, "guess", &dwim_new_local_branch, NULL,
- N_("second guess 'git checkout no-such-branch'"),
- PARSE_OPT_NOARG | PARSE_OPT_HIDDEN },
+   OPT_HIDDEN_BOOL(0, "guess", &dwim_new_local_branch,
+   N_("second guess 'git checkout 
no-such-branch'")),
OPT_END(),
};
 
diff --git a/builtin/clone.c b/builtin/clone.c
index 430307b..e7b0b13 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -64,10 +64,9 @@ static struct option builtin_clone_options[] = {
 N_("force progress reporting")),
OPT_BOOLEAN('n', "no-checkout", &option_no_checkout,
N_("don't create a checkout")),
-   OPT_BOOLEAN(0, "bare", &option_bare, N_("create a bare repository")),
-   { OPTION_BOOLEAN, 0, "naked", &option_bare, NULL,
-   N_("create a bare repository"),
-   PARSE_OPT_NOARG | PARSE_OPT_HIDDEN },
+   OPT_BOOL(0, "bare", &option_bare, N_("create a bare repository")),
+   OPT_HIDDEN_BOOL(0, "naked", &option_bare,
+   N_("create a bare repository")),
OPT_BOOLEAN(0, "mirror", &option_mirror,
N_("create a mirror repository (implies bare)")),
OPT_BOOL('l', "local", &option_local,
diff --git a/builtin/commit.c b/builtin/commit.c
index 003bd7d..b64a083 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1448,12 +1448,10 @@ int cmd_commit(int argc, const char **argv, const char 
*prefix)
{ OPTION_STRING, 'u', "untracked-files", &untracked_files_arg, 
N_("mode"), N_("show untracked files, optional modes: all, normal, no. 
(Default: all)"), PARSE_OPT_OPTARG, NULL, (intptr_t)"all" },
/* end commit contents options */
 
-   { OPTION_BOOLEAN, 0, "allow-empty", &allow_empty, NULL,
- N_("ok to record an empty change"),
- PARSE_OPT_NOARG | PARSE_OPT_HIDDEN },
-   { OPTION_BOOLEAN, 0, "allow-empty-message", 
&allow_empty_message, NULL,
- N_("ok to record a change with an empty message"),
- PARSE_OPT_NOARG | PARSE_OPT_HIDDEN },
+   OPT_HIDDEN_BOOL(0, "allow-empty", &allow_empty,
+   N_("ok to record an empty change")),
+   OPT_HIDDEN_BOOL(0, "allow-empty-message", &allow_empty_message,
+   N_("ok to record a change with an empty 
message")),
 
OPT_END()
};
diff --git a/builtin/log.c b/builtin/log.c
index 2625f98..05e374d 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1183,9 +1183,9 @@ int cmd_format_patch(int argc, const char **argv, const 
char *prefix)
N_("don't output binary diffs")),
OPT_BOOLEAN(0, "ignore-if-in-upstream", &ignore_if_in_upstream,
N_("don't include a patch matching a commit 
upstream")),
-   { OPTION_BOOLEAN, 'p', "no-stat", &use_patch_format, NULL,
+   { OPTION_SET_INT, 'p', "no-stat", &use_patch_format, NULL,
  N_("show patch format instead of default (patch + stat)"),
- PARSE_OPT_NONEG | PARSE_OPT_NOARG },
+ PARSE_OPT_NONEG | PARSE_OPT_NOARG, NULL, 1},
OPT_GROUP(N_("Messaging")),
{ OPTION_CALLBACK, 0, "add-header", NULL, N_("header"),
N_("add email header"), 0, header_callback },
diff --git a/builtin/notes.c b/bu

Re: [PATCH gitk 0/4] gitk support for git log -L

2013-07-31 Thread Thomas Rast
Jens Lehmann  writes:

> Am 29.07.2013 21:37, schrieb Thomas Rast:
>> Thomas Rast  writes:
>> 
>>> Thomas Rast  writes:
>>>
 Now that git log -L has hit master, I figure it's time to discuss the
 corresponding change to gitk.
>>>
>>> Paul, any news on this?  Any chance we can get it into the next release,
>>> since that will also be the first release to ship with 'git log -L'?
>> 
>> Jens pointed out privately that the handling of unstuck -L options is
>> unfortunate, to put it mildly.  I'll send a reroll.
>
> But as soon as that is fixed I'd really like to see this applied, as
> I think gitk is the perfect tool to show history information.

Unfortunately it's turning out to be harder than I hoped.  gitk runs the
arguments through git-rev-parse, which only knows that -n gets an
unstuck argument.  Consequently, gitk accepts an unstuck -n but only
stuck forms of -S and -G.

Fixing it through git-rev-parse feels wrong; rev-parse is supposed to
know about rev-list options, but -S and -G only make sense in
diff-generating walks, and -L only makes any sense at all for git-log.

I'm tempted to leave it at the existing patches for now.  That does mean
that -L can only be used in the stuck form; it's the same for -S and -G
already.  Then in a later series we can change gitk's argument parsing
to properly treat the options directly, passing only the remaining
arguments through to rev-parse to use the usual revision/filename
distinction logic.

-- 
Thomas Rast
trast@{inf,student}.ethz.ch
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Unexpected merge results in git-svn repo

2013-07-31 Thread Jon Seymour
To answer my own question...

My issue turned to be caused by a bad graft in my repo (unfortunately,
since hardened with filter-branch) that was making the commit that
modified F on Y reachable from X. The graft was an (incorrectly
executed) attempt to deal with the "3+ parent" problem that sometimes
occurs when git-svn pulls merges back from SVN.

jon.

On Wed, Jul 31, 2013 at 8:14 PM, Jon Seymour  wrote:
> I am getting some unexpected results from a merge and I'd like to
> understand why.
>
> I have two commits X and Y that I want to merge.
>
> git merge-base X Y # yields B
> git diff B X -- F  # is empty
> git diff B Y -- F  # contains the change I want merged
> git rev-list X ^B -- F # is empty
> git rev-list Y ^B -- F # contains one commit
>
> git checkout X
> git merge Y
>
> fails with fixable merge conflicts on other files, but uses X's copy
> of F instead of Y's.
>
> I was expecting it to use Y's copy of F, since only Y has modified F since B.
>
> What could cause this?
>
> BTW: I am using a git-svn repo that does have some 4+ parent merges in it.
>
> jon.
--
To unsubscribe from this list: send the line "unsubscribe 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] log, format-patch: accept short parameter 'q' for quiet

2013-07-31 Thread Stefan Beller
On 07/30/13 00:05, René Scharfe wrote:
> Am 29.07.2013 21:49, schrieb Stefan Beller:
>>   const struct option builtin_log_options[] = {
>> -OPT_BOOL(0, "quiet", &quiet, N_("suppress diff output")),
>> +OPT_BOOL('q', "quiet", &quiet, N_("suppress diff output")),
> 
> You can shorten it using OPT__QUIET.  But that macro should be converted
> to OPT_BOOL first, in turn.  Unless --verbose, for which it sometimes
> makes sense to print ever more output the more often it is specified, I
> wouldn't expect there to be a command with different levels of
> quietness, but didn't check.
> 

Thanks for the hint.
I'll look into that as well.



signature.asc
Description: OpenPGP digital signature


Re: [PATCH] tag: Use OPT_BOOL instead of OPT_BOOLEAN to allow one action multiple times

2013-07-31 Thread Stefan Beller
On 07/31/13 00:28, Junio C Hamano wrote:
> 
> we could just do
> 
> #define OPT_CMDMODE(s, l, v, h) \
> { OPTION_CMDMODE, (s), (l), (v), NULL, \
>   (h), PARSE_OPT_NOARG|PARSE_OPT_NONEG, NULL, (s) }
> 

I agree that's a better proposal than mine.



signature.asc
Description: OpenPGP digital signature


Unexpected merge results in git-svn repo

2013-07-31 Thread Jon Seymour
I am getting some unexpected results from a merge and I'd like to
understand why.

I have two commits X and Y that I want to merge.

git merge-base X Y # yields B
git diff B X -- F  # is empty
git diff B Y -- F  # contains the change I want merged
git rev-list X ^B -- F # is empty
git rev-list Y ^B -- F # contains one commit

git checkout X
git merge Y

fails with fixable merge conflicts on other files, but uses X's copy
of F instead of Y's.

I was expecting it to use Y's copy of F, since only Y has modified F since B.

What could cause this?

BTW: I am using a git-svn repo that does have some 4+ parent merges in it.

jon.
--
To unsubscribe from this list: send the line "unsubscribe 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 2/5] Teach mv to move submodules using a gitfile

2013-07-31 Thread Fredrik Gustafsson
On Tue, Jul 30, 2013 at 09:50:03PM +0200, Jens Lehmann wrote:
> +/* Update gitfile and core.worktree setting to connect work tree and git dir 
> */
> +void connect_work_tree_and_git_dir(const char *work_tree, const char 
> *git_dir)
> +{
> + struct strbuf file_name = STRBUF_INIT;
> + struct strbuf rel_path = STRBUF_INIT;
> + const char *real_work_tree = xstrdup(real_path(work_tree));
> + FILE *fp;
> +
> + /* Update gitfile */
> + strbuf_addf(&file_name, "%s/.git", work_tree);
> + fp = fopen(file_name.buf, "w");
> + if (!fp)
> + die(_("Could not create git link %s"), file_name.buf);
> + fprintf(fp, "gitdir: %s\n", relative_path(git_dir, real_work_tree,
> +   &rel_path));
> + fclose(fp);
> +
> + /* Update core.worktree setting */
> + strbuf_reset(&file_name);
> + strbuf_addf(&file_name, "%s/config", git_dir);
> + if (git_config_set_in_file(file_name.buf, "core.worktree",
> +relative_path(real_work_tree, git_dir,
> +  &rel_path)))
> + die(_("Could not set core.worktree in %s"),
> + file_name.buf);
> +
> + strbuf_release(&file_name);
> + strbuf_release(&rel_path);
> + free((void *)real_work_tree);
> +}

Would it make sense to return an int here and do the dying in
builtin/mv.c instead? Again thinking of "libgit" and the die_errno for
non-submodule errors are in mv.c and not in rename().

If that's the case the same applies for stage_updated_gitmodules() and
update_path_in_gitmodules().

update_path_in_gitmodules() also needs a strbuf_release() if you haven't
already found it.

-- 
Med vänliga hälsningar
Fredrik Gustafsson

tel: 0733-608274
e-post: iv...@iveqy.com
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 06/11] t4211: retire soon-to-be unimplementable tests

2013-07-31 Thread Eric Sunshine
58960978 and 99780b0a added tests which demonstrated bugs (crashes) in
range-set and line-log when handed empty ranges specified via "log
-LX:file" where X is one greater than the last line of the file.  After
these tests were added, it was realized that the ability to specify an
empty range is a loophole due to a bug in -L bounds checking. That bug
is slated to be fixed in a subsequent patch.

Unfortunately, the closure of this loophole makes it impossible to
continue checking range-set and line-log behavior with regard to empty
ranges since there is no other way to specify empty ranges via the
command-line.  APIs of both facilities are private (file static) so
there likewise is no way to test their behaviors programmatically.
Consequently, retire these two tests.

Signed-off-by: Eric Sunshine 
---
 t/t4211-line-log.sh | 13 -
 1 file changed, 13 deletions(-)

diff --git a/t/t4211-line-log.sh b/t/t4211-line-log.sh
index f98275c..769ac68 100755
--- a/t/t4211-line-log.sh
+++ b/t/t4211-line-log.sh
@@ -94,17 +94,4 @@ test_expect_success '-L ,Y (Y == nlines + 2)' '
test_must_fail git log -L ,$n:b.c
 '
 
-# There is a separate bug when an empty -L range is the first -L encountered,
-# thus to demonstrate this particular bug, the empty -L range must follow a
-# non-empty -L range.
-test_expect_success '-L {empty-range} (any -L)' '
-   n=$(expr $(wc -l http://vger.kernel.org/majordomo-info.html


[PATCH 11/11] blame: reject empty ranges -L,+0 and -L,-0

2013-07-31 Thread Eric Sunshine
Empty ranges -L,+0 and -L,-0 are nonsensical in the context of blame yet
they are accepted (in fact, both are interpreted as -L1,Y where Y is
end-of-file). Report them as invalid.

Signed-off-by: Eric Sunshine 
---
 line-range.c| 2 +-
 t/annotate-tests.sh | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/line-range.c b/line-range.c
index a816951..69e8d6b 100644
--- a/line-range.c
+++ b/line-range.c
@@ -21,7 +21,7 @@ static const char *parse_loc(const char *spec, nth_line_fn_t 
nth_line,
 * for 20 lines, or "-L ,-5" for 5 lines ending at
 * .
 */
-   if (1 < begin && (spec[0] == '+' || spec[0] == '-')) {
+   if (1 <= begin && (spec[0] == '+' || spec[0] == '-')) {
num = strtol(spec + 1, &term, 10);
if (term != spec + 1) {
if (!ret)
diff --git a/t/annotate-tests.sh b/t/annotate-tests.sh
index d7807df..e422a9e 100644
--- a/t/annotate-tests.sh
+++ b/t/annotate-tests.sh
@@ -185,7 +185,7 @@ test_expect_success 'blame -L Y,X (undocumented)' '
check_count -L6,3 B 1 B1 1 B2 1 D 1
 '
 
-test_expect_failure 'blame -L ,+0' '
+test_expect_success 'blame -L ,+0' '
test_must_fail $PROG -L,+0 file
 '
 
@@ -201,7 +201,7 @@ test_expect_success 'blame -L X,+N' '
check_count -L3,+4 B 1 B1 1 B2 1 D 1
 '
 
-test_expect_failure 'blame -L ,-0' '
+test_expect_success 'blame -L ,-0' '
test_must_fail $PROG -L,-0 file
 '
 
-- 
1.8.3.4.1120.gc240c48

--
To unsubscribe from this list: send the line "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 05/11] t4211: log: demonstrate -L bounds checking bug

2013-07-31 Thread Eric Sunshine
A bounds checking bug allows the X in -LX to extend one line past the
end of file. For example, given a file with 5 lines, -L6 is accepted as
valid. Demonstrate this problem.

While here, also add tests to check that the remaining cases of X and Y
in -LX,Y are handled correctly at and in the vicinity of end-of-file.

Signed-off-by: Eric Sunshine 
---
 t/t4211-line-log.sh | 30 ++
 1 file changed, 30 insertions(+)

diff --git a/t/t4211-line-log.sh b/t/t4211-line-log.sh
index 7665d67..f98275c 100755
--- a/t/t4211-line-log.sh
+++ b/t/t4211-line-log.sh
@@ -64,6 +64,36 @@ test_bad_opts "-L 1,1000:b.c" "has only.*lines"
 test_bad_opts "-L :b.c" "argument.*not of the form"
 test_bad_opts "-L :foo:b.c" "no match"
 
+test_expect_success '-L X (X == nlines)' '
+   n=$(wc -l http://vger.kernel.org/majordomo-info.html


[PATCH 10/11] t8001/t8002: blame: demonstrate acceptance of bogus -L,+0 and -L,-0

2013-07-31 Thread Eric Sunshine
Empty ranges -L,+0 and -L,-0 are nonsensical in the context of blame yet
they are accepted. They should be errors. Demonstrate this shortcoming.

Signed-off-by: Eric Sunshine 
---
 t/annotate-tests.sh | 8 
 1 file changed, 8 insertions(+)

diff --git a/t/annotate-tests.sh b/t/annotate-tests.sh
index 01d50c5..d7807df 100644
--- a/t/annotate-tests.sh
+++ b/t/annotate-tests.sh
@@ -185,6 +185,10 @@ test_expect_success 'blame -L Y,X (undocumented)' '
check_count -L6,3 B 1 B1 1 B2 1 D 1
 '
 
+test_expect_failure 'blame -L ,+0' '
+   test_must_fail $PROG -L,+0 file
+'
+
 test_expect_success 'blame -L X,+0' '
test_must_fail $PROG -L1,+0 file
 '
@@ -197,6 +201,10 @@ test_expect_success 'blame -L X,+N' '
check_count -L3,+4 B 1 B1 1 B2 1 D 1
 '
 
+test_expect_failure 'blame -L ,-0' '
+   test_must_fail $PROG -L,-0 file
+'
+
 test_expect_success 'blame -L X,-0' '
test_must_fail $PROG -L1,-0 file
 '
-- 
1.8.3.4.1120.gc240c48

--
To unsubscribe from this list: send the line "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 02/11] t8001/t8002: blame: demonstrate -L bounds checking bug

2013-07-31 Thread Eric Sunshine
A bounds checking bug allows the X in -LX to extend one line past the
end of file. For example, given a file with 5 lines, -L6 is accepted as
valid. Demonstrate this problem.

While here, also add tests to check that the remaining cases of X and Y
in -LX,Y are handled correctly at and in the vicinity of end-of-file.

Signed-off-by: Eric Sunshine 
---
 t/annotate-tests.sh | 22 ++
 1 file changed, 22 insertions(+)

diff --git a/t/annotate-tests.sh b/t/annotate-tests.sh
index 3524eaf..02fbbf1 100644
--- a/t/annotate-tests.sh
+++ b/t/annotate-tests.sh
@@ -225,10 +225,32 @@ test_expect_success 'blame -L /RE/,-N' '
check_count -L/99/,-3 B 1 B2 1 D 1
 '
 
+# 'file' ends with an incomplete line, so 'wc' reports one fewer lines than
+# git-blame sees, hence the last line is actually $(wc...)+1.
+test_expect_success 'blame -L X (X == nlines)' '
+   n=$(expr $(wc -l  nlines)' '
test_must_fail $PROG -L12345 file
 '
 
+test_expect_success 'blame -L ,Y (Y == nlines)' '
+   n=$(expr $(wc -l  nlines)' '
test_must_fail $PROG -L,12345 file
 '
-- 
1.8.3.4.1120.gc240c48

--
To unsubscribe from this list: send the line "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 08/11] t8001/t8002: blame: demonstrate acceptance of bogus -LX,+0 and -LX,-0

2013-07-31 Thread Eric Sunshine
Empty ranges -LX,+0 and -LX,-0 are nonsensical in the context of blame
yet they are accepted. They should be errors. Demonstrate this
shortcoming.

Signed-off-by: Eric Sunshine 
---
 t/annotate-tests.sh | 8 
 1 file changed, 8 insertions(+)

diff --git a/t/annotate-tests.sh b/t/annotate-tests.sh
index f117ef0..0f80cba 100644
--- a/t/annotate-tests.sh
+++ b/t/annotate-tests.sh
@@ -185,6 +185,10 @@ test_expect_success 'blame -L Y,X (undocumented)' '
check_count -L6,3 B 1 B1 1 B2 1 D 1
 '
 
+test_expect_failure 'blame -L X,+0' '
+   test_must_fail $PROG -L1,+0 file
+'
+
 test_expect_success 'blame -L X,+1' '
check_count -L3,+1 B2 1
 '
@@ -193,6 +197,10 @@ test_expect_success 'blame -L X,+N' '
check_count -L3,+4 B 1 B1 1 B2 1 D 1
 '
 
+test_expect_failure 'blame -L X,-0' '
+   test_must_fail $PROG -L1,-0 file
+'
+
 test_expect_success 'blame -L X,-1' '
check_count -L3,-1 B2 1
 '
-- 
1.8.3.4.1120.gc240c48

--
To unsubscribe from this list: send the line "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 07/11] log: fix -L bounds checking bug

2013-07-31 Thread Eric Sunshine
When 12da1d1f added -L support to git-log, a broken bounds check was
copied from git-blame -L which incorrectly allows -LX to extend one line
past end of file without reporting an error.  Instead, it generates an
empty range.  Fix this bug.

Signed-off-by: Eric Sunshine 
---
 line-log.c  | 4 ++--
 t/t4211-line-log.sh | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/line-log.c b/line-log.c
index c2d01dc..1c3ac8d 100644
--- a/line-log.c
+++ b/line-log.c
@@ -594,13 +594,13 @@ parse_lines(struct commit *commit, const char *prefix, 
struct string_list *args)
lines, &begin, &end,
full_name))
die("malformed -L argument '%s'", range_part);
+   if (lines < end || ((lines || begin) && lines < begin))
+   die("file %s has only %lu lines", name_part, lines);
if (begin < 1)
begin = 1;
if (end < 1)
end = lines;
begin--;
-   if (lines < end || lines < begin)
-   die("file %s has only %ld lines", name_part, lines);
line_log_data_insert(&ranges, full_name, begin, end);
 
free_filespec(spec);
diff --git a/t/t4211-line-log.sh b/t/t4211-line-log.sh
index 769ac68..b01b3dd 100755
--- a/t/t4211-line-log.sh
+++ b/t/t4211-line-log.sh
@@ -69,7 +69,7 @@ test_expect_success '-L X (X == nlines)' '
git log -L $n:b.c
 '
 
-test_expect_failure '-L X (X == nlines + 1)' '
+test_expect_success '-L X (X == nlines + 1)' '
n=$(expr $(wc -l http://vger.kernel.org/majordomo-info.html


[PATCH 01/11] t8001/t8002: blame: decompose overly-large test

2013-07-31 Thread Eric Sunshine
Checking all bogus -L syntax forms in a single test makes it difficult
to identify the offender when one case fails. Decompose this
conglomerate test in order to check each bad syntax case separately.

Signed-off-by: Eric Sunshine 
---
 t/annotate-tests.sh | 32 +---
 1 file changed, 25 insertions(+), 7 deletions(-)

diff --git a/t/annotate-tests.sh b/t/annotate-tests.sh
index 0bfee00..3524eaf 100644
--- a/t/annotate-tests.sh
+++ b/t/annotate-tests.sh
@@ -275,12 +275,30 @@ test_expect_success 'blame -L :nomatch' '
test_must_fail $PROG -L:nomatch hello.c
 '
 
-test_expect_success 'blame -L bogus' '
-   test_must_fail $PROG -L file &&
-   test_must_fail $PROG -L1,+ file &&
-   test_must_fail $PROG -L1,- file &&
-   test_must_fail $PROG -LX file &&
-   test_must_fail $PROG -L1,X file &&
-   test_must_fail $PROG -L1,+N file &&
+test_expect_success 'blame -L' '
+   test_must_fail $PROG -L file
+'
+
+test_expect_success 'blame -L X,+' '
+   test_must_fail $PROG -L1,+ file
+'
+
+test_expect_success 'blame -L X,-' '
+   test_must_fail $PROG -L1,- file
+'
+
+test_expect_success 'blame -L X (non-numeric X)' '
+   test_must_fail $PROG -LX file
+'
+
+test_expect_success 'blame -L X,Y (non-numeric Y)' '
+   test_must_fail $PROG -L1,Y file
+'
+
+test_expect_success 'blame -L X,+N (non-numeric N)' '
+   test_must_fail $PROG -L1,+N file
+'
+
+test_expect_success 'blame -L X,-N (non-numeric N)' '
test_must_fail $PROG -L1,-N file
 '
-- 
1.8.3.4.1120.gc240c48

--
To unsubscribe from this list: send the line "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 00/11] blame/log -L: additional tests and bug fixes

2013-07-31 Thread Eric Sunshine
While working on multiple -L support for git-blame, I encountered more
issues with the existing -L facility in git-blame and git-log. This
series fixes these problems and adds a slew of new tests.

Patch 6/11 (t4211: retire soon-to-be unimplementable tests) may be
controversial. Removal of these tests was effectively a decision made in
isolation since my request for input [1] regarding the issue generated
only a single response (from j6t).

This series complements (does not replace) earlier -L-related fixes [2],
[3], [4], [5].

[1]: http://thread.gmane.org/gmane.comp.version-control.git/231035/focus=231126
[2]: http://thread.gmane.org/gmane.comp.version-control.git/229917
[3]: http://thread.gmane.org/gmane.comp.version-control.git/230532
[4]: 
http://git.661346.n2.nabble.com/PATCH-0-6-fix-blame-L-regression-add-tests-tp7592174.html
[5]: http://thread.gmane.org/gmane.comp.version-control.git/231035

Eric Sunshine (11):
  t8001/t8002: blame: decompose overly-large test
  t8001/t8002: blame: demonstrate -L bounds checking bug
  t8001/t8002: blame: add empty file & partial-line tests
  blame: fix -L bounds checking bug
  t4211: log: demonstrate -L bounds checking bug
  t4211: retire soon-to-be unimplementable tests
  log: fix -L bounds checking bug
  t8001/t8002: blame: demonstrate acceptance of bogus -LX,+0 and -LX,-0
  blame: reject empty ranges -LX,+0 and -LX,-0
  t8001/t8002: blame: demonstrate acceptance of bogus -L,+0 and -L,-0
  blame: reject empty ranges -L,+0 and -L,-0

 builtin/blame.c |   4 +-
 line-log.c  |   4 +-
 line-range.c|   4 +-
 t/annotate-tests.sh | 142 +---
 t/t4211-line-log.sh |  31 +---
 5 files changed, 166 insertions(+), 19 deletions(-)

-- 
1.8.3.4.1120.gc240c48

--
To unsubscribe from this list: send the line "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 09/11] blame: reject empty ranges -LX,+0 and -LX,-0

2013-07-31 Thread Eric Sunshine
Empty ranges -LX,+0 and -LX,-0 are nonsensical in the context of blame
yet they are accepted (in fact, both are interpreted as -LX,+2).  Report
them as invalid.

Signed-off-by: Eric Sunshine 
---
 line-range.c| 2 ++
 t/annotate-tests.sh | 4 ++--
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/line-range.c b/line-range.c
index 3942475..a816951 100644
--- a/line-range.c
+++ b/line-range.c
@@ -26,6 +26,8 @@ static const char *parse_loc(const char *spec, nth_line_fn_t 
nth_line,
if (term != spec + 1) {
if (!ret)
return term;
+   if (num == 0)
+   die("-L invalid empty range");
if (spec[0] == '-')
num = 0 - num;
if (0 < num)
diff --git a/t/annotate-tests.sh b/t/annotate-tests.sh
index 0f80cba..01d50c5 100644
--- a/t/annotate-tests.sh
+++ b/t/annotate-tests.sh
@@ -185,7 +185,7 @@ test_expect_success 'blame -L Y,X (undocumented)' '
check_count -L6,3 B 1 B1 1 B2 1 D 1
 '
 
-test_expect_failure 'blame -L X,+0' '
+test_expect_success 'blame -L X,+0' '
test_must_fail $PROG -L1,+0 file
 '
 
@@ -197,7 +197,7 @@ test_expect_success 'blame -L X,+N' '
check_count -L3,+4 B 1 B1 1 B2 1 D 1
 '
 
-test_expect_failure 'blame -L X,-0' '
+test_expect_success 'blame -L X,-0' '
test_must_fail $PROG -L1,-0 file
 '
 
-- 
1.8.3.4.1120.gc240c48

--
To unsubscribe from this list: send the line "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 04/11] blame: fix -L bounds checking bug

2013-07-31 Thread Eric Sunshine
Since inception, -LX,Y has correctly reported an out-of-range error when
Y is beyond end of file, however, X was not checked, and an out-of-range
X would cause a crash.  92f9e273 (blame: prevent a segv when -L given
start > EOF; 2010-02-08) attempted to rectify this shortcoming but has
its own off-by-one error which allows X to extend one line past end of
file.  For example, given a file with 5 lines:

  git blame -L5 foo  # OK, blames line 5
  git blame -L6 foo  # accepted, no error, no output, huh?
  git blame -L7 foo  # error "fatal: file foo has only 5 lines"

Fix this bug.

In order to avoid regressing "blame foo" when foo is an empty file, the
fix is slightly more complicated than changing '<' to '<='.

Signed-off-by: Eric Sunshine 
---
 builtin/blame.c | 4 ++--
 t/annotate-tests.sh | 8 
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/builtin/blame.c b/builtin/blame.c
index 079dcd3..e70b089 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -2495,13 +2495,13 @@ parse_done:
bottom = top = 0;
if (bottomtop)
prepare_blame_range(&sb, bottomtop, lno, &bottom, &top);
+   if (lno < top || ((lno || bottom) && lno < bottom))
+   die("file %s has only %lu lines", path, lno);
if (bottom < 1)
bottom = 1;
if (top < 1)
top = lno;
bottom--;
-   if (lno < top || lno < bottom)
-   die("file %s has only %lu lines", path, lno);
 
ent = xcalloc(1, sizeof(*ent));
ent->lno = bottom;
diff --git a/t/annotate-tests.sh b/t/annotate-tests.sh
index f67332c..f117ef0 100644
--- a/t/annotate-tests.sh
+++ b/t/annotate-tests.sh
@@ -232,7 +232,7 @@ test_expect_success 'blame -L X (X == nlines)' '
check_count -L$n C 1
 '
 
-test_expect_failure 'blame -L X (X == nlines + 1)' '
+test_expect_success 'blame -L X (X == nlines + 1)' '
n=$(expr $(wc -l http://vger.kernel.org/majordomo-info.html


[PATCH 03/11] t8001/t8002: blame: add empty file & partial-line tests

2013-07-31 Thread Eric Sunshine
Add boundary case tests, with and without -L, for empty file; file with
one partial line; file with one full line.

The empty file test without -L is of particular interest. Historically,
this case has been supported (empty blame output) and this test protects
against regression by a subsequent patch fixing an off-by-one bug which
incorrectly accepts -LX where X is one past end-of-file.

Signed-off-by: Eric Sunshine 
---
 t/annotate-tests.sh | 72 +
 1 file changed, 72 insertions(+)

diff --git a/t/annotate-tests.sh b/t/annotate-tests.sh
index 02fbbf1..f67332c 100644
--- a/t/annotate-tests.sh
+++ b/t/annotate-tests.sh
@@ -297,6 +297,78 @@ test_expect_success 'blame -L :nomatch' '
test_must_fail $PROG -L:nomatch hello.c
 '
 
+test_expect_success 'setup incremental' '
+   (
+   GIT_AUTHOR_NAME=I &&
+   export GIT_AUTHOR_NAME &&
+   GIT_AUTHOR_EMAIL=i...@test.git &&
+   export GIT_AUTHOR_EMAIL &&
+   >incremental &&
+   git add incremental &&
+   git commit -m "step 0" &&
+   printf "partial" >>incremental &&
+   git commit -a -m "step 0.5" &&
+   echo >>incremental &&
+   git commit -a -m "step 1"
+   )
+'
+
+test_expect_success 'blame empty' '
+   check_count -h HEAD^^ -f incremental
+'
+
+test_expect_success 'blame -L 0 empty (undocumented)' '
+   check_count -h HEAD^^ -f incremental -L0
+'
+
+test_expect_failure 'blame -L 1 empty' '
+   test_must_fail $PROG -L1 incremental HEAD^^
+'
+
+test_expect_success 'blame -L 2 empty' '
+   test_must_fail $PROG -L2 incremental HEAD^^
+'
+
+test_expect_success 'blame half' '
+   check_count -h HEAD^ -f incremental I 1
+'
+
+test_expect_success 'blame -L 0 half (undocumented)' '
+   check_count -h HEAD^ -f incremental -L0 I 1
+'
+
+test_expect_success 'blame -L 1 half' '
+   check_count -h HEAD^ -f incremental -L1 I 1
+'
+
+test_expect_failure 'blame -L 2 half' '
+   test_must_fail $PROG -L2 incremental HEAD^
+'
+
+test_expect_success 'blame -L 3 half' '
+   test_must_fail $PROG -L3 incremental HEAD^
+'
+
+test_expect_success 'blame full' '
+   check_count -f incremental I 1
+'
+
+test_expect_success 'blame -L 0 full (undocumented)' '
+   check_count -f incremental -L0 I 1
+'
+
+test_expect_success 'blame -L 1 full' '
+   check_count -f incremental -L1 I 1
+'
+
+test_expect_failure 'blame -L 2 full' '
+   test_must_fail $PROG -L2 incremental
+'
+
+test_expect_success 'blame -L 3 full' '
+   test_must_fail $PROG -L3 incremental
+'
+
 test_expect_success 'blame -L' '
test_must_fail $PROG -L file
 '
-- 
1.8.3.4.1120.gc240c48

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html