[RFD/PATCH 5/5] RFD: Disallow out-of-refspec refs within refs/remotes/* to be used as upstream
The previous patch adds validation of upstream remote-tracking branches by
parsing the configured refspecs, and making sure that the candidate upstream
(if not already matching refs/heads/* or refs/remotes/*) is indeed a
remote-tracking branch according to some remote's refspec. For a
default/conventional setup, this check would automatically also cover
everything within refs/remotes/*, meaning that the preceding check for
refs/remotes/* is redundant (although quicker than the validation against
refspecs). One could also argue that not everything inside refs/remotes/*
should be automatically acceptable as an upstream, if one were to keep
other (non-branch) type of remote-tracking refs there.
This patch removes the simple check for refs/remotes/*, to make sure that
_only_ validated remote-tracking branches (in addition to local branches)
are allowed as upstreams.
However, this means that for unconventional setups that place refs within
refs/remotes/* without configuring a corresponding refspec, those refs will
no longer be usable as upstreams. This breaks a few existing tests, which
are marked as test_expect_failure by this patch, to make them easy to find.
---
branch.c | 1 -
t/t3200-branch.sh| 2 +-
t/t7201-co.sh| 2 +-
t/t9114-git-svn-dcommit-merge.sh | 8
4 files changed, 6 insertions(+), 7 deletions(-)
diff --git a/branch.c b/branch.c
index c9f9dec..beaf11d 100644
--- a/branch.c
+++ b/branch.c
@@ -274,7 +274,6 @@ void create_branch(const char *head,
case 1:
/* Unique completion -- good, only if it is a real branch */
if (prefixcmp(real_ref, "refs/heads/") &&
- prefixcmp(real_ref, "refs/remotes/") &&
validate_remote_tracking_branch(real_ref)) {
if (explicit_tracking)
die(_(upstream_not_branch), start_name);
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index d969f0e..41d293d 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -317,7 +317,7 @@ test_expect_success 'test tracking setup (non-wildcard,
matching)' '
test $(git config branch.my4.merge) = refs/heads/master
'
-test_expect_success 'test tracking setup (non-wildcard, not matching)' '
+test_expect_failure 'test tracking setup (non-wildcard, not matching)' '
git config remote.local.url . &&
git config remote.local.fetch refs/heads/s:refs/remotes/local/s &&
(git show-ref -q refs/remotes/local/master || git fetch local) &&
diff --git a/t/t7201-co.sh b/t/t7201-co.sh
index be9672e..7267ee2 100755
--- a/t/t7201-co.sh
+++ b/t/t7201-co.sh
@@ -429,7 +429,7 @@ test_expect_success 'detach a symbolic link HEAD' '
test "z$(git rev-parse --verify refs/heads/master)" = "z$here"
'
-test_expect_success \
+test_expect_failure \
'checkout with --track fakes a sensible -b ' '
git update-ref refs/remotes/origin/koala/bear renamer &&
diff --git a/t/t9114-git-svn-dcommit-merge.sh b/t/t9114-git-svn-dcommit-merge.sh
index 3077851..ef274fd 100755
--- a/t/t9114-git-svn-dcommit-merge.sh
+++ b/t/t9114-git-svn-dcommit-merge.sh
@@ -45,7 +45,7 @@ test_expect_success 'setup svn repository' '
)
'
-test_expect_success 'setup git mirror and merge' '
+test_expect_failure 'setup git mirror and merge' '
git svn init "$svnrepo" -t tags -T trunk -b branches &&
git svn fetch &&
git checkout --track -b svn remotes/trunk &&
@@ -67,7 +67,7 @@ test_expect_success 'setup git mirror and merge' '
test_debug 'gitk --all & sleep 1'
-test_expect_success 'verify pre-merge ancestry' "
+test_expect_failure 'verify pre-merge ancestry' "
test x\`git rev-parse --verify refs/heads/svn^2\` = \
x\`git rev-parse --verify refs/heads/merge\` &&
git cat-file commit refs/heads/svn^ | grep '^friend$'
@@ -79,7 +79,7 @@ test_expect_success 'git svn dcommit merges' "
test_debug 'gitk --all & sleep 1'
-test_expect_success 'verify post-merge ancestry' "
+test_expect_failure 'verify post-merge ancestry' "
test x\`git rev-parse --verify refs/heads/svn\` = \
x\`git rev-parse --verify refs/remotes/trunk \` &&
test x\`git rev-parse --verify refs/heads/svn^2\` = \
@@ -87,7 +87,7 @@ test_expect_success 'verify post-merge ancestry' "
git cat-file commit refs/heads/svn^ | grep '^friend$'
"
-test_expect_success 'verify merge commit message' "
+test_expect_failure 'verify merge commit message' "
git rev-list --pretty=raw -1 refs/heads/svn | \
grep \"Merge branch 'merge' into svn\"
"
--
1.8.1.3.704.g33f7d4f
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFD/PATCH 3/5] checkout: Use remote refspecs when DWIMming tracking branches
The DWIM mode of checkout allows you to run "git checkout foo" when there is
no existing local ref or path called "foo", and there is exactly one remote
with a remote-tracking branch called "foo". Git will then automatically
create a new local branch called "foo" using the remote-tracking "foo" as
its starting point and configured upstream.
However, the current code hardcodes the assumption that all remote-tracking
branches are located at refs/remotes/$remote/*, and that "git checkout foo"
must find exactly one ref matching "refs/remotes/*/foo" to succeed.
This approach fails if a user has customized the refspec of a given remote to
place remote-tracking branches elsewhere.
The better way to find a tracking branch is to use the fetch refspecs for the
configured remotes to deduce the available candidate remote-tracking branches
corresponding to a remote branch of the requested name, and if exactly one of
these exists (and points to a valid SHA1), then that is the remote-tracking
branch we will use.
For example, in the case of "git checkout foo", we map "refs/heads/foo"
through each remote's refspec to find the available candidate remote-tracking
branches, and if exactly one of these candidates exist in our local repo, then
we have found the upstream for the new local branch "foo".
This fixes most of the failing tests introduced in the previous patch.
Signed-off-by: Johan Herland
---
Documentation/git-checkout.txt | 6 +++---
builtin/checkout.c | 42 ++
t/t2024-checkout-dwim.sh | 6 +++---
3 files changed, 28 insertions(+), 26 deletions(-)
diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt
index 8edcdca..bf0c99c 100644
--- a/Documentation/git-checkout.txt
+++ b/Documentation/git-checkout.txt
@@ -131,9 +131,9 @@ entries; instead, unmerged entries are ignored.
"--track" in linkgit:git-branch[1] for details.
+
If no '-b' option is given, the name of the new branch will be
-derived from the remote-tracking branch. If "remotes/" or "refs/remotes/"
-is prefixed it is stripped away, and then the part up to the
-next slash (which would be the nickname of the remote) is removed.
+derived from the remote-tracking branch, by looking at the local part of
+the refspec configured for the corresponding remote, and then stripping
+the initial part up to the "*".
This would tell us to use "hack" as the local branch when branching
off of "origin/hack" (or "remotes/origin/hack", or even
"refs/remotes/origin/hack"). If the given name has no slash, or the above
diff --git a/builtin/checkout.c b/builtin/checkout.c
index f8033f4..d6f9c01 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -822,38 +822,40 @@ static int git_checkout_config(const char *var, const
char *value, void *cb)
}
struct tracking_name_data {
- const char *name;
- char *remote;
+ /* const */ char *src_ref;
+ char *dst_ref;
+ unsigned char *dst_sha1;
int unique;
};
-static int check_tracking_name(const char *refname, const unsigned char *sha1,
- int flags, void *cb_data)
+static int check_tracking_name(struct remote *remote, void *cb_data)
{
struct tracking_name_data *cb = cb_data;
- const char *slash;
-
- if (prefixcmp(refname, "refs/remotes/"))
- return 0;
- slash = strchr(refname + 13, '/');
- if (!slash || strcmp(slash + 1, cb->name))
+ struct refspec query;
+ memset(&query, 0, sizeof(struct refspec));
+ query.src = cb->src_ref;
+ if (remote_find_tracking(remote, &query) ||
+ get_sha1(query.dst, cb->dst_sha1))
return 0;
- if (cb->remote) {
+ if (cb->dst_ref) {
cb->unique = 0;
return 0;
}
- cb->remote = xstrdup(refname);
+ cb->dst_ref = xstrdup(query.dst);
return 0;
}
-static const char *unique_tracking_name(const char *name)
+static const char *unique_tracking_name(const char *name, unsigned char *sha1)
{
- struct tracking_name_data cb_data = { NULL, NULL, 1 };
- cb_data.name = name;
- for_each_ref(check_tracking_name, &cb_data);
+ struct tracking_name_data cb_data = { NULL, NULL, NULL, 1 };
+ char src_ref[PATH_MAX];
+ snprintf(src_ref, PATH_MAX, "refs/heads/%s", name);
+ cb_data.src_ref = src_ref;
+ cb_data.dst_sha1 = sha1;
+ for_each_remote(check_tracking_name, &cb_data);
if (cb_data.unique)
- return cb_data.remote;
- free(cb_data.remote);
+ return cb_data.dst_ref;
+ free(cb_data.dst_ref);
return NULL;
}
@@ -916,8 +918,8 @@ static int parse_branchname_arg(int argc, const char **argv,
if (dwim_new_local_branch_ok &&
!check_filename(NULL, arg) &&
argc == 1) {
- const char *remote = unique_tracking_name(arg);
-
[RFD/PATCH 4/5] branch.c: Look up refspecs to validate tracking branches
The current code for validating tracking branches (e.g. the argument to the
-t/--track option) hardcodes refs/heads/* and refs/remotes/* as the potential
locations for tracking branches. This works well with the conventional
refspecs created by "git clone" or "git remote add", but does not work if the
user tweaks the refspecs to place remote-tracking branches outside
refs/remotes/*.
This patch adds explicit checking of the refspecs for each remote to determine
whether a candidate tracking branch is indeed a valid remote-tracking branch,
even if placed outside refs/heads/* and refs/remotes/*.
This new check is added as a fallback after checking for match against
refs/heads/* and refs/remotes/*, so the code will not be run in the common
case.
This patch also fixes the last remaining test failure in t2024-checkout-dwim.
Signed-off-by: Johan Herland
---
branch.c | 18 +-
t/t2024-checkout-dwim.sh | 2 +-
2 files changed, 18 insertions(+), 2 deletions(-)
diff --git a/branch.c b/branch.c
index 6ae6a4c..c9f9dec 100644
--- a/branch.c
+++ b/branch.c
@@ -197,6 +197,21 @@ int validate_new_branchname(const char *name, struct
strbuf *ref,
return 1;
}
+static int check_tracking_branch(struct remote *remote, void *cb_data)
+{
+ char *tracking_branch = cb_data;
+ struct refspec query;
+ memset(&query, 0, sizeof(struct refspec));
+ query.dst = tracking_branch;
+ return !(remote_find_tracking(remote, &query) ||
+prefixcmp(query.src, "refs/heads/"));
+}
+
+static int validate_remote_tracking_branch(char *ref)
+{
+ return !for_each_remote(check_tracking_branch, ref);
+}
+
static const char upstream_not_branch[] =
N_("Cannot setup tracking information; starting point '%s' is not a branch.");
static const char upstream_missing[] =
@@ -259,7 +274,8 @@ void create_branch(const char *head,
case 1:
/* Unique completion -- good, only if it is a real branch */
if (prefixcmp(real_ref, "refs/heads/") &&
- prefixcmp(real_ref, "refs/remotes/")) {
+ prefixcmp(real_ref, "refs/remotes/") &&
+ validate_remote_tracking_branch(real_ref)) {
if (explicit_tracking)
die(_(upstream_not_branch), start_name);
else
diff --git a/t/t2024-checkout-dwim.sh b/t/t2024-checkout-dwim.sh
index fc6edc9..a8f0a90 100755
--- a/t/t2024-checkout-dwim.sh
+++ b/t/t2024-checkout-dwim.sh
@@ -108,7 +108,7 @@ test_expect_success 'checkout of branch from a single
remote succeeds #3' '
test_tracking_branch spam repo_c
refs/remotes/extra_dir/repo_c/extra_dir/spam
'
-test_expect_failure 'checkout of branch from a single remote succeeds #4' '
+test_expect_success 'checkout of branch from a single remote succeeds #4' '
git checkout eggs &&
test_tracking_branch eggs repo_d refs/repo_d/eggs
'
--
1.8.1.3.704.g33f7d4f
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFD/PATCH 2/5] t2024: Show failure to use refspec when DWIMming remote branch names
When using "git checkout foo" to DWIM the creation of local "foo" from some
existing upstream "foo", we assume conventional refspecs as created by "git
clone" or "git remote add", and fail to work correctly if the current
refspecs do not follow the conventional "refs/remotes/$remote/*" pattern.
Signed-off-by: Johan Herland
---
t/t2024-checkout-dwim.sh | 52 +++-
1 file changed, 51 insertions(+), 1 deletion(-)
diff --git a/t/t2024-checkout-dwim.sh b/t/t2024-checkout-dwim.sh
index 5650d21..36bf52f 100755
--- a/t/t2024-checkout-dwim.sh
+++ b/t/t2024-checkout-dwim.sh
@@ -22,6 +22,7 @@ test_tracking_branch() {
}
test_expect_success 'setup' '
+ test_commit my_master &&
(git init repo_a &&
cd repo_a &&
test_commit a_master &&
@@ -49,7 +50,7 @@ test_expect_success 'checkout of non-existing branch fails' '
test_must_fail git checkout xyzzy
'
-test_expect_success 'checkout of branch from multiple remotes fails' '
+test_expect_success 'checkout of branch from multiple remotes fails #1' '
test_must_fail git checkout foo
'
@@ -63,4 +64,53 @@ test_expect_success 'checkout of branch from a single remote
succeeds #2' '
test_tracking_branch baz repo_b refs/remotes/other_b/baz
'
+test_expect_success 'setup more remotes with unconventional refspecs' '
+ git checkout master &&
+ git branch -D bar &&
+ git branch -D baz &&
+ test "$(git rev-parse --verify HEAD)" = "$(git rev-parse --verify
my_master)" &&
+ (git init repo_c &&
+cd repo_c &&
+test_commit c_master &&
+git checkout -b bar &&
+test_commit c_bar
+git checkout -b spam &&
+test_commit c_spam
+ ) &&
+ (git init repo_d &&
+cd repo_d &&
+test_commit d_master &&
+git checkout -b baz &&
+test_commit f_baz
+git checkout -b eggs &&
+test_commit c_eggs
+ ) &&
+ git remote add repo_c repo_c &&
+ git config remote.repo_c.fetch \
+ "+refs/heads/*:refs/remotes/extra_dir/repo_c/extra_dir/*" &&
+ git fetch repo_c &&
+ git remote add repo_d repo_d &&
+ git config remote.repo_d.fetch \
+ "+refs/heads/*:refs/repo_d/*" &&
+ git fetch repo_d
+'
+
+test_expect_failure 'checkout of branch from multiple remotes fails #2' '
+ test_must_fail git checkout bar
+'
+
+test_expect_failure 'checkout of branch from multiple remotes fails #3' '
+ test_must_fail git checkout baz
+'
+
+test_expect_failure 'checkout of branch from a single remote succeeds #3' '
+ git checkout spam &&
+ test_tracking_branch spam repo_c
refs/remotes/extra_dir/repo_c/extra_dir/spam
+'
+
+test_expect_failure 'checkout of branch from a single remote succeeds #4' '
+ git checkout eggs &&
+ test_tracking_branch eggs repo_d refs/repo_d/eggs
+'
+
test_done
--
1.8.1.3.704.g33f7d4f
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFD/PATCH 1/5] t2024: Add tests verifying current DWIM behavior of 'git checkout '
The DWIM mode of checkout allows you to run "git checkout foo" when there is
no existing local ref or path called "foo", and there is exactly one remote
with a remote-tracking branch called "foo". Git will then automatically
create a new local branch called "foo" using the remote-tracking "foo" as
its starting point and configured upstream.
Signed-off-by: Johan Herland
---
t/t2024-checkout-dwim.sh | 66
1 file changed, 66 insertions(+)
create mode 100755 t/t2024-checkout-dwim.sh
diff --git a/t/t2024-checkout-dwim.sh b/t/t2024-checkout-dwim.sh
new file mode 100755
index 000..5650d21
--- /dev/null
+++ b/t/t2024-checkout-dwim.sh
@@ -0,0 +1,66 @@
+#!/bin/sh
+
+test_description='checkout
+
+Ensures that checkout on an unborn branch does what the user expects'
+
+. ./test-lib.sh
+
+# Arguments:
+#
+# Verify that we have checked out , and that it is at the same
+# commit as , and that has appropriate tracking config
+# setup against
+test_tracking_branch() {
+ branch=$1 &&
+ remote=$2 &&
+ remote_track=$3 &&
+ test "refs/heads/$branch" = "$(git rev-parse --symbolic-full-name
HEAD)" &&
+ test "$(git rev-parse --verify HEAD)" = "$(git rev-parse --verify
"$remote_track")" &&
+ test "$remote" = "$(git config "branch.$branch.remote")" &&
+ test "refs/heads/$branch" = "$(git config "branch.$branch.merge")"
+}
+
+test_expect_success 'setup' '
+ (git init repo_a &&
+cd repo_a &&
+test_commit a_master &&
+git checkout -b foo &&
+test_commit a_foo &&
+git checkout -b bar &&
+test_commit a_bar
+ ) &&
+ (git init repo_b &&
+cd repo_b &&
+test_commit b_master &&
+git checkout -b foo &&
+test_commit b_foo &&
+git checkout -b baz &&
+test_commit b_baz
+ ) &&
+ git remote add repo_a repo_a &&
+ git remote add repo_b repo_b &&
+ git config remote.repo_b.fetch \
+ "+refs/heads/*:refs/remotes/other_b/*" &&
+ git fetch --all
+'
+
+test_expect_success 'checkout of non-existing branch fails' '
+ test_must_fail git checkout xyzzy
+'
+
+test_expect_success 'checkout of branch from multiple remotes fails' '
+ test_must_fail git checkout foo
+'
+
+test_expect_success 'checkout of branch from a single remote succeeds #1' '
+ git checkout bar &&
+ test_tracking_branch bar repo_a refs/remotes/repo_a/bar
+'
+
+test_expect_success 'checkout of branch from a single remote succeeds #2' '
+ git checkout baz &&
+ test_tracking_branch baz repo_b refs/remotes/other_b/baz
+'
+
+test_done
--
1.8.1.3.704.g33f7d4f
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFD/PATCH 0/5] Improving the search for remote-tracking branches
Hi, The following patch series has three uncontroversial patches (I hope) to improve the DWIM behavivor "git checkout foo", followed by two more experimental patches meant to trigger discussion about how Git decides on what is a remote-tracking branch, and what is not: The first three patches concern the DWIMming of "git checkout foo" into "git checkout -b foo --track refs/remotes/$remote/foo". This DWIMming depends on finding exactly _one_ remote "$remote" having a remote- tracking branch called "foo". Currently we find remote-tracking branches by pattern-matching refs against "refs/remotes/*/foo", but instead we should look at the configured refspecs to find the real set of remote- tracking branch candidates. This fixes the DWIM behavior when the user has tweaked refspecs to place remote-tracking branches elsewhere (e.g. refs/remotes/$remote/heads/foo). The argument is explained more closely in patch #3. The remaining two patches tries to apply the same argument to the -t/--track argument to checkout/branch: This option takes a remote- tracking branch (or a local branch) as argument, but validating this argument is currently done by a simple match against refs/remotes/* (or refs/heads/*). By the same reasoning as above, we should instead look at the configured refspecs to determine whether the argument is a valid remote-tracking branch. However, as shown in the final patch, this would break a few tests where we currently try to --track refs that are not "proper" remote-tracking branches (in that they do not correspond to a refspec of some remote). - Should we simply "fix" these tests (t3200.39, t7201.24, t9114.*), and _require_ that all remote-tracking branches must have corresponding refspecs configured? - Are there valid real-world use cases that would be broken by such a requirement? - Is it preferable to stop after patch #4, and accept the superset of refs/remotes/* and refspec-matching refs as valid remote-tracking branches? Have fun! :) ...Johan Johan Herland (5): t2024: Add tests verifying current DWIM behavior of 'git checkout ' t2024: Show failure to use refspec when DWIMming remote branch names checkout: Use remote refspecs when DWIMming tracking branches branch.c: Look up refspecs to validate tracking branches RFD: Disallow out-of-refspec refs within refs/remotes/* to be used as upstream Documentation/git-checkout.txt | 6 +- branch.c | 17 +- builtin/checkout.c | 42 +++--- t/t2024-checkout-dwim.sh | 116 +++ t/t3200-branch.sh| 2 +- t/t7201-co.sh| 2 +- t/t9114-git-svn-dcommit-merge.sh | 8 +-- 7 files changed, 163 insertions(+), 30 deletions(-) create mode 100755 t/t2024-checkout-dwim.sh -- 1.8.1.3.704.g33f7d4f -- To unsubscribe from this list: send the line "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: Does git fast-import support deltas?
Hi Ilya, Ilya Basin wrote: > 1) a created a git repo from a foreign source using git fast-import > 2) new commits were added to the foreign source > > Can I create a fast-import input stream not containing the commits > already existing in my git repo and import it? Yes, if the foreign source is structured in a way to make it easy. Take a look at the --cat-blob-fd option, the "ls" and "cat-blob" commands, and see svn-fe from contrib/svn-fe/ as an example. Hope that helps, Jonathan -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] stash: tighten IS_STASH_LIKE logic
Junio C Hamano wrote: > [...] I'm curious. Why are you going back on what you said just one day ago? What changed? In a previous email, you wrote: > You are free to try to think of a way to tighten the implemention to > reject a random two-or-three parent merge commit that is not a > product of "stash create". People already have looked at this code > since it was written, and didn't find a reasonable way to tighten it > without triggering false negatives, so I wouldn't be surprised if > anybody tried it again today and failed. So, my patch is not a "reasonable" way to achieve this? > When was the last time you tried to run "git stash apply next"? My patch is not solving an end-user problem. Think of it as a source code comment: to answer the question "what kind of commit does stash create make?", the reader simple has to look at when IS_STASH_LIKE is set. It's helping make the source code clearer. Previously, IS_STASH_LIKE might as well have been named IS_MERGE_COMMIT, and nothing would've changed. The reader will wonder what IS_MERGE_COMMIT has to do with stashes, so we named it IS_STASH_LIKE. This is another minor improvement in the same spirit. > Is it worth it? Is it worth what? What are we losing? -- To unsubscribe from this list: send the line "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: t6200: avoid path mangling issue on Windows
Am 4/18/2013 19:05, schrieb Junio C Hamano: > Johannes Sixt writes: > >> From: Johannes Sixt >> >> MSYS bash interprets the slash in the argument core.commentchar="/" >> as root directory and mangles it into a Windows style path. Use a >> different core.commentchar to dodge the issue. >> >> Signed-off-by: Johannes Sixt >> ... >> -git -c core.commentchar="/" fmt-merge-msg --log=5 <.git/FETCH_HEAD >> >actual && >> +git -c core.commentchar="x" fmt-merge-msg --log=5 <.git/FETCH_HEAD >> >actual && > > Sigh... Again? > > Are folks working on Msys bash aware that sometimes the users may > want to say key=value on their command line without the value > getting molested in any way and giving them some escape hatch would > help them? Perhaps they have already decided that it is not > feasible after thinking about the issue, in which case I do not have > new ideas to offer. What is "the issue"? And in which way would an escape hatch help us here? We would have to apply a patch anyway after a glitch like this shows up, because disabling path mangling whole-sale (if there were a method -- there is none currently) is a no-go in the context of our test suite, let a lone in our scripted tool set. When "foo=/" appears on the command line, the most obvious interpretation of the slash for a program without mind-reading mode is that it is an absolute path, and then path mangling must happen (if and only if the invoked program is a non-MSYS program such as git). > I'll apply the patch as-is, but this feels really painful to the > users. No, generally, path mangling is a service for the user. -- Hannes -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Does git fast-import support deltas?
On Fri, Apr 19, 2013 at 12:29 AM, Ilya Basin wrote: > Hi list. > Here's what I mean: > 1) a created a git repo from a foreign source using git fast-import > 2) new commits were added to the foreign source > > Can I create a fast-import input stream not containing the commits > already existing in my git repo and import it? Yes, see the --import-marks and --export-marks options. The stream has to refer to the marks that were used int the previous run. Cheers. -- Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Does git fast-import support deltas?
Hi list. Here's what I mean: 1) a created a git repo from a foreign source using git fast-import 2) new commits were added to the foreign source Can I create a fast-import input stream not containing the commits already existing in my git repo and import it? I tried to create such streams with: cvsps --fast-export -d ... and from a shallow git repo, but the new commits are not imported (unless the import stream contains a new branch) -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 4/8] contrib: cc-cmd: add option to show commits
Instead of showing the authors and signers, show the commits themselves.
Signed-off-by: Felipe Contreras
---
contrib/cc-cmd/git-cc-cmd | 13 +
1 file changed, 13 insertions(+)
diff --git a/contrib/cc-cmd/git-cc-cmd b/contrib/cc-cmd/git-cc-cmd
index e36b1bf..f13ed8f 100755
--- a/contrib/cc-cmd/git-cc-cmd
+++ b/contrib/cc-cmd/git-cc-cmd
@@ -4,6 +4,7 @@ require 'optparse'
$since = '3-years-ago'
$min_percent = 5
+$show_commits = false
begin
OptionParser.new do |opts|
@@ -16,6 +17,9 @@ begin
opts.on('-d', '--since DATE', 'How far back to search for relevant
commits') do |v|
$since = v
end
+opts.on('-c', '--commits[=FORMAT]', [:raw], 'List commits instead of
persons') do |v|
+ $show_commits = v || true
+end
end.parse!
rescue OptionParser::InvalidOption
end
@@ -136,6 +140,15 @@ commits = Commits.new
commits.from_patches(ARGV)
commits.import
+if $show_commits
+ if $show_commits == :raw
+puts commits.items.keys
+ else
+system(*['git', 'log', '--oneline', '--no-walk'] + commits.items.keys)
+ end
+ exit 0
+end
+
# hash of hashes
persons = Hash.new { |hash, key| hash[key] = {} }
--
1.8.2.1.790.g4588561
--
To unsubscribe from this list: send the line "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 5/8] contrib: cc-cmd: add option to parse from committish
For example master..feature-a.
Signed-off-by: Felipe Contreras
---
contrib/cc-cmd/git-cc-cmd | 36 ++--
1 file changed, 34 insertions(+), 2 deletions(-)
diff --git a/contrib/cc-cmd/git-cc-cmd b/contrib/cc-cmd/git-cc-cmd
index f13ed8f..462f22c 100755
--- a/contrib/cc-cmd/git-cc-cmd
+++ b/contrib/cc-cmd/git-cc-cmd
@@ -5,11 +5,13 @@ require 'optparse'
$since = '3-years-ago'
$min_percent = 5
$show_commits = false
+$files = []
+$rev_args = []
begin
OptionParser.new do |opts|
opts.program_name = 'git cc-cmd'
-opts.banner = 'usage: git cc-cmd [options] '
+opts.banner = 'usage: git cc-cmd [options] '
opts.on('-p', '--min-percent N', Integer, 'Minium percentage of role
participation') do |v|
$min_percent = v
@@ -134,10 +136,40 @@ class Commits
end
end
+ def from_rev_args(args)
+return if args.empty?
+source = nil
+File.popen(%w[git rev-list --reverse] + args) do |p|
+ p.each do |e|
+id = e.chomp
+@main_commits[id] = true
+File.popen(%w[git --no-pager show -C --oneline] + [id]) do |p|
+ p.each do |e|
+case e
+when /^---\s+(\S+)/
+ source = $1 != '/dev/null' ? $1[2..-1] : nil
+when /^@@\s-(\d+),(\d+)/
+ get_blame(source, $1, $2, id)
+end
+ end
+end
+ end
+end
+ end
+
+end
+
+ARGV.each do |e|
+ if File.exists?(e)
+$files << e
+ else
+$rev_args << e
+ end
end
commits = Commits.new
-commits.from_patches(ARGV)
+commits.from_patches($files)
+commits.from_rev_args($rev_args)
commits.import
if $show_commits
--
1.8.2.1.790.g4588561
--
To unsubscribe from this list: send the line "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 7/8] contrib: cc-cmd: fix parsing of rev-list args
For example '-1'. Signed-off-by: Felipe Contreras --- contrib/cc-cmd/git-cc-cmd | 9 ++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/contrib/cc-cmd/git-cc-cmd b/contrib/cc-cmd/git-cc-cmd index 02e1a99..6911259 100755 --- a/contrib/cc-cmd/git-cc-cmd +++ b/contrib/cc-cmd/git-cc-cmd @@ -23,7 +23,8 @@ begin $show_commits = v || true end end.parse! -rescue OptionParser::InvalidOption +rescue OptionParser::InvalidOption => e + $rev_args += e.args end class Commit @@ -147,9 +148,11 @@ class Commits case revs.size when 1 - committish = [ '%s..HEAD' % revs[0] ] + r = revs[0] + r = '^' + r if r[0] != '-' + args = [ r, 'HEAD' ] else - committish = revs + args = revs end source = nil -- 1.8.2.1.790.g4588561 -- To unsubscribe from this list: send the line "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 8/8] contrib: cc-cmd: add option to fetch aliases
Only the mutt format is supported for now.
Signed-off-by: Felipe Contreras
---
contrib/cc-cmd/git-cc-cmd | 24
1 file changed, 24 insertions(+)
diff --git a/contrib/cc-cmd/git-cc-cmd b/contrib/cc-cmd/git-cc-cmd
index 6911259..02548c6 100755
--- a/contrib/cc-cmd/git-cc-cmd
+++ b/contrib/cc-cmd/git-cc-cmd
@@ -7,6 +7,8 @@ $min_percent = 5
$show_commits = false
$files = []
$rev_args = []
+$get_aliases = false
+$aliases = {}
begin
OptionParser.new do |opts|
@@ -22,11 +24,32 @@ begin
opts.on('-c', '--commits[=FORMAT]', [:raw], 'List commits instead of
persons') do |v|
$show_commits = v || true
end
+opts.on('-a', '--aliases', 'Use aliases') do |v|
+ $get_aliases = v
+end
end.parse!
rescue OptionParser::InvalidOption => e
$rev_args += e.args
end
+def get_aliases
+ type = %x[git config sendemail.aliasfiletype].chomp
+ return if type != 'mutt'
+ file = %x[git config sendemail.aliasesfile].chomp
+ File.open(File.expand_path(file)) do |f|
+f.each do |line|
+ if line =~ /^\s*alias\s+(?:-group\s+\S+\s+)*(\S+)\s+(.*)$/
+key, addresses = $1, $2.split(', ')
+addresses.each do |address|
+ $aliases[address] = key
+end
+ end
+end
+ end
+end
+
+get_aliases if $get_aliases
+
class Commit
attr_reader :id
@@ -60,6 +83,7 @@ class Commit
end
roles = roles.map do |person, role|
address = "%s <%s>" % person
+ person = nil, $aliases[address] if $aliases.include?(address)
[person, role]
end
[id, roles]
--
1.8.2.1.790.g4588561
--
To unsubscribe from this list: send the line "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 6/8] contrib: cc-cmd: parse committish like format-patch
Signed-off-by: Felipe Contreras --- contrib/cc-cmd/git-cc-cmd | 14 ++ 1 file changed, 14 insertions(+) diff --git a/contrib/cc-cmd/git-cc-cmd b/contrib/cc-cmd/git-cc-cmd index 462f22c..02e1a99 100755 --- a/contrib/cc-cmd/git-cc-cmd +++ b/contrib/cc-cmd/git-cc-cmd @@ -138,6 +138,20 @@ class Commits def from_rev_args(args) return if args.empty? + +revs = [] + +File.popen(%w[git rev-parse --revs-only --default=HEAD --symbolic] + args).each do |rev| + revs << rev.chomp +end + +case revs.size +when 1 + committish = [ '%s..HEAD' % revs[0] ] +else + committish = revs +end + source = nil File.popen(%w[git rev-list --reverse] + args) do |p| p.each do |e| -- 1.8.2.1.790.g4588561 -- To unsubscribe from this list: send the line "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 3/8] contrib: cc-cmd: add support for multiple patches
Signed-off-by: Felipe Contreras
---
contrib/cc-cmd/git-cc-cmd | 34 ++
1 file changed, 18 insertions(+), 16 deletions(-)
diff --git a/contrib/cc-cmd/git-cc-cmd b/contrib/cc-cmd/git-cc-cmd
index 0a5ec01..e36b1bf 100755
--- a/contrib/cc-cmd/git-cc-cmd
+++ b/contrib/cc-cmd/git-cc-cmd
@@ -8,7 +8,7 @@ $min_percent = 5
begin
OptionParser.new do |opts|
opts.program_name = 'git cc-cmd'
-opts.banner = 'usage: git cc-cmd [options] '
+opts.banner = 'usage: git cc-cmd [options] '
opts.on('-p', '--min-percent N', Integer, 'Minium percentage of role
participation') do |v|
$min_percent = v
@@ -66,6 +66,7 @@ class Commits
def initialize()
@items = {}
+@main_commits = {}
end
def size
@@ -103,24 +104,27 @@ class Commits
p.each do |line|
if line =~ /^(\h{40})/
id = $1
- @items[id] = Commit.new(id)
+ @items[id] = Commit.new(id) if not @main_commits.include?(id)
end
end
end
end
- def from_patch(file)
+ def from_patches(files)
source = nil
-from = nil
-File.open(file) do |f|
- f.each do |line|
-case line
-when /^From (\h+) (.+)$/
- from = $1
-when /^---\s+(\S+)/
- source = $1 != '/dev/null' ? $1[2..-1] : nil
-when /^@@\s-(\d+),(\d+)/
- get_blame(source, $1, $2, from)
+files.each do |file|
+ from = nil
+ File.open(file) do |f|
+f.each do |line|
+ case line
+ when /^From (\h+) (.+)$/
+from = $1
+@main_commits[from] = true
+ when /^---\s+(\S+)/
+source = $1 != '/dev/null' ? $1[2..-1] : nil
+ when /^@@\s-(\d+),(\d+)/
+get_blame(source, $1, $2, from)
+ end
end
end
end
@@ -128,10 +132,8 @@ class Commits
end
-exit 1 if ARGV.size != 1
-
commits = Commits.new
-commits.from_patch(ARGV[0])
+commits.from_patches(ARGV)
commits.import
# hash of hashes
--
1.8.2.1.790.g4588561
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 2/8] contrib: cc-cmd: add option parsing
Signed-off-by: Felipe Contreras
---
contrib/cc-cmd/git-cc-cmd | 21 +++--
1 file changed, 19 insertions(+), 2 deletions(-)
diff --git a/contrib/cc-cmd/git-cc-cmd b/contrib/cc-cmd/git-cc-cmd
index c7ecf79..0a5ec01 100755
--- a/contrib/cc-cmd/git-cc-cmd
+++ b/contrib/cc-cmd/git-cc-cmd
@@ -1,8 +1,25 @@
#!/usr/bin/env ruby
+require 'optparse'
+
$since = '3-years-ago'
$min_percent = 5
+begin
+ OptionParser.new do |opts|
+opts.program_name = 'git cc-cmd'
+opts.banner = 'usage: git cc-cmd [options] '
+
+opts.on('-p', '--min-percent N', Integer, 'Minium percentage of role
participation') do |v|
+ $min_percent = v
+end
+opts.on('-d', '--since DATE', 'How far back to search for relevant
commits') do |v|
+ $since = v
+end
+ end.parse!
+rescue OptionParser::InvalidOption
+end
+
class Commit
attr_reader :id
@@ -107,15 +124,15 @@ class Commits
end
end
end
-import
end
end
exit 1 if ARGV.size != 1
-commits = Commits.new()
+commits = Commits.new
commits.from_patch(ARGV[0])
+commits.import
# hash of hashes
persons = Hash.new { |hash, key| hash[key] = {} }
--
1.8.2.1.790.g4588561
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/8] Add new git-cc-cmd helper to contrib
The code finds the changes of a commit, runs 'git blame' for each chunk
to see which other commits are relevant, and then reports the author and
signers.
Finally, it calculates what percentage of the total relevant commits
each person was involved in, and show only the ones that pass the
threshold.
For example:
% git cc-cmd 0001-remote-hg-trivial-cleanups.patch
Felipe Contreras (author: 100%)
Jeff King (signer: 83%)
Max Horn (signer: 16%)
Junio C Hamano (signer: 16%)
Thus it can be used for 'git send-email' as a cc-cmd.
Signed-off-by: Felipe Contreras
---
contrib/cc-cmd/git-cc-cmd | 140 ++
1 file changed, 140 insertions(+)
create mode 100755 contrib/cc-cmd/git-cc-cmd
diff --git a/contrib/cc-cmd/git-cc-cmd b/contrib/cc-cmd/git-cc-cmd
new file mode 100755
index 000..c7ecf79
--- /dev/null
+++ b/contrib/cc-cmd/git-cc-cmd
@@ -0,0 +1,140 @@
+#!/usr/bin/env ruby
+
+$since = '3-years-ago'
+$min_percent = 5
+
+class Commit
+
+ attr_reader :id
+ attr_accessor :roles
+
+ def initialize(id)
+@id = id
+@roles = []
+ end
+
+ def self.parse(data)
+id = author = msg = nil
+roles = {}
+data.each_line do |line|
+ if not msg
+case line
+when /^commit (.+)$/
+ id = $1
+when /^author ([^<>]+) <(\S+)>$/
+ author = $1, $2
+ roles[author] = 'author'
+when /^$/
+ msg = true
+end
+ else
+if line =~ /^(Signed-off|Reviewed|Acked)-by: ([^<>]+) <(\S+?)>$/
+ person = $2, $3
+ roles[person] = 'signer' if person != author
+end
+ end
+end
+roles = roles.map do |person, role|
+ address = "%s <%s>" % person
+ [person, role]
+end
+[id, roles]
+ end
+
+end
+
+class Commits
+
+ attr_reader :items
+
+ def initialize()
+@items = {}
+ end
+
+ def size
+@items.size
+ end
+
+ def import
+return if @items.empty?
+format = [ 'commit %H', 'author %an <%ae>', '', '%B' ].join('%n')
+File.popen(['git', 'show', '-z', '-s', '--format=format:' + format] +
@items.keys) do |p|
+ p.each("\0") do |data|
+next if data == "\0" # bug in git show?
+id, roles = Commit.parse(data)
+commit = @items[id]
+commit.roles = roles
+ end
+end
+ end
+
+ def each_person_role
+commit_roles = @items.values.map { |commit| commit.roles }.flatten(1)
+commit_roles.group_by { |person, role| person }.each do |person,
commit_roles|
+ commit_roles.group_by { |person, role| role }.each do |role,
commit_roles|
+yield person, role, commit_roles.size
+ end
+end
+ end
+
+ def get_blame(source, start, offset, from)
+return unless source
+File.popen(['git', 'blame', '--incremental', '-C',
+ '-L', '%u,+%u' % [start, offset],
+ '--since', $since, from + '^',
+ '--', source]) do |p|
+ p.each do |line|
+if line =~ /^(\h{40})/
+ id = $1
+ @items[id] = Commit.new(id)
+end
+ end
+end
+ end
+
+ def from_patch(file)
+source = nil
+from = nil
+File.open(file) do |f|
+ f.each do |line|
+case line
+when /^From (\h+) (.+)$/
+ from = $1
+when /^---\s+(\S+)/
+ source = $1 != '/dev/null' ? $1[2..-1] : nil
+when /^@@\s-(\d+),(\d+)/
+ get_blame(source, $1, $2, from)
+end
+ end
+end
+import
+ end
+
+end
+
+exit 1 if ARGV.size != 1
+
+commits = Commits.new()
+commits.from_patch(ARGV[0])
+
+# hash of hashes
+persons = Hash.new { |hash, key| hash[key] = {} }
+
+commits.each_person_role do |person, role, count|
+ persons[person][role] = count
+end
+
+persons.each do |person, roles|
+ roles = roles.map do |role, count|
+percent = count.to_f * 100 / commits.size
+next if percent < $min_percent
+"%s: %u%%" % [role, percent]
+ end.compact
+ next if roles.empty?
+
+ name, email = person
+ # must quote chars?
+ name = '"%s"' % name if name =~ /[^\w \-]/i
+ person = name ? "%s <%s>" % [name, email] : email
+ puts "%s (%s)" % [person, roles.join(', ')]
+end
--
1.8.2.1.790.g4588561
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 0/8] New git-cc-cmd helper
Hi, This script allows you to get a list of relevant persons to Cc when sending a patch series. % git cc-cmd v1.8.1.6^^1..v1.8.1.6^^2 "Henrik Grubbström" (author: 7%) junio (signer: 84%, author: 15%) "Nguyễn Thái Ngọc Duy" (author: 30%, signer: 7%) "Jean-Noël AVILA" (author: 7%) Jean-Noel Avila (signer: 7%) Duy Nguyen (author: 7%) Michael Haggerty (author: 15%) Clemens Buchacher (author: 7%) Joshua Jensen (author: 7%) Johannes Sixt (signer: 7%) The code finds the changes in each commit in the list, runs 'git blame' to see which other commits are relevant to those lines, and then adds the author and signer to the list. Finally, it calculates what percentage of the total relevant commits each person was involved in, and if it passes the threshold, it goes in. You can also choose to show the commits themselves: % git cc-cmd --commits v1.8.1.6^^1..v1.8.1.6^^2 9db9eec attr: avoid calling find_basename() twice per path 94bc671 Add directory pattern matching to attributes 82dce99 attr: more matching optimizations from .gitignore 593cb88 exclude: split basename matching code into a separate function b559263 exclude: split pathname matching code into a separate function 4742d13 attr: avoid searching for basename on every match f950eb9 rename pathspec_prefix() to common_prefix() and move to dir.[ch] 4a085b1 consolidate pathspec_prefix and common_prefix d932f4e Rename git_checkattr() to git_check_attr() 2d72174 Extract a function collect_all_attrs() 8cf2a84 Add string comparison functions that respect the ignore_case variable. 407a963 Merge branch 'rr/remote-helper-doc' ec775c4 attr: Expand macros immediately when encountered. But wait, there's more: you can also specify a list of patch files, which means this can be used for git send-emails --cc-cmd option. Felipe Contreras (8): Add new git-cc-cmd helper to contrib contrib: cc-cmd: add option parsing contrib: cc-cmd: add support for multiple patches contrib: cc-cmd: add option to show commits contrib: cc-cmd: add option to parse from committish contrib: cc-cmd: parse committish like format-patch contrib: cc-cmd: fix parsing of rev-list args contrib: cc-cmd: add option to fetch aliases contrib/cc-cmd/git-cc-cmd | 245 ++ 1 file changed, 245 insertions(+) create mode 100755 contrib/cc-cmd/git-cc-cmd -- 1.8.2.1.790.g4588561 -- To unsubscribe from this list: send the line "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 (Apr 2013, #05; Mon, 15)
Junio C Hamano wrote: > You ran 'git add' with neither '-A (--all)' or '--no-all', whose > behaviour will change in Git 2.0 with respect to paths you > removed from your working tree. > > * 'git add --no-all ', which is the current default, > ignores paths you removed from your working tree. > > * 'git add --all ' will let you also record the > removals. > > The removed paths (e.g. '%s') are ignored with this version of Git. > Run 'git status' to remind yourself what paths you have removed > from your working tree. > > or something? That looks good. :) -- To unsubscribe from this list: send the line "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 (Apr 2013, #05; Mon, 15)
On Thu, Apr 18, 2013 at 03:10:29PM -0700, Junio C Hamano wrote: > Jeff King writes: > > > I am expecting a reaction more like "Hmm, I never thought about it > > before. Does that make sense to me or not? Let me think about which > > paths it pertains to and decide". > > Let's step back and re-review the main text. Good idea. I was very caught up in the existing message and what it made me expect, and not in what we are trying to accomplish overall. > It currently says: > > In Git 2.0, 'git add ...' will also update the > index for paths removed from the working tree that match > the given pathspec. If you want to 'add' only changed > or newly created paths, say 'git add --no-all ...' > instead. > > This was written for the old "we may want to warn" logic that did > not even check if we would be omitting a removal. The new logic > will show the text _only_ when the difference matters, we have an > opportunity to tighten it a lot, for example: > > You ran 'git add' with neither '-A (--all)' or '--no-all', whose > behaviour will change in Git 2.0 with respect to paths you > removed from your working tree. > > * 'git add --no-all ', which is the current default, > ignores paths you removed from your working tree. > > * 'git add --all ' will let you also record the > removals. > > The removed paths (e.g. '%s') are ignored with this version of Git. > Run 'git status' to remind yourself what paths you have removed > from your working tree. > > or something? Yes, I like that much better. It reads more clearly than the original, and it is more obvious why we are mentioning the path at all. And I think the hint of "git status" is good. I had considered before that the user would simply run "git status" after the message to get more data, but I didn't want to rely on them knowing to do that. Actually mentioning it is a good solution. :) Thanks for pointing us in the right direction. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 3/6] transport-helper: clarify pushing without refspecs
Eric Sunshine writes: >> + grep "remote-helper doesn.t support push; refspec needed" error > > Is "doesn.t" intentional? It certainly works by accident in grep, but > did you mean s/doesn.t/doesn't/ ? The pattern matching the expected string is not by accident, but by design. It of course can be made more strict to reject "doesnot" and require "doesn't" by doing something like this: grep "remote-helper doesn'\''t support push; refspec needed" error but at some point, it simply stops being worth it to tighten the pattern. For that matter, it could be as loose as grep "support push; refspec needed" error if you know the string is unique enough. -- To unsubscribe from this list: send the line "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] cat-file: print tags raw for "cat-file -p"
On Wed, Apr 17, 2013 at 5:00 PM, Jeff King wrote: > Subject: [PATCH] cat-file: print tags raw for "cat-file -p" > > When "cat-file -p" prints commits, it shows them in their > raw format, since git's format is already human-readable. > For tags, however, we print the whole thing raw except for > one thing: we convert the timestamp on the tagger line into a > human-readable date. > [...] > Let's drop the tagger-date formatting for "cat-file -p". It > makes us more consistent with cat-file's commit > pretty-printer, and as a bonus, we can drop the hand-rolled > tag parsing code in cat-file (which happened to behave > inconsistently with the tag pretty-printing code elsewhere). > > This is a change of output format, so it's possible that > some callers could considered this a regression. However, s/considered/consider/ > the original behavior was arguably a bug (due to the > inconsistency with commits), likely nobody was relying on it > (even we do not use it ourselves these days), and anyone > relying on the "-p" pretty-printer should be able to expect > a change in the output format (i.e., while "cat-file" is > plumbing, the output format of "-p" was never guaranteed to > be stable). > > Signed-off-by: Jeff King -- To unsubscribe from this list: send the line "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: is git-p4 compatible with p4/linux?
On Apr 18, 2013, at 7:09 PM, Pete Wyckoff wrote: > a...@aivor.com wrote on Tue, 16 Apr 2013 23:31 -0500: >> git-p4.py (1.8.2.1.418.gaec3f77) has at least two behaviors that >> seem to be incompatible with the version of p4 that I recently >> downloaded from perforce.com (P4/LINUX26X86_64/2013.1/610569). >> >> TLDR: Is git-p4 written for an old version of p4 CLI with different >> behavior? Or for a windows or mac release of p4? Or am I missing >> something? > > I had not done any testing beyond p4 12.2 (linux). But running > the unit tests through 13.1 just now, they all pass. > >$ p4 -V >Perforce - The Fast Software Configuration Management System. >Copyright 1995-2013 Perforce Software. All rights reserved. >This product includes software developed by the OpenSSL Project >for use in the OpenSSL Toolkit (http://www.openssl.org/) >See 'p4 help legal' for full OpenSSL license information >Version of OpenSSL Libraries: OpenSSL 1.0.1c 10 May 2012 >Rev. P4/LINUX26X86_64/2013.1/610569 (2013/03/19). > > I'm using python 2.7.3. > >> First issue >> --- >> >> git-p4 assumes the output of 'p4 print' adds a newline to the >> target. To work around this, git-p4.py strips the last char from >> symlinks as shown in the following snippet: >> >>if type_base =3D=3D "symlink": >>git_mode =3D "12" >># p4 print on a symlink contains "target\n"; remove the newline >>data =3D ''.join(contents) >>contents =3D [data[:-1]] >> >> But my 'p4 print' does not output the newline: >> >>$ ls -l pcre >>lrwxrwxrwx 1 user group 12 Apr 16 10:27 pcre -> ../libs/pcre >> >>$ p4 print -q pcre | od -t x1a >>000 2e 2e 2f 6c 69 62 73 2f 70 63 72 65 >> . . / l i b s / p c r e >>014 >> >> If I use 'git p4 clone' the above file shows up in git as a >> symlink to '../libs/pcr'. I had another symlink whose target had >> a strlen of 1 and the 'git p4 clone' failed b/c after stripping >> the last char the result was an empty string. > > There wasn't an explict test for symlinks, but I threw > one together quickly and it seems to work. Can you show > a bit more information about anything that potentially might > be odd with your install? > >arf-git-test$ ls -l symlink >lrwxrwxrwx 1 pw pw 14 Apr 18 20:02 symlink -> symlink-target > >$ p4 fstat symlink >... depotFile //depot/symlink >... clientFile /run/shm/trash directory.t9802-git-p4-filetype/cli/symlink >... isMapped >... headAction add >... headType symlink >... headTime 1366329740 >... headRev 1 >... headChange 6 >... headModTime 1366329740 >... haveRev 1 > >$ p4 print -q symlink | od -t x1a >000 73 79 6d 6c 69 6e 6b 2d 74 61 72 67 65 74 0a > s y m l i n k - t a r g e t nl >017 > > No idea why I get an "nl" but you do not. If you run _without_ > the "| od ...", then the shell prompt ends up on the same line > as the output? Any interesting shell or shell settings? > Yes. The shell prompt is on the same line as the output: $ PS1="FOOBAR$" FOOBAR$p4 print -q pcre ../libs/pcreFOOBAR$ Perhaps it is a configuration item on the server and/or client. It seems we are running the same version of p4. But just to be sure, check yours against mine: $ cksum $(which p4) 3254530484 2420552 /usr/bin/p4 If yours if different, can you email a copy of your p4 executable to me so I can check if it works differently than mine? I will also check with coworkers here to see how their client behaves. >> Second issue >> >> >> git-p4 uses 'p4 print -q -o o FILE' to print a file to stdout. >> At least that is how I interpret this snippet: >> >>text = p4_read_pipe(['print', '-q', '-o', '-', file['depotFile']]) >> >> However, p4/Linux prints to stdout by default and '-o -' will save >> the output in a file named '-'. >> >> My git and p4 versions: >> >>$ git --version >>git version 1.8.2.1.418.gaec3f77 >> >>$ p4 -V >>Perforce - The Fast Software Configuration Management System. >>Copyright 1995-2013 Perforce Software. All rights reserved. >>This product includes software developed by the OpenSSL Project >>for use in the OpenSSL Toolkit (http://www.openssl.org/) >>See 'p4 help legal' for full OpenSSL license information >>Version of OpenSSL Libraries: OpenSSL 1.0.1c 10 May 2012 >>Rev. P4/LINUX26X86_64/2013.1/610569 (2013/03/19). > > This code only happens on utf16 files. But running it by hand, > I cannot reproduce the different behavior: > >$ p4 print -q //depot/f-ascii >three >line >text > >$ p4 print -o - -q //depot/f-ascii >three >line > >$ ls ./- >ls: cannot access ./-: No such file or directory > > I'm again confused. Any hints you can give would be helpful. This "second issue" is a non-issue. It seems "-o -
Re: [PATCH v2 3/6] transport-helper: clarify pushing without refspecs
On Thu, Apr 18, 2013 at 7:27 PM, Eric Sunshine wrote: > On Thu, Apr 18, 2013 at 12:14 AM, Felipe Contreras > wrote: >> diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh >> index cd1873c..3eeb309 100755 >> --- a/t/t5801-remote-helpers.sh >> +++ b/t/t5801-remote-helpers.sh >> @@ -111,13 +111,13 @@ test_expect_success 'pulling without refspecs' ' >> compare_refs local2 HEAD server HEAD >> ' >> >> -test_expect_failure 'pushing without refspecs' ' >> +test_expect_success 'pushing without refspecs' ' >> test_when_finished "(cd local2 && git reset --hard origin)" && >> (cd local2 && >> echo content >>file && >> git commit -a -m ten && >> - GIT_REMOTE_TESTGIT_REFSPEC="" git push) && >> - compare_refs local2 HEAD server HEAD >> + GIT_REMOTE_TESTGIT_REFSPEC="" test_must_fail git push 2> ../error) && >> + grep "remote-helper doesn.t support push; refspec needed" error > > Is "doesn.t" intentional? It certainly works by accident in grep, but > did you mean s/doesn.t/doesn't/ ? In all git test cases we are already inside single quotes; can't do that. -- Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 3/6] transport-helper: clarify pushing without refspecs
On Thu, Apr 18, 2013 at 12:14 AM, Felipe Contreras wrote: > diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh > index cd1873c..3eeb309 100755 > --- a/t/t5801-remote-helpers.sh > +++ b/t/t5801-remote-helpers.sh > @@ -111,13 +111,13 @@ test_expect_success 'pulling without refspecs' ' > compare_refs local2 HEAD server HEAD > ' > > -test_expect_failure 'pushing without refspecs' ' > +test_expect_success 'pushing without refspecs' ' > test_when_finished "(cd local2 && git reset --hard origin)" && > (cd local2 && > echo content >>file && > git commit -a -m ten && > - GIT_REMOTE_TESTGIT_REFSPEC="" git push) && > - compare_refs local2 HEAD server HEAD > + GIT_REMOTE_TESTGIT_REFSPEC="" test_must_fail git push 2> ../error) && > + grep "remote-helper doesn.t support push; refspec needed" error Is "doesn.t" intentional? It certainly works by accident in grep, but did you mean s/doesn.t/doesn't/ ? > ' > > test_expect_success 'pulling without marks' ' -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Help with git-svn and SVN symlinks
Hi, I have been using git-svn successfully for last 1 year. But yesterday somebody checked in something in SVN using svn client and It caused inconsitency in GIT-svn repository. In one of SVN Merge commit, Some symlink files were not made "svn special" by mistake, So there was another SVN commit with only property change to convert these files into symlink. Git-svn should have picked this commit and converted these files into symlink but I think it missed it, Now I have text files in Git codebase instead of symlinks. In SVN repository these files are symlinks. How should I fix this? -- To unsubscribe from this list: send the line "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: is git-p4 compatible with p4/linux?
a...@aivor.com wrote on Tue, 16 Apr 2013 23:31 -0500: > git-p4.py (1.8.2.1.418.gaec3f77) has at least two behaviors that > seem to be incompatible with the version of p4 that I recently > downloaded from perforce.com (P4/LINUX26X86_64/2013.1/610569). > > TLDR: Is git-p4 written for an old version of p4 CLI with different > behavior? Or for a windows or mac release of p4? Or am I missing > something? I had not done any testing beyond p4 12.2 (linux). But running the unit tests through 13.1 just now, they all pass. $ p4 -V Perforce - The Fast Software Configuration Management System. Copyright 1995-2013 Perforce Software. All rights reserved. This product includes software developed by the OpenSSL Project for use in the OpenSSL Toolkit (http://www.openssl.org/) See 'p4 help legal' for full OpenSSL license information Version of OpenSSL Libraries: OpenSSL 1.0.1c 10 May 2012 Rev. P4/LINUX26X86_64/2013.1/610569 (2013/03/19). I'm using python 2.7.3. > First issue > --- > > git-p4 assumes the output of 'p4 print' adds a newline to the > target. To work around this, git-p4.py strips the last char from > symlinks as shown in the following snippet: > > if type_base =3D=3D "symlink": > git_mode =3D "12" > # p4 print on a symlink contains "target\n"; remove the newline > data =3D ''.join(contents) > contents =3D [data[:-1]] > > But my 'p4 print' does not output the newline: > > $ ls -l pcre > lrwxrwxrwx 1 user group 12 Apr 16 10:27 pcre -> ../libs/pcre > > $ p4 print -q pcre | od -t x1a > 000 2e 2e 2f 6c 69 62 73 2f 70 63 72 65 > . . / l i b s / p c r e > 014 > > If I use 'git p4 clone' the above file shows up in git as a > symlink to '../libs/pcr'. I had another symlink whose target had > a strlen of 1 and the 'git p4 clone' failed b/c after stripping > the last char the result was an empty string. There wasn't an explict test for symlinks, but I threw one together quickly and it seems to work. Can you show a bit more information about anything that potentially might be odd with your install? arf-git-test$ ls -l symlink lrwxrwxrwx 1 pw pw 14 Apr 18 20:02 symlink -> symlink-target $ p4 fstat symlink ... depotFile //depot/symlink ... clientFile /run/shm/trash directory.t9802-git-p4-filetype/cli/symlink ... isMapped ... headAction add ... headType symlink ... headTime 1366329740 ... headRev 1 ... headChange 6 ... headModTime 1366329740 ... haveRev 1 $ p4 print -q symlink | od -t x1a 000 73 79 6d 6c 69 6e 6b 2d 74 61 72 67 65 74 0a s y m l i n k - t a r g e t nl 017 No idea why I get an "nl" but you do not. If you run _without_ the "| od ...", then the shell prompt ends up on the same line as the output? Any interesting shell or shell settings? > Second issue > > > git-p4 uses 'p4 print -q -o o FILE' to print a file to stdout. > At least that is how I interpret this snippet: > > text = p4_read_pipe(['print', '-q', '-o', '-', file['depotFile']]) > > However, p4/Linux prints to stdout by default and '-o -' will save > the output in a file named '-'. > > My git and p4 versions: > > $ git --version > git version 1.8.2.1.418.gaec3f77 > > $ p4 -V > Perforce - The Fast Software Configuration Management System. > Copyright 1995-2013 Perforce Software. All rights reserved. > This product includes software developed by the OpenSSL Project > for use in the OpenSSL Toolkit (http://www.openssl.org/) > See 'p4 help legal' for full OpenSSL license information > Version of OpenSSL Libraries: OpenSSL 1.0.1c 10 May 2012 > Rev. P4/LINUX26X86_64/2013.1/610569 (2013/03/19). This code only happens on utf16 files. But running it by hand, I cannot reproduce the different behavior: $ p4 print -q //depot/f-ascii three line text $ p4 print -o - -q //depot/f-ascii three line $ ls ./- ls: cannot access ./-: No such file or directory I'm again confused. Any hints you can give would be helpful. -- Pete -- To unsubscribe from this list: send the line "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/2] submodule: drop the top-level requirement
On Thu, Apr 18, 2013 at 3:50 PM, John Keeping wrote: > Use the new rev-parse --prefix option to process all paths given to the > submodule command, dropping the requirement that it be run from the > top-level of the repository. > > Signed-off-by: John Keeping > --- > diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh > index ff26535..ca0a6ab 100755 > --- a/t/t7400-submodule-basic.sh > +++ b/t/t7400-submodule-basic.sh > @@ -212,6 +212,24 @@ test_expect_success 'submodule add with ./, /.. and // > in path' ' > test_cmp empty untracked > ' > > +test_expect_success 'submodule add in subdir' ' A particularly minor nit. Existing subdirectory-related tests in t7400 spell out "subdirectory" fully, so perhaps for consistency: s/subdir/subdirectory/ > + echo "refs/heads/master" >expect && > + >empty && > + > + mkdir addtest/sub && > + ( > + cd addtest/sub && > + git submodule add "$submodurl" ../realsubmod3 && > + git submodule init > + ) && > + > + rm -f heads head untracked && > + inspect addtest/realsubmod3 ../.. && > + test_cmp expect heads && > + test_cmp expect head && > + test_cmp empty untracked > +' > + > test_expect_success 'setup - add an example entry to .gitmodules' ' > GIT_CONFIG=.gitmodules \ > git config submodule.example.url git://example.com/init.git > @@ -319,6 +337,15 @@ test_expect_success 'status should be "up-to-date" after > update' ' > grep "^ $rev1" list > ' > > +test_expect_success 'status works correctly from a subdirectory' ' Good: "subdirectory" > + mkdir sub && > + ( > + cd sub && > + git submodule status >../list > + ) && > + grep "^ $rev1" list > +' > + > test_expect_success 'status should be "modified" after submodule commit' ' > ( > cd init && > diff --git a/t/t7401-submodule-summary.sh b/t/t7401-submodule-summary.sh > index 30b429e..992b66b 100755 > --- a/t/t7401-submodule-summary.sh > +++ b/t/t7401-submodule-summary.sh > @@ -45,6 +45,20 @@ EOF > test_cmp expected actual > " > > +test_expect_success 'run summary from subdir' ' t7401 does not have any existing subdirectory-related tests, but for consistency with t7400, perhaps: s/subdir/subdirectory/ > + mkdir sub && > + ( > + cd sub && > + git submodule summary >../actual > + ) && > + cat >expected <<-EOF && > +* ../sm1 000...$head1 (2): > + > Add foo2 > + > +EOF > + test_cmp expected actual > +' > + > commit_file sm1 && > head2=$(add_file sm1 foo3) -- To unsubscribe from this list: send the line "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 (Apr 2013, #05; Mon, 15)
On Thu, Apr 18, 2013 at 3:06 PM, Phil Hord wrote: > On Wed, Apr 17, 2013 at 2:50 PM, Felipe Contreras >> Yes please. Show me one of the instances where you hit a bisect with >> any of the remote-hg commits mentioned above by Thomas Rast. > > I made no such claim. In fact, I have never bisected to any > remote-hg-related commit. I fail to see the relevance of this > qualifier, though. Here, this is what you said: You: > Me: >> [skipping irrelevant comments] >> >> I'm sorry, did you actually hit an issue that required to look at the >> commit message to understand where the issue came from? No? Then I >> won't bother with hypotheticals. >> >> If you want to waste your time, by all means, rewrite all my commit >> messages with essays that nobody will ever read. I'm not going to do >> that for some hypothetical case that will never happen. I'm not going >> to waste my time. > > This is not a hypothetical. If something is not hypothetical, it's real, which means it actually happened, but then you said you never made the claim that it did. So what is it? Either it did happen, or it didn't; you cannot have your cake and eat it. If you are going to change your claims on the fly, and deny you ever made them, I don't see much point in discussing with you. Cheers. -- Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git p4 submit failing
christopher.yee...@gmail.com wrote on Thu, 18 Apr 2013 11:24 -0500: > The issue is caused by the line endings. I retested the problem with a > different file and in this case, the error is caused by the line > endings of the file checked out in the perforce workspace being > win-style crlf, and the line endings of the file in the git repo being > unix style lf. (In this scenario, I took out the .gitattributes, > core.autocrlf was set to false and LineEnd was set to share) > > In this case, I checked out the file in perforce, ran dos2unix against > it, and submitted that, then ran git p4 submit and it worked. > > I noticed that the error is caused by the git apply failing in the def > applyCommit(self, id) function at lines 1296-1305. > > diffcmd = "git format-patch -k --stdout \"%s^\"..\"%s\"" % (id, id) > patchcmd = diffcmd + " | git apply " > tryPatchCmd = patchcmd + "--check -" > applyPatchCmd = patchcmd + "--check --apply -" > patch_succeeded = True > > if os.system(tryPatchCmd) != 0: > fixed_rcs_keywords = False > patch_succeeded = False > print "Unfortunately applying the change failed!" > > So most likely in git apply command, it can't find the changes because > of the line endings being different between them. I couldn't find a > parameter that would magically make it work. When I added --verbose to > git apply the output only says: > error: while searching for: > That seems like exactly the correct diagnosis of the problem. What to do about it, I'm not so sure. We could suggest that people use the same line-ending conventions in both git and p4 land. This is easy if they are both lf. But, if crlf is preferred, do you know how to configure git to use crlf line endings? Does that fix it? There's also the config setting "apply.ignorewhitespace"; not sure if that would allow the apply step to apply an lf-ending patch to the crlf-ending p4 workspace. -- Pete > Hello Simon, > > I have CCed you to alert you to the possible bug. Any assistance would > be appreciated. > > > On Sat, Apr 13, 2013 at 5:09 PM, Christopher Yee Mon > wrote: > > Yes this is the case. > > > > Many of the files have crlf endings. > > > > core.autocrlf was recently set to input. I can't remember the timeline > > exactly though, but in addition to this, I have a .gitattributes file > > with the default set to text=auto (* text=auto) and the php files set > > to text eol=lf (*.php text eol=lf) Also my perforce workspace's > > LineEnd setting is set to Share. > > > > I've experienced the behavior in both .php and .xml files though > > > > Before all of this started I had core.autocrlf set to false, and no > > .gitattributes file and perforce workspace's LineEnd was set to the > > default, but I got a conflict where the only difference was the line > > endings, so I changed things to the way they are now. > > > > Any recommendations? Should I change everything back the way it was? > > > > On Sat, Apr 13, 2013 at 5:51 PM, Pete Wyckoff wrote: > >> l...@diamand.org wrote on Thu, 11 Apr 2013 21:19 +0100: > >>> Just a thought, but check the files that are failing to see if they've > >>> got RCS keywords in them ($Id$, $File$, $Date$, etc). These cause all > >>> sorts of nasty problems. > >>> > >>> That's assuming it's definitely not a CRLF line ending problem on Windows. > >> > >> I had recently debugged a similar-looking problem > >> where core.autocrlf was set to "input". > >> > >> Christopher, if you have this set and/or the .xml files > >> have ^M (CRLF) line endings, please let us know. > >> > >> -- Pete > >> > >>> > >>> On Thu, Apr 11, 2013 at 8:01 PM, Christopher Yee Mon > >>> wrote: > >>> > I tried running git p4 submit on a repo that I've been running as an > >>> > interim bridge between git and perforce. Multiple people are using the > >>> > repo as a remote and its being periodically submitted back to > >>> > perforce. > >>> > > >>> > It's been working mostly fine. Then one day out of the blue I get this > >>> > error. I can no longer push any git commits to perforce. (This is from > >>> > the remote repo which I am pushing back to perforce) > >>> > > >>> > user@hostname:~/Source/code$ git p4 submit -M --export-labels > >>> > Perforce checkout for depot path //depot/perforce/workspace/ located > >>> > at /home/user/Source/git-p4-area/perforce/workspace/ > >>> > Synchronizing p4 checkout... > >>> > ... - file(s) up-to-date. > >>> > Applying ffa390f comments in config xml files > >>> > //depot/perforce/workspace/sub/folder/structure/first.xml#3 - opened > >>> > for edit > >>> > //depot/perforce/workspace/sub/folder/structure/second.xml#3 - opened > >>> > for edit > >>> > //depot/perforce/workspace/sub/folder/structure/third.xml#3 - opened > >>> > for edit > >>> > //depot/perforce/workspace/sub/folder/structure/forth.xml#3 - opened > >>> > for edit > >>> > //depot/perforce/workspace/sub/folder/st
[PATCH v4 13/13] pretty: support %>> that steal trailing spaces
This is pretty useful in `%<(100)%s%Cred%>(20)% an' where %s does not
use up all 100 columns and %an needs more than 20 columns. By
replacing %>(20) with %>>(20), %an can steal spaces from %s.
%>> understands escape sequences, so %Cred does not stop it from
stealing spaces in %<(100).
Signed-off-by: Nguyễn Thái Ngọc Duy
---
Documentation/pretty-formats.txt | 5 -
pretty.c | 34 ++
t/t4205-log-pretty-formats.sh| 14 ++
utf8.c | 2 +-
utf8.h | 1 +
5 files changed, 54 insertions(+), 2 deletions(-)
diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index d3450bf..c96ff41 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -173,7 +173,10 @@ The placeholders are:
columns, padding spaces on the right if necessary
- '%>()', '%>|()': similar to '%<()', '%<|()'
respectively, but padding spaces on the left
-- '%><()', '%><|()': similar to '%<()', '%<|()'
+- '%>>()', '%>>|()': similar to '%>()', '%>|()'
+ respectively, except that if the next placeholder takes more spaces
+ than given and there are spaces on its left, use those spaces
+- '%><()', '%><|()': similar to '% <()', '%<|()'
respectively, but padding both sides (i.e. the text is centered)
NOTE: Some placeholders may depend on other options given to the
diff --git a/pretty.c b/pretty.c
index f7c6442..d59579a 100644
--- a/pretty.c
+++ b/pretty.c
@@ -773,6 +773,7 @@ enum flush_type {
no_flush,
flush_right,
flush_left,
+ flush_left_and_steal,
flush_both
};
@@ -1026,6 +1027,9 @@ static size_t parse_padding_placeholder(struct strbuf *sb,
if (*ch == '<') {
flush_type = flush_both;
ch++;
+ } else if (*ch == '>') {
+ flush_type = flush_left_and_steal;
+ ch++;
} else
flush_type = flush_left;
break;
@@ -1334,6 +1338,36 @@ static size_t format_and_pad_commit(struct strbuf *sb,
/* in UTF-8 */
total_consumed++;
}
len = utf8_strnwidth(local_sb.buf, -1, 1);
+
+ if (c->flush_type == flush_left_and_steal) {
+ const char *ch = sb->buf + sb->len - 1;
+ while (len > padding && ch > sb->buf) {
+ const char *p;
+ if (*ch == ' ') {
+ ch--;
+ padding++;
+ continue;
+ }
+ /* check for trailing ansi sequences */
+ if (*ch != 'm')
+ break;
+ p = ch - 1;
+ while (ch - p < 10 && *p != '\033')
+ p--;
+ if (*p != '\033' ||
+ ch + 1 - p != display_mode_esc_sequence_len(p))
+ break;
+ /*
+* got a good ansi sequence, put it back to
+* local_sb as we're cutting sb
+*/
+ strbuf_insert(&local_sb, 0, p, ch + 1 - p);
+ ch = p - 1;
+ }
+ strbuf_setlen(sb, ch + 1 - sb->buf);
+ c->flush_type = flush_left;
+ }
+
if (len > padding) {
switch (c->truncate) {
case trunc_left:
diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh
index b8685b9..26fbfde 100755
--- a/t/t4205-log-pretty-formats.sh
+++ b/t/t4205-log-pretty-formats.sh
@@ -260,4 +260,18 @@ EOF
test_cmp expected actual
'
+test_expect_success 'left/right alignment formatting with stealing' '
+ git commit --amend -m short --author "long long long " &&
+ git log --pretty="format:%<(10,trunc)%s%>>(10,ltrunc)% an" >actual &&
+ # complete the incomplete line at the end
+ echo >>actual &&
+ cat <<\EOF >expected &&
+short long long long
+message .. A U Thor
+add bar A U Thor
+initial A U Thor
+EOF
+ test_cmp expected actual
+'
+
test_done
diff --git a/utf8.c b/utf8.c
index 197414a..b1e1303 100644
--- a/utf8.c
+++ b/utf8.c
@@ -9,7 +9,7 @@ struct interval {
int last;
};
-static size_t display_mode_esc_sequence_len(const char *s)
+size_t display_mode_esc_sequence_len(const char *s)
{
const char *p = s;
if (*p++ != '\033')
diff --git a/utf8.h b/utf8.h
index edde8ee..32a7bfb 100644
--- a/utf8.h
+++ b/utf8.h
@@ -3,6 +3,7 @@
typedef unsigned int ucs_char_t; /* assuming 32bit int */
+size_t display_mode_esc_sequence_len(const char *s);
int utf8_width(const char **start, size_t *remainder_p);
int utf8_strnwidth(const char *string, int len, int skip_ansi);
int ut
[PATCH v4 12/13] pretty: support truncating in %>, %< and %>
%>(N,trunc) truncates the right part after N columns and replace the
last two letters with "..". ltrunc does the same on the left. mtrunc
cuts the middle out.
Signed-off-by: Nguyễn Thái Ngọc Duy
---
Documentation/pretty-formats.txt | 7 --
pretty.c | 51 +---
t/t4205-log-pretty-formats.sh| 39 ++
utf8.c | 46
utf8.h | 2 ++
5 files changed, 140 insertions(+), 5 deletions(-)
diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index e2345d2..d3450bf 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -164,8 +164,11 @@ The placeholders are:
- '%x00': print a byte from a hex code
- '%w([[,[,]]])': switch line wrapping, like the -w option of
linkgit:git-shortlog[1].
-- '%<()': make the next placeholder take at least N columns,
- padding spaces on the right if necessary
+- '%<([,trunc|ltrunc|mtrunc])': make the next placeholder take at
+ least N columns, padding spaces on the right if necessary.
+ Optionally truncate at the beginning (ltrunc), the middle (mtrunc)
+ or the end (trunc) if the output is longer than N columns.
+ Note that truncating only works correctly with N >= 2.
- '%<|()': make the next placeholder take at least until Nth
columns, padding spaces on the right if necessary
- '%>()', '%>|()': similar to '%<()', '%<|()'
diff --git a/pretty.c b/pretty.c
index b50ebf5..f7c6442 100644
--- a/pretty.c
+++ b/pretty.c
@@ -776,6 +776,13 @@ enum flush_type {
flush_both
};
+enum trunc_type {
+ trunc_none,
+ trunc_left,
+ trunc_middle,
+ trunc_right
+};
+
struct format_commit_context {
const struct commit *commit;
const struct pretty_print_context *pretty_ctx;
@@ -783,6 +790,7 @@ struct format_commit_context {
unsigned commit_message_parsed:1;
struct signature_check signature_check;
enum flush_type flush_type;
+ enum trunc_type truncate;
char *message;
char *commit_encoding;
size_t width, indent1, indent2;
@@ -1033,7 +1041,7 @@ static size_t parse_padding_placeholder(struct strbuf *sb,
if (*ch == '(') {
const char *start = ch + 1;
- const char *end = strchr(start, ')');
+ const char *end = start + strcspn(start, ",)");
char *next;
int width;
if (!end || end == start)
@@ -1043,6 +1051,23 @@ static size_t parse_padding_placeholder(struct strbuf
*sb,
return 0;
c->padding = to_column ? -width : width;
c->flush_type = flush_type;
+
+ if (*end == ',') {
+ start = end + 1;
+ end = strchr(start, ')');
+ if (!end || end == start)
+ return 0;
+ if (!prefixcmp(start, "trunc)"))
+ c->truncate = trunc_right;
+ else if (!prefixcmp(start, "ltrunc)"))
+ c->truncate = trunc_left;
+ else if (!prefixcmp(start, "mtrunc)"))
+ c->truncate = trunc_middle;
+ else
+ return 0;
+ } else
+ c->truncate = trunc_none;
+
return end - placeholder + 1;
}
return 0;
@@ -1309,9 +1334,29 @@ static size_t format_and_pad_commit(struct strbuf *sb,
/* in UTF-8 */
total_consumed++;
}
len = utf8_strnwidth(local_sb.buf, -1, 1);
- if (len > padding)
+ if (len > padding) {
+ switch (c->truncate) {
+ case trunc_left:
+ strbuf_utf8_replace(&local_sb,
+ 0, len - (padding - 2),
+ "..");
+ break;
+ case trunc_middle:
+ strbuf_utf8_replace(&local_sb,
+ padding / 2 - 1,
+ len - (padding - 2),
+ "..");
+ break;
+ case trunc_right:
+ strbuf_utf8_replace(&local_sb,
+ padding - 2, len - (padding - 2),
+ "..");
+ break;
+ case trunc_none:
+ break;
+ }
strbuf_addstr(sb, local_sb.buf);
- else {
+ } else {
int sb_len = sb->len, offset = 0;
if (c->flush_type == flush_left)
offset = padding - len;
diff --git a/t/t4205-lo
[PATCH v4 11/13] pretty: support padding placeholders, %< %> and %>
Either %<, %> or %>< standing before a placeholder specifies how many
columns (at least as the placeholder can exceed it) it takes. Each
differs on how spaces are padded:
%< pads on the right (aka left alignment)
%> pads on the left (aka right alignment)
%>< pads both ways equally (aka centered)
The () follows them, e.g. `%<(100)', to specify the number of
columns the next placeholder takes.
However, if '|' stands before (), e.g. `%>|(100)', then the number
of columns is calculated so that it reaches the Nth column on screen.
Signed-off-by: Nguyễn Thái Ngọc Duy
---
Documentation/pretty-formats.txt | 8 +++
pretty.c | 117 -
t/t4205-log-pretty-formats.sh| 122 +++
3 files changed, 246 insertions(+), 1 deletion(-)
diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index bad627a..e2345d2 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -164,6 +164,14 @@ The placeholders are:
- '%x00': print a byte from a hex code
- '%w([[,[,]]])': switch line wrapping, like the -w option of
linkgit:git-shortlog[1].
+- '%<()': make the next placeholder take at least N columns,
+ padding spaces on the right if necessary
+- '%<|()': make the next placeholder take at least until Nth
+ columns, padding spaces on the right if necessary
+- '%>()', '%>|()': similar to '%<()', '%<|()'
+ respectively, but padding spaces on the left
+- '%><()', '%><|()': similar to '%<()', '%<|()'
+ respectively, but padding both sides (i.e. the text is centered)
NOTE: Some placeholders may depend on other options given to the
revision traversal engine. For example, the `%g*` reflog options will
diff --git a/pretty.c b/pretty.c
index 3612f82..b50ebf5 100644
--- a/pretty.c
+++ b/pretty.c
@@ -769,16 +769,25 @@ struct chunk {
size_t len;
};
+enum flush_type {
+ no_flush,
+ flush_right,
+ flush_left,
+ flush_both
+};
+
struct format_commit_context {
const struct commit *commit;
const struct pretty_print_context *pretty_ctx;
unsigned commit_header_parsed:1;
unsigned commit_message_parsed:1;
struct signature_check signature_check;
+ enum flush_type flush_type;
char *message;
char *commit_encoding;
size_t width, indent1, indent2;
int auto_color;
+ int padding;
/* These offsets are relative to the start of the commit message. */
struct chunk author;
@@ -993,6 +1002,52 @@ static size_t parse_color(struct strbuf *sb, /* in UTF-8
*/
return 0;
}
+static size_t parse_padding_placeholder(struct strbuf *sb,
+ const char *placeholder,
+ struct format_commit_context *c)
+{
+ const char *ch = placeholder;
+ enum flush_type flush_type;
+ int to_column = 0;
+
+ switch (*ch++) {
+ case '<':
+ flush_type = flush_right;
+ break;
+ case '>':
+ if (*ch == '<') {
+ flush_type = flush_both;
+ ch++;
+ } else
+ flush_type = flush_left;
+ break;
+ default:
+ return 0;
+ }
+
+ /* the next value means "wide enough to that column" */
+ if (*ch == '|') {
+ to_column = 1;
+ ch++;
+ }
+
+ if (*ch == '(') {
+ const char *start = ch + 1;
+ const char *end = strchr(start, ')');
+ char *next;
+ int width;
+ if (!end || end == start)
+ return 0;
+ width = strtoul(start, &next, 10);
+ if (next == start || width == 0)
+ return 0;
+ c->padding = to_column ? -width : width;
+ c->flush_type = flush_type;
+ return end - placeholder + 1;
+ }
+ return 0;
+}
+
static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
const char *placeholder,
void *context)
@@ -1057,6 +1112,10 @@ static size_t format_commit_one(struct strbuf *sb, /* in
UTF-8 */
return end - placeholder + 1;
} else
return 0;
+
+ case '<':
+ case '>':
+ return parse_padding_placeholder(sb, placeholder, c);
}
/* these depend on the commit */
@@ -1221,6 +1280,59 @@ static size_t format_commit_one(struct strbuf *sb, /* in
UTF-8 */
return 0; /* unknown placeholder */
}
+static size_t format_and_pad_commit(struct strbuf *sb, /* in UTF-8 */
+ const char *placeholder,
+ struct format_commit_context *c)
+{
+ struct st
[PATCH v4 10/13] pretty: add %C(auto) for auto-coloring
This is not simply convenient over %C(auto,xxx). Some placeholders
(actually only one, %d) do multi coloring and we can't emit a multiple
colors with %C(auto,xxx).
Signed-off-by: Nguyễn Thái Ngọc Duy
---
Documentation/pretty-formats.txt | 3 ++-
pretty.c | 26 +++---
2 files changed, 25 insertions(+), 4 deletions(-)
diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index 6bde67e..bad627a 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -156,7 +156,8 @@ The placeholders are:
adding `auto,` at the beginning will emit color only when colors are
enabled for log output (by `color.diff`, `color.ui`, or `--color`, and
respecting the `auto` settings of the former if we are going to a
- terminal)
+ terminal). `auto` alone (i.e. `%C(auto)`) will turn on auto coloring
+ on the next placeholders until the color is switched again.
- '%m': left, right or boundary mark
- '%n': newline
- '%%': a raw '%'
diff --git a/pretty.c b/pretty.c
index e0413e3..3612f82 100644
--- a/pretty.c
+++ b/pretty.c
@@ -778,6 +778,7 @@ struct format_commit_context {
char *message;
char *commit_encoding;
size_t width, indent1, indent2;
+ int auto_color;
/* These offsets are relative to the start of the commit message. */
struct chunk author;
@@ -1005,7 +1006,20 @@ static size_t format_commit_one(struct strbuf *sb, /* in
UTF-8 */
/* these are independent of the commit */
switch (placeholder[0]) {
case 'C':
- return parse_color(sb, placeholder, c);
+ if (!prefixcmp(placeholder + 1, "(auto)")) {
+ c->auto_color = 1;
+ return 7; /* consumed 7 bytes, "C(auto)" */
+ } else {
+ int ret = parse_color(sb, placeholder, c);
+ if (ret)
+ c->auto_color = 0;
+ /*
+* Otherwise, we decided to treat %C
+* as a literal string, and the previous
+* %C(auto) is still valid.
+*/
+ return ret;
+ }
case 'n': /* newline */
strbuf_addch(sb, '\n');
return 1;
@@ -1051,13 +1065,19 @@ static size_t format_commit_one(struct strbuf *sb, /*
in UTF-8 */
switch (placeholder[0]) {
case 'H': /* commit hash */
+ strbuf_addstr(sb, diff_get_color(c->auto_color, DIFF_COMMIT));
strbuf_addstr(sb, sha1_to_hex(commit->object.sha1));
+ strbuf_addstr(sb, diff_get_color(c->auto_color, DIFF_RESET));
return 1;
case 'h': /* abbreviated commit hash */
- if (add_again(sb, &c->abbrev_commit_hash))
+ strbuf_addstr(sb, diff_get_color(c->auto_color, DIFF_COMMIT));
+ if (add_again(sb, &c->abbrev_commit_hash)) {
+ strbuf_addstr(sb, diff_get_color(c->auto_color,
DIFF_RESET));
return 1;
+ }
strbuf_addstr(sb, find_unique_abbrev(commit->object.sha1,
c->pretty_ctx->abbrev));
+ strbuf_addstr(sb, diff_get_color(c->auto_color, DIFF_RESET));
c->abbrev_commit_hash.len = sb->len - c->abbrev_commit_hash.off;
return 1;
case 'T': /* tree hash */
@@ -1095,7 +1115,7 @@ static size_t format_commit_one(struct strbuf *sb, /* in
UTF-8 */
return 1;
case 'd':
load_ref_decorations(DECORATE_SHORT_REFS);
- format_decorations(sb, commit, 0);
+ format_decorations(sb, commit, c->auto_color);
return 1;
case 'g': /* reflog info */
switch(placeholder[1]) {
--
1.8.2.82.gc24b958
--
To unsubscribe from this list: send the line "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 v4 09/13] pretty: split color parsing into a separate function
Signed-off-by: Nguyễn Thái Ngọc Duy
---
pretty.c | 71 +++-
1 file changed, 39 insertions(+), 32 deletions(-)
diff --git a/pretty.c b/pretty.c
index 5947275..e0413e3 100644
--- a/pretty.c
+++ b/pretty.c
@@ -954,6 +954,44 @@ static int format_reflog_person(struct strbuf *sb,
return format_person_part(sb, part, ident, strlen(ident), dmode);
}
+static size_t parse_color(struct strbuf *sb, /* in UTF-8 */
+ const char *placeholder,
+ struct format_commit_context *c)
+{
+ if (placeholder[1] == '(') {
+ const char *begin = placeholder + 2;
+ const char *end = strchr(begin, ')');
+ char color[COLOR_MAXLEN];
+
+ if (!end)
+ return 0;
+ if (!prefixcmp(begin, "auto,")) {
+ if (!want_color(c->pretty_ctx->color))
+ return end - placeholder + 1;
+ begin += 5;
+ }
+ color_parse_mem(begin,
+ end - begin,
+ "--pretty format", color);
+ strbuf_addstr(sb, color);
+ return end - placeholder + 1;
+ }
+ if (!prefixcmp(placeholder + 1, "red")) {
+ strbuf_addstr(sb, GIT_COLOR_RED);
+ return 4;
+ } else if (!prefixcmp(placeholder + 1, "green")) {
+ strbuf_addstr(sb, GIT_COLOR_GREEN);
+ return 6;
+ } else if (!prefixcmp(placeholder + 1, "blue")) {
+ strbuf_addstr(sb, GIT_COLOR_BLUE);
+ return 5;
+ } else if (!prefixcmp(placeholder + 1, "reset")) {
+ strbuf_addstr(sb, GIT_COLOR_RESET);
+ return 6;
+ } else
+ return 0;
+}
+
static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
const char *placeholder,
void *context)
@@ -967,38 +1005,7 @@ static size_t format_commit_one(struct strbuf *sb, /* in
UTF-8 */
/* these are independent of the commit */
switch (placeholder[0]) {
case 'C':
- if (placeholder[1] == '(') {
- const char *begin = placeholder + 2;
- const char *end = strchr(begin, ')');
- char color[COLOR_MAXLEN];
-
- if (!end)
- return 0;
- if (!prefixcmp(begin, "auto,")) {
- if (!want_color(c->pretty_ctx->color))
- return end - placeholder + 1;
- begin += 5;
- }
- color_parse_mem(begin,
- end - begin,
- "--pretty format", color);
- strbuf_addstr(sb, color);
- return end - placeholder + 1;
- }
- if (!prefixcmp(placeholder + 1, "red")) {
- strbuf_addstr(sb, GIT_COLOR_RED);
- return 4;
- } else if (!prefixcmp(placeholder + 1, "green")) {
- strbuf_addstr(sb, GIT_COLOR_GREEN);
- return 6;
- } else if (!prefixcmp(placeholder + 1, "blue")) {
- strbuf_addstr(sb, GIT_COLOR_BLUE);
- return 5;
- } else if (!prefixcmp(placeholder + 1, "reset")) {
- strbuf_addstr(sb, GIT_COLOR_RESET);
- return 6;
- } else
- return 0;
+ return parse_color(sb, placeholder, c);
case 'n': /* newline */
strbuf_addch(sb, '\n');
return 1;
--
1.8.2.82.gc24b958
--
To unsubscribe from this list: send the line "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 v4 08/13] pretty: two phase conversion for non utf-8 commits
Always assume format_commit_item() takes an utf-8 string for string
handling simplicity (we can handle utf-8 strings, but can't with other
encodings).
If commit message is in non-utf8, or output encoding is not, then the
commit is first converted to utf-8, processed, then output converted
to output encoding. This of course only works with encodings that are
compatible with Unicode.
This also fixes the iso8859-1 test in t6006. It's supposed to create
an iso8859-1 commit, but the commit content in t6006 is in UTF-8.
t6006 is now converted back in UTF-8 (the downside is we can't put
utf-8 strings there anymore).
Signed-off-by: Nguyễn Thái Ngọc Duy
---
pretty.c | 24 ++--
t/t6006-rev-list-format.sh | 12 ++--
2 files changed, 28 insertions(+), 8 deletions(-)
diff --git a/pretty.c b/pretty.c
index e0f93ba..5947275 100644
--- a/pretty.c
+++ b/pretty.c
@@ -954,7 +954,8 @@ static int format_reflog_person(struct strbuf *sb,
return format_person_part(sb, part, ident, strlen(ident), dmode);
}
-static size_t format_commit_one(struct strbuf *sb, const char *placeholder,
+static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
+ const char *placeholder,
void *context)
{
struct format_commit_context *c = context;
@@ -1193,7 +1194,8 @@ static size_t format_commit_one(struct strbuf *sb, const
char *placeholder,
return 0; /* unknown placeholder */
}
-static size_t format_commit_item(struct strbuf *sb, const char *placeholder,
+static size_t format_commit_item(struct strbuf *sb, /* in UTF-8 */
+const char *placeholder,
void *context)
{
int consumed;
@@ -1273,6 +1275,7 @@ void format_commit_message(const struct commit *commit,
{
struct format_commit_context context;
const char *output_enc = pretty_ctx->output_encoding;
+ const char *utf8 = "UTF-8";
memset(&context, 0, sizeof(context));
context.commit = commit;
@@ -1285,6 +1288,23 @@ void format_commit_message(const struct commit *commit,
strbuf_expand(sb, format, format_commit_item, &context);
rewrap_message_tail(sb, &context, 0, 0, 0);
+ if (output_enc) {
+ if (same_encoding(utf8, output_enc))
+ output_enc = NULL;
+ } else {
+ if (context.commit_encoding &&
+ !same_encoding(context.commit_encoding, utf8))
+ output_enc = context.commit_encoding;
+ }
+
+ if (output_enc) {
+ int outsz;
+ char *out = reencode_string_len(sb->buf, sb->len,
+ output_enc, utf8, &outsz);
+ if (out)
+ strbuf_attach(sb, out, outsz, outsz + 1);
+ }
+
free(context.commit_encoding);
logmsg_free(context.message, commit);
free(context.signature_check.gpg_output);
diff --git a/t/t6006-rev-list-format.sh b/t/t6006-rev-list-format.sh
index 3fc3b74..0393c9f 100755
--- a/t/t6006-rev-list-format.sh
+++ b/t/t6006-rev-list-format.sh
@@ -184,7 +184,7 @@ Test printing of complex bodies
This commit message is much longer than the others,
and it will be encoded in iso8859-1. We should therefore
-include an iso8859 character: ¡bueno!
+include an iso8859 character: �bueno!
EOF
test_expect_success 'setup complex body' '
git config i18n.commitencoding iso8859-1 &&
@@ -192,14 +192,14 @@ git config i18n.commitencoding iso8859-1 &&
'
test_format complex-encoding %e <<'EOF'
-commit f58db70b055c5718631e5c61528b28b12090cdea
+commit 1ed88da4a5b5ed8c449114ac131efc62178734c3
iso8859-1
commit 131a310eb913d107dd3c09a65d1651175898735d
commit 86c75cfd708a0e5868dc876ed5b8bb66c80b4873
EOF
test_format complex-subject %s <<'EOF'
-commit f58db70b055c5718631e5c61528b28b12090cdea
+commit 1ed88da4a5b5ed8c449114ac131efc62178734c3
Test printing of complex bodies
commit 131a310eb913d107dd3c09a65d1651175898735d
changed foo
@@ -208,17 +208,17 @@ added foo
EOF
test_format complex-body %b <<'EOF'
-commit f58db70b055c5718631e5c61528b28b12090cdea
+commit 1ed88da4a5b5ed8c449114ac131efc62178734c3
This commit message is much longer than the others,
and it will be encoded in iso8859-1. We should therefore
-include an iso8859 character: ¡bueno!
+include an iso8859 character: �bueno!
commit 131a310eb913d107dd3c09a65d1651175898735d
commit 86c75cfd708a0e5868dc876ed5b8bb66c80b4873
EOF
test_expect_success '%x00 shows NUL' '
- echo >expect commit f58db70b055c5718631e5c61528b28b12090cdea &&
+ echo >expect commit 1ed88da4a5b5ed8c449114ac131efc62178734c3 &&
echo >>expect fooQbar &&
git rev-list -1 --format=foo%x00bar HEAD >actual.nul &&
nul_to_q actual &&
--
1.8.2.82.gc24b958
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body o
[PATCH v4 07/13] utf8.c: add reencode_string_len() that can handle NULs in string
Signed-off-by: Nguyễn Thái Ngọc Duy
---
compat/precompose_utf8.c | 2 +-
utf8.c | 10 +++---
utf8.h | 19 ---
3 files changed, 24 insertions(+), 7 deletions(-)
diff --git a/compat/precompose_utf8.c b/compat/precompose_utf8.c
index 8cf5955..d9203d0 100644
--- a/compat/precompose_utf8.c
+++ b/compat/precompose_utf8.c
@@ -78,7 +78,7 @@ void precompose_argv(int argc, const char **argv)
size_t namelen;
oldarg = argv[i];
if (has_non_ascii(oldarg, (size_t)-1, &namelen)) {
- newarg = reencode_string_iconv(oldarg, namelen,
ic_precompose);
+ newarg = reencode_string_iconv(oldarg, namelen,
ic_precompose, NULL);
if (newarg)
argv[i] = newarg;
}
diff --git a/utf8.c b/utf8.c
index e7ba33c..7c342ff 100644
--- a/utf8.c
+++ b/utf8.c
@@ -468,7 +468,7 @@ int utf8_fprintf(FILE *stream, const char *format, ...)
#else
typedef char * iconv_ibp;
#endif
-char *reencode_string_iconv(const char *in, size_t insz, iconv_t conv)
+char *reencode_string_iconv(const char *in, size_t insz, iconv_t conv, int
*outsz_p)
{
size_t outsz, outalloc;
char *out, *outpos;
@@ -502,13 +502,17 @@ char *reencode_string_iconv(const char *in, size_t insz,
iconv_t conv)
}
else {
*outpos = '\0';
+ if (outsz_p)
+ *outsz_p = outpos - out;
break;
}
}
return out;
}
-char *reencode_string(const char *in, const char *out_encoding, const char
*in_encoding)
+char *reencode_string_len(const char *in, int insz,
+ const char *out_encoding, const char *in_encoding,
+ int *outsz)
{
iconv_t conv;
char *out;
@@ -534,7 +538,7 @@ char *reencode_string(const char *in, const char
*out_encoding, const char *in_e
return NULL;
}
- out = reencode_string_iconv(in, strlen(in), conv);
+ out = reencode_string_iconv(in, insz, conv, outsz);
iconv_close(conv);
return out;
}
diff --git a/utf8.h b/utf8.h
index d3da96f..a43ef9a 100644
--- a/utf8.h
+++ b/utf8.h
@@ -17,12 +17,25 @@ void strbuf_add_wrapped_bytes(struct strbuf *buf, const
char *data, int len,
int indent, int indent2, int width);
#ifndef NO_ICONV
-char *reencode_string_iconv(const char *in, size_t insz, iconv_t conv);
-char *reencode_string(const char *in, const char *out_encoding, const char
*in_encoding);
+char *reencode_string_iconv(const char *in, size_t insz,
+ iconv_t conv, int *outsz);
+char *reencode_string_len(const char *in, int insz,
+ const char *out_encoding,
+ const char *in_encoding,
+ int *outsz);
#else
-#define reencode_string(a,b,c) NULL
+#define reencode_string_len(a,b,c,d,e) NULL
#endif
+static inline char *reencode_string(const char *in,
+ const char *out_encoding,
+ const char *in_encoding)
+{
+ return reencode_string_len(in, strlen(in),
+ out_encoding, in_encoding,
+ NULL);
+}
+
int mbs_chrlen(const char **text, size_t *remainder_p, const char *encoding);
#endif
--
1.8.2.82.gc24b958
--
To unsubscribe from this list: send the line "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 v4 05/13] utf8.c: move display_mode_esc_sequence_len() for use by other functions
Signed-off-by: Nguyễn Thái Ngọc Duy
---
utf8.c | 28 ++--
1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/utf8.c b/utf8.c
index 7f64857..6ed93c3 100644
--- a/utf8.c
+++ b/utf8.c
@@ -9,6 +9,20 @@ struct interval {
int last;
};
+static size_t display_mode_esc_sequence_len(const char *s)
+{
+ const char *p = s;
+ if (*p++ != '\033')
+ return 0;
+ if (*p++ != '[')
+ return 0;
+ while (isdigit(*p) || *p == ';')
+ p++;
+ if (*p++ != 'm')
+ return 0;
+ return p - s;
+}
+
/* auxiliary function for binary search in interval table */
static int bisearch(ucs_char_t ucs, const struct interval *table, int max)
{
@@ -303,20 +317,6 @@ static void strbuf_add_indented_text(struct strbuf *buf,
const char *text,
}
}
-static size_t display_mode_esc_sequence_len(const char *s)
-{
- const char *p = s;
- if (*p++ != '\033')
- return 0;
- if (*p++ != '[')
- return 0;
- while (isdigit(*p) || *p == ';')
- p++;
- if (*p++ != 'm')
- return 0;
- return p - s;
-}
-
/*
* Wrap the text, if necessary. The variable indent is the indent for the
* first line, indent2 is the indent for all other lines.
--
1.8.2.82.gc24b958
--
To unsubscribe from this list: send the line "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 v4 06/13] utf8.c: add utf8_strnwidth() with the ability to skip ansi sequences
Signed-off-by: Nguyễn Thái Ngọc Duy
---
utf8.c | 20 ++--
utf8.h | 1 +
2 files changed, 15 insertions(+), 6 deletions(-)
diff --git a/utf8.c b/utf8.c
index 6ed93c3..e7ba33c 100644
--- a/utf8.c
+++ b/utf8.c
@@ -266,18 +266,26 @@ int utf8_width(const char **start, size_t *remainder_p)
* string, assuming that the string is utf8. Returns strlen() instead
* if the string does not look like a valid utf8 string.
*/
-int utf8_strwidth(const char *string)
+int utf8_strnwidth(const char *string, int len, int skip_ansi)
{
int width = 0;
const char *orig = string;
- while (1) {
- if (!string)
- return strlen(orig);
- if (!*string)
- return width;
+ if (len == -1)
+ len = strlen(string);
+ while (string && string < orig + len) {
+ int skip;
+ while (skip_ansi &&
+ (skip = display_mode_esc_sequence_len(string)) != 0)
+ string += skip;
width += utf8_width(&string, NULL);
}
+ return string ? width : len;
+}
+
+int utf8_strwidth(const char *string)
+{
+ return utf8_strnwidth(string, -1, 0);
}
int is_utf8(const char *text)
diff --git a/utf8.h b/utf8.h
index 1f8ecad..d3da96f 100644
--- a/utf8.h
+++ b/utf8.h
@@ -4,6 +4,7 @@
typedef unsigned int ucs_char_t; /* assuming 32bit int */
int utf8_width(const char **start, size_t *remainder_p);
+int utf8_strnwidth(const char *string, int len, int skip_ansi);
int utf8_strwidth(const char *string);
int is_utf8(const char *text);
int is_encoding_utf8(const char *name);
--
1.8.2.82.gc24b958
--
To unsubscribe from this list: send the line "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 v4 04/13] pretty: share code between format_decoration and show_decorations
This also adds color support to format_decorations()
Signed-off-by: Nguyễn Thái Ngọc Duy
---
log-tree.c | 48 ++--
log-tree.h | 1 +
pretty.c | 20 ++---
t/t4207-log-decoration-colors.sh | 8 +++
4 files changed, 39 insertions(+), 38 deletions(-)
diff --git a/log-tree.c b/log-tree.c
index 7cc7d59..1946e9c 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -175,36 +175,52 @@ static void show_children(struct rev_info *opt, struct
commit *commit, int abbre
}
}
-void show_decorations(struct rev_info *opt, struct commit *commit)
+/*
+ * The caller makes sure there is no funny color before
+ * calling. format_decorations makes sure the same after return.
+ */
+void format_decorations(struct strbuf *sb,
+ const struct commit *commit,
+ int use_color)
{
const char *prefix;
struct name_decoration *decoration;
const char *color_commit =
- diff_get_color_opt(&opt->diffopt, DIFF_COMMIT);
+ diff_get_color(use_color, DIFF_COMMIT);
const char *color_reset =
- decorate_get_color_opt(&opt->diffopt, DECORATION_NONE);
+ decorate_get_color(use_color, DECORATION_NONE);
- if (opt->show_source && commit->util)
- printf("\t%s", (char *) commit->util);
- if (!opt->show_decorations)
- return;
decoration = lookup_decoration(&name_decoration, &commit->object);
if (!decoration)
return;
prefix = " (";
while (decoration) {
- printf("%s", prefix);
- fputs(decorate_get_color_opt(&opt->diffopt, decoration->type),
- stdout);
+ strbuf_addstr(sb, color_commit);
+ strbuf_addstr(sb, prefix);
+ strbuf_addstr(sb, decorate_get_color(use_color,
decoration->type));
if (decoration->type == DECORATION_REF_TAG)
- fputs("tag: ", stdout);
- printf("%s", decoration->name);
- fputs(color_reset, stdout);
- fputs(color_commit, stdout);
+ strbuf_addstr(sb, "tag: ");
+ strbuf_addstr(sb, decoration->name);
+ strbuf_addstr(sb, color_reset);
prefix = ", ";
decoration = decoration->next;
}
- putchar(')');
+ strbuf_addstr(sb, color_commit);
+ strbuf_addch(sb, ')');
+ strbuf_addstr(sb, color_reset);
+}
+
+void show_decorations(struct rev_info *opt, struct commit *commit)
+{
+ struct strbuf sb = STRBUF_INIT;
+
+ if (opt->show_source && commit->util)
+ printf("\t%s", (char *) commit->util);
+ if (!opt->show_decorations)
+ return;
+ format_decorations(&sb, commit, opt->diffopt.use_color);
+ fputs(sb.buf, stdout);
+ strbuf_release(&sb);
}
static unsigned int digits_in_number(unsigned int number)
@@ -540,8 +556,8 @@ void show_log(struct rev_info *opt)
printf(" (from %s)",
find_unique_abbrev(parent->object.sha1,
abbrev_commit));
+ fputs(diff_get_color_opt(&opt->diffopt, DIFF_RESET), stdout);
show_decorations(opt, commit);
- printf("%s", diff_get_color_opt(&opt->diffopt, DIFF_RESET));
if (opt->commit_format == CMIT_FMT_ONELINE) {
putchar(' ');
} else {
diff --git a/log-tree.h b/log-tree.h
index 9140f48..d6ecd4d 100644
--- a/log-tree.h
+++ b/log-tree.h
@@ -13,6 +13,7 @@ int log_tree_diff_flush(struct rev_info *);
int log_tree_commit(struct rev_info *, struct commit *);
int log_tree_opt_parse(struct rev_info *, const char **, int);
void show_log(struct rev_info *opt);
+void format_decorations(struct strbuf *sb, const struct commit *commit, int
use_color);
void show_decorations(struct rev_info *opt, struct commit *commit);
void log_write_email_headers(struct rev_info *opt, struct commit *commit,
const char **subject_p,
diff --git a/pretty.c b/pretty.c
index e59688b..e0f93ba 100644
--- a/pretty.c
+++ b/pretty.c
@@ -908,23 +908,6 @@ static void parse_commit_message(struct
format_commit_context *c)
c->commit_message_parsed = 1;
}
-static void format_decoration(struct strbuf *sb, const struct commit *commit)
-{
- struct name_decoration *d;
- const char *prefix = " (";
-
- load_ref_decorations(DECORATE_SHORT_REFS);
- d = lookup_decoration(&name_decoration, &commit->object);
- while (d) {
- strbuf_addstr(sb, prefix);
- prefix = ", ";
- strbuf_addstr(sb, d->name);
- d = d->next;
- }
- if (prefix[0] == ',')
- strbuf_addch(sb, ')');
-
[PATCH v4 03/13] pretty-formats.txt: wrap long lines
Signed-off-by: Nguyễn Thái Ngọc Duy
---
Documentation/pretty-formats.txt | 18 --
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index afac703..6bde67e 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -106,18 +106,22 @@ The placeholders are:
- '%P': parent hashes
- '%p': abbreviated parent hashes
- '%an': author name
-- '%aN': author name (respecting .mailmap, see linkgit:git-shortlog[1] or
linkgit:git-blame[1])
+- '%aN': author name (respecting .mailmap, see linkgit:git-shortlog[1]
+ or linkgit:git-blame[1])
- '%ae': author email
-- '%aE': author email (respecting .mailmap, see linkgit:git-shortlog[1] or
linkgit:git-blame[1])
+- '%aE': author email (respecting .mailmap, see
+ linkgit:git-shortlog[1] or linkgit:git-blame[1])
- '%ad': author date (format respects --date= option)
- '%aD': author date, RFC2822 style
- '%ar': author date, relative
- '%at': author date, UNIX timestamp
- '%ai': author date, ISO 8601 format
- '%cn': committer name
-- '%cN': committer name (respecting .mailmap, see linkgit:git-shortlog[1] or
linkgit:git-blame[1])
+- '%cN': committer name (respecting .mailmap, see
+ linkgit:git-shortlog[1] or linkgit:git-blame[1])
- '%ce': committer email
-- '%cE': committer email (respecting .mailmap, see linkgit:git-shortlog[1] or
linkgit:git-blame[1])
+- '%cE': committer email (respecting .mailmap, see
+ linkgit:git-shortlog[1] or linkgit:git-blame[1])
- '%cd': committer date
- '%cD': committer date, RFC2822 style
- '%cr': committer date, relative
@@ -138,9 +142,11 @@ The placeholders are:
- '%gD': reflog selector, e.g., `refs/stash@{1}`
- '%gd': shortened reflog selector, e.g., `stash@{1}`
- '%gn': reflog identity name
-- '%gN': reflog identity name (respecting .mailmap, see
linkgit:git-shortlog[1] or linkgit:git-blame[1])
+- '%gN': reflog identity name (respecting .mailmap, see
+ linkgit:git-shortlog[1] or linkgit:git-blame[1])
- '%ge': reflog identity email
-- '%gE': reflog identity email (respecting .mailmap, see
linkgit:git-shortlog[1] or linkgit:git-blame[1])
+- '%gE': reflog identity email (respecting .mailmap, see
+ linkgit:git-shortlog[1] or linkgit:git-blame[1])
- '%gs': reflog subject
- '%Cred': switch color to red
- '%Cgreen': switch color to green
--
1.8.2.82.gc24b958
--
To unsubscribe from this list: send the line "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 v4 01/13] pretty: save commit encoding from logmsg_reencode if the caller needs it
The commit encoding is parsed by logmsg_reencode, there's no need for
the caller to re-parse it again. The reencoded message now has the new
encoding, not the original one. The caller would need to read commit
object again before parsing.
Signed-off-by: Nguyễn Thái Ngọc Duy
---
builtin/blame.c | 2 +-
builtin/commit.c | 2 +-
commit.h | 1 +
pretty.c | 16
revision.c | 2 +-
5 files changed, 16 insertions(+), 7 deletions(-)
diff --git a/builtin/blame.c b/builtin/blame.c
index 86100e9..104a948 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -1425,7 +1425,7 @@ static void get_commit_info(struct commit *commit,
commit_info_init(ret);
encoding = get_log_output_encoding();
- message = logmsg_reencode(commit, encoding);
+ message = logmsg_reencode(commit, NULL, encoding);
get_ac_line(message, "\nauthor ",
&ret->author, &ret->author_mail,
&ret->author_time, &ret->author_tz);
diff --git a/builtin/commit.c b/builtin/commit.c
index 4620437..d2f30d9 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -955,7 +955,7 @@ static const char *read_commit_message(const char *name)
if (!commit)
die(_("could not lookup commit %s"), name);
out_enc = get_commit_output_encoding();
- return logmsg_reencode(commit, out_enc);
+ return logmsg_reencode(commit, NULL, out_enc);
}
static int parse_and_validate_options(int argc, const char *argv[],
diff --git a/commit.h b/commit.h
index 87b4b6c..ad55213 100644
--- a/commit.h
+++ b/commit.h
@@ -101,6 +101,7 @@ struct userformat_want {
extern int has_non_ascii(const char *text);
struct rev_info; /* in revision.h, it circularly uses enum cmit_fmt */
extern char *logmsg_reencode(const struct commit *commit,
+char **commit_encoding,
const char *output_encoding);
extern void logmsg_free(char *msg, const struct commit *commit);
extern void get_commit_format(const char *arg, struct rev_info *);
diff --git a/pretty.c b/pretty.c
index d3a82d2..c361b9b 100644
--- a/pretty.c
+++ b/pretty.c
@@ -594,6 +594,7 @@ static char *replace_encoding_header(char *buf, const char
*encoding)
}
char *logmsg_reencode(const struct commit *commit,
+ char **commit_encoding,
const char *output_encoding)
{
static const char *utf8 = "UTF-8";
@@ -615,9 +616,15 @@ char *logmsg_reencode(const struct commit *commit,
sha1_to_hex(commit->object.sha1), typename(type));
}
- if (!output_encoding || !*output_encoding)
+ if (!output_encoding || !*output_encoding) {
+ if (commit_encoding)
+ *commit_encoding =
+ get_header(commit, msg, "encoding");
return msg;
+ }
encoding = get_header(commit, msg, "encoding");
+ if (commit_encoding)
+ *commit_encoding = encoding;
use_encoding = encoding ? encoding : utf8;
if (same_encoding(use_encoding, output_encoding)) {
/*
@@ -658,7 +665,8 @@ char *logmsg_reencode(const struct commit *commit,
if (out)
out = replace_encoding_header(out, output_encoding);
- free(encoding);
+ if (!commit_encoding)
+ free(encoding);
/*
* If the re-encoding failed, out might be NULL here; in that
* case we just return the commit message verbatim.
@@ -1288,7 +1296,7 @@ void format_commit_message(const struct commit *commit,
context.commit = commit;
context.pretty_ctx = pretty_ctx;
context.wrap_start = sb->len;
- context.message = logmsg_reencode(commit, output_enc);
+ context.message = logmsg_reencode(commit, NULL, output_enc);
strbuf_expand(sb, format, format_commit_item, &context);
rewrap_message_tail(sb, &context, 0, 0, 0);
@@ -1451,7 +1459,7 @@ void pretty_print_commit(const struct
pretty_print_context *pp,
}
encoding = get_log_output_encoding();
- msg = reencoded = logmsg_reencode(commit, encoding);
+ msg = reencoded = logmsg_reencode(commit, NULL, encoding);
if (pp->fmt == CMIT_FMT_ONELINE || pp->fmt == CMIT_FMT_EMAIL)
indent = 0;
diff --git a/revision.c b/revision.c
index 71e62d8..2e77397 100644
--- a/revision.c
+++ b/revision.c
@@ -2314,7 +2314,7 @@ static int commit_match(struct commit *commit, struct
rev_info *opt)
* in it.
*/
encoding = get_log_output_encoding();
- message = logmsg_reencode(commit, encoding);
+ message = logmsg_reencode(commit, NULL, encoding);
/* Copy the commit to temporary if we are using "fake" headers */
if (buf.len)
--
1.8.2.82.gc24b958
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.or
[PATCH v4 02/13] pretty: get the correct encoding for --pretty:format=%e
parse_commit_header() provides the commit encoding for '%e' and it
reads it from the re-encoded message, which contains the new encoding,
not the original one in the commit object. This never happens because
--pretty=format:xxx never respects i18n.logoutputencoding. But that's
a different story.
Get the commit encoding from logmsg_reencode() instead.
Signed-off-by: Nguyễn Thái Ngọc Duy
---
pretty.c | 13 +++--
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/pretty.c b/pretty.c
index c361b9b..e59688b 100644
--- a/pretty.c
+++ b/pretty.c
@@ -776,12 +776,12 @@ struct format_commit_context {
unsigned commit_message_parsed:1;
struct signature_check signature_check;
char *message;
+ char *commit_encoding;
size_t width, indent1, indent2;
/* These offsets are relative to the start of the commit message. */
struct chunk author;
struct chunk committer;
- struct chunk encoding;
size_t message_off;
size_t subject_off;
size_t body_off;
@@ -828,9 +828,6 @@ static void parse_commit_header(struct
format_commit_context *context)
} else if (!prefixcmp(msg + i, "committer ")) {
context->committer.off = i + 10;
context->committer.len = eol - i - 10;
- } else if (!prefixcmp(msg + i, "encoding ")) {
- context->encoding.off = i + 9;
- context->encoding.len = eol - i - 9;
}
i = eol;
}
@@ -1185,7 +1182,8 @@ static size_t format_commit_one(struct strbuf *sb, const
char *placeholder,
msg + c->committer.off, c->committer.len,
c->pretty_ctx->date_mode);
case 'e': /* encoding */
- strbuf_add(sb, msg + c->encoding.off, c->encoding.len);
+ if (c->commit_encoding)
+ strbuf_addstr(sb, c->commit_encoding);
return 1;
case 'B': /* raw body */
/* message_off is always left at the initial newline */
@@ -1296,11 +1294,14 @@ void format_commit_message(const struct commit *commit,
context.commit = commit;
context.pretty_ctx = pretty_ctx;
context.wrap_start = sb->len;
- context.message = logmsg_reencode(commit, NULL, output_enc);
+ context.message = logmsg_reencode(commit,
+ &context.commit_encoding,
+ output_enc);
strbuf_expand(sb, format, format_commit_item, &context);
rewrap_message_tail(sb, &context, 0, 0, 0);
+ free(context.commit_encoding);
logmsg_free(context.message, commit);
free(context.signature_check.gpg_output);
free(context.signature_check.signer);
--
1.8.2.82.gc24b958
--
To unsubscribe from this list: send the line "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 v4 00/13] nd/pretty-formats
v4 fixes comments from v3, mainly in the auto-coloring patch, and uses qz_to_tab_space for changes in t4205. It also fixes a coding style issue in 06/13, spotted in v2 but I missed it in v3. Nguyễn Thái Ngọc Duy (13): pretty: save commit encoding from logmsg_reencode if the caller needs it pretty: get the correct encoding for --pretty:format=%e pretty-formats.txt: wrap long lines pretty: share code between format_decoration and show_decorations utf8.c: move display_mode_esc_sequence_len() for use by other functions utf8.c: add utf8_strnwidth() with the ability to skip ansi sequences utf8.c: add reencode_string_len() that can handle NULs in string pretty: two phase conversion for non utf-8 commits pretty: split color parsing into a separate function pretty: add %C(auto) for auto-coloring pretty: support padding placeholders, %< %> and %>< pretty: support truncating in %>, %< and %>< pretty: support %>> that steal trailing spaces Documentation/pretty-formats.txt | 35 +++- builtin/blame.c | 2 +- builtin/commit.c | 2 +- commit.h | 1 + compat/precompose_utf8.c | 2 +- log-tree.c | 48 -- log-tree.h | 1 + pretty.c | 358 --- revision.c | 2 +- t/t4205-log-pretty-formats.sh| 175 +++ t/t4207-log-decoration-colors.sh | 8 +- t/t6006-rev-list-format.sh | 12 +- utf8.c | 104 +--- utf8.h | 23 ++- 14 files changed, 648 insertions(+), 125 deletions(-) -- 1.8.2.82.gc24b958 -- To unsubscribe from this list: send the line "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] help.c: add a compatibility comment to cmd_version()
From: "Junio C Hamano" Sent: Thursday, April 18, 2013 1:13 AM "Philip Oakley" writes: How about * E.g. git gui uses the extended regular expression "^git version [1-9]+(\.[0-9]+)+.*" * to check for an acceptable version string. The ERE is from git-gui.sh:L958. That is exactly the kind of guarantee we do _not_ want to give. So you are trying to avoid giving _any_ "guarantee" about what visible manifestation a user may obtain about a system that passes the test suite we have set out. I was hoping we could give a basic minimum indication of the expected style of the version string for a git which *passes* our tests. But like you say, it doesn't form a real guarantee - caveat emptor still applies. ... Hence my suggestion of the basic test that a "passing" git would produce a consistent version string. I have been assuming that you are trying to avoid an exchange like this one we had in the past: http://thread.gmane.org/gmane.comp.version-control.git/216923/focus=217007 In a sense yes. As David noted, others do attempt to trust us via our current version string format. I thought it worthwhile (given my earlier 'mistake' in 216923/focus=216925) to suggest such a basic indication of our current string style. I also have been assuming that you are pushing to limit the possible versioning scheme, but I do not know what that extra limitation would achieve in the real world. By sticking to the pattern "git gui" happens to use, "git gui" may be able to guess that the next version of Git says it is verison "1.8.3". That is the _only_ thing your "test" buys. But having parsed the "1.8.3" substring out of it, "git gui" does not know anything about what 1.8.3 gives it. As far as it is concerned, it is a version whose "git version" output it has never seen (unless it has been kept up to date with the development of Git). You are focusssing on the wrong side of issue (from my viewpoint). If my earlier patch had gone in, it would have passsed our tests but not played nicely with the community tools. That would have been IMHO a regression, so from my viewpoint I believed it was worth having a test to avoid such a 'regression'. By matching against "git version [1-9]+(\.[0-9]+)+", it is accepting that future versions may break assumptions it makes for some known versions (which is warranted) and all future versions (which is unwarranted) of Git. Maybe the version 2.0 of Git adds all changes in the directory "d", including removals, when you say "git add d", but it may have assumed that with Git version 1.5.0 or newer, saying "git add d" would result in added and modified inside that directory getting updated in the index, but paths removed from the working tree will stay in the index. If it was a gross incompatibility then (a) we are likley to be signalling it for a while, and (b) other tools would need updating as well, and they would hope that they could 'read' the version in a consistent style. We would also still have the choice of changing our existing string style, which would explicitly signal a change via the test fail. The only thing the scripts that read from the output of "git version" can reliably tell is if it is interacting with a version of Git it knows about. If it made any unwarranted assumption on the versions it hasn't seen, it has to be fixed in "git gui" _anyway_. Of course, we would not change the output of "git version" willy-nilly without good reason, but that is a different topic. Ah. I thought it was the [original] topic. I was calibrating the willy-nillyness ;-) So I do not know what you want to achieve in the real world with that extra limitation on the "git version" output format. Maybe you are proposing something else. I dunno. I was just using a slightly different philosophical approach. -- Philip -- To unsubscribe from this list: send the line "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: Pushing/fetching from/into a shallow-cloned repository
"Philip Oakley" writes: >> So I observe pushing/fetching works OK at least for a simple case like >> this one. >> >> Hence I'd like to ask: if the manual page is wrong or I'm observing >> some corner case? >> -- > The manual is deliberately misleading. Yes, it is erring on the safe side. It was not coded with the case where the gap widens (e.g. either side rewinds the history) in mind as you explained; for simple cases the code just happens to work, but the users are encouraged not to rely on it to be safe than sorry. -- To unsubscribe from this list: send the line "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/2] submodule: drop the top-level requirement
John Keeping writes:
> +relative_path ()
> +{
> + local target curdir result
> + target=$1
> + curdir=${2-$wt_prefix}
> + curdir=${curdir%/}
> + result=
> +
> + while test -n "$curdir"
> + do
> + case "$target" in
> + "$curdir/"*)
> + target=${target#$curdir/}
> + break
> + ;;
> + esac
Could $curdir have glob wildcard to throw this part of the logic
off? It is OK to have limitations like "you cannot have a glob
characters in your path to submodule working tree" (at least until
we start rewriting these in C or Perl or Python), but we need to be
aware of them.
> module_list()
> {
> + eval "set $(git rev-parse --sq --prefix "$wt_prefix" -- "$@")"
An efficient reuse of "--" ;-)
> +test_expect_success 'run summary from subdir' '
> + mkdir sub &&
> + (
> + cd sub &&
> + git submodule summary >../actual
> + ) &&
> + cat >expected <<-EOF &&
> +* ../sm1 000...$head1 (2):
> + > Add foo2
> +
> +EOF
It somewhat looks strange to start with "<<-EOF" and then not to
indent the body nor EOF.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] stash: tighten IS_STASH_LIKE logic
Ramkumar Ramachandra writes: > Currently, 'git stash show' and 'git stash apply' can show/apply any > merge commit, as the test for setting IS_STASH_LIKE simply asserts if > the commit is a merge. Improve the situation by asserting if the > index_commit and the worktree_commit are based off the same commit, by > checking that $REV^1 is equal to $REV^2^1. When was the last time you tried to run "git stash apply next"? The current code parses the ancestors and trees because the real work needs them, and the 'does this look like a stash?' is a side effect, but reading it^2^1 and comparing with it^1 is a pure overhead for the sake of checking. It looks like a futile mental masturbation to solve theoretical non-issue to me. Is it worth it? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Apr 2013, #05; Mon, 15)
Jeff King writes: > I am expecting a reaction more like "Hmm, I never thought about it > before. Does that make sense to me or not? Let me think about which > paths it pertains to and decide". Let's step back and re-review the main text. It currently says: In Git 2.0, 'git add ...' will also update the index for paths removed from the working tree that match the given pathspec. If you want to 'add' only changed or newly created paths, say 'git add --no-all ...' instead. This was written for the old "we may want to warn" logic that did not even check if we would be omitting a removal. The new logic will show the text _only_ when the difference matters, we have an opportunity to tighten it a lot, for example: You ran 'git add' with neither '-A (--all)' or '--no-all', whose behaviour will change in Git 2.0 with respect to paths you removed from your working tree. * 'git add --no-all ', which is the current default, ignores paths you removed from your working tree. * 'git add --all ' will let you also record the removals. The removed paths (e.g. '%s') are ignored with this version of Git. Run 'git status' to remind yourself what paths you have removed from your working tree. or something? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Apr 2013, #05; Mon, 15)
On Thu, Apr 18, 2013 at 02:37:53PM -0700, Junio C Hamano wrote: > Because this is to help people who are _used_ to seeing "git add" > not take the removals into account, I doubt that "Did I want those > updated or not? Let me see the details of them." will be the > question they will be asking [*1*]. > > I dunno. > > > [Footnote] > > *1* "I know I didn't want to include these removals to the index, > but I learned today that in later Git I should make myself more > clear if I want to keep doing so; thanks for letting me know.", or > "I've long been assuming that I have to say 'git add' and 'git rm' > separately, but I learned today that I can say 'add --all', and in > later Git I do not even have to; thanks for letting me know." are > the two reactions I expected. I am expecting a reaction more like "Hmm, I never thought about it before. Does that make sense to me or not? Let me think about which paths it pertains to and decide". Which I admit is no more likely than the scenarios you outlined, but it is close to what I thought the first time I saw the warning. -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: What's cooking in git.git (Apr 2013, #05; Mon, 15)
Jeff King writes: >> But I think we are doing users a disservice by listing tons of >> paths. Where the difference of versions matters _most_ is when the >> user has tons of removed paths in the working tree. Either with one >> warning per path, or a block of collected paths at the end, we are >> scrolling the more important part of the message up. > > I'm not sure I agree. Even with a handful, it made me wonder why one was > mentioned and not others. That _could_ be cleared up by rewording (i.e., > making it clear that this is an example, and there may be more). But > somehow listing them is what I would expect. Perhaps because it gives > the user a clue about what to do next; they ask themselves "did I want > those updated or not?". > > In the orphaned-commit message when leaving a detached HEAD, we collect > the answer, say "you are leaving N commits", and show the first 5 five > of them, with an ellipsis at the end if we didn't show them all. Would > it makes sense to do that here? Because this is to help people who are _used_ to seeing "git add" not take the removals into account, I doubt that "Did I want those updated or not? Let me see the details of them." will be the question they will be asking [*1*]. I dunno. [Footnote] *1* "I know I didn't want to include these removals to the index, but I learned today that in later Git I should make myself more clear if I want to keep doing so; thanks for letting me know.", or "I've long been assuming that I have to say 'git add' and 'git rm' separately, but I learned today that I can say 'add --all', and in later Git I do not even have to; thanks for letting me know." are the two reactions I expected. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
What's cooking in git.git (Apr 2013, #06; Thu, 18)
Here are the topics that have been cooking. Commits prefixed with '-' are only in 'pu' (proposed updates) while commits prefixed with '+' are in 'next'. 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"] * fc/completion (2013-04-14) 8 commits (merged to 'next' on 2013-04-14 at a509746) + completion: small optimization + completion: inline __gitcomp_1 to its sole callsite + completion: get rid of compgen + completion: add __gitcomp_nl tests + completion: add new __gitcompadd helper + completion: get rid of empty COMPREPLY assignments + completion: trivial test improvement + completion: add more cherry-pick options In addition to a user visible change to offer more options to cherry-pick, generally cleans up and simplifies the code. * fc/send-email-annotate (2013-04-14) 7 commits (merged to 'next' on 2013-04-14 at 4af1076) + rebase-am: explicitly disable cover-letter + format-patch: trivial cleanups + format-patch: add format.coverLetter configuration variable + log: update to OPT_BOOL + format-patch: refactor branch name calculation + format-patch: improve head calculation for cover-letter + send-email: make annotate configurable Allows format-patch --cover-letter to be configurable; the most notable is the "auto" mode to create cover-letter only for multi patch series. * jc/detached-head-doc (2013-04-05) 1 commit (merged to 'next' on 2013-04-14 at 24b9271) + glossary: extend "detached HEAD" description Describe what happens when a command that operates on "the current branch" is run on a detached HEAD. * jk/daemon-user-doc (2013-04-12) 1 commit (merged to 'next' on 2013-04-14 at 56c08ff) + doc: clarify that "git daemon --user=" option does not export HOME=~user Document where the configuration is read by the git-daemon when its --user option is used. * jk/http-dumb-namespaces (2013-04-09) 1 commit (merged to 'next' on 2013-04-15 at 4bfa834) + http-backend: respect GIT_NAMESPACE with dumb clients Allow smart-capable HTTP servers to be restricted via the GIT_NAMESPACE mechanism when talking with commit-walker clients (they already do so when talking with smart HTTP clients). * jk/http-error-messages (2013-04-16) 1 commit (merged to 'next' on 2013-04-16 at 4a32517) + http: set curl FAILONERROR each time we select a handle A regression fix for the recently graduated topic. * jk/merge-tree-added-identically (2013-04-08) 1 commit (merged to 'next' on 2013-04-15 at 35fd4b9) + merge-tree: don't print entries that match "local" The resolution of some corner cases by "git merge-tree" were inconsistent between top-of-the-tree and in a subdirectory. * jk/test-trash (2013-04-14) 2 commits (merged to 'next' on 2013-04-15 at 15a6624) + t/test-lib.sh: drop "$test" variable + t/test-lib.sh: fix TRASH_DIRECTORY handling Fix longstanding issues with the test harness when used with --root= option. * kb/co-orphan-suggestion-short-sha1 (2013-04-08) 1 commit (merged to 'next' on 2013-04-14 at 8caf7fd) + checkout: abbreviate hash in suggest_reattach Update the informational message when "git checkout" leaves the detached head state. * rs/empty-archive (2013-04-10) 1 commit (merged to 'next' on 2013-04-15 at eab39bc) + t5004: fix issue with empty archive test and bsdtar Implementations of "tar" of BSD descend have found to have trouble with reading an otherwise empty tar archive with pax headers and causes an unnecessary test failure. * th/t9903-symlinked-workdir (2013-04-11) 1 commit (merged to 'next' on 2013-04-15 at f062dc6) + t9903: Don't fail when run from path accessed through symlink * tr/packed-object-info-wo-recursion (2013-03-27) 3 commits (merged to 'next' on 2013-03-29 at b1c3858) + sha1_file: remove recursion in unpack_entry + Refactor parts of in_delta_base_cache/cache_or_unpack_entry + sha1_file: remove recursion in packed_object_info Attempts to reduce the stack footprint of sha1_object_info() and unpack_entry() codepaths. -- [New Topics] * jk/a-thread-only-dies-once (2013-04-16) 2 commits (merged to 'next' on 2013-04-18 at 3208f44) + run-command: use thread-aware die_is_recursing routine + usage: allow pluggable die-recursion checks A regression fix for the logic to detect die() handler triggering itself recursively. Will fast-track to 'master'. * tr/copy-revisions-from-stdin (2013-04-16) 1 commit (merged to 'next' on 2013-04-16 at d882870) + read_revisions_from_stdin: make copies for handle_revision_arg A fix to a long-standing issue in the command line parser for revisions, which was triggered by mv/sequence-pick-error-diag topic (now in 'next'). Will merge to 'master'. * jc/prune-all (2013-04-18) 3 commits - api-parse-options.txt: document "no
Re: Git over HTTPS with basic authentication
On Thu, Apr 18, 2013 at 09:54:32PM +0200, Matthieu Moy wrote: > Sebastian Schmidt writes: > > > Why is git not working over HTTPS with basic authentication? I can clone > > just fine, but when I try to push, it tells me > > What are you using on the server? Dumb HTTP works by serving the > repository files as static pages, which is fundamentally read-only. The > recommended way is to use smart-HTTP (see man git-http-backend, requires > Git on the server), and the alternative is to use webdav (much slower). Yeah, this is definitely dumb http (since http-push is involved at all, which the original error message showed). Code 22 is curl's "there was an HTTP error" code, but http-push annoyingly does not output the actual HTTP error[1]. You can see more by setting GIT_CURL_VERBOSE=1 in the environment. Though if you know you did not set up WebDAV on the server, then we can know that is the problem even without seeing the HTTP code. :) -Peff [1] The dumb-http push code is largely unloved and unmaintained at this point. Yet another reason to move to smart http. -- To unsubscribe from this list: send the line "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 (Apr 2013, #05; Mon, 15)
On Thu, Apr 18, 2013 at 11:16:33AM -0700, Junio C Hamano wrote:
> Jeff King writes:
>
> > could work for both cases. Something like "not considering" (or another
> > synonym for "considering") might be even more accurate. It is not just
> > that we did not stage it; it is what we did not even consider it an item
> > for staging under the current rules.
>
> Yes, "not considering" is much more sensible, while side-stepping
> the dryrun issue. Or
>
>warning("ignoring removal of '%s'")
I like that much better than either of my suggestions.
> > Note that the "not staging" warnings may potentially be interspersed
> > with the normal dry-run output. I think that's OK.
>
> As long as the top-text makes it clear what "not considering" (or
> "ignoring") in the following text means, I think it is fine.
Agreed, and I think the current text is fine for that (though neither of
us is the best judge at this point of how a less familiar user would
interpret it).
> But I think we are doing users a disservice by listing tons of
> paths. Where the difference of versions matters _most_ is when the
> user has tons of removed paths in the working tree. Either with one
> warning per path, or a block of collected paths at the end, we are
> scrolling the more important part of the message up.
I'm not sure I agree. Even with a handful, it made me wonder why one was
mentioned and not others. That _could_ be cleared up by rewording (i.e.,
making it clear that this is an example, and there may be more). But
somehow listing them is what I would expect. Perhaps because it gives
the user a clue about what to do next; they ask themselves "did I want
those updated or not?".
In the orphaned-commit message when leaving a detached HEAD, we collect
the answer, say "you are leaving N commits", and show the first 5 five
of them, with an ellipsis at the end if we didn't show them all. Would
it makes sense to do that here?
Yet another alternative would be to print a warning for each path, but
hold the main warning for the end, so that it is the first thing the
user sees. That has the added bonus that regular "--dry-run" output
will not scroll it away, either.
-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: Pushing/fetching from/into a shallow-cloned repository
From: "Konstantin Khomoutov" Sent: Thursday, April 18, 2013 10:52 AM The git-clone manual page, both [1] and my local copy coming with Git for Windows 1.8.1, say about the --depth command-line option: --depth Create a shallow clone with a history truncated to the specified number of revisions. A shallow repository has a number of limitations (you cannot clone or fetch from it, nor push from nor into it), but is adequate if you are only interested in the recent history of a large project with a long history, and would want to send in fixes as patches. But having done a shallow clone (--depth=1) of one of my repositories, I was able to record a new commit, push it out to a "reference" bare repository and then fetch back to another clone of the same repository just fine. I have then killed my test commit doing a forced push from another clone and subsequently was able to do `git fetch` in my shallow clone just fine. So I observe pushing/fetching works OK at least for a simple case like this one. Hence I'd like to ask: if the manual page is wrong or I'm observing some corner case? -- The manual is deliberately misleading. The problem is that the depth is a movable feast that depends on how far the branches have progressed. The DAG will be missing the historic merge bases, and in some scenarios can form disconnected runs of commits. The ref negotiation can also be a problem. The git\Documentation\technical\shallow.txt has some extra details on issues. Philip -- To unsubscribe from this list: send the line "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 (Apr 2013, #05; Mon, 15)
On Wed, Apr 17, 2013 at 2:50 PM, Felipe Contreras wrote: > On Tue, Apr 16, 2013 at 5:45 PM, Phil Hord wrote: >> On Tue, Apr 16, 2013 at 3:04 PM, Felipe Contreras >>> If you want to waste your time, by all means, rewrite all my commit >>> messages with essays that nobody will ever read. I'm not going to do >>> that for some hypothetical case that will never happen. I'm not going >>> to waste my time. >> >> This is not a hypothetical. Almost every time I bisect a regression >> in git.git, I find the commit message tells me exactly why the commit >> did what it did and what the expected result was. I find this to be >> amazingly useful. Do I need to show you real instances of that >> happening? No. I promise it did, though. > > Yes please. Show me one of the instances where you hit a bisect with > any of the remote-hg commits mentioned above by Thomas Rast. I made no such claim. In fact, I have never bisected to any remote-hg-related commit. I fail to see the relevance of this qualifier, though. P -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Git over HTTPS with basic authentication
Sebastian Schmidt writes: > Why is git not working over HTTPS with basic authentication? I can clone > just fine, but when I try to push, it tells me What are you using on the server? Dumb HTTP works by serving the repository files as static pages, which is fundamentally read-only. The recommended way is to use smart-HTTP (see man git-http-backend, requires Git on the server), and the alternative is to use webdav (much slower). -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 2/2] submodule: drop the top-level requirement
Use the new rev-parse --prefix option to process all paths given to the
submodule command, dropping the requirement that it be run from the
top-level of the repository.
Signed-off-by: John Keeping
---
Changes since v2:
- Handle relative paths given to "submodule add --reference=./..."
- Check for absolute path before prepending $wt_prefix in cmd_add
- Export relative sm_path in cmd_foreach
- Change displayed sm_path to be relative
- Move a mkdir out of a subshell in t7400
I'm not sure if the relative_path function here is sufficient on
Windows. We should be okay with the "target" argument since that comes
from git-ls-files but I think we're in trouble if "git rev-parse
--show-prefix" output the prefix with '\' instead of '/'.
git-submodule.sh | 113 ---
t/t7400-submodule-basic.sh | 27 +++
t/t7401-submodule-summary.sh | 14 ++
3 files changed, 127 insertions(+), 27 deletions(-)
diff --git a/git-submodule.sh b/git-submodule.sh
index 79bfaac..0eee703 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -14,10 +14,13 @@ USAGE="[--quiet] add [-b ] [-f|--force] [--name
] [--reference
or: $dashless [--quiet] sync [--recursive] [--] [...]"
OPTIONS_SPEC=
+SUBDIRECTORY_OK=Yes
. git-sh-setup
. git-sh-i18n
. git-parse-remote
require_work_tree
+wt_prefix=$(git rev-parse --show-prefix)
+cd_to_toplevel
command=
branch=
@@ -106,12 +109,48 @@ resolve_relative_url ()
echo "${is_relative:+${up_path}}${remoteurl#./}"
}
+# Resolve a path to be relative to another path. This is intended for
+# converting submodule paths when git-submodule is run in a subdirectory
+# and only handles paths where the directory separator is '/'.
+#
+# The output is the first argument as a path relative to the second argument,
+# which defaults to $wt_prefix if it is omitted.
+relative_path ()
+{
+ local target curdir result
+ target=$1
+ curdir=${2-$wt_prefix}
+ curdir=${curdir%/}
+ result=
+
+ while test -n "$curdir"
+ do
+ case "$target" in
+ "$curdir/"*)
+ target=${target#$curdir/}
+ break
+ ;;
+ esac
+
+ result="${result}../"
+ if test "$curdir" = "${curdir%/*}"
+ then
+ curdir=
+ else
+ curdir="${curdir%/*}"
+ fi
+ done
+
+ echo "$result$target"
+}
+
#
# Get submodule info for registered submodules
# $@ = path to limit submodule list
#
module_list()
{
+ eval "set $(git rev-parse --sq --prefix "$wt_prefix" -- "$@")"
(
git ls-files --error-unmatch --stage -- "$@" ||
echo "unmatched pathspec exists"
@@ -282,6 +321,7 @@ isnumber()
cmd_add()
{
# parse $args after "submodule ... add".
+ reference_path=
while test $# -ne 0
do
case "$1" in
@@ -298,11 +338,11 @@ cmd_add()
;;
--reference)
case "$2" in '') usage ;; esac
- reference="--reference=$2"
+ reference_path=$2
shift
;;
--reference=*)
- reference="$1"
+ reference_path="${1#--reference=}"
;;
--name)
case "$2" in '') usage ;; esac
@@ -323,6 +363,14 @@ cmd_add()
shift
done
+ if test -n "$reference_path"
+ then
+ is_absolute_path "$reference_path" ||
+ reference_path="$wt_prefix$reference_path"
+
+ reference="--reference=$reference_path"
+ fi
+
repo=$1
sm_path=$2
@@ -335,6 +383,8 @@ cmd_add()
usage
fi
+ is_absolute_path "$sm_path" || sm_path="$wt_prefix$sm_path"
+
# assure repo is absolute or relative to parent
case "$repo" in
./*|../*)
@@ -479,6 +529,7 @@ cmd_foreach()
# we make $path available to scripts ...
path=$sm_path
cd "$sm_path" &&
+ sm_path=$(relative_path "$sm_path") &&
eval "$@" &&
if test -n "$recursive"
then
@@ -594,27 +645,29 @@ cmd_deinit()
die_if_unmatched "$mode"
name=$(module_name "$sm_path") || exit
+ displaypath=$(relative_path "$sm_path")
+
# Remove the submodule work tree (unless the user already did
it)
if test -d "$sm_path"
then
# Protect submodules containing a .git directory
if test -d "$sm
[PATCH v3 1/2] rev-parse: add --prefix option
This makes 'git rev-parse' behave as if it were invoked from the
specified subdirectory of a repository, with the difference that any
file paths which it prints are prefixed with the full path from the top
of the working tree.
This is useful for shell scripts where we may want to cd to the top of
the working tree but need to handle relative paths given by the user on
the command line.
Signed-off-by: John Keeping
---
Changes since v2:
- Rewrite commit message
- Add a new test for 'prefix ignored with HEAD:top'
- Use '<<-\EOF' where appropriate in t1513
Documentation/git-rev-parse.txt | 16 +++
builtin/rev-parse.c | 24 ---
t/t1513-rev-parse-prefix.sh | 96 +
3 files changed, 131 insertions(+), 5 deletions(-)
create mode 100755 t/t1513-rev-parse-prefix.sh
diff --git a/Documentation/git-rev-parse.txt b/Documentation/git-rev-parse.txt
index 1f9ed6c..0ab2b77 100644
--- a/Documentation/git-rev-parse.txt
+++ b/Documentation/git-rev-parse.txt
@@ -59,6 +59,22 @@ OPTIONS
If there is no parameter given by the user, use ``
instead.
+--prefix ::
+ Behave as if 'git rev-parse' was invoked from the ``
+ subdirectory of the working tree. Any relative filenames are
+ resolved as if they are prefixed by `` and will be printed
+ in that form.
++
+This can be used to convert arguments to a command run in a subdirectory
+so that they can still be used after moving to the top-level of the
+repository. For example:
++
+
+prefix=$(git rev-parse --show-prefix)
+cd "$(git rev-parse --show-toplevel)"
+eval "set -- $(git rev-parse --sq --prefix "$prefix" "$@")"
+
+
--verify::
Verify that exactly one parameter is provided, and that it
can be turned into a raw 20-byte SHA-1 that can be used to
diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index f267a1d..de894c7 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -212,11 +212,17 @@ static void show_datestring(const char *flag, const char
*datestr)
show(buffer);
}
-static int show_file(const char *arg)
+static int show_file(const char *arg, int output_prefix)
{
show_default();
if ((filter & (DO_NONFLAGS|DO_NOREV)) == (DO_NONFLAGS|DO_NOREV)) {
- show(arg);
+ if (output_prefix) {
+ const char *prefix = startup_info->prefix;
+ show(prefix_filename(prefix,
+prefix ? strlen(prefix) : 0,
+arg));
+ } else
+ show(arg);
return 1;
}
return 0;
@@ -470,6 +476,7 @@ N_("git rev-parse --parseopt [options] -- [...]\n"
int cmd_rev_parse(int argc, const char **argv, const char *prefix)
{
int i, as_is = 0, verify = 0, quiet = 0, revs_count = 0, type = 0;
+ int output_prefix = 0;
unsigned char sha1[20];
const char *name = NULL;
@@ -503,7 +510,7 @@ int cmd_rev_parse(int argc, const char **argv, const char
*prefix)
const char *arg = argv[i];
if (as_is) {
- if (show_file(arg) && as_is < 2)
+ if (show_file(arg, output_prefix) && as_is < 2)
verify_filename(prefix, arg, 0);
continue;
}
@@ -527,7 +534,7 @@ int cmd_rev_parse(int argc, const char **argv, const char
*prefix)
as_is = 2;
/* Pass on the "--" if we show anything but
files.. */
if (filter & (DO_FLAGS | DO_REVS))
- show_file(arg);
+ show_file(arg, 0);
continue;
}
if (!strcmp(arg, "--default")) {
@@ -535,6 +542,13 @@ int cmd_rev_parse(int argc, const char **argv, const char
*prefix)
i++;
continue;
}
+ if (!strcmp(arg, "--prefix")) {
+ prefix = argv[i+1];
+ startup_info->prefix = prefix;
+ output_prefix = 1;
+ i++;
+ continue;
+ }
if (!strcmp(arg, "--revs-only")) {
filter &= ~DO_NOREV;
continue;
@@ -754,7 +768,7 @@ int cmd_rev_parse(int argc, const char **argv, const char
*prefix)
if (verify)
die_no_single_rev(quiet);
as_is = 1;
- if (!show_file(arg))
+ if (!show_file(arg, output_prefix))
continue;
verify_filename(pre
[PATCH v3 0/2] submodule: drop the top-level requirement
Thanks to Ram and Jens for the feedback on v2. I've addressed most of their comments in this version, but I've ignored a couple of Ram's nits about test cases where leaving it how it is makes the added tests consistent with those already in the same file. Since v2, the main improvement is that the output from git-submodule now contains paths relative to the directory in which the command is run, not the top-level of the working tree. John Keeping (2): rev-parse: add --prefix option submodule: drop the top-level requirement Documentation/git-rev-parse.txt | 16 ++ builtin/rev-parse.c | 24 +++-- git-submodule.sh| 113 ++-- t/t1513-rev-parse-prefix.sh | 96 ++ t/t7400-submodule-basic.sh | 27 ++ t/t7401-submodule-summary.sh| 14 + 6 files changed, 258 insertions(+), 32 deletions(-) create mode 100755 t/t1513-rev-parse-prefix.sh -- 1.8.2.1.424.g1899e5e.dirty -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Git over HTTPS with basic authentication
Why is git not working over HTTPS with basic authentication? I can clone just fine, but when I try to push, it tells me error: Cannot access URL https://..., return code 22 fatal: git-http-push failed I have googled for an hour now, all I find is people that have the same problem and the solution is always to use SSH. Sibbo -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 4/6] transport-helper: warn when refspec is not used
On Thu, Apr 18, 2013 at 12:30 PM, Junio C Hamano wrote: > Felipe Contreras writes: > >> For the modes that need it. In the future we should probably error out, >> instead of providing half-assed support. >> >> The reason we want to do this is because if it's not present, the remote >> helper might be updating refs/heads/*, or refs/remotes/origin/*, >> directly, and in the process fetch will get confused trying to update >> refs that are already updated, or older than what they should be. We >> shouldn't be messing with the rest of git. > > So that answers my question in the response to an earlier one in > this series. We expect the ref updates to be done by the fetch or > push that drives the helper, and do not want the helper to interfere > with its ref updates. > > So it is not just 'refspec' _allows_ the refs to be constrained to a > private namespace, like the earlier updates made the documentation > say; it _is_ mandatory to use refspecs to constrain them to avoid > touching refs/heads and refs/remotes namespace. Yeah, it was not stated that it was mandatory, but I think it's in everybody's best interest that it is. -- Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/6] transport-helper: clarify *:* refspec
On Thu, Apr 18, 2013 at 12:27 PM, Junio C Hamano wrote: > Felipe Contreras writes: > >> The *:* refspec doesn't work, and never has, clarify the code and >> documentation to reflect that. This in effect reverts commit 9e7673e >> (gitremote-helpers(1): clarify refspec behaviour). >> ... >> applicable refspec takes precedence. The left-hand of refspecs >> advertised with this capability must cover all refs reported by >> -the list command. If a helper does not need a specific 'refspec' >> -capability then it should advertise `refspec *:*`. >> +the list command. If no 'refspec' capability is advertised, >> +there is an implied `refspec *:*`. > > I presume > > s/.$/, but `*:*` does not work so do not use it./ > > is implied? I believe s/.$/, but you should specify an apropriate one as described above./ Would be better. >> diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh >> index f387027..cd1873c 100755 >> --- a/t/t5801-remote-helpers.sh >> +++ b/t/t5801-remote-helpers.sh >> @@ -120,21 +120,6 @@ test_expect_failure 'pushing without refspecs' ' >> compare_refs local2 HEAD server HEAD >> ' >> >> -test_expect_success 'pulling with straight refspec' ' >> - (cd local2 && >> - GIT_REMOTE_TESTGIT_REFSPEC="*:*" git pull) && >> - compare_refs local2 HEAD server HEAD >> -' > > This one made me raise my eyebrows, first. > > The only reason this test "passes" is because this particular > "pull", due to what local2 and server happens to have before this > test runs, is an "Already up-to-date" and a no-op. You are removing > this because it is not really testing anything useful, and if you > change it to test something real, e.g. by rewinding local2, it will > reveal the breakage. > > Am I reading you correctly? Not really. This particular fetch does work, and it's tricky to explain all the reasons why, even if you update the server ref before fetching. The reason it's written this way is that it comes from from a branch of mine that has a hack to make the push above works, and since the transport-helper's push doesn't update the ref in this version, the test did actually do something and was working. I am removing the test because we don't care if it works or not, remote-helpers should not be doing that. The fact that the test wasn't actually doing anything is icing on the cake. Cheers. -- Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/6] transport-helper: clarify pushing without refspecs
On 04/18/2013 02:05 AM, Felipe Contreras wrote: > This has never worked, since it's inception the code simply skips all > s/it's/its/ (sorry for nitpicking) > the refs, essentially telling fast-export to do nothing. > > Let's at least tell the user what's going on. > > [SNIP] > Regards, Stefano -- To unsubscribe from this list: send the line "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 3/6] transport-helper: clarify pushing without refspecs
On Thu, Apr 18, 2013 at 10:29:30AM -0700, Junio C Hamano wrote:
> John Keeping writes:
> >> diff --git a/transport-helper.c b/transport-helper.c
> >> index cea787c..4d98567 100644
> >> --- a/transport-helper.c
> >> +++ b/transport-helper.c
> >> @@ -785,6 +785,9 @@ static int push_refs_with_export(struct transport
> >> *transport,
> >>struct string_list revlist_args = STRING_LIST_INIT_NODUP;
> >>struct strbuf buf = STRBUF_INIT;
> >>
> >> + if (!data->refspecs)
> >> + die("remote-helper doesn't support push; refspec needed");
> >
> > I think the "refspec needed" text is likely to be confusing if an
> > end-user ever sees this message. I'm not sure how we can provide useful
> > feedback for both remote helper authors and end-users though.
>
> This "refspecs" only come via the helper and not directly from the
> end user, no?
>
> If that is the case, I do not think "confusing" is much of an issue.
> Not _("localizing") is also the right thing to do. We may want to
> say "BUG: " at front to clarify it is not the end-user's fault, but
> a problem they may want to report. If we at this point know what
> helper attempted export without giving refspecs, it may help to show
> it here, so that developers will know with what helper the user
> had problems with.
I like this idea. Perhaps we should say "Bug in remote helper; refspec
needed with export", so that it clearly indicates to both end-users and
remote helper authors that the error is in the remote helper.
--
To unsubscribe from this list: send the line "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 (Apr 2013, #05; Mon, 15)
Jeff King writes:
> could work for both cases. Something like "not considering" (or another
> synonym for "considering") might be even more accurate. It is not just
> that we did not stage it; it is what we did not even consider it an item
> for staging under the current rules.
Yes, "not considering" is much more sensible, while side-stepping
the dryrun issue. Or
warning("ignoring removal of '%s'")
> Note that the "not staging" warnings may potentially be interspersed
> with the normal dry-run output. I think that's OK.
As long as the top-text makes it clear what "not considering" (or
"ignoring") in the following text means, I think it is fine.
But I think we are doing users a disservice by listing tons of
paths. Where the difference of versions matters _most_ is when the
user has tons of removed paths in the working tree. Either with one
warning per path, or a block of collected paths at the end, we are
scrolling the more important part of the message up.
That was why I originally showed one path as an example and stopped
there. Perhaps it is a better solution to keep that behaviour and
rephrase the message to say that we ignored removal of paths like
this one '%s' you lost from the working tree but it will change in
Git 2.0 and you will better learn to use the --no-all option now.
The inter-topic conflicts between stages of three "add in Git 2.0"
topics is getting cumbersome even with the help from rerere, so I'd
like to merge their preparatory steps as I have them now to 'next'
and merge them down to 'master' first, and start applying tweaks
from there, or something.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Apr 2013, #05; Mon, 15)
On Thu, Apr 18, 2013 at 10:51:12AM -0700, Junio C Hamano wrote:
> Jeff King writes:
>
> > +static const char *add_would_remove_warning = N_(
> > +/* indent for "warning: " */
> > + "In Git 2.0, 'git add ...' will also update the\n"
> > +"index for paths removed from the working tree that match the given\n"
> > +"pathspec. If you want to 'add' only changed or newly created paths,\n"
> > +"say 'git add --no-all ...' instead.\n");
> > +
> > static void warn_add_would_remove(const char *path)
> > {
> > - warning(_("In Git 2.0, 'git add ...' will also update the\n"
> > - "index for paths removed from the working tree that match\n"
> > - "the given pathspec. If you want to 'add' only changed\n"
> > - "or newly created paths, say 'git add --no-all ...'"
> > - " instead.\n\n"
> > - "'%s' would be removed from the index without --no-all."),
> > - path);
> > + static int warned_once;
> > + if (!warned_once++)
> > + warning(_(add_would_remove_warning));
> > + warning("did not stage removal of '%s'", path);
> > }
>
> Would "add --dry-run" say this, too?
It probably makes sense to continue to have the warning in the dry-run
case, but it may make sense to tweak it grammatically when we are in
dry-run mode. Saying "would stage removal" is technically correct, but I
think it is somewhat ambiguous: would git do it if we were not in a
--dry-run, or would git do it if it were Git 2.0?
Doing it as:
warning: not staging removal of '%s'
could work for both cases. Something like "not considering" (or another
synonym for "considering") might be even more accurate. It is not just
that we did not stage it; it is what we did not even consider it an item
for staging under the current rules.
Note that the "not staging" warnings may potentially be interspersed
with the normal dry-run output. I think that's OK. But another
alternative would be to collect the paths and then print:
warning: In Git 2.0, ...
The following deleted paths were not considered under the current
rule. Use "git add -A" to stage their removal now.
foo
bar
-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: What's cooking in git.git (Apr 2013, #05; Mon, 15)
Jeff King writes:
> +static const char *add_would_remove_warning = N_(
> +/* indent for "warning: " */
> + "In Git 2.0, 'git add ...' will also update the\n"
> +"index for paths removed from the working tree that match the given\n"
> +"pathspec. If you want to 'add' only changed or newly created paths,\n"
> +"say 'git add --no-all ...' instead.\n");
> +
> static void warn_add_would_remove(const char *path)
> {
> - warning(_("In Git 2.0, 'git add ...' will also update the\n"
> - "index for paths removed from the working tree that match\n"
> - "the given pathspec. If you want to 'add' only changed\n"
> - "or newly created paths, say 'git add --no-all ...'"
> - " instead.\n\n"
> - "'%s' would be removed from the index without --no-all."),
> - path);
> + static int warned_once;
> + if (!warned_once++)
> + warning(_(add_would_remove_warning));
> + warning("did not stage removal of '%s'", path);
> }
Would "add --dry-run" say this, too?
> static void update_callback(struct diff_queue_struct *q,
> @@ -84,10 +88,8 @@ static void update_callback(struct diff_queue_struct *q,
> }
> break;
> case DIFF_STATUS_DELETED:
> - if (data->warn_add_would_remove) {
> + if (data->warn_add_would_remove)
> warn_add_would_remove(path);
> - data->warn_add_would_remove = 0;
> - }
> if (data->flags & ADD_CACHE_IGNORE_REMOVAL)
> break;
> if (!(data->flags & ADD_CACHE_PRETEND))
--
To unsubscribe from this list: send the line "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] remote-hg: fix commit messages
Felipe Contreras writes: > git fast-import expects an extra newline after the commit message data, > but we are adding it only on hg-git compat mode, which is why the > bidirectionality tests pass. > > We should add it unconditionally. > > Signed-off-by: Felipe Contreras > --- Without knowing that hg-git compat mode is what is used in bidi test (the only mode that supports bidi), "which is why" was ungrokkable. This is a trivial change without downside risk so I do not mind applying it to 'maint', as you say it is an appropriate there. > contrib/remote-helpers/git-remote-hg | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/contrib/remote-helpers/git-remote-hg > b/contrib/remote-helpers/git-remote-hg > index a5f0013..5481331 100755 > --- a/contrib/remote-helpers/git-remote-hg > +++ b/contrib/remote-helpers/git-remote-hg > @@ -362,6 +362,8 @@ def export_ref(repo, name, kind, head): > else: > modified, removed = get_filechanges(repo, c, parents[0]) > > +desc += '\n' > + > if mode == 'hg': > extra_msg = '' > > @@ -385,7 +387,6 @@ def export_ref(repo, name, kind, head): > else: > extra_msg += "extra : %s : %s\n" % (key, > urllib.quote(value)) > > -desc += '\n' > if extra_msg: > desc += '\n--HG--\n' + extra_msg -- To unsubscribe from this list: send the line "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 6/6] transport-helper: update remote helper namespace
Felipe Contreras writes: > When pushing, the remote namespace is updated correctly > (e.g. refs/origin/master), but not the remote helper's > (e.g. refs/testgit/origin/master), which currently is only updated while > fetching. > > Since the remote namespace is used to tell fast-export which commits to > avoid (because they were already imported/exported), it makes sense to > have them in sync so they don't get generated twice. If the remote > helper was implemented properly, they would be ignored, if not, they > probably would end up repeated (probably). > > Signed-off-by: Felipe Contreras > --- This one looks suspiciously familiar ;-). I'll drop the last one I queued and pushed out a few days ago and replace it with these 6 patches. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 4/6] transport-helper: warn when refspec is not used
Felipe Contreras writes:
> For the modes that need it. In the future we should probably error out,
> instead of providing half-assed support.
>
> The reason we want to do this is because if it's not present, the remote
> helper might be updating refs/heads/*, or refs/remotes/origin/*,
> directly, and in the process fetch will get confused trying to update
> refs that are already updated, or older than what they should be. We
> shouldn't be messing with the rest of git.
So that answers my question in the response to an earlier one in
this series. We expect the ref updates to be done by the fetch or
push that drives the helper, and do not want the helper to interfere
with its ref updates.
So it is not just 'refspec' _allows_ the refs to be constrained to a
private namespace, like the earlier updates made the documentation
say; it _is_ mandatory to use refspecs to constrain them to avoid
touching refs/heads and refs/remotes namespace.
Am I reading you correctly?
Assuming I am, the patch in this message looks reasonable.
It makes the documentation updates a few patches ago look a bit
wanting, though.
Thanks.
> Signed-off-by: Felipe Contreras
> ---
> t/t5801-remote-helpers.sh | 6 --
> transport-helper.c| 2 ++
> 2 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh
> index 3eeb309..1bb7529 100755
> --- a/t/t5801-remote-helpers.sh
> +++ b/t/t5801-remote-helpers.sh
> @@ -100,14 +100,16 @@ test_expect_failure 'push new branch with old:new
> refspec' '
>
> test_expect_success 'cloning without refspec' '
> GIT_REMOTE_TESTGIT_REFSPEC="" \
> - git clone "testgit::${PWD}/server" local2 &&
> + git clone "testgit::${PWD}/server" local2 2> error &&
> + grep "This remote helper should implement refspec capability" error &&
> compare_refs local2 HEAD server HEAD
> '
>
> test_expect_success 'pulling without refspecs' '
> (cd local2 &&
> git reset --hard &&
> - GIT_REMOTE_TESTGIT_REFSPEC="" git pull) &&
> + GIT_REMOTE_TESTGIT_REFSPEC="" git pull 2> ../error) &&
> + grep "This remote helper should implement refspec capability" error &&
> compare_refs local2 HEAD server HEAD
> '
>
> diff --git a/transport-helper.c b/transport-helper.c
> index 4d98567..573eaf7 100644
> --- a/transport-helper.c
> +++ b/transport-helper.c
> @@ -215,6 +215,8 @@ static struct child_process *get_helper(struct transport
> *transport)
> free((char *)refspecs[i]);
> }
> free(refspecs);
> + } else if (data->import || data->bidi_import || data->export) {
> + warning("This remote helper should implement refspec
> capability.");
> }
> strbuf_release(&buf);
> if (debug)
--
To unsubscribe from this list: send the line "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 3/6] transport-helper: clarify pushing without refspecs
John Keeping writes:
>> +It is recommended that all importers providing the 'import'
>> +capability use this. It's mandatory for 'export'.
>
> s/It's/It is/
I personally do not care _too_ deeply either way, but I agree our
documentation tends to use the latter more and being consistent
would be good.
>> diff --git a/transport-helper.c b/transport-helper.c
>> index cea787c..4d98567 100644
>> --- a/transport-helper.c
>> +++ b/transport-helper.c
>> @@ -785,6 +785,9 @@ static int push_refs_with_export(struct transport
>> *transport,
>> struct string_list revlist_args = STRING_LIST_INIT_NODUP;
>> struct strbuf buf = STRBUF_INIT;
>>
>> +if (!data->refspecs)
>> +die("remote-helper doesn't support push; refspec needed");
>
> I think the "refspec needed" text is likely to be confusing if an
> end-user ever sees this message. I'm not sure how we can provide useful
> feedback for both remote helper authors and end-users though.
This "refspecs" only come via the helper and not directly from the
end user, no?
If that is the case, I do not think "confusing" is much of an issue.
Not _("localizing") is also the right thing to do. We may want to
say "BUG: " at front to clarify it is not the end-user's fault, but
a problem they may want to report. If we at this point know what
helper attempted export without giving refspecs, it may help to show
it here, so that developers will know with what helper the user
had problems with.
--
To unsubscribe from this list: send the line "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 3/6] transport-helper: clarify pushing without refspecs
Felipe Contreras writes:
> This has never worked, since it's inception the code simply skips all
> the refs, essentially telling fast-export to do nothing.
Good description.
>
> Let's at least tell the user what's going on.
>
> Signed-off-by: Felipe Contreras
> ---
> Documentation/gitremote-helpers.txt | 4 ++--
> t/t5801-remote-helpers.sh | 6 +++---
> transport-helper.c | 5 +++--
> 3 files changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/Documentation/gitremote-helpers.txt
> b/Documentation/gitremote-helpers.txt
> index ba7240c..4d26e37 100644
> --- a/Documentation/gitremote-helpers.txt
> +++ b/Documentation/gitremote-helpers.txt
> @@ -162,8 +162,8 @@ Miscellaneous capabilities
> For remote helpers that implement 'import' or 'export', this capability
> allows the refs to be constrained to a private namespace, instead of
> writing to refs/heads or refs/remotes directly.
> - It is recommended that all importers providing the 'import' or 'export'
> - capabilities use this.
> + It is recommended that all importers providing the 'import'
> + capability use this. It's mandatory for 'export'.
As [1/6] said "*:*" does not work and has never worked, and
especially on the push side the patch below makes it clear it was a
glorified no-op, it may be better to say a bit more than "use
this. It's mandatory". It is not like any value makes sense, right?
Perhaps the documentation will be further updated in a later step in
the series to clarify it. Let's read on til the end of the series...
> diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh
> index cd1873c..3eeb309 100755
> --- a/t/t5801-remote-helpers.sh
> +++ b/t/t5801-remote-helpers.sh
> @@ -111,13 +111,13 @@ test_expect_success 'pulling without refspecs' '
> compare_refs local2 HEAD server HEAD
> '
>
> -test_expect_failure 'pushing without refspecs' '
> +test_expect_success 'pushing without refspecs' '
> test_when_finished "(cd local2 && git reset --hard origin)" &&
> (cd local2 &&
> echo content >>file &&
> git commit -a -m ten &&
> - GIT_REMOTE_TESTGIT_REFSPEC="" git push) &&
> - compare_refs local2 HEAD server HEAD
> + GIT_REMOTE_TESTGIT_REFSPEC="" test_must_fail git push 2> ../error) &&
> + grep "remote-helper doesn.t support push; refspec needed" error
> '
I somehow like this change that turns a _failure into a _success by
expecting test_must_fail, especially because it is clear why the
tested "push" should fail when you look at the rest of the patch ;-)
Fun.
> diff --git a/transport-helper.c b/transport-helper.c
> index cea787c..4d98567 100644
> --- a/transport-helper.c
> +++ b/transport-helper.c
> @@ -785,6 +785,9 @@ static int push_refs_with_export(struct transport
> *transport,
> struct string_list revlist_args = STRING_LIST_INIT_NODUP;
> struct strbuf buf = STRBUF_INIT;
>
> + if (!data->refspecs)
> + die("remote-helper doesn't support push; refspec needed");
> +
> helper = get_helper(transport);
>
> write_constant(helper->in, "export\n");
> @@ -795,8 +798,6 @@ static int push_refs_with_export(struct transport
> *transport,
> char *private;
> unsigned char sha1[20];
>
> - if (!data->refspecs)
> - continue;
> private = apply_refspecs(data->refspecs, data->refspec_nr,
> ref->name);
> if (private && !get_sha1(private, sha1)) {
> strbuf_addf(&buf, "^%s", private);
--
To unsubscribe from this list: send the line "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 (Apr 2013, #05; Mon, 15)
On Wed, Apr 17, 2013 at 06:39:06PM -0700, Junio C Hamano wrote:
> Subject: [PATCH] git add: rework the logic to warn "git add ..."
> default change
>
> [...]
>
> Rework the logic to detect the case where the behaviour will be
> different in Git 2.0, and issue a warning only when it matters.
> Even with the code before this warning, "git add subdir" will have
> to traverse the directory in order to find _new_ files the index
> does not know about _anyway_, so we can do this check without adding
> an extra pass to find if matches any removed file.
Thanks, I think this is looking much better.
A few minor nits on the message itself:
> +static void warn_add_would_remove(const char *path)
> +{
> + warning(_("In Git 2.0, 'git add ...' will also update the\n"
> + "index for paths removed from the working tree that match\n"
> + "the given pathspec. If you want to 'add' only changed\n"
> + "or newly created paths, say 'git add --no-all ...'"
> + " instead.\n\n"
This wrapping looks funny in the actual output, due to the extra
"warning:" and the lack of newline before "instead":
warning: In Git 2.0, 'git add ...' will also update the
index for paths removed from the working tree that match
the given pathspec. If you want to 'add' only changed
or newly created paths, say 'git add --no-all ...' instead.
Wrapping it like this ends up with a much more natural-looking
paragraph:
warning: In Git 2.0, 'git add ...' will also update the
index for paths removed from the working tree that match the given
pathspec. If you want to 'add' only changed or newly created paths,
say 'git add --no-all ...' instead.
> + "'%s' would be removed from the index without --no-all."),
> + path);
I think mentioning the filename is a good thing; the original message
left me scratching my head and wondering "so, did you add it or not?".
I still think your "would be" is unnecessarily confusing, though. It is
"would be in Git 2.0 without --no-all, but we did not now". Which makes
sense when you think about it, but it took me a moment to parse.
Perhaps we can be more direct with something like:
warning: did not stage removal of 'foo'
or perhaps the present tense "not staging removal of..." would be
better.
I also think it makes sense to show every path that is affected, not
just the first one, to be clear about what was done (and what _would_
have been done in Git 2.0).
A patch with all of the suggestions together is below. I still think the
multi-line warning block looks ugly. I kind of like the way advise()
puts "hint:" on each line. I wonder if we should do the same here.
diff --git a/builtin/add.c b/builtin/add.c
index 4242bce..aae550a 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -52,15 +52,19 @@ static void warn_add_would_remove(const char *path)
return DIFF_STATUS_MODIFIED;
}
+static const char *add_would_remove_warning = N_(
+/* indent for "warning: " */
+ "In Git 2.0, 'git add ...' will also update the\n"
+"index for paths removed from the working tree that match the given\n"
+"pathspec. If you want to 'add' only changed or newly created paths,\n"
+"say 'git add --no-all ...' instead.\n");
+
static void warn_add_would_remove(const char *path)
{
- warning(_("In Git 2.0, 'git add ...' will also update the\n"
- "index for paths removed from the working tree that match\n"
- "the given pathspec. If you want to 'add' only changed\n"
- "or newly created paths, say 'git add --no-all ...'"
- " instead.\n\n"
- "'%s' would be removed from the index without --no-all."),
- path);
+ static int warned_once;
+ if (!warned_once++)
+ warning(_(add_would_remove_warning));
+ warning("did not stage removal of '%s'", path);
}
static void update_callback(struct diff_queue_struct *q,
@@ -84,10 +88,8 @@ static void update_callback(struct diff_queue_struct *q,
}
break;
case DIFF_STATUS_DELETED:
- if (data->warn_add_would_remove) {
+ if (data->warn_add_would_remove)
warn_add_would_remove(path);
- data->warn_add_would_remove = 0;
- }
if (data->flags & ADD_CACHE_IGNORE_REMOVAL)
break;
if (!(data->flags & ADD_CACHE_PRETEND))
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/6] transport-helper: clarify *:* refspec
Felipe Contreras writes: > The *:* refspec doesn't work, and never has, clarify the code and > documentation to reflect that. This in effect reverts commit 9e7673e > (gitremote-helpers(1): clarify refspec behaviour). > ... > applicable refspec takes precedence. The left-hand of refspecs > advertised with this capability must cover all refs reported by > -the list command. If a helper does not need a specific 'refspec' > -capability then it should advertise `refspec *:*`. > +the list command. If no 'refspec' capability is advertised, > +there is an implied `refspec *:*`. I presume s/.$/, but `*:*` does not work so do not use it./ is implied? > diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh > index f387027..cd1873c 100755 > --- a/t/t5801-remote-helpers.sh > +++ b/t/t5801-remote-helpers.sh > @@ -120,21 +120,6 @@ test_expect_failure 'pushing without refspecs' ' > compare_refs local2 HEAD server HEAD > ' > > -test_expect_success 'pulling with straight refspec' ' > - (cd local2 && > - GIT_REMOTE_TESTGIT_REFSPEC="*:*" git pull) && > - compare_refs local2 HEAD server HEAD > -' This one made me raise my eyebrows, first. The only reason this test "passes" is because this particular "pull", due to what local2 and server happens to have before this test runs, is an "Already up-to-date" and a no-op. You are removing this because it is not really testing anything useful, and if you change it to test something real, e.g. by rewinding local2, it will reveal the breakage. Am I reading you correctly? > diff --git a/transport-helper.c b/transport-helper.c > index dcd8d97..cea787c 100644 > --- a/transport-helper.c > +++ b/transport-helper.c > @@ -469,7 +469,7 @@ static int fetch_with_import(struct transport *transport, >* were fetching. >* >* (If no "refspec" capability was specified, for historical > - * reasons we default to *:*.) > + * reasons we default to the equivalent of *:*.) >* >* Store the result in to_fetch[i].old_sha1. Callers such >* as "git fetch" can use the value to write feedback to the -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 07/13] utf8.c: add reencode_string_len() that can handle NULs in string
Nguyễn Thái Ngọc Duy writes:
> diff --git a/utf8.h b/utf8.h
> index d3da96f..a43ef9a 100644
> --- a/utf8.h
> +++ b/utf8.h
> @@ -17,12 +17,25 @@ void strbuf_add_wrapped_bytes(struct strbuf *buf, const
> char *data, int len,
>int indent, int indent2, int width);
>
> #ifndef NO_ICONV
> -char *reencode_string_iconv(const char *in, size_t insz, iconv_t conv);
> -char *reencode_string(const char *in, const char *out_encoding, const char
> *in_encoding);
> +char *reencode_string_iconv(const char *in, size_t insz,
> + iconv_t conv, int *outsz);
> +char *reencode_string_len(const char *in, int insz,
> + const char *out_encoding,
> + const char *in_encoding,
> + int *outsz);
> #else
> -#define reencode_string(a,b,c) NULL
> +#define reencode_string_len(a,b,c,d,e) NULL
> #endif
>
> +static inline char *reencode_string(const char *in,
> + const char *out_encoding,
> + const char *in_encoding)
> +{
> + return reencode_string_len(in, strlen(in),
> +out_encoding, in_encoding,
> +NULL);
> +}
> +
> int mbs_chrlen(const char **text, size_t *remainder_p, const char *encoding);
>
> #endif
Nicely done.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Bug with rev-list --reverse?
Thomas Rast writes: > You could come up with a patch series that first starts emitting > warnings whenever the user asks for behavior that will change, and later > flips the default and removes the warning (the latter would be merged > for 2.0 or so). Please don't. The fact that "reverse then count" mode may be useful does not mean "count then show in reverse" mode does not have any use. "git log --oneline --reverse -20" is a very valid way to ask "what did I do recently" and get "you did this, that and then..." in that order. Adding a new option can be done anytime without any complex transition plan. You may want to introduce a --show-in-reverse synonym to the current --reverse when you add the new mode of reversing (--reverse-before-count?) so that eventually we won't have to ask "which kind of reverse an unadorned --reverse option mean?" by deprecating a plain "--reverse", though. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv2] l10n: de.po: translate 39 new messages
Translate 39 new messages came from git.pot update in c138af5 (l10n: git.pot: v1.8.3 round 1 (54 new, 15 removed)). While at there, fix some small issues. Signed-off-by: Ralf Thielow Acked-by: Thomas Rast --- po/de.po | 206 +-- 1 file changed, 108 insertions(+), 98 deletions(-) diff --git a/po/de.po b/po/de.po index 2cd3fa9..88d7919 100644 --- a/po/de.po +++ b/po/de.po @@ -136,12 +136,13 @@ msgstr "" #: branch.c:201 #, c-format msgid "Cannot setup tracking information; starting point '%s' is not a branch." -msgstr "" +msgstr "Kann Informationen zum Übernahmezweig nicht einrichten; " +"Startpunkt '%s' ist kein Zweig." #: branch.c:203 -#, fuzzy, c-format +#, c-format msgid "the requested upstream branch '%s' does not exist" -msgstr "Zweig '%s' existiert nicht" +msgstr "der angeforderte externe Übernahmezweig '%s' existiert nicht" #: branch.c:205 msgid "" @@ -154,6 +155,15 @@ msgid "" "will track its remote counterpart, you may want to use\n" "\"git push -u\" to set the upstream config as you push." msgstr "" +"\n" +"Falls Sie vorhaben, Ihre Arbeit auf einem bereits existierenden,\n" +"externen Übernahmezweig aufzubauen, sollten Sie \"git fetch\"\n" +"ausführen, um diesen abzurufen.\n" +"\n" +"Falls Sie vorhaben, einen neuen lokalen Zweig zu versenden\n" +"der seinem externen Gegenstück folgen soll, können Sie\n" +"\"git push -u\" verwenden, um den externen Übernahmezweig\n" +"beim Versand zu konfigurieren." #: bundle.c:36 #, c-format @@ -181,22 +191,22 @@ msgid "revision walk setup failed" msgstr "Einrichtung des Revisionsgangs fehlgeschlagen" #: bundle.c:186 -#, fuzzy, c-format +#, c-format msgid "The bundle contains this ref:" msgid_plural "The bundle contains these %d refs:" -msgstr[0] "Das Paket enthält %d Referenz" -msgstr[1] "Das Paket enthält %d Referenzen" +msgstr[0] "Das Paket enthält diese Referenz:" +msgstr[1] "Das Paket enthält diese %d Referenzen:" #: bundle.c:193 msgid "The bundle records a complete history." msgstr "Das Paket speichert eine komplette Historie." #: bundle.c:195 -#, fuzzy, c-format +#, c-format msgid "The bundle requires this ref:" msgid_plural "The bundle requires these %d refs:" -msgstr[0] "Das Paket benötigt diese Referenz" -msgstr[1] "Das Paket benötigt diese %d Referenzen" +msgstr[0] "Das Paket benötigt diese Referenz:" +msgstr[1] "Das Paket benötigt diese %d Referenzen:" #: bundle.c:294 msgid "rev-list died" @@ -731,7 +741,7 @@ msgid "Unable to write index." msgstr "Konnte Bereitstellung nicht schreiben." #: object.c:195 -#, fuzzy, c-format +#, c-format msgid "unable to parse object: %s" msgstr "Konnte Objekt '%s' nicht parsen." @@ -933,7 +943,7 @@ msgstr "Kann keine Versionsbeschreibung für %s bekommen" #: sequencer.c:621 #, c-format msgid "could not revert %s... %s" -msgstr "Konnte %s nicht zurücksetzen... %s" +msgstr "Konnte %s nicht zurücknehmen... %s" #: sequencer.c:622 #, c-format @@ -1056,11 +1066,11 @@ msgstr "Konnte %s nicht formatieren." #: sequencer.c:1101 msgid "Can't revert as initial commit" -msgstr "Kann nicht zu initialer Version zurücksetzen." +msgstr "Rücknahme-Version kann nicht initial sein." #: sequencer.c:1102 msgid "Can't cherry-pick into empty head" -msgstr "Kann \"cherry-pick\" nicht in einem leerem Kopf ausführen." +msgstr "Kann \"cherry-pick\" nicht in einem leeren Zweig ausführen." #: sha1_name.c:1036 msgid "HEAD does not point to a branch" @@ -1376,33 +1386,29 @@ msgid " (all conflicts fixed: run \"git commit\")" msgstr " (alle Konflikte behoben: führen Sie \"git commit\" aus)" #: wt-status.c:972 -#, fuzzy, c-format +#, c-format msgid "You are currently reverting commit %s." -msgstr "Sie sind gerade bei einer binären Suche in Zweig '%s'." +msgstr "Sie nehmen gerade Version '%s' zurück." #: wt-status.c:977 -#, fuzzy msgid " (fix conflicts and run \"git revert --continue\")" msgstr "" -" (beheben Sie die Konflikte und führen Sie dann \"git rebase --continue\" " +" (beheben Sie die Konflikte und führen Sie dann \"git revert --continue\" " "aus)" #: wt-status.c:980 -#, fuzzy msgid " (all conflicts fixed: run \"git revert --continue\")" -msgstr " (alle Konflikte behoben: führen Sie \"git rebase --continue\" aus)" +msgstr " (alle Konflikte behoben: führen Sie \"git revert --continue\" aus)" #: wt-status.c:982 -#, fuzzy msgid " (use \"git revert --abort\" to cancel the revert operation)" msgstr "" -" (benutzen Sie \"git rebase --abort\" um den ursprünglichen Zweig " -"auszuchecken)" +" (benutzen Sie \"git revert --abort\" um die Umkehroperation abzubrechen)" #: wt-status.c:993 -#, fuzzy, c-format +#, c-format msgid "You are currently bisecting, started from branch '%s'." -msgstr "Sie sind gerade bei einer binären Suche in Zweig '%s'." +msgstr "Sie sind gerade bei einer binären Suche, gestartet von Zweig '%s'." #: wt-status.c:997 msgid "You are currently bisecting." @@ -1419,13 +1425,
Re: t6200: avoid path mangling issue on Windows
Johannes Sixt writes: > From: Johannes Sixt > > MSYS bash interprets the slash in the argument core.commentchar="/" > as root directory and mangles it into a Windows style path. Use a > different core.commentchar to dodge the issue. > > Signed-off-by: Johannes Sixt > ... > - git -c core.commentchar="/" fmt-merge-msg --log=5 <.git/FETCH_HEAD > >actual && > + git -c core.commentchar="x" fmt-merge-msg --log=5 <.git/FETCH_HEAD > >actual && Sigh... Again? Are folks working on Msys bash aware that sometimes the users may want to say key=value on their command line without the value getting molested in any way and giving them some escape hatch would help them? Perhaps they have already decided that it is not feasible after thinking about the issue, in which case I do not have new ideas to offer. I'll apply the patch as-is, but this feels really painful to the users. Thanks. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] blame: handle broken commit headers gracefully
On Wed, Apr 17, 2013 at 02:55:29PM -0700, Junio C Hamano wrote: > Or you can imagine nastier input strings, like > >Name <>- 123456789 - >Name - 123456789 - >Name 56789 - > > I am afraid that at some point "we should salvage as much as we > can", which is a worthy goal, becomes a losing proposition. Good point. In the worst cases, even if you cleaned things up, you might even need to allocate a new string (like your middle one), which would make calling split_ident_line a lot more annoying. Probably not worth the effort. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] t3400 (rebase): add failing test for a peculiar rev spec
Ramkumar Ramachandra writes:
> 'git rebase' does not recognize revisions specified as :/text. This
> is because the attempts to rev-parse ${REV}^0, which fails in this
> case. Add a test to document this failure.
> - The failure occurs in git-rebase.sh:403. Is it using the ^0 only
>to make sure that the revision specified is a commit? Surely,
>there'a a better way to do this?
>
> Can someone point me in the right direction?
How about ${REV}^0 into
nREV=$(git rev-parse "${REV}")^0
and use it where you need an object name that needs to be parsed by
get_sha1(), e.g.
git checkout -q "$nREV^0"
I would suggest a helper function in git-sh-setup, something like:
peel_committish () {
case "$1" in
:/*)
peeltmp=$(git rev-parse --verify "$1") &&
git rev-parse --verify "$peeltmp^0"
;;
*)
git rev-parse --verify "$1^0"
;;
esac
}
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git p4 submit failing
The issue is caused by the line endings. I retested the problem with a different file and in this case, the error is caused by the line endings of the file checked out in the perforce workspace being win-style crlf, and the line endings of the file in the git repo being unix style lf. (In this scenario, I took out the .gitattributes, core.autocrlf was set to false and LineEnd was set to share) In this case, I checked out the file in perforce, ran dos2unix against it, and submitted that, then ran git p4 submit and it worked. I noticed that the error is caused by the git apply failing in the def applyCommit(self, id) function at lines 1296-1305. diffcmd = "git format-patch -k --stdout \"%s^\"..\"%s\"" % (id, id) patchcmd = diffcmd + " | git apply " tryPatchCmd = patchcmd + "--check -" applyPatchCmd = patchcmd + "--check --apply -" patch_succeeded = True if os.system(tryPatchCmd) != 0: fixed_rcs_keywords = False patch_succeeded = False print "Unfortunately applying the change failed!" So most likely in git apply command, it can't find the changes because of the line endings being different between them. I couldn't find a parameter that would magically make it work. When I added --verbose to git apply the output only says: error: while searching for: Hello Simon, I have CCed you to alert you to the possible bug. Any assistance would be appreciated. On Sat, Apr 13, 2013 at 5:09 PM, Christopher Yee Mon wrote: > Yes this is the case. > > Many of the files have crlf endings. > > core.autocrlf was recently set to input. I can't remember the timeline > exactly though, but in addition to this, I have a .gitattributes file > with the default set to text=auto (* text=auto) and the php files set > to text eol=lf (*.php text eol=lf) Also my perforce workspace's > LineEnd setting is set to Share. > > I've experienced the behavior in both .php and .xml files though > > Before all of this started I had core.autocrlf set to false, and no > .gitattributes file and perforce workspace's LineEnd was set to the > default, but I got a conflict where the only difference was the line > endings, so I changed things to the way they are now. > > Any recommendations? Should I change everything back the way it was? > > On Sat, Apr 13, 2013 at 5:51 PM, Pete Wyckoff wrote: >> l...@diamand.org wrote on Thu, 11 Apr 2013 21:19 +0100: >>> Just a thought, but check the files that are failing to see if they've >>> got RCS keywords in them ($Id$, $File$, $Date$, etc). These cause all >>> sorts of nasty problems. >>> >>> That's assuming it's definitely not a CRLF line ending problem on Windows. >> >> I had recently debugged a similar-looking problem >> where core.autocrlf was set to "input". >> >> Christopher, if you have this set and/or the .xml files >> have ^M (CRLF) line endings, please let us know. >> >> -- Pete >> >>> >>> On Thu, Apr 11, 2013 at 8:01 PM, Christopher Yee Mon >>> wrote: >>> > I tried running git p4 submit on a repo that I've been running as an >>> > interim bridge between git and perforce. Multiple people are using the >>> > repo as a remote and its being periodically submitted back to >>> > perforce. >>> > >>> > It's been working mostly fine. Then one day out of the blue I get this >>> > error. I can no longer push any git commits to perforce. (This is from >>> > the remote repo which I am pushing back to perforce) >>> > >>> > user@hostname:~/Source/code$ git p4 submit -M --export-labels >>> > Perforce checkout for depot path //depot/perforce/workspace/ located >>> > at /home/user/Source/git-p4-area/perforce/workspace/ >>> > Synchronizing p4 checkout... >>> > ... - file(s) up-to-date. >>> > Applying ffa390f comments in config xml files >>> > //depot/perforce/workspace/sub/folder/structure/first.xml#3 - opened for >>> > edit >>> > //depot/perforce/workspace/sub/folder/structure/second.xml#3 - opened for >>> > edit >>> > //depot/perforce/workspace/sub/folder/structure/third.xml#3 - opened for >>> > edit >>> > //depot/perforce/workspace/sub/folder/structure/forth.xml#3 - opened for >>> > edit >>> > //depot/perforce/workspace/sub/folder/structure/fifth.xml#1 - opened for >>> > edit >>> > error: patch failed: sub/folder/structure/first.xml:1 >>> > error: sub/folder/structure/first.xml: patch does not apply >>> > error: patch failed: sub/folder/structure/second.xml:1 >>> > error: sub/folder/structure/second.xml: patch does not apply >>> > error: patch failed: sub/folder/structure/third.xml:1 >>> > error: sub/folder/structure/third.xml: patch does not apply >>> > error: patch failed: sub/folder/structure/forth.xml:1 >>> > error: sub/folder/structure/forth.xml: patch does not apply >>> > error: patch failed: sub/folder/structure/fifth.xml:1 >>> > error: sub/folder/structure/fifth.xml: patch does not apply >>> > Unfortunately applying the change failed! >>> > //depot/perforce/workspace/sub/folder/structure/first.xml#1
Re: Git crash in Ubuntu 12.04
Hi, The git crashed during one of the commits by a developer I think, the remote is not even showing the working branch. The local branch of is all right, but the remote repo is corrupted and could not git fsck also. Is restoring the last night's backup is my only option?? $ git remote show origin * remote origin Fetch URL: git@gitserver:sggroup/sgrid.git Push URL: git@gitserver:sggroup/sgrid.git HEAD branch: master Remote branches: SGRID_5_5_0_BRANCH tracked master tracked refs/remotes/origin/4_4_Release_Branch stale (use 'git remote prune' to remo ve) Local branch configured for 'git pull': 4_4_Release_Branch merges with remote 4_4_Release_Branch Thanks, ./Siva. On Thu, Apr 18, 2013 at 5:02 PM, Sivaram Kannan wrote: > Hi, > >> Probably not because there are no debugging symbols. Not sure how >> ubuntu packages these symbols.. > > Would recompiling the source packages and debugging would give > different results? > >> >> Any chance you could publish the repository that causes the crash? >> -- >> Duy > > I don't think I can publish the repo online. But I am willing to try > any steps on the repo to get more info. Most likely I would restore > the latest repo to the stanby server I tested which works with any > crash. Let me know what I can do to get more information. > > ./Siva. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/2] submodule: drop the top-level requirement
On Thu, Apr 18, 2013 at 08:16:42PM +0530, Ramkumar Ramachandra wrote:
> John Keeping wrote:
> > Use the new rev-parse --prefix option to process all paths given to the
> > submodule command, dropping the requirement that it be run from the
> > top-level of the repository.
>
> Yay!
>
> > diff --git a/git-submodule.sh b/git-submodule.sh
> > index 79bfaac..bbf7983 100755
> > --- a/git-submodule.sh
> > +++ b/git-submodule.sh
> > @@ -112,6 +115,7 @@ resolve_relative_url ()
> > #
> > module_list()
> > {
> > + eval "set $(git rev-parse --sq --prefix "$wt_prefix" -- "$@")"
>
> Nit: Why not use "--" to disambiguate between options and arguments,
> like you showed in your intended usage?
Erm... it does. What's before "$@"?
Ah, you mean after "set". In this case, rev-parse will keep the "--" we
supply to it, so we get that from there and do not want to give set an
extra one.
> So, this essentially converts all the paths specified in "$@" to
> toplevel-relative paths. Beautiful, as this is practically the only
> change you require in each function.
>
> > + sm_path="$wt_prefix$sm_path"
>
> Wait, isn't sm_path the value of submodule..path in .gitmodules?
> Why does it need to be prefixed?
I think you've lost too much context here. This is in cmd_add and at
this point sm_path holds the argument supplied by the user, which we
must adjust as it will be relative to the current directory.
We should probably be testing for an absolute path here though.
> > @@ -942,6 +948,7 @@ cmd_summary() {
> > fi
> >
> > cd_to_toplevel
> > + eval "set $(git rev-parse --sq --prefix "$wt_prefix" -- "$@")"
>
> Um, what about other functions? Yeah, it looks like all of them
> except this one call module_list().
Exactly. Apart from this and cmd_add everything uses module_list.
> > diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
> > index ff26535..7795f21 100755
> > --- a/t/t7400-submodule-basic.sh
> > +++ b/t/t7400-submodule-basic.sh
> > @@ -212,6 +212,23 @@ test_expect_success 'submodule add with ./, /.. and //
> > in path' '
> > test_cmp empty untracked
> > '
> >
> > +test_expect_success 'submodule add in subdir' '
> > + echo "refs/heads/master" >expect &&
> > + >empty &&
>
> Nit: Use : >empty. It's clearer.
>
> > + (
> > + mkdir addtest/sub &&
>
> Why is the mkdir inside the subshell? The next statement (cd) onwards
> should be in the subshell, to save you cd'ing back.
>
> > + cd addtest/sub &&
> > + git submodule add "$submodurl" ../realsubmod3 &&
> > + git submodule init
> > + ) &&
> > +
> > + rm -f heads head untracked &&
>
> Huh? What is this in the middle? The next statement (call to
> inspect) write-truncates them anyway, so this is unnecessary.
I just followed the surrounding tests here. I think it's better for
follow the pattern of the surrounding code here than get this one test
perfect. A cleanup can follow on top if someone wants to do 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 v2 2/2] submodule: drop the top-level requirement
John Keeping wrote:
> Use the new rev-parse --prefix option to process all paths given to the
> submodule command, dropping the requirement that it be run from the
> top-level of the repository.
Yay!
> diff --git a/git-submodule.sh b/git-submodule.sh
> index 79bfaac..bbf7983 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -112,6 +115,7 @@ resolve_relative_url ()
> #
> module_list()
> {
> + eval "set $(git rev-parse --sq --prefix "$wt_prefix" -- "$@")"
Nit: Why not use "--" to disambiguate between options and arguments,
like you showed in your intended usage?
So, this essentially converts all the paths specified in "$@" to
toplevel-relative paths. Beautiful, as this is practically the only
change you require in each function.
> + sm_path="$wt_prefix$sm_path"
Wait, isn't sm_path the value of submodule..path in .gitmodules?
Why does it need to be prefixed?
> @@ -942,6 +948,7 @@ cmd_summary() {
> fi
>
> cd_to_toplevel
> + eval "set $(git rev-parse --sq --prefix "$wt_prefix" -- "$@")"
Um, what about other functions? Yeah, it looks like all of them
except this one call module_list().
> diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
> index ff26535..7795f21 100755
> --- a/t/t7400-submodule-basic.sh
> +++ b/t/t7400-submodule-basic.sh
> @@ -212,6 +212,23 @@ test_expect_success 'submodule add with ./, /.. and //
> in path' '
> test_cmp empty untracked
> '
>
> +test_expect_success 'submodule add in subdir' '
> + echo "refs/heads/master" >expect &&
> + >empty &&
Nit: Use : >empty. It's clearer.
> + (
> + mkdir addtest/sub &&
Why is the mkdir inside the subshell? The next statement (cd) onwards
should be in the subshell, to save you cd'ing back.
> + cd addtest/sub &&
> + git submodule add "$submodurl" ../realsubmod3 &&
> + git submodule init
> + ) &&
> +
> + rm -f heads head untracked &&
Huh? What is this in the middle? The next statement (call to
inspect) write-truncates them anyway, so this is unnecessary.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/2] rev-parse: add --filename-prefix option
On Thu, Apr 18, 2013 at 07:58:25PM +0530, Ramkumar Ramachandra wrote:
> John Keeping wrote:
> > This adds a prefix string to any filename arguments encountered after it
> > has been specified.
>
> Very nice. I thought we'd have to resort to path mangling in shell to
> fix git-submodule.sh. Glad to see that we can go with something
> cleaner.
>
> Perhaps pull some bits from your nice Documentation into the commit message?
Good idea. I intended to re-write the commit message for v2 since the
patch was completely re-written but forgot by the time I'd sorted out
patch 2 as well. I will do for v3.
> > diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
> > index f267a1d..de894c7 100644
> > --- a/builtin/rev-parse.c
> > +++ b/builtin/rev-parse.c
> > @@ -212,11 +212,17 @@ static void show_datestring(const char *flag, const
> > char *datestr)
> > show(buffer);
> > }
> >
> > -static int show_file(const char *arg)
> > +static int show_file(const char *arg, int output_prefix)
>
> Okay, so you've essentially patched show_file() to accept an
> additional argument, and modified callers to call with this additional
> argument. I suppose
> show_(rev|reference|default|flag|rev|with_type|datestring|abbrev)
> don't need to be patched, as they are path-independent.
>
> > {
> > show_default();
> > if ((filter & (DO_NONFLAGS|DO_NOREV)) == (DO_NONFLAGS|DO_NOREV)) {
> > - show(arg);
> > + if (output_prefix) {
> > + const char *prefix = startup_info->prefix;
> > + show(prefix_filename(prefix,
> > +prefix ? strlen(prefix) : 0,
> > +arg));
> > + } else
> > + show(arg);
>
> Uh, why do you need output_prefix? If startup_info->prefix is set,
> use it. Is startup_info->prefix set by anyone by cmd_rev_parse()?
output_prefix is a flag to say "do we want to show the prefix". We need
it because show_file is used for the "--" argument separator as well as
file paths. Without a separate flag we end up prefixing "--" with the
prefix path.
> > @@ -470,6 +476,7 @@ N_("git rev-parse --parseopt [options] -- [...]\n"
> > int cmd_rev_parse(int argc, const char **argv, const char *prefix)
> > @@ -535,6 +542,13 @@ int cmd_rev_parse(int argc, const char **argv, const
> > char *prefix)
> > i++;
> > continue;
> > }
> > + if (!strcmp(arg, "--prefix")) {
> > + prefix = argv[i+1];
> > + startup_info->prefix = prefix;
> > + output_prefix = 1;
> > + i++;
> > + continue;
> > + }
>
> Wait, why isn't prefix filled in when run_builtin() calls this? Oh,
> right: because we didn't mark this builtin with RUN_SETUP or
> RUN_SETUP_GENTLY. Okay, now why didn't we change that? Because it
> would be a major problem (all our scripts would break) if rev-parse
> did cd-to-toplevel.
prefix is already set, by setup_git_git_directory. The point is that we
just change the values set in setup_git_directory so that the command
behaves as if it were run from a subdirectory.
> Why are you setting prefix to argv[i+1], and then setting
> startup_info->prefix to that? Is anyone else in cmd_rev_parse() going
> to use it?
>
> > +prefix=$(git rev-parse --show-prefix)
> > +cd "$(git rev-parse --show-toplevel)"
> > +eval "set -- $(git rev-parse --sq --prefix "$prefix" "$@")"
>
> I'm wondering if you need such a convoluted usage though. Will you
> ever need to specify a prefix by hand that is different from what git
> rev-parse --show-toplevel returns? If not, why don't you just
> rev-parse --emulate-toplevel, and get rid of specifying prefix by hand
> altogether? Then again, this is a plumbing command, so the simplicity
> is probably more valuable.
How does that work? When we run rev-parse with the --prefix argument
we're no longer in the subdirectory.
While this may look convoluted here, I don't think it is in normal usage
inside a script. If you look at the way it's used in patch 2 we're
careful not to just remap all the arguments but to extract the flags
before remapping file paths when we know that everything we have is a
file path.
> > diff --git a/t/t1513-rev-parse-prefix.sh b/t/t1513-rev-parse-prefix.sh
> > new file mode 100755
> > index 000..5ef48d2
> > --- /dev/null
> > +++ b/t/t1513-rev-parse-prefix.sh
> > +test_expect_success 'empty prefix -- file' '
> > + git rev-parse --prefix "" -- top sub1/file1 >actual &&
> > + cat <<-EOF >expected &&
>
> Nit: when you're not putting in variables, you can cat <<-\EOF.
>
> > +test_expect_success 'empty prefix HEAD:./path' '
> > + git rev-parse --prefix "" HEAD:./top >actual &&
> > +
Re: [PATCH v2 1/2] rev-parse: add --filename-prefix option
John Keeping wrote:
> This adds a prefix string to any filename arguments encountered after it
> has been specified.
Very nice. I thought we'd have to resort to path mangling in shell to
fix git-submodule.sh. Glad to see that we can go with something
cleaner.
Perhaps pull some bits from your nice Documentation into the commit message?
> diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
> index f267a1d..de894c7 100644
> --- a/builtin/rev-parse.c
> +++ b/builtin/rev-parse.c
> @@ -212,11 +212,17 @@ static void show_datestring(const char *flag, const
> char *datestr)
> show(buffer);
> }
>
> -static int show_file(const char *arg)
> +static int show_file(const char *arg, int output_prefix)
Okay, so you've essentially patched show_file() to accept an
additional argument, and modified callers to call with this additional
argument. I suppose
show_(rev|reference|default|flag|rev|with_type|datestring|abbrev)
don't need to be patched, as they are path-independent.
> {
> show_default();
> if ((filter & (DO_NONFLAGS|DO_NOREV)) == (DO_NONFLAGS|DO_NOREV)) {
> - show(arg);
> + if (output_prefix) {
> + const char *prefix = startup_info->prefix;
> + show(prefix_filename(prefix,
> +prefix ? strlen(prefix) : 0,
> +arg));
> + } else
> + show(arg);
Uh, why do you need output_prefix? If startup_info->prefix is set,
use it. Is startup_info->prefix set by anyone by cmd_rev_parse()?
> @@ -470,6 +476,7 @@ N_("git rev-parse --parseopt [options] -- [...]\n"
> int cmd_rev_parse(int argc, const char **argv, const char *prefix)
> @@ -535,6 +542,13 @@ int cmd_rev_parse(int argc, const char **argv, const
> char *prefix)
> i++;
> continue;
> }
> + if (!strcmp(arg, "--prefix")) {
> + prefix = argv[i+1];
> + startup_info->prefix = prefix;
> + output_prefix = 1;
> + i++;
> + continue;
> + }
Wait, why isn't prefix filled in when run_builtin() calls this? Oh,
right: because we didn't mark this builtin with RUN_SETUP or
RUN_SETUP_GENTLY. Okay, now why didn't we change that? Because it
would be a major problem (all our scripts would break) if rev-parse
did cd-to-toplevel.
Why are you setting prefix to argv[i+1], and then setting
startup_info->prefix to that? Is anyone else in cmd_rev_parse() going
to use it?
> +prefix=$(git rev-parse --show-prefix)
> +cd "$(git rev-parse --show-toplevel)"
> +eval "set -- $(git rev-parse --sq --prefix "$prefix" "$@")"
I'm wondering if you need such a convoluted usage though. Will you
ever need to specify a prefix by hand that is different from what git
rev-parse --show-toplevel returns? If not, why don't you just
rev-parse --emulate-toplevel, and get rid of specifying prefix by hand
altogether? Then again, this is a plumbing command, so the simplicity
is probably more valuable.
> diff --git a/t/t1513-rev-parse-prefix.sh b/t/t1513-rev-parse-prefix.sh
> new file mode 100755
> index 000..5ef48d2
> --- /dev/null
> +++ b/t/t1513-rev-parse-prefix.sh
> +test_expect_success 'empty prefix -- file' '
> + git rev-parse --prefix "" -- top sub1/file1 >actual &&
> + cat <<-EOF >expected &&
Nit: when you're not putting in variables, you can cat <<-\EOF.
> +test_expect_success 'empty prefix HEAD:./path' '
> + git rev-parse --prefix "" HEAD:./top >actual &&
> + git rev-parse HEAD:top >expected &&
Nit: why did you change "./top" to "top"? Your --prefix option
doesn't require you to change your arguments accordingly, does 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: Bug with rev-list --reverse?
Felipe Contreras writes:
> On Thu, Apr 18, 2013 at 5:47 AM, Peter Krefting
> wrote:
>
>>> % git log --oneline -1 v1.8.1.5^..v1.8.1.6
>>> % git log --oneline --reverse -1 v1.8.1.5^..v1.8.1.6
>>>
>>> I expect to get a different output, and not both showing v1.8.1.6.
>>> Wouldn't you agree?
>>
>>
>> Quoting the manual page:
>>
>> Commit Limiting
>>Besides specifying a range of commits that should be listed using the
>> special notations explained in the description, additional commit limiting
>> may be applied. Note that they are applied before commit ordering and
>> formatting options, such as --reverse.
>>
>> Given that, I would expect the output to be the same.
>
> If expectations were based on documentation, all one has to do is
> document bugs, and there would be no bugs anymore :)
>
> Code can be changed to fit more appropriately user expectations (which
> are independent of documentation), and the documentation updated
> accordingly.
It's been this way forever, and applies to rev-list where we can't just
break how options work (for fear of breaking scripts).
You could come up with a patch series that first starts emitting
warnings whenever the user asks for behavior that will change, and later
flips the default and removes the warning (the latter would be merged
for 2.0 or so).
--
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 1/6] transport-helper: clarify *:* refspec
On Thu, Apr 18, 2013 at 7:39 AM, Thomas Rast wrote: > Felipe Contreras writes: > >> On Thu, Apr 18, 2013 at 2:28 AM, Thomas Rast wrote: >>> Felipe Contreras writes: >>> The *:* refspec doesn't work, and never has, clarify the code and documentation to reflect that. This in effect reverts commit 9e7673e (gitremote-helpers(1): clarify refspec behaviour). >>> [...] -test_expect_success 'pulling with straight refspec' ' - (cd local2 && - GIT_REMOTE_TESTGIT_REFSPEC="*:*" git pull) && - compare_refs local2 HEAD server HEAD -' - -test_expect_failure 'pushing with straight refspec' ' - test_when_finished "(cd local2 && git reset --hard origin)" && - (cd local2 && - echo content >>file && - git commit -a -m eleven && - GIT_REMOTE_TESTGIT_REFSPEC="*:*" git push) && - compare_refs local2 HEAD server HEAD -' >>> >>> So what's wrong with the tests? Do they fail to test what they claim >>> (how?), test something that wasn't reasonable to begin with, or >>> something entirely different? >> >> Look at the code comment, and look at the now updated documentation >> that assumes that *:* was reasonable. Given the available information, >> it would be reasonable to assume that *:* did work, but it didn't >> work, and it's not really possible to fix it, even if we wanted to, it >> would be a hack. It's better to accept that fact and stop worrying too >> much about what would be the best way to do the wrong thing. > > Ok, you say that the *failing* test set an expectation that is > unrealistic, so let's drop it. > > But then what about the successful test? Does it actually work (and by > removing the test, you are saying that we don't care if we subsequently > break that (mis)feature)? Or did it test the wrong thing? Yeah, it works, in the sense that peeing in a bottle is a solution; it might work, but it's not recommendable. So, if suddenly working, frankly I don't care. I added those tests, and I don't think they are needed. In a not too distant future it should not be permitted to "work"; we don't want developers to shoot themselves in the foot, and heir users too. -- Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/6] transport-helper: clarify *:* refspec
Felipe Contreras writes:
> On Thu, Apr 18, 2013 at 2:28 AM, Thomas Rast wrote:
>> Felipe Contreras writes:
>>
>>> The *:* refspec doesn't work, and never has, clarify the code and
>>> documentation to reflect that. This in effect reverts commit 9e7673e
>>> (gitremote-helpers(1): clarify refspec behaviour).
>> [...]
>>> -test_expect_success 'pulling with straight refspec' '
>>> - (cd local2 &&
>>> - GIT_REMOTE_TESTGIT_REFSPEC="*:*" git pull) &&
>>> - compare_refs local2 HEAD server HEAD
>>> -'
>>> -
>>> -test_expect_failure 'pushing with straight refspec' '
>>> - test_when_finished "(cd local2 && git reset --hard origin)" &&
>>> - (cd local2 &&
>>> - echo content >>file &&
>>> - git commit -a -m eleven &&
>>> - GIT_REMOTE_TESTGIT_REFSPEC="*:*" git push) &&
>>> - compare_refs local2 HEAD server HEAD
>>> -'
>>
>> So what's wrong with the tests? Do they fail to test what they claim
>> (how?), test something that wasn't reasonable to begin with, or
>> something entirely different?
>
> Look at the code comment, and look at the now updated documentation
> that assumes that *:* was reasonable. Given the available information,
> it would be reasonable to assume that *:* did work, but it didn't
> work, and it's not really possible to fix it, even if we wanted to, it
> would be a hack. It's better to accept that fact and stop worrying too
> much about what would be the best way to do the wrong thing.
Ok, you say that the *failing* test set an expectation that is
unrealistic, so let's drop it.
But then what about the successful test? Does it actually work (and by
removing the test, you are saying that we don't care if we subsequently
break that (mis)feature)? Or did it test the wrong thing?
--
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: What's cooking in git.git (Apr 2013, #05; Mon, 15)
On Thu, Apr 18, 2013 at 6:46 AM, Ramkumar Ramachandra wrote: > Okay, one more segment needs to be responded to. > > Felipe Contreras wrote: >> If remote-hg wasn't available for users, they would be hurt; if stash >> wasn't available, if rebase --interactive didn't exist, if there was >> no msysgit, if it wasn't so fast, if the object model wasn't so simple >> and extensible; users would be hurt. And if users didn't have all >> these, there would be less users, and if there were less users, there >> would be less developers, and mercurial might have been more popular, >> and most repositories you have to work on would be in mercurial, and >> you might be developing mercurial right now. > > Flawed logic. > > A large number of users doesn't automatically imply good software with > lots of features, or even a great development community. A great > development community leads to great software. And great software > leads to lots of users. Sure, there's a feedback loop pushing users > to become developers; but it doesn't start with users of vaporware, > leading to more developers joining the effort to turn that vaporware > into a great product. > > Life doesn't begin with users. Nobody knows how life began, and it doesn't matter now, what matters is how life evolves. It doesn't matter if the chicken was first, or the egg, what matters is that if all the chickens and eggs are gone, there won't be more. Plenty of projects have died because they stopped caring about their users, and without users there's no new developers, and the old developers eventually move on, and all the literary quality of commit messages have no eyes to see it. I repeat: no project is more important than its users. -- Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Apr 2013, #05; Mon, 15)
On Thu, Apr 18, 2013 at 6:31 AM, Ramkumar Ramachandra wrote: > Since you disagreed with the rest, I'll only respond to this part: > > Felipe Contreras wrote: >> But I won't bother trying to convince you that no project is more >> important than its users (in the words of Linus Torvalds), because >> most people don't see the big picture. > > I didn't say otherwise. What I'm saying is: my personal incentive to > write code does not prioritize the supposed benefit of some unknown > "user" somewhere on the planet above everything else. My personal > incentive prioritizes me, and my immediate circle (ie. the git > community). The benefit propagates outwards to extended circles until > it reaches the people I care least about: incidental end-users. If the people that matter most are given the worst prioritization, it means the prioritization is wrong. > That's how people are connected: how can I care about distant unknown > people I'm not connected to? It's called empathy. > The people in the outermost circles > benefit the least, because they didn't get a say in the development. > All they can do is write a rant about it on their blog, and hope that > it gets fixed someday. To the detriment of the project. > You just ditched us, the inner circle of people who care about your > work the most, and are instead trying to convince us that we're > hurting some unknown hypothetical "users" by not merging your code > immediately. The users are real, the developers that will look retroatcively to the commit message of this patch are not. > If you think these users are more important to you than we are, then > why are you posting your code on this mailing list? What other way is there for this code to reach the users? > Start your own > project that's focused on satisfying these users. Start a new project so I can include a patch that hasn't made it yet into the "what's cooking" in one week? That's ridiculous. > It doesn't even > need to be open source or have a community of reviewers, because all > you care about are users. Who said *all* that matters are the users? And even if somebody did, ultimately a closed source proprietary software doesn't benefit the users, so either way it has to be open and active to benefit the users. -- Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Apr 2013, #05; Mon, 15)
Okay, one more segment needs to be responded to. Felipe Contreras wrote: > If remote-hg wasn't available for users, they would be hurt; if stash > wasn't available, if rebase --interactive didn't exist, if there was > no msysgit, if it wasn't so fast, if the object model wasn't so simple > and extensible; users would be hurt. And if users didn't have all > these, there would be less users, and if there were less users, there > would be less developers, and mercurial might have been more popular, > and most repositories you have to work on would be in mercurial, and > you might be developing mercurial right now. Flawed logic. A large number of users doesn't automatically imply good software with lots of features, or even a great development community. A great development community leads to great software. And great software leads to lots of users. Sure, there's a feedback loop pushing users to become developers; but it doesn't start with users of vaporware, leading to more developers joining the effort to turn that vaporware into a great product. Life doesn't begin with users. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Git crash in Ubuntu 12.04
Hi, > Probably not because there are no debugging symbols. Not sure how > ubuntu packages these symbols.. Would recompiling the source packages and debugging would give different results? > > Any chance you could publish the repository that causes the crash? > -- > Duy I don't think I can publish the repo online. But I am willing to try any steps on the repo to get more info. Most likely I would restore the latest repo to the stanby server I tested which works with any crash. Let me know what I can do to get more information. ./Siva. -- To unsubscribe from this list: send the line "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 (Apr 2013, #05; Mon, 15)
Since you disagreed with the rest, I'll only respond to this part: Felipe Contreras wrote: > But I won't bother trying to convince you that no project is more > important than its users (in the words of Linus Torvalds), because > most people don't see the big picture. I didn't say otherwise. What I'm saying is: my personal incentive to write code does not prioritize the supposed benefit of some unknown "user" somewhere on the planet above everything else. My personal incentive prioritizes me, and my immediate circle (ie. the git community). The benefit propagates outwards to extended circles until it reaches the people I care least about: incidental end-users. That's how people are connected: how can I care about distant unknown people I'm not connected to? The people in the outermost circles benefit the least, because they didn't get a say in the development. All they can do is write a rant about it on their blog, and hope that it gets fixed someday. You just ditched us, the inner circle of people who care about your work the most, and are instead trying to convince us that we're hurting some unknown hypothetical "users" by not merging your code immediately. If you think these users are more important to you than we are, then why are you posting your code on this mailing list? Start your own project that's focused on satisfying these users. It doesn't even need to be open source or have a community of reviewers, because all you care about are users. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Bug with rev-list --reverse?
On Thu, Apr 18, 2013 at 5:47 AM, Peter Krefting wrote: >> % git log --oneline -1 v1.8.1.5^..v1.8.1.6 >> % git log --oneline --reverse -1 v1.8.1.5^..v1.8.1.6 >> >> I expect to get a different output, and not both showing v1.8.1.6. >> Wouldn't you agree? > > > Quoting the manual page: > > Commit Limiting >Besides specifying a range of commits that should be listed using the > special notations explained in the description, additional commit limiting > may be applied. Note that they are applied before commit ordering and > formatting options, such as --reverse. > > Given that, I would expect the output to be the same. If expectations were based on documentation, all one has to do is document bugs, and there would be no bugs anymore :) Code can be changed to fit more appropriately user expectations (which are independent of documentation), and the documentation updated accordingly. -- Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Bug with rev-list --reverse?
On Thu, Apr 18, 2013 at 5:26 AM, John Keeping wrote: > On Thu, Apr 18, 2013 at 05:17:14AM -0500, Felipe Contreras wrote: >> If I do these: >> >> % git log --oneline -1 v1.8.1.5^..v1.8.1.6 >> % git log --oneline --reverse -1 v1.8.1.5^..v1.8.1.6 >> >> I expect to get a different output, and not both showing v1.8.1.6. >> Wouldn't you agree? > > I expect to get the same output. This is probably because I consider > --reverse to be an output filter. So I expect to show the commits > "v1.8.1.5^..v1.8.1.6 -1" which selects a single commit and then show > that in reverse order. How about this: % git log --oneline --reverse --max-count=1 v1.8.1.5^..v1.8.1.6 In this case --max-count is acting as "start from the first commit before the tip", not as "output a maximum of one commit". Given that the name is max-count, I expect it to be the later. And if max-count doesn't select a maximum of n commits, then what does? -- Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: put THEIR commits AFTER my commits with a single rebase command
Am 4/18/2013 10:33, schrieb Ilya Basin: > > JS> Perhaps this one: > > JS>git merge origin/master > JS>git rebase ORIG_HEAD > > JS> -- Hannes > > Wouldn't I have to resolve conflicts twice? Yes. But you did run 'git config rerere.enabled true' when you started with git, didn't you? ;-) Anyway, Johan's idea to use git cherry-pick is much better. > BTW, during the rebase, can I tell git to rewrite a different branch > upon rebase success or abort? > > git branch -f tmp origin/master > git rebase --onto master master tmp > if [ $? -ne 0 ]; then ># modify some file in .git/ ? What do you expect here? Failure of git rebase means that it is not complete, yet. So far, nothing has been rewritten. So what? Perhaps you mean: # never mind git rebase --abort > else > git branch -f master > git checkout master > fi -- Hannes -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
