* Re: Feature request: use relative path in worktree config files
From: Duy Nguyen @ 2016-10-09 11:37 UTC (permalink / raw)
To: Stéphane Klein; +Cc: Git Mailing List
In-Reply-To: <CADKxhpd-gDczp05Ny6wP57RXafWWCimArfA9ki2phh+-gHez4A@mail.gmail.com>
On Sun, Oct 9, 2016 at 6:22 PM, Stéphane Klein
<contact@stephane-klein.info> wrote:
> 2016-10-09 13:11 GMT+02:00 Duy Nguyen <pclouds@gmail.com>:
>
>>> * [worktree_foobar]/.git
>> This is made absolute on purpose. So that if you move worktree_foobar
>> away manually, it can still point back to
>> "[main_worktree]/.git/worktrees/[woktree_foobar]".
>
> Same problem if you move origin git repository.
We could fix up after moving the origin repository (because "gitdir"
file so far uses absolute paths, so we know where all the worktrees
are). But we have not done that.
>>> Why:
>>>
>>> 1. I configure worktree on my host
>>> 2. next I use this git working copy in Docker with volume share
>>> 3. next I've some git error in Docker because config files use absolute path
>>
>> I think the common way of dealing with this in docker is put things in
>> the same path where it actually is outside docker. If you have stuff
>> at /path/to/foo, then you create the same /path/to/foo inside docker
>> and bind the data to that path. Does that work?
>
> It's not always possible. I can't in my project.
>
> I think there are some pros and some cons for relative path and absolute path.
> Maybe append a "--relative" option with `git worktree add` ?
>
> I've converted all path to relative and all work with success.
>
> What do you think to append this --relative option.
Patches are welcome.
--
Duy
^ permalink raw reply
* Re: Feature request: use relative path in worktree config files
From: Stéphane Klein @ 2016-10-09 11:39 UTC (permalink / raw)
To: Duy Nguyen; +Cc: Git Mailing List
In-Reply-To: <CACsJy8DGvyWz2_VBsEfmDWOPGUGAUhKYhCa1qLEMcOdn83ocSQ@mail.gmail.com>
2016-10-09 13:37 GMT+02:00 Duy Nguyen <pclouds@gmail.com>:
> On Sun, Oct 9, 2016 at 6:22 PM, Stéphane Klein
> <contact@stephane-klein.info> wrote:
>> 2016-10-09 13:11 GMT+02:00 Duy Nguyen <pclouds@gmail.com>:
>>>> Why:
>>>>
>>>> 1. I configure worktree on my host
>>>> 2. next I use this git working copy in Docker with volume share
>>>> 3. next I've some git error in Docker because config files use absolute path
>>>
>>> I think the common way of dealing with this in docker is put things in
>>> the same path where it actually is outside docker. If you have stuff
>>> at /path/to/foo, then you create the same /path/to/foo inside docker
>>> and bind the data to that path. Does that work?
>>
>> It's not always possible. I can't in my project.
>>
>> I think there are some pros and some cons for relative path and absolute path.
>> Maybe append a "--relative" option with `git worktree add` ?
>>
>> I've converted all path to relative and all work with success.
>>
>> What do you think to append this --relative option.
>
> Patches are welcome.
Thanks :)
^ permalink raw reply
* [PATCH] contrib: add credential helper for libsecret
From: Mantas Mikulėnas @ 2016-10-09 12:34 UTC (permalink / raw)
To: git; +Cc: Mantas Mikulėnas
This is based on the existing gnome-keyring helper, but instead of
libgnome-keyring (which was specific to GNOME and is deprecated), it
uses libsecret which can support other implementations of XDG Secret
Service API.
Passes t0303-credential-external.sh.
Signed-off-by: Mantas Mikulėnas <grawity@gmail.com>
---
contrib/credential/libsecret/Makefile | 25 ++
.../libsecret/git-credential-libsecret.c | 370 +++++++++++++++++++++
2 files changed, 395 insertions(+)
create mode 100644 contrib/credential/libsecret/Makefile
create mode 100644 contrib/credential/libsecret/git-credential-libsecret.c
diff --git a/contrib/credential/libsecret/Makefile b/contrib/credential/libsecret/Makefile
new file mode 100644
index 000000000000..3e67552cc5b5
--- /dev/null
+++ b/contrib/credential/libsecret/Makefile
@@ -0,0 +1,25 @@
+MAIN:=git-credential-libsecret
+all:: $(MAIN)
+
+CC = gcc
+RM = rm -f
+CFLAGS = -g -O2 -Wall
+PKG_CONFIG = pkg-config
+
+-include ../../../config.mak.autogen
+-include ../../../config.mak
+
+INCS:=$(shell $(PKG_CONFIG) --cflags libsecret-1 glib-2.0)
+LIBS:=$(shell $(PKG_CONFIG) --libs libsecret-1 glib-2.0)
+
+SRCS:=$(MAIN).c
+OBJS:=$(SRCS:.c=.o)
+
+%.o: %.c
+ $(CC) $(CFLAGS) $(CPPFLAGS) $(INCS) -o $@ -c $<
+
+$(MAIN): $(OBJS)
+ $(CC) -o $@ $(LDFLAGS) $^ $(LIBS)
+
+clean:
+ @$(RM) $(MAIN) $(OBJS)
diff --git a/contrib/credential/libsecret/git-credential-libsecret.c b/contrib/credential/libsecret/git-credential-libsecret.c
new file mode 100644
index 000000000000..4c56979d8a08
--- /dev/null
+++ b/contrib/credential/libsecret/git-credential-libsecret.c
@@ -0,0 +1,370 @@
+/*
+ * Copyright (C) 2011 John Szakmeister <john@szakmeister.net>
+ * 2012 Philipp A. Hartmann <pah@qo.cx>
+ * 2016 Mantas Mikulėnas <grawity@gmail.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ */
+
+/*
+ * Credits:
+ * - GNOME Keyring API handling originally written by John Szakmeister
+ * - ported to credential helper API by Philipp A. Hartmann
+ */
+
+#include <stdio.h>
+#include <string.h>
+#include <stdlib.h>
+#include <glib.h>
+#include <libsecret/secret.h>
+
+/*
+ * This credential struct and API is simplified from git's credential.{h,c}
+ */
+struct credential {
+ char *protocol;
+ char *host;
+ unsigned short port;
+ char *path;
+ char *username;
+ char *password;
+};
+
+#define CREDENTIAL_INIT { NULL, NULL, 0, NULL, NULL, NULL }
+
+typedef int (*credential_op_cb)(struct credential *);
+
+struct credential_operation {
+ char *name;
+ credential_op_cb op;
+};
+
+#define CREDENTIAL_OP_END { NULL, NULL }
+
+/* ----------------- Secret Service functions ----------------- */
+
+static char *make_label(struct credential *c)
+{
+ if (c->port)
+ return g_strdup_printf("Git: %s://%s:%hu/%s",
+ c->protocol, c->host, c->port, c->path ? c->path : "");
+ else
+ return g_strdup_printf("Git: %s://%s/%s",
+ c->protocol, c->host, c->path ? c->path : "");
+}
+
+static GHashTable *make_attr_list(struct credential *c)
+{
+ GHashTable *al = g_hash_table_new_full(g_str_hash, g_str_equal, NULL, g_free);
+
+ if (c->username)
+ g_hash_table_insert(al, "user", g_strdup(c->username));
+ if (c->protocol)
+ g_hash_table_insert(al, "protocol", g_strdup(c->protocol));
+ if (c->host)
+ g_hash_table_insert(al, "server", g_strdup(c->host));
+ if (c->port)
+ g_hash_table_insert(al, "port", g_strdup_printf("%hu", c->port));
+ if (c->path)
+ g_hash_table_insert(al, "object", g_strdup(c->path));
+
+ return al;
+}
+
+static int keyring_get(struct credential *c)
+{
+ SecretService *service = NULL;
+ GHashTable *attributes = NULL;
+ GError *error = NULL;
+ GList *items = NULL;
+
+ if (!c->protocol || !(c->host || c->path))
+ return EXIT_FAILURE;
+
+ service = secret_service_get_sync(0, NULL, &error);
+ if (error != NULL) {
+ g_critical("could not connect to Secret Service: %s", error->message);
+ g_error_free(error);
+ return EXIT_FAILURE;
+ }
+
+ attributes = make_attr_list(c);
+ items = secret_service_search_sync(service,
+ SECRET_SCHEMA_COMPAT_NETWORK,
+ attributes,
+ SECRET_SEARCH_LOAD_SECRETS,
+ NULL,
+ &error);
+ g_hash_table_unref(attributes);
+ if (error != NULL) {
+ g_critical("lookup failed: %s", error->message);
+ g_error_free(error);
+ return EXIT_FAILURE;
+ }
+
+ if (items != NULL) {
+ SecretItem *item;
+ SecretValue *secret;
+ const char *s;
+
+ item = items->data;
+ secret = secret_item_get_secret(item);
+ attributes = secret_item_get_attributes(item);
+
+ s = g_hash_table_lookup(attributes, "user");
+ if (s) {
+ g_free(c->username);
+ c->username = g_strdup(s);
+ }
+
+ s = secret_value_get_text(secret);
+ if (s) {
+ g_free(c->password);
+ c->password = g_strdup(s);
+ }
+
+ g_hash_table_unref(attributes);
+ secret_value_unref(secret);
+ g_list_free_full(items, g_object_unref);
+ }
+
+ return EXIT_SUCCESS;
+}
+
+
+static int keyring_store(struct credential *c)
+{
+ char *label = NULL;
+ GHashTable *attributes = NULL;
+ GError *error = NULL;
+
+ /*
+ * Sanity check that what we are storing is actually sensible.
+ * In particular, we can't make a URL without a protocol field.
+ * Without either a host or pathname (depending on the scheme),
+ * we have no primary key. And without a username and password,
+ * we are not actually storing a credential.
+ */
+ if (!c->protocol || !(c->host || c->path) ||
+ !c->username || !c->password)
+ return EXIT_FAILURE;
+
+ label = make_label(c);
+ attributes = make_attr_list(c);
+ secret_password_storev_sync(SECRET_SCHEMA_COMPAT_NETWORK,
+ attributes,
+ NULL,
+ label,
+ c->password,
+ NULL,
+ &error);
+ g_free(label);
+ g_hash_table_unref(attributes);
+
+ if (error != NULL) {
+ g_critical("store failed: %s", error->message);
+ g_error_free(error);
+ return EXIT_FAILURE;
+ }
+
+ return EXIT_SUCCESS;
+}
+
+static int keyring_erase(struct credential *c)
+{
+ GHashTable *attributes = NULL;
+ GError *error = NULL;
+
+ /*
+ * Sanity check that we actually have something to match
+ * against. The input we get is a restrictive pattern,
+ * so technically a blank credential means "erase everything".
+ * But it is too easy to accidentally send this, since it is equivalent
+ * to empty input. So explicitly disallow it, and require that the
+ * pattern have some actual content to match.
+ */
+ if (!c->protocol && !c->host && !c->path && !c->username)
+ return EXIT_FAILURE;
+
+ attributes = make_attr_list(c);
+ secret_password_clearv_sync(SECRET_SCHEMA_COMPAT_NETWORK,
+ attributes,
+ NULL,
+ &error);
+ g_hash_table_unref(attributes);
+
+ if (error != NULL) {
+ g_critical("erase failed: %s", error->message);
+ g_error_free(error);
+ return EXIT_FAILURE;
+ }
+
+ return EXIT_SUCCESS;
+}
+
+/*
+ * Table with helper operation callbacks, used by generic
+ * credential helper main function.
+ */
+static struct credential_operation const credential_helper_ops[] = {
+ { "get", keyring_get },
+ { "store", keyring_store },
+ { "erase", keyring_erase },
+ CREDENTIAL_OP_END
+};
+
+/* ------------------ credential functions ------------------ */
+
+static void credential_init(struct credential *c)
+{
+ memset(c, 0, sizeof(*c));
+}
+
+static void credential_clear(struct credential *c)
+{
+ g_free(c->protocol);
+ g_free(c->host);
+ g_free(c->path);
+ g_free(c->username);
+ g_free(c->password);
+
+ credential_init(c);
+}
+
+static int credential_read(struct credential *c)
+{
+ char *buf;
+ size_t line_len;
+ char *key;
+ char *value;
+
+ key = buf = g_malloc(1024);
+
+ while (fgets(buf, 1024, stdin)) {
+ line_len = strlen(buf);
+
+ if (line_len && buf[line_len-1] == '\n')
+ buf[--line_len] = '\0';
+
+ if (!line_len)
+ break;
+
+ value = strchr(buf, '=');
+ if (!value) {
+ g_warning("invalid credential line: %s", key);
+ g_free(buf);
+ return -1;
+ }
+ *value++ = '\0';
+
+ if (!strcmp(key, "protocol")) {
+ g_free(c->protocol);
+ c->protocol = g_strdup(value);
+ } else if (!strcmp(key, "host")) {
+ g_free(c->host);
+ c->host = g_strdup(value);
+ value = strrchr(c->host, ':');
+ if (value) {
+ *value++ = '\0';
+ c->port = atoi(value);
+ }
+ } else if (!strcmp(key, "path")) {
+ g_free(c->path);
+ c->path = g_strdup(value);
+ } else if (!strcmp(key, "username")) {
+ g_free(c->username);
+ c->username = g_strdup(value);
+ } else if (!strcmp(key, "password")) {
+ g_free(c->password);
+ c->password = g_strdup(value);
+ while (*value)
+ *value++ = '\0';
+ }
+ /*
+ * Ignore other lines; we don't know what they mean, but
+ * this future-proofs us when later versions of git do
+ * learn new lines, and the helpers are updated to match.
+ */
+ }
+
+ g_free(buf);
+
+ return 0;
+}
+
+static void credential_write_item(FILE *fp, const char *key, const char *value)
+{
+ if (!value)
+ return;
+ fprintf(fp, "%s=%s\n", key, value);
+}
+
+static void credential_write(const struct credential *c)
+{
+ /* only write username/password, if set */
+ credential_write_item(stdout, "username", c->username);
+ credential_write_item(stdout, "password", c->password);
+}
+
+static void usage(const char *name)
+{
+ struct credential_operation const *try_op = credential_helper_ops;
+ const char *basename = strrchr(name, '/');
+
+ basename = (basename) ? basename + 1 : name;
+ fprintf(stderr, "usage: %s <", basename);
+ while (try_op->name) {
+ fprintf(stderr, "%s", (try_op++)->name);
+ if (try_op->name)
+ fprintf(stderr, "%s", "|");
+ }
+ fprintf(stderr, "%s", ">\n");
+}
+
+int main(int argc, char *argv[])
+{
+ int ret = EXIT_SUCCESS;
+
+ struct credential_operation const *try_op = credential_helper_ops;
+ struct credential cred = CREDENTIAL_INIT;
+
+ if (!argv[1]) {
+ usage(argv[0]);
+ exit(EXIT_FAILURE);
+ }
+
+ g_set_application_name("Git Credential Helper");
+
+ /* lookup operation callback */
+ while (try_op->name && strcmp(argv[1], try_op->name))
+ try_op++;
+
+ /* unsupported operation given -- ignore silently */
+ if (!try_op->name || !try_op->op)
+ goto out;
+
+ ret = credential_read(&cred);
+ if (ret)
+ goto out;
+
+ /* perform credential operation */
+ ret = (*try_op->op)(&cred);
+
+ credential_write(&cred);
+
+out:
+ credential_clear(&cred);
+ return ret;
+}
--
2.10.0
^ permalink raw reply related
* Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd
From: Duy Nguyen @ 2016-10-09 11:32 UTC (permalink / raw)
To: Jeff King
Cc: Johannes Schindelin, Johannes Sixt, Junio C Hamano,
Git Mailing List
In-Reply-To: <20161009060149.voqjoiltqi6jub7g@sigill.intra.peff.net>
On Sun, Oct 9, 2016 at 1:01 PM, Jeff King <peff@peff.net> wrote:
> On Sat, Oct 08, 2016 at 10:36:13AM +0200, Johannes Schindelin wrote:
>
>> > > Maybe it's time to aim for
>> > >
>> > > git config alias.d2u.shell \
>> > > 'f() { git ls-files "$@" | xargs dos2unix; }; f'
>> > > git config alias.d2u.cdup false
>> > > git d2u *.c # yada!
>> >
>> > That would be nice. It would also allow "alias.foo_bar.shell"; right now
>> > "alias.foo_bar" is forbidden because of the config syntax, which does
>> > not allow underscores outside of the "subsection" name.
>>
>> So what about this?
>>
>> [alias]
>> d2u = !dos2unix
>> [alias "d2u"]
>> shell = 'f() { git ls-files "$@" | xargs dos2unix; }; f'
>> exec = C:/cygwin64/bin/dos2unix.exe
>>
>> You introduce all kinds of ambiguities here that did not exist before...
>
> If you mean ambiguity between the old "alias.X" and the new "alias.X.*",
> then yes, I think that's an unavoidable part of the transition. IMHO,
> the new should take precedence over the old, and people will gradually
> move from one to the other.
Do we really need to treat this differently than
[alias]
d2u = !dos2unix
d2u = C:/cygwin/bin/dos3unix.exe
?
Another similar case is one d2u (could be either old syntax or new) is
defined in ~/.gitconfig and the other d2u in $GIT_DIR/config. In
either case, the "latest" d2u definition wins.
> If you mean the ambiguity between alias.X.shell and alias.X.exec in your
> example, the problem is that you have keys with overlapping meanings.
> One solution is "don't do that" (so have a key like "cmd", and another
> to select "shell or git-cmd", etc). Another is to define some rule, like
> "last one wins" (so "exec" overrides "shell" in your example).
>
> I'd prefer the "don't do that" path. The config you showed is
> nonsensical, and it doesn't really matter that much how we behave. But
> it is better still if we have a config scheme that makes it hard to
> write nonsensical things in the first place.
Any suggestion? I suppose we can have _one_ key for the command. How
to execute that command (exec, shell, nocd...) are boolean options.
People can still write conflicting things. We have been nice so far,
always dying when the user specify conflicting command line options.
We could do the same here, I guess.
--
Duy
^ permalink raw reply
* RE: A head scratcher, clone results in modified files (tested linux and cygwin) - .gitattributes file?
From: Jason Pyeron @ 2016-10-09 12:48 UTC (permalink / raw)
To: git; +Cc: 'Torsten Bögershausen'
In-Reply-To: <c9c90c24-3bf7-cd57-cad6-db28db16f323@web.de>
> -----Original Message-----
> From: Torsten Bögershausen
> Sent: Sunday, October 09, 2016 06:27
>
> On 09/10/16 08:48, Jason Pyeron wrote:
>
>
> The whole .gitattributes needs to be adopted, I think
>
> Git 2.10 or higher has "git ls-files --eol":
>
> git ls-files --eol | grep "i/crlf.*auto"
> i/crlf w/crlf attr/text=auto src/site/xdoc/upgradeto2_3.xml
> i/crlf w/crlf attr/text=auto
> src/test/resources/org/apache/commons/io/FileUtilsTestDataCRLF.dat
> i/crlf w/crlf attr/text=auto src/test/resources/test-file-gbk.bin
> i/crlf w/crlf attr/text=auto
> src/test/resources/test-file-iso8859-1-shortlines-win-linebr.bin
> i/crlf w/crlf attr/text=auto
> src/test/resources/test-file-utf8-win-linebr.bin
> i/crlf w/crlf attr/text=auto
> src/test/resources/test-file-windows-31j.bin
> i/crlf w/crlf attr/text=auto
> src/test/resources/test-file-x-windows-949.bin
> i/crlf w/crlf attr/text=auto
> src/test/resources/test-file-x-windows-950.bin
> ############
> Problem:
> xml file had been commited with CRLF : either normalize it
> or declare "-text".
>
> The same is valid for the other files as well.
> They are identified by auto as text, and commited with CRLF.
> My feeling is that they should be declared as "-text".
> Or, to be more compatible, with "-crlf":
>
Good call.
> Solution:
> Make up your mind about the xml file and the html files.
> If they are text, they need to be normalized.
>
>
> Question:
> What happens, if you do this:
> # Auto detect text files and perform LF normalization
> * crlf=auto
>
> *.bin -crlf
> *.dat -crlf
*.bin -text
*.dat -text
#fix that issue
> *.java crlf diff=java
> *.html crlf diff=html
> *.css crlf
> *.js crlf
> *.sql crlf
>
Or create a subordinate
src/test/resources/.gitattributes:
* -text
Since these are "test" resources, some with text extensions from above.
Thanks!
-Jason
^ permalink raw reply
* Re: %C(auto) not working as expected
From: René Scharfe @ 2016-10-09 13:24 UTC (permalink / raw)
To: Tom Hale, git; +Cc: Duy Nguyen, Junio C Hamano, Jeff King
In-Reply-To: <65d8def3-df62-6c45-7d8f-79b6a8769bf5@hale.ee>
Am 09.10.2016 um 12:04 schrieb Tom Hale:
> On 2016-10-09 13:47, René Scharfe wrote:
>
>> %Cgreen emits color codes unconditionally. %C(auto,green) would respect
>> the config settings.
>
> Thanks, I've never seen the (,) syntax documented before!
Both the prefix "auto," for terminal-detection and "%C(auto)" for
choosing colors automatically are mentioned in the manpage for git log
(from Documentation/pretty-formats.txt):
- '%C(...)': color specification, as described in color.branch.* config option;
adding `auto,` at the beginning will emit color only when colors are
enabled for log output (by `color.diff`, `color.ui`, or `--color`, and
respecting the `auto` settings of the former if we are going to a
terminal). `auto` alone (i.e. `%C(auto)`) will turn on auto coloring
on the next placeholders until the color is switched again.
> What's strange is that this works:
> %C(auto,green bold)
> but
> %C(auto,green,bold)
> does not.
Looking at the code that's not surprising; colors and attributes are
interpreted as a space-separated list. The prefix "auto," is handled
specially. For a user it may look strange, admittedly.
Supporting "auto " as well would be easy. Supporting it in such a way
that it can be mixed freely with colors and attributes as in
%C(bold auto green) would be a bit harder. Could this lead to confusion
between the auto for terminal-detection and the one for automatic color
selection?
The documentation cited above says the color specification was explained
together with the color.branch.* config option, but that part only says
(from Documentation/config.txt):
color.branch.<slot>::
Use customized color for branch coloration. `<slot>` is one of
`current` (the current branch), `local` (a local branch),
`remote` (a remote-tracking branch in refs/remotes/),
`upstream` (upstream tracking branch), `plain` (other
refs).
It really is described earlier in the same file, in the Values section
(a fitting place, I think). Here's just the first sentence:
color::
The value for a variable that takes a color is a list of
colors (at most two, one for foreground and one for background)
and attributes (as many as you want), separated by spaces.
Patch below. Does it help a little?
> Also:
> Given it's very rare to want only part of a string to emit colour codes,
> if something like "bold" carries through until a "no-bold", why doesn't
> "auto" do the same thing?
No state is kept for "auto,". Attributes and colors are switched
separately by terminals, that's why e.g. bold stays in effect through
a color change -- unless you specify an attribute change as well.
Offering a way to enable terminal-detection for all color codes of a
format would be useful, but using the existing "auto," prefix for that
would be a behaviour change that could surprise users.
René
-- >8 --
Subject: [PATCH] pretty: fix document reference for color specification
Signed-off-by: Rene Scharfe <l.s.r@web.de>
---
Documentation/pretty-formats.txt | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index a942d57..89e3bc6 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -166,7 +166,8 @@ endif::git-rev-list[]
- '%Cgreen': switch color to green
- '%Cblue': switch color to blue
- '%Creset': reset color
-- '%C(...)': color specification, as described in color.branch.* config option;
+- '%C(...)': color specification, as described under Values, color in the
+ "CONFIGURATION FILE" section of linkgit:git-config[1];
adding `auto,` at the beginning will emit color only when colors are
enabled for log output (by `color.diff`, `color.ui`, or `--color`, and
respecting the `auto` settings of the former if we are going to a
--
2.10.1
^ permalink raw reply related
* Re: Bug? git worktree fails with master on bare repo
From: Michael Tutty @ 2016-10-09 13:42 UTC (permalink / raw)
To: Duy Nguyen; +Cc: Dennis Kaarsemaker, Git Mailing List, Michael Rappazzo
In-Reply-To: <CACsJy8CmgmGLEi0xQUY9Eo-4FkA4eDNk9WJ2LtEDVFQBjbFdCA@mail.gmail.com>
Dennis,
Thanks for the great response, and for spending time on my issue.
I'll try that first patch and see what happens.
In the meantime, it got weirder...
I created a brand-new (bare) repo and was able to git add worktree
/path master. I was able to do this repeatedly, even using the
worktree to merge other branches to master. I didn't find any
condition or step that caused some kind of orphan master work tree,
which was what I thought the underlying problem might be.
So, on the one hand, you found code validating my initial experience.
But on the other hand, I found a test case that didn't appear to have
that problem.
WAT.
M.
On Sun, Oct 9, 2016 at 5:52 AM, Duy Nguyen <pclouds@gmail.com> wrote:
> On Sun, Oct 9, 2016 at 2:51 PM, Dennis Kaarsemaker
> <dennis@kaarsemaker.net> wrote:
>> On Sat, 2016-10-08 at 19:30 -0500, Michael Tutty wrote:
>>> Hey all,
>>> I'm working on some server-side software to do a merge. By using git
>>> worktree it's possible to check out a given branch for a bare repo and
>>> merge another branch into it. It's very fast, even with large
>>> repositories.
>>>
>>> The only exception seems to be merging to master. When I do git
>>> worktree add /tmp/path/to/worktree master I get an error:
>>>
>>> [fatal: 'master' is already checked out at '/path/to/bare/repo']
>>>
>>> But this is clearly not true, git worktree list gives:
>>>
>>> [/path/to/bare/repo (bare)]
>>>
>>> ...and of course, there is no work tree at that path, just the bare
>>> repo files you'd expect.
>>
>> The worktree code treats the base repo as a worktree, even if it's
>> bare. For the purpose of being able to do a checkout of the main branch
>> of a bare repo, this patch should do:
>>
>> diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh
>> index 4bcc335..b618d6b 100755
>> --- a/t/t2025-worktree-add.sh
>> +++ b/t/t2025-worktree-add.sh
>> @@ -138,6 +138,14 @@ test_expect_success 'checkout from a bare repo without "add"' '
>> )
>> '
>>
>> +test_expect_success '"add" default branch of a bare repo' '
>> + (
>> + git clone --bare . bare2 &&
>> + cd bare2 &&
>> + git worktree add ../there3 master
>> + )
>> +'
>> +
>> test_expect_success 'checkout with grafts' '
>> test_when_finished rm .git/info/grafts &&
>> test_commit abc &&
>> diff --git a/worktree.c b/worktree.c
>> index 5acfe4c..35e95b7 100644
>> --- a/worktree.c
>> +++ b/worktree.c
>> @@ -345,6 +345,8 @@ const struct worktree *find_shared_symref(const char *symref,
>>
>> for (i = 0; worktrees[i]; i++) {
>> struct worktree *wt = worktrees[i];
>> + if(wt->is_bare)
>> + continue;
>>
>> if (wt->is_detached && !strcmp(symref, "HEAD")) {
>> if (is_worktree_being_rebased(wt, target)) {
>>
>>
>
> You're fast :) I'm still studying 8d9fdd7 (worktree.c: check whether
> branch is rebased in another worktree - 2016-04-22). But yeah that
> should fix it.
>
>> But I'm wondering why the worktree code does this. A bare repo isn't a
>> worktree and I think it shouldn't treat it as one. A patch that rips
>> out this feature and updates the tests to match would look like this:
>>
>>
>> diff --git a/builtin/worktree.c b/builtin/worktree.c
>> index 5c4854d..3600530 100644
>> --- a/builtin/worktree.c
>> +++ b/builtin/worktree.c
>> @@ -382,15 +382,11 @@ static int add(int ac, const char **av, const char *prefix)
>> static void show_worktree_porcelain(struct worktree *wt)
>> {
>> printf("worktree %s\n", wt->path);
>> - if (wt->is_bare)
>> - printf("bare\n");
>> - else {
>> - printf("HEAD %s\n", sha1_to_hex(wt->head_sha1));
>> - if (wt->is_detached)
>> - printf("detached\n");
>> - else
>> - printf("branch %s\n", wt->head_ref);
>> - }
>> + printf("HEAD %s\n", sha1_to_hex(wt->head_sha1));
>> + if (wt->is_detached)
>> + printf("detached\n");
>> + else
>> + printf("branch %s\n", wt->head_ref);
>> printf("\n");
>> }
>
> This goes back to the first very first commit of "git worktree list":
> bb9c03b (worktree: add 'list' command - 2015-10-08) and was sort of
> pointed out during review [1] but nobody answered it.
>
> A bare repo does not have an associated worktree. However only main
> worktree can be bare. If we take this out, "git worktree list"'s first
> line will no longer be about the main worktree (because it does not
> exist). That may cause trouble since we promised in "git-worktree.txt"
> that the main worktree is listed first. I don't think we have any way
> else to determine if the main worktree exists. Showing "bare" may be
> the way to see if we have a main worktree or not. So we probably want
> to keep this function unchanged.
>
> [1] https://public-inbox.org/git/%3CCANoM8SWeqxD2vWLQmEfxxxn8Dz4yPfjGOoOH=Azn1A3So+wz2Q@mail.gmail.com%3E/
> --
> Duy
--
Michael Tutty, CTO
e: mtutty@gforgegroup.com
t: @mtutty, @gforgegroup
v: 515-789-0772
w: http://gforgegroup.com, http://gforge.com
^ permalink raw reply
* Problem with submodules
From: venv21 @ 2016-10-09 14:41 UTC (permalink / raw)
To: git
Hi, I want to report a regression.
After cloning for example https://git.gnome.org/browse/epiphany with
git 2.10 and running ./autogen.sh I get the following errors:
http://pastebin.com/93AunRhu
The developer told me that it is probably not an issue caused by
epiphany and I should try an older git version. I installed 2.7.2 and
it works perfectly. So I think theres a bug in git 2.10.
Thanks and regards
^ permalink raw reply
* Re: Problem with submodules
From: Dennis Kaarsemaker @ 2016-10-09 17:51 UTC (permalink / raw)
To: venv21@gmail.com, git
In-Reply-To: <CAL4SumgJbrirymt5+iyNbpo++xXfzJZRiHDm8=0+eCArpCX=DA@mail.gmail.com>
On Sun, 2016-10-09 at 16:41 +0200, venv21@gmail.com wrote:
> Hi, I want to report a regression.
>
> After cloning for example https://git.gnome.org/browse/epiphany with
> git 2.10 and running ./autogen.sh I get the following errors:
> http://pastebin.com/93AunRhu
>
> The developer told me that it is probably not an issue caused by
> epiphany and I should try an older git version. I installed 2.7.2 and
> it works perfectly. So I think theres a bug in git 2.10.
I can't reproduce the problem with git 2.10.1
Can you share the .git/config file in your clone of epiphany please.
D.
^ permalink raw reply
* How to watch a mailing list & repo for patches which affect a certain area of code?
From: Ian Kelling @ 2016-10-09 19:03 UTC (permalink / raw)
To: git
I've got patches in various projects, and I don't have time to keep up
with the mailing list, but I'd like to help out with maintenance of that
code, or the functions/files it touches. People don't cc me. I figure I
could filter the list, test patches submitted, commits made, mentions of
files/functions, build filters based on the code I have in the repo even
if it's been moved or changed subsequently. I'm wondering what other
people have implemented already for automation around this, or general
thoughts. Web search is not showing me much.
^ permalink raw reply
* Re: Problem with submodules
From: venv21 @ 2016-10-09 19:15 UTC (permalink / raw)
To: Dennis Kaarsemaker; +Cc: git
In-Reply-To: <1476035501.26041.1.camel@kaarsemaker.net>
Sure, http://pastebin.com/bUFBDj0Q
^ permalink raw reply
* Re: Problem with submodules
From: venv21 @ 2016-10-09 19:22 UTC (permalink / raw)
To: Dennis Kaarsemaker; +Cc: git
In-Reply-To: <CAL4Sumj7r-RNXBi2Oh_=KAkcksM13sJjLzJEpszWXPXZ8hP6Kw@mail.gmail.com>
hm okay, it works with 2.10.0, when I remove the word 'epiphany' from
the urls in line 13 and 15
2016-10-09 21:15 GMT+02:00 venv21@gmail.com <venv21@gmail.com>:
> Sure, http://pastebin.com/bUFBDj0Q
^ permalink raw reply
* Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd
From: Jeff King @ 2016-10-09 20:58 UTC (permalink / raw)
To: Duy Nguyen
Cc: Johannes Schindelin, Johannes Sixt, Junio C Hamano,
Git Mailing List
In-Reply-To: <CACsJy8BpYYJmBm32YsQyuP58uhLE+sn8WdhiHyY6xzcqPVjMVQ@mail.gmail.com>
On Sun, Oct 09, 2016 at 06:32:38PM +0700, Duy Nguyen wrote:
> > If you mean ambiguity between the old "alias.X" and the new "alias.X.*",
> > then yes, I think that's an unavoidable part of the transition. IMHO,
> > the new should take precedence over the old, and people will gradually
> > move from one to the other.
>
> Do we really need to treat this differently than
>
> [alias]
> d2u = !dos2unix
> d2u = C:/cygwin/bin/dos3unix.exe
>
> ?
>
> Another similar case is one d2u (could be either old syntax or new) is
> defined in ~/.gitconfig and the other d2u in $GIT_DIR/config. In
> either case, the "latest" d2u definition wins.
Yeah, that's reasonable, too. So:
[alias]
d2u = "!dos2unix"
acts exactly as if:
[alias "d2u"]
command = dos2unix
type = shell
was specified at that point, which is easy to understand.
> > If you mean the ambiguity between alias.X.shell and alias.X.exec in your
> > example, the problem is that you have keys with overlapping meanings.
> > One solution is "don't do that" (so have a key like "cmd", and another
> > to select "shell or git-cmd", etc). Another is to define some rule, like
> > "last one wins" (so "exec" overrides "shell" in your example).
> [...]
>
> Any suggestion? I suppose we can have _one_ key for the command. How
> to execute that command (exec, shell, nocd...) are boolean options.
> People can still write conflicting things. We have been nice so far,
> always dying when the user specify conflicting command line options.
> We could do the same here, I guess.
Having separate exec/shell boolean options just punts the overlap from
the command key to those keys. If you have two mutually exclusive
options, I think the best thing is a single option, like:
type = <shell | exec | whatever>
and then it is obvious that a second appearance of "type" overrides an
earlier one, by our usual "last one wins" convention. As opposed to:
shell = true
exec = true
where you have to understand the meaning of each option to know that
"exec" overrides "shell".
-Peff
^ permalink raw reply
* RE: How to watch a mailing list & repo for patches which affect a certain area of code?
From: Jason Pyeron @ 2016-10-09 21:26 UTC (permalink / raw)
To: git; +Cc: 'Ian Kelling'
In-Reply-To: <1476039798.3060702.750483225.1DE6C48B@webmail.messagingengine.com>
> -----Original Message-----
> From: Ian Kelling
> Sent: Sunday, October 09, 2016 15:03
>
> I've got patches in various projects, and I don't have time to keep up
> with the mailing list, but I'd like to help out with
> maintenance of that
> code, or the functions/files it touches. People don't cc me.
> I figure I
> could filter the list, test patches submitted, commits made,
> mentions of
> files/functions, build filters based on the code I have in
> the repo even
> if it's been moved or changed subsequently. I'm wondering what other
> people have implemented already for automation around this, or general
> thoughts. Web search is not showing me much.
>
One thought would be to apply every patch automatically (to the branches of interest?). Then trigger on the [successful] changed
code. This would simplify the logic to working on the source only and not parsing the emails.
-Jason
^ permalink raw reply
* Re: %C(auto) not working as expected
From: Jeff King @ 2016-10-09 23:46 UTC (permalink / raw)
To: René Scharfe; +Cc: Tom Hale, git, Duy Nguyen, Junio C Hamano
In-Reply-To: <25c17e16-2456-7da3-ae22-2dc812a3aa0d@web.de>
On Sun, Oct 09, 2016 at 03:24:17PM +0200, René Scharfe wrote:
> Offering a way to enable terminal-detection for all color codes of a
> format would be useful, but using the existing "auto," prefix for that
> would be a behaviour change that could surprise users.
Yeah. In retrospect, it probably would have been saner to make %C(red) a
noop when --color is not in effect (either because of --no-color, or
more likely when --color=auto is in effect and stdout is not a
terminal). But that ship has long since sailed, I think.
If we do a revamp of the pretty-formats to bring them more in line with
ref-filter (e.g., something like "%(color:red)") maybe that would be an
opportunity to make minor adjustments. Though, hmm, it looks like
for-each-ref already knows "%(color:red)", and it's unconditional.
<sigh> So perhaps we would need to go through some deprecation period or
other transition.
> ---
> Documentation/pretty-formats.txt | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
> index a942d57..89e3bc6 100644
> --- a/Documentation/pretty-formats.txt
> +++ b/Documentation/pretty-formats.txt
> @@ -166,7 +166,8 @@ endif::git-rev-list[]
> - '%Cgreen': switch color to green
> - '%Cblue': switch color to blue
> - '%Creset': reset color
> -- '%C(...)': color specification, as described in color.branch.* config option;
> +- '%C(...)': color specification, as described under Values, color in the
> + "CONFIGURATION FILE" section of linkgit:git-config[1];
I like the intent here, though I found "Values, color" hard to parse (it
was not immediately clear that you mean "the color paragraph of the
Values section", as commas are already being used in that sentence for
the parenthetical phrase).
I'm not sure how to say that succinctly, as we are four levels deep
(git-config -> configuration file -> values -> color). Too bad there is
no good link syntax for it. Maybe:
...color specification, as described in linkgit:git-config[1] (see the
paragraph on colors in the "Values" section, under "CONFIGURATION
FILE")
or something.
-Peff
^ permalink raw reply
* Re: [PATCH] use strbuf_add_unique_abbrev() for adding short hashes, part 3
From: Jeff King @ 2016-10-10 0:00 UTC (permalink / raw)
To: René Scharfe; +Cc: Git List, Junio C Hamano
In-Reply-To: <c830e99d-4fc9-b001-cce3-38f66dc2c678@web.de>
On Sat, Oct 08, 2016 at 05:38:47PM +0200, René Scharfe wrote:
> Call strbuf_add_unique_abbrev() to add abbreviated hashes to strbufs
> instead of taking detours through find_unique_abbrev() and its static
> buffer. This is shorter in most cases and a bit more efficient.
>
> The changes here are not easily handled by a semantic patch because
> they involve removing temporary variables and deconstructing format
> strings for strbuf_addf().
Yeah, the thing to look for here is whether the sha1 variable holds the
same value at both times.
These all look OK to me. Mild rambling below.
> diff --git a/merge-recursive.c b/merge-recursive.c
> index 5200d5c..9041c2f 100644
> --- a/merge-recursive.c
> +++ b/merge-recursive.c
> @@ -202,9 +202,9 @@ static void output_commit_title(struct merge_options *o, struct commit *commit)
> strbuf_addf(&o->obuf, "virtual %s\n",
> merge_remote_util(commit)->name);
> else {
> - strbuf_addf(&o->obuf, "%s ",
> - find_unique_abbrev(commit->object.oid.hash,
> - DEFAULT_ABBREV));
> + strbuf_add_unique_abbrev(&o->obuf, commit->object.oid.hash,
> + DEFAULT_ABBREV);
> + strbuf_addch(&o->obuf, ' ');
I've often wondered whether a big strbuf_addf() is more efficient than
several strbuf_addstrs. It amortizes the size-checks, certainly, though
those are probably not very big. It shouldn't matter much for amortizing
the cost of malloc, as it's very unlikely to have a case where:
strbuf_addf("%s%s", foo, bar);
would require one malloc, but:
strbuf_addstr(foo);
strbuf_addstr(bar);
would require two (one of the strings would have to be around the same
size as the ALLOC_GROW() doubling).
So it probably doesn't matter much in practice (not that most of these
cases are very performance sensitive anyway). Mostly just something I've
pondered while tweaking strbuf invocations.
And anyway, in this case replacing find_unique_abbrev lets us write
directly into the final buffer rather than copying through a static
buffer, so it's almost certainly a win there.
> diff --git a/pretty.c b/pretty.c
> index 25efbca..0c31495 100644
> --- a/pretty.c
> +++ b/pretty.c
> @@ -544,15 +544,13 @@ static void add_merge_info(const struct pretty_print_context *pp,
> strbuf_addstr(sb, "Merge:");
>
> while (parent) {
> - struct commit *p = parent->item;
> - const char *hex = NULL;
> + struct object_id *oidp = &parent->item->object.oid;
> + strbuf_addch(sb, ' ');
> if (pp->abbrev)
> - hex = find_unique_abbrev(p->object.oid.hash, pp->abbrev);
> - if (!hex)
> - hex = oid_to_hex(&p->object.oid);
Wow, this existing code was hard to follow. I wondered when
find_unique_abbrev() would return NULL, but it never does. This "if
(!hex)" should have been an "else" all along.
> + strbuf_add_unique_abbrev(sb, oidp->hash, pp->abbrev);
> + else
> + strbuf_addstr(sb, oid_to_hex(oidp));
...which I see you changed here, though perhaps it is not immediately
obvious that it is correct without knowing find_unique_abbrev's return
value assumptions.
Just thinking aloud, I've also wondered if strbuf_addoid() would be
handy. We already have oid_to_hex_r(). In fact, you could write it
already as:
strbuf_add_unique_abbrev(sb, oidp->hash, 0);
but that is probably not helping maintainability. ;)
> diff --git a/submodule.c b/submodule.c
> index 2de06a3..476f60b 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -392,10 +392,9 @@ static void show_submodule_header(FILE *f, const char *path,
> }
>
> output_header:
> - strbuf_addf(&sb, "%s%sSubmodule %s %s..", line_prefix, meta, path,
> - find_unique_abbrev(one->hash, DEFAULT_ABBREV));
> - if (!fast_backward && !fast_forward)
> - strbuf_addch(&sb, '.');
> + strbuf_addf(&sb, "%s%sSubmodule %s ", line_prefix, meta, path);
> + strbuf_add_unique_abbrev(&sb, one->hash, DEFAULT_ABBREV);
> + strbuf_addstr(&sb, (fast_backward || fast_forward) ? ".." : "...");
Spelling out ".." and "..." as you do makes this much clearer, IMHO.
-Peff
^ permalink raw reply
* Re: git 2.10.1 test regression in t4014-format-patch.sh
From: Josh Triplett @ 2016-10-10 0:18 UTC (permalink / raw)
To: Jeremy Huddleston Sequoia; +Cc: gitster, git
In-Reply-To: <3CE95FD5-C5A9-4DE3-8B57-80B858769D1B@freedesktop.org>
On October 9, 2016 5:15:22 PM PDT, Jeremy Huddleston Sequoia <jeremyhu@freedesktop.org> wrote:
>Hey Josh,
>
>Hope you're doing well.
>
>I wanted to let you know that this patch of yours, which landed in git
>2.10.1, introduced some test failures, seen on macOS.
>
>Let me know if you need any additional information to track these down.
>
>Thanks,
>Jeremy
>
>
>not ok 65 - format-patch default signature
>#
># git format-patch --stdout -1 | tail -n 3 >output &&
># signature >expect &&
># test_cmp expect output
>#
>
>not ok 132 - format-patch --base
>#
># git checkout side &&
># git format-patch --stdout --base=HEAD~3 -1 | tail -n 7 >actual &&
># echo >expected &&
># echo "base-commit: $(git rev-parse HEAD~3)" >>expected &&
># echo "prerequisite-patch-id: $(git show --patch HEAD~2 | git
>patch-id --stable | awk "{print \$1}")" >>expected &&
># echo "prerequisite-patch-id: $(git show --patch HEAD~1 | git
>patch-id --stable | awk "{print \$1}")" >>expected &&
># signature >> expected &&
># test_cmp expected actual
>#
Can you run the test with the option to show the expected and actual strings?
Did the testsuite run with the wrong git somehow?
^ permalink raw reply
* git 2.10.1 test regression in t4014-format-patch.sh
From: Jeremy Huddleston Sequoia @ 2016-10-10 0:15 UTC (permalink / raw)
To: Josh Triplett; +Cc: gitster, git
[-- Attachment #1: Type: text/plain, Size: 2040 bytes --]
Hey Josh,
Hope you're doing well.
I wanted to let you know that this patch of yours, which landed in git 2.10.1, introduced some test failures, seen on macOS.
Let me know if you need any additional information to track these down.
Thanks,
Jeremy
not ok 65 - format-patch default signature
#
# git format-patch --stdout -1 | tail -n 3 >output &&
# signature >expect &&
# test_cmp expect output
#
not ok 132 - format-patch --base
#
# git checkout side &&
# git format-patch --stdout --base=HEAD~3 -1 | tail -n 7 >actual &&
# echo >expected &&
# echo "base-commit: $(git rev-parse HEAD~3)" >>expected &&
# echo "prerequisite-patch-id: $(git show --patch HEAD~2 | git patch-id --stable | awk "{print \$1}")" >>expected &&
# echo "prerequisite-patch-id: $(git show --patch HEAD~1 | git patch-id --stable | awk "{print \$1}")" >>expected &&
# signature >> expected &&
# test_cmp expected actual
#
commit 480871e09ed2e5275b4ba16b278681e5a8c122ae
Author: Josh Triplett <josh@joshtriplett.org>
Date: Wed Sep 7 18:12:01 2016 -0700
format-patch: show base info before email signature
Any text below the "-- " for the email signature gets treated as part of
the signature, and many mail clients will trim it from the quoted text
for a reply. Move it above the signature, so people can reply to it
more easily.
Similarly, when producing the patch as a MIME attachment, the
original code placed the base info after the attached part, which
would be discarded. Move the base info to the end of the part,
still inside the part boundary.
Add tests for the exact format of the email signature, and add tests
to ensure that the base info appears before the email signature when
producing a plain-text output, and that it appears before the part
boundary when producing a MIME attachment.
Signed-off-by: Josh Triplett <josh@joshtriplett.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 4465 bytes --]
^ permalink raw reply
* git 2.10.1 test regression in t3700-add.sh
From: Jeremy Huddleston Sequoia @ 2016-10-10 0:15 UTC (permalink / raw)
To: t.gummerer; +Cc: gitster, git
[-- Attachment #1: Type: text/plain, Size: 1311 bytes --]
Hi Thomas,
I wanted to let you know that this patch of yours, which landed in git 2.10.1, introduced some test failures, seen on macOS.
Let me know if you need any additional information to track these down.
Thanks,
Jeremy
not ok 40 - git add --chmod=[+-]x changes index with already added file
#
# echo foo >foo3 &&
# git add foo3 &&
# git add --chmod=+x foo3 &&
# test_mode_in_index 100755 foo3 &&
# echo foo >xfoo3 &&
# chmod 755 xfoo3 &&
# git add xfoo3 &&
# git add --chmod=-x xfoo3 &&
# test_mode_in_index 100644 xfoo3
#
commit 610d55af0f082f6b866dc858e144c03d8ed4424c
Author: Thomas Gummerer <t.gummerer@gmail.com>
Date: Wed Sep 14 22:07:47 2016 +0100
add: modify already added files when --chmod is given
When the chmod option was added to git add, it was hooked up to the diff
machinery, meaning that it only works when the version in the index
differs from the version on disk.
As the option was supposed to mirror the chmod option in update-index,
which always changes the mode in the index, regardless of the status of
the file, make sure the option behaves the same way in git add.
Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 4465 bytes --]
^ permalink raw reply
* Re: git 2.10.1 test regression in t3700-add.sh
From: Jeremy Huddleston Sequoia @ 2016-10-10 2:51 UTC (permalink / raw)
To: t.gummerer; +Cc: gitster, git
In-Reply-To: <A433322C-2B75-4B4B-A8CC-BAC3179EE7A7@freedesktop.org>
[-- Attachment #1: Type: text/plain, Size: 2237 bytes --]
> On Oct 9, 2016, at 17:15, Jeremy Huddleston Sequoia <jeremyhu@freedesktop.org> wrote:
>
> Hi Thomas,
>
> I wanted to let you know that this patch of yours, which landed in git 2.10.1, introduced some test failures, seen on macOS.
>
> Let me know if you need any additional information to track these down.
>
> Thanks,
> Jeremy
>
> not ok 40 - git add --chmod=[+-]x changes index with already added file
> #
> # echo foo >foo3 &&
> # git add foo3 &&
> # git add --chmod=+x foo3 &&
> # test_mode_in_index 100755 foo3 &&
> # echo foo >xfoo3 &&
> # chmod 755 xfoo3 &&
> # git add xfoo3 &&
> # git add --chmod=-x xfoo3 &&
> # test_mode_in_index 100644 xfoo3
> #
>
> commit 610d55af0f082f6b866dc858e144c03d8ed4424c
> Author: Thomas Gummerer <t.gummerer@gmail.com>
> Date: Wed Sep 14 22:07:47 2016 +0100
>
> add: modify already added files when --chmod is given
>
> When the chmod option was added to git add, it was hooked up to the diff
> machinery, meaning that it only works when the version in the index
> differs from the version on disk.
>
> As the option was supposed to mirror the chmod option in update-index,
> which always changes the mode in the index, regardless of the status of
> the file, make sure the option behaves the same way in git add.
>
> Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
This failure looks odd. I'll dig into it a bit more as it looks like something odd is going on here...
expecting success:
echo foo >foo3 &&
git add foo3 &&
git add --chmod=+x foo3 &&
test_mode_in_index 100755 foo3 &&
echo foo >xfoo3 &&
chmod 755 xfoo3 &&
git add xfoo3 &&
git add --chmod=-x xfoo3 &&
test_mode_in_index 100644 xfoo3
pass
cannot chmod 'xfoo3'fail
120000 c5c4ca97a3a080c32920941b665e94a997901491 0 xfoo3
not ok 40 - git add --chmod=[+-]x changes index with already added file
#
# echo foo >foo3 &&
# git add foo3 &&
# git add --chmod=+x foo3 &&
# test_mode_in_index 100755 foo3 &&
# echo foo >xfoo3 &&
# chmod 755 xfoo3 &&
# git add xfoo3 &&
# git add --chmod=-x xfoo3 &&
# test_mode_in_index 100644 xfoo3
#
[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 4465 bytes --]
^ permalink raw reply
* Re: git 2.10.1 test regression in t4014-format-patch.sh
From: Jeremy Huddleston Sequoia @ 2016-10-10 2:55 UTC (permalink / raw)
To: Josh Triplett; +Cc: gitster, git
In-Reply-To: <F069D0FD-C3CF-4F40-9338-D84E76DB310B@joshtriplett.org>
[-- Attachment #1: Type: text/plain, Size: 1833 bytes --]
> On Oct 9, 2016, at 17:18, Josh Triplett <josh@joshtriplett.org> wrote:
>
> On October 9, 2016 5:15:22 PM PDT, Jeremy Huddleston Sequoia <jeremyhu@freedesktop.org> wrote:
>> Hey Josh,
>>
>> Hope you're doing well.
>>
>> I wanted to let you know that this patch of yours, which landed in git
>> 2.10.1, introduced some test failures, seen on macOS.
>>
>> Let me know if you need any additional information to track these down.
>>
>> Thanks,
>> Jeremy
>>
>>
>> not ok 65 - format-patch default signature
>> #
>> # git format-patch --stdout -1 | tail -n 3 >output &&
>> # signature >expect &&
>> # test_cmp expect output
>> #
>>
>> not ok 132 - format-patch --base
>> #
>> # git checkout side &&
>> # git format-patch --stdout --base=HEAD~3 -1 | tail -n 7 >actual &&
>> # echo >expected &&
>> # echo "base-commit: $(git rev-parse HEAD~3)" >>expected &&
>> # echo "prerequisite-patch-id: $(git show --patch HEAD~2 | git
>> patch-id --stable | awk "{print \$1}")" >>expected &&
>> # echo "prerequisite-patch-id: $(git show --patch HEAD~1 | git
>> patch-id --stable | awk "{print \$1}")" >>expected &&
>> # signature >> expected &&
>> # test_cmp expected actual
>> #
>
> Can you run the test with the option to show the expected and actual strings?
> Did the testsuite run with the wrong git somehow?
Nope, it's the right version being tested. The failure seems due to your git_version change not liking our formatting
$ git --version
git version 2.10.1 (Apple Git-99)
(the 'Apple Git-XX' being added because this was from a build that had Apple's patch series, including https://github.com/jeremyhu/git/commit/f99905d0752d923e5ec61e14c675a300c6d04284)
We modify DEF_VER, which confused your regex. Simple patch inc in a separate email.
Thanks,
Jeremy
[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 4465 bytes --]
^ permalink raw reply
* Re: [PATCH] t4014-format-patch: Adjust git_version regex to better handle distro changes to DEF_VER
From: Josh Triplett @ 2016-10-10 3:04 UTC (permalink / raw)
To: Jeremy Huddleston Sequoia, git; +Cc: Junio C Hamano
In-Reply-To: <20161010025323.9415-1-jeremyhu@apple.com>
On October 9, 2016 7:53:23 PM PDT, Jeremy Huddleston Sequoia <jeremyhu@apple.com> wrote:
>Regressed-in: 480871e09ed2e5275b4ba16b278681e5a8c122ae
>Signed-off-by: Jeremy Huddleston Sequoia <jeremyhu@apple.com>
>CC: Josh Triplett <josh@joshtriplett.org>
>CC: Junio C Hamano <gitster@pobox.com>
Looks reasonable to me. Didn't realize git versions could have spaces.
Reviewed-by: Josh Triplett <josh@joshtriplett.org>
> t/t4014-format-patch.sh | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
>index 8d90a6e..33f6940 100755
>--- a/t/t4014-format-patch.sh
>+++ b/t/t4014-format-patch.sh
>@@ -754,7 +754,7 @@ test_expect_success 'format-patch
>--ignore-if-in-upstream HEAD' '
> git format-patch --ignore-if-in-upstream HEAD
> '
>
>-git_version="$(git --version | sed "s/.* //")"
>+git_version="$(git --version | sed "s/git version //")"
>
> signature() {
> printf "%s\n%s\n\n" "-- " "${1:-$git_version}"
^ permalink raw reply
* [PATCH] t4014-format-patch: Adjust git_version regex to better handle distro changes to DEF_VER
From: Jeremy Huddleston Sequoia @ 2016-10-10 2:53 UTC (permalink / raw)
To: git; +Cc: Jeremy Huddleston Sequoia, Josh Triplett, Junio C Hamano
Regressed-in: 480871e09ed2e5275b4ba16b278681e5a8c122ae
Signed-off-by: Jeremy Huddleston Sequoia <jeremyhu@apple.com>
CC: Josh Triplett <josh@joshtriplett.org>
CC: Junio C Hamano <gitster@pobox.com>
---
t/t4014-format-patch.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index 8d90a6e..33f6940 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -754,7 +754,7 @@ test_expect_success 'format-patch --ignore-if-in-upstream HEAD' '
git format-patch --ignore-if-in-upstream HEAD
'
-git_version="$(git --version | sed "s/.* //")"
+git_version="$(git --version | sed "s/git version //")"
signature() {
printf "%s\n%s\n\n" "-- " "${1:-$git_version}"
--
2.10.1 (Apple Git-99)
^ permalink raw reply related
* Re: git 2.10.1 test regression in t3700-add.sh
From: Jeremy Huddleston Sequoia @ 2016-10-10 3:22 UTC (permalink / raw)
To: t.gummerer; +Cc: gitster, git
In-Reply-To: <98EB348E-221D-488C-A466-1FACB1C768FF@freedesktop.org>
[-- Attachment #1: Type: text/plain, Size: 2573 bytes --]
The issue is that the whitespace before the filename in $(git ls-files -s "$2") is a tab, and test_mode_in_index only looks for a space.
><
> On Oct 9, 2016, at 19:51, Jeremy Huddleston Sequoia <jeremyhu@freedesktop.org> wrote:
>
>
>> On Oct 9, 2016, at 17:15, Jeremy Huddleston Sequoia <jeremyhu@freedesktop.org> wrote:
>>
>> Hi Thomas,
>>
>> I wanted to let you know that this patch of yours, which landed in git 2.10.1, introduced some test failures, seen on macOS.
>>
>> Let me know if you need any additional information to track these down.
>>
>> Thanks,
>> Jeremy
>>
>> not ok 40 - git add --chmod=[+-]x changes index with already added file
>> #
>> # echo foo >foo3 &&
>> # git add foo3 &&
>> # git add --chmod=+x foo3 &&
>> # test_mode_in_index 100755 foo3 &&
>> # echo foo >xfoo3 &&
>> # chmod 755 xfoo3 &&
>> # git add xfoo3 &&
>> # git add --chmod=-x xfoo3 &&
>> # test_mode_in_index 100644 xfoo3
>> #
>>
>> commit 610d55af0f082f6b866dc858e144c03d8ed4424c
>> Author: Thomas Gummerer <t.gummerer@gmail.com>
>> Date: Wed Sep 14 22:07:47 2016 +0100
>>
>> add: modify already added files when --chmod is given
>>
>> When the chmod option was added to git add, it was hooked up to the diff
>> machinery, meaning that it only works when the version in the index
>> differs from the version on disk.
>>
>> As the option was supposed to mirror the chmod option in update-index,
>> which always changes the mode in the index, regardless of the status of
>> the file, make sure the option behaves the same way in git add.
>>
>> Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
>> Signed-off-by: Junio C Hamano <gitster@pobox.com>
>
>
> This failure looks odd. I'll dig into it a bit more as it looks like something odd is going on here...
>
> expecting success:
> echo foo >foo3 &&
> git add foo3 &&
> git add --chmod=+x foo3 &&
> test_mode_in_index 100755 foo3 &&
> echo foo >xfoo3 &&
> chmod 755 xfoo3 &&
> git add xfoo3 &&
> git add --chmod=-x xfoo3 &&
> test_mode_in_index 100644 xfoo3
>
> pass
> cannot chmod 'xfoo3'fail
> 120000 c5c4ca97a3a080c32920941b665e94a997901491 0 xfoo3
> not ok 40 - git add --chmod=[+-]x changes index with already added file
> #
> # echo foo >foo3 &&
> # git add foo3 &&
> # git add --chmod=+x foo3 &&
> # test_mode_in_index 100755 foo3 &&
> # echo foo >xfoo3 &&
> # chmod 755 xfoo3 &&
> # git add xfoo3 &&
> # git add --chmod=-x xfoo3 &&
> # test_mode_in_index 100644 xfoo3
> #
>
[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 4465 bytes --]
^ permalink raw reply
* Re: git 2.10.1 test regression in t3700-add.sh
From: Jeremy Huddleston Sequoia @ 2016-10-10 3:46 UTC (permalink / raw)
To: t.gummerer; +Cc: gitster, git
In-Reply-To: <6CA5F44F-38C1-4C15-9B85-FE84FB93E616@freedesktop.org>
[-- Attachment #1: Type: text/plain, Size: 2967 bytes --]
> On Oct 9, 2016, at 20:22, Jeremy Huddleston Sequoia <jeremyhu@freedesktop.org> wrote:
>
> The issue is that the whitespace before the filename in $(git ls-files -s "$2") is a tab, and test_mode_in_index only looks for a space.
Actually, looks like that as just a rabbit hole. The real issue looks to be because an earlier test drops down xfoo3 as a symlink, which causes this test to fail due to the collision. I'll get out a patch in a bit.
>
>> <
>
>> On Oct 9, 2016, at 19:51, Jeremy Huddleston Sequoia <jeremyhu@freedesktop.org> wrote:
>>
>>
>>> On Oct 9, 2016, at 17:15, Jeremy Huddleston Sequoia <jeremyhu@freedesktop.org> wrote:
>>>
>>> Hi Thomas,
>>>
>>> I wanted to let you know that this patch of yours, which landed in git 2.10.1, introduced some test failures, seen on macOS.
>>>
>>> Let me know if you need any additional information to track these down.
>>>
>>> Thanks,
>>> Jeremy
>>>
>>> not ok 40 - git add --chmod=[+-]x changes index with already added file
>>> #
>>> # echo foo >foo3 &&
>>> # git add foo3 &&
>>> # git add --chmod=+x foo3 &&
>>> # test_mode_in_index 100755 foo3 &&
>>> # echo foo >xfoo3 &&
>>> # chmod 755 xfoo3 &&
>>> # git add xfoo3 &&
>>> # git add --chmod=-x xfoo3 &&
>>> # test_mode_in_index 100644 xfoo3
>>> #
>>>
>>> commit 610d55af0f082f6b866dc858e144c03d8ed4424c
>>> Author: Thomas Gummerer <t.gummerer@gmail.com>
>>> Date: Wed Sep 14 22:07:47 2016 +0100
>>>
>>> add: modify already added files when --chmod is given
>>>
>>> When the chmod option was added to git add, it was hooked up to the diff
>>> machinery, meaning that it only works when the version in the index
>>> differs from the version on disk.
>>>
>>> As the option was supposed to mirror the chmod option in update-index,
>>> which always changes the mode in the index, regardless of the status of
>>> the file, make sure the option behaves the same way in git add.
>>>
>>> Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
>>> Signed-off-by: Junio C Hamano <gitster@pobox.com>
>>
>>
>> This failure looks odd. I'll dig into it a bit more as it looks like something odd is going on here...
>>
>> expecting success:
>> echo foo >foo3 &&
>> git add foo3 &&
>> git add --chmod=+x foo3 &&
>> test_mode_in_index 100755 foo3 &&
>> echo foo >xfoo3 &&
>> chmod 755 xfoo3 &&
>> git add xfoo3 &&
>> git add --chmod=-x xfoo3 &&
>> test_mode_in_index 100644 xfoo3
>>
>> pass
>> cannot chmod 'xfoo3'fail
>> 120000 c5c4ca97a3a080c32920941b665e94a997901491 0 xfoo3
>> not ok 40 - git add --chmod=[+-]x changes index with already added file
>> #
>> # echo foo >foo3 &&
>> # git add foo3 &&
>> # git add --chmod=+x foo3 &&
>> # test_mode_in_index 100755 foo3 &&
>> # echo foo >xfoo3 &&
>> # chmod 755 xfoo3 &&
>> # git add xfoo3 &&
>> # git add --chmod=-x xfoo3 &&
>> # test_mode_in_index 100644 xfoo3
>> #
>>
>
[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 4465 bytes --]
^ 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