[PATCH v2 0/3] fix http deadlock on giant ref negotiations
On Fri, May 15, 2015 at 02:28:37PM -0400, Konstantin Ryabitsev wrote: On 15/05/15 02:22 PM, Junio C Hamano wrote: Also, is it worth allocating small and then growing up to the maximum? I think this only relays one request at a time anyway, and I suspect that a single 1MB allocation at the first call kept getting reused may be sufficient (and much simpler). Does it make sense to make that configurable via an env variable at all? I suspect the vast majority of people would not hit this bug unless they are dealing with repositories polluted with hundreds of refs created by automation (like the codeaurora chromium repo). E.g. can be set via an Apache directive like SetEnv FOO_MAX_SIZE 2048 The backend can then be configured to emit an error message when the spool size is exhausted saying foo exhausted, increase FOO_MAX_SIZE to allow for moar foo. Yeah, that was the same conclusion I came to elsewhere in the thread. Here's a re-roll: [1/3]: http-backend: fix die recursion with custom handler [2/3]: t5551: factor out tag creation [3/3]: http-backend: spool ref negotiation requests to buffer It makes the size configurable (either through the environment, which is convenient for setting via Apache; or through the config, which is convenient if you have one absurdly-sized repo). It mentions the variable name when it barfs into the Apache log. I also bumped the default to 10MB, which I think should be enough to handle even ridiculous cases. I also adapted Dennis's test into the third patch. Beware that it's quite slow to run (and is protected by the EXPENSIVE prerequisite). Patch 2 is new, and just refactors the script to make adding the new test easier. I really wanted to add a test like: diff --git a/t/t5551-http-fetch-smart.sh b/t/t5551-http-fetch-smart.sh index e2a2fa1..1fff812 100755 --- a/t/t5551-http-fetch-smart.sh +++ b/t/t5551-http-fetch-smart.sh @@ -273,6 +273,16 @@ test_expect_success 'large fetch-pack requests can be split across POSTs' ' test_line_count = 2 posts ' +test_expect_success 'http-backend does not buffer arbitrarily large requests' ' + test_when_finished ( + cd \$HTTPD_DOCUMENT_ROOT_PATH/repo.git\ + test_unconfig http.maxrequestbuffer + ) + git -C $HTTPD_DOCUMENT_ROOT_PATH/repo.git \ + config http.maxrequestbuffer 100 + test_must_fail git clone $HTTPD_URL/smart/repo.git foo.git +' + test_expect_success EXPENSIVE 'http can handle enormous ref negotiation' ' ( cd $HTTPD_DOCUMENT_ROOT_PATH/repo.git to test that the maxRequestBuffer code does indeed work. Unfortunately, even though the server behaves reasonably in this case, the client ends up hanging forever. I'm not sure there is a simple solution to that; I think it is a protocol issue where remote-http is waiting for fetch-pack to speak, but fetch-pack is waiting for more data from the remote. Personally, I think I'd be much more interested in pursuing a saner, full duplex http solution like git-over-websockets than trying to iron out all of the corner cases in the stateless-rpc protocol. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 3/3] http-backend: spool ref negotiation requests to buffer
When http-backend spawns upload-pack to do ref
negotiation, it streams the http request body to
upload-pack, who then streams the http response back to the
client as it reads. In theory, git can go full-duplex; the
client can consume our response while it is still sending
the request. In practice, however, HTTP is a half-duplex
protocol. Even if our client is ready to read and write
simultaneously, we may have other HTTP infrastructure in the
way, including the webserver that spawns our CGI, or any
intermediate proxies.
In at least one documented case[1], this leads to deadlock
when trying a fetch over http. What happens is basically:
1. Apache proxies the request to the CGI, http-backend.
2. http-backend gzip-inflates the data and sends
the result to upload-pack.
3. upload-pack acts on the data and generates output over
the pipe back to Apache. Apache isn't reading because
it's busy writing (step 1).
This works fine most of the time, because the upload-pack
output ends up in a system pipe buffer, and Apache reads
it as soon as it finishes writing. But if both the request
and the response exceed the system pipe buffer size, then we
deadlock (Apache blocks writing to http-backend,
http-backend blocks writing to upload-pack, and upload-pack
blocks writing to Apache).
We need to break the deadlock by spooling either the input
or the output. In this case, it's ideal to spool the input,
because Apache does not start reading either stdout _or_
stderr until we have consumed all of the input. So until we
do so, we cannot even get an error message out to the
client.
The solution is fairly straight-forward: we read the request
body into an in-memory buffer in http-backend, freeing up
Apache, and then feed the data ourselves to upload-pack. But
there are a few important things to note:
1. We limit the in-memory buffer to prevent an obvious
denial-of-service attack. This is a new hard limit on
requests, but it's unlikely to come into play. The
default value is 10MB, which covers even the ridiculous
100,000-ref negotation in the included test (that
actually caps out just over 5MB). But it's configurable
on the off chance that you don't mind spending some
extra memory to make even ridiculous requests work.
2. We must take care only to buffer when we have to. For
pushes, the incoming packfile may be of arbitrary
size, and we should connect the input directly to
receive-pack. There's no deadlock problem here, though,
because we do not produce any output until the whole
packfile has been read.
For upload-pack's initial ref advertisement, we
similarly do not need to buffer. Even though we may
generate a lot of output, there is no request body at
all (i.e., it is a GET, not a POST).
[1] http://article.gmane.org/gmane.comp.version-control.git/269020
Test-adapted-from: Dennis Kaarsemaker den...@kaarsemaker.net
Signed-off-by: Jeff King p...@peff.net
---
Documentation/git-http-backend.txt | 9
http-backend.c | 97 +-
t/t5551-http-fetch-smart.sh| 15 ++
3 files changed, 110 insertions(+), 11 deletions(-)
diff --git a/Documentation/git-http-backend.txt
b/Documentation/git-http-backend.txt
index d422ba4..8c6acbe 100644
--- a/Documentation/git-http-backend.txt
+++ b/Documentation/git-http-backend.txt
@@ -255,6 +255,15 @@ The GIT_HTTP_EXPORT_ALL environmental variable may be
passed to
'git-http-backend' to bypass the check for the git-daemon-export-ok
file in each repository before allowing export of that repository.
+The `GIT_HTTP_MAX_REQUEST_BUFFER` environment variable (or the
+`http.maxRequestBuffer` config variable) may be set to change the
+largest ref negotiation request that git will handle during a fetch; any
+fetch requiring a larger buffer will not succeed. This value should not
+normally need to be changed, but may be helpful if you are fetching from
+a repository with an extremely large number of refs. The value can be
+specified with a unit (e.g., `100M` for 100 megabytes). The default is
+10 megabytes.
+
The backend process sets GIT_COMMITTER_NAME to '$REMOTE_USER' and
GIT_COMMITTER_EMAIL to '$\{REMOTE_USER}@http.$\{REMOTE_ADDR\}',
ensuring that any reflogs created by 'git-receive-pack' contain some
diff --git a/http-backend.c b/http-backend.c
index 3ad82a8..d1333b8 100644
--- a/http-backend.c
+++ b/http-backend.c
@@ -13,18 +13,20 @@ static const char content_type[] = Content-Type;
static const char content_length[] = Content-Length;
static const char last_modified[] = Last-Modified;
static int getanyfile = 1;
+static unsigned long max_request_buffer = 10 * 1024 * 1024;
static struct string_list *query_params;
struct rpc_service {
const char *name;
const char *config_name;
+ unsigned buffer_input : 1;
signed enabled : 2;
};
static struct rpc_service rpc_service[] = {
-
[PATCH v4] mergetools: add winmerge as a builtin tool
Add a winmerge scriptlet with the commands described in [1] so
that users can use winmerge without needing to perform any
additional configuration.
[1] http://thread.gmane.org/gmane.comp.version-control.git/268631
Helped-by: Philip Oakley philipoak...@iee.org
Helped-by: Johannes Schindelin johannes.schinde...@gmx.de
Helped-by: Sebastian Schuberth sschube...@gmail.com
Helped-by: SZEDER Gábor sze...@ira.uka.de
Signed-off-by: David Aguilar dav...@gmail.com
---
Changes since v3:
* type -p is used instead of type.
* printf is consistently used instead of echo.
* an env | grep pipeline is used to get the variables instead of hard-coding
a set of variables in the script, as suggested by Sebastian in
http://thread.gmane.org/gmane.comp.version-control.git/269437/focus=269441
mergetools/winmerge | 45 +
1 file changed, 45 insertions(+)
create mode 100644 mergetools/winmerge
diff --git a/mergetools/winmerge b/mergetools/winmerge
new file mode 100644
index 000..fc364c769
--- /dev/null
+++ b/mergetools/winmerge
@@ -0,0 +1,45 @@
+diff_cmd () {
+ $merge_tool_path -u -e $LOCAL $REMOTE
+ return 0
+}
+
+merge_cmd () {
+ # mergetool.winmerge.trustExitCode is implicitly false.
+ # touch $BACKUP so that we can check_unchanged.
+ touch $BACKUP
+ $merge_tool_path -u -e -dl Local -dr Remote \
+ $LOCAL $REMOTE $MERGED
+ check_unchanged
+}
+
+translate_merge_tool_path() {
+ # Use WinMergeU.exe if it exists in $PATH
+ if type -p WinMergeU.exe /dev/null 21
+ then
+ printf WinMergeU.exe
+ return
+ fi
+
+ # Look for WinMergeU.exe in the typical locations
+ winmerge_exe=WinMerge/WinMergeU.exe
+ found=false
+ OIFS=$IFS
+ IFS='
+'
+ for directory in $(env | grep -Ei '^PROGRAM(FILES(\(X86\))?|W6432)=' |
+ cut -d '=' -f 2- | sort -u)
+ do
+ if test -n $directory test -x $directory/$winmerge_exe
+ then
+ found=true
+ printf '%s' $directory/$winmerge_exe
+ break
+ fi
+ done
+ IFS=$OIFS
+
+ if test $found != true
+ then
+ printf WinMergeU.exe
+ fi
+}
--
2.4.1.218.gc09a0e5
--
To unsubscribe from this list: send the line 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 log --follow for directories
Junio C Hamano gitster at pobox.com writes: Laszlo Papp lpapp at kde.org writes: Is there any benefit about having it like it is today or is it just the usual no one has implemented it yet? It actually is even more sketchy than that. A single file following was done merely as a checkbox item that works majority of the time, and any mergy history that renames the file on one side of the side branch but not on the other may not truly follow it. Well, in worst case, why not have something like --follow-dirs, then? The case at hand is that we unfortunately named a directory based on the codename of some software for the time. Now, we have improved that software and the codename is different accordingly. Now, instead of pastcodename, I would change the directory name to src to be future proof, but here, I face these difficulties: 1) The directory name is stuck with some ancient codename and it therefore will be confusing for the rest of the life cycle for this project. 2) We get unfollowable directories. At best, we could use some scripts to work this around, but why not address this directly in git? I think renaming a directory without losing the history would be really cool to have, one way or another. If that requires a separate option, I am happy with that, but what I would really like to avoid is not being able to rename directories without losing the history. Note that I am speaking from user point of view. I do not know the nitty-gritty technical details, but that is just implementation details as far as I am concerned, anyway. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4] mergetools: add winmerge as a builtin tool
On Wed, May 20, 2015 at 9:42 AM, David Aguilar dav...@gmail.com wrote: + OIFS=$IFS + IFS=' +' I guess this is just a formatting issue with the mail export as it should read IFS=$'\n' Otherwise looks good to me. -- Sebastian Schuberth -- To unsubscribe from this list: send the line 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 v5 1/2] mergetool--lib: set IFS for difftool and mergetool
git-sh-setup sets IFS but it is not used by git-difftool--helper.
Set IFS in git-mergetool--lib so that the mergetool scriptlets,
diftool, and mergetool do not need to do so.
Signed-off-by: David Aguilar dav...@gmail.com
---
This patch is new since last time. It simplifies the patch that follows.
git-mergetool--lib.sh | 3 +++
git-mergetool.sh | 2 --
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index fe61e89..14b039d 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -2,6 +2,9 @@
: ${MERGE_TOOLS_DIR=$(git --exec-path)/mergetools}
+IFS='
+'
+
mode_ok () {
if diff_mode
then
diff --git a/git-mergetool.sh b/git-mergetool.sh
index d20581c..9f77e3a 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -451,8 +451,6 @@ fi
printf Merging:\n
printf %s\n $files
-IFS='
-'
rc=0
for i in $files
do
--
2.4.1.218.gc09a0e5
--
To unsubscribe from this list: send the line 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] pull: handle --log=n
On di, 2015-05-19 at 19:19 -0700, Junio C Hamano wrote: Hopefully now you have some idea how your approach is problematic. Yes, thanks for the thorough explanation! Two more bad sideeffects, so two more reasons not to take this approach: - test_must_fail tests might now fail for the wrong reason, undetectedly (ref does not exist instead of can't handle ref) - same for TODO tests -- Dennis Kaarsemaker www.kaarsemaker.net -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/4] for-each-ref: rename refinfo members to match similar structures
From: Jeff King p...@peff.net
Written-by: Jeff King p...@peff.net
Signed-off-by: Karthik Nayak karthik@gmail.com
---
builtin/for-each-ref.c | 40
1 file changed, 20 insertions(+), 20 deletions(-)
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 83f9cf9..2721228 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -32,9 +32,9 @@ struct ref_sort {
};
struct refinfo {
- char *refname;
- unsigned char objectname[20];
- int flag;
+ char *name;
+ unsigned char sha1[20];
+ int flags;
const char *symref;
struct atom_value *value;
};
@@ -632,9 +632,9 @@ static void populate_value(struct refinfo *ref)
ref-value = xcalloc(used_atom_cnt, sizeof(struct atom_value));
- if (need_symref (ref-flag REF_ISSYMREF) !ref-symref) {
+ if (need_symref (ref-flags REF_ISSYMREF) !ref-symref) {
unsigned char unused1[20];
- ref-symref = resolve_refdup(ref-refname, RESOLVE_REF_READING,
+ ref-symref = resolve_refdup(ref-name, RESOLVE_REF_READING,
unused1, NULL);
if (!ref-symref)
ref-symref = ;
@@ -655,14 +655,14 @@ static void populate_value(struct refinfo *ref)
}
if (starts_with(name, refname))
- refname = ref-refname;
+ refname = ref-name;
else if (starts_with(name, symref))
refname = ref-symref ? ref-symref : ;
else if (starts_with(name, upstream)) {
/* only local branches may have an upstream */
- if (!starts_with(ref-refname, refs/heads/))
+ if (!starts_with(ref-name, refs/heads/))
continue;
- branch = branch_get(ref-refname + 11);
+ branch = branch_get(ref-name + 11);
if (!branch || !branch-merge || !branch-merge[0] ||
!branch-merge[0]-dst)
@@ -677,9 +677,9 @@ static void populate_value(struct refinfo *ref)
continue;
} else if (!strcmp(name, flag)) {
char buf[256], *cp = buf;
- if (ref-flag REF_ISSYMREF)
+ if (ref-flags REF_ISSYMREF)
cp = copy_advance(cp, ,symref);
- if (ref-flag REF_ISPACKED)
+ if (ref-flags REF_ISPACKED)
cp = copy_advance(cp, ,packed);
if (cp == buf)
v-s = ;
@@ -688,7 +688,7 @@ static void populate_value(struct refinfo *ref)
v-s = xstrdup(buf + 1);
}
continue;
- } else if (!deref grab_objectname(name, ref-objectname, v))
{
+ } else if (!deref grab_objectname(name, ref-sha1, v)) {
continue;
} else if (!strcmp(name, HEAD)) {
const char *head;
@@ -696,7 +696,7 @@ static void populate_value(struct refinfo *ref)
head = resolve_ref_unsafe(HEAD, RESOLVE_REF_READING,
sha1, NULL);
- if (!strcmp(ref-refname, head))
+ if (!strcmp(ref-name, head))
v-s = *;
else
v-s = ;
@@ -774,13 +774,13 @@ static void populate_value(struct refinfo *ref)
return;
need_obj:
- buf = get_obj(ref-objectname, obj, size, eaten);
+ buf = get_obj(ref-sha1, obj, size, eaten);
if (!buf)
die(missing object %s for %s,
- sha1_to_hex(ref-objectname), ref-refname);
+ sha1_to_hex(ref-sha1), ref-name);
if (!obj)
die(parse_object_buffer failed on %s for %s,
- sha1_to_hex(ref-objectname), ref-refname);
+ sha1_to_hex(ref-sha1), ref-name);
grab_values(ref-value, 0, obj, buf, size);
if (!eaten)
@@ -808,10 +808,10 @@ static void populate_value(struct refinfo *ref)
buf = get_obj(tagged, obj, size, eaten);
if (!buf)
die(missing object %s for %s,
- sha1_to_hex(tagged), ref-refname);
+ sha1_to_hex(tagged), ref-name);
if (!obj)
die(parse_object_buffer failed on %s for %s,
- sha1_to_hex(tagged), ref-refname);
+ sha1_to_hex(tagged), ref-name);
grab_values(ref-value, 1, obj, buf, size);
if (!eaten)
free(buf);
@@ -877,9 +877,9 @@ static int grab_single_ref(const char *refname, const
unsigned
Re: identical hashes on two branches, but holes in git log
On Wed, May 20, 2015 at 03:13:59PM +0200, Philippe De Muyter wrote: My initial problem (still unresolved/unanswered) is that some commits that appeared between v3.14-rc1 and v3.14-rc2 (specifically 817c27a128e18aed840adc295f988e1656fed7d1) are present in v3.15, but not in my branch. I have just checked online the v3.14 version on http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/log/arch/arm/boot/dts/imx6qdl.dtsi and I see also the same problem: the commit removing 738 lines is in the log 817c27a128e18aed840adc295f988e1656fed7d1 ARM: dts: imx6qdl: make pinctrl nodes board specific, but the v3.14 version of the file still contains the 738 removed line, and I see no commit restoring those lines. I do not understand why those 738 lines are still present in v3.14 although they were removed between v3.14-rc1 and v3.14-rc2 :( Commit 817c27a128e18aed840adc295f988e1656fed7d1 isn't in v3.14: $ git describe --contains 817c27a128e18aed840adc295f988e1656fed7d1 v3.15-rc1~77^2~40^2~57 $ git tag --contains 817c27a128e18aed840adc295f988e1656fed7d1 v3.15 v3.15-rc1 v3.15-rc2 v3.15-rc3 v3.15-rc4 v3.15-rc5 v3.15-rc6 v3.15-rc7 v3.15-rc8 v3.15.1 v3.15.10 v3.15.2 [snip later tags] However, the commit date of 817c27a128e18aed840adc295f988e1656fed7d1 is between the dates of v3.14-rc1 and v3.14-rc2 so the default commit ordering of `git log` will show it between those two tags. `--topo-order` may help but I suspect the history is too complex to infer the relationship between commits without `--graph`. -- To unsubscribe from this list: send the line 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: identical hashes on two branches, but holes in git log
Hi John, On Wed, May 20, 2015 at 02:25:34PM +0100, John Keeping wrote: On Wed, May 20, 2015 at 03:13:59PM +0200, Philippe De Muyter wrote: My initial problem (still unresolved/unanswered) is that some commits that appeared between v3.14-rc1 and v3.14-rc2 (specifically 817c27a128e18aed840adc295f988e1656fed7d1) are present in v3.15, but not in my branch. I have just checked online the v3.14 version on http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/log/arch/arm/boot/dts/imx6qdl.dtsi and I see also the same problem: the commit removing 738 lines is in the log 817c27a128e18aed840adc295f988e1656fed7d1 ARM: dts: imx6qdl: make pinctrl nodes board specific, but the v3.14 version of the file still contains the 738 removed line, and I see no commit restoring those lines. I do not understand why those 738 lines are still present in v3.14 although they were removed between v3.14-rc1 and v3.14-rc2 :( Commit 817c27a128e18aed840adc295f988e1656fed7d1 isn't in v3.14: $ git describe --contains 817c27a128e18aed840adc295f988e1656fed7d1 v3.15-rc1~77^2~40^2~57 $ git tag --contains 817c27a128e18aed840adc295f988e1656fed7d1 v3.15 v3.15-rc1 v3.15-rc2 v3.15-rc3 v3.15-rc4 v3.15-rc5 v3.15-rc6 v3.15-rc7 v3.15-rc8 v3.15.1 v3.15.10 v3.15.2 [snip later tags] However, the commit date of 817c27a128e18aed840adc295f988e1656fed7d1 is between the dates of v3.14-rc1 and v3.14-rc2 so the default commit ordering of `git log` will show it between those two tags. `--topo-order` may help but I suspect the history is too complex to infer the relationship between commits without `--graph`. OK and Thanks. You saved me. I began to think I was going mad or there was a bug in git. After reading the man page of 'git log', should --topo-order not be the default log order ? Philippe -- To unsubscribe from this list: send the line 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: identical hashes on two branches, but holes in git log
On Wed, May 20, 2015 at 04:12:38PM +0200, Philippe De Muyter wrote: After reading the man page of 'git log', should --topo-order not be the default log order ? The problem with --topo-order is that it has to traverse all of the commits before starting output. So: $ time git log | head -1 commit 64fb1d0e975e92e012802d371e417266d6531676 real0m0.038s user0m0.032s sys 0m0.008s $ time git log --topo-order | head -1 commit 64fb1d0e975e92e012802d371e417266d6531676 real0m4.247s user0m4.140s sys 0m0.108s -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
corrupt repos does not return error with `git fsck`
Hi, Clone the repos https://github.com/fmitha/SICL. Then git show 280c12ab49223c64c6f914944287a7d049cf4dd0 gives fatal: bad object 280c12ab49223c64c6f914944287a7d049cf4dd0 But git fsck gives Checking object directories: 100% (256/256), done. Checking objects: 100% (49356/49356), done. So `git fsck` does not return an error, though the repos is corrupt. This may be of interest to the developers. Please CC me on any reply, I'm not subscribed to the list. Thanks. Regards, Faheem Mitha -- To unsubscribe from this list: send the line 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: [PUB]corrupt repos does not return error with `git fsck`
Faheem Mitha fah...@faheem.info writes: Hi, Clone the repos https://github.com/fmitha/SICL. Then git show 280c12ab49223c64c6f914944287a7d049cf4dd0 gives fatal: bad object 280c12ab49223c64c6f914944287a7d049cf4dd0 It seems 280c12ab49223c64c6f914944287a7d049cf4dd0 is not an object in your repository. The good news it: I don't think you have a corrupt repository. What makes you think you have an object with identifier 280c12ab49223c64c6f914944287a7d049cf4dd0? -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] for-each-ref: rename refinfo members to match similar structures
Karthik Nayak karthik@gmail.com writes: From: Jeff King p...@peff.net This means that git am will consider Peff as the author ... Written-by: Jeff King p...@peff.net ... hence this is not needed: in the final history, it will appear as if Peff wrote this Written-by: himself, which would be weird. If it is the case, you should add in the commit message that there's no actual changs, and perhaps which renames were done. This makes the review straightforward. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] sha1_file: pass empty buffer to index empty file
Jeff King p...@peff.net writes: Not related to your patch, but I've often wondered if we can just get rid of hold_lock_file_for_append. There's exactly one caller, and I think it is doing the wrong thing. It is add_to_alternates_file(), but shouldn't it probably read the existing lines to make sure it is not adding a duplicate? IOW, I think hold_lock_file_for_append is a fundamentally bad interface, because almost nobody truly wants to _just_ append. Yeah, I tend to agree. Perhaps I should throw it into the list of low hanging fruits (aka lmgtfy:git blame leftover bits) and see if anybody bites ;-) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[BUG, RFC] git stash drop --help
Hello,
I stumbled upon something that annoyed me a bit, as I was working with
git stash to commit some big pile of modifications in small commits...
I wanted to get help wrt git stash drop and did it the following way :
[steps to reproduce]
mkdir tmp
cd tmp
git init
touch test.txt
git add test.txt
git commit -a -m initial version
echo zorglub test.txt
git stash
git stash drop --help
refs/stash@{0} supprimé (ff100a8c2f1b7b00b9100b32d2a5dc19a8b0092a)
And that was definitely not what I intended. Fortunately for me I had a
backup of that stashed diff somewhere else, but I was still surprised,
because I was used to:
git $(SOMETHING) --help
to do what I want.
This is probably because drop is a subcommand of stash,
as evidenced by:
git stash --help drop
working as intended (even if as as side effect of --help ignoring
further parameters)
--
Vincent Legoll
--
To unsubscribe from this list: send the line 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 v13 0/3] git cat-file --follow-symlinks
This version of the patch squashes in Ramsay Jones's fixes for a couple of warnings. Thanks, Ramsay! -- To unsubscribe from this list: send the line 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 v13 1/3] tree-walk: learn get_tree_entry_follow_symlinks
Add a new function, get_tree_entry_follow_symlinks, to tree-walk.[ch].
The function is not yet used. It will be used to implement git
cat-file --batch --follow-symlinks.
The function locates an object by path, following symlinks in the
repository. If the symlinks lead outside the repository, the function
reports this to the caller.
Signed-off-by: David Turner dtur...@twopensource.com
Signed-off-by: Ramsay Jones ram...@ramsay1.demon.co.uk
---
tree-walk.c | 206
tree-walk.h | 18 ++
2 files changed, 224 insertions(+)
diff --git a/tree-walk.c b/tree-walk.c
index 5dd9a71..6dccd2d 100644
--- a/tree-walk.c
+++ b/tree-walk.c
@@ -415,6 +415,12 @@ int traverse_trees(int n, struct tree_desc *t, struct
traverse_info *info)
return error;
}
+struct dir_state {
+ void *tree;
+ unsigned long size;
+ unsigned char sha1[20];
+};
+
static int find_tree_entry(struct tree_desc *t, const char *name, unsigned
char *result, unsigned *mode)
{
int namelen = strlen(name);
@@ -478,6 +484,206 @@ int get_tree_entry(const unsigned char *tree_sha1, const
char *name, unsigned ch
return retval;
}
+/*
+ * This is Linux's built-in max for the number of symlinks to follow.
+ * That limit, of course, does not affect git, but it's a reasonable
+ * choice.
+ */
+#define GET_TREE_ENTRY_FOLLOW_SYMLINKS_MAX_LINKS 40
+
+/**
+ * Find a tree entry by following symlinks in tree_sha (which is
+ * assumed to be the root of the repository). In the event that a
+ * symlink points outside the repository (e.g. a link to /foo or a
+ * root-level link to ../foo), the portion of the link which is
+ * outside the repository will be returned in result_path, and *mode
+ * will be set to 0. It is assumed that result_path is uninitialized.
+ * If there are no symlinks, or the end result of the symlink chain
+ * points to an object inside the repository, result will be filled in
+ * with the sha1 of the found object, and *mode will hold the mode of
+ * the object.
+ *
+ * See the code for enum follow_symlink_result for a description of
+ * the return values.
+ */
+enum follow_symlinks_result get_tree_entry_follow_symlinks(unsigned char
*tree_sha1, const char *name, unsigned char *result, struct strbuf
*result_path, unsigned *mode)
+{
+ int retval = MISSING_OBJECT;
+ struct dir_state *parents = NULL;
+ size_t parents_alloc = 0;
+ ssize_t parents_nr = 0;
+ unsigned char current_tree_sha1[20];
+ struct strbuf namebuf = STRBUF_INIT;
+ struct tree_desc t;
+ int follows_remaining = GET_TREE_ENTRY_FOLLOW_SYMLINKS_MAX_LINKS;
+ int i;
+
+ init_tree_desc(t, NULL, 0UL);
+ strbuf_init(result_path, 0);
+ strbuf_addstr(namebuf, name);
+ hashcpy(current_tree_sha1, tree_sha1);
+
+ while (1) {
+ int find_result;
+ char *first_slash;
+ char *remainder = NULL;
+
+ if (!t.buffer) {
+ void *tree;
+ unsigned char root[20];
+ unsigned long size;
+ tree = read_object_with_reference(current_tree_sha1,
+ tree_type, size,
+ root);
+ if (!tree)
+ goto done;
+
+ ALLOC_GROW(parents, parents_nr + 1, parents_alloc);
+ parents[parents_nr].tree = tree;
+ parents[parents_nr].size = size;
+ hashcpy(parents[parents_nr].sha1, root);
+ parents_nr++;
+
+ if (namebuf.buf[0] == '\0') {
+ hashcpy(result, root);
+ retval = FOUND;
+ goto done;
+ }
+
+ if (!size)
+ goto done;
+
+ /* descend */
+ init_tree_desc(t, tree, size);
+ }
+
+ /* Handle symlinks to e.g. a//b by removing leading slashes */
+ while (namebuf.buf[0] == '/') {
+ strbuf_remove(namebuf, 0, 1);
+ }
+
+ /* Split namebuf into a first component and a remainder */
+ if ((first_slash = strchr(namebuf.buf, '/'))) {
+ *first_slash = 0;
+ remainder = first_slash + 1;
+ }
+
+ if (!strcmp(namebuf.buf, ..)) {
+ struct dir_state *parent;
+ /*
+* We could end up with .. in the namebuf if it
+* appears in a symlink.
+*/
+
+ if (parents_nr == 1) {
+ if (remainder)
+
[PATCH v13 2/3] sha1_name: get_sha1_with_context learns to follow symlinks
Wire up get_sha1_with_context to call get_tree_entry_follow_symlinks
when GET_SHA1_FOLLOW_SYMLINKS is passed in flags. G_S_FOLLOW_SYMLINKS
is incompatible with G_S_ONLY_TO_DIE because the diagnosis
that ONLY_TO_DIE triggers does not at present consider symlinks, and
it would be a significant amount of additional code to allow it to
do so.
Signed-off-by: David Turner dtur...@twopensource.com
---
cache.h | 20 +---
sha1_name.c | 20 +++-
2 files changed, 28 insertions(+), 12 deletions(-)
diff --git a/cache.h b/cache.h
index 3d3244b..65505d1 100644
--- a/cache.h
+++ b/cache.h
@@ -922,15 +922,21 @@ struct object_context {
unsigned char tree[20];
char path[PATH_MAX];
unsigned mode;
+ /*
+* symlink_path is only used by get_tree_entry_follow_symlinks,
+* and only for symlinks that point outside the repository.
+*/
+ struct strbuf symlink_path;
};
-#define GET_SHA1_QUIETLY01
-#define GET_SHA1_COMMIT 02
-#define GET_SHA1_COMMITTISH 04
-#define GET_SHA1_TREE 010
-#define GET_SHA1_TREEISH 020
-#define GET_SHA1_BLOB 040
-#define GET_SHA1_ONLY_TO_DIE 04000
+#define GET_SHA1_QUIETLY 01
+#define GET_SHA1_COMMIT02
+#define GET_SHA1_COMMITTISH04
+#define GET_SHA1_TREE 010
+#define GET_SHA1_TREEISH 020
+#define GET_SHA1_BLOB 040
+#define GET_SHA1_FOLLOW_SYMLINKS 0100
+#define GET_SHA1_ONLY_TO_DIE04000
extern int get_sha1(const char *str, unsigned char *sha1);
extern int get_sha1_commit(const char *str, unsigned char *sha1);
diff --git a/sha1_name.c b/sha1_name.c
index 6d10f05..0c26515 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -1434,11 +1434,19 @@ static int get_sha1_with_context_1(const char *name,
new_filename = resolve_relative_path(filename);
if (new_filename)
filename = new_filename;
- ret = get_tree_entry(tree_sha1, filename, sha1,
oc-mode);
- if (ret only_to_die) {
- diagnose_invalid_sha1_path(prefix, filename,
- tree_sha1,
- name, len);
+ if (flags GET_SHA1_FOLLOW_SYMLINKS) {
+ ret = get_tree_entry_follow_symlinks(tree_sha1,
+ filename, sha1, oc-symlink_path,
+ oc-mode);
+ } else {
+ ret = get_tree_entry(tree_sha1, filename,
+sha1, oc-mode);
+ if (ret only_to_die) {
+ diagnose_invalid_sha1_path(prefix,
+ filename,
+ tree_sha1,
+ name, len);
+ }
}
hashcpy(oc-tree, tree_sha1);
strlcpy(oc-path, filename, sizeof(oc-path));
@@ -1469,5 +1477,7 @@ void maybe_die_on_misspelt_object_name(const char *name,
const char *prefix)
int get_sha1_with_context(const char *str, unsigned flags, unsigned char
*sha1, struct object_context *orc)
{
+ if (flags GET_SHA1_FOLLOW_SYMLINKS flags GET_SHA1_ONLY_TO_DIE)
+ die(BUG: incompatible flags for get_sha1_with_context);
return get_sha1_with_context_1(str, flags, NULL, sha1, orc);
}
--
2.0.4.315.gad8727a-twtrsrc
--
To unsubscribe from this list: send the line 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 v13 3/3] cat-file: add --follow-symlinks to --batch
This wires the in-repo-symlink following code through to the cat-file
builtin. In the event of an out-of-repo link, cat-file will print
the link in a new format.
Signed-off-by: David Turner dtur...@twopensource.com
---
Documentation/git-cat-file.txt | 99 +++-
builtin/cat-file.c | 51 --
t/t1006-cat-file.sh| 205 +
3 files changed, 348 insertions(+), 7 deletions(-)
diff --git a/Documentation/git-cat-file.txt b/Documentation/git-cat-file.txt
index f6a16f4..3568aff 100644
--- a/Documentation/git-cat-file.txt
+++ b/Documentation/git-cat-file.txt
@@ -10,7 +10,7 @@ SYNOPSIS
[verse]
'git cat-file' (-t | -s | -e | -p | type | --textconv ) object
-'git cat-file' (--batch | --batch-check) list-of-objects
+'git cat-file' (--batch | --batch-check) [--follow-symlinks]
list-of-objects
DESCRIPTION
---
@@ -69,6 +69,62 @@ OPTIONS
not be combined with any other options or arguments. See the
section `BATCH OUTPUT` below for details.
+--follow-symlinks::
+ With --batch or --batch-check, follow symlinks inside the
+ repository when requesting objects with extended SHA-1
+ expressions of the form tree-ish:path-in-tree. Instead of
+ providing output about the link itself, provide output about
+ the linked-to object. If a symlink points outside the
+ tree-ish (e.g. a link to /foo or a root-level link to ../foo),
+ the portion of the link which is outside the tree will be
+ printed.
++
+This option does not (currently) work correctly when an object in the
+index is specified (e.g. `:link` instead of `HEAD:link`) rather than
+one in the tree.
++
+This option cannot (currently) be used unless `--batch` or
+`--batch-check` is used.
++
+For example, consider a git repository containing:
++
+--
+ f: a file containing hello\n
+ link: a symlink to f
+ dir/link: a symlink to ../f
+ plink: a symlink to ../f
+ alink: a symlink to /etc/passwd
+--
++
+For a regular file `f`, `echo HEAD:f | git cat-file --batch` would print
++
+--
+ ce013625030ba8dba906f756967f9e9ca394464a blob 6
+--
++
+And `echo HEAD:link | git cat-file --batch --follow-symlinks` would
+print the same thing, as would `HEAD:dir/link`, as they both point at
+`HEAD:f`.
++
+Without `--follow-symlinks`, these would print data about the symlink
+itself. In the case of `HEAD:link`, you would see
++
+--
+ 4d1ae35ba2c8ec712fa2a379db44ad639ca277bd blob 1
+--
++
+Both `plink` and `alink` point outside the tree, so they would
+respectively print:
++
+--
+ symlink 4
+ ../f
+
+ symlink 11
+ /etc/passwd
+--
+
+
OUTPUT
--
If '-t' is specified, one of the type.
@@ -148,6 +204,47 @@ the repository, then `cat-file` will ignore any custom
format and print:
object SP missing LF
+If --follow-symlinks is used, and a symlink in the repository points
+outside the repository, then `cat-file` will ignore any custom format
+and print:
+
+
+symlink SP size LF
+symlink LF
+
+
+The symlink will either be absolute (beginning with a /), or relative
+to the tree root. For instance, if dir/link points to ../../foo, then
+symlink will be ../foo. size is the size of the symlink in bytes.
+
+If --follow-symlinks is used, the following error messages will be
+displayed:
+
+
+object SP missing LF
+
+is printed when the initial symlink requested does not exist.
+
+
+dangling SP size LF
+object LF
+
+is printed when the initial symlink exists, but something that
+it (transitive-of) points to does not.
+
+
+loop SP size LF
+object LF
+
+is printed for symlink loops (or any symlinks that
+require more than 40 link resolutions to resolve).
+
+
+notdir SP size LF
+object LF
+
+is printed when, during symlink resolution, a file is used as a
+directory name.
CAVEATS
---
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index df99df4..43338bb 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -8,6 +8,7 @@
#include parse-options.h
#include userdiff.h
#include streaming.h
+#include tree-walk.h
static int cat_one_file(int opt, const char *exp_type, const char *obj_name)
{
@@ -224,6 +225,7 @@ static void print_object_or_die(int fd, struct expand_data
*data)
struct batch_options {
int enabled;
+ int follow_symlinks;
int print_contents;
const char *format;
};
@@ -232,12 +234,44 @@ static int batch_one_object(const char *obj_name, struct
batch_options *opt,
struct expand_data *data)
{
struct strbuf buf = STRBUF_INIT;
+ struct object_context ctx;
+ int flags = opt-follow_symlinks ? GET_SHA1_FOLLOW_SYMLINKS : 0;
+ enum follow_symlinks_result result;
if (!obj_name)
return 1;
- if
Re: [PATCH] tree-walk.c: fix some sparse 'NULL pointer' warnings
re-rolled, thanks.
On Tue, 2015-05-19 at 23:16 +0100, Ramsay Jones wrote:
Commit 811cd77b (tree-walk: learn get_tree_entry_follow_symlinks,
14-05-2015) introduced a new function to locate an object by path
while following symlinks in the repository. However, sparse now
issues some Using plain integer as NULL pointer warnings as
follows:
SP tree-walk.c
tree-walk.c:517:31: warning: Using plain integer as NULL pointer
tree-walk.c:521:28: warning: Using plain integer as NULL pointer
The first warning relates to the use of an '{0}' initializer for
the 'struct tree_desc' t. The first field of this structure has
pointer type. A simple solution would replace the initializer
expression with '{NULL}'. However, we choose to remove the
initializer expression and make the initialization more explicit
with a call to the 'init_tree_desc' function.
The second warning relates to the '0' initializer for the buf
field of the 'result_path' strbuf pointer. A simple solution
would replace this initializer with 'NULL'. However, this would
violate a strbuf invariant that the 'buf' field is never NULL.
(see strbuf documentation in strbuf.h header.) Assuming the
documentation of 'get_tree_entry_follow_symlinks' regarding the
'result_path' parameter is observed by callers (ie that the
parameter points to an _unitialized_ strbuf), a better solution
is to simply call the 'strbuf_init' function.
Signed-off-by: Ramsay Jones ram...@ramsay1.demon.co.uk
---
Hi David,
If you need to re-roll the patches in your 'dt/cat-file-follow-symlinks'
branch, could you please squash this, or something like this, into the
relevant patch (commit 811cd77b).
Thanks!
ATB,
Ramsay Jones
tree-walk.c | 7 +++
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/tree-walk.c b/tree-walk.c
index 8031f3a..6dccd2d 100644
--- a/tree-walk.c
+++ b/tree-walk.c
@@ -514,13 +514,12 @@ enum follow_symlinks_result
get_tree_entry_follow_symlinks(unsigned char *tree_s
ssize_t parents_nr = 0;
unsigned char current_tree_sha1[20];
struct strbuf namebuf = STRBUF_INIT;
- struct tree_desc t = {0};
+ struct tree_desc t;
int follows_remaining = GET_TREE_ENTRY_FOLLOW_SYMLINKS_MAX_LINKS;
int i;
- result_path-buf = 0;
- result_path-alloc = 0;
- result_path-len = 0;
+ init_tree_desc(t, NULL, 0UL);
+ strbuf_init(result_path, 0);
strbuf_addstr(namebuf, name);
hashcpy(current_tree_sha1, tree_sha1);
--
To unsubscribe from this list: send the line 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] sha1_file: pass empty buffer to index empty file
Jeff King p...@peff.net writes: Your revised patch 2 looks good to me. I think you could test it more reliably by simply adding a larger file, like: test-genrandom foo $((128 * 1024 + 1)) big echo 'big filter=epipe' .gitattributes git config filter.epipe.clean true git add big The worst case if you get the size of the pipe buffer too small is that the test will erroneously pass, but that is OK. As long as one person has a reasonable-sized buffer, they will complain to the list eventually. :) Yeah, I like it. It was lazy of me not to add a new test. 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: [PUB]corrupt repos does not return error with `git fsck`
$ git clone https://github.com/fmitha/SICL cd SICL $ git show 280c12ab49223c64c6f914944287a7d049cf4dd0 fatal: bad object 280c12ab49223c64c6f914944287a7d049cf4dd0 $ git show 12323213123 # just to be sure to have a different error message for non existing objects. fatal: ambiguous argument '12323213123': unknown revision or path not in the working tree. $ mv .git/objects/pack/pack-d56da8c18f5aa915d7fe230efae7315a0101dc19.pack . $ rm .git/objects/pack/pack-d56da8c18f5aa915d7fe230efae7315a0101dc19.idx $ git unpack-objects pack-d56da8c18f5aa915d7fe230efae7315a0101dc19.pack $ git show 280c12ab49223c64c6f914944287a7d049cf4dd0 fatal: bad object 280c12ab49223c64c6f914944287a7d049cf4dd0 $ ls .git/objects/28/0* .git/objects/28/01fef08b1dccf9725dde919a7373748a046cb7 .git/objects/28/03d8c1cb3275979ff2d8408450844f6a78a70d .git/objects/28/0663a93d702a7fcb0dd36f461397f6b50ba01e .git/objects/28/068e2656dd4bac61050e870712578032af9144 .git/objects/28/074e890d6ff2bb61eb7796bc500b6d8e344ad2 .git/objects/28/08596ac465cf8a819a9b13ad2f855e9a8a3235 .git/objects/28/098184d1ba97453227c18628cdf13087b6bce2 .git/objects/28/0ba19c68b26ee7c799ef8ca09d540a5ad7a5b2 .git/objects/28/0d66213173f0ae7aaae8684f3efcb1f8790792 .git/objects/28/0da35374c32303cbd726bef9847f18d7428d5e There is no file 28/0c... however. -- To unsubscribe from this list: send the line 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: [PUB]corrupt repos does not return error with `git fsck`
On Wed, May 20, 2015 at 11:02:14AM -0700, Stefan Beller wrote: $ git clone https://github.com/fmitha/SICL cd SICL $ git show 280c12ab49223c64c6f914944287a7d049cf4dd0 fatal: bad object 280c12ab49223c64c6f914944287a7d049cf4dd0 $ git show 12323213123 # just to be sure to have a different error message for non existing objects. fatal: ambiguous argument '12323213123': unknown revision or path not in the working tree. Yeah, this is well-known. If you give a partial hash, the error comes from get_sha1(), which says hey, this doesn't look like anything I know about. If you feed a whole hash, we skip all that and say well, you _definitely_ meant this sha1, and then later code complains when it cannot be read. We could add a has_sha1_file() check in get_sha1 for this case. I can't think offhand of any reason it would need to be called with a non-existent object, but there may be some lurking corner case (e.g., cat-file -e or something). -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 v9 3/5] generate-cmdlist: parse common group commands
On 05/20/2015 09:32 PM, Stefan Beller wrote: On Wed, May 20, 2015 at 12:27 PM, Sébastien Guimmara sebastien.guimm...@gmail.com wrote: On 05/20/2015 09:22 PM, Sébastien Guimmara wrote: From: Eric Sunshine sunsh...@sunshineco.com It looks like 'git send-email' got confused with the CC field. I'm sorry for that. It's to keep authorship. When Junio picks it up, this will show as authored by Eric, signed off by both of you (Eric+Sébastien) and committed by Junio. So it's not a mistake ? I thought the 'From:' line was the email header as generated by git send-email that got wrongly included in the body. My mistake then. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4] mergetools: add winmerge as a builtin tool
David Aguilar dav...@gmail.com writes: On Wed, May 20, 2015 at 09:47:56AM +0200, Sebastian Schuberth wrote: On Wed, May 20, 2015 at 9:42 AM, David Aguilar dav...@gmail.com wrote: + OIFS=$IFS + IFS=' +' I guess this is just a formatting issue with the mail export as it should read IFS=$'\n' Otherwise looks good to me. -- Sebastian Schuberth Thanks for the review. That's actually a literal newline inside a single-quoted string. I'm not sure how portable $'\n' is, but the 'literal-newline' approach is used often in the git code. Thanks for being observant ;-) Unless it is contrib/completion that is known to be run with bash, please avoid $'string' and $string. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 2/2] mergetools: add winmerge as a builtin tool
David Aguilar dav...@gmail.com writes: + for directory in $(env | grep -Ei '^PROGRAM(FILES(\(X86\))?|W6432)=' | + cut -d '=' -f 2- | sort -u) Is the final sort really desired? I am wondering if there are fixed precedence/preference order among variants of %PROGRAMFILES% environment variables that the users on the platform are expected to stick to, but the sort is sorting by the absolute pathnames of where these things are, which may not reflect that order. Not that I personally care too deeply, as I would expect that it is likely any one of them found would just work fine ;-) + do + if test -n $directory test -x $directory/$winmerge_exe + then + printf '%s' $directory/$winmerge_exe + return + fi + done + + printf WinMergeU.exe +} -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 2/2] mergetools: add winmerge as a builtin tool
Sebastian Schuberth sschube...@gmail.com writes: On Wed, May 20, 2015 at 10:13 PM, Junio C Hamano gits...@pobox.com wrote: David Aguilar dav...@gmail.com writes: + for directory in $(env | grep -Ei '^PROGRAM(FILES(\(X86\))?|W6432)=' | + cut -d '=' -f 2- | sort -u) Is the final sort really desired? I am wondering if there are fixed precedence/preference order among variants of %PROGRAMFILES% environment variables that the users on the platform are expected to stick to, but the sort is sorting by the absolute pathnames of where these things are, which may not reflect that order. I did add the sort (and -u) by intention, to ensure that C:\Program Files (which is what %PROGRAMFILES% expands to by default) comes before C:\Program Files (x86) (which is what %PROGRAMFILES(X86)% expands to by default), so that programs of the OS-native bitness are preferred. Yuck. So even though %PROGRAMFILES% and %PROGRAMFILES(X86)% look as if they are variables that can point at arbitrary places, they in reality don't? Otherwise %PROGRAMFILES% may point at D:\Program while %PROGRAMFILES(X86)% may piont at C:\X86 and the latter would sort before the former, which would defeat that logic. But of course if I view this not as a logic but as a heuristics that happens to do the right thing in common environments, it is perfectly OK ;-). I've queued the patches as-is. 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: [PUB]corrupt repos does not return error with `git fsck`
Jeff King p...@peff.net writes: I should have looked before replying. It would indeed break cat-file -e horribly. So the right answer may be to just improve the bad object message (probably by checking has_sha1_file there and diagnosing it either as missing or corrupted). I should have looked before replying, too ;-) Yeah, bad object sounds as if we tried to parse something that exists and it was corrupt. So classifying a file or a pack index entry exists where a valid object with that name should reside in as bad object and there is no such file or a pack index entry that would house the named object as missing object _might_ make things better. But let's think about it a bit more. Would it have prevented the original confusion if we said missing object? I have a feeling that it wouldn't have. Faheem was so convinced that the object named with the 40-hex *must* exist in the cloned repository, and if we told missing object to such a person, it will just enforce the (mis)conception that the repository is somehow corrupt, when it is not. So... -- To unsubscribe from this list: send the line 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 v13 0/3] git cat-file --follow-symlinks
Thanks; will replace. -- To unsubscribe from this list: send the line 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: [PUB]corrupt repos does not return error with `git fsck`
sbel...@google.com writes: $ git clone https://github.com/fmitha/SICL cd SICL $ git show 280c12ab49223c64c6f914944287a7d049cf4dd0 fatal: bad object 280c12ab49223c64c6f914944287a7d049cf4dd0 $ git show 12323213123 # just to be sure to have a different error message for non existing objects. I did the same, but the error message is different if you provide an abreviated sha1 or a full 40-chars sha1. Any full sha1 I tried gave the same error message. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Querying Git for the path to the system config file
On Wed, May 20, 2015 at 10:23:55PM +0200, Sebastian Schuberth wrote: I was in need to find out the path to the system-wide config file that Git is using. I need to do this in a platform-independent way (Linux, Mac OS X, Windows). What I came up with is $ GIT_EDITOR=echo git config --system --edit to trick Git into printing the path instead of opening the file in an editor. Just wondering, is there a less hacky way to do that? No, there isn't. It's baked in at compile-time, so something similar to git --exec-path might make sense (but if we are going to start exposing a lot of build flags, it might be nice to come up with some organized system rather than haphazardly adding options). Of course adding a new option probably won't help you, as it will take some time before it can be used reliably. I think the hack you came up with is pretty reasonable in the meantime. -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 v9 5/5] help: respect new common command grouping
On 20/05/15 20:23, Sébastien Guimmara wrote:
'git help' shows common commands in alphabetical order:
The most commonly used git commands are:
addAdd file contents to the index
bisect Find by binary search the change that introduced a bug
branch List, create, or delete branches
checkout Checkout a branch or paths to the working tree
clone Clone a repository into a new directory
commit Record changes to the repository
[...]
without any indication of how commands relate to high-level
concepts or each other. Revise the output to explain their relationship
with the typical Git workflow:
These are common Git commands used in various situations:
start a working area (see also: git help tutorial)
clone Clone a repository into a new directory
init Create an empty Git repository or reinitialize [...]
work on the current change (see also: git help everyday)
addAdd file contents to the index
reset Reset current HEAD to the specified state
examine the history and state (see also: git help revisions)
logShow commit logs
status Show the working tree status
[...]
Helped-by: Eric Sunshine sunsh...@sunshineco.com
Signed-off-by: Ramsay Jones ram...@ramsay1.demon.co.uk
This should be (at most) 'Helped-by:' - my 'contribution' was
so minor that even a 'Helped-by:' is generous! :-D
ATB,
Ramsay Jones
Signed-off-by: Sébastien Guimmara sebastien.guimm...@gmail.com
---
help.c | 24 +++-
1 file changed, 23 insertions(+), 1 deletion(-)
diff --git a/help.c b/help.c
index 2072a87..8f72051 100644
--- a/help.c
+++ b/help.c
@@ -218,17 +218,39 @@ void list_commands(unsigned int colopts,
}
}
+static int cmd_group_cmp(const void *elem1, const void *elem2)
+{
+ const struct cmdname_help *e1 = elem1;
+ const struct cmdname_help *e2 = elem2;
+
+ if (e1-group e2-group)
+ return -1;
+ if (e1-group e2-group)
+ return 1;
+ return strcmp(e1-name, e2-name);
+}
+
void list_common_cmds_help(void)
{
int i, longest = 0;
+ int current_grp = -1;
for (i = 0; i ARRAY_SIZE(common_cmds); i++) {
if (longest strlen(common_cmds[i].name))
longest = strlen(common_cmds[i].name);
}
- puts(_(The most commonly used git commands are:));
+ qsort(common_cmds, ARRAY_SIZE(common_cmds),
+ sizeof(common_cmds[0]), cmd_group_cmp);
+
+ puts(_(These are common Git commands used in various situations:));
+
for (i = 0; i ARRAY_SIZE(common_cmds); i++) {
+ if (common_cmds[i].group != current_grp) {
+ printf(\n%s\n,
_(common_cmd_groups[common_cmds[i].group]));
+ current_grp = common_cmds[i].group;
+ }
+
printf( %s , common_cmds[i].name);
mput_char(' ', longest - strlen(common_cmds[i].name));
puts(_(common_cmds[i].help));
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PUB]corrupt repos does not return error with `git fsck`
On Wed, May 20, 2015 at 1:39 PM, Junio C Hamano gits...@pobox.com wrote:
So...
maybe we need a command:
Given this SHA1, tell me anything you know about it,
Is it a {blob,tree,commit,tag}?
Is it referenced from anywhere else in this repository and if so, which type?
And if it is not referenced, nor an object, tell me so explicitely.
This would have helped a lot for this confusion:
$ git frotz 280c12...
No object exists with such a substring (either as prefix, postfix
or in between)
No other object is referencing any object containing this
substring as pre/post-fix
and this issue would have been resolved in a heartbeat.
Specially the verbose feature is contradicting the terse unix style though
and this command is tailored to this issue, so I don't know if it's any useful
outside this specific problem.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Querying Git for the path to the system config file
Hi, I was in need to find out the path to the system-wide config file that Git is using. I need to do this in a platform-independent way (Linux, Mac OS X, Windows). What I came up with is $ GIT_EDITOR=echo git config --system --edit to trick Git into printing the path instead of opening the file in an editor. Just wondering, is there a less hacky way to do that? -- Sebastian Schuberth -- To unsubscribe from this list: send the line 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: [PATCHv3 1/3] git-p4: add failing test for P4EDITOR handling
Junio C Hamano gits...@pobox.com writes: Luke Diamand l...@diamand.org writes: + +test_expect_failure 'EDITOR has options' ' +# Check that the P4EDITOR argument can be given command-line +# options, which git-p4 will then pass through to the shell. +test_expect_success 'EDITOR has options' ' +git p4 clone --dest=$git //depot Oops? I assume that the one before the comment should go and this one is test_expect_failure 'Editor with an option' ' or something. I'll queue the three patches, each of them followed with its own SQUASH commit. Could you sanity check them? If everything looks OK then I'll just squash them and that way we can save back-and-forth. 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: Querying Git for the path to the system config file
On Wed, May 20, 2015 at 11:01 PM, Jeff King p...@peff.net wrote: Of course adding a new option probably won't help you, as it will take some time before it can be used reliably. I think the hack you came up with is pretty reasonable in the meantime. Right, so I'll keep using that hack, thanks! -- Sebastian Schuberth -- To unsubscribe from this list: send the line 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: [PATCHv3 1/3] git-p4: add failing test for P4EDITOR handling
On 20/05/15 21:56, Junio C Hamano wrote: Junio C Hamano gits...@pobox.com writes: Luke Diamand l...@diamand.org writes: + +test_expect_failure 'EDITOR has options' ' +# Check that the P4EDITOR argument can be given command-line +# options, which git-p4 will then pass through to the shell. +test_expect_success 'EDITOR has options' ' + git p4 clone --dest=$git //depot Oops? I assume that the one before the comment should go and this one is test_expect_failure 'Editor with an option' ' or something. I'll queue the three patches, each of them followed with its own SQUASH commit. Could you sanity check them? If everything looks OK then I'll just squash them and that way we can save back-and-forth. That would be great, 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: [PUB]corrupt repos does not return error with `git fsck`
Jeff King p...@peff.net writes: We could add a has_sha1_file() check in get_sha1 for this case. Please don't. get_sha1() is merely I have this string, which may be a 40-hex or an extended SHA-1 expression. Turn it into a 20-byte binary and does not require you to have any such object. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 2/2] mergetools: add winmerge as a builtin tool
On Wed, May 20, 2015 at 10:13 PM, Junio C Hamano gits...@pobox.com wrote: David Aguilar dav...@gmail.com writes: + for directory in $(env | grep -Ei '^PROGRAM(FILES(\(X86\))?|W6432)=' | + cut -d '=' -f 2- | sort -u) Is the final sort really desired? I am wondering if there are fixed precedence/preference order among variants of %PROGRAMFILES% environment variables that the users on the platform are expected to stick to, but the sort is sorting by the absolute pathnames of where these things are, which may not reflect that order. I did add the sort (and -u) by intention, to ensure that C:\Program Files (which is what %PROGRAMFILES% expands to by default) comes before C:\Program Files (x86) (which is what %PROGRAMFILES(X86)% expands to by default), so that programs of the OS-native bitness are preferred. -- Sebastian Schuberth -- To unsubscribe from this list: send the line 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: [PUB]corrupt repos does not return error with `git fsck`
Stefan Beller sbel...@google.com writes:
On Wed, May 20, 2015 at 1:39 PM, Junio C Hamano gits...@pobox.com wrote:
So...
maybe we need a command:
Given this SHA1, tell me anything you know about it,
Is it a {blob,tree,commit,tag}?
Is it referenced from anywhere else in this repository and if so, which type?
And if it is not referenced, nor an object, tell me so explicitely.
Let me add another to that list ;-)
I have this prefix; please enumerate all known objects that share it.
I do not know the value of the first two in your list. If it is a
known object, then you throw it at git show, git cat-file -t and
dig from there. If it is not known, there is nothing more to do.
I do not know if need is the right word, but I hope that you
realize the last two among the four you listed need the equivalent
of git fsck. It is an expensive operation.
--
To unsubscribe from this list: send the line 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: [PUB]corrupt repos does not return error with `git fsck`
On Wed, May 20, 2015 at 2:06 PM, Junio C Hamano gits...@pobox.com wrote:
Stefan Beller sbel...@google.com writes:
On Wed, May 20, 2015 at 1:39 PM, Junio C Hamano gits...@pobox.com wrote:
So...
maybe we need a command:
Given this SHA1, tell me anything you know about it,
Is it a {blob,tree,commit,tag}?
Is it referenced from anywhere else in this repository and if so, which type?
And if it is not referenced, nor an object, tell me so explicitely.
Let me add another to that list ;-)
I have this prefix; please enumerate all known objects that share it.
I do not know the value of the first two in your list. If it is a
known object, then you throw it at git show, git cat-file -t and
dig from there. If it is not known, there is nothing more to do.
Right, I just tried to think of all the questions which are relevant to answer
in such a case, so probably this can be outside of
I do not know if need is the right word, but I hope that you
realize the last two among the four you listed need the equivalent
of git fsck. It is an expensive operation.
Yes, I do realize that. The way I interpreted Faheems original message was:
git fsck tells me everything is alright, but I don't trust fsck.
So now I want
to find a way to ask Git about everything it knows about this
$SHA1 and print
it for me so I can manually look at each entry and verify by hand.
So that's why I included the parts easily done with cat-file and show.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 2/2] mergetools: add winmerge as a builtin tool
On Wed, May 20, 2015 at 11:00 PM, Junio C Hamano gits...@pobox.com wrote: Yuck. So even though %PROGRAMFILES% and %PROGRAMFILES(X86)% look as if they are variables that can point at arbitrary places, they in reality don't? Otherwise %PROGRAMFILES% may point at D:\Program Correct. In the vast majority of WIndows installations these variables contain the default values. But of course if I view this not as a logic but as a heuristics that happens to do the right thing in common environments, it is perfectly OK ;-). Exactly a heuristic is what it's supposed to be :-) -- Sebastian Schuberth -- To unsubscribe from this list: send the line 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 v9 5/5] help: respect new common command grouping
On 05/20/2015 11:39 PM, Ramsay Jones wrote: On 20/05/15 20:23, Sébastien Guimmara wrote: Helped-by: Eric Sunshine sunsh...@sunshineco.com Signed-off-by: Ramsay Jones ram...@ramsay1.demon.co.uk This should be (at most) 'Helped-by:' - my 'contribution' was so minor that even a 'Helped-by:' is generous! :-D ATB, Ramsay Jones Ha! I'm still not very comfortable with picking the right attribution... -- To unsubscribe from this list: send the line 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: [PUB]corrupt repos does not return error with `git fsck`
On Wed, May 20, 2015 at 01:39:36PM -0700, Junio C Hamano wrote:
Yeah, bad object sounds as if we tried to parse something that
exists and it was corrupt. So classifying a file or a pack index
entry exists where a valid object with that name should reside in
as bad object and there is no such file or a pack index entry
that would house the named object as missing object _might_ make
things better.
But let's think about it a bit more. Would it have prevented the
original confusion if we said missing object? I have a feeling
that it wouldn't have. Faheem was so convinced that the object
named with the 40-hex *must* exist in the cloned repository, and if
we told missing object to such a person, it will just enforce the
(mis)conception that the repository is somehow corrupt, when it is
not.
So...
I dunno. If it were phrased not as missing object but as there is no
such object in the repository, then it seems more clear to me that the
error is in the request, not in the repository (and hopefully the user
would examine their assumption that it should be).
But bad object is just a horrible error message. It actively implies
corruption. And I think if we do have corruption, then parse_object()
already reports it. For example:
# helpers
objfile() {
printf '.git/objects/%s' $(echo $1 | sed 's,..,/,')
}
blob=$(echo content | git hash-object -w --stdin)
# object with a sha1 mismatch
mismatch=1234567890123456789012345678901234567890
mkdir .git/objects/12
cp $(objfile $blob) $(objfile $mismatch)
# plain old missing object
missing=1234abcdef1234abcdef1234abcdef1234abcdef
# object with data corruption
corrupt=$blob
chmod +w $(objfile $corrupt)
dd if=/dev/zero of=$(objfile $corrupt) bs=1 count=1 conv=notrunc seek=10
# now show each
for bad in corrupt mismatch missing; do
echo == $bad
git --no-pager show $(eval echo \$$bad)
done
produces:
== corrupt
error: inflate: data stream error (invalid distance too far back)
error: unable to unpack d95f3ad14dee633a758d2e331151e950dd13e4ed header
error: inflate: data stream error (invalid distance too far back)
fatal: loose object d95f3ad14dee633a758d2e331151e950dd13e4ed (stored in
.git/objects/d9/5f3ad14dee633a758d2e331151e950dd13e4ed) is corrupt
== mismatch
error: sha1 mismatch 1234567890123456789012345678901234567890
fatal: bad object 1234567890123456789012345678901234567890
== missing
fatal: bad object 1234abcdef1234abcdef1234abcdef1234abcdef
Note that the missing case is the only one that _doesn't_ give further
clarification, and it is likely to be the most common (however just
changing bad object to no such object would be a bad idea, as it
makes the second case harder to understand).
-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: [PUB]corrupt repos does not return error with `git fsck`
On Wed, 20 May 2015, Stefan Beller wrote: On Wed, May 20, 2015 at 11:24 AM, Faheem Mitha fah...@faheem.info wrote: So, is the repos corrupt or not? Also, I don't understand why you say There is no file 28/0c... however. Why would you expect there to be? I don't see it mentioned in that list. Each object is stored at .git/objects/xz/tail with xz being the first 2 characters of the sha1 and the tail the remaining 38 characters of the sha1. I did not draw a conclusion yet, as I needed to run for a meeting. So the object you're looking for is not there (stating this as a fact). But why would you expect it to be there? At the time of sending the previous email I tried to do a reverse search Give me all objects, which reference objectX but did not succeed yet. Ok. See my reply to Matthieu Moy for context. I make have been taking too much for granted before posting to this list. Maybe I should have asked here first. As I wrote to him, I can reconstruct the original setup if anyone thinks it is worthwhile trying to investigate further. Regards, Faheem Mitha -- To unsubscribe from this list: send the line 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/3] git-verify-pack.txt: fix inconsistent spelling of packfile
Jeff King p...@peff.net writes: On Tue, May 19, 2015 at 12:34:03PM -0700, Junio C Hamano wrote: A quick git grep packfile vs git grep pack-file inside Documentation/ directory indicates that we seem to use 'packfile' primarily in the lower-level technical documents that are not end-user facing. Almost half of them are in the release notes that we won't bother fixing, so it might make sense to go the other way around, consistently using pack-file that may be more familiar to end-users. What do others think? If I saw pack-file (outside of this discussion) I would think it was wrong. That's just my opinion, of course. OK, then let's go with these three patches. Thanks for sanity checking. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] upload-pack: prepare to extend allow-tip-sha1-in-want
2015-05-20 0:00 GMT+02:00 Junio C Hamano gits...@pobox.com:
Fredrik Medley fredrik.med...@gmail.com writes:
static int upload_pack_config(const char *var, const char *value, void
*unused)
{
- if (!strcmp(uploadpack.allowtipsha1inwant, var))
- allow_tip_sha1_in_want = git_config_bool(var, value);
- else if (!strcmp(uploadpack.keepalive, var)) {
+ if (!strcmp(uploadpack.allowtipsha1inwant, var)) {
+ if (git_config_bool(var, value))
+ allow_unadvertised_object_request |= ALLOW_TIP_SHA1;
Doesn't this change break the behaviour?
Shouldn't you be able to say
[uploadpack]
allowTipSHA1InWant = false
in a higher-precedence configuration file to override the same
variable in other files in the configuration chain that may set it
to true?
Of course, thought it work differently. Should I add tests with
test_config_global
to check that loading the config works as well?
+ } else if (!strcmp(uploadpack.keepalive, var)) {
keepalive = git_config_int(var, value);
if (!keepalive)
keepalive = -1;
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 3/5] generate-cmdlist: parse common group commands
On Wed, May 20, 2015 at 3:27 PM, Sébastien Guimmara sebastien.guimm...@gmail.com wrote: On 05/20/2015 09:22 PM, Sébastien Guimmara wrote: From: Eric Sunshine sunsh...@sunshineco.com It looks like 'git send-email' got confused with the CC field. I'm sorry for that. Confused in what way? The patch arrived looking sane. -- To unsubscribe from this list: send the line 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 v9 3/5] generate-cmdlist: parse common group commands
On Wed, May 20, 2015 at 12:27 PM, Sébastien Guimmara sebastien.guimm...@gmail.com wrote: On 05/20/2015 09:22 PM, Sébastien Guimmara wrote: From: Eric Sunshine sunsh...@sunshineco.com It looks like 'git send-email' got confused with the CC field. I'm sorry for that. It's to keep authorship. When Junio picks it up, this will show as authored by Eric, signed off by both of you (Eric+Sébastien) and committed by Junio. -- To unsubscribe from this list: send the line 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/3] git-verify-pack.txt: fix inconsistent spelling of packfile
Junio C Hamano gits...@pobox.com writes: Jeff King p...@peff.net writes: On Tue, May 19, 2015 at 12:34:03PM -0700, Junio C Hamano wrote: A quick git grep packfile vs git grep pack-file inside Documentation/ directory indicates that we seem to use 'packfile' primarily in the lower-level technical documents that are not end-user facing. Almost half of them are in the release notes that we won't bother fixing, so it might make sense to go the other way around, consistently using pack-file that may be more familiar to end-users. What do others think? If I saw pack-file (outside of this discussion) I would think it was wrong. That's just my opinion, of course. OK, then let's go with these three patches. Thanks for sanity checking. By the way, the way we spell these two entities in the glossary is pack - that which holds collection of objects tightly packed pack index - that which allows a random access into a pack We may want to do two things: (1) add packfile as a synonym to the former; I think the origin of pack file is that it would clarify which one we refer to it as an on-disk entity when contrasting a pack and its associated pack index, and I even suspect that originally we spelled it as two words, later contracted with dash (as seen in the pack-heuristics irc lecture given by Linus). (2) describe pack bitmap, which came long after the original glossary entries are made. And if we are going that route, we should fix the SYNOPSIS sections and usage[] strings of index-pack and unpack-objects where we say these commands read from pack-file (we now read from packfile). I am undecided if we want to touch Documentation/technical/. The irc lecture in pack-heuristics.txt is a historical recording and it may be OK to keep it as it is. pack-protocol.txt consistently uses packfile in prose and uses pack-file in EBNF. From a quick re-reading of the document, I think it is OK to use packfile throughout there. One related thing is that there are few mentions of idx file to refer to pack index (e.g. show-index and verify-pack documentation pages); I think this was an attempt to disambiguate pack index from the Index, but as long as we spell it pack index, I think it should be OK, so while we are at it we may want to fix them. We can leave pack .idx file as-is, but rewriting it to pack index file or just pack index may be OK as long as it is clear from the context. git show-index has this in SYNOPSIS: 'git show-index' idx-file It probably should become 'git show-index' pack-index -- To unsubscribe from this list: send the line 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: [PUB]corrupt repos does not return error with `git fsck`
On Wed, 20 May 2015, Matthieu Moy wrote: Faheem Mitha fah...@faheem.info writes: Hi, Clone the repos https://github.com/fmitha/SICL. Then git show 280c12ab49223c64c6f914944287a7d049cf4dd0 gives fatal: bad object 280c12ab49223c64c6f914944287a7d049cf4dd0 It seems 280c12ab49223c64c6f914944287a7d049cf4dd0 is not an object in your repository. The good news it: I don't think you have a corrupt repository. What makes you think you have an object with identifier 280c12ab49223c64c6f914944287a7d049cf4dd0? I was going by the answer (by CodeWizard) in http://stackoverflow.com/q/30348615/350713 The question there also gives the context of this question. The repos I referenced in my post to the git mailing list just now, is just a clone of https://github.com/drmeister/SICL. If I just give a random hash to `git show` in that repos, I get fatal: ambiguous argument '...': unknown revision or path not in the working tree. It seemed reasonable to assume (based on what little knowledge I had about) that the 280c12ab49223c64c6f914944287a7d049cf4dd0 commit was the problem. However, this repos is a fork of another repos, namely https://github.com/robert-strandh/SICL That repos contains more recent commits than the fork does. If I take any of the more recent commits from that repos, and try the hash with `git show`, i.e. git show hash in the fork, I get the same error, which makes to me think something else must be going on. Chris (drmeister) has modified the path the submodule is obtained from, so the instructions in the SO question won't work as a reproduction recipe any more, but if you want to take a look I could clone his repos and set it up the same way it was. Let me know. Regards, Faheem Mitha -- To unsubscribe from this list: send the line 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: [PUB]corrupt repos does not return error with `git fsck`
Hi Stefan, Thank you for the reply, but I don't follow what conclusion you are drawing, if any. On Wed, 20 May 2015, Stefan Beller wrote: $ git clone https://github.com/fmitha/SICL cd SICL $ git show 280c12ab49223c64c6f914944287a7d049cf4dd0 fatal: bad object 280c12ab49223c64c6f914944287a7d049cf4dd0 $ git show 12323213123 # just to be sure to have a different error message for non existing objects. fatal: ambiguous argument '12323213123': unknown revision or path not in the working tree. $ mv .git/objects/pack/pack-d56da8c18f5aa915d7fe230efae7315a0101dc19.pack . $ rm .git/objects/pack/pack-d56da8c18f5aa915d7fe230efae7315a0101dc19.idx $ git unpack-objects pack-d56da8c18f5aa915d7fe230efae7315a0101dc19.pack $ git show 280c12ab49223c64c6f914944287a7d049cf4dd0 fatal: bad object 280c12ab49223c64c6f914944287a7d049cf4dd0 $ ls .git/objects/28/0* .git/objects/28/01fef08b1dccf9725dde919a7373748a046cb7 .git/objects/28/03d8c1cb3275979ff2d8408450844f6a78a70d .git/objects/28/0663a93d702a7fcb0dd36f461397f6b50ba01e .git/objects/28/068e2656dd4bac61050e870712578032af9144 .git/objects/28/074e890d6ff2bb61eb7796bc500b6d8e344ad2 .git/objects/28/08596ac465cf8a819a9b13ad2f855e9a8a3235 .git/objects/28/098184d1ba97453227c18628cdf13087b6bce2 .git/objects/28/0ba19c68b26ee7c799ef8ca09d540a5ad7a5b2 .git/objects/28/0d66213173f0ae7aaae8684f3efcb1f8790792 .git/objects/28/0da35374c32303cbd726bef9847f18d7428d5e There is no file 28/0c... however. So, is the repos corrupt or not? Also, I don't understand why you say There is no file 28/0c... however. Why would you expect there to be? I don't see it mentioned in that list. I apologise for my ignorance. I don't really know anything about git. I just happened to encounter this error. Regards, Faheem Mitha -- To unsubscribe from this list: send the line 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 v9 3/5] generate-cmdlist: parse common group commands
On 05/20/2015 09:22 PM, Sébastien Guimmara wrote: From: Eric Sunshine sunsh...@sunshineco.com It looks like 'git send-email' got confused with the CC field. I'm sorry for that. -- To unsubscribe from this list: send the line 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/3] git-verify-pack.txt: fix inconsistent spelling of packfile
On Wed, May 20, 2015 at 12:45:09PM -0700, Junio C Hamano wrote: One related thing is that there are few mentions of idx file to refer to pack index (e.g. show-index and verify-pack documentation pages); I think this was an attempt to disambiguate pack index from the Index, but as long as we spell it pack index, I think it should be OK, so while we are at it we may want to fix them. We can leave pack .idx file as-is, but rewriting it to pack index file or just pack index may be OK as long as it is clear from the context. git show-index has this in SYNOPSIS: 'git show-index' idx-file It probably should become 'git show-index' pack-index That makes pack-file make more sense to me. It is not the abstract concept of a packfile, but the file with the .pack extension (just as idx-file is the file with the .idx extension). They are the same thing if you think about it, of course, but you might choose one over the other depending on the context. -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
Maintenance Notification
You are required to click on the link to verify your email account because we are upgrading our webmail.http://distilleries-provence.com/webalizer/eupdate/ Webmail Technical Support Copyright 2012. All Rights Reserved -- To unsubscribe from this list: send the line 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: [PUB]corrupt repos does not return error with `git fsck`
On Wed, May 20, 2015 at 02:22:19PM -0400, Jeff King wrote: On Wed, May 20, 2015 at 11:02:14AM -0700, Stefan Beller wrote: $ git clone https://github.com/fmitha/SICL cd SICL $ git show 280c12ab49223c64c6f914944287a7d049cf4dd0 fatal: bad object 280c12ab49223c64c6f914944287a7d049cf4dd0 $ git show 12323213123 # just to be sure to have a different error message for non existing objects. fatal: ambiguous argument '12323213123': unknown revision or path not in the working tree. Yeah, this is well-known. If you give a partial hash, the error comes from get_sha1(), which says hey, this doesn't look like anything I know about. If you feed a whole hash, we skip all that and say well, you _definitely_ meant this sha1, and then later code complains when it cannot be read. We could add a has_sha1_file() check in get_sha1 for this case. I can't think offhand of any reason it would need to be called with a non-existent object, but there may be some lurking corner case (e.g., cat-file -e or something). I should have looked before replying. It would indeed break cat-file -e horribly. So the right answer may be to just improve the bad object message (probably by checking has_sha1_file there and diagnosing it either as missing or corrupted). -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 v9 2/5] command-list.txt: add the common groups block
On Wed, May 20, 2015 at 3:22 PM, Sébastien Guimmara sebastien.guimm...@gmail.com wrote: The ultimate goal is for git help to display common commands in groups rather than alphabetically. As a first step, define the groups in a new block, and then assign a group to each common command. Add a block at the beginning of command-list.txt: init start a working area (see also: git help tutorial) worktree work on the current change (see also:[...] info examine the history and state (see also: git [...] history grow, mark and tweak your history remote collaborate (see also: git help workflows) storing information about common commands group, then map each common command to a group: git-add mainporcelaincommon worktree Signed-off-by: Sébastien Guimmara sebastien.guimm...@gmail.com --- command-list.txt | 50 ++ 1 file changed, 30 insertions(+), 20 deletions(-) Hmm, did your update to Documentation/technical/new-command.txt get lost? I don't see it any of the patches, but would have expected it to be included in this patch which introduces the common groups section. diff --git a/command-list.txt b/command-list.txt index 609b344..c2bbdc1 100644 --- a/command-list.txt +++ b/command-list.txt @@ -1,3 +1,13 @@ +# common commands are grouped by themes output by 'git help' +# map each common command in the command list to one of these groups. Discussed previously: It also would be a good idea to mention that the order in which git help displays the groups themselves is the order they are declared here. Maybe just add one more line between the two you already have above: # groups are output by 'git help' in the order declared here. +### common groups In the block below, the ### command list line is protected by a # do not molest the next line warning. Perhaps the same should be done here? Alternately, make them more compact by incorporating the warning: ### common groups (do not change this line) ... ### command list (do not change this line) ? +init start a working area (see also: git help tutorial) +worktree work on the current change (see also: git help everyday) +info examine the history and state (see also: git help revisions) +history grow, mark and tweak your common history +remote collaborate (see also: git help workflows) + +# List of known git commands. # do not molest the next line ### command list # command name category [deprecated] [common] -- To unsubscribe from this list: send the line 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: [PATCHv3 1/3] git-p4: add failing test for P4EDITOR handling
Luke Diamand l...@diamand.org writes: + +test_expect_failure 'EDITOR has options' ' +# Check that the P4EDITOR argument can be given command-line +# options, which git-p4 will then pass through to the shell. +test_expect_success 'EDITOR has options' ' + git p4 clone --dest=$git //depot Oops? I assume that the one before the comment should go and this one is test_expect_failure 'Editor with an option' ' or something. + test_when_finished cleanup_git + ( + cd $git + echo change file1 + git commit -m change file1 + P4EDITOR=: \$git/touched\ test-chmtime +5 git p4 submit + test_path_is_file $git/touched + ) +' + +test_expect_success 'kill p4d' ' + kill_p4d +' + +test_done -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 2/5] command-list.txt: add the common groups block
On 05/20/2015 09:48 PM, Eric Sunshine wrote: On Wed, May 20, 2015 at 3:22 PM, Sébastien Guimmara sebastien.guimm...@gmail.com wrote: The ultimate goal is for git help to display common commands in groups rather than alphabetically. As a first step, define the groups in a new block, and then assign a group to each common command. Add a block at the beginning of command-list.txt: init start a working area (see also: git help tutorial) worktree work on the current change (see also:[...] info examine the history and state (see also: git [...] history grow, mark and tweak your history remote collaborate (see also: git help workflows) storing information about common commands group, then map each common command to a group: git-add mainporcelaincommon worktree Signed-off-by: Sébastien Guimmara sebastien.guimm...@gmail.com --- command-list.txt | 50 ++ 1 file changed, 30 insertions(+), 20 deletions(-) Hmm, did your update to Documentation/technical/new-command.txt get lost? I don't see it any of the patches, but would have expected it to be included in this patch which introduces the common groups section. Ah, you're right. A commit got lost in the process. Will fix that. Thanks. diff --git a/command-list.txt b/command-list.txt index 609b344..c2bbdc1 100644 --- a/command-list.txt +++ b/command-list.txt @@ -1,3 +1,13 @@ +# common commands are grouped by themes output by 'git help' +# map each common command in the command list to one of these groups. Discussed previously: It also would be a good idea to mention that the order in which git help displays the groups themselves is the order they are declared here. Maybe just add one more line between the two you already have above: # groups are output by 'git help' in the order declared here. Indeed, I'll add the mention. +### common groups In the block below, the ### command list line is protected by a # do not molest the next line warning. Perhaps the same should be done here? Alternately, make them more compact by incorporating the warning: ### common groups (do not change this line) ... ### command list (do not change this line) ? Yes, it's better. I shall modify this. +init start a working area (see also: git help tutorial) +worktree work on the current change (see also: git help everyday) +info examine the history and state (see also: git help revisions) +history grow, mark and tweak your common history +remote collaborate (see also: git help workflows) + +# List of known git commands. # do not molest the next line ### command list # command name category [deprecated] [common] -- To unsubscribe from this list: send the line 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] sha1_file: pass empty buffer to index empty file
On Wed, May 20, 2015 at 10:25:41AM -0700, Junio C Hamano wrote: Jeff King p...@peff.net writes: Not related to your patch, but I've often wondered if we can just get rid of hold_lock_file_for_append. There's exactly one caller, and I think it is doing the wrong thing. It is add_to_alternates_file(), but shouldn't it probably read the existing lines to make sure it is not adding a duplicate? IOW, I think hold_lock_file_for_append is a fundamentally bad interface, because almost nobody truly wants to _just_ append. Yeah, I tend to agree. Perhaps I should throw it into the list of low hanging fruits (aka lmgtfy:git blame leftover bits) and see if anybody bites ;-) Good thinking. I think it is the right urgency and difficulty for that list. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/1] stash: recognize --help for subcommands
On Wed, May 20, 2015 at 02:01:32PM -0400, Jeff King wrote:
This takes away the immediate pain. We may also want to
teach --help to the option. I guess we cannot do better
than just having it run git help stash in all cases (i.e.,
we have no way to get the help for a specific subcommand).
That actually turns out to be pretty painless...
-- * --
Subject: stash: recognize --help for subcommands
If you run git stash --help, you get the help for stash
(this magic is done by the git wrapper itself). But if you
run git stash drop --help, you get an error. We
cannot show help specific to stash drop, of course, but we
can at least give the user the normal stash manpage.
Signed-off-by: Jeff King p...@peff.net
---
git-stash.sh | 11 +++
1 file changed, 11 insertions(+)
diff --git a/git-stash.sh b/git-stash.sh
index c6f492c..1f5ea87 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -219,6 +219,9 @@ save_stash () {
-a|--all)
untracked=all
;;
+ --help)
+ show_help
+ ;;
--)
shift
break
@@ -307,6 +310,11 @@ show_stash () {
git diff ${FLAGS:---stat} $b_commit $w_commit
}
+show_help () {
+ exec git help stash
+ exit 1
+}
+
#
# Parses the remaining options looking for flags and
# at most one revision defaulting to ${ref_stash}@{0}
@@ -373,6 +381,9 @@ parse_flags_and_rev()
--index)
INDEX_OPTION=--index
;;
+ --help)
+ show_help
+ ;;
-*)
test $ALLOW_UNKNOWN_FLAGS = t ||
die $(eval_gettext unknown option:
\$opt)
--
2.4.1.396.g7ba6d7b
--
To unsubscribe from this list: send the line 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: [PUB]corrupt repos does not return error with `git fsck`
Hi, On 2015-05-20 19:19, Matthieu Moy wrote: Faheem Mitha fah...@faheem.info writes: Clone the repos https://github.com/fmitha/SICL. Then git show 280c12ab49223c64c6f914944287a7d049cf4dd0 gives fatal: bad object 280c12ab49223c64c6f914944287a7d049cf4dd0 It seems 280c12ab49223c64c6f914944287a7d049cf4dd0 is not an object in your repository. The good news it: I don't think you have a corrupt repository. What makes you think you have an object with identifier 280c12ab49223c64c6f914944287a7d049cf4dd0? I had a similar problem some time ago and tracked it down to a graft that was active while pushing to the public repository. Maybe it's the same problem here? Ciao, Johannes -- To unsubscribe from this list: send the line 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] stash: complain about unknown flags
The option parser for git-stash stuffs unknown flags into
the $FLAGS variable, where they can be accessed by the
individual commands. However, most commands do not even look
at these extra flags, leading to unexpected results like
this:
$ git stash drop --help
Dropped refs/stash@{0} (e6cf6d80faf92bb7828f7b60c47fc61c03bd30a1)
We should notice the extra flags and bail. Rather than
annotate each command to reject a non-empty $FLAGS variable,
we can notice that stash show is the only command that
actually _wants_ arbitrary flags. So we switch the default
mode to reject unknown flags, and let stash_show() opt into
the feature.
Reported-by: Vincent Legoll vincent.leg...@gmail.com
Signed-off-by: Jeff King p...@peff.net
---
This takes away the immediate pain. We may also want to
teach --help to the option. I guess we cannot do better
than just having it run git help stash in all cases (i.e.,
we have no way to get the help for a specific subcommand).
git-stash.sh | 6 +-
t/t3903-stash.sh | 4
2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/git-stash.sh b/git-stash.sh
index 7911f30..c6f492c 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -301,6 +301,7 @@ list_stash () {
}
show_stash () {
+ ALLOW_UNKNOWN_FLAGS=t
assert_stash_like $@
git diff ${FLAGS:---stat} $b_commit $w_commit
@@ -332,13 +333,14 @@ show_stash () {
#
# GIT_QUIET is set to t if -q is specified
# INDEX_OPTION is set to --index if --index is specified.
-# FLAGS is set to the remaining flags
+# FLAGS is set to the remaining flags (if allowed)
#
# dies if:
# * too many revisions specified
# * no revision is specified and there is no stash stack
# * a revision is specified which cannot be resolve to a SHA1
# * a non-existent stash reference is specified
+# * unknown flags were set and ALLOW_UNKNOWN_FLAGS is not t
#
parse_flags_and_rev()
@@ -372,6 +374,8 @@ parse_flags_and_rev()
INDEX_OPTION=--index
;;
-*)
+ test $ALLOW_UNKNOWN_FLAGS = t ||
+ die $(eval_gettext unknown option:
\$opt)
FLAGS=${FLAGS}${FLAGS:+ }$opt
;;
esac
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index 0746eee..7396ca9 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -100,6 +100,10 @@ test_expect_success 'unstashing in a subdirectory' '
)
'
+test_expect_success 'stash drop complains of extra options' '
+ test_must_fail git stash drop --foo
+'
+
test_expect_success 'drop top stash' '
git reset --hard
git stash list stashlist1
--
2.4.1.396.g7ba6d7b
--
To unsubscribe from this list: send the line 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: [PUB]corrupt repos does not return error with `git fsck`
On Wed, May 20, 2015 at 11:24 AM, Faheem Mitha fah...@faheem.info wrote: So, is the repos corrupt or not? Also, I don't understand why you say There is no file 28/0c... however. Why would you expect there to be? I don't see it mentioned in that list. Each object is stored at .git/objects/xz/tail with xz being the first 2 characters of the sha1 and the tail the remaining 38 characters of the sha1. I did not draw a conclusion yet, as I needed to run for a meeting. So the object you're looking for is not there (stating this as a fact). But why would you expect it to be there? At the time of sending the previous email I tried to do a reverse search Give me all objects, which reference objectX but did not succeed yet. Stefan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4] ref-filter: add ref-filter API
On Wed, May 20, 2015 at 9:18 AM, Karthik Nayak karthik@gmail.com wrote: add a ref-filter API to provide functions to filter refs for listing. This will act as a common library for commands like 'tag -l', 'branch -l' and 'for-each-ref'. ref-filter will enable each of these commands to benefit from the features of the others. Mentored-by: Christian Couder christian.cou...@gmail.com Mentored-by: Matthieu Moy matthieu@grenoble-inp.fr Signed-off-by: Karthik Nayak karthik@gmail.com --- Makefile | 1 + ref-filter.c | 73 ref-filter.h | 47 ++ 3 files changed, 121 insertions(+) create mode 100644 ref-filter.c create mode 100644 ref-filter.h A shortcoming of this approach is that it's not blame-friendly. Although those of us following this patch series know that much of the code in this patch was copied from for-each-ref.c, git-blame will not recognize this unless invoked in the very expensive git blame -C -C -C fashion (if I understand correctly). The most blame-friendly way to perform this re-organization is to have the code relocation (line removals and line additions) occur in one patch. There are multiple ways you could arrange to do so. One would be to first have a patch which introduces just a skeleton of the intended API, with do-nothing function implementations. A subsequent patch would then relocate the code from for-each-ref.c to ref-filter.c, and update for-each-ref.c to call into the new (now fleshed-out) API. -- To unsubscribe from this list: send the line 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 v9 5/5] help: respect new common command grouping
'git help' shows common commands in alphabetical order:
The most commonly used git commands are:
addAdd file contents to the index
bisect Find by binary search the change that introduced a bug
branch List, create, or delete branches
checkout Checkout a branch or paths to the working tree
clone Clone a repository into a new directory
commit Record changes to the repository
[...]
without any indication of how commands relate to high-level
concepts or each other. Revise the output to explain their relationship
with the typical Git workflow:
These are common Git commands used in various situations:
start a working area (see also: git help tutorial)
clone Clone a repository into a new directory
init Create an empty Git repository or reinitialize [...]
work on the current change (see also: git help everyday)
addAdd file contents to the index
reset Reset current HEAD to the specified state
examine the history and state (see also: git help revisions)
logShow commit logs
status Show the working tree status
[...]
Helped-by: Eric Sunshine sunsh...@sunshineco.com
Signed-off-by: Ramsay Jones ram...@ramsay1.demon.co.uk
Signed-off-by: Sébastien Guimmara sebastien.guimm...@gmail.com
---
help.c | 24 +++-
1 file changed, 23 insertions(+), 1 deletion(-)
diff --git a/help.c b/help.c
index 2072a87..8f72051 100644
--- a/help.c
+++ b/help.c
@@ -218,17 +218,39 @@ void list_commands(unsigned int colopts,
}
}
+static int cmd_group_cmp(const void *elem1, const void *elem2)
+{
+ const struct cmdname_help *e1 = elem1;
+ const struct cmdname_help *e2 = elem2;
+
+ if (e1-group e2-group)
+ return -1;
+ if (e1-group e2-group)
+ return 1;
+ return strcmp(e1-name, e2-name);
+}
+
void list_common_cmds_help(void)
{
int i, longest = 0;
+ int current_grp = -1;
for (i = 0; i ARRAY_SIZE(common_cmds); i++) {
if (longest strlen(common_cmds[i].name))
longest = strlen(common_cmds[i].name);
}
- puts(_(The most commonly used git commands are:));
+ qsort(common_cmds, ARRAY_SIZE(common_cmds),
+ sizeof(common_cmds[0]), cmd_group_cmp);
+
+ puts(_(These are common Git commands used in various situations:));
+
for (i = 0; i ARRAY_SIZE(common_cmds); i++) {
+ if (common_cmds[i].group != current_grp) {
+ printf(\n%s\n,
_(common_cmd_groups[common_cmds[i].group]));
+ current_grp = common_cmds[i].group;
+ }
+
printf( %s , common_cmds[i].name);
mput_char(' ', longest - strlen(common_cmds[i].name));
puts(_(common_cmds[i].help));
--
2.4.0.GIT
--
To unsubscribe from this list: send the line 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 v9 3/5] generate-cmdlist: parse common group commands
From: Eric Sunshine sunsh...@sunshineco.com
Parse the group block to create the array of group descriptions:
static char *common_cmd_groups[] = {
N_(starting a working area),
N_(working on the current change),
N_(working with others),
N_(examining the history and state),
N_(growing, marking and tweaking your history),
};
then map each element of common_cmds[] to a group via its index:
static struct cmdname_help common_cmds[] = {
{add, N_(Add file contents to the index), 1},
{branch, N_(List, create, or delete branches), 4},
{checkout, N_(Checkout a branch or paths to the ...), 4},
{clone, N_(Clone a repository into a new directory), 0},
{commit, N_(Record changes to the repository), 4},
...
};
so that 'git help' can print those commands grouped by theme.
Only commands tagged with an attribute from the group block are emitted to
common_cmds[].
[commit message by Sébastien Guimmara sebastien.guimm...@gmail.com]
Signed-off-by: Eric Sunshine sunsh...@sunshineco.com
Signed-off-by: Sébastien Guimmara sebastien.guimm...@gmail.com
---
Makefile | 4 ++--
generate-cmdlist.perl | 50 ++
generate-cmdlist.sh | 23 ---
3 files changed, 52 insertions(+), 25 deletions(-)
create mode 100755 generate-cmdlist.perl
delete mode 100755 generate-cmdlist.sh
diff --git a/Makefile b/Makefile
index 655740d..54ec511 100644
--- a/Makefile
+++ b/Makefile
@@ -1694,10 +1694,10 @@ $(BUILT_INS): git$X
ln -s $ $@ 2/dev/null || \
cp $ $@
-common-cmds.h: ./generate-cmdlist.sh command-list.txt
+common-cmds.h: generate-cmdlist.perl command-list.txt
common-cmds.h: $(wildcard Documentation/git-*.txt)
- $(QUIET_GEN)./generate-cmdlist.sh $@+ mv $@+ $@
+ $(QUIET_GEN)$(PERL_PATH) generate-cmdlist.perl command-list.txt $@+
mv $@+ $@
SCRIPT_DEFINES = $(SHELL_PATH_SQ):$(DIFF_SQ):$(GIT_VERSION):\
$(localedir_SQ):$(NO_CURL):$(USE_GETTEXT_SCHEME):$(SANE_TOOL_PATH_SQ):\
diff --git a/generate-cmdlist.perl b/generate-cmdlist.perl
new file mode 100755
index 000..31516e3
--- /dev/null
+++ b/generate-cmdlist.perl
@@ -0,0 +1,50 @@
+#!/usr/bin/perl
+use strict;
+use warnings;
+
+print EOT;
+/* Automatically generated by $0 */
+
+struct cmdname_help {
+ char name[16];
+ char help[80];
+ unsigned char group;
+};
+
+static char *common_cmd_groups[] = {
+EOT
+
+my $n = 0;
+my %grp;
+while () {
+ last if /^### command list/;
+ next if (1../^### common groups/) || /^#/ || /^\s*$/;
+ chop;
+ my ($k, $v) = split ' ', $_, 2;
+ $grp{$k} = $n++;
+ print \tN_(\$v\),\n;
+}
+
+print };\n\nstatic struct cmdname_help common_cmds[] = {\n;
+
+while () {
+ next if /^#/ || /^\s*$/;
+ my @tags = split;
+ my $cmd = shift @tags;
+ for my $t (@tags) {
+ if (exists $grp{$t}) {
+ my $s;
+ open my $f, '', Documentation/$cmd.txt or die;
+ while ($f) {
+ ($s) = /^$cmd - (.+)$/;
+ last if $s;
+ }
+ close $f;
+ $cmd =~ s/^git-//;
+ print \t{\$cmd\, N_(\$s\), $grp{$t}},\n;
+ last;
+ }
+ }
+}
+
+print };\n;
diff --git a/generate-cmdlist.sh b/generate-cmdlist.sh
deleted file mode 100755
index 9a4c9b9..000
--- a/generate-cmdlist.sh
+++ /dev/null
@@ -1,23 +0,0 @@
-#!/bin/sh
-
-echo /* Automatically generated by $0 */
-struct cmdname_help {
-char name[16];
-char help[80];
-};
-
-static struct cmdname_help common_cmds[] = {
-
-sed -n -e 's/^git-\([^ ]*\)[ ].* common.*/\1/p' command-list.txt |
-sort |
-while read cmd
-do
- sed -n '
- /^NAME/,/git-'$cmd'/H
- ${
- x
- s/.*git-'$cmd' - \(.*\)/ {'$cmd', N_(\1)},/
- p
- }' Documentation/git-$cmd.txt
-done
-echo };
--
2.4.0.GIT
--
To unsubscribe from this list: send the line 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 v9 2/5] command-list.txt: add the common groups block
The ultimate goal is for git help to display common commands in groups rather than alphabetically. As a first step, define the groups in a new block, and then assign a group to each common command. Add a block at the beginning of command-list.txt: init start a working area (see also: git help tutorial) worktree work on the current change (see also:[...] info examine the history and state (see also: git [...] history grow, mark and tweak your history remote collaborate (see also: git help workflows) storing information about common commands group, then map each common command to a group: git-add mainporcelaincommon worktree Helped-by: Eric Sunshine sunsh...@sunshineco.com Helped-by: Junio C Hamano gits...@pobox.com Helped-by: Emma Jane Hogbin Westby emma.wes...@gmail.com Signed-off-by: Sébastien Guimmara sebastien.guimm...@gmail.com --- command-list.txt | 50 ++ 1 file changed, 30 insertions(+), 20 deletions(-) diff --git a/command-list.txt b/command-list.txt index 609b344..c2bbdc1 100644 --- a/command-list.txt +++ b/command-list.txt @@ -1,3 +1,13 @@ +# common commands are grouped by themes output by 'git help' +# map each common command in the command list to one of these groups. +### common groups +init start a working area (see also: git help tutorial) +worktree work on the current change (see also: git help everyday) +info examine the history and state (see also: git help revisions) +history grow, mark and tweak your common history +remote collaborate (see also: git help workflows) + +# List of known git commands. # do not molest the next line ### command list # command name category [deprecated] [common] @@ -7,24 +17,24 @@ git-annotate ancillaryinterrogators git-apply plumbingmanipulators git-archimport foreignscminterface git-archive mainporcelain -git-bisect mainporcelain common +git-bisect mainporcelain common info git-blame ancillaryinterrogators -git-branch mainporcelain common +git-branch mainporcelain common history git-bundle mainporcelain git-cat-fileplumbinginterrogators git-check-attr purehelpers git-check-ignorepurehelpers git-check-mailmap purehelpers -git-checkoutmainporcelain common +git-checkoutmainporcelain common history git-checkout-index plumbingmanipulators git-check-ref-formatpurehelpers git-cherry ancillaryinterrogators git-cherry-pick mainporcelain git-citool mainporcelain git-clean mainporcelain -git-clone mainporcelain common +git-clone mainporcelain common init git-column purehelpers -git-commit mainporcelain common +git-commit mainporcelain common history git-commit-tree plumbingmanipulators git-config ancillarymanipulators git-count-objects ancillaryinterrogators @@ -36,14 +46,14 @@ git-cvsimport foreignscminterface git-cvsserver foreignscminterface git-daemon synchingrepositories git-describemainporcelain -git-diffmainporcelain common +git-diffmainporcelain common history git-diff-files plumbinginterrogators git-diff-index plumbinginterrogators git-diff-tree plumbinginterrogators git-difftoolancillaryinterrogators git-fast-export ancillarymanipulators git-fast-import ancillarymanipulators -git-fetch mainporcelain common +git-fetch mainporcelain common remote git-fetch-pack synchingrepositories git-filter-branch ancillarymanipulators git-fmt-merge-msg purehelpers @@ -52,7 +62,7 @@ git-format-patchmainporcelain git-fsckancillaryinterrogators git-gc
[PATCH v9 1/5] command-list: prepare machinery for upcoming common groups section
From: Eric Sunshine sunsh...@sunshineco.com
The ultimate goal is for git help to classify common commands by
group. Toward this end, a subsequent patch will add a new common
groups section to command-list.txt preceding the actual command list.
As preparation, teach existing command-list.txt parsing machinery, which
doesn't care about grouping, to skip over this upcoming common groups
section.
Signed-off-by: Eric Sunshine sunsh...@sunshineco.com
Signed-off-by: Sébastien Guimmara sebastien.guimm...@gmail.com
---
Documentation/cmd-list.perl | 4
Makefile| 5 +++--
command-list.txt| 3 ++-
3 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/Documentation/cmd-list.perl b/Documentation/cmd-list.perl
index 04f9977..5aa73cf 100755
--- a/Documentation/cmd-list.perl
+++ b/Documentation/cmd-list.perl
@@ -38,6 +38,10 @@ sub format_one {
}
}
+while () {
+ last if /^### command list/;
+}
+
my %cmds = ();
for (sort ) {
next if /^#/;
diff --git a/Makefile b/Makefile
index 323c401..655740d 100644
--- a/Makefile
+++ b/Makefile
@@ -2455,7 +2455,7 @@ check-docs::
esac ; \
test -f Documentation/$$v.txt || \
echo no doc: $$v; \
- sed -e '/^#/d' command-list.txt | \
+ sed -e '1,/^### command list/d' -e '/^#/d' command-list.txt | \
grep -q ^$$v[ ] || \
case $$v in \
git) ;; \
@@ -2463,7 +2463,8 @@ check-docs::
esac ; \
done; \
( \
- sed -e '/^#/d' \
+ sed -e '1,/^### command list/d' \
+ -e '/^#/d' \
-e 's/[ ].*//' \
-e 's/^/listed /' command-list.txt; \
$(MAKE) -C Documentation print-man1 | \
diff --git a/command-list.txt b/command-list.txt
index 54d8d21..609b344 100644
--- a/command-list.txt
+++ b/command-list.txt
@@ -1,4 +1,5 @@
-# List of known git commands.
+# do not molest the next line
+### command list
# command name category [deprecated] [common]
git-add mainporcelain common
git-am mainporcelain
--
2.4.0.GIT
--
To unsubscribe from this list: send the line 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 v9 4/5] command-list.txt: drop the common tag
command-list.sh, retired in the previous patch, was the only consumer of the common tag, so drop this now-unnecessary attribute. before: git-add mainporcelaincommon worktree after: git-add mainporcelainworktree Helped-by: Eric Sunshine sunsh...@sunshineco.com Signed-off-by: Sébastien Guimmara sebastien.guimm...@gmail.com --- command-list.txt | 42 +- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/command-list.txt b/command-list.txt index c2bbdc1..080d6d9 100644 --- a/command-list.txt +++ b/command-list.txt @@ -11,30 +11,30 @@ remote collaborate (see also: git help workflows) # do not molest the next line ### command list # command name category [deprecated] [common] -git-add mainporcelain common +git-add mainporcelain worktree git-am mainporcelain git-annotateancillaryinterrogators git-apply plumbingmanipulators git-archimport foreignscminterface git-archive mainporcelain -git-bisect mainporcelain common info +git-bisect mainporcelain info git-blame ancillaryinterrogators -git-branch mainporcelain common history +git-branch mainporcelain history git-bundle mainporcelain git-cat-fileplumbinginterrogators git-check-attr purehelpers git-check-ignorepurehelpers git-check-mailmap purehelpers -git-checkoutmainporcelain common history +git-checkoutmainporcelain history git-checkout-index plumbingmanipulators git-check-ref-formatpurehelpers git-cherry ancillaryinterrogators git-cherry-pick mainporcelain git-citool mainporcelain git-clean mainporcelain -git-clone mainporcelain common init +git-clone mainporcelain init git-column purehelpers -git-commit mainporcelain common history +git-commit mainporcelain history git-commit-tree plumbingmanipulators git-config ancillarymanipulators git-count-objects ancillaryinterrogators @@ -46,14 +46,14 @@ git-cvsimport foreignscminterface git-cvsserver foreignscminterface git-daemon synchingrepositories git-describemainporcelain -git-diffmainporcelain common history +git-diffmainporcelain history git-diff-files plumbinginterrogators git-diff-index plumbinginterrogators git-diff-tree plumbinginterrogators git-difftoolancillaryinterrogators git-fast-export ancillarymanipulators git-fast-import ancillarymanipulators -git-fetch mainporcelain common remote +git-fetch mainporcelain remote git-fetch-pack synchingrepositories git-filter-branch ancillarymanipulators git-fmt-merge-msg purehelpers @@ -62,7 +62,7 @@ git-format-patchmainporcelain git-fsckancillaryinterrogators git-gc mainporcelain git-get-tar-commit-id ancillaryinterrogators -git-grepmainporcelain common info +git-grepmainporcelain info git-gui mainporcelain git-hash-object plumbingmanipulators git-helpancillaryinterrogators @@ -71,17 +71,17 @@ git-http-fetch synchelpers git-http-push synchelpers git-imap-send foreignscminterface git-index-pack plumbingmanipulators -git-initmainporcelain common init +git-init
Re: [PATCH v9 0/5] group common commands by theme
On Wed, May 20, 2015 at 3:22 PM, Sébastien Guimmara sebastien.guimm...@gmail.com wrote: The major modification of this reroll [1] is the use of the perl version of generate-cmdlist instead of the awk one. help.c: 1. change the introductory message from: The typical Git workflow includes: to: These are common Git commands used in various situations: Which is just a longer way to say what the original said: The most commonly used git commands are: I don't care strongly, but the terseness of the original has a certain appeal. -- To unsubscribe from this list: send the line 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: [PUB]corrupt repos does not return error with `git fsck`
On Wed, May 20, 2015 at 11:02:14AM -0700, Stefan Beller wrote: $ git clone https://github.com/fmitha/SICL cd SICL $ git show 280c12ab49223c64c6f914944287a7d049cf4dd0 fatal: bad object 280c12ab49223c64c6f914944287a7d049cf4dd0 $ git show 12323213123 # just to be sure to have a different error message for non existing objects. fatal: ambiguous argument '12323213123': unknown revision or path not in the working tree. I think 40 hex characters is special cased. Using CGit as a repository with a submodule so I can easily get an unrelated SHA1 and short name: cgit $ git show $(git -C git rev-parse @) fatal: bad object bb8577532add843833ebf8b5324f94f84cb71ca0 cgit $ git show $(git -C git rev-parse --short @) fatal: ambiguous argument 'bb85775': unknown revision or path not in the working tree. Use '--' to separate paths from revisions, like this: 'git command [revision...] -- [file...]' -- To unsubscribe from this list: send the line 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 v9 0/5] group common commands by theme
The major modification of this reroll [1] is the use of the perl version of generate-cmdlist instead of the awk one. help.c: 1. change the introductory message from: The typical Git workflow includes: to: These are common Git commands used in various situations: 2. include Ramsay's patch [2] [1]: v8: http://thread.gmane.org/gmane.comp.version-control.git/269305 [2]: http://thread.gmane.org/gmane.comp.version-control.git/269387 Eric Sunshine (2): command-list: prepare machinery for upcoming common groups section generate-cmdlist: parse common group commands Sébastien Guimmara (3): command-list.txt: add the common groups block command-list.txt: drop the common tag help: respect new common command grouping Documentation/cmd-list.perl | 4 Makefile| 9 command-list.txt| 53 +++-- generate-cmdlist.perl | 50 ++ generate-cmdlist.sh | 23 help.c | 24 +++- 6 files changed, 114 insertions(+), 49 deletions(-) create mode 100755 generate-cmdlist.perl delete mode 100755 generate-cmdlist.sh -- 2.4.0.GIT -- To unsubscribe from this list: send the line 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] submodule documentation: Reorder introductory paragraphs
It's better to start the man page with a description of what submodules actually are instead of saying what they are not. Reorder the paragraphs such that the first short paragraph introduces the submodule concept, the second paragraph highlights the usage of the submodule command, the third paragraph giving background information, and finally the fourth paragraph discusing alternatives such as subtrees and remotes, which we don't want to be confused with. This ordering deepens the knowledge on submodules with each paragraph. First the basic questions like How/what will be answered, while the underlying concepts will be taught at a later time. Making sure it is not confused with subtrees and remotes is not really enhancing knowledge of submodules itself, but rather painting the big picture of git concepts, so you could also argue to have it as the second paragraph. Personally I think this may confuse readers, specially newcomers though. Signed-off-by: Stefan Beller sbel...@google.com --- Documentation/git-submodule.txt | 54 - 1 file changed, 26 insertions(+), 28 deletions(-) diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt index 2c25916..6c38c0d 100644 --- a/Documentation/git-submodule.txt +++ b/Documentation/git-submodule.txt @@ -25,35 +25,12 @@ SYNOPSIS DESCRIPTION --- -Submodules allow foreign repositories to be embedded within -a dedicated subdirectory of the source tree, always pointed -at a particular commit. +Submodules allow other repositories to be embedded within +a dedicated subdirectory of the source tree pointing +at a particular commit in the other repository. -They are not to be confused with remotes, which are meant mainly -for branches of the same project; submodules are meant for -different projects you would like to make part of your source tree, -while the history of the two projects still stays completely -independent and you cannot modify the contents of the submodule -from within the main project. -If you want to merge the project histories and want to treat the -aggregated whole as a single project from then on, you may want to -add a remote for the other project and use the 'subtree' merge strategy, -instead of treating the other project as a submodule. Directories -that come from both projects can be cloned and checked out as a whole -if you choose to go that route. - -Submodules are composed from a so-called `gitlink` tree entry -in the main repository that refers to a particular commit object -within the inner repository that is completely separate. -A record in the `.gitmodules` (see linkgit:gitmodules[5]) file at the -root of the source tree assigns a logical name to the submodule and -describes the default URL the submodule shall be cloned from. -The logical name can be used for overriding this URL within your -local repository configuration (see 'submodule init'). - -This command will manage the tree entries and contents of the -gitmodules file for you, as well as inspect the status of your -submodules and update them. +This command will manage the submodules for you, as well as +inspect the status of your submodules and update them. When adding a new submodule to the tree, the 'add' subcommand is to be used. However, when pulling a tree containing submodules, these will not be checked out by default; @@ -64,6 +41,27 @@ using the 'status' subcommand and get a detailed overview of the difference between the index and checkouts using the 'summary' subcommand. +Submodules are composed from a so-called `gitlink` tree entry +in the main repository that refers to a particular commit object +within the inner repository that is completely separate. +A record in the `.gitmodules` (see linkgit:gitmodules[5]) file at the +root of the source tree assigns a logical name to the submodule and +describes the default URL the submodule shall be cloned from. +The logical name can be used for overriding this URL within your +local repository configuration (see 'submodule init'). + +Submodules are not to be confused with remotes, which are meant +mainly for branches of the same project; submodules are meant for +different projects you would like to make part of your source tree, +while the history of the two projects still stays completely +independent and you cannot modify the contents of the submodule +from within the main project. +If you want to merge the project histories and want to treat the +aggregated whole as a single project from then on, you may want to +add a remote for the other project and use the 'subtree' merge strategy, +instead of treating the other project as a submodule. Directories +that come from both projects can be cloned and checked out as a whole +if you choose to go that route. COMMANDS -- 2.4.0.194.gc518059 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at
Re: [PATCH 1/3] git-verify-pack.txt: fix inconsistent spelling of packfile
Jeff King p...@peff.net writes: On Wed, May 20, 2015 at 12:45:09PM -0700, Junio C Hamano wrote: One related thing is that there are few mentions of idx file to refer to pack index (e.g. show-index and verify-pack documentation pages); I think this was an attempt to disambiguate pack index from the Index, but as long as we spell it pack index, I think it should be OK, so while we are at it we may want to fix them. We can leave pack .idx file as-is, but rewriting it to pack index file or just pack index may be OK as long as it is clear from the context. git show-index has this in SYNOPSIS: 'git show-index' idx-file It probably should become 'git show-index' pack-index That makes pack-file make more sense to me. It is not the abstract concept of a packfile, but the file with the .pack extension (just as idx-file is the file with the .idx extension). They are the same thing if you think about it, of course, but you might choose one over the other depending on the context. Hmm, that is also true. In any case, even though I merged these three to 'next', I think we need to either revert 3/3 or do s/pack-file/packfile/ throughout the pack-protocol documentation. The original has something like this: The pack-file MUST NOT be sent if the only command used is 'delete'. A pack-file MUST be sent if either create or update command is used, even if the server already has all the necessary objects. In this case the client MUST send an empty pack-file. The only time this is likely to happen is if the client is creating a new branch or a tag that points to an existing obj-id. and these are explicitly referring to what EBNF defines as pack-file. Changing them to packfile is simply wrong. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] for-each-ref: convert to ref-filter
Karthik Nayak karthik@gmail.com writes: convert 'for-each-ref' to use the common API provided by 'ref-filter'. Start a sentence with capital? More importantly, the above is misleading, as if you invented a new ref-filter API and made for-each-ref build on that new infrastructure. This series is in a form that is very unfriendly to reviewers. The previous step did not introduce any callers to ref-filter, so for the purpose of review, it needs to be read together with this step anyway. And when reading these patches that way, what this half is really doing is to move the code from for-each-ref to ref-filter, but it does unnecessary or unrelated renaming of a handful of symbols. It makes it even harder to compare and contrast the original code that was in the original for-each-ref and moved code that ends up in the new ref-filter. Don't do that. You would probably want to organize them in these two steps instead: * Rename symbols as necessary while all the code is still in for-each-ref. Do not create ref-filter in this step. Justify it along the lines of some symbol names were fine while they were file scope static implementation detail of for-each-ref, but we will make the machinery available from other commands by moving it to a library-ish place, so rename X to foo_X to clarify that this is about foo (which is now necessary as it is not specific to for-each-ref... * If you want to do other tweaks like wrapping refs num_refs into a single structure, do so while the code is still in for-each-ref. You can do that in the same patch as the above (i.e. it's just part of preparatory step for a move). * Create ref-filter by _moving_ code from for-each-ref. Do not touch these moved lines in this step. You would need to add include at the top of for-each-ref and ref-filter, of course. - for_each_rawref(grab_single_ref, cbdata); - refs = cbdata.grab_array; - num_refs = cbdata.grab_cnt; + refs.name_patterns = argv; + for_each_rawref(ref_filter_add, refs); I think ref_filter_add() may be misnamed as a public API function. grab_single_ref() was OK only because it was an implementation dtail, but if you are making it public, the name should make it clear that it is meant to be used as a for_each_*ref() callback function. Otherwise people may be tempted to add random parameter to it in the future, but the signature of that function is dictated by for_each_*ref() API. -- To unsubscribe from this list: send the line 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/3] git-verify-pack.txt: fix inconsistent spelling of packfile
Jeff King p...@peff.net writes: Yeah, I agree they should agree with the EBNF. And my inclination is for packfile, as it is refering to the concept of the on-the-wire packfile data (there is no file ending in .pack in this context). Which I guess argues for a further patch. I'm fine with that, then. If I had a time machine, I would have used pack data (or pack stream) vs pack file (or .pack file) to differentiatee (as the pack-protocol is not about transferring any file, but just carries pack data), but that is a rename with more cost than warranted for a minuscule gain at this point. -- To unsubscribe from this list: send the line 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 08/14] remote.c: report specific errors from branch_get_upstream
When the previous commit introduced the branch_get_upstream
helper, there was one call-site that could not be converted:
the one in sha1_name.c, which gives detailed error messages
for each possible failure.
Let's teach the helper to optionally report these specific
errors. This lets us convert another callsite, and means we
can use the helper in other locations that want to give the
same error messages.
The logic and error messages come straight from sha1_name.c,
with the exception that we start each error with a lowercase
letter, as is our usual style (note that a few tests need
updated as a result).
Signed-off-by: Jeff King p...@peff.net
---
builtin/branch.c | 2 +-
builtin/for-each-ref.c| 2 +-
builtin/log.c | 2 +-
remote.c | 33 +
remote.h | 6 +-
sha1_name.c | 25 +++--
t/t1507-rev-parse-upstream.sh | 8
7 files changed, 48 insertions(+), 30 deletions(-)
diff --git a/builtin/branch.c b/builtin/branch.c
index 1eb6215..cc55ff2 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -123,7 +123,7 @@ static int branch_merged(int kind, const char *name,
if (kind == REF_LOCAL_BRANCH) {
struct branch *branch = branch_get(name);
- const char *upstream = branch_get_upstream(branch);
+ const char *upstream = branch_get_upstream(branch, NULL);
unsigned char sha1[20];
if (upstream
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index dc2a201..18d209b 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -664,7 +664,7 @@ static void populate_value(struct refinfo *ref)
continue;
branch = branch_get(ref-refname + 11);
- refname = branch_get_upstream(branch);
+ refname = branch_get_upstream(branch, NULL);
if (!refname)
continue;
} else if (starts_with(name, color:)) {
diff --git a/builtin/log.c b/builtin/log.c
index fb61c08..6faeb82 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1632,7 +1632,7 @@ int cmd_cherry(int argc, const char **argv, const char
*prefix)
break;
default:
current_branch = branch_get(NULL);
- upstream = branch_get_upstream(current_branch);
+ upstream = branch_get_upstream(current_branch, NULL);
if (!upstream) {
fprintf(stderr, _(Could not find a tracked
remote branch, please
diff --git a/remote.c b/remote.c
index dca3442..1b7051a 100644
--- a/remote.c
+++ b/remote.c
@@ -1705,10 +1705,35 @@ int branch_merge_matches(struct branch *branch,
return refname_match(branch-merge[i]-src, refname);
}
-const char *branch_get_upstream(struct branch *branch)
+__attribute((format (printf,2,3)))
+static const char *error_buf(struct strbuf *err, const char *fmt, ...)
{
- if (!branch || !branch-merge || !branch-merge[0])
- return NULL;
+ if (err) {
+ va_list ap;
+ va_start(ap, fmt);
+ strbuf_vaddf(err, fmt, ap);
+ va_end(ap);
+ }
+ return NULL;
+}
+
+const char *branch_get_upstream(struct branch *branch, struct strbuf *err)
+{
+ if (!branch)
+ return error_buf(err, _(HEAD does not point to a branch));
+ if (!branch-merge || !branch-merge[0] || !branch-merge[0]-dst) {
+ if (!ref_exists(branch-refname))
+ return error_buf(err, _(no such branch: '%s'),
+branch-name);
+ if (!branch-merge)
+ return error_buf(err,
+_(no upstream configured for branch
'%s'),
+branch-name);
+ return error_buf(err,
+_(upstream branch '%s' not stored as a
remote-tracking branch),
+branch-merge[0]-src);
+ }
+
return branch-merge[0]-dst;
}
@@ -1921,7 +1946,7 @@ int stat_tracking_info(struct branch *branch, int
*num_ours, int *num_theirs)
int rev_argc;
/* Cannot stat unless we are marked to build on top of somebody else. */
- base = branch_get_upstream(branch);
+ base = branch_get_upstream(branch, NULL);
if (!base)
return 0;
diff --git a/remote.h b/remote.h
index d968952..03ca005 100644
--- a/remote.h
+++ b/remote.h
@@ -222,8 +222,12 @@ int branch_merge_matches(struct branch *, int n, const
char *);
* Return the fully-qualified refname of the tracking branch for `branch`.
* I.e., what branch@{upstream} would give you. Returns NULL if no
* upstream is defined.
+ *
[PATCH v3 06/14] remote.c: hoist read_config into remote_get_1
Before the previous commit, we had to make sure that
read_config() was called before entering remote_get_1,
because we needed to pass pushremote_name by value. But now
that we pass a function, we can let remote_get_1 handle
loading the config itself, turning our wrappers into true
one-liners.
Signed-off-by: Jeff King p...@peff.net
---
remote.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/remote.c b/remote.c
index a91d063..e6b29b3 100644
--- a/remote.c
+++ b/remote.c
@@ -718,6 +718,8 @@ static struct remote *remote_get_1(const char *name,
struct remote *ret;
int name_given = 0;
+ read_config();
+
if (name)
name_given = 1;
else
@@ -741,13 +743,11 @@ static struct remote *remote_get_1(const char *name,
struct remote *remote_get(const char *name)
{
- read_config();
return remote_get_1(name, remote_for_branch);
}
struct remote *pushremote_get(const char *name)
{
- read_config();
return remote_get_1(name, pushremote_for_branch);
}
--
2.4.1.528.g00591e3
--
To unsubscribe from this list: send the line 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 12/14] sha1_name: implement @{push} shorthand
In a triangular workflow, each branch may have two distinct
points of interest: the @{upstream} that you normally pull
from, and the destination that you normally push to. There
isn't a shorthand for the latter, but it's useful to have.
For instance, you may want to know which commits you haven't
pushed yet:
git log @{push}..
Or as a more complicated example, imagine that you normally
pull changes from origin/master (which you set as your
@{upstream}), and push changes to your own personal fork
(e.g., as myfork/topic). You may push to your fork from
multiple machines, requiring you to integrate the changes
from the push destination, rather than upstream. With this
patch, you can just do:
git rebase @{push}
rather than typing out the full name.
The heavy lifting is all done by branch_get_push; here we
just wire it up to the @{push} syntax.
Signed-off-by: Jeff King p...@peff.net
---
Documentation/revisions.txt | 25 ++
sha1_name.c | 14 +-
t/t1514-rev-parse-push.sh | 63 +
3 files changed, 101 insertions(+), 1 deletion(-)
create mode 100755 t/t1514-rev-parse-push.sh
diff --git a/Documentation/revisions.txt b/Documentation/revisions.txt
index 0796118..d85e303 100644
--- a/Documentation/revisions.txt
+++ b/Documentation/revisions.txt
@@ -98,6 +98,31 @@ some output processing may assume ref names in UTF-8.
`branch.name.merge`). A missing branchname defaults to the
current one.
+'branchname@\{push\}', e.g. 'master@\{push\}', '@\{push\}'::
+ The suffix '@\{push}' reports the branch where we would push to if
+ `git push` were run while `branchname` was checked out (or the current
+ 'HEAD' if no branchname is specified). Since our push destination is
+ in a remote repository, of course, we report the local tracking branch
+ that corresponds to that branch (i.e., something in 'refs/remotes/').
++
+Here's an example to make it more clear:
++
+--
+$ git config push.default current
+$ git config remote.pushdefault myfork
+$ git checkout -b mybranch origin/master
+
+$ git rev-parse --symbolic-full-name @{upstream}
+refs/remotes/origin/master
+
+$ git rev-parse --symbolic-full-name @{push}
+refs/remotes/myfork/mybranch
+--
++
+Note in the example that we set up a triangular workflow, where we pull
+from one location and push to another. In a non-triangular workflow,
+'@\{push}' is the same as '@\{upstream}', and there is no need for it.
+
'rev{caret}', e.g. 'HEAD{caret}, v1.5.1{caret}0'::
A suffix '{caret}' to a revision parameter means the first parent of
that commit object. '{caret}n' means the nth parent (i.e.
diff --git a/sha1_name.c b/sha1_name.c
index 506e0c9..1096943 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -435,6 +435,12 @@ static inline int upstream_mark(const char *string, int
len)
return at_mark(string, len, suffix, ARRAY_SIZE(suffix));
}
+static inline int push_mark(const char *string, int len)
+{
+ const char *suffix[] = { @{push} };
+ return at_mark(string, len, suffix, ARRAY_SIZE(suffix));
+}
+
static int get_sha1_1(const char *name, int len, unsigned char *sha1, unsigned
lookup_flags);
static int interpret_nth_prior_checkout(const char *name, int namelen, struct
strbuf *buf);
@@ -482,7 +488,8 @@ static int get_sha1_basic(const char *str, int len,
unsigned char *sha1,
nth_prior = 1;
continue;
}
- if (!upstream_mark(str + at, len - at)) {
+ if (!upstream_mark(str + at, len - at)
+ !push_mark(str + at, len - at)) {
reflog_len = (len-1) - (at+2);
len = at;
}
@@ -1145,6 +1152,11 @@ int interpret_branch_name(const char *name, int namelen,
struct strbuf *buf)
upstream_mark, branch_get_upstream);
if (len 0)
return len;
+
+ len = interpret_branch_mark(name, namelen, at - name, buf,
+ push_mark, branch_get_push);
+ if (len 0)
+ return len;
}
return -1;
diff --git a/t/t1514-rev-parse-push.sh b/t/t1514-rev-parse-push.sh
new file mode 100755
index 000..7214f5b
--- /dev/null
+++ b/t/t1514-rev-parse-push.sh
@@ -0,0 +1,63 @@
+#!/bin/sh
+
+test_description='test branch@{push} syntax'
+. ./test-lib.sh
+
+resolve () {
+ echo $2 expect
+ git rev-parse --symbolic-full-name $1 actual
+ test_cmp expect actual
+}
+
+test_expect_success 'setup' '
+ git init --bare parent.git
+ git init --bare other.git
+ git remote add origin parent.git
+ git remote add other
[PATCH v3 07/14] remote.c: introduce branch_get_upstream helper
All of the information needed to find the @{upstream} of a
branch is included in the branch struct, but callers have to
navigate a series of possible-NULL values to get there.
Let's wrap that logic up in an easy-to-read helper.
Signed-off-by: Jeff King p...@peff.net
---
builtin/branch.c | 8 +++-
builtin/for-each-ref.c | 5 ++---
builtin/log.c | 7 ++-
remote.c | 12 +---
remote.h | 7 +++
5 files changed, 23 insertions(+), 16 deletions(-)
diff --git a/builtin/branch.c b/builtin/branch.c
index 258fe2f..1eb6215 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -123,14 +123,12 @@ static int branch_merged(int kind, const char *name,
if (kind == REF_LOCAL_BRANCH) {
struct branch *branch = branch_get(name);
+ const char *upstream = branch_get_upstream(branch);
unsigned char sha1[20];
- if (branch
- branch-merge
- branch-merge[0]
- branch-merge[0]-dst
+ if (upstream
(reference_name = reference_name_to_free =
-resolve_refdup(branch-merge[0]-dst, RESOLVE_REF_READING,
+resolve_refdup(upstream, RESOLVE_REF_READING,
sha1, NULL)) != NULL)
reference_rev = lookup_commit_reference(sha1);
}
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 83f9cf9..dc2a201 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -664,10 +664,9 @@ static void populate_value(struct refinfo *ref)
continue;
branch = branch_get(ref-refname + 11);
- if (!branch || !branch-merge || !branch-merge[0] ||
- !branch-merge[0]-dst)
+ refname = branch_get_upstream(branch);
+ if (!refname)
continue;
- refname = branch-merge[0]-dst;
} else if (starts_with(name, color:)) {
char color[COLOR_MAXLEN] = ;
diff --git a/builtin/log.c b/builtin/log.c
index dd8f3fc..fb61c08 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1632,16 +1632,13 @@ int cmd_cherry(int argc, const char **argv, const char
*prefix)
break;
default:
current_branch = branch_get(NULL);
- if (!current_branch || !current_branch-merge
- || !current_branch-merge[0]
- || !current_branch-merge[0]-dst) {
+ upstream = branch_get_upstream(current_branch);
+ if (!upstream) {
fprintf(stderr, _(Could not find a tracked
remote branch, please
specify upstream manually.\n));
usage_with_options(cherry_usage, options);
}
-
- upstream = current_branch-merge[0]-dst;
}
init_revisions(revs, prefix);
diff --git a/remote.c b/remote.c
index e6b29b3..dca3442 100644
--- a/remote.c
+++ b/remote.c
@@ -1705,6 +1705,13 @@ int branch_merge_matches(struct branch *branch,
return refname_match(branch-merge[i]-src, refname);
}
+const char *branch_get_upstream(struct branch *branch)
+{
+ if (!branch || !branch-merge || !branch-merge[0])
+ return NULL;
+ return branch-merge[0]-dst;
+}
+
static int ignore_symref_update(const char *refname)
{
unsigned char sha1[20];
@@ -1914,12 +1921,11 @@ int stat_tracking_info(struct branch *branch, int
*num_ours, int *num_theirs)
int rev_argc;
/* Cannot stat unless we are marked to build on top of somebody else. */
- if (!branch ||
- !branch-merge || !branch-merge[0] || !branch-merge[0]-dst)
+ base = branch_get_upstream(branch);
+ if (!base)
return 0;
/* Cannot stat if what we used to build on no longer exists */
- base = branch-merge[0]-dst;
if (read_ref(base, sha1))
return -1;
theirs = lookup_commit_reference(sha1);
diff --git a/remote.h b/remote.h
index 30a11da..d968952 100644
--- a/remote.h
+++ b/remote.h
@@ -218,6 +218,13 @@ const char *pushremote_for_branch(struct branch *branch,
int *explicit);
int branch_has_merge_config(struct branch *branch);
int branch_merge_matches(struct branch *, int n, const char *);
+/**
+ * Return the fully-qualified refname of the tracking branch for `branch`.
+ * I.e., what branch@{upstream} would give you. Returns NULL if no
+ * upstream is defined.
+ */
+const char *branch_get_upstream(struct branch *branch);
+
/* Flags to match_refs. */
enum match_refs_flags {
MATCH_REFS_NONE = 0,
--
2.4.1.528.g00591e3
--
To unsubscribe from this list:
[PATCH v3 14/14] for-each-ref: accept %(push) format
Just as we have %(upstream) to report the @{upstream}
for each ref, this patch adds %(push) to match @{push}.
It supports the same tracking format modifiers as upstream
(because you may want to know, for example, which branches
have commits to push).
Signed-off-by: Jeff King p...@peff.net
---
Documentation/git-for-each-ref.txt | 6 ++
builtin/for-each-ref.c | 17 +++--
t/t6300-for-each-ref.sh| 13 -
3 files changed, 33 insertions(+), 3 deletions(-)
diff --git a/Documentation/git-for-each-ref.txt
b/Documentation/git-for-each-ref.txt
index 4240875..7f8d9a5 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -97,6 +97,12 @@ upstream::
or = (in sync). Has no effect if the ref does not have
tracking information associated with it.
+push::
+ The name of a local ref which represents the `@{push}` location
+ for the displayed ref. Respects `:short`, `:track`, and
+ `:trackshort` options as `upstream` does. Produces an empty
+ string if no `@{push}` ref is configured.
+
HEAD::
'*' if HEAD matches current ref (the checked out branch), ' '
otherwise.
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 345d8dd..6847400 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -74,6 +74,7 @@ static struct {
{ contents:body },
{ contents:signature },
{ upstream },
+ { push },
{ symref },
{ flag },
{ HEAD },
@@ -669,6 +670,16 @@ static void populate_value(struct refinfo *ref)
refname = branch_get_upstream(branch, NULL);
if (!refname)
continue;
+ } else if (starts_with(name, push)) {
+ const char *branch_name;
+ if (!skip_prefix(ref-refname, refs/heads/,
+branch_name))
+ continue;
+ branch = branch_get(branch_name);
+
+ refname = branch_get_push(branch, NULL);
+ if (!refname)
+ continue;
} else if (starts_with(name, color:)) {
char color[COLOR_MAXLEN] = ;
@@ -714,7 +725,8 @@ static void populate_value(struct refinfo *ref)
refname = shorten_unambiguous_ref(refname,
warn_ambiguous_refs);
else if (!strcmp(formatp, track)
-starts_with(name, upstream)) {
+(starts_with(name, upstream) ||
+ starts_with(name, push))) {
char buf[40];
if (stat_tracking_info(branch, num_ours,
@@ -736,7 +748,8 @@ static void populate_value(struct refinfo *ref)
}
continue;
} else if (!strcmp(formatp, trackshort)
- starts_with(name, upstream)) {
+ (starts_with(name, upstream) ||
+ starts_with(name, push))) {
assert(branch);
if (stat_tracking_info(branch, num_ours,
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index c66bf79..24fc2ba 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -28,7 +28,10 @@ test_expect_success setup '
git update-ref refs/remotes/origin/master master
git remote add origin nowhere
git config branch.master.remote origin
- git config branch.master.merge refs/heads/master
+ git config branch.master.merge refs/heads/master
+ git remote add myfork elsewhere
+ git config remote.pushdefault myfork
+ git config push.default current
'
test_atom() {
@@ -47,6 +50,7 @@ test_atom() {
test_atom head refname refs/heads/master
test_atom head upstream refs/remotes/origin/master
+test_atom head push refs/remotes/myfork/master
test_atom head objecttype commit
test_atom head objectsize 171
test_atom head objectname $(git rev-parse refs/heads/master)
@@ -83,6 +87,7 @@ test_atom head HEAD '*'
test_atom tag refname refs/tags/testtag
test_atom tag upstream ''
+test_atom tag push ''
test_atom tag objecttype tag
test_atom tag objectsize 154
test_atom tag objectname $(git rev-parse refs/tags/testtag)
@@ -347,6 +352,12 @@ test_expect_success 'Check that :track[short] works when
upstream is invalid' '
test_cmp expected actual
'
+test_expect_success '%(push) supports tracking specifiers, too' '
+ echo [ahead 1] expected
+ git for-each-ref --format=%(push:track) refs/heads actual
+ test_cmp expected actual
+'
+
cat expected EOF
[PATCH v3 05/14] remote.c: provide per-branch pushremote name
When remote.c loads its config, it records the
branch.*.pushremote for the current branch along with the
global remote.pushDefault value, and then binds them into a
single value: the default push for the current branch. We
then pass this value (which may be NULL) to remote_get_1
when looking up a remote for push.
This has a few downsides:
1. It's confusing. The early-binding of the current
value led to bugs like the one fixed by 98b406f
(remote: handle pushremote config in any order,
2014-02-24). And the fact that pushremotes fall back to
ordinary remotes is not explicit at all; it happens
because remote_get_1 cannot tell the difference between
we are not asking for the push remote and there is
no push remote configured.
2. It throws away intermediate data. After read_config()
finishes, we have no idea what the value of
remote.pushDefault was, because the string has been
overwritten by the current branch's
branch.*.pushremote.
3. It doesn't record other data. We don't note the
branch.*.pushremote value for anything but the current
branch.
Let's make this more like the fetch-remote config. We'll
record the pushremote for each branch, and then explicitly
compute the correct remote for the current branch at the
time of reading.
Signed-off-by: Jeff King p...@peff.net
---
remote.c | 40 ++--
remote.h | 2 ++
2 files changed, 24 insertions(+), 18 deletions(-)
diff --git a/remote.c b/remote.c
index 0d2976b..a91d063 100644
--- a/remote.c
+++ b/remote.c
@@ -49,7 +49,6 @@ static int branches_alloc;
static int branches_nr;
static struct branch *current_branch;
-static const char *branch_pushremote_name;
static const char *pushremote_name;
static struct rewrites rewrites;
@@ -367,9 +366,7 @@ static int handle_config(const char *key, const char
*value, void *cb)
if (!strcmp(subkey, .remote)) {
return git_config_string(branch-remote_name, key,
value);
} else if (!strcmp(subkey, .pushremote)) {
- if (branch == current_branch)
- if (git_config_string(branch_pushremote_name,
key, value))
- return -1;
+ return git_config_string(branch-pushremote_name, key,
value);
} else if (!strcmp(subkey, .merge)) {
if (!value)
return config_error_nonbool(key);
@@ -510,10 +507,6 @@ static void read_config(void)
current_branch = make_branch(head_ref, 0);
}
git_config(handle_config, NULL);
- if (branch_pushremote_name) {
- free((char *)pushremote_name);
- pushremote_name = branch_pushremote_name;
- }
alias_all_urls();
}
@@ -704,20 +697,31 @@ const char *remote_for_branch(struct branch *branch, int
*explicit)
return origin;
}
-static struct remote *remote_get_1(const char *name, const char
*pushremote_name)
+const char *pushremote_for_branch(struct branch *branch, int *explicit)
+{
+ if (branch branch-pushremote_name) {
+ if (explicit)
+ *explicit = 1;
+ return branch-pushremote_name;
+ }
+ if (pushremote_name) {
+ if (explicit)
+ *explicit = 1;
+ return pushremote_name;
+ }
+ return remote_for_branch(branch, explicit);
+}
+
+static struct remote *remote_get_1(const char *name,
+ const char *(*get_default)(struct branch *,
int *))
{
struct remote *ret;
int name_given = 0;
if (name)
name_given = 1;
- else {
- if (pushremote_name) {
- name = pushremote_name;
- name_given = 1;
- } else
- name = remote_for_branch(current_branch, name_given);
- }
+ else
+ name = get_default(current_branch, name_given);
ret = make_remote(name, 0);
if (valid_remote_nick(name)) {
@@ -738,13 +742,13 @@ static struct remote *remote_get_1(const char *name,
const char *pushremote_name
struct remote *remote_get(const char *name)
{
read_config();
- return remote_get_1(name, NULL);
+ return remote_get_1(name, remote_for_branch);
}
struct remote *pushremote_get(const char *name)
{
read_config();
- return remote_get_1(name, pushremote_name);
+ return remote_get_1(name, pushremote_for_branch);
}
int remote_is_configured(const char *name)
diff --git a/remote.h b/remote.h
index 2a7e7a6..30a11da 100644
--- a/remote.h
+++ b/remote.h
@@ -203,6 +203,7 @@ struct branch {
const char *refname;
const char *remote_name;
+ const char *pushremote_name;
const char **merge_name;
struct refspec
[PATCH v3 13/14] for-each-ref: use skip_prefix instead of starts_with
This saves us having to maintain a magic number to skip past
the matched prefix.
Signed-off-by: Jeff King p...@peff.net
---
builtin/for-each-ref.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 18d209b..345d8dd 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -659,10 +659,12 @@ static void populate_value(struct refinfo *ref)
else if (starts_with(name, symref))
refname = ref-symref ? ref-symref : ;
else if (starts_with(name, upstream)) {
+ const char *branch_name;
/* only local branches may have an upstream */
- if (!starts_with(ref-refname, refs/heads/))
+ if (!skip_prefix(ref-refname, refs/heads/,
+branch_name))
continue;
- branch = branch_get(ref-refname + 11);
+ branch = branch_get(branch_name);
refname = branch_get_upstream(branch, NULL);
if (!refname)
--
2.4.1.528.g00591e3
--
To unsubscribe from this list: send the line 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 04/14] remote.c: hoist branch.*.remote lookup out of remote_get_1
We'll want to use this logic as a fallback when looking up
the pushremote, so let's pull it out into its own function.
We don't technically need to make this available outside of
remote.c, but doing so will provide a consistent API with
pushremote_for_branch, which we will add later.
Signed-off-by: Jeff King p...@peff.net
---
remote.c | 21 ++---
remote.h | 1 +
2 files changed, 15 insertions(+), 7 deletions(-)
diff --git a/remote.c b/remote.c
index c298a43..0d2976b 100644
--- a/remote.c
+++ b/remote.c
@@ -692,6 +692,18 @@ static int valid_remote_nick(const char *name)
return !strchr(name, '/'); /* no slash */
}
+const char *remote_for_branch(struct branch *branch, int *explicit)
+{
+ if (branch branch-remote_name) {
+ if (explicit)
+ *explicit = 1;
+ return branch-remote_name;
+ }
+ if (explicit)
+ *explicit = 0;
+ return origin;
+}
+
static struct remote *remote_get_1(const char *name, const char
*pushremote_name)
{
struct remote *ret;
@@ -703,13 +715,8 @@ static struct remote *remote_get_1(const char *name, const
char *pushremote_name
if (pushremote_name) {
name = pushremote_name;
name_given = 1;
- } else {
- if (current_branch current_branch-remote_name) {
- name = current_branch-remote_name;
- name_given = 1;
- } else
- name = origin;
- }
+ } else
+ name = remote_for_branch(current_branch, name_given);
}
ret = make_remote(name, 0);
diff --git a/remote.h b/remote.h
index 4bb6672..2a7e7a6 100644
--- a/remote.h
+++ b/remote.h
@@ -211,6 +211,7 @@ struct branch {
};
struct branch *branch_get(const char *name);
+const char *remote_for_branch(struct branch *branch, int *explicit);
int branch_has_merge_config(struct branch *branch);
int branch_merge_matches(struct branch *, int n, const char *);
--
2.4.1.528.g00591e3
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 09/14] remote.c: add branch_get_push
In a triangular workflow, the place you pull from and the
place you push to may be different. As we have
branch_get_upstream for the former, this patch adds
branch_get_push for the latter (and as the former implements
@{upstream}, so will this implement @{push} in a future
patch).
Note that the memory-handling for the return value bears
some explanation. Some code paths require allocating a new
string, and some let us return an existing string. We should
provide a consistent interface to the caller, so it knows
whether to free the result or not.
We could do so by xstrdup-ing any existing strings, and
having the caller always free. But that makes us
inconsistent with branch_get_upstream, so we would prefer to
simply take ownership of the resulting string. We do so by
storing it inside the struct branch, just as we do with
the upstream refname (in that case we compute it when the
branch is created, but there's no reason not to just fill
it in lazily in this case).
Signed-off-by: Jeff King p...@peff.net
---
remote.c | 85
remote.h | 10
2 files changed, 95 insertions(+)
diff --git a/remote.c b/remote.c
index 1b7051a..be45a39 100644
--- a/remote.c
+++ b/remote.c
@@ -1737,6 +1737,91 @@ const char *branch_get_upstream(struct branch *branch,
struct strbuf *err)
return branch-merge[0]-dst;
}
+static const char *tracking_for_push_dest(struct remote *remote,
+ const char *refname,
+ struct strbuf *err)
+{
+ char *ret;
+
+ ret = apply_refspecs(remote-fetch, remote-fetch_refspec_nr, refname);
+ if (!ret)
+ return error_buf(err,
+_(push destination '%s' on remote '%s' has no
local tracking branch),
+refname, remote-name);
+ return ret;
+}
+
+static const char *branch_get_push_1(struct branch *branch, struct strbuf *err)
+{
+ struct remote *remote;
+
+ if (!branch)
+ return error_buf(err, _(HEAD does not point to a branch));
+
+ remote = remote_get(pushremote_for_branch(branch, NULL));
+ if (!remote)
+ return error_buf(err,
+_(branch '%s' has no remote for pushing),
+branch-name);
+
+ if (remote-push_refspec_nr) {
+ char *dst;
+ const char *ret;
+
+ dst = apply_refspecs(remote-push, remote-push_refspec_nr,
+branch-refname);
+ if (!dst)
+ return error_buf(err,
+_(push refspecs for '%s' do not
include '%s'),
+remote-name, branch-name);
+
+ ret = tracking_for_push_dest(remote, dst, err);
+ free(dst);
+ return ret;
+ }
+
+ if (remote-mirror)
+ return tracking_for_push_dest(remote, branch-refname, err);
+
+ switch (push_default) {
+ case PUSH_DEFAULT_NOTHING:
+ return error_buf(err, _(push has no destination (push.default
is 'nothing')));
+
+ case PUSH_DEFAULT_MATCHING:
+ case PUSH_DEFAULT_CURRENT:
+ return tracking_for_push_dest(remote, branch-refname, err);
+
+ case PUSH_DEFAULT_UPSTREAM:
+ return branch_get_upstream(branch, err);
+
+ case PUSH_DEFAULT_UNSPECIFIED:
+ case PUSH_DEFAULT_SIMPLE:
+ {
+ const char *up, *cur;
+
+ up = branch_get_upstream(branch, err);
+ if (!up)
+ return NULL;
+ cur = tracking_for_push_dest(remote, branch-refname,
err);
+ if (!cur)
+ return NULL;
+ if (strcmp(cur, up))
+ return error_buf(err,
+_(cannot resolve 'simple'
push to a single destination));
+ return cur;
+ }
+ }
+
+ die(BUG: unhandled push situation);
+}
+
+const char *branch_get_push(struct branch *branch, struct strbuf *err)
+{
+ if (!branch-push_tracking_ref)
+ branch-push_tracking_ref = branch_get_push_1(branch, err);
+ return branch-push_tracking_ref;
+}
+
static int ignore_symref_update(const char *refname)
{
unsigned char sha1[20];
diff --git a/remote.h b/remote.h
index 03ca005..3326f2b 100644
--- a/remote.h
+++ b/remote.h
@@ -209,6 +209,8 @@ struct branch {
struct refspec **merge;
int merge_nr;
int merge_alloc;
+
+ const char *push_tracking_ref;
};
struct branch *branch_get(const char *name);
@@ -229,6 +231,14 @@ int branch_merge_matches(struct branch *, int n, const
char *);
*/
const char
[PATCH v3 10/14] sha1_name: refactor upstream_mark
We will be adding new mark types in the future, so separate
the suffix data from the logic.
Signed-off-by: Jeff King p...@peff.net
---
sha1_name.c | 12 +---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/sha1_name.c b/sha1_name.c
index 461157a..1005f45 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -415,12 +415,12 @@ static int ambiguous_path(const char *path, int len)
return slash;
}
-static inline int upstream_mark(const char *string, int len)
+static inline int at_mark(const char *string, int len,
+ const char **suffix, int nr)
{
- const char *suffix[] = { @{upstream}, @{u} };
int i;
- for (i = 0; i ARRAY_SIZE(suffix); i++) {
+ for (i = 0; i nr; i++) {
int suffix_len = strlen(suffix[i]);
if (suffix_len = len
!memcmp(string, suffix[i], suffix_len))
@@ -429,6 +429,12 @@ static inline int upstream_mark(const char *string, int
len)
return 0;
}
+static inline int upstream_mark(const char *string, int len)
+{
+ const char *suffix[] = { @{upstream}, @{u} };
+ return at_mark(string, len, suffix, ARRAY_SIZE(suffix));
+}
+
static int get_sha1_1(const char *name, int len, unsigned char *sha1, unsigned
lookup_flags);
static int interpret_nth_prior_checkout(const char *name, int namelen, struct
strbuf *buf);
--
2.4.1.528.g00591e3
--
To unsubscribe from this list: send the line 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 11/14] sha1_name: refactor interpret_upstream_mark
Now that most of the logic for our local get_upstream_branch
has been pushed into the generic branch_get_upstream, we can
fold the remainder into interpret_upstream_mark.
Furthermore, what remains is generic to any branch-related
@{foo} we might add in the future, and there's enough
boilerplate that we'd like to reuse it. Let's parameterize
the two operations (parsing the mark and computing its
value) so that we can reuse this for @{push} in the near
future.
Signed-off-by: Jeff King p...@peff.net
---
sha1_name.c | 44 +++-
1 file changed, 23 insertions(+), 21 deletions(-)
diff --git a/sha1_name.c b/sha1_name.c
index 1005f45..506e0c9 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -1061,35 +1061,36 @@ static void set_shortened_ref(struct strbuf *buf, const
char *ref)
free(s);
}
-static const char *get_upstream_branch(const char *branch_buf, int len)
-{
- char *branch = xstrndup(branch_buf, len);
- struct branch *upstream = branch_get(*branch ? branch : NULL);
- struct strbuf err = STRBUF_INIT;
- const char *ret;
-
- free(branch);
-
- ret = branch_get_upstream(upstream, err);
- if (!ret)
- die(%s, err.buf);
-
- return ret;
-}
-
-static int interpret_upstream_mark(const char *name, int namelen,
- int at, struct strbuf *buf)
+static int interpret_branch_mark(const char *name, int namelen,
+int at, struct strbuf *buf,
+int (*get_mark)(const char *, int),
+const char *(*get_data)(struct branch *,
+struct strbuf *))
{
int len;
+ struct branch *branch;
+ struct strbuf err = STRBUF_INIT;
+ const char *value;
- len = upstream_mark(name + at, namelen - at);
+ len = get_mark(name + at, namelen - at);
if (!len)
return -1;
if (memchr(name, ':', at))
return -1;
- set_shortened_ref(buf, get_upstream_branch(name, at));
+ if (at) {
+ char *name_str = xmemdupz(name, at);
+ branch = branch_get(name_str);
+ free(name_str);
+ } else
+ branch = branch_get(NULL);
+
+ value = get_data(branch, err);
+ if (!value)
+ die(%s, err.buf);
+
+ set_shortened_ref(buf, value);
return len + at;
}
@@ -1140,7 +1141,8 @@ int interpret_branch_name(const char *name, int namelen,
struct strbuf *buf)
if (len 0)
return reinterpret(name, namelen, len, buf);
- len = interpret_upstream_mark(name, namelen, at - name, buf);
+ len = interpret_branch_mark(name, namelen, at - name, buf,
+ upstream_mark, branch_get_upstream);
if (len 0)
return len;
}
--
2.4.1.528.g00591e3
--
To unsubscribe from this list: send the line 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 02/14] remote.c: refactor setup of branch-merge list
When we call branch_get() to lookup or create a struct
branch, we make sure the merge field is filled in so that
callers can access it. But the conditions under which we do
so are a little confusing, and can lead to two funny
situations:
1. If there's no branch.*.remote config, we cannot provide
branch-merge (because it is really just an application
of branch.*.merge to our remote's refspecs). But
branch-merge_nr may be non-zero, leading callers to be
believe they can access branch-merge (e.g., in
branch_merge_matches and elsewhere).
It doesn't look like this can cause a segfault in
practice, as most code paths dealing with merge config
will bail early if there is no remote defined. But it's
a bit of a dangerous construct.
We can fix this by setting merge_nr to 0 explicitly
when we realize that we have no merge config. Note that
merge_nr also counts the merge_name fields (which we
_do_ have; that's how merge_nr got incremented), so we
will lose access to them, in the sense that we forget
how many we had. But no callers actually care; we use
merge_name only while iteratively reading the config,
and then convert it to the final merge form the first
time somebody calls branch_get().
2. We set up the merge field every time branch_get is
called, even if it has already been done. This leaks
memory.
It's not a big deal in practice, since most code paths
will access only one branch, or perhaps each branch
only one time. But if you want to be pathological, you
can leak arbitrary memory with:
yes @{upstream} | head -1000 | git rev-list --stdin
We can fix this by skipping setup when branch-merge is
already non-NULL.
In addition to those two fixes, this patch pushes the do we
need to setup merge? logic down into set_merge, where it is
a bit easier to follow.
Signed-off-by: Jeff King p...@peff.net
---
remote.c | 19 +++
1 file changed, 15 insertions(+), 4 deletions(-)
diff --git a/remote.c b/remote.c
index bec8b31..ac17e66 100644
--- a/remote.c
+++ b/remote.c
@@ -1636,6 +1636,19 @@ static void set_merge(struct branch *ret)
unsigned char sha1[20];
int i;
+ if (!ret)
+ return; /* no branch */
+ if (ret-merge)
+ return; /* already run */
+ if (!ret-remote_name || !ret-merge_nr) {
+ /*
+* no merge config; let's make sure we don't confuse callers
+* with a non-zero merge_nr but a NULL merge
+*/
+ ret-merge_nr = 0;
+ return;
+ }
+
ret-merge = xcalloc(ret-merge_nr, sizeof(*ret-merge));
for (i = 0; i ret-merge_nr; i++) {
ret-merge[i] = xcalloc(1, sizeof(**ret-merge));
@@ -1660,11 +1673,9 @@ struct branch *branch_get(const char *name)
ret = current_branch;
else
ret = make_branch(name, 0);
- if (ret ret-remote_name) {
+ if (ret ret-remote_name)
ret-remote = remote_get(ret-remote_name);
- if (ret-merge_nr)
- set_merge(ret);
- }
+ set_merge(ret);
return ret;
}
--
2.4.1.528.g00591e3
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 03/14] remote.c: drop remote pointer from struct branch
When we create each branch struct, we fill in the
remote_name field from the config, and then fill in the
actual remote field (with a struct remote) based on that
name. However, it turns out that nobody really cares about
the latter field. The only two sites that access it at all
are:
1. git-merge, which uses it to notice when the branch does
not have a remote defined. But we can easily replace this
with looking at remote_name instead.
2. remote.c itself, when setting up the @{upstream} merge
config. But we don't need to save the remote in the
struct branch for that; we can just look it up for
the duration of the operation.
So there is no need to have both fields; they are redundant
with each other (the struct remote contains the name, or you
can look up the struct from the name). It would be nice to
simplify this, especially as we are going to add matching
pushremote config in a future patch (and it would be nice to
keep them consistent).
So which one do we keep and which one do we get rid of?
If we had a lot of callers accessing the struct, it would be
more efficient to keep it (since you have to do a lookup to
go from the name to the struct, but not vice versa). But we
don't have a lot of callers; we have exactly one, so
efficiency doesn't matter. We can decide this based on
simplicity and readability.
And the meaning of the struct value is somewhat unclear. Is
it always the remote matching remote_name? If remote_name is
NULL (i.e., no per-branch config), does the struct fall back
to the origin remote, or is it also NULL? These questions
will get even more tricky with pushremotes, whose fallback
behavior is more complicated. So let's just store the name,
which pretty clearly represents the branch.*.remote config.
Any lookup or fallback behavior can then be implemented in
helper functions.
Signed-off-by: Jeff King p...@peff.net
---
Documentation/technical/api-remote.txt | 4
builtin/merge.c| 2 +-
remote.c | 7 ---
remote.h | 1 -
4 files changed, 5 insertions(+), 9 deletions(-)
diff --git a/Documentation/technical/api-remote.txt
b/Documentation/technical/api-remote.txt
index 5d245aa..2cfdd22 100644
--- a/Documentation/technical/api-remote.txt
+++ b/Documentation/technical/api-remote.txt
@@ -97,10 +97,6 @@ It contains:
The name of the remote listed in the configuration.
-`remote`::
-
- The struct remote for that remote.
-
`merge_name`::
An array of the merge lines in the configuration.
diff --git a/builtin/merge.c b/builtin/merge.c
index f89f60e..85c54dc 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -933,7 +933,7 @@ static int setup_with_upstream(const char ***argv)
if (!branch)
die(_(No current branch.));
- if (!branch-remote)
+ if (!branch-remote_name)
die(_(No remote for the current branch.));
if (!branch-merge_nr)
die(_(No default upstream defined for the current branch.));
diff --git a/remote.c b/remote.c
index ac17e66..c298a43 100644
--- a/remote.c
+++ b/remote.c
@@ -1632,6 +1632,7 @@ void set_ref_status_for_push(struct ref *remote_refs, int
send_mirror,
static void set_merge(struct branch *ret)
{
+ struct remote *remote;
char *ref;
unsigned char sha1[20];
int i;
@@ -1649,11 +1650,13 @@ static void set_merge(struct branch *ret)
return;
}
+ remote = remote_get(ret-remote_name);
+
ret-merge = xcalloc(ret-merge_nr, sizeof(*ret-merge));
for (i = 0; i ret-merge_nr; i++) {
ret-merge[i] = xcalloc(1, sizeof(**ret-merge));
ret-merge[i]-src = xstrdup(ret-merge_name[i]);
- if (!remote_find_tracking(ret-remote, ret-merge[i]) ||
+ if (!remote_find_tracking(remote, ret-merge[i]) ||
strcmp(ret-remote_name, .))
continue;
if (dwim_ref(ret-merge_name[i], strlen(ret-merge_name[i]),
@@ -1673,8 +1676,6 @@ struct branch *branch_get(const char *name)
ret = current_branch;
else
ret = make_branch(name, 0);
- if (ret ret-remote_name)
- ret-remote = remote_get(ret-remote_name);
set_merge(ret);
return ret;
}
diff --git a/remote.h b/remote.h
index 02d66ce..4bb6672 100644
--- a/remote.h
+++ b/remote.h
@@ -203,7 +203,6 @@ struct branch {
const char *refname;
const char *remote_name;
- struct remote *remote;
const char **merge_name;
struct refspec **merge;
--
2.4.1.528.g00591e3
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 01/14] remote.c: drop default_remote_name variable
When we read the remote config from disk, we update a
default_remote_name variable if we see branch.*.remote
config for the current branch. This isn't wrong, or even all
that complicated, but it is a bit simpler (because it
reduces our overall state) to just lazily compute the
default when we need it.
The ulterior motive here is that the push config uses a
similar structure, and _is_ much more complicated as a
result. That will be simplified in a future patch, and it's
more readable if the logic for remotes and push-remotes
matches.
Note that we also used default_remote_name as a signal that
the remote config has been loaded; after this patch, we now
use an explicit flag.
Signed-off-by: Jeff King p...@peff.net
---
remote.c | 23 +++
1 file changed, 11 insertions(+), 12 deletions(-)
diff --git a/remote.c b/remote.c
index 68901b0..bec8b31 100644
--- a/remote.c
+++ b/remote.c
@@ -49,10 +49,8 @@ static int branches_alloc;
static int branches_nr;
static struct branch *current_branch;
-static const char *default_remote_name;
static const char *branch_pushremote_name;
static const char *pushremote_name;
-static int explicit_default_remote_name;
static struct rewrites rewrites;
static struct rewrites rewrites_push;
@@ -367,12 +365,7 @@ static int handle_config(const char *key, const char
*value, void *cb)
return 0;
branch = make_branch(name, subkey - name);
if (!strcmp(subkey, .remote)) {
- if (git_config_string(branch-remote_name, key, value))
- return -1;
- if (branch == current_branch) {
- default_remote_name = branch-remote_name;
- explicit_default_remote_name = 1;
- }
+ return git_config_string(branch-remote_name, key,
value);
} else if (!strcmp(subkey, .pushremote)) {
if (branch == current_branch)
if (git_config_string(branch_pushremote_name,
key, value))
@@ -501,12 +494,15 @@ static void alias_all_urls(void)
static void read_config(void)
{
+ static int loaded;
unsigned char sha1[20];
const char *head_ref;
int flag;
- if (default_remote_name) /* did this already */
+
+ if (loaded)
return;
- default_remote_name = origin;
+ loaded = 1;
+
current_branch = NULL;
head_ref = resolve_ref_unsafe(HEAD, 0, sha1, flag);
if (head_ref (flag REF_ISSYMREF)
@@ -708,8 +704,11 @@ static struct remote *remote_get_1(const char *name, const
char *pushremote_name
name = pushremote_name;
name_given = 1;
} else {
- name = default_remote_name;
- name_given = explicit_default_remote_name;
+ if (current_branch current_branch-remote_name) {
+ name = current_branch-remote_name;
+ name_given = 1;
+ } else
+ name = origin;
}
}
--
2.4.1.528.g00591e3
--
To unsubscribe from this list: send the line 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/RFC 0/3] --seed as an alias for --dissociate --reference
In a thread a few months ago[1], we discussed the idea that the --dissociate --reference=foo interface was somewhat awkward for somebody who just wants to optimize their clone. This is mostly due to the historical development of the features. The logical interface for somebody who just wants a faster clone is something like git clone --optimize-my-clone-from=foo.git git://example.com/bar.git But we got stuck in that thread on coming up with a decent name for the option. Having just read through it, I think a succinct name for the idea is seed. That is, we seed the clone with objects from another repository. That thread also brought up the idea that we do not necessarily need to seed from a local repository; we could do something like: 1. Fetch from the seed repo into refs/seed/* 2. Fetch from the real clone source; the fetch is optimized by the presence of refs/seed/*. 3. Delete refs/seed/*. Optionally repack to drop any objects needed only by the seed refs. This is awkward with the --reference interface, because its implementation is publicly tied to the concept of alternates. Whereas --seed is about the end result you want; we can implement it using alternates or with a clone, depending on where the repo is located. There are a few open issues with this series: 1. Assuming that seed is a reasonable verb for this concept, is --seed=repo OK for the option? Would --seed-from=repo be better? (Also, the response bleh, seed is a terrible name is fine, too, but only if accompanied by your own suggestion :) ). 2. My main goal here is making the concept easier to explain to users. The documentation in the third patch explains --seed as an alias for the other options, which probably isn't helping much. It might make sense to have a patch 4/3 that explains --seed first, and then explains --reference as like --seed, but keep the relationship after the clone. Or maybe they should just get their own descriptions entirely. 3. We can't dissociate from a specific alternate, so using --seed implies that all --reference options get dissociated. In this series, I issue a warning in that case. But that would be easily solved if --seed used the fetch strategy described above, even for local clones (which would probably still be quite fast if we took clone's usual hard-link shortcut instead of actually fetching from a local clone). I don't have particular plans to implement generic --seed from remotes anytime soon. I think this takes us a step in the right direction interface-wise, and it does introduce a succinct concept and option. But the abstraction does leak (e.g., in that it implies --dissociate). So one response might be yes, this is a good building block, and later we can extend --seed; how it works is an implementation detail. But equally valid would be eh, I like the name and the concept, but this implementation is too hacky; let's wait for somebody to implement it for real. Hence the RFC label. The patches are: [1/3]: clone: use OPT_STRING_LIST for --reference [2/3]: clone: reorder --dissociate and --reference options [3/3]: clone: add `--seed` shorthand The third one is the interesting one, and the first two are nearby cleanups. Whether we pursue the third one or not, I think the first two are worth taking by themselves. -Peff [1] http://thread.gmane.org/gmane.comp.version-control.git/264178/focus=264234 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/3] clone: use OPT_STRING_LIST for --reference
Not only does this save us having to implement a custom
callback, but it handles --no-reference in the usual way
(to clear the list).
The generic callback does copy the string, which we don't
technically need, but that should not hurt anything.
Signed-off-by: Jeff King p...@peff.net
---
builtin/clone.c | 13 ++---
1 file changed, 2 insertions(+), 11 deletions(-)
diff --git a/builtin/clone.c b/builtin/clone.c
index 166a645..1426ef5 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -51,15 +51,6 @@ static struct string_list option_config;
static struct string_list option_reference;
static int option_dissociate;
-static int opt_parse_reference(const struct option *opt, const char *arg, int
unset)
-{
- struct string_list *option_reference = opt-value;
- if (!arg)
- return -1;
- string_list_append(option_reference, arg);
- return 0;
-}
-
static struct option builtin_clone_options[] = {
OPT__VERBOSITY(option_verbosity),
OPT_BOOL(0, progress, option_progress,
@@ -83,8 +74,8 @@ static struct option builtin_clone_options[] = {
N_(initialize submodules in the clone)),
OPT_STRING(0, template, option_template, N_(template-directory),
N_(directory from which templates will be used)),
- OPT_CALLBACK(0 , reference, option_reference, N_(repo),
-N_(reference repository), opt_parse_reference),
+ OPT_STRING_LIST(0, reference, option_reference, N_(repo),
+ N_(reference repository)),
OPT_STRING('o', origin, option_origin, N_(name),
N_(use name instead of 'origin' to track upstream)),
OPT_STRING('b', branch, option_branch, N_(branch),
--
2.4.1.528.g00591e3
--
To unsubscribe from this list: send the line 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 0/14] implement @{push} shorthand
This is a re-roll of the series at:
http://thread.gmane.org/gmane.comp.version-control.git/268185
The only changes here are the addition of patches 2 and 6, which are
both cleanups that help make the other patches more readable/sensible.
They are the same as what was posted during review of the thread linked
above. So there's nothing new here, but of course fresh reviews are
welcome.
[01/14]: remote.c: drop default_remote_name variable
[02/14]: remote.c: refactor setup of branch-merge list
[03/14]: remote.c: drop remote pointer from struct branch
[04/14]: remote.c: hoist branch.*.remote lookup out of remote_get_1
[05/14]: remote.c: provide per-branch pushremote name
[06/14]: remote.c: hoist read_config into remote_get_1
[07/14]: remote.c: introduce branch_get_upstream helper
[08/14]: remote.c: report specific errors from branch_get_upstream
[09/14]: remote.c: add branch_get_push
[10/14]: sha1_name: refactor upstream_mark
[11/14]: sha1_name: refactor interpret_upstream_mark
[12/14]: sha1_name: implement @{push} shorthand
[13/14]: for-each-ref: use skip_prefix instead of starts_with
[14/14]: for-each-ref: accept %(push) format
-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/RFC 0/3] --seed as an alias for --dissociate --reference
On Wed, May 20, 2015 at 10:01:49PM -0700, Junio C Hamano wrote: 1. Assuming that seed is a reasonable verb for this concept, is --seed=repo OK for the option? Would --seed-from=repo be better? (Also, the response bleh, seed is a terrible name is fine, too, but only if accompanied by your own suggestion :) ). The seed may not even have to be a repository. A bundle file hosted on CDN that is reachable via (resumable) wget would be another good way to prime the well, and it would fit with the above framework nicely. Grab it, fetch from it into a temporary hierarchy and then run fetch --prune against the repository you originally wanted to clone from. Yeah, I was just looking over the list archives for the past few months, for things I had marked as to read and think about later[1]. That's how I recalled our prior discussion on --dissociate. Anyway, I happened upon the prime the clone from a bundle concept being discussed again recently, and had the same thought. We already treat local bundles as a possible source for fetching/cloning. Once upon a time I had some patches that would let you clone straight from a bundle over http (it just spooled to disk, which is not the _most_ efficient way to do it, but trying to massage the bundle straight into a packfile[2] ends up every complex very quickly). I should resurrect those patches. -Peff [1] My think about later mailbox has ~5000 messages in it, some of which are from 2010. I think I may need to just declare bankruptcy. [2] There's that word again. -- To unsubscribe from this list: send the line 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/3] git-verify-pack.txt: fix inconsistent spelling of packfile
On Wed, May 20, 2015 at 03:37:23PM -0700, Junio C Hamano wrote: In any case, even though I merged these three to 'next', I think we need to either revert 3/3 or do s/pack-file/packfile/ throughout the pack-protocol documentation. The original has something like this: The pack-file MUST NOT be sent if the only command used is 'delete'. A pack-file MUST be sent if either create or update command is used, even if the server already has all the necessary objects. In this case the client MUST send an empty pack-file. The only time this is likely to happen is if the client is creating a new branch or a tag that points to an existing obj-id. and these are explicitly referring to what EBNF defines as pack-file. Changing them to packfile is simply wrong. Yeah, I agree they should agree with the EBNF. And my inclination is for packfile, as it is refering to the concept of the on-the-wire packfile data (there is no file ending in .pack in this context). Which I guess argues for a further patch. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/3] clone: add `--seed` shorthand
The safe way to use `--reference` is to add in the recent
`--dissociate` option, which optimizes the initial clone,
but does not create any obligation to avoid pruning or
deleting the reference repository. However, it can be rather
tricky to explain why two options are necessary, and why
using `--reference` alone is unsafe.
This patch introduces a single option, `--seed`, which does
the right thing; we can steer users towards it rather than
explaining the complexities. It also provides a natural
interface if we later want to allow seeding from non-local
repositories.
Note that git-repack cannot selectively dissociate from
particular alternates. Therefore using `--reference` and
`--seed` together will dissociate from _all_ referenced
repositories. We issue a warning to the user in this case.
Signed-off-by: Jeff King p...@peff.net
---
Documentation/git-clone.txt | 3 +++
builtin/clone.c | 12 +++-
t/t5700-clone-reference.sh | 6 --
3 files changed, 18 insertions(+), 3 deletions(-)
diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt
index f1f2a3f..ffeb03b 100644
--- a/Documentation/git-clone.txt
+++ b/Documentation/git-clone.txt
@@ -107,6 +107,9 @@ objects from the source repository into a pack in the
cloned repository.
transfer and stop borrowing from them after a clone is made
by making necessary local copies of borrowed objects.
+--seed repository::
+ A convenient shorthand for `--dissociate --reference=repository`.
+
--quiet::
-q::
Operate quietly. Progress is not reported to the standard
diff --git a/builtin/clone.c b/builtin/clone.c
index a0ec1a9..dd53bbd 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -49,6 +49,7 @@ static int option_verbosity;
static int option_progress = -1;
static struct string_list option_config;
static struct string_list option_reference;
+static struct string_list option_seed;
static int option_dissociate;
static struct option builtin_clone_options[] = {
@@ -78,6 +79,8 @@ static struct option builtin_clone_options[] = {
N_(reference repository)),
OPT_BOOL(0, dissociate, option_dissociate,
N_(use --reference only while cloning)),
+ OPT_STRING_LIST(0, seed, option_seed, N_(repo),
+ N_(reference and dissociate from repo)),
OPT_STRING('o', origin, option_origin, N_(name),
N_(use name instead of 'origin' to track upstream)),
OPT_STRING('b', branch, option_branch, N_(branch),
@@ -263,6 +266,7 @@ static int add_one_reference(struct string_list_item *item,
void *cb_data)
static void setup_reference(void)
{
for_each_string_list(option_reference, add_one_reference, NULL);
+ for_each_string_list(option_seed, add_one_reference, NULL);
}
static void copy_alternates(struct strbuf *src, struct strbuf *dst,
@@ -884,7 +888,13 @@ int cmd_clone(int argc, const char **argv, const char
*prefix)
git_config_set(key.buf, repo);
strbuf_reset(key);
- if (option_reference.nr)
+ if (option_seed.nr) {
+ if (option_reference.nr)
+ warning(_(--seed and --reference used together implies
--dissociate));
+ option_dissociate = 1;
+ }
+
+ if (option_reference.nr || option_seed.nr)
setup_reference();
else if (option_dissociate) {
warning(_(--dissociate given, but there is no --reference));
diff --git a/t/t5700-clone-reference.sh b/t/t5700-clone-reference.sh
index 3e783fc..80a794c 100755
--- a/t/t5700-clone-reference.sh
+++ b/t/t5700-clone-reference.sh
@@ -209,10 +209,12 @@ test_expect_success 'clone and dissociate from reference'
'
)
git clone --no-local --reference=P Q R
git clone --no-local --reference=P --dissociate Q S
- # removing the reference P would corrupt R but not S
+ git clone --no-local --seed=P Q T
+ # removing the reference P would corrupt R but not S or T
rm -fr P
test_must_fail git -C R fsck
- git -C S fsck
+ git -C S fsck
+ git -C T fsck
'
test_done
--
2.4.1.528.g00591e3
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/3] clone: reorder --dissociate and --reference options
These options are intimately related, so it makes sense to
list them nearby in the -h output (they are already
adjacent in the manpage).
Signed-off-by: Jeff King p...@peff.net
---
builtin/clone.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/builtin/clone.c b/builtin/clone.c
index 1426ef5..a0ec1a9 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -76,6 +76,8 @@ static struct option builtin_clone_options[] = {
N_(directory from which templates will be used)),
OPT_STRING_LIST(0, reference, option_reference, N_(repo),
N_(reference repository)),
+ OPT_BOOL(0, dissociate, option_dissociate,
+N_(use --reference only while cloning)),
OPT_STRING('o', origin, option_origin, N_(name),
N_(use name instead of 'origin' to track upstream)),
OPT_STRING('b', branch, option_branch, N_(branch),
@@ -86,8 +88,6 @@ static struct option builtin_clone_options[] = {
N_(create a shallow clone of that depth)),
OPT_BOOL(0, single-branch, option_single_branch,
N_(clone only one branch, HEAD or --branch)),
- OPT_BOOL(0, dissociate, option_dissociate,
-N_(use --reference only while cloning)),
OPT_STRING(0, separate-git-dir, real_git_dir, N_(gitdir),
N_(separate git dir from working tree)),
OPT_STRING_LIST('c', config, option_config, N_(key=value),
--
2.4.1.528.g00591e3
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/14] implement @{push} shorthand
On Thu, May 21, 2015 at 12:44:29AM -0400, Jeff King wrote: This is a re-roll of the series at: http://thread.gmane.org/gmane.comp.version-control.git/268185 The only changes here are the addition of patches 2 and 6, which are both cleanups that help make the other patches more readable/sensible. They are the same as what was posted during review of the thread linked above. So there's nothing new here, but of course fresh reviews are welcome. Actually, that's not quite true; I forgot to mention one change: [03/14]: remote.c: drop remote pointer from struct branch In this version, this one remembers to also update the API documentation. -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
