Re: git feature-branch
On 07/25/2014 11:10 AM, Sheldon Els wrote: It is just a shell script yes, and a man page to make things a bit more discoverable for git help feature-branch. The brew command comes from homebrew, a popular OSX package manager. My platform of choice. You might want to at least add these instructions are for people using macs. Otherwise it seems like you assume everyone is using macs, and nothing else exists in the world as far as you are concerned. Perhaps I can get support for an easy install for your platform. Do When I said more generic I meant it's just *one* shell script; put it somewhere on your $PATH. That should be sufficient for something like this (at the risk of going a bit off-topic for the list). you think a Makefile that installs to /usr/local/bin and /usr/local/share/man would fit, or are you on windows? Ouch. That hurt. On 25 July 2014 05:11, Sitaram Chamarty sitar...@gmail.com wrote: On 07/25/2014 03:45 AM, Sheldon Els wrote: Hi A small tool I wrote that is useful for some workflows. I thought it'd be worth sharing. https://github.com/sheldon/git-feature-branch/ As far as I can tell it's just a shell script; does it really need installation instructions, and if so can they not be more generic than brew install? Speaking for myself I have NO clue what that is. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: `ab | (cd cd git apply -)' fails with v2.0.0
Junio C Hamano venit, vidit, dixit 24.07.2014 19:19: Michael J Gruber g...@drmicha.warpmail.net writes: Steffen Nurpmeso venit, vidit, dixit 24.07.2014 15:29: Hello (again, psst, after a long time), it happened yesterday that i needed to do $ git diff HEAD:FILE COMMIT:SAME-FILE | (cd src git apply -) ... Ah little more context would help. Are you diffing files in the subdir src, or a file at the root which happens to be present in the subdir src as well? As the treeish:path form is not meant to produce git apply applicable patch in the first place, I am not sure what the OP is trying to achieve in the first place. Not just how many leading levels to strip? but which file is being modified? does not appear in a usable form. For example, here is what you would see: $ git diff HEAD:GIT-VERSION-GEN maint:GIT-VERSION-GEN diff --git a/HEAD:GIT-VERSION-GEN b/maint:GIT-VERSION-GEN index 40adbf7..0d1a86c 100755 --- a/HEAD:GIT-VERSION-GEN +++ b/maint:GIT-VERSION-GEN @@ -1,7 +1,7 @@ ... and neither HEAD:GIT-VERSION-GEN nor maint:GIT-VERSION-GEN is the file being modified (GIT-VERSION-GEN is). I thought git apply knows how to strip the rev part. I would understand if the upstream of the pipe were $ git diff HEAD maint -- GIT-VERSION-GEN | ... though. Needless to say, if the place cd goes is not a worktree controlled by git, then git apply would not know where the top-level of the target tree is, so even though the input with the corrected command on the upstream side of the pipe tells it which file is being modified, it needs to be told with the proper -pn parameter how many leading levels to strip. I think it's a common mistake to think of git apply as some sort of magic extension of patch which can do anything that patch does and more, and can be fed anything that git diff produces, figuring out by itself what to do with it :) Michael -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/5] checkout --to: no auto-detach if the ref is already checked out
Junio C Hamano venit, vidit, dixit 24.07.2014 23:30: Duy Nguyen pclo...@gmail.com writes: On Wed, Jul 23, 2014 at 8:48 PM, Michael J Gruber g...@drmicha.warpmail.net wrote: Nguyễn Thái Ngọc Duy venit, vidit, dixit 23.07.2014 13:43: + if (advice_checkout_to) + die(_(%s is already checked out at %s.\n + Either use --detach or -b together with --to + or switch branch in the the other checkout.), or switch to a different branch in the other checkout. But maybe we can be even more helpful, like: %s is already checked out at %s.\n Either checkout the detached head of branch %s using\n git checkout --detach --to %s %s\n or checkout a new branch based off %s using\n git checkout --to %s -b %s newbranch %s\n or switch to a different branch in the other checkout. if we can figure out the appropriate arguments at this point in the code. We would not be able to construct the exact command that the user has entered, so perhaps git checkout --detach more options %s git checkout -b new branch more options %s ? Note that this does not only occur when --to is given. If you have two checkouts associated to the same repo, git checkout foo on one checkout when foo is held by another checkout would cause the same error. I'll need to think of a better name than advice.checkoutTo. I am not sure if we need to say anything more than the first line of the message you have in your patch. To fork a new branch at the commit the user is interested in to check it out, or not bothering with the branch and detach, are both very normal parts of user's Git toolchest, nothing particularly special. As long as the most important point, i.e. in the new world order, unlike the contrib/workdir hack, you cannot check out the same branch at two different places, is clearly conveyed and understood, everything else should follow naturally, no? As an error message that is completely sufficient. The advice messages are meant to teach the user about the normal parts of the toolchest to use in a situation of conflict, aren't they? Maybe we should ask someone who hasn't turned them off... Michael -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/7] enforce `xfuncname` precedence over `funcname`
Tanay Abhra tanay...@gmail.com writes: On 7/25/2014 2:52 AM, Ramsay Jones wrote: However, I think it you could create a list of pointer to hash-table entry, string-list index pairs in the config_set and use that to do the iteration. A bit ugly, but it should work. Thanks for the advice, that is exactly what I am doing. I'd just replace list with array and use Documentation/technical/api-allocation-growing.txt. But I can't think of a better way. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-svn: Initialize SVN::Client with svn config instead of, auth for git svn branch.
Thanks for your reply, I updated commit message and subject, hoping it would be clearer. However I messed up message-id so it appear as a new message and not in the current thread. Sorry, still learning. Le 24/07/2014 00:33, Eric Wong a écrit : Monard Vong travelingsou...@gmail.com wrote: If a client certificate is required to connect to svn, git svn branch always prompt the user for the certificate location and password, even though those parameters are stored in svn file server located in svn config dir (generally ~/.subversion). On the opposite, git svn info/init/dcommit read the config dir and do not prompt if the parameters are set. This commit initializes for git svn branch, the SVN::Client with the 'config' option instead of 'auth'. As decribed in the SVN documentation, http://search.cpan.org/~mschwern/Alien-SVN-v1.7.17.1/src/subversion/subversion/bindings/swig/perl/native/Client.pm#METHODS the SVN::Client will then read cached authentication options. Signed-off-by: Monard Vong travelingsou...@gmail.com Thanks, I do not have a good way to test this, but the idea seems correct. Your patch is whitespace mangled, and the commit message and subject needs to be improved (see Documentation/SubmittingPatches on how to describe your changes and how to send them without whitespace mangling.) Thanks again. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/4] Consolidate ref parsing code
On Thu, Jul 24, 2014 at 4:16 AM, Junio C Hamano gits...@pobox.com wrote: - three places now knows what a textual symref looks like (i.e. begins with ref:, zero or more whitespaces, the target ref and then zero or more trailing whitespaces). Perhaps we need to consolidate the code further, so that this knowledge does not leak out of refs.c? I started on top of nd/multiple-work-trees but it conflicts badly with rs/ref-transaction-0 because this is basically code move. So I think we should make it a separate topic instead, based on latest 'master'. Junio, you still hit conflicts when merging this with nd/multiple-work-trees, but that's simpler to resolve (git_snpath - strbuf_git_path). I promise to replace the ref: code in checkout.c later when both topics graduate. So.. first cut. The end result looks nice. Nguyễn Thái Ngọc Duy (4): strbuf.c: keep errno in strbuf_read_file() refs.c: refactor resolve_ref_unsafe() to use strbuf internally refs.c: move ref parsing code out of resolve_ref() refs.c: rewrite resolve_gitlink_ref() to use parse_ref() cache.h | 12 +++ refs.c | 332 --- strbuf.c | 7 +- 3 files changed, 188 insertions(+), 163 deletions(-) -- 1.9.1.346.ga2b5940 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/4] refs.c: refactor resolve_ref_unsafe() to use strbuf internally
In the beginning, we had resolve_ref() that returns a buffer owned by
this function. Then we started to move away from that direction because
the buffer could be overwritten by the next resolve_ref() call and
introduced two new functions: resolve_ref_unsafe() and resolve_refdup().
The static buffer is still kept internally.
This patch makes the core of resolve_ref use a strbuf instead of static
buffer. Which makes resolve_refdup() more efficient (no need to copy
from the static buffer to a new buffer). It also removes the (random?)
256 char limit. In future, resolve_ref() could be used directly without
going through resolve_refdup() wrapper.
A minor bonus. resolve_ref(dup) are now more thread-friendly (although I'm
not 100% sure if they are thread-safe yet).
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
cache.h | 1 +
refs.c | 122 +++-
2 files changed, 68 insertions(+), 55 deletions(-)
diff --git a/cache.h b/cache.h
index fcb511d..5ffbafb 100644
--- a/cache.h
+++ b/cache.h
@@ -1002,6 +1002,7 @@ extern int read_ref(const char *refname, unsigned char
*sha1);
*/
extern const char *resolve_ref_unsafe(const char *ref, unsigned char *sha1,
int reading, int *flag);
extern char *resolve_refdup(const char *ref, unsigned char *sha1, int reading,
int *flag);
+extern int resolve_ref(const char *refname, struct strbuf *result, unsigned
char *sha1, int reading, int *flag);
extern int dwim_ref(const char *str, int len, unsigned char *sha1, char **ref);
extern int dwim_log(const char *str, int len, unsigned char *sha1, char **ref);
diff --git a/refs.c b/refs.c
index 84b9070..bec2bb1 100644
--- a/refs.c
+++ b/refs.c
@@ -1506,10 +1506,10 @@ static struct ref_entry *get_packed_ref(const char
*refname)
* A loose ref file doesn't exist; check for a packed ref. The
* options are forwarded from resolve_safe_unsafe().
*/
-static const char *handle_missing_loose_ref(const char *refname,
- unsigned char *sha1,
- int reading,
- int *flag)
+static int handle_missing_loose_ref(const char *refname,
+ unsigned char *sha1,
+ int reading,
+ int *flag)
{
struct ref_entry *entry;
@@ -1522,45 +1522,53 @@ static const char *handle_missing_loose_ref(const char
*refname,
hashcpy(sha1, entry-u.value.sha1);
if (flag)
*flag |= REF_ISPACKED;
- return refname;
+ return 0;
}
/* The reference is not a packed reference, either. */
if (reading) {
- return NULL;
+ return -1;
} else {
hashclr(sha1);
- return refname;
+ return 0;
}
}
-/* This function needs to return a meaningful errno on failure */
-const char *resolve_ref_unsafe(const char *refname, unsigned char *sha1, int
reading, int *flag)
+/*
+ * 'result' content will be destroyed. Its value may be undefined if
+ * resolve_ref returns -1.
+ *
+ * This function needs to return a meaningful errno on failure
+ */
+int resolve_ref(const char *refname, struct strbuf *result,
+ unsigned char *sha1, int reading, int *flag)
{
+ struct strbuf buffer = STRBUF_INIT;
int depth = MAXDEPTH;
- ssize_t len;
- char buffer[256];
- static char refname_buffer[256];
+ int ret = -1;
if (flag)
*flag = 0;
if (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL)) {
errno = EINVAL;
- return NULL;
+ return -1;
}
+ strbuf_reset(result);
+ strbuf_addstr(result, refname);
+
for (;;) {
char path[PATH_MAX];
+ const char *ref = result-buf;
struct stat st;
- char *buf;
- int fd;
+ const char *buf;
if (--depth 0) {
errno = ELOOP;
- return NULL;
+ break;
}
- git_snpath(path, sizeof(path), %s, refname);
+ git_snpath(path, sizeof(path), %s, ref);
/*
* We might have to loop back here to avoid a race
@@ -1574,27 +1582,25 @@ const char *resolve_ref_unsafe(const char *refname,
unsigned char *sha1, int rea
stat_ref:
if (lstat(path, st) 0) {
if (errno == ENOENT)
- return handle_missing_loose_ref(refname, sha1,
- reading, flag);
- else
- return NULL;
+ ret =
[PATCH 4/4] refs.c: rewrite resolve_gitlink_ref() to use parse_ref()
resolve_gitlink_ref_recursive() is less strict than the old
resolve_ref_unsafe() (which is now parse_ref()). It also has another
random limit 128 bytes for symrefs.
This brings MAXREFLEN check to resolve_ref* family. It looks like an
arbitrary limit though (added in 0ebde32 (Add 'resolve_gitlink_ref()'
helper function - 2007-04-09)
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
refs.c | 68 +-
1 file changed, 26 insertions(+), 42 deletions(-)
diff --git a/refs.c b/refs.c
index 2769f20..24503e5 100644
--- a/refs.c
+++ b/refs.c
@@ -1436,48 +1436,11 @@ static int resolve_gitlink_packed_ref(struct ref_cache
*refs,
return 0;
}
-static int resolve_gitlink_ref_recursive(struct ref_cache *refs,
-const char *refname, unsigned char
*sha1,
-int recursion)
-{
- int fd, len;
- char buffer[128], *p;
- char *path;
-
- if (recursion MAXDEPTH || strlen(refname) MAXREFLEN)
- return -1;
- path = *refs-name
- ? git_path_submodule(refs-name, %s, refname)
- : git_path(%s, refname);
- fd = open(path, O_RDONLY);
- if (fd 0)
- return resolve_gitlink_packed_ref(refs, refname, sha1);
-
- len = read(fd, buffer, sizeof(buffer)-1);
- close(fd);
- if (len 0)
- return -1;
- while (len isspace(buffer[len-1]))
- len--;
- buffer[len] = 0;
-
- /* Was it a detached head or an old-fashioned symlink? */
- if (!get_sha1_hex(buffer, sha1))
- return 0;
-
- /* Symref? */
- if (strncmp(buffer, ref:, 4))
- return -1;
- p = buffer + 4;
- while (isspace(*p))
- p++;
-
- return resolve_gitlink_ref_recursive(refs, p, sha1, recursion+1);
-}
-
int resolve_gitlink_ref(const char *path, const char *refname, unsigned char
*sha1)
{
- int len = strlen(path), retval;
+ struct strbuf result = STRBUF_INIT;
+ int len = strlen(path), retval = 0;
+ int depth = MAXDEPTH;
char *submodule;
struct ref_cache *refs;
@@ -1489,8 +1452,24 @@ int resolve_gitlink_ref(const char *path, const char
*refname, unsigned char *sh
refs = get_ref_cache(submodule);
free(submodule);
- retval = resolve_gitlink_ref_recursive(refs, refname, sha1, 0);
- return retval;
+ strbuf_addstr(result, refname);
+ while (!retval) {
+ if (--depth 0) {
+ errno = ELOOP;
+ retval = -1;
+ break;
+ }
+ path = *refs-name
+ ? git_path_submodule(refs-name, %s, result.buf)
+ : git_path(%s, result.buf);
+ retval = parse_ref(path, result, sha1, NULL);
+ if (retval == -2) {
+ retval = resolve_gitlink_packed_ref(refs, result.buf,
sha1);
+ retval = retval ? -1 : 1;
+ }
+ }
+ strbuf_release(result);
+ return retval 0 ? 0 : -1;
}
/*
@@ -1540,6 +1519,11 @@ int parse_ref(const char *path, struct strbuf *ref,
struct stat st;
const char *buf;
+ if (ref-len MAXREFLEN) {
+ errno = ENAMETOOLONG;
+ return -1;
+ }
+
/*
* We might have to loop back here to avoid a race condition:
* first we lstat() the file, then we try to read it as a link
--
1.9.1.346.ga2b5940
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/4] refs.c: move ref parsing code out of resolve_ref()
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
cache.h | 11
refs.c | 204 ++--
2 files changed, 120 insertions(+), 95 deletions(-)
diff --git a/cache.h b/cache.h
index 5ffbafb..40a63d9 100644
--- a/cache.h
+++ b/cache.h
@@ -1003,6 +1003,17 @@ extern int read_ref(const char *refname, unsigned char
*sha1);
extern const char *resolve_ref_unsafe(const char *ref, unsigned char *sha1,
int reading, int *flag);
extern char *resolve_refdup(const char *ref, unsigned char *sha1, int reading,
int *flag);
extern int resolve_ref(const char *refname, struct strbuf *result, unsigned
char *sha1, int reading, int *flag);
+/*
+ * Given a ref in ref and its path, returns
+ *
+ * -2 failed to open with ENOENT, the caller is responsible for
+ * checking missing loose ref (see resolve_ref for example)
+ * -1 if there's an error, ref can no longer be trusted, flag may
+ * be set. errno is valid.
+ * 0 this is a symref, ref now contains the linked ref
+ * +1 normal ref, sha1 is valid
+ */
+extern int parse_ref(const char *path, struct strbuf *ref, unsigned char
*sha1, int *flag);
extern int dwim_ref(const char *str, int len, unsigned char *sha1, char **ref);
extern int dwim_log(const char *str, int len, unsigned char *sha1, char **ref);
diff --git a/refs.c b/refs.c
index bec2bb1..2769f20 100644
--- a/refs.c
+++ b/refs.c
@@ -1533,6 +1533,105 @@ static int handle_missing_loose_ref(const char *refname,
}
}
+int parse_ref(const char *path, struct strbuf *ref,
+ unsigned char *sha1, int *flag)
+{
+ struct strbuf buffer = STRBUF_INIT;
+ struct stat st;
+ const char *buf;
+
+ /*
+* We might have to loop back here to avoid a race condition:
+* first we lstat() the file, then we try to read it as a link
+* or as a file. But if somebody changes the type of the file
+* (file - directory - symlink) between the lstat() and
+* reading, then we don't want to report that as an error but
+* rather try again starting with the lstat().
+*/
+stat_ref:
+ if (lstat(path, st) 0)
+ return errno == ENOENT ? -2 : -1;
+
+ /* Follow normalized - ie refs/.. symlinks by hand */
+ if (S_ISLNK(st.st_mode)) {
+ struct strbuf new_path = STRBUF_INIT;
+ if (strbuf_readlink(new_path, path, 256) 0) {
+ strbuf_release(new_path);
+ if (errno == ENOENT || errno == EINVAL)
+ /* inconsistent with lstat; retry */
+ goto stat_ref;
+ else
+ return -1;
+ }
+ if (starts_with(new_path.buf, refs/)
+ !check_refname_format(new_path.buf, 0)) {
+ strbuf_reset(ref);
+ strbuf_addbuf(ref, new_path);
+ if (flag)
+ *flag |= REF_ISSYMREF;
+ strbuf_release(new_path);
+ return 0;
+ }
+ strbuf_release(new_path);
+ }
+
+ /* Is it a directory? */
+ if (S_ISDIR(st.st_mode)) {
+ errno = EISDIR;
+ return -1;
+ }
+
+ /*
+* Anything else, just open it and try to use it as
+* a ref
+*/
+ if (strbuf_read_file(buffer, path, 256) 0) {
+ strbuf_release(buffer);
+ if (errno == ENOENT)
+ /* inconsistent with lstat; retry */
+ goto stat_ref;
+ else
+ return -1;
+ }
+ strbuf_rtrim(buffer);
+
+ /*
+* Is it a symbolic ref?
+*/
+ if (!skip_prefix(buffer.buf, ref:, buf)) {
+ int ret;
+ /*
+* Please note that FETCH_HEAD has a second line
+* containing other data.
+*/
+ if (get_sha1_hex(buffer.buf, sha1) ||
+ (buffer.buf[40] != '\0' !isspace(buffer.buf[40]))) {
+ if (flag)
+ *flag |= REF_ISBROKEN;
+ errno = EINVAL;
+ ret = -1;
+ } else
+ ret = 1;
+ strbuf_release(buffer);
+ return ret;
+ }
+ if (flag)
+ *flag |= REF_ISSYMREF;
+ while (isspace(*buf))
+ buf++;
+ if (check_refname_format(buf, REFNAME_ALLOW_ONELEVEL)) {
+ if (flag)
+ *flag |= REF_ISBROKEN;
+ strbuf_release(buffer);
+ errno = EINVAL;
+ return -1;
+ }
+ strbuf_reset(ref);
+ strbuf_addstr(ref, buf);
+ strbuf_release(buffer);
+ return 0;
+}
+
/*
* 'result' content
[PATCH 1/4] strbuf.c: keep errno in strbuf_read_file()
This function is used to replaced some code in the next patch that
does this (i.e. keep the errno when read() fails)
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
strbuf.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/strbuf.c b/strbuf.c
index 33018d8..61d685d 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -454,15 +454,18 @@ int strbuf_getwholeline_fd(struct strbuf *sb, int fd, int
term)
int strbuf_read_file(struct strbuf *sb, const char *path, size_t hint)
{
- int fd, len;
+ int fd, len, saved_errno;
fd = open(path, O_RDONLY);
if (fd 0)
return -1;
len = strbuf_read(sb, fd, hint);
+ saved_errno = errno;
close(fd);
- if (len 0)
+ if (len 0) {
+ errno = saved_errno;
return -1;
+ }
return len;
}
--
1.9.1.346.ga2b5940
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
MDaemon Notification -- Attachment Removed
? ? ? ??, ??? ???: From : nore...@vger.kernel.org To: git@vger.kernel.org Subject : Returned mail: Data format error Message-ID: ?? ???: - mail.zip (mail.doc .com) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: `ab | (cd cd git apply -)' fails with v2.0.0
Hello and good morning, Michael J Gruber g...@drmicha.warpmail.net wrote: |Junio C Hamano venit, vidit, dixit 24.07.2014 19:19: | Michael J Gruber g...@drmicha.warpmail.net writes: | Steffen Nurpmeso venit, vidit, dixit 24.07.2014 15:29: | Hello (again, psst, after a long time), | | it happened yesterday that i needed to do | | $ git diff HEAD:FILE COMMIT:SAME-FILE | | (cd src git apply -) | ... | | Ah little more context would help. Are you diffing files in the subdir | src, or a file at the root which happens to be present in the subdir src | as well? | | As the treeish:path form is not meant to produce git apply | applicable patch in the first place, I am not sure what the OP is | trying to achieve in the first place. Not just how many leading | levels to strip? but which file is being modified? does not | appear in a usable form. For example, here is what you would see: | | $ git diff HEAD:GIT-VERSION-GEN maint:GIT-VERSION-GEN | diff --git a/HEAD:GIT-VERSION-GEN b/maint:GIT-VERSION-GEN | index 40adbf7..0d1a86c 100755 | --- a/HEAD:GIT-VERSION-GEN | +++ b/maint:GIT-VERSION-GEN | @@ -1,7 +1,7 @@ | ... | | and neither HEAD:GIT-VERSION-GEN nor maint:GIT-VERSION-GEN is | the file being modified (GIT-VERSION-GEN is). | |I thought git apply knows how to strip the rev part. That would brighten the sky of the glorious future. Perfect! | I would understand if the upstream of the pipe were | | $ git diff HEAD maint -- GIT-VERSION-GEN | ... | | though. Yes, in this case it applies the patch. | Needless to say, if the place cd goes is not a worktree controlled | by git, then git apply would not know where the top-level of the | target tree is, so even though the input with the corrected command | on the upstream side of the pipe tells it which file is being | modified, it needs to be told with the proper -pn parameter how | many leading levels to strip. | |I think it's a common mistake to think of git apply as some sort of |magic extension of patch which can do anything that patch does and |more, and can be fed anything that git diff produces, figuring out by |itself what to do with it :) This was indeed my mistake. But regardless i think the current behaviour sucks: ?0[steffen@sherwood x.git]$ git diff HEAD:XY 5d0d74:XY | (cd src patch -p4) can't find file to patch at input line 5 Perhaps you used the wrong -p or --strip option? The text leading up to this was: ?130[steffen@sherwood x.git]$ git diff HEAD:XY 5d0d74:XY | (cd src git apply -p4) ?0[steffen@sherwood x.git]$ and ?0[steffen@sherwood groff.git]$ git diff HEAD:XY 5d0d74:XY | (cd src git apply -p2) ?0[steffen@sherwood groff.git]$ git diff HEAD:XY 5d0d74:XY | (cd src patch -p2) patching file XY ?0[steffen@sherwood groff.git]$ The number after `?' is the exit status of the last command, btw. Ciao (and yes, thanks a lot for git(1)!) --steffen -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Jul 2014, #04; Tue, 22)
On Wed, Jul 23, 2014 at 9:17 PM, Karsten Blees karsten.bl...@gmail.com wrote: With the version in pu, three tests fail. t7001 is fixed with a newer 'cp'. The other two are unrelated (introduced by nd/multiple-work-trees topic). * t1501-worktree: failed 1 As of 5bbcb072 setup.c: support multi-checkout repo setup Using $TRASH_DIRECTORY doesn't work on Windows. * t2026-prune-linked-checkouts: failed 1 As of 404a45f1 prune: strategies for linked checkouts Dito. I need your help here. Would saving $(pwd) to a variable and using it instead of $TRASH_DIRECTORY work? Some tests cd around and $(pwd) may not be the same as $TRASH_DIRECTORY. -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello, how are you?
Good afternoon, dear! Are you tired of being alone? You'll know how dreams come true All what you need is here Visit our website, this is something very special! http://ta.gg/6sk -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 2/6] add line number and file name info to `config_set`
Store file name and line number for each key-value pair in the cache
during parsing of the configuration files.
Signed-off-by: Tanay Abhra tanay...@gmail.com
---
config.c | 19 ++-
1 file changed, 18 insertions(+), 1 deletion(-)
diff --git a/config.c b/config.c
index 06257d9..110f9a5 100644
--- a/config.c
+++ b/config.c
@@ -1232,6 +1232,11 @@ int git_config_with_options(config_fn_t fn, void *data,
return ret;
}
+struct key_value_info {
+ const char *filename;
+ int linenr;
+};
+
int git_config(config_fn_t fn, void *data)
{
return git_config_with_options(fn, data, NULL, 1);
@@ -1262,6 +1267,9 @@ static struct config_set_element
*configset_find_element(struct config_set *cs,
static int configset_add_value(struct config_set *cs, const char *key, const
char *value)
{
struct config_set_element *e;
+ struct string_list_item *si;
+ struct key_value_info *kv_info = xmalloc(sizeof(*kv_info));
+
e = configset_find_element(cs, key);
/*
* Since the keys are being fed by git_config*() callback mechanism,
they
@@ -1274,7 +1282,16 @@ static int configset_add_value(struct config_set *cs,
const char *key, const cha
string_list_init(e-value_list, 1);
hashmap_add(cs-config_hash, e);
}
- string_list_append_nodup(e-value_list, value ? xstrdup(value) : NULL);
+ si = string_list_append_nodup(e-value_list, value ? xstrdup(value) :
NULL);
+ if (cf) {
+ kv_info-filename = strintern(cf-name);
+ kv_info-linenr = cf-linenr;
+ } else {
+ /* for values read from `git_config_from_parameters()` */
+ kv_info-filename = strintern(parameter);
+ kv_info-linenr = 0;
+ }
+ si-util = kv_info;
return 0;
}
--
1.9.0.GIT
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/6] config.c: fix accuracy of line number in errors
From: Matthieu Moy matthieu@grenoble-inp.fr
If a callback returns a negative value to `git_config*()` family,
they call `die()` while printing the line number and the file name.
Currently the printed line number is off by one, thus printing the
wrong line number.
Make `linenr` point to the line we just parsed during the call
to callback to get accurate line number in error messages.
Commit-message-by: Tanay Abhra tanay...@gmail.com
Signed-off-by: Tanay Abhra tanay...@gmail.com
---
config.c | 12 +++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/config.c b/config.c
index 0d799e0..06257d9 100644
--- a/config.c
+++ b/config.c
@@ -244,6 +244,7 @@ static int get_next_char(void)
cf-linenr++;
if (c == EOF) {
cf-eof = 1;
+ cf-linenr++;
c = '\n';
}
return c;
@@ -319,6 +320,7 @@ static int get_value(config_fn_t fn, void *data, struct
strbuf *name)
{
int c;
char *value;
+ int ret;
/* Get the full name */
for (;;) {
@@ -341,7 +343,15 @@ static int get_value(config_fn_t fn, void *data, struct
strbuf *name)
if (!value)
return -1;
}
- return fn(name-buf, value, data);
+ /*
+* We already consumed the \n, but we need linenr to point to
+* the line we just parsed during the call to fn to get
+* accurate line number in error messages.
+*/
+ cf-linenr--;
+ ret = fn(name-buf, value, data);
+ cf-linenr++;
+ return ret;
}
static int get_extended_base_var(struct strbuf *name, int c)
--
1.9.0.GIT
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 0/6] Rewrite `git_config()` using config-set API
[PATCH V2]: All the suggestions in [2] incorporated. git_config() now follows correct parsing order. Reordered the patches. Removed xfuncname patch as it was unnecssary. This series builds on the top of 1d2856f (ta/config-set) in pu or topic[1] in the mailing list with name git config cache special querying API utilizing the cache. This series aims to do these three things, * Use the config-set API to rewrite git_config(). * Solve any legacy bugs in the previous system while at it. * To be feature complete compared to the previous git_config() implementation, which I think it is now. (added the line number and file name info just for completeness) Also, I haven't yet checked the exact improvements but still as a teaser, git status now only rereads the configuration files twice instead of four times. [1]: http://thread.gmane.org/gmane.comp.version-control.git/253862 [2]: http://thread.gmane.org/gmane.comp.version-control.git/254101 Tanay Abhra (6): config.c: fix accuracy of line number in errors add line number and file name info to `config_set` rewrite git_config() to use the config-set API add a test for semantic errors in config files config: add `git_die_config()` to the config-set API Add tests for `git_config_get_string()` Documentation/technical/api-config.txt | 5 ++ cache.h| 26 +++ config.c | 135 - t/t1308-config-set.sh | 20 + test-config.c | 10 +++ 5 files changed, 175 insertions(+), 21 deletions(-) -- 1.9.0.GIT -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 4/6] add a test for semantic errors in config files
Semantic errors (for example, for alias.* variables NULL values are not allowed) in configuration files cause a die printing the line number and file name of the offending value. Add a test documenting that such errors cause a die printing the accurate line number and file name. Signed-off-by: Tanay Abhra tanay...@gmail.com --- t/t1308-config-set.sh | 11 +++ 1 file changed, 11 insertions(+) diff --git a/t/t1308-config-set.sh b/t/t1308-config-set.sh index 7fdf840..35c6ee2 100755 --- a/t/t1308-config-set.sh +++ b/t/t1308-config-set.sh @@ -197,4 +197,15 @@ test_expect_success 'proper error on error in custom config files' ' test_cmp expect actual ' +test_expect_success 'check line errors for malformed values' ' + mv .git/config .git/config.old + test_when_finished mv .git/config.old .git/config + cat .git/config -\EOF + [alias] + br + EOF + test_expect_code 128 git br 2result + grep fatal: bad config file line 2 in .git/config result +' + test_done -- 1.9.0.GIT -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 3/6] rewrite git_config() to use the config-set API
Of all the functions in `git_config*()` family, `git_config()` has the
most invocations in the whole code base. Each `git_config()` invocation
causes config file rereads which can be avoided using the config-set API.
Use the config-set API to rewrite `git_config()` to use the config caching
layer to avoid config file rereads on each invocation during a git process
lifetime. First invocation constructs the cache, and after that for each
successive invocation, `git_config()` feeds values from the config cache
instead of rereading the configuration files.
Signed-off-by: Tanay Abhra tanay...@gmail.com
---
cache.h | 25 +
config.c | 60 +++-
2 files changed, 76 insertions(+), 9 deletions(-)
diff --git a/cache.h b/cache.h
index 7292aef..11ded5a 100644
--- a/cache.h
+++ b/cache.h
@@ -8,6 +8,7 @@
#include gettext.h
#include convert.h
#include trace.h
+#include string-list.h
#include SHA1_HEADER
#ifndef git_SHA_CTX
@@ -1351,9 +1352,33 @@ extern int parse_config_key(const char *var,
const char **subsection, int *subsection_len,
const char **key);
+struct config_set_element {
+ struct hashmap_entry ent;
+ char *key;
+ struct string_list value_list;
+};
+
+struct configset_list_item {
+ struct config_set_element *e;
+ int value_index;
+};
+
+/*
+ * the contents of the list are ordered according to their
+ * position in the config files and order of parsing the files.
+ * (i.e. key-value pair at the last position of .git/config will
+ * be at the last item of the list)
+ */
+
+struct configset_list {
+ struct configset_list_item *items;
+ unsigned int nr, alloc;
+};
+
struct config_set {
struct hashmap config_hash;
int hash_initialized;
+ struct configset_list list;
};
extern void git_configset_init(struct config_set *cs);
diff --git a/config.c b/config.c
index 110f9a5..aa5c0ad 100644
--- a/config.c
+++ b/config.c
@@ -35,12 +35,6 @@ struct config_source {
long (*do_ftell)(struct config_source *c);
};
-struct config_set_element {
- struct hashmap_entry ent;
- char *key;
- struct string_list value_list;
-};
-
static struct config_source *cf;
static int zlib_compression_seen;
@@ -1237,11 +1231,44 @@ struct key_value_info {
int linenr;
};
-int git_config(config_fn_t fn, void *data)
+static int git_config_raw(config_fn_t fn, void *data)
{
return git_config_with_options(fn, data, NULL, 1);
}
+static int configset_iter(struct config_set *cs, config_fn_t fn, void *data)
+{
+ int i, value_index;
+ struct string_list *values;
+ struct config_set_element *entry;
+ struct configset_list *list = cs-list;
+ struct key_value_info *kv_info;
+
+ for (i = 0; i list-nr; i++) {
+ entry = list-items[i].e;
+ value_index = list-items[i].value_index;
+ values = entry-value_list;
+ if (fn(entry-key, values-items[value_index].string, data)
0) {
+ kv_info = values-items[value_index].util;
+ if (!kv_info-linenr)
+ die(unable to parse command-line config);
+ else
+ die(bad config file line %d in %s,
+ kv_info-linenr,
+ kv_info-filename);
+ }
+ }
+ return 0;
+}
+
+static void git_config_check_init(void);
+
+int git_config(config_fn_t fn, void *data)
+{
+ git_config_check_init();
+ return configset_iter(the_config_set, fn, data);
+}
+
static struct config_set_element *configset_find_element(struct config_set
*cs, const char *key)
{
struct config_set_element k;
@@ -1268,6 +1295,7 @@ static int configset_add_value(struct config_set *cs,
const char *key, const cha
{
struct config_set_element *e;
struct string_list_item *si;
+ struct configset_list_item *l_item;
struct key_value_info *kv_info = xmalloc(sizeof(*kv_info));
e = configset_find_element(cs, key);
@@ -1283,6 +1311,13 @@ static int configset_add_value(struct config_set *cs,
const char *key, const cha
hashmap_add(cs-config_hash, e);
}
si = string_list_append_nodup(e-value_list, value ? xstrdup(value) :
NULL);
+
+ if (cs-list.nr + 1 cs-list.alloc)
+ ALLOC_GROW(cs-list.items, cs-list.nr + 20, cs-list.alloc);
+ l_item = cs-list.items[cs-list.nr++];
+ l_item-e = e;
+ l_item-value_index = e-value_list.nr - 1;
+
if (cf) {
kv_info-filename = strintern(cf-name);
kv_info-linenr = cf-linenr;
@@ -1306,6 +1341,9 @@ void git_configset_init(struct config_set *cs)
{
hashmap_init(cs-config_hash, (hashmap_cmp_fn)config_set_element_cmp,
0);
[PATCH v2 6/6] Add tests for `git_config_get_string()`
Add tests for `git_config_get_string()`, check whether it
dies printing the line number and the file name if a NULL
value is retrieved for the given key.
Signed-off-by: Tanay Abhra tanay...@gmail.com
---
t/t1308-config-set.sh | 9 +
test-config.c | 10 ++
2 files changed, 19 insertions(+)
diff --git a/t/t1308-config-set.sh b/t/t1308-config-set.sh
index 35c6ee2..d7cdc6e 100755
--- a/t/t1308-config-set.sh
+++ b/t/t1308-config-set.sh
@@ -119,6 +119,15 @@ test_expect_success 'find integer value for a key' '
check_config get_int lamb.chop 65
'
+test_expect_success 'find string value for a key' '
+ check_config get_string case.baz hask
+'
+
+test_expect_success 'check line error when NULL string is queried' '
+ test_expect_code 128 test-config get_string case.foo 2result
+ grep fatal: bad config file line 7 in .git/config result
+'
+
test_expect_success 'find integer if value is non parse-able' '
check_config expect_code 128 get_int lamb.head
'
diff --git a/test-config.c b/test-config.c
index 9dd1b22..6a77552 100644
--- a/test-config.c
+++ b/test-config.c
@@ -16,6 +16,8 @@
*
* get_bool - print bool value for the entered key or die
*
+ * get_string - print string value for the entered key or die
+ *
* configset_get_value - returns value with the highest priority for the
entered key
* from a config_set constructed from files entered as
arguments.
*
@@ -84,6 +86,14 @@ int main(int argc, char **argv)
printf(Value not found for \%s\\n, argv[2]);
goto exit1;
}
+ } else if (argc == 3 !strcmp(argv[1], get_string)) {
+ if (!git_config_get_string_const(argv[2], v)) {
+ printf(%s\n, v);
+ goto exit0;
+ } else {
+ printf(Value not found for \%s\\n, argv[2]);
+ goto exit1;
+ }
} else if (!strcmp(argv[1], configset_get_value)) {
for (i = 3; i argc; i++) {
int err;
--
1.9.0.GIT
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 5/6] config: add `git_die_config()` to the config-set API
Add `git_die_config` that dies printing the line number and the file name
of the highest priority value for the configuration variable `key`.
It has usage in non-callback based config value retrieval where we can
raise an error and die if there is a semantic error.
For example,
if (!git_config_get_value(key, value)) {
/* NULL values not allowed */
if (!value)
git_config_die(key);
else
/* do work */
}
Signed-off-by: Tanay Abhra tanay...@gmail.com
---
Documentation/technical/api-config.txt | 5
cache.h| 1 +
config.c | 44 ++
3 files changed, 40 insertions(+), 10 deletions(-)
diff --git a/Documentation/technical/api-config.txt
b/Documentation/technical/api-config.txt
index 815c1ee..e7ec7cc 100644
--- a/Documentation/technical/api-config.txt
+++ b/Documentation/technical/api-config.txt
@@ -155,6 +155,11 @@ as well as retrieval for the queried variable, including:
Similar to `git_config_get_string`, but expands `~` or `~user` into
the user's home directory when found at the beginning of the path.
+`void git_die_config(const char *key)`::
+
+ Dies printing the line number and the file name of the highest
+ priority value for the configuration variable `key`.
+
See test-config.c for usage examples.
Value Parsing Helpers
diff --git a/cache.h b/cache.h
index 11ded5a..5c3dd88 100644
--- a/cache.h
+++ b/cache.h
@@ -1407,6 +1407,7 @@ extern int git_config_get_bool(const char *key, int
*dest);
extern int git_config_get_bool_or_int(const char *key, int *is_bool, int
*dest);
extern int git_config_get_maybe_bool(const char *key, int *dest);
extern int git_config_get_pathname(const char *key, const char **dest);
+extern void git_die_config(const char *key);
extern int committer_ident_sufficiently_given(void);
extern int author_ident_sufficiently_given(void);
diff --git a/config.c b/config.c
index aa5c0ad..1e49ae7 100644
--- a/config.c
+++ b/config.c
@@ -1403,11 +1403,12 @@ const struct string_list
*git_configset_get_value_multi(struct config_set *cs, c
int git_configset_get_string_const(struct config_set *cs, const char *key,
const char **dest)
{
- const char *value;
- if (!git_configset_get_value(cs, key, value))
- return git_config_string(dest, key, value);
- else
- return 1;
+ int ret;
+ char *value;
+ ret = git_configset_get_string(cs, key, value);
+ if (ret = 0)
+ *dest = (const char*)value;
+ return ret;
}
int git_configset_get_string(struct config_set *cs, const char *key, char
**dest)
@@ -1418,8 +1419,7 @@ int git_configset_get_string(struct config_set *cs, const
char *key, char **dest
return config_error_nonbool(key);
*dest = xstrdup(value);
return 0;
- }
- else
+ } else
return 1;
}
@@ -1514,14 +1514,22 @@ const struct string_list
*git_config_get_value_multi(const char *key)
int git_config_get_string_const(const char *key, const char **dest)
{
+ int ret;
git_config_check_init();
- return git_configset_get_string_const(the_config_set, key, dest);
+ ret = git_configset_get_string_const(the_config_set, key, dest);
+ if (ret 0)
+ git_die_config(key);
+ return ret;
}
int git_config_get_string(const char *key, char **dest)
{
+ int ret;
git_config_check_init();
- return git_configset_get_string(the_config_set, key, dest);
+ ret = git_configset_get_string(the_config_set, key, dest);
+ if (ret 0)
+ git_die_config(key);
+ return ret;
}
int git_config_get_int(const char *key, int *dest)
@@ -1556,8 +1564,24 @@ int git_config_get_maybe_bool(const char *key, int *dest)
int git_config_get_pathname(const char *key, const char **dest)
{
+ int ret;
git_config_check_init();
- return git_configset_get_pathname(the_config_set, key, dest);
+ ret = git_configset_get_pathname(the_config_set, key, dest);
+ if (ret 0)
+ git_die_config(key);
+ return ret;
+}
+
+void git_die_config(const char *key)
+{
+ const struct string_list *values;
+ struct key_value_info *kv_info;
+ values = git_config_get_value_multi(key);
+ kv_info = values-items[values-nr - 1].util;
+ if (!kv_info-linenr)
+ die(unable to parse command-line config);
+ else
+ die(bad config file line %d in %s,kv_info-linenr,
kv_info-filename);
}
/*
--
1.9.0.GIT
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC PATCH 3/3] commit: advertize config --global --edit on guessed identity
When the user has no user-wide configuration file, it's faster to use the
newly introduced config file template than to run two commands to set
user.name and user.email. Advise this to the user.
The old advice is kept if the user already has a configuration file since
the template feature would not trigger in this case.
Signed-off-by: Matthieu Moy matthieu@imag.fr
---
builtin/commit.c | 34 --
1 file changed, 32 insertions(+), 2 deletions(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index f2d7979..52ef5a8 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -42,7 +42,20 @@ static const char * const builtin_status_usage[] = {
NULL
};
-static const char implicit_ident_advice[] =
+static const char implicit_ident_advice_noconfig[] =
+N_(Your name and email address were configured automatically based\n
+on your username and hostname. Please check that they are accurate.\n
+You can suppress this message by setting them explicitly. Run the\n
+following command and follow the instructions in your editor to edit\n
+your configuration file:\n
+\n
+git config --global --edit\n
+\n
+After doing this, you may fix the identity used for this commit with:\n
+\n
+git commit --amend --reset-author\n);
+
+static const char implicit_ident_advice_config[] =
N_(Your name and email address were configured automatically based\n
on your username and hostname. Please check that they are accurate.\n
You can suppress this message by setting them explicitly:\n
@@ -1403,6 +1416,23 @@ int cmd_status(int argc, const char **argv, const char
*prefix)
return 0;
}
+static const char *implicit_ident_advice() {
+ char *user_config = NULL;
+ char *xdg_config = NULL;
+ int config_exists;
+
+ home_config_paths(user_config, xdg_config, config);
+ config_exists = file_exists(user_config) || file_exists(xdg_config);
+ free(user_config);
+ free(xdg_config);
+
+ if (config_exists)
+ return _(implicit_ident_advice_config);
+ else
+ return _(implicit_ident_advice_noconfig);
+
+}
+
static void print_summary(const char *prefix, const unsigned char *sha1,
int initial_commit)
{
@@ -1441,7 +1471,7 @@ static void print_summary(const char *prefix, const
unsigned char *sha1,
strbuf_addbuf_percentquote(format, committer_ident);
if (advice_implicit_identity) {
strbuf_addch(format, '\n');
- strbuf_addstr(format, _(implicit_ident_advice));
+ strbuf_addstr(format, implicit_ident_advice());
}
}
strbuf_release(author_ident);
--
2.0.2.737.gfb43bde
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC PATCH 1/3] config --global --edit: create a template file if needed
When the user has no ~/.gitconfig file, git config --global --edit used
to launch an editor on an nonexistant file name.
Instead, create a file with a default content before launching the
editor. The template contains only commented-out entries, to save a few
keystrokes for the user. If the values are guessed properly, the user
will only have to uncomment the entries.
Advanced users teaching newbies can create a minimalistic configuration
faster for newbies. Beginners reading a tutorial advising to run git
config --global --edit as a first step will be slightly more guided for
their first contact with Git.
Signed-off-by: Matthieu Moy matthieu@imag.fr
---
builtin/config.c | 30 +++---
cache.h | 1 +
ident.c | 2 +-
3 files changed, 29 insertions(+), 4 deletions(-)
diff --git a/builtin/config.c b/builtin/config.c
index fcd8474..3821697 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -445,6 +445,20 @@ static int get_urlmatch(const char *var, const char *url)
return 0;
}
+static char *default_user_config()
+{
+ struct strbuf buf = STRBUF_INIT;
+ strbuf_addf(buf,
+ _(# This is Git's user-wide configuration file.\n
+ [core]\n
+ # Please, adapt and uncomment the following lines:\n
+ #user = %s\n
+ #email = %s\n),
+ ident_default_name(),
+ ident_default_email());
+ return strbuf_detach(buf, NULL);
+}
+
int cmd_config(int argc, const char **argv, const char *prefix)
{
int nongit = !startup_info-have_repository;
@@ -551,6 +565,8 @@ int cmd_config(int argc, const char **argv, const char
*prefix)
}
}
else if (actions == ACTION_EDIT) {
+ const char *config_file = given_config_source.file ?
+ given_config_source.file : git_path(config);
check_argc(argc, 0, 0);
if (!given_config_source.file nongit)
die(not in a git directory);
@@ -559,9 +575,17 @@ int cmd_config(int argc, const char **argv, const char
*prefix)
if (given_config_source.blob)
die(editing blobs is not supported);
git_config(git_default_config, NULL);
- launch_editor(given_config_source.file ?
- given_config_source.file : git_path(config),
- NULL, NULL);
+ if (use_global_config) {
+ int fd = open(config_file, O_CREAT | O_EXCL | O_WRONLY,
0666);
+ if (fd) {
+ char *content = default_user_config();
+ write_str_in_full(fd, content);
+ free(content);
+ }
+ else if (errno != EEXIST)
+ die_errno(_(Cannot create configuration file
%s), config_file);
+ }
+ launch_editor(config_file, NULL, NULL);
}
else if (actions == ACTION_SET) {
int ret;
diff --git a/cache.h b/cache.h
index 2f63fd1..8c79c0c 100644
--- a/cache.h
+++ b/cache.h
@@ -1061,6 +1061,7 @@ extern const char *git_author_info(int);
extern const char *git_committer_info(int);
extern const char *fmt_ident(const char *name, const char *email, const char
*date_str, int);
extern const char *fmt_name(const char *name, const char *email);
+extern const char *ident_default_name(void);
extern const char *ident_default_email(void);
extern const char *git_editor(void);
extern const char *git_pager(int stdout_is_tty);
diff --git a/ident.c b/ident.c
index 1d9b6e7..77bc882 100644
--- a/ident.c
+++ b/ident.c
@@ -102,7 +102,7 @@ static void copy_email(const struct passwd *pw, struct
strbuf *email)
add_domainname(email);
}
-static const char *ident_default_name(void)
+const char *ident_default_name(void)
{
if (!git_default_name.len) {
copy_gecos(xgetpwuid_self(), git_default_name);
--
2.0.2.737.gfb43bde
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC PATCH 2/3] home_config_path: allow NULL xdg parameter
This allows a caller to requst the global config file without requesting
the XDG one.
Signed-off-by: Matthieu Moy matthieu@imag.fr
---
This is actually not needed, but I wrote this for a previous version,
and it seems sensible anyway.
path.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/path.c b/path.c
index 3afcdb4..f68df0c 100644
--- a/path.c
+++ b/path.c
@@ -148,10 +148,12 @@ void home_config_paths(char **global, char **xdg, char
*file)
*global = mkpathdup(%s/.gitconfig, home);
}
- if (!xdg_home)
- *xdg = NULL;
- else
- *xdg = mkpathdup(%s/git/%s, xdg_home, file);
+ if (xdg) {
+ if (!xdg_home)
+ *xdg = NULL;
+ else
+ *xdg = mkpathdup(%s/git/%s, xdg_home, file);
+ }
free(to_free);
}
--
2.0.2.737.gfb43bde
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 2/3] home_config_path: allow NULL xdg parameter
On 7/25/2014 7:14 PM, Matthieu Moy wrote: This allows a caller to requst the global config file without requesting nit s/requst/request/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 3/6] rewrite git_config() to use the config-set API
Tanay Abhra tanay...@gmail.com writes:
+struct config_set_element {
+ struct hashmap_entry ent;
+ char *key;
+ struct string_list value_list;
+};
+
+struct configset_list_item {
+ struct config_set_element *e;
+ int value_index;
+};
I originally wondered why you had two levels of pointers, but
config_set_element is not new, just moved. It's OK.
+/*
+ * the contents of the list are ordered according to their
+ * position in the config files and order of parsing the files.
+ * (i.e. key-value pair at the last position of .git/config will
+ * be at the last item of the list)
+ */
+
+struct configset_list {
I wouldn't put a blank line between comment and decl if the comment
applies to the decl.
-int git_config(config_fn_t fn, void *data)
+static int git_config_raw(config_fn_t fn, void *data)
{
return git_config_with_options(fn, data, NULL, 1);
}
I would have done this and the new git_config() as a separate patch, but
I do not mind strongly.
static struct config_set_element *configset_find_element(struct config_set
*cs, const char *key)
{
struct config_set_element k;
@@ -1268,6 +1295,7 @@ static int configset_add_value(struct config_set *cs,
const char *key, const cha
{
struct config_set_element *e;
struct string_list_item *si;
+ struct configset_list_item *l_item;
struct key_value_info *kv_info = xmalloc(sizeof(*kv_info));
e = configset_find_element(cs, key);
@@ -1283,6 +1311,13 @@ static int configset_add_value(struct config_set *cs,
const char *key, const cha
hashmap_add(cs-config_hash, e);
}
si = string_list_append_nodup(e-value_list, value ? xstrdup(value) :
NULL);
+
+ if (cs-list.nr + 1 cs-list.alloc)
The if is already in ALLOC_GROW.
+ ALLOC_GROW(cs-list.items, cs-list.nr + 20, cs-list.alloc);
The 20 should be just 1 I guess. You're adding 1 element, and ALLOC_GROW
will take care of allocating more than 1 for you (see alloc_nr and
ALLOC_GROW's defs in cache.h).
@@ -1318,10 +1356,14 @@ void git_configset_clear(struct config_set *cs)
hashmap_iter_init(cs-config_hash, iter);
while ((entry = hashmap_iter_next(iter))) {
free(entry-key);
- string_list_clear(entry-value_list, 0);
+ string_list_clear(entry-value_list, 1);
Doesn't this change belong to PATCH 2/6 ?
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 5/6] config: add `git_die_config()` to the config-set API
Tanay Abhra tanay...@gmail.com writes:
--- a/config.c
+++ b/config.c
@@ -1403,11 +1403,12 @@ const struct string_list
*git_configset_get_value_multi(struct config_set *cs, c
int git_configset_get_string_const(struct config_set *cs, const char *key,
const char **dest)
{
- const char *value;
- if (!git_configset_get_value(cs, key, value))
- return git_config_string(dest, key, value);
- else
- return 1;
+ int ret;
+ char *value;
+ ret = git_configset_get_string(cs, key, value);
+ if (ret = 0)
+ *dest = (const char*)value;
+ return ret;
}
Isn't this a fixup meant for another series?
int git_configset_get_string(struct config_set *cs, const char *key, char
**dest)
@@ -1418,8 +1419,7 @@ int git_configset_get_string(struct config_set *cs,
const char *key, char **dest
return config_error_nonbool(key);
*dest = xstrdup(value);
return 0;
- }
- else
+ } else
return 1;
Useless churn.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 3/6] rewrite git_config() to use the config-set API
On 7/25/2014 7:28 PM, Matthieu Moy wrote:
Tanay Abhra tanay...@gmail.com writes:
+struct config_set_element {
+struct hashmap_entry ent;
+char *key;
+struct string_list value_list;
+};
+
+struct configset_list_item {
+struct config_set_element *e;
+int value_index;
+};
I originally wondered why you had two levels of pointers, but
config_set_element is not new, just moved. It's OK.
+/*
+ * the contents of the list are ordered according to their
+ * position in the config files and order of parsing the files.
+ * (i.e. key-value pair at the last position of .git/config will
+ * be at the last item of the list)
+ */
+
+struct configset_list {
I wouldn't put a blank line between comment and decl if the comment
applies to the decl.
Noted.
-int git_config(config_fn_t fn, void *data)
+static int git_config_raw(config_fn_t fn, void *data)
{
return git_config_with_options(fn, data, NULL, 1);
}
I would have done this and the new git_config() as a separate patch, but
I do not mind strongly.
static struct config_set_element *configset_find_element(struct config_set
*cs, const char *key)
{
struct config_set_element k;
@@ -1268,6 +1295,7 @@ static int configset_add_value(struct config_set *cs,
const char *key, const cha
{
struct config_set_element *e;
struct string_list_item *si;
+struct configset_list_item *l_item;
struct key_value_info *kv_info = xmalloc(sizeof(*kv_info));
e = configset_find_element(cs, key);
@@ -1283,6 +1311,13 @@ static int configset_add_value(struct config_set *cs,
const char *key, const cha
hashmap_add(cs-config_hash, e);
}
si = string_list_append_nodup(e-value_list, value ? xstrdup(value) :
NULL);
+
+if (cs-list.nr + 1 cs-list.alloc)
The if is already in ALLOC_GROW.
+ALLOC_GROW(cs-list.items, cs-list.nr + 20, cs-list.alloc);
The 20 should be just 1 I guess. You're adding 1 element, and ALLOC_GROW
will take care of allocating more than 1 for you (see alloc_nr and
ALLOC_GROW's defs in cache.h).
Oh, you are are right, I thought alloc grew by one, not by a factor.
@@ -1318,10 +1356,14 @@ void git_configset_clear(struct config_set *cs)
hashmap_iter_init(cs-config_hash, iter);
while ((entry = hashmap_iter_next(iter))) {
free(entry-key);
-string_list_clear(entry-value_list, 0);
+string_list_clear(entry-value_list, 1);
Doesn't this change belong to PATCH 2/6 ?
Yup, you are right again.
Thanks.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 5/6] config: add `git_die_config()` to the config-set API
On 7/25/2014 7:33 PM, Matthieu Moy wrote:
Tanay Abhra tanay...@gmail.com writes:
--- a/config.c
+++ b/config.c
@@ -1403,11 +1403,12 @@ const struct string_list
*git_configset_get_value_multi(struct config_set *cs, c
int git_configset_get_string_const(struct config_set *cs, const char *key,
const char **dest)
{
-const char *value;
-if (!git_configset_get_value(cs, key, value))
-return git_config_string(dest, key, value);
-else
-return 1;
+int ret;
+char *value;
+ret = git_configset_get_string(cs, key, value);
+if (ret = 0)
+*dest = (const char*)value;
+return ret;
}
Isn't this a fixup meant for another series?
Though v12 is in pu, Junio commented that git_configset_get_string_const()
git_configset_get_string() can be done more concisely, I was trying to do
that but I failed.
int git_configset_get_string(struct config_set *cs, const char *key, char
**dest)
@@ -1418,8 +1419,7 @@ int git_configset_get_string(struct config_set *cs,
const char *key, char **dest
return config_error_nonbool(key);
*dest = xstrdup(value);
return 0;
-}
-else
+} else
return 1;
Useless churn.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 1/3] config --global --edit: create a template file if needed
On Fri, Jul 25, 2014 at 9:44 AM, Matthieu Moy matthieu@imag.fr wrote:
When the user has no ~/.gitconfig file, git config --global --edit used
to launch an editor on an nonexistant file name.
Instead, create a file with a default content before launching the
editor. The template contains only commented-out entries, to save a few
keystrokes for the user. If the values are guessed properly, the user
will only have to uncomment the entries.
Advanced users teaching newbies can create a minimalistic configuration
faster for newbies. Beginners reading a tutorial advising to run git
config --global --edit as a first step will be slightly more guided for
their first contact with Git.
Signed-off-by: Matthieu Moy matthieu@imag.fr
---
builtin/config.c | 30 +++---
cache.h | 1 +
ident.c | 2 +-
3 files changed, 29 insertions(+), 4 deletions(-)
diff --git a/builtin/config.c b/builtin/config.c
index fcd8474..3821697 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -445,6 +445,20 @@ static int get_urlmatch(const char *var, const char *url)
return 0;
}
+static char *default_user_config()
+{
+ struct strbuf buf = STRBUF_INIT;
+ strbuf_addf(buf,
+ _(# This is Git's user-wide configuration file.\n
+ [core]\n
+ # Please, adapt and uncomment the following lines:\n
+ #user = %s\n
+ #email = %s\n),
[core], user =, email = should not be translated. Would it make
sense to keep these outside of _()?
+ ident_default_name(),
+ ident_default_email());
+ return strbuf_detach(buf, NULL);
+}
+
int cmd_config(int argc, const char **argv, const char *prefix)
{
int nongit = !startup_info-have_repository;
@@ -551,6 +565,8 @@ int cmd_config(int argc, const char **argv, const char
*prefix)
}
}
else if (actions == ACTION_EDIT) {
+ const char *config_file = given_config_source.file ?
+ given_config_source.file : git_path(config);
check_argc(argc, 0, 0);
if (!given_config_source.file nongit)
die(not in a git directory);
@@ -559,9 +575,17 @@ int cmd_config(int argc, const char **argv, const char
*prefix)
if (given_config_source.blob)
die(editing blobs is not supported);
git_config(git_default_config, NULL);
- launch_editor(given_config_source.file ?
- given_config_source.file : git_path(config),
- NULL, NULL);
+ if (use_global_config) {
+ int fd = open(config_file, O_CREAT | O_EXCL |
O_WRONLY, 0666);
+ if (fd) {
+ char *content = default_user_config();
+ write_str_in_full(fd, content);
close(fd);
+ free(content);
+ }
+ else if (errno != EEXIST)
+ die_errno(_(Cannot create configuration file
%s), config_file);
Other error messages in this file (including those just above this
block) begin with a lowercase letter.
+ }
+ launch_editor(config_file, NULL, NULL);
}
else if (actions == ACTION_SET) {
int ret;
diff --git a/cache.h b/cache.h
index 2f63fd1..8c79c0c 100644
--- a/cache.h
+++ b/cache.h
@@ -1061,6 +1061,7 @@ extern const char *git_author_info(int);
extern const char *git_committer_info(int);
extern const char *fmt_ident(const char *name, const char *email, const char
*date_str, int);
extern const char *fmt_name(const char *name, const char *email);
+extern const char *ident_default_name(void);
extern const char *ident_default_email(void);
extern const char *git_editor(void);
extern const char *git_pager(int stdout_is_tty);
diff --git a/ident.c b/ident.c
index 1d9b6e7..77bc882 100644
--- a/ident.c
+++ b/ident.c
@@ -102,7 +102,7 @@ static void copy_email(const struct passwd *pw, struct
strbuf *email)
add_domainname(email);
}
-static const char *ident_default_name(void)
+const char *ident_default_name(void)
{
if (!git_default_name.len) {
copy_gecos(xgetpwuid_self(), git_default_name);
--
2.0.2.737.gfb43bde
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] strbuf.c: keep errno in strbuf_read_file()
On Fri, Jul 25, 2014 at 6:43 AM, Nguyễn Thái Ngọc Duy pclo...@gmail.com wrote:
This function is used to replaced some code in the next patch that
s/replaced/replace/
does this (i.e. keep the errno when read() fails)
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
strbuf.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/strbuf.c b/strbuf.c
index 33018d8..61d685d 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -454,15 +454,18 @@ int strbuf_getwholeline_fd(struct strbuf *sb, int fd,
int term)
int strbuf_read_file(struct strbuf *sb, const char *path, size_t hint)
{
- int fd, len;
+ int fd, len, saved_errno;
fd = open(path, O_RDONLY);
if (fd 0)
return -1;
len = strbuf_read(sb, fd, hint);
+ saved_errno = errno;
close(fd);
- if (len 0)
+ if (len 0) {
+ errno = saved_errno;
return -1;
+ }
return len;
}
--
1.9.1.346.ga2b5940
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 3/3] commit: advertize config --global --edit on guessed identity
On Fri, Jul 25, 2014 at 9:44 AM, Matthieu Moy matthieu@imag.fr wrote:
commit: advertize config --global --edit on guessed identity
s/advertize/advertise/
When the user has no user-wide configuration file, it's faster to use the
newly introduced config file template than to run two commands to set
user.name and user.email. Advise this to the user.
The old advice is kept if the user already has a configuration file since
the template feature would not trigger in this case.
Signed-off-by: Matthieu Moy matthieu@imag.fr
---
builtin/commit.c | 34 --
1 file changed, 32 insertions(+), 2 deletions(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index f2d7979..52ef5a8 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -42,7 +42,20 @@ static const char * const builtin_status_usage[] = {
NULL
};
-static const char implicit_ident_advice[] =
+static const char implicit_ident_advice_noconfig[] =
+N_(Your name and email address were configured automatically based\n
+on your username and hostname. Please check that they are accurate.\n
+You can suppress this message by setting them explicitly. Run the\n
+following command and follow the instructions in your editor to edit\n
+your configuration file:\n
+\n
+git config --global --edit\n
+\n
+After doing this, you may fix the identity used for this commit with:\n
+\n
+git commit --amend --reset-author\n);
+
+static const char implicit_ident_advice_config[] =
N_(Your name and email address were configured automatically based\n
on your username and hostname. Please check that they are accurate.\n
You can suppress this message by setting them explicitly:\n
@@ -1403,6 +1416,23 @@ int cmd_status(int argc, const char **argv, const char
*prefix)
return 0;
}
+static const char *implicit_ident_advice() {
+ char *user_config = NULL;
+ char *xdg_config = NULL;
+ int config_exists;
+
+ home_config_paths(user_config, xdg_config, config);
+ config_exists = file_exists(user_config) || file_exists(xdg_config);
+ free(user_config);
+ free(xdg_config);
+
+ if (config_exists)
+ return _(implicit_ident_advice_config);
+ else
+ return _(implicit_ident_advice_noconfig);
+
+}
+
static void print_summary(const char *prefix, const unsigned char *sha1,
int initial_commit)
{
@@ -1441,7 +1471,7 @@ static void print_summary(const char *prefix, const
unsigned char *sha1,
strbuf_addbuf_percentquote(format, committer_ident);
if (advice_implicit_identity) {
strbuf_addch(format, '\n');
- strbuf_addstr(format, _(implicit_ident_advice));
+ strbuf_addstr(format, implicit_ident_advice());
}
}
strbuf_release(author_ident);
--
2.0.2.737.gfb43bde
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4] refs.c: refactor resolve_ref_unsafe() to use strbuf internally
On Fri, Jul 25, 2014 at 6:43 AM, Nguyễn Thái Ngọc Duy pclo...@gmail.com wrote:
In the beginning, we had resolve_ref() that returns a buffer owned by
this function. Then we started to move away from that direction because
the buffer could be overwritten by the next resolve_ref() call and
introduced two new functions: resolve_ref_unsafe() and resolve_refdup().
The static buffer is still kept internally.
This patch makes the core of resolve_ref use a strbuf instead of static
buffer. Which makes resolve_refdup() more efficient (no need to copy
from the static buffer to a new buffer). It also removes the (random?)
256 char limit. In future, resolve_ref() could be used directly without
going through resolve_refdup() wrapper.
A minor bonus. resolve_ref(dup) are now more thread-friendly (although I'm
not 100% sure if they are thread-safe yet).
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
cache.h | 1 +
refs.c | 122
+++-
2 files changed, 68 insertions(+), 55 deletions(-)
diff --git a/cache.h b/cache.h
index fcb511d..5ffbafb 100644
--- a/cache.h
+++ b/cache.h
+const char *resolve_ref_unsafe(const char *refname, unsigned char *sha1, int
reading, int *flag)
+{
+ static struct strbuf buf = STRBUF_INIT;
+ if (!resolve_ref(refname, buf, sha1, reading, flag))
+ return buf.buf;
+ else
+ return NULL;
}
char *resolve_refdup(const char *ref, unsigned char *sha1, int reading, int
*flag)
{
- const char *ret = resolve_ref_unsafe(ref, sha1, reading, flag);
- return ret ? xstrdup(ret) : NULL;
+ struct strbuf buf = STRBUF_INIT;
+ if (!resolve_ref(ref, buf, sha1, reading, flag))
+ return buf.buf;
return strbuf_detach(buf, NULL);
+ else {
+ strbuf_release(buf);
+ return NULL;
+ }
}
/* The argument to filter_refs */
--
1.9.1.346.ga2b5940
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 1/3] config --global --edit: create a template file if needed
Eric Sunshine sunsh...@sunshineco.com writes:
+static char *default_user_config()
+{
+ struct strbuf buf = STRBUF_INIT;
+ strbuf_addf(buf,
+ _(# This is Git's user-wide configuration file.\n
+ [core]\n
+ # Please, adapt and uncomment the following lines:\n
+ #user = %s\n
+ #email = %s\n),
[core], user =, email = should not be translated. Would it make
sense to keep these outside of _()?
I would say no, as the code and the string to translate would be much
less readable without core, user and email inline.
Were you suggesting stg like
_(# This is Git's user-wide configuration file.\n
[%s]\n
# Please, adapt and uncomment the following lines:\n
#%s = %s\n
#%s = %s\n),
core, name, ..., email, ...
?
+ if (fd) {
+ char *content = default_user_config();
+ write_str_in_full(fd, content);
close(fd);
Indeed.
+ free(content);
+ }
+ else if (errno != EEXIST)
+ die_errno(_(Cannot create configuration
file %s), config_file);
Other error messages in this file (including those just above this
block) begin with a lowercase letter.
Applied.
Thanks,
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] refs.c: move ref parsing code out of resolve_ref()
Nice.
On Fri, Jul 25, 2014 at 3:43 AM, Nguyễn Thái Ngọc Duy pclo...@gmail.com wrote:
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
cache.h | 11
refs.c | 204
++--
2 files changed, 120 insertions(+), 95 deletions(-)
diff --git a/cache.h b/cache.h
index 5ffbafb..40a63d9 100644
--- a/cache.h
+++ b/cache.h
@@ -1003,6 +1003,17 @@ extern int read_ref(const char *refname, unsigned char
*sha1);
extern const char *resolve_ref_unsafe(const char *ref, unsigned char *sha1,
int reading, int *flag);
extern char *resolve_refdup(const char *ref, unsigned char *sha1, int
reading, int *flag);
extern int resolve_ref(const char *refname, struct strbuf *result, unsigned
char *sha1, int reading, int *flag);
+/*
+ * Given a ref in ref and its path, returns
+ *
+ * -2 failed to open with ENOENT, the caller is responsible for
+ * checking missing loose ref (see resolve_ref for example)
+ * -1 if there's an error, ref can no longer be trusted, flag may
+ * be set. errno is valid.
+ * 0 this is a symref, ref now contains the linked ref
+ * +1 normal ref, sha1 is valid
+ */
+extern int parse_ref(const char *path, struct strbuf *ref, unsigned char
*sha1, int *flag);
extern int dwim_ref(const char *str, int len, unsigned char *sha1, char
**ref);
extern int dwim_log(const char *str, int len, unsigned char *sha1, char
**ref);
diff --git a/refs.c b/refs.c
index bec2bb1..2769f20 100644
--- a/refs.c
+++ b/refs.c
@@ -1533,6 +1533,105 @@ static int handle_missing_loose_ref(const char
*refname,
}
}
+int parse_ref(const char *path, struct strbuf *ref,
+ unsigned char *sha1, int *flag)
Can you make this function static?
It is not used by anything outside of this series and thus making it
static avoids growing the public refs api.
+{
+ struct strbuf buffer = STRBUF_INIT;
+ struct stat st;
+ const char *buf;
+
+ /*
+* We might have to loop back here to avoid a race condition:
+* first we lstat() the file, then we try to read it as a link
+* or as a file. But if somebody changes the type of the file
+* (file - directory - symlink) between the lstat() and
+* reading, then we don't want to report that as an error but
+* rather try again starting with the lstat().
+*/
+stat_ref:
+ if (lstat(path, st) 0)
+ return errno == ENOENT ? -2 : -1;
+
+ /* Follow normalized - ie refs/.. symlinks by hand */
+ if (S_ISLNK(st.st_mode)) {
+ struct strbuf new_path = STRBUF_INIT;
+ if (strbuf_readlink(new_path, path, 256) 0) {
+ strbuf_release(new_path);
+ if (errno == ENOENT || errno == EINVAL)
+ /* inconsistent with lstat; retry */
+ goto stat_ref;
+ else
+ return -1;
+ }
+ if (starts_with(new_path.buf, refs/)
+ !check_refname_format(new_path.buf, 0)) {
+ strbuf_reset(ref);
+ strbuf_addbuf(ref, new_path);
+ if (flag)
+ *flag |= REF_ISSYMREF;
+ strbuf_release(new_path);
+ return 0;
+ }
+ strbuf_release(new_path);
+ }
+
+ /* Is it a directory? */
+ if (S_ISDIR(st.st_mode)) {
+ errno = EISDIR;
+ return -1;
+ }
+
+ /*
+* Anything else, just open it and try to use it as
+* a ref
+*/
+ if (strbuf_read_file(buffer, path, 256) 0) {
+ strbuf_release(buffer);
+ if (errno == ENOENT)
+ /* inconsistent with lstat; retry */
+ goto stat_ref;
+ else
+ return -1;
+ }
+ strbuf_rtrim(buffer);
+
+ /*
+* Is it a symbolic ref?
+*/
+ if (!skip_prefix(buffer.buf, ref:, buf)) {
+ int ret;
+ /*
+* Please note that FETCH_HEAD has a second line
+* containing other data.
+*/
+ if (get_sha1_hex(buffer.buf, sha1) ||
+ (buffer.buf[40] != '\0' !isspace(buffer.buf[40]))) {
+ if (flag)
+ *flag |= REF_ISBROKEN;
+ errno = EINVAL;
+ ret = -1;
+ } else
+ ret = 1;
+ strbuf_release(buffer);
+ return ret;
+ }
+ if (flag)
+ *flag |= REF_ISSYMREF;
+ while (isspace(*buf))
+ buf++;
+ if
[PATCH 1/5] refs.c: allow passing raw git_committer_info as email to _update_reflog
In many places in the code we do not have access to the individual fields
in the committer data. Instead we might only have access to prebaked data
such as what is returned by git_committer_info() containing a string
that consists of email, timestamp, zone etc.
This makes it inconvenient to use transaction_update_reflog since it means
you would have to first parse git_committer_info before you can call
update_reflog.
Add a new flag REFLOG_EMAIL_IS_COMMITTER to _update_reflog to tell it
that what we pass in as email is already the fully baked committer string
we can use as-is.
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
refs.c | 20
refs.h | 1 +
2 files changed, 13 insertions(+), 8 deletions(-)
diff --git a/refs.c b/refs.c
index 2662ef6..6c55032 100644
--- a/refs.c
+++ b/refs.c
@@ -3522,14 +3522,18 @@ int transaction_update_reflog(struct ref_transaction
*transaction,
hashcpy(update-old_sha1, old_sha1);
update-reflog_fd = -1;
if (email) {
- struct strbuf buf = STRBUF_INIT;
- char sign = (tz 0) ? '-' : '+';
- int zone = (tz 0) ? (-tz) : tz;
-
- strbuf_addf(buf, %s %lu %c%04d, email, timestamp, sign,
- zone);
- update-committer = xstrdup(buf.buf);
- strbuf_release(buf);
+ if (flags REFLOG_EMAIL_IS_COMMITTER)
+ update-committer = xstrdup(email);
+ else {
+ struct strbuf buf = STRBUF_INIT;
+ char sign = (tz 0) ? '-' : '+';
+ int zone = (tz 0) ? (-tz) : tz;
+
+ strbuf_addf(buf, %s %lu %c%04d, email, timestamp,
+ sign, zone);
+ update-committer = xstrdup(buf.buf);
+ strbuf_release(buf);
+ }
}
if (msg)
update-msg = xstrdup(msg);
diff --git a/refs.h b/refs.h
index 0172f48..eb918a0 100644
--- a/refs.h
+++ b/refs.h
@@ -309,6 +309,7 @@ int transaction_delete_sha1(struct ref_transaction
*transaction,
* Flags = 0x100 are reserved for internal use.
*/
#define REFLOG_TRUNCATE 0x01
+#define REFLOG_EMAIL_IS_COMMITTER 0x02
/*
* Append a reflog entry for refname. If the REFLOG_TRUNCATE flag is set
* this update will first truncate the reflog before writing the entry.
--
2.0.1.508.g763ab16
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/5] refs.c: update rename_ref to use a transaction
Change refs.c to use a single transaction to copy/rename both the refs and
its reflog. Since we are no longer using rename() to move the reflog file
we no longer need to disallow rename_ref for refs with a symlink for its
reflog so we can remove that test from the testsuite.
Change the function to return 1 on failure instead of either -1 or 1.
These changes make the rename_ref operation atomic. This also eliminates the
need to use rename() to shift the reflog around via a temporary filename.
As an extension to this, since we no longer use rename() on the reflog file,
we can now safely perform renames even if the reflog is a symbolic link
and thus can remove the check and fail for that condition.
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
refs.c| 192 ++
t/t3200-branch.sh | 7 --
2 files changed, 65 insertions(+), 134 deletions(-)
diff --git a/refs.c b/refs.c
index 0d800f1..a5053bf 100644
--- a/refs.c
+++ b/refs.c
@@ -2616,82 +2616,43 @@ int delete_ref(const char *refname, const unsigned char
*sha1, int delopt)
return 0;
}
-/*
- * People using contrib's git-new-workdir have .git/logs/refs -
- * /some/other/path/.git/logs/refs, and that may live on another device.
- *
- * IOW, to avoid cross device rename errors, the temporary renamed log must
- * live into logs/refs.
- */
-#define TMP_RENAMED_LOG logs/refs/.tmp-renamed-log
+struct rename_reflog_cb {
+ struct ref_transaction *transaction;
+ const char *refname;
+ struct strbuf *err;
+};
-static int rename_tmp_log(const char *newrefname)
+static int rename_reflog_ent(unsigned char *osha1, unsigned char *nsha1,
+const char *email, unsigned long timestamp, int tz,
+const char *message, void *cb_data)
{
- int attempts_remaining = 4;
-
- retry:
- switch (safe_create_leading_directories(git_path(logs/%s,
newrefname))) {
- case SCLD_OK:
- break; /* success */
- case SCLD_VANISHED:
- if (--attempts_remaining 0)
- goto retry;
- /* fall through */
- default:
- error(unable to create directory for %s, newrefname);
- return -1;
- }
+ struct rename_reflog_cb *cb = cb_data;
- if (rename(git_path(TMP_RENAMED_LOG), git_path(logs/%s, newrefname)))
{
- if ((errno==EISDIR || errno==ENOTDIR) --attempts_remaining
0) {
- /*
-* rename(a, b) when b is an existing
-* directory ought to result in ISDIR, but
-* Solaris 5.8 gives ENOTDIR. Sheesh.
-*/
- if (remove_empty_directories(git_path(logs/%s,
newrefname))) {
- error(Directory not empty: logs/%s,
newrefname);
- return -1;
- }
- goto retry;
- } else if (errno == ENOENT --attempts_remaining 0) {
- /*
-* Maybe another process just deleted one of
-* the directories in the path to newrefname.
-* Try again from the beginning.
-*/
- goto retry;
- } else {
- error(unable to move logfile TMP_RENAMED_LOG to
logs/%s: %s,
- newrefname, strerror(errno));
- return -1;
- }
- }
- return 0;
+ return transaction_update_reflog(cb-transaction, cb-refname,
+nsha1, osha1, email, timestamp, tz,
+message, 0, cb-err);
}
-static int write_ref_sha1(struct ref_lock *lock, const unsigned char *sha1,
- const char *logmsg);
-
int rename_ref(const char *oldrefname, const char *newrefname, const char
*logmsg)
{
- unsigned char sha1[20], orig_sha1[20];
- int flag = 0, logmoved = 0;
- struct ref_lock *lock;
- struct stat loginfo;
- int log = !lstat(git_path(logs/%s, oldrefname), loginfo);
+ unsigned char sha1[20];
+ int flag = 0, log;
+ struct ref_transaction *transaction = NULL;
+ struct strbuf err = STRBUF_INIT;
const char *symref = NULL;
+ struct rename_reflog_cb cb;
- if (log S_ISLNK(loginfo.st_mode))
- return error(reflog for %s is a symlink, oldrefname);
-
- symref = resolve_ref_unsafe(oldrefname, orig_sha1,
+ symref = resolve_ref_unsafe(oldrefname, sha1,
RESOLVE_REF_READING, flag);
- if (flag REF_ISSYMREF)
- return error(refname %s is a symbolic ref, renaming it is not
supported,
+ if (flag REF_ISSYMREF) {
+ error(refname %s is
[PATCH 5/5] refs.c: rollback the lockfile before we die() in repack_without_refs
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
refs.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/refs.c b/refs.c
index a5053bf..619725a 100644
--- a/refs.c
+++ b/refs.c
@@ -2570,8 +2570,10 @@ int repack_without_refs(const char **refnames, int n,
struct strbuf *err)
/* Remove any other accumulated cruft */
do_for_each_entry_in_dir(packed, 0, curate_packed_ref_fn,
refs_to_delete);
for_each_string_list_item(ref_to_delete, refs_to_delete) {
- if (remove_entry(packed, ref_to_delete-string) == -1)
+ if (remove_entry(packed, ref_to_delete-string) == -1) {
+ rollback_packed_refs();
die(internal error);
+ }
}
/* Write what remains */
--
2.0.1.508.g763ab16
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/5] refs.c: return error instead of dying when locking fails during transaction
Change lock_ref_sha1_basic to return an error instead of dying when we fail to lock a file during a transaction. Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- refs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/refs.c b/refs.c index 6c55032..2ea85a8 100644 --- a/refs.c +++ b/refs.c @@ -2214,7 +2214,7 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname, */ goto retry; else - unable_to_lock_index_die(ref_file, errno); + goto error_return; } if (bad_ref) return lock; -- 2.0.1.508.g763ab16 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/5] ref-transactions-rename
This patch series adds support for using transactions and atomic renames. It focuses on what needs to be done in order to support fully atomic and rollbackable renames that may or may not involve name conflicts. By performing the actual delete old/create new via a single operation to the packed refs file this process will also appear as an atomic change for any external observers. (While this series is short it contains a very important change where we start performing ref changes as operations inside a locked packed refs file instead of as discreete operations of loose ref files. This approach will be extended in the next patch series where we will start using it also to make multi-ref creations/updates become fully atomic for external observers.) This series is built on iand depend on the previous reflog series: * rs/ref-transaction-reflog (2014-07-23) 15 commits - refs.c: allow deleting refs with a broken sha1 - refs.c: remove lock_any_ref_for_update - refs.c: make unlock_ref/close_ref/commit_ref static - refs.c: rename log_ref_setup to create_reflog - reflog.c: use a reflog transaction when writing during expire - refs.c: allow multiple reflog updates during a single transaction - refs.c: only write reflog update if msg is non-NULL - refs.c: add a flag to allow reflog updates to truncate the log - refs.c: add a transaction function to append a reflog entry - lockfile.c: make hold_lock_file_for_append preserve meaningful errno - refs.c: add a function to append a reflog entry to a fd - refs.c: add a new update_type field to ref_update - refs.c: rename the transaction functions - refs.c: make ref_transaction_delete a wrapper for ref_transaction_update - refs.c: make ref_transaction_create a wrapper to ref_transaction_update (this branch uses rs/ref-transaction and rs/ref-transaction-1.) Ronnie Sahlberg (5): refs.c: allow passing raw git_committer_info as email to _update_reflog refs.c: return error instead of dying when locking fails during transaction refs.c: use packed refs when deleting refs during a transaction refs.c: update rename_ref to use a transaction refs.c: rollback the lockfile before we die() in repack_without_refs builtin/remote.c | 13 +- refs.c| 369 +- refs.h| 1 + t/t3200-branch.sh | 7 -- 4 files changed, 210 insertions(+), 180 deletions(-) -- 2.0.1.508.g763ab16 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/5] refs.c: use packed refs when deleting refs during a transaction
Make the deletion of refs during a transaction more atomic.
Start by first copying all loose refs we will be deleting to the packed
refs file and then commit the packed refs file. Then re-lock the packed refs
file to stop anyone else from modifying these refs and keep it locked until
we are finished.
Since all refs we are about to delete are now safely held in the packed refs
file we can proceed to immediately unlink any corresponding loose refs
and still be fully rollback-able.
The exception is for refs that can not be resolved. Those refs are never
added to the packed refs and will just be un-rollback-ably deleted during
commit.
By deleting all the loose refs at the start of the transaction we make make
it possible to both delete one ref and then re-create a different ref in
the same transaction even if the two refs would normally conflict.
Example: rename m-m/m
In that example we want to delete the file 'm' so that we make room so
that we can create a directory with the same name in order to lock and
write to the ref m/m and its lock-file m/m.lock.
If there is a failure during the commit phase we can rollback without losing
any refs since we have so far only deleted loose refs that that are
guaranteed to also have a corresponding entry in the packed refs file.
Once we have finished all updates for refs and their reflogs we can repack
the packed refs file and remove the to-be-deleted refs from the packed refs,
at which point all the deleted refs will disappear in one atomic rename
operation.
This also means that for an outside observer, deletion of multiple refs
in a single transaction will look atomic instead of one ref being deleted
at a time.
In order to do all this we need to change the semantics for the
repack_without_refs function so that we can lock the packed refs file,
do other stuff, and later be able to call repack_without_refs with the
lock already taken.
This means we need some additional changes in remote.c to reflect the
changes to the repack_without_refs semantics.
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
builtin/remote.c | 13 +++--
refs.c | 151 +++
2 files changed, 128 insertions(+), 36 deletions(-)
diff --git a/builtin/remote.c b/builtin/remote.c
index be8ebac..9a9cc92 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -756,6 +756,8 @@ static int remove_branches(struct string_list *branches)
branch_names = xmalloc(branches-nr * sizeof(*branch_names));
for (i = 0; i branches-nr; i++)
branch_names[i] = branches-items[i].string;
+ if (lock_packed_refs(0))
+ result |= unable_to_lock_error(git_path(packed-refs), errno);
result |= repack_without_refs(branch_names, branches-nr, NULL);
free(branch_names);
@@ -1333,9 +1335,14 @@ static int prune_remote(const char *remote, int dry_run)
delete_refs = xmalloc(states.stale.nr * sizeof(*delete_refs));
for (i = 0; i states.stale.nr; i++)
delete_refs[i] = states.stale.items[i].util;
- if (!dry_run)
- result |= repack_without_refs(delete_refs,
- states.stale.nr, NULL);
+ if (!dry_run) {
+ if (lock_packed_refs(0))
+ result |= unable_to_lock_error(
+ git_path(packed-refs), errno);
+ else
+ result |= repack_without_refs(delete_refs,
+ states.stale.nr, NULL);
+ }
free(delete_refs);
}
diff --git a/refs.c b/refs.c
index 2ea85a8..0d800f1 100644
--- a/refs.c
+++ b/refs.c
@@ -1330,7 +1330,7 @@ static struct ref_entry *get_packed_ref(const char
*refname)
/*
* A loose ref file doesn't exist; check for a packed ref. The
- * options are forwarded from resolve_safe_unsafe().
+ * options are forwarded from resolve_ref_unsafe().
*/
static const char *handle_missing_loose_ref(const char *refname,
unsigned char *sha1,
@@ -1387,7 +1387,6 @@ const char *resolve_ref_unsafe(const char *refname,
unsigned char *sha1, int fla
}
git_snpath(path, sizeof(path), %s, refname);
-
/*
* We might have to loop back here to avoid a race
* condition: first we lstat() the file, then we try
@@ -2532,6 +2531,9 @@ static int curate_packed_ref_fn(struct ref_entry *entry,
void *cb_data)
return 0;
}
+/*
+ * Must be called with packed refs already locked (and sorted)
+ */
int repack_without_refs(const char **refnames, int n, struct strbuf *err)
{
struct ref_dir *packed;
@@ -2544,19 +2546,12 @@ int repack_without_refs(const char **refnames, int n,
struct strbuf *err)
if
Re: [PATCH 5/7] enforce `xfuncname` precedence over `funcname`
Matthieu Moy matthieu@grenoble-inp.fr writes: Tanay Abhra tanay...@gmail.com writes: On 7/25/2014 2:52 AM, Ramsay Jones wrote: However, I think it you could create a list of pointer to hash-table entry, string-list index pairs in the config_set and use that to do the iteration. A bit ugly, but it should work. Thanks for the advice, that is exactly what I am doing. I'd just replace list with array and use Documentation/technical/api-allocation-growing.txt. But I can't think of a better way. Presumably this array will reflect the order the source file told us about the keys and their values; I wonder if the filename, lineno information we already have can be used (or unified) with it? Is the when the last variable in a section disappears, make the section header go away, with sensible handling of comments around it part of this GSoC project? If so, I suspect that the feature would also want to know where things appear in the original file, so this data structure may play a role there, too. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 5/6] config: add `git_die_config()` to the config-set API
Tanay Abhra tanay...@gmail.com writes:
On 7/25/2014 7:33 PM, Matthieu Moy wrote:
Tanay Abhra tanay...@gmail.com writes:
--- a/config.c
+++ b/config.c
@@ -1403,11 +1403,12 @@ const struct string_list
*git_configset_get_value_multi(struct config_set *cs, c
int git_configset_get_string_const(struct config_set *cs, const char *key,
const char **dest)
{
- const char *value;
- if (!git_configset_get_value(cs, key, value))
- return git_config_string(dest, key, value);
- else
- return 1;
+ int ret;
+ char *value;
+ ret = git_configset_get_string(cs, key, value);
+ if (ret = 0)
+ *dest = (const char*)value;
+ return ret;
}
Isn't this a fixup meant for another series?
Though v12 is in pu, Junio commented that git_configset_get_string_const()
git_configset_get_string() can be done more concisely, I was trying to do
that but I failed.
My comment on that version was not about conciseness. You had one
that called git_config_string() to let the callee do the nonbool
error handling and xstrdup() of the non-error return value, and the
other one that did exactly what a call to git_config_string() would
have done. That is being redundant, not just failing to be concise.
I was actually hoping that we would see just
int git_configset_get_string(struct config_set *cs, const char *key, char
**dest)
{
return git_configset_get_string_const(cs, key, (const char **)dest);
}
with the implementation of _const() variant be the one from v12.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 1/3] config --global --edit: create a template file if needed
Matthieu Moy matthieu@imag.fr writes:
When the user has no ~/.gitconfig file, git config --global --edit used
to launch an editor on an nonexistant file name.
Instead, create a file with a default content before launching the
editor. The template contains only commented-out entries, to save a few
keystrokes for the user. If the values are guessed properly, the user
will only have to uncomment the entries.
Advanced users teaching newbies can create a minimalistic configuration
faster for newbies. Beginners reading a tutorial advising to run git
config --global --edit as a first step will be slightly more guided for
their first contact with Git.
Signed-off-by: Matthieu Moy matthieu@imag.fr
---
Probably a good idea; I do not think of any possible interactions we
have to worry about with the configuration file init-db creates with
possible templating.
Do we use user-wide as a phrase to refer to these? It sounds
somewhat funny to call anything specific to $frotz $frotz-wide, at
least to me.
Surely, /etc/gitconfig is called site-wide. But .git/config is
per-project (or project-specific), and I would always have thought
that ~/.gitconfig was per-user.
builtin/config.c | 30 +++---
cache.h | 1 +
ident.c | 2 +-
3 files changed, 29 insertions(+), 4 deletions(-)
diff --git a/builtin/config.c b/builtin/config.c
index fcd8474..3821697 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -445,6 +445,20 @@ static int get_urlmatch(const char *var, const char *url)
return 0;
}
+static char *default_user_config()
static char *default_user_config(void)
+{
+ struct strbuf buf = STRBUF_INIT;
+ strbuf_addf(buf,
+ _(# This is Git's user-wide configuration file.\n
+ [core]\n
+ # Please, adapt and uncomment the following lines:\n
tangent: is it a French tradition to always have comma after please?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/7] enforce `xfuncname` precedence over `funcname`
Junio C Hamano gits...@pobox.com writes: Matthieu Moy matthieu@grenoble-inp.fr writes: Tanay Abhra tanay...@gmail.com writes: On 7/25/2014 2:52 AM, Ramsay Jones wrote: However, I think it you could create a list of pointer to hash-table entry, string-list index pairs in the config_set and use that to do the iteration. A bit ugly, but it should work. Thanks for the advice, that is exactly what I am doing. I'd just replace list with array and use Documentation/technical/api-allocation-growing.txt. But I can't think of a better way. Presumably this array will reflect the order the source file told us about the keys and their values; I wonder if the filename, lineno information we already have can be used (or unified) with it? I've thought about this too, and I think it would be really hard. First, there are several files in the picture (eg. /etc/gitconfig, ~/.gitconfig and .git/config), and even included files hence it's not even a lexical order (file, line). Then, even if we had a way to order elements with (file, line), iterating over the hashtable in this order wouldn't be easy (the naive way, get the smallest, get the second smallest, ... would be O(n^2)). -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 1/3] config --global --edit: create a template file if needed
On Fri, Jul 25, 2014 at 12:01 PM, Matthieu Moy
matthieu@grenoble-inp.fr wrote:
Eric Sunshine sunsh...@sunshineco.com writes:
+static char *default_user_config()
+{
+ struct strbuf buf = STRBUF_INIT;
+ strbuf_addf(buf,
+ _(# This is Git's user-wide configuration file.\n
+ [core]\n
+ # Please, adapt and uncomment the following lines:\n
+ #user = %s\n
+ #email = %s\n),
[core], user =, email = should not be translated. Would it make
sense to keep these outside of _()?
I would say no, as the code and the string to translate would be much
less readable without core, user and email inline.
Were you suggesting stg like
_(# This is Git's user-wide configuration file.\n
[%s]\n
# Please, adapt and uncomment the following lines:\n
#%s = %s\n
#%s = %s\n),
core, name, ..., email, ...
?
That or some equivalent variation. I'm not a translator, but the above
seems to convey sufficient context for a translator to understand what
needs to be said, while preventing accidental translations of those
strings which should not be translated.
+ if (fd) {
+ char *content = default_user_config();
+ write_str_in_full(fd, content);
close(fd);
Indeed.
+ free(content);
+ }
+ else if (errno != EEXIST)
+ die_errno(_(Cannot create configuration
file %s), config_file);
Other error messages in this file (including those just above this
block) begin with a lowercase letter.
Applied.
Thanks,
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 1/3] config --global --edit: create a template file if needed
Junio C Hamano gits...@pobox.com writes:
Probably a good idea; I do not think of any possible interactions we
have to worry about with the configuration file init-db creates with
possible templating.
The feature should trigger only for --global, so it shouldn't interfer
with .git/config and templates.
Do we use user-wide as a phrase to refer to these? It sounds
somewhat funny to call anything specific to $frotz $frotz-wide, at
least to me.
Surely, /etc/gitconfig is called site-wide. But .git/config is
per-project (or project-specific), and I would always have thought
that ~/.gitconfig was per-user.
I'm not a native speaker, but to me, user-wide insists on the fact
that it applies to everything for this user, and per-user insists on
the fact that it does not apply to other users.
Perhaps just Git's user configuration file would be enough.
builtin/config.c | 30 +++---
cache.h | 1 +
ident.c | 2 +-
3 files changed, 29 insertions(+), 4 deletions(-)
diff --git a/builtin/config.c b/builtin/config.c
index fcd8474..3821697 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -445,6 +445,20 @@ static int get_urlmatch(const char *var, const char
*url)
return 0;
}
+static char *default_user_config()
static char *default_user_config(void)
Right. Doing too much C++.
+{
+struct strbuf buf = STRBUF_INIT;
+strbuf_addf(buf,
+_(# This is Git's user-wide configuration file.\n
+ [core]\n
+ # Please, adapt and uncomment the following lines:\n
tangent: is it a French tradition to always have comma after please?
Perhaps. In French, the comma would be required after S'il vous plait
(litterally, if you like). I'll remove it.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/5] refs.c: make repack_without_refs static
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
refs.c | 2 +-
refs.h | 3 ---
2 files changed, 1 insertion(+), 4 deletions(-)
diff --git a/refs.c b/refs.c
index efa4f0d..5696d18 100644
--- a/refs.c
+++ b/refs.c
@@ -2534,7 +2534,7 @@ static int curate_packed_ref_fn(struct ref_entry *entry,
void *cb_data)
/*
* Must be called with packed refs already locked (and sorted)
*/
-int repack_without_refs(const char **refnames, int n, struct strbuf *err)
+static int repack_without_refs(const char **refnames, int n, struct strbuf
*err)
{
struct ref_dir *packed;
struct string_list refs_to_delete = STRING_LIST_INIT_DUP;
diff --git a/refs.h b/refs.h
index eb918a0..7183e8e 100644
--- a/refs.h
+++ b/refs.h
@@ -155,9 +155,6 @@ extern void rollback_packed_refs(void);
*/
int pack_refs(unsigned int flags);
-extern int repack_without_refs(const char **refnames, int n,
- struct strbuf *err);
-
extern int ref_exists(const char *);
/*
--
2.0.1.518.g4f5a8ad
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/5] refs.c: move reflog updates into its own function
write_ref_sha1 tries to update the reflog while updating the ref.
Move these reflog changes out into its own function so that we can do the
same thing if we write a sha1 ref differently, for example by writing a ref
to the packed refs file instead.
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
refs.c | 62 --
1 file changed, 36 insertions(+), 26 deletions(-)
diff --git a/refs.c b/refs.c
index 619725a..1048017 100644
--- a/refs.c
+++ b/refs.c
@@ -2869,6 +2869,39 @@ static int is_branch(const char *refname)
return !strcmp(refname, HEAD) || starts_with(refname, refs/heads/);
}
+static int write_sha1_update_reflog(struct ref_lock *lock,
+ const unsigned char *sha1, const char *logmsg)
+{
+ if (log_ref_write(lock-ref_name, lock-old_sha1, sha1, logmsg) 0 ||
+ (strcmp(lock-ref_name, lock-orig_ref_name)
+log_ref_write(lock-orig_ref_name, lock-old_sha1, sha1, logmsg)
0)) {
+ return -1;
+ }
+ if (strcmp(lock-orig_ref_name, HEAD) != 0) {
+ /*
+* Special hack: If a branch is updated directly and HEAD
+* points to it (may happen on the remote side of a push
+* for example) then logically the HEAD reflog should be
+* updated too.
+* A generic solution implies reverse symref information,
+* but finding all symrefs pointing to the given branch
+* would be rather costly for this rare event (the direct
+* update of a branch) to be worth it. So let's cheat and
+* check with HEAD only which should cover 99% of all usage
+* scenarios (even 100% of the default ones).
+*/
+ unsigned char head_sha1[20];
+ int head_flag;
+ const char *head_ref;
+ head_ref = resolve_ref_unsafe(HEAD, head_sha1,
+ RESOLVE_REF_READING, head_flag);
+ if (head_ref (head_flag REF_ISSYMREF)
+ !strcmp(head_ref, lock-ref_name))
+ log_ref_write(HEAD, lock-old_sha1, sha1, logmsg);
+ }
+ return 0;
+}
+
/*
* Writes sha1 into the ref specified by the lock. Makes sure that errno
* is sane on error.
@@ -2912,34 +2945,10 @@ static int write_ref_sha1(struct ref_lock *lock,
return -1;
}
clear_loose_ref_cache(ref_cache);
- if (log_ref_write(lock-ref_name, lock-old_sha1, sha1, logmsg) 0 ||
- (strcmp(lock-ref_name, lock-orig_ref_name)
-log_ref_write(lock-orig_ref_name, lock-old_sha1, sha1, logmsg)
0)) {
+ if (write_sha1_update_reflog(lock, sha1, logmsg)) {
unlock_ref(lock);
return -1;
}
- if (strcmp(lock-orig_ref_name, HEAD) != 0) {
- /*
-* Special hack: If a branch is updated directly and HEAD
-* points to it (may happen on the remote side of a push
-* for example) then logically the HEAD reflog should be
-* updated too.
-* A generic solution implies reverse symref information,
-* but finding all symrefs pointing to the given branch
-* would be rather costly for this rare event (the direct
-* update of a branch) to be worth it. So let's cheat and
-* check with HEAD only which should cover 99% of all usage
-* scenarios (even 100% of the default ones).
-*/
- unsigned char head_sha1[20];
- int head_flag;
- const char *head_ref;
- head_ref = resolve_ref_unsafe(HEAD, head_sha1,
- RESOLVE_REF_READING, head_flag);
- if (head_ref (head_flag REF_ISSYMREF)
- !strcmp(head_ref, lock-ref_name))
- log_ref_write(HEAD, lock-old_sha1, sha1, logmsg);
- }
if (commit_ref(lock)) {
error(Couldn't set %s, lock-ref_name);
unlock_ref(lock);
@@ -3649,7 +3658,8 @@ int transaction_commit(struct ref_transaction
*transaction,
continue;
if (get_packed_ref(update-refname))
continue;
- if (!resolve_ref_unsafe(update-refname, sha1, 1, NULL))
+ if (!resolve_ref_unsafe(update-refname, sha1,
+ RESOLVE_REF_READING, NULL))
continue;
add_packed_ref(update-refname, sha1);
--
2.0.1.518.g4f5a8ad
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/5] refs.c: write updates to packed refs when a transaction has more than one ref
When we are updating more than one single ref, i.e. not a commit, then
write the updated refs directly to the packed refs file instead of writing
them as loose refs.
Change clone to use a transaction instead of using the packed refs API.
This changes the behavior of clone slightly. Previously clone would always
clone all refs into a packed refs file. With this change clone will only
clone into packed refs iff there are two or more refs being cloned.
If the repository we are cloning from only contains exactly one single ref
then clone will now store this as a loose ref. The benefit here is that
we no longer need to export a bunch of API functions to clone to access
packed refs directly. Clone can now just use a normal transaction and all
the packed refs goodness will happen automatically.
Update the t5516 test to cope with the fact that clone now only uses
packed refs if there are more than one ref being cloned.
We still use loose refs for single ref transactions, such as are used
by 'git commit' and friends. The reason for this is that if you have very
many refs then having to re-write the whole packed refs file for every
common operation like commit would have a performance impact.
That said, with these changes it should now be fairly straightforward to
add support to optionally start using packed refs for ALL updates
which could solve existing issues with name clashes in case insensitive
filesystems.
This change also means that multi-ref updates will now appear as a single
atomic change to any external observers instead of a sequence of discreete
changes.
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
builtin/clone.c | 16 +++---
refs.c| 83 +--
t/t5516-fetch-push.sh | 2 +-
3 files changed, 74 insertions(+), 27 deletions(-)
diff --git a/builtin/clone.c b/builtin/clone.c
index f7307e6..7737e12 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -497,17 +497,25 @@ static struct ref *wanted_peer_refs(const struct ref
*refs,
static void write_remote_refs(const struct ref *local_refs)
{
const struct ref *r;
+ struct ref_transaction *transaction;
+ struct strbuf err = STRBUF_INIT;
- lock_packed_refs(LOCK_DIE_ON_ERROR);
+ transaction = transaction_begin(err);
+ if (!transaction)
+ die(%s, err.buf);
for (r = local_refs; r; r = r-next) {
if (!r-peer_ref)
continue;
- add_packed_ref(r-peer_ref-name, r-old_sha1);
+ if (transaction_update_sha1(transaction, r-peer_ref-name,
+ r-old_sha1, NULL, 0, 0, NULL,
+ err))
+ die(%s, err.buf);
}
- if (commit_packed_refs())
- die_errno(unable to overwrite old ref-pack file);
+ if (transaction_commit(transaction, err))
+ die(%s, err.buf);
+ transaction_free(transaction);
}
static void write_followtags(const struct ref *refs, const char *msg)
diff --git a/refs.c b/refs.c
index 1048017..efa4f0d 100644
--- a/refs.c
+++ b/refs.c
@@ -2539,33 +2539,18 @@ int repack_without_refs(const char **refnames, int n,
struct strbuf *err)
struct ref_dir *packed;
struct string_list refs_to_delete = STRING_LIST_INIT_DUP;
struct string_list_item *ref_to_delete;
- int i, ret, removed = 0;
+ int i, ret;
/* Look for a packed ref */
for (i = 0; i n; i++)
if (get_packed_ref(refnames[i]))
break;
- /* Avoid processing if we have nothing to do */
- if (i == n) {
- rollback_packed_refs();
- return 0; /* no refname exists in packed refs */
- }
-
packed = get_packed_refs(ref_cache);
/* Remove refnames from the cache */
for (i = 0; i n; i++)
- if (remove_entry(packed, refnames[i]) != -1)
- removed = 1;
- if (!removed) {
- /*
-* All packed entries disappeared while we were
-* acquiring the lock.
-*/
- rollback_packed_refs();
- return 0;
- }
+ remove_entry(packed, refnames[i]);
/* Remove any other accumulated cruft */
do_for_each_entry_in_dir(packed, 0, curate_packed_ref_fn,
refs_to_delete);
@@ -3614,6 +3599,7 @@ int transaction_commit(struct ref_transaction
*transaction,
struct strbuf *err)
{
int ret = 0, delnum = 0, i, df_conflict = 0, need_repack = 0;
+ int num_updates = 0;
const char **delnames;
int n = transaction-nr;
struct packed_ref_cache *packed_ref_cache;
@@ -3647,19 +3633,38 @@ int transaction_commit(struct ref_transaction
*transaction,
goto cleanup;
}
- /* any loose refs are to be
[PATCH 0/5] use packed refs for ref-transactions
This is a small patch series that continues ontop of the ref-transactions-rename series I posted earlier today. This series expands the use of the packed refs file to make multi-ref updates much more atomic. Additionally it allows us to continue pushing external caller to use transactions instead of accessing (packed) refs directly which allows us to remove a whole bunch of refs functions from the public api. After this series there is no real need any more to expose that packed refs even exist to external callers. This series is built ontop of the previous series that was posted earlier today as : [PATCH 0/5] ref-transactions-rename Ronnie Sahlberg (5): refs.c: move reflog updates into its own function refs.c: write updates to packed refs when a transaction has more than one ref remote.c: use a transaction for deleting refs refs.c: make repack_without_refs static refs.c: make the *_packed_refs functions static builtin/clone.c | 16 -- builtin/remote.c | 70 +-- refs.c| 153 +- refs.h| 33 --- t/t5516-fetch-push.sh | 2 +- 5 files changed, 153 insertions(+), 121 deletions(-) -- 2.0.1.518.g4f5a8ad -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/5] remote.c: use a transaction for deleting refs
Transactions now use packed refs when deleting multiple refs so there is no
need to do it manually from remote.c any more.
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
builtin/remote.c | 70 +++-
1 file changed, 39 insertions(+), 31 deletions(-)
diff --git a/builtin/remote.c b/builtin/remote.c
index 9a9cc92..3e6ef08 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -750,25 +750,27 @@ static int mv(int argc, const char **argv)
static int remove_branches(struct string_list *branches)
{
- const char **branch_names;
int i, result = 0;
+ struct ref_transaction *transaction;
+ struct strbuf err = STRBUF_INIT;
+
+ transaction = transaction_begin(err);
+ if (!transaction)
+ die(%s, err.buf);
- branch_names = xmalloc(branches-nr * sizeof(*branch_names));
for (i = 0; i branches-nr; i++)
- branch_names[i] = branches-items[i].string;
- if (lock_packed_refs(0))
- result |= unable_to_lock_error(git_path(packed-refs), errno);
- result |= repack_without_refs(branch_names, branches-nr, NULL);
- free(branch_names);
-
- for (i = 0; i branches-nr; i++) {
- struct string_list_item *item = branches-items + i;
- const char *refname = item-string;
-
- if (delete_ref(refname, NULL, 0))
- result |= error(_(Could not remove branch %s),
refname);
- }
+ if (transaction_delete_sha1(transaction,
+ branches-items[i].string, NULL,
+ 0, 0, remote-branches, err)) {
+ result |= error(%s, err.buf);
+ goto cleanup;
+ }
+ if (transaction_commit(transaction, err))
+ result |= error(%s, err.buf);
+ cleanup:
+ strbuf_release(err);
+ transaction_free(transaction);
return result;
}
@@ -1317,10 +1319,11 @@ static int prune_remote(const char *remote, int dry_run)
int result = 0, i;
struct ref_states states;
struct string_list delete_refs_list = STRING_LIST_INIT_NODUP;
- const char **delete_refs;
const char *dangling_msg = dry_run
? _( %s will become dangling!)
: _( %s has become dangling!);
+ struct ref_transaction *transaction = NULL;
+ struct strbuf err = STRBUF_INIT;
memset(states, 0, sizeof(states));
get_remote_ref_states(remote, states, GET_REF_STATES);
@@ -1331,28 +1334,26 @@ static int prune_remote(const char *remote, int dry_run)
states.remote-url_nr
? states.remote-url[0]
: _((no URL)));
-
- delete_refs = xmalloc(states.stale.nr * sizeof(*delete_refs));
- for (i = 0; i states.stale.nr; i++)
- delete_refs[i] = states.stale.items[i].util;
- if (!dry_run) {
- if (lock_packed_refs(0))
- result |= unable_to_lock_error(
- git_path(packed-refs), errno);
- else
- result |= repack_without_refs(delete_refs,
- states.stale.nr, NULL);
- }
- free(delete_refs);
}
+ if (!dry_run) {
+ transaction = transaction_begin(err);
+ if (!transaction)
+ die(%s, err.buf);
+ }
for (i = 0; i states.stale.nr; i++) {
const char *refname = states.stale.items[i].util;
string_list_insert(delete_refs_list, refname);
- if (!dry_run)
- result |= delete_ref(refname, NULL, 0);
+ if (!dry_run) {
+ if (transaction_delete_sha1(transaction,
+ refname, NULL,
+ 0, 0, remote-branches, err)) {
+ result |= error(%s, err.buf);
+ goto cleanup;
+ }
+ }
if (dry_run)
printf_ln(_( * [would prune] %s),
@@ -1362,6 +1363,13 @@ static int prune_remote(const char *remote, int dry_run)
abbrev_ref(refname, refs/remotes/));
}
+ if (!dry_run)
+ if (transaction_commit(transaction, err))
+ result |= error(%s, err.buf);
+
+ cleanup:
+ strbuf_release(err);
+ transaction_free(transaction);
warn_dangling_symrefs(stdout, dangling_msg, delete_refs_list);
string_list_clear(delete_refs_list, 0);
--
2.0.1.518.g4f5a8ad
--
To unsubscribe from this list: send the line unsubscribe git in
the
[PATCH 5/5] refs.c: make the *_packed_refs functions static
We no longer need to expose the lock/add/commit/rollback functions
for packed refs anymore so make them static and remove them from the
public api.
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
refs.c | 8
refs.h | 30 --
2 files changed, 4 insertions(+), 34 deletions(-)
diff --git a/refs.c b/refs.c
index 5696d18..d2ee8a2 100644
--- a/refs.c
+++ b/refs.c
@@ -1135,7 +1135,7 @@ static struct ref_dir *get_packed_refs(struct ref_cache
*refs)
return get_packed_ref_dir(get_packed_ref_cache(refs));
}
-void add_packed_ref(const char *refname, const unsigned char *sha1)
+static void add_packed_ref(const char *refname, const unsigned char *sha1)
{
struct packed_ref_cache *packed_ref_cache =
get_packed_ref_cache(ref_cache);
@@ -2268,7 +2268,7 @@ static int write_packed_entry_fn(struct ref_entry *entry,
void *cb_data)
}
/* This should return a meaningful errno on failure */
-int lock_packed_refs(int flags)
+static int lock_packed_refs(int flags)
{
struct packed_ref_cache *packed_ref_cache;
@@ -2291,7 +2291,7 @@ int lock_packed_refs(int flags)
* Commit the packed refs changes.
* On error we must make sure that errno contains a meaningful value.
*/
-int commit_packed_refs(void)
+static int commit_packed_refs(void)
{
struct packed_ref_cache *packed_ref_cache =
get_packed_ref_cache(ref_cache);
@@ -2316,7 +2316,7 @@ int commit_packed_refs(void)
return error;
}
-void rollback_packed_refs(void)
+static void rollback_packed_refs(void)
{
struct packed_ref_cache *packed_ref_cache =
get_packed_ref_cache(ref_cache);
diff --git a/refs.h b/refs.h
index 7183e8e..ec52e13 100644
--- a/refs.h
+++ b/refs.h
@@ -112,36 +112,6 @@ extern void warn_dangling_symref(FILE *fp, const char
*msg_fmt, const char *refn
extern void warn_dangling_symrefs(FILE *fp, const char *msg_fmt, const struct
string_list* refnames);
/*
- * Lock the packed-refs file for writing. Flags is passed to
- * hold_lock_file_for_update(). Return 0 on success.
- * Errno is set to something meaningful on error.
- */
-extern int lock_packed_refs(int flags);
-
-/*
- * Add a reference to the in-memory packed reference cache. This may
- * only be called while the packed-refs file is locked (see
- * lock_packed_refs()). To actually write the packed-refs file, call
- * commit_packed_refs().
- */
-extern void add_packed_ref(const char *refname, const unsigned char *sha1);
-
-/*
- * Write the current version of the packed refs cache from memory to
- * disk. The packed-refs file must already be locked for writing (see
- * lock_packed_refs()). Return zero on success.
- * Sets errno to something meaningful on error.
- */
-extern int commit_packed_refs(void);
-
-/*
- * Rollback the lockfile for the packed-refs file, and discard the
- * in-memory packed reference cache. (The packed-refs file will be
- * read anew if it is needed again after this function is called.)
- */
-extern void rollback_packed_refs(void);
-
-/*
* Flags for controlling behaviour of pack_refs()
* PACK_REFS_PRUNE: Prune loose refs after packing
* PACK_REFS_ALL: Pack _all_ refs, not just tags and already packed refs
--
2.0.1.518.g4f5a8ad
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 1/3] config --global --edit: create a template file if needed
Matthieu Moy matthieu@grenoble-inp.fr writes:
Eric Sunshine sunsh...@sunshineco.com writes:
+static char *default_user_config()
+{
+ struct strbuf buf = STRBUF_INIT;
+ strbuf_addf(buf,
+ _(# This is Git's user-wide configuration file.\n
+ [core]\n
+ # Please, adapt and uncomment the following lines:\n
+ #user = %s\n
+ #email = %s\n),
[core], user =, email = should not be translated. Would it make
sense to keep these outside of _()?
I would say no, as the code and the string to translate would be much
less readable without core, user and email inline.
Were you suggesting stg like
_(# This is Git's user-wide configuration file.\n
[%s]\n
# Please, adapt and uncomment the following lines:\n
#%s = %s\n
#%s = %s\n),
core, name, ..., email, ...
?
;-) That is a clever way to say what my first reaction to Eric's
comment was, which was to have this as multiple strbuf_addf().
Technically speaking, the '#' at the beginning of lines must not be
translated, either, and if that goes without saying, i.e. if the
translators know well enough not to change them, then I can be
persuaded that we can expect that translators know well enough not
to touch the three substrings Eric pointed out.
So, the original message may be fine as-is.
+ if (fd) {
+ char *content = default_user_config();
+ write_str_in_full(fd, content);
close(fd);
Indeed.
+ free(content);
+ }
+ else if (errno != EEXIST)
+ die_errno(_(Cannot create configuration
file %s), config_file);
Other error messages in this file (including those just above this
block) begin with a lowercase letter.
Applied.
Thanks,
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 2/3] home_config_path: allow NULL xdg parameter
This allows a caller to request the global config file without requesting
the XDG one.
Signed-off-by: Matthieu Moy matthieu@imag.fr
---
path.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/path.c b/path.c
index 3afcdb4..f68df0c 100644
--- a/path.c
+++ b/path.c
@@ -148,10 +148,12 @@ void home_config_paths(char **global, char **xdg, char
*file)
*global = mkpathdup(%s/.gitconfig, home);
}
- if (!xdg_home)
- *xdg = NULL;
- else
- *xdg = mkpathdup(%s/git/%s, xdg_home, file);
+ if (xdg) {
+ if (!xdg_home)
+ *xdg = NULL;
+ else
+ *xdg = mkpathdup(%s/git/%s, xdg_home, file);
+ }
free(to_free);
}
--
2.0.2.737.gfb43bde
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/3] config --global --edit: create a template file if needed
When the user has no ~/.gitconfig file, git config --global --edit used
to launch an editor on an nonexistant file name.
Instead, create a file with a default content before launching the
editor. The template contains only commented-out entries, to save a few
keystrokes for the user. If the values are guessed properly, the user
will only have to uncomment the entries.
Advanced users teaching newbies can create a minimalistic configuration
faster for newbies. Beginners reading a tutorial advising to run git
config --global --edit as a first step will be slightly more guided for
their first contact with Git.
Signed-off-by: Matthieu Moy matthieu@imag.fr
---
Hopefully, all remarks applied. I went for Git's user configuration, but
I can change it to whatever native speakers think is better.
builtin/config.c | 31 ---
cache.h | 1 +
ident.c | 2 +-
3 files changed, 30 insertions(+), 4 deletions(-)
diff --git a/builtin/config.c b/builtin/config.c
index fcd8474..1363478 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -445,6 +445,20 @@ static int get_urlmatch(const char *var, const char *url)
return 0;
}
+static char *default_user_config(void)
+{
+ struct strbuf buf = STRBUF_INIT;
+ strbuf_addf(buf,
+ _(# This is Git's user configuration file.\n
+ [core]\n
+ # Please adapt and uncomment the following lines:\n
+ #user = %s\n
+ #email = %s\n),
+ ident_default_name(),
+ ident_default_email());
+ return strbuf_detach(buf, NULL);
+}
+
int cmd_config(int argc, const char **argv, const char *prefix)
{
int nongit = !startup_info-have_repository;
@@ -551,6 +565,8 @@ int cmd_config(int argc, const char **argv, const char
*prefix)
}
}
else if (actions == ACTION_EDIT) {
+ const char *config_file = given_config_source.file ?
+ given_config_source.file : git_path(config);
check_argc(argc, 0, 0);
if (!given_config_source.file nongit)
die(not in a git directory);
@@ -559,9 +575,18 @@ int cmd_config(int argc, const char **argv, const char
*prefix)
if (given_config_source.blob)
die(editing blobs is not supported);
git_config(git_default_config, NULL);
- launch_editor(given_config_source.file ?
- given_config_source.file : git_path(config),
- NULL, NULL);
+ if (use_global_config) {
+ int fd = open(config_file, O_CREAT | O_EXCL | O_WRONLY,
0666);
+ if (fd) {
+ char *content = default_user_config();
+ write_str_in_full(fd, content);
+ free(content);
+ close(fd);
+ }
+ else if (errno != EEXIST)
+ die_errno(_(cannot create configuration file
%s), config_file);
+ }
+ launch_editor(config_file, NULL, NULL);
}
else if (actions == ACTION_SET) {
int ret;
diff --git a/cache.h b/cache.h
index fcb511d..b06cbb2 100644
--- a/cache.h
+++ b/cache.h
@@ -1061,6 +1061,7 @@ extern const char *git_author_info(int);
extern const char *git_committer_info(int);
extern const char *fmt_ident(const char *name, const char *email, const char
*date_str, int);
extern const char *fmt_name(const char *name, const char *email);
+extern const char *ident_default_name(void);
extern const char *ident_default_email(void);
extern const char *git_editor(void);
extern const char *git_pager(int stdout_is_tty);
diff --git a/ident.c b/ident.c
index 1d9b6e7..77bc882 100644
--- a/ident.c
+++ b/ident.c
@@ -102,7 +102,7 @@ static void copy_email(const struct passwd *pw, struct
strbuf *email)
add_domainname(email);
}
-static const char *ident_default_name(void)
+const char *ident_default_name(void)
{
if (!git_default_name.len) {
copy_gecos(xgetpwuid_self(), git_default_name);
--
2.0.2.737.gfb43bde
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 3/3] commit: advertise config --global --edit on guessed identity
When the user has no user-wide configuration file, it's faster to use the
newly introduced config file template than to run two commands to set
user.name and user.email. Advise this to the user.
The old advice is kept if the user already has a configuration file since
the template feature would not trigger in this case.
Signed-off-by: Matthieu Moy matthieu@imag.fr
---
builtin/commit.c | 34 --
1 file changed, 32 insertions(+), 2 deletions(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index 5ed6036..7502b0e 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -42,7 +42,20 @@ static const char * const builtin_status_usage[] = {
NULL
};
-static const char implicit_ident_advice[] =
+static const char implicit_ident_advice_noconfig[] =
+N_(Your name and email address were configured automatically based\n
+on your username and hostname. Please check that they are accurate.\n
+You can suppress this message by setting them explicitly. Run the\n
+following command and follow the instructions in your editor to edit\n
+your configuration file:\n
+\n
+git config --global --edit\n
+\n
+After doing this, you may fix the identity used for this commit with:\n
+\n
+git commit --amend --reset-author\n);
+
+static const char implicit_ident_advice_config[] =
N_(Your name and email address were configured automatically based\n
on your username and hostname. Please check that they are accurate.\n
You can suppress this message by setting them explicitly:\n
@@ -1402,6 +1415,23 @@ int cmd_status(int argc, const char **argv, const char
*prefix)
return 0;
}
+static const char *implicit_ident_advice() {
+ char *user_config = NULL;
+ char *xdg_config = NULL;
+ int config_exists;
+
+ home_config_paths(user_config, xdg_config, config);
+ config_exists = file_exists(user_config) || file_exists(xdg_config);
+ free(user_config);
+ free(xdg_config);
+
+ if (config_exists)
+ return _(implicit_ident_advice_config);
+ else
+ return _(implicit_ident_advice_noconfig);
+
+}
+
static void print_summary(const char *prefix, const unsigned char *sha1,
int initial_commit)
{
@@ -1440,7 +1470,7 @@ static void print_summary(const char *prefix, const
unsigned char *sha1,
strbuf_addbuf_percentquote(format, committer_ident);
if (advice_implicit_identity) {
strbuf_addch(format, '\n');
- strbuf_addstr(format, _(implicit_ident_advice));
+ strbuf_addstr(format, implicit_ident_advice());
}
}
strbuf_release(author_ident);
--
2.0.2.737.gfb43bde
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 2/3] home_config_path: allow NULL xdg parameter
Matthieu Moy matthieu@imag.fr writes:
This allows a caller to requst the global config file without requesting
the XDG one.
Signed-off-by: Matthieu Moy matthieu@imag.fr
---
This is actually not needed, but I wrote this for a previous version,
and it seems sensible anyway.
I was about to say Let's not do this until some caller needs it,
implicitly assuming that we do not let global=NULL to signal that
the caller is interested only in XDG, but I checked and we do check
the NULL-ness of global, so it is consistent to do so for xdg. The
change makes very good sense.
I wouldn't have had to spend the time to dig, if the log message
justified it that way, instead of having actually not needed
comment there ;-)
home_config_paths(): let the caller ignore xdg path
The caller can signal that it is not interested in learning
the location of $HOME/.gitconfig by passing global=NULL, but
there is no way to decline the ptah to the configuration
file based on $XDG_CONFIG_HOME.
Allow the caller to pass xdg=NULL to signal that it is not
interested in the XDG location.
or something, perhaps?
path.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/path.c b/path.c
index 3afcdb4..f68df0c 100644
--- a/path.c
+++ b/path.c
@@ -148,10 +148,12 @@ void home_config_paths(char **global, char **xdg, char
*file)
*global = mkpathdup(%s/.gitconfig, home);
}
- if (!xdg_home)
- *xdg = NULL;
- else
- *xdg = mkpathdup(%s/git/%s, xdg_home, file);
+ if (xdg) {
+ if (!xdg_home)
+ *xdg = NULL;
+ else
+ *xdg = mkpathdup(%s/git/%s, xdg_home, file);
+ }
free(to_free);
}
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 2/3] home_config_path: allow NULL xdg parameter
Junio C Hamano gits...@pobox.com writes: Matthieu Moy matthieu@imag.fr writes: This allows a caller to requst the global config file without requesting the XDG one. Signed-off-by: Matthieu Moy matthieu@imag.fr --- This is actually not needed, but I wrote this for a previous version, and it seems sensible anyway. I was about to say Let's not do this until some caller needs it, implicitly assuming that we do not let global=NULL to signal that the caller is interested only in XDG, but I checked and we do check the NULL-ness of global, so it is consistent to do so for xdg. The change makes very good sense. I wouldn't have had to spend the time to dig, if the log message justified it that way, instead of having actually not needed comment there ;-) home_config_paths(): let the caller ignore xdg path The caller can signal that it is not interested in learning the location of $HOME/.gitconfig by passing global=NULL, but there is no way to decline the ptah to the configuration file based on $XDG_CONFIG_HOME. Allow the caller to pass xdg=NULL to signal that it is not interested in the XDG location. Good point. Applied locally, I'll resend later. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/7] enforce `xfuncname` precedence over `funcname`
Matthieu Moy matthieu@grenoble-inp.fr writes: I'd just replace list with array and use Documentation/technical/api-allocation-growing.txt. But I can't think of a better way. Presumably this array will reflect the order the source file told us about the keys and their values; I wonder if the filename, lineno information we already have can be used (or unified) with it? I've thought about this too, and I think it would be really hard. Yeah, I do not offhand think of a good solution, either, without going pointer-crazy and have bidirectional link everywhere X-. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/3] config --global --edit: create a template file if needed
Matthieu Moy matthieu@imag.fr writes:
When the user has no ~/.gitconfig file, git config --global --edit used
to launch an editor on an nonexistant file name.
Instead, create a file with a default content before launching the
editor. The template contains only commented-out entries, to save a few
keystrokes for the user. If the values are guessed properly, the user
will only have to uncomment the entries.
Advanced users teaching newbies can create a minimalistic configuration
faster for newbies. Beginners reading a tutorial advising to run git
config --global --edit as a first step will be slightly more guided for
their first contact with Git.
Signed-off-by: Matthieu Moy matthieu@imag.fr
---
Hopefully, all remarks applied. I went for Git's user configuration, but
I can change it to whatever native speakers think is better.
I am not native but I have a slight worry that a template that lists
only core.{user,email} and marked as Git's user configuration will
easily mislead the reader that the file is only about these two
(i.e. I am telling the stupid Git who I am), and is not meant to
hold any other configuration that are private to the user. I still
have funny feeling about user-wide, but Git's user configuration
somehow smells worse for that reason.
What the patch attempts to do is a good idea and the code looks done
right. Let's queue it and hope people can polish the wording.
Thanks.
builtin/config.c | 31 ---
cache.h | 1 +
ident.c | 2 +-
3 files changed, 30 insertions(+), 4 deletions(-)
diff --git a/builtin/config.c b/builtin/config.c
index fcd8474..1363478 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -445,6 +445,20 @@ static int get_urlmatch(const char *var, const char *url)
return 0;
}
+static char *default_user_config(void)
+{
+ struct strbuf buf = STRBUF_INIT;
+ strbuf_addf(buf,
+ _(# This is Git's user configuration file.\n
+ [core]\n
+ # Please adapt and uncomment the following lines:\n
+ #user = %s\n
+ #email = %s\n),
+ ident_default_name(),
+ ident_default_email());
+ return strbuf_detach(buf, NULL);
+}
+
int cmd_config(int argc, const char **argv, const char *prefix)
{
int nongit = !startup_info-have_repository;
@@ -551,6 +565,8 @@ int cmd_config(int argc, const char **argv, const char
*prefix)
}
}
else if (actions == ACTION_EDIT) {
+ const char *config_file = given_config_source.file ?
+ given_config_source.file : git_path(config);
check_argc(argc, 0, 0);
if (!given_config_source.file nongit)
die(not in a git directory);
@@ -559,9 +575,18 @@ int cmd_config(int argc, const char **argv, const char
*prefix)
if (given_config_source.blob)
die(editing blobs is not supported);
git_config(git_default_config, NULL);
- launch_editor(given_config_source.file ?
- given_config_source.file : git_path(config),
- NULL, NULL);
+ if (use_global_config) {
+ int fd = open(config_file, O_CREAT | O_EXCL | O_WRONLY,
0666);
+ if (fd) {
+ char *content = default_user_config();
+ write_str_in_full(fd, content);
+ free(content);
+ close(fd);
+ }
+ else if (errno != EEXIST)
+ die_errno(_(cannot create configuration file
%s), config_file);
+ }
+ launch_editor(config_file, NULL, NULL);
}
else if (actions == ACTION_SET) {
int ret;
diff --git a/cache.h b/cache.h
index fcb511d..b06cbb2 100644
--- a/cache.h
+++ b/cache.h
@@ -1061,6 +1061,7 @@ extern const char *git_author_info(int);
extern const char *git_committer_info(int);
extern const char *fmt_ident(const char *name, const char *email, const char
*date_str, int);
extern const char *fmt_name(const char *name, const char *email);
+extern const char *ident_default_name(void);
extern const char *ident_default_email(void);
extern const char *git_editor(void);
extern const char *git_pager(int stdout_is_tty);
diff --git a/ident.c b/ident.c
index 1d9b6e7..77bc882 100644
--- a/ident.c
+++ b/ident.c
@@ -102,7 +102,7 @@ static void copy_email(const struct passwd *pw, struct
strbuf *email)
add_domainname(email);
}
-static const char *ident_default_name(void)
+const char *ident_default_name(void)
{
if (!git_default_name.len) {
copy_gecos(xgetpwuid_self(), git_default_name);
--
To unsubscribe from this
Re: [PATCH v2 2/3] home_config_path: allow NULL xdg parameter
Matthieu Moy matthieu@imag.fr writes:
This allows a caller to request the global config file without requesting
the XDG one.
Signed-off-by: Matthieu Moy matthieu@imag.fr
---
Will rephrase
A caller can ask only for XDG location by passing global=NULL.
Allow it to ask only for the $HOME/.gitconfig by passing
xdg=NULL.
as you seem to have forgotten ;-)
path.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/path.c b/path.c
index 3afcdb4..f68df0c 100644
--- a/path.c
+++ b/path.c
@@ -148,10 +148,12 @@ void home_config_paths(char **global, char **xdg, char
*file)
*global = mkpathdup(%s/.gitconfig, home);
}
- if (!xdg_home)
- *xdg = NULL;
- else
- *xdg = mkpathdup(%s/git/%s, xdg_home, file);
+ if (xdg) {
+ if (!xdg_home)
+ *xdg = NULL;
+ else
+ *xdg = mkpathdup(%s/git/%s, xdg_home, file);
+ }
free(to_free);
}
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 1/3] config --global --edit: create a template file if needed
When the user has no ~/.gitconfig file, git config --global --edit used
to launch an editor on an nonexistant file name.
Instead, create a file with a default content before launching the
editor. The template contains only commented-out entries, to save a few
keystrokes for the user. If the values are guessed properly, the user
will only have to uncomment the entries.
Advanced users teaching newbies can create a minimalistic configuration
faster for newbies. Beginners reading a tutorial advising to run git
config --global --edit as a first step will be slightly more guided for
their first contact with Git.
Signed-off-by: Matthieu Moy matthieu@imag.fr
---
I am not native but I have a slight worry that a template that lists
only core.{user,email} and marked as Git's user configuration will
easily mislead the reader that the file is only about these two
Good point, here's the one with per-user, and PATCH 2 with your
improved commit message.
builtin/config.c | 31 ---
cache.h | 1 +
ident.c | 2 +-
3 files changed, 30 insertions(+), 4 deletions(-)
diff --git a/builtin/config.c b/builtin/config.c
index fcd8474..aba7135 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -445,6 +445,20 @@ static int get_urlmatch(const char *var, const char *url)
return 0;
}
+static char *default_user_config(void)
+{
+ struct strbuf buf = STRBUF_INIT;
+ strbuf_addf(buf,
+ _(# This is Git's per-user configuration file.\n
+ [core]\n
+ # Please adapt and uncomment the following lines:\n
+ #user = %s\n
+ #email = %s\n),
+ ident_default_name(),
+ ident_default_email());
+ return strbuf_detach(buf, NULL);
+}
+
int cmd_config(int argc, const char **argv, const char *prefix)
{
int nongit = !startup_info-have_repository;
@@ -551,6 +565,8 @@ int cmd_config(int argc, const char **argv, const char
*prefix)
}
}
else if (actions == ACTION_EDIT) {
+ const char *config_file = given_config_source.file ?
+ given_config_source.file : git_path(config);
check_argc(argc, 0, 0);
if (!given_config_source.file nongit)
die(not in a git directory);
@@ -559,9 +575,18 @@ int cmd_config(int argc, const char **argv, const char
*prefix)
if (given_config_source.blob)
die(editing blobs is not supported);
git_config(git_default_config, NULL);
- launch_editor(given_config_source.file ?
- given_config_source.file : git_path(config),
- NULL, NULL);
+ if (use_global_config) {
+ int fd = open(config_file, O_CREAT | O_EXCL | O_WRONLY,
0666);
+ if (fd) {
+ char *content = default_user_config();
+ write_str_in_full(fd, content);
+ free(content);
+ close(fd);
+ }
+ else if (errno != EEXIST)
+ die_errno(_(cannot create configuration file
%s), config_file);
+ }
+ launch_editor(config_file, NULL, NULL);
}
else if (actions == ACTION_SET) {
int ret;
diff --git a/cache.h b/cache.h
index fcb511d..b06cbb2 100644
--- a/cache.h
+++ b/cache.h
@@ -1061,6 +1061,7 @@ extern const char *git_author_info(int);
extern const char *git_committer_info(int);
extern const char *fmt_ident(const char *name, const char *email, const char
*date_str, int);
extern const char *fmt_name(const char *name, const char *email);
+extern const char *ident_default_name(void);
extern const char *ident_default_email(void);
extern const char *git_editor(void);
extern const char *git_pager(int stdout_is_tty);
diff --git a/ident.c b/ident.c
index 1d9b6e7..77bc882 100644
--- a/ident.c
+++ b/ident.c
@@ -102,7 +102,7 @@ static void copy_email(const struct passwd *pw, struct
strbuf *email)
add_domainname(email);
}
-static const char *ident_default_name(void)
+const char *ident_default_name(void)
{
if (!git_default_name.len) {
copy_gecos(xgetpwuid_self(), git_default_name);
--
2.0.2.737.gfb43bde
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 2/3] home_config_paths(): let the caller ignore xdg path
The caller can signal that it is not interested in learning
the location of $HOME/.gitconfig by passing global=NULL, but
there is no way to decline the path to the configuration
file based on $XDG_CONFIG_HOME.
Allow the caller to pass xdg=NULL to signal that it is not
interested in the XDG location.
Commit-message-by: Junio C Hamano gits...@pobox.com
Signed-off-by: Matthieu Moy matthieu@imag.fr
---
path.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/path.c b/path.c
index 3afcdb4..f68df0c 100644
--- a/path.c
+++ b/path.c
@@ -148,10 +148,12 @@ void home_config_paths(char **global, char **xdg, char
*file)
*global = mkpathdup(%s/.gitconfig, home);
}
- if (!xdg_home)
- *xdg = NULL;
- else
- *xdg = mkpathdup(%s/git/%s, xdg_home, file);
+ if (xdg) {
+ if (!xdg_home)
+ *xdg = NULL;
+ else
+ *xdg = mkpathdup(%s/git/%s, xdg_home, file);
+ }
free(to_free);
}
--
2.0.2.737.gfb43bde
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 3/3] commit: advertise config --global --edit on guessed identity
When the user has no user-wide configuration file, it's faster to use the
newly introduced config file template than to run two commands to set
user.name and user.email. Advise this to the user.
The old advice is kept if the user already has a configuration file since
the template feature would not trigger in this case.
Signed-off-by: Matthieu Moy matthieu@imag.fr
---
builtin/commit.c | 34 --
1 file changed, 32 insertions(+), 2 deletions(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index 5ed6036..7502b0e 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -42,7 +42,20 @@ static const char * const builtin_status_usage[] = {
NULL
};
-static const char implicit_ident_advice[] =
+static const char implicit_ident_advice_noconfig[] =
+N_(Your name and email address were configured automatically based\n
+on your username and hostname. Please check that they are accurate.\n
+You can suppress this message by setting them explicitly. Run the\n
+following command and follow the instructions in your editor to edit\n
+your configuration file:\n
+\n
+git config --global --edit\n
+\n
+After doing this, you may fix the identity used for this commit with:\n
+\n
+git commit --amend --reset-author\n);
+
+static const char implicit_ident_advice_config[] =
N_(Your name and email address were configured automatically based\n
on your username and hostname. Please check that they are accurate.\n
You can suppress this message by setting them explicitly:\n
@@ -1402,6 +1415,23 @@ int cmd_status(int argc, const char **argv, const char
*prefix)
return 0;
}
+static const char *implicit_ident_advice() {
+ char *user_config = NULL;
+ char *xdg_config = NULL;
+ int config_exists;
+
+ home_config_paths(user_config, xdg_config, config);
+ config_exists = file_exists(user_config) || file_exists(xdg_config);
+ free(user_config);
+ free(xdg_config);
+
+ if (config_exists)
+ return _(implicit_ident_advice_config);
+ else
+ return _(implicit_ident_advice_noconfig);
+
+}
+
static void print_summary(const char *prefix, const unsigned char *sha1,
int initial_commit)
{
@@ -1440,7 +1470,7 @@ static void print_summary(const char *prefix, const
unsigned char *sha1,
strbuf_addbuf_percentquote(format, committer_ident);
if (advice_implicit_identity) {
strbuf_addch(format, '\n');
- strbuf_addstr(format, _(implicit_ident_advice));
+ strbuf_addstr(format, implicit_ident_advice());
}
}
strbuf_release(author_ident);
--
2.0.2.737.gfb43bde
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 1/3] config --global --edit: create a template file if needed
Thanks; will queue the patches from this iteration. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/5] refs.c: allow passing raw git_committer_info as email to _update_reflog
Ronnie Sahlberg wrote: Add a new flag REFLOG_EMAIL_IS_COMMITTER to _update_reflog to tell it that what we pass in as email is already the fully baked committer string we can use as-is. With and without the new flag, the 'email' argument has two different meanings: * with the new flag, it should be an ident string, like 'Jonathan Nieder jrnie...@gmail.com 1406251347 -0700' * without it, it should be the name-part of an ident string, like 'Jonathan Nieder jrnie...@gmail.com In neither case is it an email address. This seems unnecessarily confusing. Is the caller responsible for checking the argument for validity? Do callers do so? Is this performance-critical or could the transaction_update_reflog function do a sanity-check? [...] /* * Append a reflog entry for refname. If the REFLOG_TRUNCATE flag is set * this update will first truncate the reflog before writing the entry. * If msg is NULL no update will be written to the log. */ int transaction_update_reflog(struct ref_transaction *transaction, const char *refname, const unsigned char *new_sha1, const unsigned char *old_sha1, const char *email, unsigned long timestamp, int tz, const char *msg, int flags, struct strbuf *err); This is a lot of parameters, some optional, not all documented. Would it make sense to pack some into a struct? Thanks and hope that helps, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 3/3] commit: advertise config --global --edit on guessed identity
Matthieu Moy matthieu@imag.fr writes:
+static const char *implicit_ident_advice() {
Style:
static const char *implicit_ident_advice(void)
{
No need to resend.
Thanks.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/5] refs.c: return error instead of dying when locking fails during transaction
Ronnie Sahlberg wrote: --- a/refs.c +++ b/refs.c @@ -2214,7 +2214,7 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname, */ goto retry; else - unable_to_lock_index_die(ref_file, errno); + goto error_return; Should probably save last_errno so error_return can pass that information back. Can the caller recover from this error? Does it have enough information to produce the same helpful message as unable_to_lock_index_die? If this could be done without regressing behavior (e.g., by passing back error information as a message instead of through errno) then I think it would make sense. Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/5] refs.c: use packed refs when deleting refs during a transaction
Ronnie Sahlberg wrote:
Make the deletion of refs during a transaction more atomic.
Start by first copying all loose refs we will be deleting to the packed
refs file and then commit the packed refs file. Then re-lock the packed refs
file to stop anyone else from modifying these refs and keep it locked until
we are finished.
[...]
By deleting all the loose refs at the start of the transaction we make make
it possible to both delete one ref and then re-create a different ref in
the same transaction even if the two refs would normally conflict.
Example: rename m-m/m
Makes a lot of sense.
This can potentially slow down a single-ref deletion in a repository with
a huge amount of refs (e.g., a Gerrit server with lots of
refs/changes/* refs), right? But the cost is not more than a factor
of 2 worse than if the ref had been packed (since you have to
repack_without_refs anyway) so I think this is acceptable.
[...]
The exception is for refs that can not be resolved. Those refs are never
added to the packed refs and will just be un-rollback-ably deleted during
commit.
This also seems reasonable --- there's no need to be able to roll back
into an invalid state. :)
[...]
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -756,6 +756,8 @@ static int remove_branches(struct string_list *branches)
branch_names = xmalloc(branches-nr * sizeof(*branch_names));
for (i = 0; i branches-nr; i++)
branch_names[i] = branches-items[i].string;
+ if (lock_packed_refs(0))
+ result |= unable_to_lock_error(git_path(packed-refs), errno);
result |= repack_without_refs(branch_names, branches-nr, NULL);
What should happen if lock_packed_refs fails? Since
repack_without_refs is now documented to require a locked packed-refs
file, shouldn't the repack_without_refs call be guarded, like it is
below?
[...]
@@ -1333,9 +1335,14 @@ static int prune_remote(const char *remote, int
dry_run)
delete_refs = xmalloc(states.stale.nr * sizeof(*delete_refs));
for (i = 0; i states.stale.nr; i++)
delete_refs[i] = states.stale.items[i].util;
- if (!dry_run)
- result |= repack_without_refs(delete_refs,
- states.stale.nr, NULL);
+ if (!dry_run) {
+ if (lock_packed_refs(0))
+ result |= unable_to_lock_error(
+ git_path(packed-refs), errno);
+ else
+ result |= repack_without_refs(delete_refs,
+ states.stale.nr, NULL);
[...]
--- a/refs.c
+++ b/refs.c
@@ -1330,7 +1330,7 @@ static struct ref_entry *get_packed_ref(const char
*refname)
/*
* A loose ref file doesn't exist; check for a packed ref. The
- * options are forwarded from resolve_safe_unsafe().
+ * options are forwarded from resolve_ref_unsafe().
Unrelated changes snuck in? (A good change, but it presumably belongs
as a separate patch.)
[...]
@@ -1387,7 +1387,6 @@ const char *resolve_ref_unsafe(const char *refname,
unsigned char *sha1, int fla
}
git_snpath(path, sizeof(path), %s, refname);
-
/*
Why?
[...]
@@ -2532,6 +2531,9 @@ static int curate_packed_ref_fn(struct ref_entry
*entry, void *cb_data)
return 0;
}
+/*
+ * Must be called with packed refs already locked (and sorted)
+ */
int repack_without_refs(const char **refnames, int n, struct strbuf *err)
Aren't packed refs always sorted? If it needs mentioning, that seems
more like an implementation comment.
The 'packed refs must be locked' kind of documentation belongs in
refs.h since this is part of the API. Usually when there are API
changes like this we like to change the function name or signature to
make it easy to catch callers that were introduced without noticing
the change --- would that be easy to do here?
E.g. introducing a new repack_without_refs_locked (name is just a
placeholder, I'm bad at names) and keeping repack_without_refs with
the current semantics as a wrapper around that.
[...]
@@ -2544,19 +2546,12 @@ int repack_without_refs(const char **refnames, int n,
struct strbuf *err)
if (get_packed_ref(refnames[i]))
break;
- /* Avoid locking if we have nothing to do */
+ /* Avoid processing if we have nothing to do */
This implies a small speed regression but I don't think anyone would
mind.
[...]
@@ -3692,12 +3689,65 @@ int transaction_commit(struct ref_transaction
*transaction,
goto cleanup;
}
- /* Acquire all ref locks while verifying old values */
+ /* Lock packed refs during commit */
+ if (lock_packed_refs(0)) {
I stopped reviewing here but assume the rest is okay. :)
Thanks,
Jonathan
--
To unsubscribe from this list: send the line
Re: [PATCH 4/5] refs.c: update rename_ref to use a transaction
Ronnie Sahlberg wrote: Change refs.c to use a single transaction to copy/rename both the refs and its reflog. Yay! Since we are no longer using rename() to move the reflog file we no longer need to disallow rename_ref for refs with a symlink for its reflog so we can remove that test from the testsuite. It's generally better to update the testsuite with the new expected behavior instead. :) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Amending merge commits?
Hi folks, I think one of my coworkers has stumbled on a git bug -- if you amend a merge commit, and then pull, your amends are lost. Is this expected behavior? I've reproduced the problem in a script (attached). I ran it against a couple of versions of git (1.7.1, 1.7.9, 1.8.4, 2.0.0) and in each case it seemed to lose the amend. - Dave amend-merge.sh Description: amend-merge.sh
Re: What's cooking in git.git (Jul 2014, #04; Tue, 22)
Junio C Hamano gits...@pobox.com writes:
Philip Oakley philipoak...@iee.org writes:
which still makes me feel hesitant to promote this
document without updating its contents, though.
I hadn't viewed it as a 'promotion', rather it was simply ensuring
access to the guide via the help system, instead of leaving it
somewhat hidden.
Stale or incorrect pieces of advice that are hidden will not harm
(non-)readers. Making them more available would mean giving them
more chances to do harm ;-).
Let's disect it. I do not have time/concentration to finish all in
one message, so I'll do the first two.
| Everyday Git With 20 Commands Or So
| ===
|
| Individual Developer (Standalone) commands are essential for
| anybody who makes a commit, even for somebody who works alone.
|
| If you work with other people, you will need commands listed in
| the Individual Developer (Participant) section as well.
|
| People who play the Integrator role need to learn some more
| commands in addition to the above.
|
| Repository Administration commands are for system
| administrators who are responsible for the care and feeding
| of Git repositories.
Let's assume the categorization above is sensible for now.
| Individual Developer (Standalone)[[Individual Developer (Standalone)]]
| --
|
| A standalone individual developer does not exchange patches with
| other people, and works alone in a single repository, using the
| following commands.
| ...
Everything in the enumeration looks OK except for this one,
| * linkgit:git-show-branch[1] to see where you are.
which is probably no longer the case, for two reasons. In the
original design of Git UI, you see many things that encourage you to
view and think about the whole collection of your branches as a whole.
git show-branch whose default is to show all of your branches is
just one of them (other examples include git push whose default
used to be matching, expecting that you will work on completing
all the branches before you push the result out for all of them).
Over time, Git UI mutated into put a lot more focus and stress on
the current branch, disregarding what are on the branches you are
not corrently on. git log @{u}.. to see how much more you have
done on top of others' work would be more in line with such an
attitude.
I'd suggest dropping this command from this enumeration.
| Examples
|
|
| Use a tarball as a starting point for a new repository.::
| ...
Perfectly fine.
| Create a topic branch and develop.::
| +
|
| $ git checkout -b alsa-audio 1
| $ edit/compile/test
| $ git checkout -- curses/ux_audio_oss.c 2
| $ git add curses/ux_audio_alsa.c 3
| $ edit/compile/test
| $ git diff HEAD 4
| $ git commit -a -s 5
Perefectly fine up to this point.
| $ edit/compile/test
| $ git reset --soft HEAD^ 6
| $ edit/compile/test
| $ git diff ORIG_HEAD 7
| $ git commit -a -c ORIG_HEAD 8
This shows how a typical sequence I try to further tweak what I
committed looked like. With the modern Git, you would do
$ edit/compile/test
$ git diff HEAD^ 7
$ git commit -a --amend
without the soft reset, which was invented solely because there
wasn't commit --amend available.
The example is perfectly fine after this step, but 6 and 8 need
to be dropped and others renumbered.
| Individual Developer (Participant)[[Individual Developer (Participant)]]
|
| ...
| * linkgit:git-format-patch[1] to prepare e-mail submission, if
| you adopt Linux kernel-style public forum workflow.
Probably git-send-email needs to be appended to the end of the
enumeration.
| Examples
|
|
| Clone the upstream and work on it. Feed changes to upstream.::
| +
|
| $ git clone git://git.kernel.org/pub/scm/.../torvalds/linux-2.6 my2.6
| $ cd my2.6
| $ edit/compile/test; git commit -a -s 1
| $ git format-patch origin 2
| $ git pull 3
| $ git log -p ORIG_HEAD.. arch/i386 include/asm-i386 4
| $ git pull git://git.kernel.org/pub/.../jgarzik/libata-dev.git ALL 5
| $ git reset --hard ORIG_HEAD 6
| $ git gc 7
| $ git fetch --tags 8
|
| +
| 1 repeat as needed.
| 2 extract patches from your branch for e-mail submission.
| 3 `git pull` fetches from `origin` by default and merges into the
| current branch.
| 4 immediately after pulling, look at the changes done upstream
| since last time we checked, only in the
| area we are interested in.
| 5 fetch from a specific branch from a specific repository and merge.
| 6 revert the pull.
| 7 garbage collect leftover objects from reverted pull.
| 8 from time to time, obtain official tags from the `origin`
| and store them under `.git/refs/tags/`.
This example works directly on 'master', which is not ideal. If I
were writing this today, I would have made it work on 'mine' branch,
produced a patch series out of that branch
Re: Amending merge commits?
Besen, David david.besen at hp.com writes: Hi folks, I think one of my coworkers has stumbled on a git bug -- if you amend a merge commit, and then pull, your amends are lost. Is this expected behavior? I've reproduced the problem in a script (attached). I ran it against a couple of versions of git (1.7.1, 1.7.9, 1.8.4, 2.0.0) and in each case it seemed to lose the amend. - Dave Attachment (amend-merge.sh): application/octet-stream, 1061 bytes Whoops, accidentally encoded the script, here it is inline: #!/bin/bash set -ex if [ -z $GIT ]; then GIT=git; fi GIT_MERGE_AUTOEDIT=no # Clean up from the last run rm -rf repo.git repo repo2 || : # Set up a bare remote repo $GIT init --bare repo.git # Check out the remote repo $GIT clone repo.git repo # Add a commit cd repo echo file file.txt $GIT add file.txt $GIT commit -m Add file.txt $GIT push origin master # Make a branch $GIT checkout -b mybranch # Add a commit on the branch echo mybranch file.txt $GIT add . $GIT commit -m Add 'mybranch' line # Go back to master $GIT checkout master # Merge in mybranch to create a merge commit $GIT merge --no-ff mybranch # Push that back $GIT push # Amend the merge commit echo amended file.txt $GIT add . $GIT commit -C HEAD --amend cd .. # Make a second checkout $GIT clone repo.git repo2 cd repo2 # Add some unrelated changes to be pulled echo repo2 file2.txt $GIT add . $GIT commit -m Add file2 $GIT push cd .. cd repo # Pull $GIT pull --rebase # Now, we expect the text amended to be in file.txt grep amended file.txt -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Amending merge commits?
Besen, David wrote: I think one of my coworkers has stumbled on a git bug -- if you amend a merge commit, and then pull, your amends are lost. This is how pull --rebase works. It turns your single-parent commits into a sequence of patches on top of upstream and completely ignores your merge commits. There is a --rebase=preserve option that makes a halfhearted attempt to preserve your merges --- perhaps that would help? The git-rebase(1) documentation has more details. In an ideal world, I think pull --rebase would do the following: 1. Do the same thing it does today 2. Behind the scenes, *also* try a 'pull --merge' but don't save the result. 3. Compare the results. If they differ, show a diff and explain to the user what happened. I may be the only one that wants that, though. Hope that helps, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: Amending merge commits?
Ah thanks, I'll RTFM better in the future. - Dave -Original Message- From: Jonathan Nieder [mailto:jrnie...@gmail.com] Sent: Friday, July 25, 2014 4:19 PM To: Besen, David Cc: git@vger.kernel.org Subject: Re: Amending merge commits? Besen, David wrote: I think one of my coworkers has stumbled on a git bug -- if you amend a merge commit, and then pull, your amends are lost. This is how pull --rebase works. It turns your single-parent commits into a sequence of patches on top of upstream and completely ignores your merge commits. There is a --rebase=preserve option that makes a halfhearted attempt to preserve your merges --- perhaps that would help? The git-rebase(1) documentation has more details. In an ideal world, I think pull --rebase would do the following: 1. Do the same thing it does today 2. Behind the scenes, *also* try a 'pull --merge' but don't save the result. 3. Compare the results. If they differ, show a diff and explain to the user what happened. I may be the only one that wants that, though. Hope that helps, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Amending merge commits?
David Besen wrote: Jonathan Nieder wrote: This is how pull --rebase works. It turns your single-parent commits into a sequence of patches on top of upstream and completely ignores your merge commits. There is a --rebase=preserve option that makes a halfhearted attempt to preserve your merges --- perhaps that would help? The git-rebase(1) documentation has more details. Ah thanks, I'll RTFM better in the future. No, not a problem. It's very useful to see examples of where git's behavior was counterintuitive and the documentation was more obscure than it could have been. I should also emphasize the halfhearted above. There's a lot of room for improvement in rebase --preserve-merges's handling of evil and otherwise amended merges. Thanks again, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] refs.c: move ref parsing code out of resolve_ref()
On Fri, Jul 25, 2014 at 11:12 PM, Ronnie Sahlberg sahlb...@google.com wrote: diff --git a/refs.c b/refs.c index bec2bb1..2769f20 100644 --- a/refs.c +++ b/refs.c @@ -1533,6 +1533,105 @@ static int handle_missing_loose_ref(const char *refname, } } +int parse_ref(const char *path, struct strbuf *ref, + unsigned char *sha1, int *flag) Can you make this function static? It is not used by anything outside of this series and thus making it static avoids growing the public refs api. It's to be used by builtin/checkout.c in nd/multiple-work-trees. I could mark it static now and unmark it later, but I'd need to add the static prototype back in refs.c because in the next patch resolve_gitlink_ref() uses this function and resolve_gitlink_ref() is before parse_ref(). -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
