* [PATCH 1/4] doc: add articles (grammar)
From: Kristoffer Haugsbakk @ 2016-12-09 15:51 UTC (permalink / raw)
To: git; +Cc: Kristoffer Haugsbakk
In-Reply-To: <20161209155112.2112-1-kristoffer.haugsbakk@gmail.com>
Add definite and indefinite articles in three places where they were
missing.
- Use "the" in front of a directory name
- Use "the" in front of "style of cooperation"
- Use an indefinite article in front of "CVS background"
Signed-off-by: Kristoffer Haugsbakk <kristoffer.haugsbakk@gmail.com>
---
Documentation/gitcore-tutorial.txt | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/Documentation/gitcore-tutorial.txt b/Documentation/gitcore-tutorial.txt
index 4546fa0d7..6c434aff3 100644
--- a/Documentation/gitcore-tutorial.txt
+++ b/Documentation/gitcore-tutorial.txt
@@ -1368,7 +1368,7 @@ $ git repack
will do it for you. If you followed the tutorial examples, you
would have accumulated about 17 objects in `.git/objects/??/`
directories by now. 'git repack' tells you how many objects it
-packed, and stores the packed file in `.git/objects/pack`
+packed, and stores the packed file in the `.git/objects/pack`
directory.
[NOTE]
@@ -1543,9 +1543,9 @@ like this:
Working with Others, Shared Repository Style
--------------------------------------------
-If you are coming from CVS background, the style of cooperation
+If you are coming from a CVS background, the style of cooperation
suggested in the previous section may be new to you. You do not
-have to worry. Git supports "shared public repository" style of
+have to worry. Git supports the "shared public repository" style of
cooperation you are probably more familiar with as well.
See linkgit:gitcvs-migration[7] for the details.
--
2.11.0
^ permalink raw reply related
* [PATCH 0/4] doc: fixes to gitcore-tutorial.txt
From: Kristoffer Haugsbakk @ 2016-12-09 15:51 UTC (permalink / raw)
To: git; +Cc: Kristoffer Haugsbakk
This series of patches attempts to fix some minor mistakes in
gitcore-tutorial.txt that I found while reading it. They are all
concerned with grammar and things like accidentally omitted words.
I previously sent a single patch on 2016-11-04 ("[PATCH] doc: fill in
omitted word"). The patch "doc: omit needless "for"" is a follow-up to
that, taking into consideration the feedback from Junio C Hamano and
Jeff King.
Kristoffer Haugsbakk (4):
doc: add articles (grammar)
doc: add verb in front of command to run
doc: make the intent of sentence clearer
doc: omit needless "for"
Documentation/gitcore-tutorial.txt | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
--
2.11.0
^ permalink raw reply
* Re: [BUG] Colon in remote urls
From: Jeff King @ 2016-12-09 15:22 UTC (permalink / raw)
To: Klaus Ethgen; +Cc: git
In-Reply-To: <20161209140215.qlam6bexm5irpro2@ikki.ethgen.ch>
On Fri, Dec 09, 2016 at 03:02:15PM +0100, Klaus Ethgen wrote:
> I have some repositories where I have a colon in the (local) url for a
> remote. That was no problem until now but with 2.11.0, I see the
> following problem:
> ~> git push
> Counting objects: 11, done.
> Delta compression using up to 8 threads.
> Compressing objects: 100% (10/10), done.
> Writing objects: 100% (11/11), 1.26 KiB | 0 bytes/s, done.
> Total 11 (delta 7), reused 0 (delta 0)
> remote: error: object directory /home/xxx does not exist; check .git/objects/info/alternates.
> remote: error: object directory yyy.git/objects does not exist; check .git/objects/info/alternates.
> remote: fatal: unresolved deltas left after unpacking
Hrm. The problem v2.11's new object-quarantine system. The receiving
side of a push will write into a new temporary object directory, and
refer to the original with the GIT_ALTERNATE_OBJECT_DIRECTORIES
environment variable. But that variable splits its entries on ":", and
has no way of representing a path that includes a ":".
So I think the solution would probably be to introduce some kind of
quoting mechanism to that variable, so that it can pass through
arbitrary paths. Or alternatively use a separate variable, but that does
nothing to help other cases where somebody wants to use xxx:yyy.git as
an alternate.
(One other option is to just declare that the quarantine feature doesn't
work with colons in the pathname, but stop turning it on by default. I'm
not sure I like that, though).
Here's a rough idea of what the quoting solution could look like. It
should make your case work, but I'm not sure what we want to do about
backwards-compatibility, if anything.
---
sha1_file.c | 41 ++++++++++++++++++++++++++++++-----------
tmp-objdir.c | 18 ++++++++++++++++--
2 files changed, 46 insertions(+), 13 deletions(-)
diff --git a/sha1_file.c b/sha1_file.c
index 9c86d1924..a0b341b5a 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -329,13 +329,35 @@ static int link_alt_odb_entry(const char *entry, const char *relative_base,
return 0;
}
+const char *parse_alt_odb_entry(const char *string, int sep,
+ struct strbuf *out)
+{
+ const char *p;
+ int literal = 0;
+
+ strbuf_reset(out);
+
+ for (p = string; *p; p++) {
+ if (literal) {
+ strbuf_addch(out, *p);
+ literal = 0;
+ } else {
+ if (*p == '\\')
+ literal = 1;
+ else if (*p == sep)
+ return p + 1;
+ else
+ strbuf_addch(out, *p);
+ }
+ }
+ return p;
+}
+
static void link_alt_odb_entries(const char *alt, int len, int sep,
const char *relative_base, int depth)
{
- struct string_list entries = STRING_LIST_INIT_NODUP;
- char *alt_copy;
- int i;
struct strbuf objdirbuf = STRBUF_INIT;
+ struct strbuf entry = STRBUF_INIT;
if (depth > 5) {
error("%s: ignoring alternate object stores, nesting too deep.",
@@ -348,16 +370,13 @@ static void link_alt_odb_entries(const char *alt, int len, int sep,
die("unable to normalize object directory: %s",
objdirbuf.buf);
- alt_copy = xmemdupz(alt, len);
- string_list_split_in_place(&entries, alt_copy, sep, -1);
- for (i = 0; i < entries.nr; i++) {
- const char *entry = entries.items[i].string;
- if (entry[0] == '\0' || entry[0] == '#')
+ while (*alt) {
+ alt = parse_alt_odb_entry(alt, sep, &entry);
+ if (!entry.len || entry.buf[0] == '#')
continue;
- link_alt_odb_entry(entry, relative_base, depth, objdirbuf.buf);
+ link_alt_odb_entry(entry.buf, relative_base, depth, objdirbuf.buf);
}
- string_list_clear(&entries, 0);
- free(alt_copy);
+ strbuf_release(&entry);
strbuf_release(&objdirbuf);
}
diff --git a/tmp-objdir.c b/tmp-objdir.c
index 64435f23a..900e15af1 100644
--- a/tmp-objdir.c
+++ b/tmp-objdir.c
@@ -70,6 +70,17 @@ static void remove_tmp_objdir_on_signal(int signo)
raise(signo);
}
+static char *backslash_quote(const char *s, int delim)
+{
+ struct strbuf buf = STRBUF_INIT;
+ while (*s) {
+ if (*s == '\\' || *s == delim)
+ strbuf_addch(&buf, '\\');
+ strbuf_addch(&buf, *s++);
+ }
+ return strbuf_detach(&buf, NULL);
+}
+
/*
* These env_* functions are for setting up the child environment; the
* "replace" variant overrides the value of any existing variable with that
@@ -79,12 +90,15 @@ static void remove_tmp_objdir_on_signal(int signo)
*/
static void env_append(struct argv_array *env, const char *key, const char *val)
{
+ char *quoted = backslash_quote(val, PATH_SEP);
const char *old = getenv(key);
if (!old)
- argv_array_pushf(env, "%s=%s", key, val);
+ argv_array_pushf(env, "%s=%s", key, quoted);
else
- argv_array_pushf(env, "%s=%s%c%s", key, old, PATH_SEP, val);
+ argv_array_pushf(env, "%s=%s%c%s", key, old, PATH_SEP, quoted);
+
+ free(quoted);
}
static void env_replace(struct argv_array *env, const char *key, const char *val)
--
2.11.0.201.g51bd297
^ permalink raw reply related
* [BUG] Colon in remote urls
From: Klaus Ethgen @ 2016-12-09 14:02 UTC (permalink / raw)
To: git
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512
Hello,
I have some repositories where I have a colon in the (local) url for a
remote. That was no problem until now but with 2.11.0, I see the
following problem:
~> git push
Counting objects: 11, done.
Delta compression using up to 8 threads.
Compressing objects: 100% (10/10), done.
Writing objects: 100% (11/11), 1.26 KiB | 0 bytes/s, done.
Total 11 (delta 7), reused 0 (delta 0)
remote: error: object directory /home/xxx does not exist; check .git/objects/info/alternates.
remote: error: object directory yyy.git/objects does not exist; check .git/objects/info/alternates.
remote: fatal: unresolved deltas left after unpacking
error: unpack failed: unpack-objects abnormal exit
To /home/xxx:yyy.git
! [remote rejected] master -> master (unpacker error)
error: failed to push some refs to '/home/xxx:yyy.git'
Prepending the path with "file://" does not help.
It seems that the new git version splits the url at ":" which ends in
two incorrect paths.
Pull seems tho work well currently.
Regards
Klaus
Ps. I am not subscribet to the mailing list, so please keep me in Cc.
- --
Klaus Ethgen http://www.ethgen.ch/
pub 4096R/4E20AF1C 2011-05-16 Klaus Ethgen <Klaus@Ethgen.ch>
Fingerprint: 85D4 CA42 952C 949B 1753 62B3 79D0 B06F 4E20 AF1C
-----BEGIN PGP SIGNATURE-----
Comment: Charset: ISO-8859-1
iQGzBAEBCgAdFiEEMWF28vh4/UMJJLQEpnwKsYAZ9qwFAlhKuV4ACgkQpnwKsYAZ
9qz81Qv6AsgZHlaEHEybERAvGikjZgUvjyC7dzQ2zCmc8iv0eb8kGLiBtVtInsWr
eo/CiMSdX2emoCD5LQq/sxcRIgIoWpF8m2NEXiBMl94d6YLOpvWV1yC1kQ8qK6bt
Pyeo9/LofAnpLcQRn1am8tFwrcCMLZxSM7cxMxjtP6i+RU0MHc/rS1HqhdzptpsH
jvB0x41X7LNoeRLqG5n8lVlyHP1PiGyP0Dl4Aa9rFBHn+hydiJEmLEwdn9w4wgIz
vo2DZmqGm0r4vTaTP1gQRPxoW6fsBZ1uUWKRMjd947R1eaELpyROj4SFt0bWNqwW
cx9izYUuTg+xSe1KTaSgPlmZn1817DlG2yJ/YduKH8v61ZJ2r6B1awO2tz32g7cA
QdjsnzAyz6eVLrrsJ5OrJPRF2Fl7gM22jo9gs3BJrHeJdC9dU6kVIAR3eryoFvUg
fG/Vl2zvfbMRQAaUDGMyxNk5TGVFB6ANw0KS/NczRvmbPA9kukBz012+8ZY8MHzD
aEvmk/yz
=tDwn
-----END PGP SIGNATURE-----
^ permalink raw reply
* Re: [PATCH v2 3/4] real_path: create real_pathdup
From: Johannes Sixt @ 2016-12-09 14:35 UTC (permalink / raw)
To: Brandon Williams
Cc: git, sbeller, peff, jacob.keller, gitster, ramsay, tboegi,
pclouds
In-Reply-To: <1481241494-6861-4-git-send-email-bmwill@google.com>
Am 09.12.2016 um 00:58 schrieb Brandon Williams:
> +char *real_pathdup(const char *path)
> +{
> + struct strbuf realpath = STRBUF_INIT;
> + char *retval = NULL;
> +
> + if(strbuf_realpath(&realpath, path, 0))
Style nit: blank after if is missing.
-- Hannes
^ permalink raw reply
* Re: [PATCH v2 1/4] real_path: resolve symlinks by hand
From: Johannes Sixt @ 2016-12-09 14:33 UTC (permalink / raw)
To: Brandon Williams
Cc: git, sbeller, peff, jacob.keller, gitster, ramsay, tboegi,
pclouds
In-Reply-To: <1481241494-6861-2-git-send-email-bmwill@google.com>
Am 09.12.2016 um 00:58 schrieb Brandon Williams:
> The current implementation of real_path uses chdir() in order to resolve
> symlinks. Unfortunately this isn't thread-safe as chdir() affects a
> process as a whole and not just an individual thread. Instead perform
> the symlink resolution by hand so that the calls to chdir() can be
> removed, making real_path one step closer to being reentrant.
>
> Signed-off-by: Brandon Williams <bmwill@google.com>
> ---
> abspath.c | 183 +++++++++++++++++++++++++++++++++++++++++---------------------
> 1 file changed, 122 insertions(+), 61 deletions(-)
>
> diff --git a/abspath.c b/abspath.c
> index 2825de8..92f2a29 100644
> --- a/abspath.c
> +++ b/abspath.c
> @@ -11,8 +11,38 @@ int is_directory(const char *path)
> return (!stat(path, &st) && S_ISDIR(st.st_mode));
> }
>
> +/* removes the last path component from 'path' except if 'path' is root */
> +static void strip_last_component(struct strbuf *path)
> +{
> + if (path->len > offset_1st_component(path->buf)) {
> + char *last_slash = find_last_dir_sep(path->buf);
> + strbuf_setlen(path, last_slash - path->buf);
> + }
> +}
This implementation is not correct because when the input is "/foo", the
result is "" when it should be "/". Also, can the input be a
non-normalized path? When the input is "foo///bar", should the result be
"foo" or would "foo//" be an acceptable result? I think it should be the
former. find_last_dir_sep() returns the last of the three slashes, not
the first one. Therefore, I've rewritten the function thusly:
static void strip_last_component(struct strbuf *path)
{
size_t offset = offset_1st_component(path->buf);
size_t len = path->len;
while (offset < len && !is_dir_sep(path->buf[len - 1]))
len--;
while (offset < len && is_dir_sep(path->buf[len - 1]))
len--;
strbuf_setlen(path, len);
}
> +
> +/* get (and remove) the next component in 'remaining' and place it in 'next' */
> +static void get_next_component(struct strbuf *next, struct strbuf *remaining)
> +{
> + char *start = NULL;
> + char *end = NULL;
> +
> + strbuf_reset(next);
> +
> + /* look for the next component */
> + /* Skip sequences of multiple path-separators */
> + for (start = remaining->buf; is_dir_sep(*start); start++)
> + ; /* nothing */
> + /* Find end of the path component */
> + for (end = start; *end && !is_dir_sep(*end); end++)
> + ; /* nothing */
> +
> + strbuf_add(next, start, end - start);
> + /* remove the component from 'remaining' */
> + strbuf_remove(remaining, 0, end - remaining->buf);
> +}
Mental note: This function strips leading slashes and the first path
component; post-condition: remaining is empty or has leading slashes.
> +
> /* We allow "recursive" symbolic links. Only within reason, though. */
> -#define MAXDEPTH 5
> +#define MAXSYMLINKS 5
>
> /*
> * Return the real path (i.e., absolute path, with symlinks resolved
> @@ -21,7 +51,6 @@ int is_directory(const char *path)
> * absolute_path().) The return value is a pointer to a static
> * buffer.
> *
> - * The input and all intermediate paths must be shorter than MAX_PATH.
> * The directory part of path (i.e., everything up to the last
> * dir_sep) must denote a valid, existing directory, but the last
> * component need not exist. If die_on_error is set, then die with an
> @@ -33,22 +62,16 @@ int is_directory(const char *path)
> */
> static const char *real_path_internal(const char *path, int die_on_error)
> {
> - static struct strbuf sb = STRBUF_INIT;
> + static struct strbuf resolved = STRBUF_INIT;
> + struct strbuf remaining = STRBUF_INIT;
> + struct strbuf next = STRBUF_INIT;
> + struct strbuf symlink = STRBUF_INIT;
> char *retval = NULL;
> -
> - /*
> - * If we have to temporarily chdir(), store the original CWD
> - * here so that we can chdir() back to it at the end of the
> - * function:
> - */
> - struct strbuf cwd = STRBUF_INIT;
> -
> - int depth = MAXDEPTH;
> - char *last_elem = NULL;
> + int num_symlinks = 0;
> struct stat st;
>
> /* We've already done it */
> - if (path == sb.buf)
> + if (path == resolved.buf)
> return path;
>
> if (!*path) {
> @@ -58,74 +81,112 @@ static const char *real_path_internal(const char *path, int die_on_error)
> goto error_out;
> }
>
> - strbuf_reset(&sb);
> - strbuf_addstr(&sb, path);
> -
> - while (depth--) {
> - if (!is_directory(sb.buf)) {
> - char *last_slash = find_last_dir_sep(sb.buf);
> - if (last_slash) {
> - last_elem = xstrdup(last_slash + 1);
> - strbuf_setlen(&sb, last_slash - sb.buf + 1);
> - } else {
> - last_elem = xmemdupz(sb.buf, sb.len);
> - strbuf_reset(&sb);
> - }
> + strbuf_reset(&resolved);
> +
> + if (is_absolute_path(path)) {
> + /* absolute path; start with only root as being resolved */
> + int offset = offset_1st_component(path);
> + strbuf_add(&resolved, path, offset);
> + strbuf_addstr(&remaining, path + offset);
OK.
> + } else {
> + /* relative path; can use CWD as the initial resolved path */
> + if (strbuf_getcwd(&resolved)) {
> + if (die_on_error)
> + die_errno("unable to get current working directory");
> + else
> + goto error_out;
> + }
> + strbuf_addstr(&remaining, path);
> + }
> +
> + /* Iterate over the remaining path components */
> + while (remaining.len > 0) {
> + get_next_component(&next, &remaining);
> +
> + if (next.len == 0) {
> + continue; /* empty component */
Can this happen? I think it can: when 'path' is a root directory, or
ends with path separators.
> + } else if (next.len == 1 && !strcmp(next.buf, ".")) {
> + continue; /* '.' component */
Good.
I don't mind strcmp(), but others might point out that a single
character comparison should be sufficient.
> + } else if (next.len == 2 && !strcmp(next.buf, "..")) {
> + /* '..' component; strip the last path component */
> + strip_last_component(&resolved);
> + continue;
Good.
> }
>
> - if (sb.len) {
> - if (!cwd.len && strbuf_getcwd(&cwd)) {
> + /* append the next component and resolve resultant path */
> + if (!is_dir_sep(resolved.buf[resolved.len - 1]))
> + strbuf_addch(&resolved, '/');
Can we access resolved.buf[resolved.len - 1] here? At this point,
resolved has been filled with an absolute path or from getcwd(); it
cannot be empty in the first iteration. In subsequent iterations, it
cannot be empty as long as strip_last_component() does not make it
empty. Your original version of strip_last_component() could make it
empty; my rewrite should not.
Do we have to check for the path separator at all? Typically, resolved
does not end with a slash, but if we went all the way up to the root,
there is a trailing slash. Good; the condition is required.
> + strbuf_addbuf(&resolved, &next);
> +
> + if (lstat(resolved.buf, &st)) {
> + /* error out unless this was the last component */
> + if (!(errno == ENOENT && !remaining.len)) {
Perhaps it was easier to _write_ the condition in this way, but I would
have an easier time to _read_ it when it is
if (errno != ENOENT || remaining.len) {
> if (die_on_error)
> - die_errno("Could not get current working directory");
> + die_errno("Invalid path '%s'",
> + resolved.buf);
> else
> goto error_out;
> }
> + } else if (S_ISLNK(st.st_mode)) {
> + ssize_t len;
> + strbuf_reset(&symlink);
>
> - if (chdir(sb.buf)) {
> + if (num_symlinks++ > MAXSYMLINKS) {
> if (die_on_error)
> - die_errno("Could not switch to '%s'",
> - sb.buf);
> + die("More than %d nested symlinks "
> + "on path '%s'", MAXSYMLINKS, path);
> else
> goto error_out;
> }
> - }
> - if (strbuf_getcwd(&sb)) {
> - if (die_on_error)
> - die_errno("Could not get current working directory");
> - else
> - goto error_out;
> - }
> -
> - if (last_elem) {
> - if (sb.len && !is_dir_sep(sb.buf[sb.len - 1]))
> - strbuf_addch(&sb, '/');
> - strbuf_addstr(&sb, last_elem);
> - free(last_elem);
> - last_elem = NULL;
> - }
>
> - if (!lstat(sb.buf, &st) && S_ISLNK(st.st_mode)) {
> - struct strbuf next_sb = STRBUF_INIT;
> - ssize_t len = strbuf_readlink(&next_sb, sb.buf, 0);
> + len = strbuf_readlink(&symlink, resolved.buf,
> + st.st_size);
> if (len < 0) {
> if (die_on_error)
> die_errno("Invalid symlink '%s'",
> - sb.buf);
> + resolved.buf);
> else
> goto error_out;
> }
> - strbuf_swap(&sb, &next_sb);
> - strbuf_release(&next_sb);
> - } else
> - break;
> +
> + if (is_absolute_path(symlink.buf)) {
> + /* absolute symlink; set resolved to root */
> + int offset = offset_1st_component(symlink.buf);
> + strbuf_reset(&resolved);
> + strbuf_add(&resolved, symlink.buf, offset);
> + strbuf_remove(&symlink, 0, offset);
Good. I would have expected some kind of "goto repeat;" because when we
encounter a symlink to an absolute path, most, if not all, work done so
far is obsoleted. But I haven't thought too deeply about this.
> + } else {
> + /*
> + * relative symlink
> + * strip off the last component since it will
> + * be replaced with the contents of the symlink
> + */
> + strip_last_component(&resolved);
> + }
> +
> + /*
> + * if there are still remaining components to resolve
> + * then append them to symlink
> + */
> + if (remaining.len) {
> + strbuf_addch(&symlink, '/');
> + strbuf_addbuf(&symlink, &remaining);
> + }
> +
> + /*
> + * use the symlink as the remaining components that
> + * need to be resloved
> + */
> + strbuf_swap(&symlink, &remaining);
Good. This looks very reasonable.
> + }
> }
>
> - retval = sb.buf;
> + retval = resolved.buf;
> +
> error_out:
> - free(last_elem);
> - if (cwd.len && chdir(cwd.buf))
> - die_errno("Could not change back to '%s'", cwd.buf);
> - strbuf_release(&cwd);
> + strbuf_release(&remaining);
> + strbuf_release(&next);
> + strbuf_release(&symlink);
>
> return retval;
> }
>
^ permalink raw reply
* Re: git add -p with new file
From: Jeff King @ 2016-12-09 14:11 UTC (permalink / raw)
To: Ariel; +Cc: git
In-Reply-To: <alpine.DEB.2.11.1612062012540.13185@cherryberry.dsgml.com>
On Tue, Dec 06, 2016 at 08:18:59PM -0500, Ariel wrote:
> If you do git add -p new_file it says:
>
> No changes.
>
> Which is a rather confusing message. I would expect it to show me the
> content of the file in patch form, in the normal way that -p works, let me
> edit it, etc.
>
> (Note: I am aware I can do -N first, but when I specifically enter the name
> of a new file I feel it should figure out what I mean.)
Keep in mind that the argument to "git add -p" is not a filename, but a
"pathspec" that can match many files. What should:
git init
mkdir subdir
for i in one two; do echo $i >subdir/$i; done
git add -p subdir
do? Or for that matter, "git add -p ."? It's contrary to the rest of
git-add for specifying pathspecs to actually make things _more_
inclusive rather than less.
So I don't think triggering the change of behavior based on the presence
of a pathspec makes sense.
Historically "add -p" has been more like "add -u" in updating tracked
files. We have "-A" for "update everything _and_ new files". It doesn't
seem unreasonable to me to have a variant of "-p" that is similar.
I don't think that variant should just be "add -N everything, and then
run add -p". As Duy points out, the patch for a new file is just one big
block. But the decision of "do I want to start tracking this untracked
file" is potentially an interesting one.
-Peff
^ permalink raw reply
* Re: [PATCH v8 18/19] branch: use ref-filter printing APIs
From: Jeff King @ 2016-12-09 14:03 UTC (permalink / raw)
To: Karthik Nayak; +Cc: git, jacob.keller, gitster, jnareb
In-Reply-To: <20161207153627.1468-19-Karthik.188@gmail.com>
On Wed, Dec 07, 2016 at 09:06:26PM +0530, Karthik Nayak wrote:
> +const char *quote_literal_for_format(const char *s)
> {
> + struct strbuf buf = STRBUF_INIT;
>
> + strbuf_reset(&buf);
> + while (*s) {
> + const char *ep = strchrnul(s, '%');
> + if (s < ep)
> + strbuf_add(&buf, s, ep - s);
> + if (*ep == '%') {
> + strbuf_addstr(&buf, "%%");
> + s = ep + 1;
> + } else {
> + s = ep;
> + }
> }
> + return buf.buf;
> }
You should use strbuf_detach() to return the buffer from a strbuf.
Otherwise it is undefined whether the pointer is allocated or not (and
whether it needs to be freed).
In this case, if "s" is empty, buf.buf would point to a string literal,
but otherwise to allocated memory. strbuf_detach() normalizes that.
But...
> + branch_get_color(BRANCH_COLOR_REMOTE), maxwidth, quote_literal_for_format(remote_prefix),
This caller never stores the return value, and it ends up leaking. So I
wonder if you wanted "static struct strbuf" in the first place (and that
would explain the strbuf_reset() in your function).
-Peff
^ permalink raw reply
* Re: Bug: git-sh-setup giving no such file or directory
From: Paul Boyle @ 2016-12-09 14:00 UTC (permalink / raw)
To: Jeff King; +Cc: Anders Kaseorg, git
In-Reply-To: <20161209134544.z2xbly5xjyito62w@sigill.intra.peff.net>
> Hmm. Did you run "make install"? Or are you trying to run git directly
> out of the build directory?
>
> If the latter, that has been unsupported for a while, though it mostly
> works. The "right" way is to either set up GIT_EXEC_PATH as appropriate,
> or to just .../git/bin-wrappers into your $PATH.
This was the source of it. Root cause, a stupid user ;) I'd a PATH
setup to the build directory. Changing the path to bin-wrappers fixed
it up.
Thanks!
On Fri, Dec 9, 2016 at 1:45 PM, Jeff King <peff@peff.net> wrote:
> On Fri, Dec 09, 2016 at 12:00:36PM +0000, Paul Boyle wrote:
>
>> There appears to be an issue with the latest master.
>>
>> "git submodule init" is producing the following error:
>>
>> /home/paul.boyle/bin/git/git-sh-setup: line 46:
>> /home/paul.boyle/libexec/git-core/git-sh-i18n: No such file or
>> directory
>
> Hmm. Did you run "make install"? Or are you trying to run git directly
> out of the build directory?
>
> If the latter, that has been unsupported for a while, though it mostly
> works. The "right" way is to either set up GIT_EXEC_PATH as appropriate,
> or to just .../git/bin-wrappers into your $PATH.
>
>> Broken sha: 8d7a455ed52e2a96debc080dfc011b6bb00db5d2
>>
>> Checking out an older version works fine.
>>
>> git checkout 'master@{2016-11-01 18:30:00}'
>>
>> Sha: 3cdd5d19178a54d2e51b5098d43b57571241d0ab
>>
>> This can be reproduced simply by:
>>
>> make clean ; make ; git submodule init
>>
>>
>> I didn't track it down further than to a commit sometime in the last month.
>
> You could probably find the exact commit with git-bisect. However, I'd
> be surprised if it is anything but 1073094f3 (git-sh-setup: be explicit
> where to dot-source git-sh-i18n from., 2016-10-29). Before that commit,
> we found git-sh-i18n in the $PATH, which would work if you were adding
> git's build directory to your $PATH (but not work for people who
> actually did an install).
>
> -Peff
^ permalink raw reply
* Re: Bug: git-sh-setup giving no such file or directory
From: Jeff King @ 2016-12-09 13:45 UTC (permalink / raw)
To: Paul Boyle; +Cc: Anders Kaseorg, git
In-Reply-To: <CABZ0BffSi6h8Zhg8vjo1dZhxXg3fUt_U6TAtqMvpDShOX6HyyA@mail.gmail.com>
On Fri, Dec 09, 2016 at 12:00:36PM +0000, Paul Boyle wrote:
> There appears to be an issue with the latest master.
>
> "git submodule init" is producing the following error:
>
> /home/paul.boyle/bin/git/git-sh-setup: line 46:
> /home/paul.boyle/libexec/git-core/git-sh-i18n: No such file or
> directory
Hmm. Did you run "make install"? Or are you trying to run git directly
out of the build directory?
If the latter, that has been unsupported for a while, though it mostly
works. The "right" way is to either set up GIT_EXEC_PATH as appropriate,
or to just .../git/bin-wrappers into your $PATH.
> Broken sha: 8d7a455ed52e2a96debc080dfc011b6bb00db5d2
>
> Checking out an older version works fine.
>
> git checkout 'master@{2016-11-01 18:30:00}'
>
> Sha: 3cdd5d19178a54d2e51b5098d43b57571241d0ab
>
> This can be reproduced simply by:
>
> make clean ; make ; git submodule init
>
>
> I didn't track it down further than to a commit sometime in the last month.
You could probably find the exact commit with git-bisect. However, I'd
be surprised if it is anything but 1073094f3 (git-sh-setup: be explicit
where to dot-source git-sh-i18n from., 2016-10-29). Before that commit,
we found git-sh-i18n in the $PATH, which would work if you were adding
git's build directory to your $PATH (but not work for people who
actually did an install).
-Peff
^ permalink raw reply
* [PATCH] describe: add tests for unusual graphs
From: Quinn Grier @ 2016-12-09 13:11 UTC (permalink / raw)
To: git; +Cc: Quinn Grier
git describe may give incorrect results if there are backdated commits
or multiple roots. This commit adds two test_expect_failure tests that
demonstrate these problems.
Signed-off-by: Quinn Grier <quinn@quinngrier.com>
---
t/t6120-describe.sh | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 48 insertions(+)
diff --git a/t/t6120-describe.sh b/t/t6120-describe.sh
index 85f2694..ca82837 100755
--- a/t/t6120-describe.sh
+++ b/t/t6120-describe.sh
@@ -206,4 +206,52 @@ test_expect_success 'describe --contains with the exact tags' '
test_cmp expect actual
'
+#
+# A---B*--D master
+# \ /
+# .---C topic
+#
+
+test_expect_failure 'backdated commit' '(
+ test_tick &&
+ b=$GIT_COMMITTER_DATE && test_tick &&
+ test_create_repo backdated-commit &&
+ cd backdated-commit &&
+ git commit --allow-empty -m A && test_tick &&
+ GIT_COMMITTER_DATE=$b git commit --allow-empty -m B && test_tick &&
+ git checkout -b topic :/A &&
+ git commit --allow-empty -m C && test_tick &&
+ git checkout master &&
+ git merge -m D topic && test_tick &&
+ git tag -m B B :/B && test_tick &&
+ git describe :/D >tmp &&
+ sed s/-g.\*// tmp >actual &&
+ echo B-2 >expected &&
+ test_cmp expected actual
+)'
+
+#
+# A---B*--D master
+# /
+# C* other
+#
+
+test_expect_failure 'multiple roots' '(
+ test_tick &&
+ test_create_repo multiple-roots &&
+ cd multiple-roots &&
+ git commit --allow-empty -m A && test_tick &&
+ git commit --allow-empty -m B && test_tick &&
+ git checkout --orphan other &&
+ git commit --allow-empty -m C && test_tick &&
+ git checkout master &&
+ git merge --allow-unrelated-histories -m D other && test_tick &&
+ git tag -m B B :/B && test_tick &&
+ git tag -m C C :/C && test_tick &&
+ git describe :/D >tmp &&
+ sed s/-g.\*// tmp >actual &&
+ echo B-2 >expected &&
+ test_cmp expected actual
+)'
+
test_done
--
2.8.3
^ permalink raw reply related
* Re: [PATCH 02/17] dir: convert create_simplify to use the pathspec struct interface
From: Duy Nguyen @ 2016-12-09 13:08 UTC (permalink / raw)
To: Brandon Williams; +Cc: Git Mailing List, Stefan Beller, Junio C Hamano
In-Reply-To: <20161208181957.GP116201@google.com>
On Fri, Dec 9, 2016 at 1:19 AM, Brandon Williams <bmwill@google.com> wrote:
> On 12/08, Duy Nguyen wrote:
>> On Thu, Dec 8, 2016 at 7:03 AM, Brandon Williams <bmwill@google.com> wrote:
>> > On 12/07, Duy Nguyen wrote:
>> >> On Wed, Dec 7, 2016 at 4:51 AM, Brandon Williams <bmwill@google.com> wrote:
>> >> > Convert 'create_simplify()' to use the pathspec struct interface from
>> >> > using the '_raw' entry in the pathspec.
>> >>
>> >> It would be even better to kill this create_simplify() and let
>> >> simplify_away() handle struct pathspec directly.
>> >>
>> >> There is a bug in this code, that might have been found if we
>> >> simpify_away() handled pathspec directly: the memcmp() in
>> >> simplify_away() will not play well with :(icase) magic. My bad. If
>> >> :(icase) is used, the easiest/safe way is simplify nothing. Later on
>> >> maybe we can teach simplify_away() to do strncasecmp instead. We could
>> >> ignore exclude patterns there too (although not excluding is not a
>> >> bug).
>> >
>> > So are you implying that the simplify struct needs to be killed? That
>> > way the pathspec struct itself is being passed around instead?
>>
>> Yes. simplify struct was a thing when pathspec was an array of char *.
>> At this point I think it can retire (when we have time to retire it)
>
> Alright, then for now I can leave this change as is and have a follow up
> series that kills the simplify struct.
Do let me know if you decide to drop it, so I can put it back in my backlog.
--
Duy
^ permalink raw reply
* Re: [PATCHv6 4/7] worktree: get worktrees from submodules
From: Duy Nguyen @ 2016-12-09 12:46 UTC (permalink / raw)
To: Stefan Beller; +Cc: Brandon Williams, Git Mailing List, Junio C Hamano
In-Reply-To: <CAGZ79kYtEUvuTX09sJm3C0rG0-BrBz4bN0FCs6E5d2jHhtKN6w@mail.gmail.com>
On Fri, Dec 9, 2016 at 1:55 AM, Stefan Beller <sbeller@google.com> wrote:
> On Thu, Dec 8, 2016 at 2:09 AM, Duy Nguyen <pclouds@gmail.com> wrote:
>> On Thu, Dec 8, 2016 at 8:46 AM, Stefan Beller <sbeller@google.com> wrote:
>>>
>>> worktree = xcalloc(1, sizeof(*worktree));
>>> worktree->path = strbuf_detach(&worktree_path, NULL);
>>> @@ -101,7 +101,8 @@ static struct worktree *get_main_worktree(void)
>>
>> All the good stuff is outside context lines again.. Somewhere between
>> here we call add_head_info() which calls resolve_ref_unsafe(), which
>> always uses data from current repo, not the submodule we want it to
>> look at.
>
> Unrelated side question: What would you think of "variable context line
> configuration" ? e.g. you could configure it to include anything from
> up that line
> that is currently shown after the @@ which is the function signature line.
Hmm.. no idea. I once dreamt of writing "Diff-Options: -U10" in the
commit message and let git-log and everybody use that option
automatically, though. I guess it's unrelated to your question :D
> As to the add_head_info/resolve_ref_unsafe what impact does that have?
> It produces a wrong head info but AFAICT it will never die(), such that for the
> purposes of this series (which only wants to know if a submodule uses the
> worktree feature) it should be fine.
>
> It is highly misleading though for others to build upon this.
> So maybe I'll only add the functionality internally in worktree.c
> and document why the values are wrong, and only expose the
> "int submodule_uses_worktrees(const char *path)" ?
Yeah for submodule use then it should be ok. But people may start
using it for something else, not realizing that it does not work as
expected.
--
Duy
^ permalink raw reply
* Re: [PATCH/RFC 0/7] Pie-in-the-sky attempt to fix the early config
From: Duy Nguyen @ 2016-12-09 12:42 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Git Mailing List, Junio C Hamano, Jeff King
In-Reply-To: <cover.1481211338.git.johannes.schindelin@gmx.de>
On Thu, Dec 8, 2016 at 10:35 PM, Johannes Schindelin
<johannes.schindelin@gmx.de> wrote:
> Hopefully these patches will lead to something that we can integrate,
> and that eventually will make Git's startup sequence much less
> surprising.
What did it surprise you with? Just curious. I can see that I
disrespect the ceiling directory setting, perhaps that's that.
--
Duy
^ permalink raw reply
* Re: Feature request: read git config from parent directory
From: Duy Nguyen @ 2016-12-09 12:38 UTC (permalink / raw)
To: dod; +Cc: Git Mailing List
In-Reply-To: <3881793.6JIRvg1BPW@ylum>
On Fri, Dec 9, 2016 at 2:49 AM, Dominique Dumont <dod@debian.org> wrote:
> Hello
>
> I use the same machine for work and open-source contribution. In both cases, I
> deal with a lot of repositories. Depending on whether I commit for work or
> open-source activities, I must use a different mail address. I used to setup
> work address for each work repo in git local config, but this is no longer
> practical.
Sounds like the same problem I have (and the reason I came up with
conditional include [1]). Would that work for you (check out the
example part in that patch)?
It's not supported yet. I'll need to address a few comments in the
future reroll.
[1] http://public-inbox.org/git/20160712164216.24072-1-pclouds@gmail.com/
> Since I use different directories for work and open-source, would it be
> possible for git to read irs config also from parent directories ? I.e. setup
> git to read config from ./.git/config then ../.gitconfig, ../../gitconfig until
> global ~/.gitconfig.
--
Duy
^ permalink raw reply
* Re: [PATCH v2 0/4] road to reentrant real_path
From: Duy Nguyen @ 2016-12-09 12:33 UTC (permalink / raw)
To: Brandon Williams
Cc: Git Mailing List, Stefan Beller, Jeff King, Jacob Keller,
Junio C Hamano, Ramsay Jones, Torsten Bögershausen,
Johannes Sixt
In-Reply-To: <1481241494-6861-1-git-send-email-bmwill@google.com>
On Fri, Dec 9, 2016 at 6:58 AM, Brandon Williams <bmwill@google.com> wrote:
> diff --git a/setup.c b/setup.c
> index fe572b8..0d9fdd0 100644
> --- a/setup.c
> +++ b/setup.c
> @@ -254,10 +254,12 @@ int get_common_dir_noenv(struct strbuf *sb, const char *gitdir)
> if (!is_absolute_path(data.buf))
> strbuf_addf(&path, "%s/", gitdir);
> strbuf_addbuf(&path, &data);
> - strbuf_addstr(sb, real_path(path.buf));
> + strbuf_realpath(sb, path.buf, 1);
This is not the same because of this hunk in strbuf_realpath()
> @@ -81,17 +73,18 @@ static const char *real_path_internal(const char *path, int die_on_error)
> goto error_out;
> }
>
> - strbuf_reset(&resolved);
> + strbuf_reset(resolved);
>
> if (is_absolute_path(path)) {
But if you you remove that, then all (old) callers of
strbuf_realpath() must do a strbuf_reset() in advance if needed
(probably just real_path does) which sounds reasonable to me. You're
probably want to be careful about the strbuf_reset() at the end of the
function too.
Other than that, I think this diff looks nice.
--
Duy
^ permalink raw reply
* Re: [PATCH] doc: fill in omitted word
From: Kristoffer Haugsbakk @ 2016-12-09 12:31 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, Kristoffer Haugsbakk, git
In-Reply-To: <20161110230605.pwgzhai6xhud7pnu@sigill.intra.peff.net>
I agree. Just writing "... what the plumbing does ..." is clearer and
less redundant.
I'll probably be sending a patch series that includes your proposed fix
sometime soon.
--
Kristoffer Haugsbakk
^ permalink raw reply
* Re: [PATCH v2 14/16] pathspec: create strip submodule slash helpers
From: Duy Nguyen @ 2016-12-09 12:16 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Brandon Williams, Git Mailing List, Stefan Beller
In-Reply-To: <xmqqfulyrlmf.fsf@gitster.mtv.corp.google.com>
On Fri, Dec 9, 2016 at 7:28 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Brandon Williams <bmwill@google.com> writes:
>
>> +static void strip_submodule_slash_cheap(struct pathspec_item *item)
>> +{
>> + int i;
>> +
>> + if ((item->len >= 1 && item->match[item->len - 1] == '/') &&
>> + (i = cache_name_pos(item->match, item->len - 1)) >= 0 &&
>> + S_ISGITLINK(active_cache[i]->ce_mode)) {
>> + item->len--;
>> + item->match[item->len] = '\0';
>> + }
>> +}
>
> I know that this is merely a moved code, but while I was reading
> this, it triggered "Do not make an assignment inside if condition"
> check.
Yeah with a function of its own, it's probably better to separate that
assignment.
> But more importantly, is the code even correct? If the path
> for the submodule is unmerged, we would get a negative i that points
> at the conflicting entry; don't we want to do something about it, at
> least when we have a submodule entry at stage #2 (i.e. ours)?
In my defense I was simply moving (again!) the code from
strip_trailing_slash_from_submodules() in b69bb3f:builtin/ls-files.c.
Could be an improvement point for submodule people, I guess.
--
Duy
^ permalink raw reply
* Bug: git-sh-setup giving no such file or directory
From: Paul Boyle @ 2016-12-09 12:00 UTC (permalink / raw)
To: git
Hi
There appears to be an issue with the latest master.
"git submodule init" is producing the following error:
/home/paul.boyle/bin/git/git-sh-setup: line 46:
/home/paul.boyle/libexec/git-core/git-sh-i18n: No such file or
directory
Broken sha: 8d7a455ed52e2a96debc080dfc011b6bb00db5d2
Checking out an older version works fine.
git checkout 'master@{2016-11-01 18:30:00}'
Sha: 3cdd5d19178a54d2e51b5098d43b57571241d0ab
This can be reproduced simply by:
make clean ; make ; git submodule init
I didn't track it down further than to a commit sometime in the last month.
Details of machine that this is happening on:
[paul.boyle@gonzo git]$ cat /etc/redhat-release
Scientific Linux release 6.8 (Carbon)
[paul.boyle@gonzo git]$ env
SSH_AGENT_PID=29474
HOSTNAME=gonzo
TERM=xterm
SHELL=/bin/bash
HISTSIZE=1000
SSH_CLIENT=
QTDIR=/usr/lib64/qt-3.3
QTINC=/usr/lib64/qt-3.3/include
SSH_TTY=/dev/pts/135
USER=paul.boyle
LS_COLORS=rs=0:di=01;34:ln=01;36:mh=00:pi=40;33:so=01;35:do=01;35:bd=40;33;01:cd=40;33;01:or=40;31;01:mi=01;05;37;41:su=37;41:sg=30;43:ca=30;41:tw=30;42:ow=34;42:st=37;44:ex=01;32:*.tar=01;31:*.tgz=01;31:*.arj=01;31:*.taz=01;31:*.lzh=01;31:*.lzma=01;31:*.tlz=01;31:*.txz=01;31:*.zip=01;31:*.z=01;31:*.Z=01;31:*.dz=01;31:*.gz=01;31:*.lz=01;31:*.xz=01;31:*.bz2=01;31:*.tbz=01;31:*.tbz2=01;31:*.bz=01;31:*.tz=01;31:*.deb=01;31:*.rpm=01;31:*.jar=01;31:*.rar=01;31:*.ace=01;31:*.zoo=01;31:*.cpio=01;31:*.7z=01;31:*.rz=01;31:*.jpg=01;35:*.jpeg=01;35:*.gif=01;35:*.bmp=01;35:*.pbm=01;35:*.pgm=01;35:*.ppm=01;35:*.tga=01;35:*.xbm=01;35:*.xpm=01;35:*.tif=01;35:*.tiff=01;35:*.png=01;35:*.svg=01;35:*.svgz=01;35:*.mng=01;35:*.pcx=01;35:*.mov=01;35:*.mpg=01;35:*.mpeg=01;35:*.m2v=01;35:*.mkv=01;35:*.ogm=01;35:*.mp4=01;35:*.m4v=01;35:*.mp4v=01;35:*.vob=01;35:*.qt=01;35:*.nuv=01;35:*.wmv=01;35:*.asf=01;35:*.rm=01;35:*.rmvb=01;35:*.flc=01;35:*.avi=01;35:*.fli=01;35:*.flv=01;35:*.gl=01;35:*.dl=01;35:*.xcf=01;35:*.xwd=01;35:*.yuv=01;35:*.cgm=01;35:*.emf=01;35:*.axv=01;35:*.anx=01;35:*.ogv=01;35:*.ogx=01;35:*.aac=01;36:*.au=01;36:*.flac=01;36:*.mid=01;36:*.midi=01;36:*.mka=01;36:*.mp3=01;36:*.mpc=01;36:*.ogg=01;36:*.ra=01;36:*.wav=01;36:*.axa=01;36:*.oga=01;36:*.spx=01;36:*.xspf=01;36:
SSH_AUTH_SOCK=/tmp/ssh-oXONs29470/agent.29470
MAIL=/var/spool/mail/paul.boyle
PATH=/home/paul.boyle/bin/git:/home/paul.boyle/bin/tig:/home/paul.boyle/bin:/usr/lib64/qt-3.3/bin:/usr/local/bin:/bin:/usr/bin:/usr/local/sbin:/usr/sbin:/sbin:/home/paul.boyle/bin
PWD=/home/paul.boyle/bin/git
LANG=en_IE.UTF-8
HISTCONTROL=ignoredups
SHLVL=1
HOME=/home/paul.boyle
LOGNAME=paul.boyle
QTLIB=/usr/lib64/qt-3.3/lib
CVS_RSH=ssh
SSH_CONNECTION=
LESSOPEN=||/usr/bin/lesspipe.sh %s
G_BROKEN_FILENAMES=1
OLDPWD=/home/paul.boyle/
_=/bin/env
^ permalink raw reply
* Re: [PATCHv7 4/6] worktree: have a function to check if worktrees are in use
From: Duy Nguyen @ 2016-12-09 12:00 UTC (permalink / raw)
To: Stefan Beller; +Cc: bmwill, git, gitster
In-Reply-To: <20161208210329.12919-5-sbeller@google.com>
On Thu, Dec 08, 2016 at 01:03:27PM -0800, Stefan Beller wrote:
> +/*
> + * NEEDSWORK: The values in the returned worktrees are broken, e.g.
> + * the refs or path resolution is influenced by the current repository.
> + */
> +static struct worktree **get_submodule_worktrees(const char *path, unsigned flags)
I'm ok with this (at least we know the function is half-broken). But I
wonder if we could go simpler, with something like this. It's more
efficient as well. And we can replace its implementation with
get_worktrees() or something when we are able to.
As long as this function stays in worktree.c I think it's ok for it to
peek into "worktrees" directory directly. That's what all other
functions in this file do after all.
int submodule_uses_worktrees(const char *path)
{
struct strbuf path = STRBUF_INIT;
DIR *dir;
struct dirent *d;
int ret = 0;
strbuf_addf(&path, "%s/worktrees", path);
dir = opendir(path.buf);
strbuf_release(&path);
if (!dir)
return 0;
while ((d = readdir(dir)) != NULL) {
if (is_dot_or_dotdot(d->d_name))
continue;
ret = 1;
break;
}
closedir(dir);
return ret;
}
--
Duy
^ permalink raw reply
* Resend: Gitk: memory consumption improvements
From: Markus Hitter @ 2016-12-09 11:51 UTC (permalink / raw)
To: git@vger.kernel.org; +Cc: Paul Mackerras
It's a month now since I sent three patches to this list for reducing memory consumption of Gitk considerably:
https://public-inbox.org/git/de7cd593-0c10-4e93-1681-7e123504f5d5@jump-ing.de/
https://public-inbox.org/git/e09a5309-351d-d246-d272-f527f50ad444@jump-ing.de/
https://public-inbox.org/git/8e1c5923-d2a6-bc77-97ab-3f154b41d2ea@jump-ing.de/
https://public-inbox.org/git/2cb7f76f-0004-a5b6-79f1-9bb4f979cf14@jump-ing.de/
Everybody cheered, but apparently nobody picked these patches up and applied them to either the Git or Gitk repository. They don't appear in the regular "what's cooking" post either.
Anything missing? I'd like to put a 'done' checkmark behind this.
Markus
--
- - - - - - - - - - - - - - - - - - -
Dipl. Ing. (FH) Markus Hitter
http://www.jump-ing.de/
^ permalink raw reply
* [PATCH] revert, cherry-pick: rename --quit to be consistent with rebase
From: Nguyễn Thái Ngọc Duy @ 2016-12-09 11:34 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, s-beyer, christian.couder, szeder,
Nguyễn Thái Ngọc Duy
In-Reply-To: <20161209113427.6039-1-pclouds@gmail.com>
The old --quit remains supported, just hidden away.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
Documentation/git-cherry-pick.txt | 2 +-
Documentation/git-revert.txt | 2 +-
Documentation/sequencer.txt | 2 +-
builtin/revert.c | 7 +++++--
contrib/completion/git-completion.bash | 4 ++--
sequencer.c | 2 +-
t/t3510-cherry-pick-sequence.sh | 14 +++++++-------
t/t3511-cherry-pick-x.sh | 2 +-
8 files changed, 19 insertions(+), 16 deletions(-)
diff --git a/Documentation/git-cherry-pick.txt b/Documentation/git-cherry-pick.txt
index 6154e57..de878ff 100644
--- a/Documentation/git-cherry-pick.txt
+++ b/Documentation/git-cherry-pick.txt
@@ -11,7 +11,7 @@ SYNOPSIS
'git cherry-pick' [--edit] [-n] [-m parent-number] [-s] [-x] [--ff]
[-S[<keyid>]] <commit>...
'git cherry-pick' --continue
-'git cherry-pick' --quit
+'git cherry-pick' --forget
'git cherry-pick' --abort
DESCRIPTION
diff --git a/Documentation/git-revert.txt b/Documentation/git-revert.txt
index 573616a..c21a43b 100644
--- a/Documentation/git-revert.txt
+++ b/Documentation/git-revert.txt
@@ -10,7 +10,7 @@ SYNOPSIS
[verse]
'git revert' [--[no-]edit] [-n] [-m parent-number] [-s] [-S[<keyid>]] <commit>...
'git revert' --continue
-'git revert' --quit
+'git revert' --forget
'git revert' --abort
DESCRIPTION
diff --git a/Documentation/sequencer.txt b/Documentation/sequencer.txt
index 5747f44..ddfaad6 100644
--- a/Documentation/sequencer.txt
+++ b/Documentation/sequencer.txt
@@ -3,7 +3,7 @@
'.git/sequencer'. Can be used to continue after resolving
conflicts in a failed cherry-pick or revert.
---quit::
+--forget::
Forget about the current operation in progress. Can be used
to clear the sequencer state after a failed cherry-pick or
revert.
diff --git a/builtin/revert.c b/builtin/revert.c
index 56a2c36..663eaf7 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -77,7 +77,10 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)
const char *me = action_name(opts);
int cmd = 0;
struct option options[] = {
- OPT_CMDMODE(0, "quit", &cmd, N_("end revert or cherry-pick sequence"), 'q'),
+ OPT_CMDMODE(0, "forget", &cmd, N_("end revert or cherry-pick sequence"), 'q'),
+ { OPTION_CMDMODE, 0, "quit", &cmd, NULL,
+ N_("end revert or cherry-pick sequence"),
+ PARSE_OPT_NOARG|PARSE_OPT_NONEG|PARSE_OPT_HIDDEN, NULL, 'q' },
OPT_CMDMODE(0, "continue", &cmd, N_("resume revert or cherry-pick sequence"), 'c'),
OPT_CMDMODE(0, "abort", &cmd, N_("cancel revert or cherry-pick sequence"), 'a'),
OPT_BOOL('n', "no-commit", &opts->no_commit, N_("don't automatically commit")),
@@ -134,7 +137,7 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)
if (opts->subcommand != REPLAY_NONE) {
char *this_operation;
if (opts->subcommand == REPLAY_REMOVE_STATE)
- this_operation = "--quit";
+ this_operation = "--forget";
else if (opts->subcommand == REPLAY_CONTINUE)
this_operation = "--continue";
else {
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 8159f28..d5c74e7 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1047,7 +1047,7 @@ _git_cherry_pick ()
{
local dir="$(__gitdir)"
if [ -f "$dir"/CHERRY_PICK_HEAD ]; then
- __gitcomp "--continue --quit --abort"
+ __gitcomp "--continue --forget --abort"
return
fi
case "$cur" in
@@ -2303,7 +2303,7 @@ _git_revert ()
{
local dir="$(__gitdir)"
if [ -f "$dir"/REVERT_HEAD ]; then
- __gitcomp "--continue --quit --abort"
+ __gitcomp "--continue --forget --abort"
return
fi
case "$cur" in
diff --git a/sequencer.c b/sequencer.c
index e66f2fe..12d10d0 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -812,7 +812,7 @@ static int create_seq_dir(void)
{
if (file_exists(git_path_seq_dir())) {
error(_("a cherry-pick or revert is already in progress"));
- advise(_("try \"git cherry-pick (--continue | --quit | --abort)\""));
+ advise(_("try \"git cherry-pick (--continue | --forget | --abort)\""));
return -1;
}
else if (mkdir(git_path_seq_dir(), 0777) < 0)
diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh
index 7b7a89d..d84fafa 100755
--- a/t/t3510-cherry-pick-sequence.sh
+++ b/t/t3510-cherry-pick-sequence.sh
@@ -18,7 +18,7 @@ test_description='Test cherry-pick continuation features
_r10='\1\1\1\1\1\1\1\1\1\1'
pristine_detach () {
- git cherry-pick --quit &&
+ git cherry-pick --forget &&
git checkout -f "$1^0" &&
git read-tree -u --reset HEAD &&
git clean -d -f -f -q -x
@@ -89,9 +89,9 @@ test_expect_success 'cherry-pick cleans up sequencer state upon success' '
test_path_is_missing .git/sequencer
'
-test_expect_success '--quit does not complain when no cherry-pick is in progress' '
+test_expect_success '--forget does not complain when no cherry-pick is in progress' '
pristine_detach initial &&
- git cherry-pick --quit
+ git cherry-pick --forget
'
test_expect_success '--abort requires cherry-pick in progress' '
@@ -99,14 +99,14 @@ test_expect_success '--abort requires cherry-pick in progress' '
test_must_fail git cherry-pick --abort
'
-test_expect_success '--quit cleans up sequencer state' '
+test_expect_success '--forget cleans up sequencer state' '
pristine_detach initial &&
test_expect_code 1 git cherry-pick base..picked &&
- git cherry-pick --quit &&
+ git cherry-pick --forget &&
test_path_is_missing .git/sequencer
'
-test_expect_success '--quit keeps HEAD and conflicted index intact' '
+test_expect_success '--forget keeps HEAD and conflicted index intact' '
pristine_detach initial &&
cat >expect <<-\EOF &&
OBJID
@@ -116,7 +116,7 @@ test_expect_success '--quit keeps HEAD and conflicted index intact' '
:000000 100644 OBJID OBJID A unrelated
EOF
test_expect_code 1 git cherry-pick base..picked &&
- git cherry-pick --quit &&
+ git cherry-pick --forget &&
test_path_is_missing .git/sequencer &&
test_must_fail git update-index --refresh &&
{
diff --git a/t/t3511-cherry-pick-x.sh b/t/t3511-cherry-pick-x.sh
index 9cce5ae..a56d48e 100755
--- a/t/t3511-cherry-pick-x.sh
+++ b/t/t3511-cherry-pick-x.sh
@@ -5,7 +5,7 @@ test_description='Test cherry-pick -x and -s'
. ./test-lib.sh
pristine_detach () {
- git cherry-pick --quit &&
+ git cherry-pick --forget &&
git checkout -f "$1^0" &&
git read-tree -u --reset HEAD &&
git clean -d -f -f -q -x
--
2.8.2.524.g6ff3d78
^ permalink raw reply related
* [PATCH] rebase: rename --forget to be consistent with sequencer
From: Nguyễn Thái Ngọc Duy @ 2016-12-09 11:34 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, s-beyer, christian.couder, szeder,
Nguyễn Thái Ngọc Duy
In-Reply-To: <CACsJy8CX0HO=LxcEK3K+pCecgFY=40R+gpFoy7CGeN5zEJFJVQ@mail.gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
Documentation/git-rebase.txt | 4 ++--
contrib/completion/git-completion.bash | 4 ++--
git-rebase.sh | 6 +++---
t/t3407-rebase-abort.sh | 8 ++++----
4 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index e01d78e..f892458 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -12,7 +12,7 @@ SYNOPSIS
[<upstream> [<branch>]]
'git rebase' [-i | --interactive] [options] [--exec <cmd>] [--onto <newbase>]
--root [<branch>]
-'git rebase' --continue | --skip | --abort | --forget | --edit-todo
+'git rebase' --continue | --skip | --abort | --quit | --edit-todo
DESCRIPTION
-----------
@@ -252,7 +252,7 @@ leave out at most one of A and B, in which case it defaults to HEAD.
will be reset to where it was when the rebase operation was
started.
---forget::
+--quit::
Abort the rebase operation but HEAD is not reset back to the
original branch. The index and working tree are also left
unchanged as a result.
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 8159f28..2c134f8 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1670,10 +1670,10 @@ _git_rebase ()
{
local dir="$(__gitdir)"
if [ -f "$dir"/rebase-merge/interactive ]; then
- __gitcomp "--continue --skip --abort --forget --edit-todo"
+ __gitcomp "--continue --skip --abort --quit --edit-todo"
return
elif [ -d "$dir"/rebase-apply ] || [ -d "$dir"/rebase-merge ]; then
- __gitcomp "--continue --skip --abort --forget"
+ __gitcomp "--continue --skip --abort --quit"
return
fi
__git_complete_strategy && return
diff --git a/git-rebase.sh b/git-rebase.sh
index f0de633..c62b178 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -43,7 +43,7 @@ continue! continue
abort! abort and check out the original branch
skip! skip current patch and continue
edit-todo! edit the todo list during an interactive rebase
-forget! abort but keep HEAD where it is
+quit! abort but keep HEAD where it is
"
. git-sh-setup
. git-sh-i18n
@@ -240,7 +240,7 @@ do
--verify)
ok_to_skip_pre_rebase=
;;
- --continue|--skip|--abort|--forget|--edit-todo)
+ --continue|--skip|--abort|--quit|--edit-todo)
test $total_argc -eq 2 || usage
action=${1##--}
;;
@@ -403,7 +403,7 @@ abort)
finish_rebase
exit
;;
-forget)
+quit)
exec rm -rf "$state_dir"
;;
edit-todo)
diff --git a/t/t3407-rebase-abort.sh b/t/t3407-rebase-abort.sh
index 6bc5e71..910f218 100755
--- a/t/t3407-rebase-abort.sh
+++ b/t/t3407-rebase-abort.sh
@@ -99,26 +99,26 @@ testrebase() {
testrebase "" .git/rebase-apply
testrebase " --merge" .git/rebase-merge
-test_expect_success 'rebase --forget' '
+test_expect_success 'rebase --quit' '
cd "$work_dir" &&
# Clean up the state from the previous one
git reset --hard pre-rebase &&
test_must_fail git rebase master &&
test_path_is_dir .git/rebase-apply &&
head_before=$(git rev-parse HEAD) &&
- git rebase --forget &&
+ git rebase --quit &&
test $(git rev-parse HEAD) = $head_before &&
test ! -d .git/rebase-apply
'
-test_expect_success 'rebase --merge --forget' '
+test_expect_success 'rebase --merge --quit' '
cd "$work_dir" &&
# Clean up the state from the previous one
git reset --hard pre-rebase &&
test_must_fail git rebase --merge master &&
test_path_is_dir .git/rebase-merge &&
head_before=$(git rev-parse HEAD) &&
- git rebase --forget &&
+ git rebase --quit &&
test $(git rev-parse HEAD) = $head_before &&
test ! -d .git/rebase-merge
'
--
2.8.2.524.g6ff3d78
^ permalink raw reply related
* Re: BUG: "cherry-pick A..B || git reset --hard OTHER"
From: Duy Nguyen @ 2016-12-09 11:33 UTC (permalink / raw)
To: Junio C Hamano
Cc: Stephan Beyer, Git Mailing List, Christian Couder,
SZEDER Gábor
In-Reply-To: <xmqq8trry08k.fsf@gitster.mtv.corp.google.com>
On Thu, Dec 8, 2016 at 3:04 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Stephan Beyer <s-beyer@gmx.net> writes:
>
>> [1] By the way: git cherry-pick --quit, git rebase --forget ...
>> different wording for the same thing makes things unintuitive.
>
> It is not too late to STOP "--forget" from getting added to "rebase"
> and give it a better name.
Having the same operation with different names only increases git
reputation of bad/inconsistent UI. Either forget is renamed to quit,
or vice versa. I prefer forget, but the decision is yours and the
community's. So I'm sending two patches to rename in either direction.
You can pick one.
--
Duy
^ permalink raw reply
* Re: Any interest in 'git merge --continue' as a command
From: Jacob Keller @ 2016-12-09 10:37 UTC (permalink / raw)
To: Jeff King, Chris Packham; +Cc: GIT
In-Reply-To: <20161209091127.sxxczhfslrqsqs3m@sigill.intra.peff.net>
On December 9, 2016 1:11:27 AM PST, Jeff King <peff@peff.net> wrote:
>On Fri, Dec 09, 2016 at 08:57:58PM +1300, Chris Packham wrote:
>
>> I hit this at $dayjob recently.
>>
>> A developer had got themselves into a confused state when needing to
>> resolve a merge conflict.
>>
>> They knew about git rebase --continue (and git am and git
>cherry-pick)
>> but they were unsure how to "continue" a merge (it didn't help that
>> the advice saying to use 'git commit' was scrolling off the top of
>the
>> terminal). I know that using 'git commit' has been the standard way
>to
>> complete a merge but given other commands have a --continue should
>> merge have it as well?
>
>It seems like that would be in line with 35d2fffdb (Provide 'git merge
>--abort' as a synonym to 'git reset --merge', 2010-11-09), whose stated
>goal was providing consistency with other multi-command operations.
>
>I assume it would _just_ run a vanilla "git commit", and not try to do
>any trickery with updating the index (which could be disastrous).
>
>-Peff
This makes sense to me.
Thanks,
Jake
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox