Re: [PATCH/RFC 2/2] git rebase -i: Warn removed or dupplicated commits
Galan Rémi remi.galan-alfonso at ensimag.grenoble-inp.fr writes: Check if commits were removed (i.e. a line was deleted) or dupplicated (e.g. the same commit is picked twice), can print warnings or abort git rebase according to the value of the configuration variable rebase.checkLevel. I sometimes duplicate commits deliberately if I want to split a commit in two. I move a copy up and fix the conflict, and I know that I'll still get the right thing later even if I make a mistake with the conflict resolution.
[PATCH] t7063: hide stderr from setup inside prereq
When t7063 starts, it runs update-index --untracked-cache
to see if we support the untracked cache. Its output goes
straight to stderr, even if the test is not run with -v.
Let's wrap it in a prereq that will hide the output by
default, but show it with -v.
Signed-off-by: Jeff King p...@peff.net
---
I noticed this messing up my prove output. And it always runs first
with prove --state=slow, because it has a whopping 17 seconds of
sleeps in it.
t/t7063-status-untracked-cache.sh | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/t/t7063-status-untracked-cache.sh
b/t/t7063-status-untracked-cache.sh
index 2b2ffd7..bd4806c 100755
--- a/t/t7063-status-untracked-cache.sh
+++ b/t/t7063-status-untracked-cache.sh
@@ -8,10 +8,14 @@ avoid_racy() {
sleep 1
}
-git update-index --untracked-cache
# It's fine if git update-index returns an error code other than one,
# it'll be caught in the first test.
-if test $? -eq 1; then
+test_lazy_prereq UNTRACKED_CACHE '
+ { git update-index --untracked-cache; ret=$?; }
+ test $ret -ne 1
+'
+
+if ! test_have_prereq UNTRACKED_CACHE; then
skip_all='This system does not support untracked cache'
test_done
fi
--
2.4.1.552.g6de66a4
--
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
Recovering from 'fatal: core.bare and core.worktree do not make sense'
Hi, the other day I said 'git config core.worktree /somewhere' in a bare repo while thinking I was in a regular one, user error. The 'fatal: core.bare and core.worktree do not make sense' error from the next command made me realize immediately that I was wrong, that's good. However... OK, let's have a look and recover from the situation: $ git config --edit fatal: core.bare and core.worktree do not make sense Well, all was well before I set 'core.worktree', so let's unset it: $ git config --unset core.worktree fatal: core.bare and core.worktree do not make sense Hmph, not expecting much, but how about unsetting the other variable? $ git config --unset core.bare fatal: core.bare and core.worktree do not make sense Good, at least it's pretty consistent, though I still don't get what 'git config' has to do with the worktree that is so important that it has to bail out. Time to look for help: $ git help config fatal: core.bare and core.worktree do not make sense WTF :) Alright, I give up: $ vim config $ # happy It was two days later that I had a bit of a lightbulb moment, reproduced the situation and just for fun tried this: $ git -c core.bare=false config --unset core.bare I didn't expect, but it worked! Great. Some thoughts: 1) Perhaps 'git config' should be more careful in the first place and refuse to set 'core.worktree' when 'core.bare' is already true and vice versa. 2) The damage was done with 'git config', so I expected that I can repair it with plain 'git config' (i.e. without 'git -c') as well. 'git config' has nothing to do with the path to the worktree after all. And 'git config --edit' should work regardless of the mess that might be in the config file. 3) 'git help cmd' should always work, shouldn't it? (Though that's the easiest to remedy, just cd out of the repo, or fire up a new terminal window.) Gábor -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC 2/2] git rebase -i: Warn removed or dupplicated commits
Stephen Kelly steve...@gmail.com writes: Galan Rémi remi.galan-alfonso at ensimag.grenoble-inp.fr writes: Check if commits were removed (i.e. a line was deleted) or dupplicated (e.g. the same commit is picked twice), can print warnings or abort git rebase according to the value of the configuration variable rebase.checkLevel. I sometimes duplicate commits deliberately if I want to split a commit in two. I move a copy up and fix the conflict, and I know that I'll still get the right thing later even if I make a mistake with the conflict resolution. The more I think about it, the more I think we should either not warn at all on duplicate commits, or have a separate config variable. It's rare to duplicate by mistake, and when you do so, it's already easy to notice: you get conflicts, and you can git rebase --skip the second occurence. Accidentally dropped commits are another story: it's rather easy to cut-and-forget-to-paste, and the consequence currently is silent data loss ... -- 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-new-workdir: add windows compatibility
Hi Daniel, On 2015-05-26 19:16, Daniel Smith wrote: Thanks to everyone for reviewing my proposed patch an providing valuable feedback. This was my first patch submission to a large open source project like Git and the whole process was a little daunting. Heh, yeah, it can be quite overwhelming (and this mailing list is very male, too). Sorry! I hope, though, that it was obvious we all only wanted to be helpful? On Tue, May 26, 2015 at 9:48 AM, Karsten Blees karsten.bl...@gmail.com wrote: AFAICT, the MSys2 symlink() implementation is pretty smart to detect these conditions and fall back to deep copy (aka 'cp -a') if symlinks are not supported. IOW, using 'ln -s' will hopefully just work in the upcoming Git for Windows 2, thus trying to fix it for MSys1 / Git for Windows 1.9x is probably a lost cause. In that case, I'll abandon this patch and wait for Git for Windows 2 to come out. Speaking of which: Could you try `git-new-workdir` with Git for Windows 2.x (developers' preview)? https://git-for-windows.github.io/#download It *might* be necessary to define the `MSYS` environment variable to `winsymlinks:nativestrict` before starting the Git Bash, to enable symlinking. It would be really good to know if that is the case so that I can do something about this in the Git for Windows installer. Ciao, Johannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] clone: add `--seed` shorthand
On Sun, May 24, 2015 at 12:07:53PM -0700, Junio C Hamano wrote: Jeff King p...@peff.net writes: Having slept on it, I really think --seed should be fetch from the seed into temp refs, and not what I posted earlier. Yeah, I think that is the right way to do it. In the meantime, do you want to pick up patches 1 and 2? I think they are cleanups that stand on their own, whether we do patch 3 or not. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
FW: Query on git submodules
Thanks Heiko for getting back to me. Correct when I referred to 10+ layers I meant nested repositories which make up a large hierarchy. Some repositories are repeated across the hierarchy. We check-out submodules to tag versions (as opposed to master branch). If we need to roll out a particular change across a hierarchy (e.g. 60 repos) then every level in the hierarchy needs to pick up new submodule tags. The main 2 issues that we see are: 1. Enforcement of absolute paths in git submodules - I am currently trialing using a pre-commit hook to highlight this issue to users so that they can fix their submodule urls to relative paths. 2. Slowness Integrating updates to submodule hierarchy -I have been looking at ways of automating such a roll out - this can be useful for new project setups or for rolling out an update to all repos and quickly integrating it into the submodule hierarchy. The link below shows something similar however it checks out a master branch of each submodule in its hierarchy. https://chrisjean.com/recursively-updating-git-submodules/ Thanks, Sarah -Original Message- From: Heiko Voigt [mailto:hvo...@hvoigt.net] Sent: Tuesday, May 26, 2015 6:01 PM To: Frawley, Sarah Cc: git@vger.kernel.org Subject: Re: Query on git submodules Hi, On Fri, May 22, 2015 at 01:46:24PM +, Frawley, Sarah wrote: I am a design automation engineer supporting 200+ designers who use git for hardware design. We also use the submodule feature where we can have quite complex hierarchy's with 10+ layers. What does this 10+ layers mean? Nested submodule repositories 10 recursion steps deep? We have experience issues with re-use of design projects was we move from one derivative to another due to the complexity of the hierarchy along with lack of discipline (using absolute paths in .gitmodule files). To enforce more discipline I am currently working on a pre-commit hook to check the integrity of .gitmodule files. I would be interested in seeing how other users in the community find submodules for large scale projects and if there are any best known methods for using them. I do not have anything to share here since our projects did not have such problems (not that large scale). It would be interesting to see what you come up with. Maybe we can add some of that into core git to support such large scale projects better. So maybe you can share exactly what problems you have (only absolute paths?) or the pre-commit hooks you will use. Cheers Heiko - Intel Ireland Limited (Branch) Collinstown Industrial Park, Leixlip, County Kildare, Ireland Registered Number: E902934 This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC 2/2] git rebase -i: Warn removed or dupplicated commits
Thank you for reviewing the code.
Eric Sunshinesunsh...@sunshineco.com writes:
+ # To uppercase
+ checkLevel=$(echo $checkLevel | tr '[:lower:]' '[:upper:]')
Is there precedence elsewhere for recognizing uppercase and lowercase
variants of config values?
It seems to be commonly used when parsing options in the C files
through strcasecmp. For exemple, in config.c:818 :
if (!strcmp(var, core.safecrlf)) {
if (value !strcasecmp(value, warn)) {
[...]
However we didn't see any precedence in shell files. Do you think we
should remove it?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/5] Fix verify_lock() to report errors via strbuf
On 05/23/2015 01:34 AM, Michael Haggerty wrote: verify_lock() is a helper function called while committing reference transactions. But when it fails, instead of recording its error message in a strbuf to be passed back to the caller of ref_transaction_commit(), the error message was being written directly to stderr. Instead, report the errors via a strbuf so that they make it back to the caller of ref_transaction_commit(). [...] This is the patch series that I mentioned here [1]. It applies on top of mh/ref-directory-file [2] and is thematically a continuation of that series in the sense that it further cleans up the error handling within reference transactions. It would be easy to rebase to master if that is preferred. The last sentence is nonsense. This patch series relies on lock_ref_sha1_basic() having a strbuf *err parameter, which is only the case since 4a32b2e lock_ref_sha1_basic(): report errors via a struct strbuf *err (2015-05-11) The latter commit is in mh/ref-directory-file (which has now been merged to master, so technically the last sentence is now correct again). Sorry for the confusion. Michael -- Michael Haggerty mhag...@alum.mit.edu -- 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/WIP 0/8] Make git-am a builtin
git-am is a commonly used command for applying a series of patches from a mailbox to the current branch. Currently, it is implemented by the shell script git-am.sh. However, compared to C, shell scripts have certain deficiencies: they need to spawn a lot of processes, introduce a lot of dependencies and cannot take advantage of git's internal caches. This WIP patch series rewrites git-am.sh into optimized C builtin/am.c. It is based on my finished prototype of the rewrite[1], and over the next 5 weeks I will be cutting out small patches from the prototype to make it easier to review and refine the patch series. This is part of my GSoC project to rewrite git-pull.sh into git-am.sh into C builtins[2]. A small benchmark that applies 50 patches[3]: #!/bin/sh git init echo initial file git add file git commit -m initial git branch before-am for x in $(seq 50) do echo $x file git commit -a -m $x done git format-patch --stdout before-am.. patches git checkout before-am time git patches /dev/null I ran this benchmark on my *Linux* system. Timings for git on master: 1.40s, 1.42s, 1.25s, 1.32s, 1.10s. Avg: ~1.30s Timings for git on master + this patch series applied: 0.24s, 0.22s, 0.22s, 0.19s, 0.25s. Avg: ~0.22s This is around a 6x speedup. It's not because this patch series does less than git-am.sh -- similar speedups can be observed with the prototype, which passes the test suite[4]. (Sorry for leaving the other reviews hanging. I was too preoccupied with the git-am rewrite, and was afraid of forgetting important details should I context switch. Now that the prototype is finished I can deal with the other patch series'.) [1] https://github.com/pyokagan/git/compare/master...pyokagan:pt/ref-builtin-am [2] https://gist.github.com/pyokagan/1b7b0d1f4dab6ba3cef1 [3] Since a 56-patch series was posted recently ;-) [4] The subset of the test suite that calls git-rebase and git-am. Paul Tan (8): wrapper: implement xopen() wrapper: implement xfopen() am: implement patch queue mechanism am: split out mbox/maildir patches with git-mailsplit am: detect mbox patches am: extract patch, message and authorship with git-mailinfo am: apply patch with git-apply am: commit applied patch Makefile | 2 +- builtin.h | 1 + builtin/am.c | 687 ++ git-compat-util.h | 2 + git.c | 1 + wrapper.c | 37 +++ 6 files changed, 729 insertions(+), 1 deletion(-) create mode 100644 builtin/am.c -- 2.1.4 -- 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/WIP 2/8] wrapper: implement xfopen()
A common usage pattern of fopen() is to check if it succeeded, and die()
if it failed:
FILE *fp = fopen(path, w);
if (!fp)
die_errno(_(could not open '%s' for writing), path);
Implement a wrapper function xfopen() for the above, so that we can save
a few lines of code and make the die() messages consistent.
Signed-off-by: Paul Tan pyoka...@gmail.com
---
git-compat-util.h | 1 +
wrapper.c | 19 +++
2 files changed, 20 insertions(+)
diff --git a/git-compat-util.h b/git-compat-util.h
index 9745962..914d450 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -723,6 +723,7 @@ extern ssize_t xread(int fd, void *buf, size_t len);
extern ssize_t xwrite(int fd, const void *buf, size_t len);
extern ssize_t xpread(int fd, void *buf, size_t len, off_t offset);
extern int xdup(int fd);
+extern FILE *xfopen(const char *path, const char *mode);
extern FILE *xfdopen(int fd, const char *mode);
extern int xmkstemp(char *template);
extern int xmkstemp_mode(char *template, int mode);
diff --git a/wrapper.c b/wrapper.c
index 971665a..d5ed780 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -329,6 +329,25 @@ int xdup(int fd)
return ret;
}
+/**
+ * xfopen() is the same as fopen(), but it die()s if the fopen() fails.
+ */
+FILE *xfopen(const char *path, const char *mode)
+{
+ FILE *fp;
+
+ assert(path);
+ assert(mode);
+ fp = fopen(path, mode);
+ if (!fp) {
+ if (*mode == 'w' || *mode == 'a')
+ die_errno(_(could not open '%s' for writing), path);
+ else
+ die_errno(_(could not open '%s' for reading), path);
+ }
+ return fp;
+}
+
FILE *xfdopen(int fd, const char *mode)
{
FILE *stream = fdopen(fd, mode);
--
2.1.4
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC 2/2] git rebase -i: Warn removed or dupplicated commits
Eric Sunshinesunsh...@sunshineco.com writes: Shouldn't this case also 'die' when rebase.checkLevel is error? And, why doesn't the user get advice about configuring rebase.checkLevel in this case? Stephen Kellysteve...@gmail.com writes: I sometimes duplicate commits deliberately if I want to split a commit in two. Matthieu Moymatthieu@grenoble-inp.fr writes: The more I think about it, the more I think we should either not warn at all on duplicate commits, or have a separate config variable. Put in common because two config variables would have an effect on the 'die' and advise part. In this patch we didn't put the 'die' in the duplicate commit part since there was only one config variable and there are cases where the user might want to duplicate commits. After the code reviewing of Eric Sunshine and Stephen Kelly, we also came to the conclusion that we should use two config variables, one about missing commits and the other about duplicate commits. This way if you deliberately want to use duplicate commits, you can just set the value to 'ignore' for duplicate commits and still have 'warn'/'error' for missing commits. Moreover, each part would have its 'die' depending on the value of the corresponding config variable. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Bug: .gitconfig folder
If you have a folder named ~/.gitconfig instead of a file with that name, when you try to run some global config editing command it will fail with a wrong error message: fatal: Out of memory? mmap failed: No such device You can reproduce it: $rm ~/.gitconfig $mkdir ~/.gitconfig $ls -la ~ ... drwxr-xr-x 24 hit hit 4096 may 4 12:30 .gimp-2.8 drwxr-xr-x 2 hit hit 4096 may 27 15:26 .gitconfig drwxr-xr-x 6 hit hit 4096 may 27 14:01 github ... $git config --global user.name foo fatal: Out of memory? mmap failed: No such device $git config --global core.editor vim fatal: Out of memory? mmap failed: No such device -- 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/WIP 1/8] wrapper: implement xopen()
A common usage pattern of open() is to check if it was successful, and
die() if it was not:
int fd = open(path, O_WRONLY | O_CREAT, 0777);
if (fd 0)
die_errno(_(Could not open '%s' for writing.), path);
Implement a wrapper function xopen() that does the above so that we can
save a few lines of code, and make the die() messages consistent.
Signed-off-by: Paul Tan pyoka...@gmail.com
---
git-compat-util.h | 1 +
wrapper.c | 18 ++
2 files changed, 19 insertions(+)
diff --git a/git-compat-util.h b/git-compat-util.h
index 17584ad..9745962 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -718,6 +718,7 @@ extern char *xstrndup(const char *str, size_t len);
extern void *xrealloc(void *ptr, size_t size);
extern void *xcalloc(size_t nmemb, size_t size);
extern void *xmmap(void *start, size_t length, int prot, int flags, int fd,
off_t offset);
+extern int xopen(const char *path, int flags, mode_t mode);
extern ssize_t xread(int fd, void *buf, size_t len);
extern ssize_t xwrite(int fd, const void *buf, size_t len);
extern ssize_t xpread(int fd, void *buf, size_t len, off_t offset);
diff --git a/wrapper.c b/wrapper.c
index c1a663f..971665a 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -189,6 +189,24 @@ void *xcalloc(size_t nmemb, size_t size)
# endif
#endif
+/**
+ * xopen() is the same as open(), but it die()s if the open() fails.
+ */
+int xopen(const char *path, int flags, mode_t mode)
+{
+ int fd;
+
+ assert(path);
+ fd = open(path, flags, mode);
+ if (fd 0) {
+ if ((flags O_WRONLY) || (flags O_RDWR))
+ die_errno(_(could not open '%s' for writing), path);
+ else
+ die_errno(_(could not open '%s' for reading), path);
+ }
+ return fd;
+}
+
/*
* xread() is the same a read(), but it automatically restarts read()
* operations with a recoverable error (EAGAIN and EINTR). xread()
--
2.1.4
--
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/WIP 3/8] am: implement patch queue mechanism
git-am applies a series of patches. If the process terminates
abnormally, we want to be able to resume applying the series of patches.
This requires the session state to be saved in a persistent location.
Implement the mechanism of a patch queue, represented by 2 integers --
the index of the current patch we are applying and the index of the last
patch, as well as its lifecycle through the following functions:
* am_setup(), which will set up the state directory
$GIT_DIR/rebase-apply. As such, even if the process exits abnormally,
the last-known state will still persist.
* am_state_load(), which is called if there is an am session in
progress, to load the last known state from the state directory so we
can resume applying patches.
* am_run(), which will do the actual patch application. After applying a
patch, it calls am_next() to increment the current patch index. The
logic for applying and committing a patch is not implemented yet.
* am_destroy(), which is finally called when we successfully applied all
the patches in the queue, to clean up by removing the state directory
and its contents.
Signed-off-by: Paul Tan pyoka...@gmail.com
---
Makefile | 2 +-
builtin.h| 1 +
builtin/am.c | 167 +++
git.c| 1 +
4 files changed, 170 insertions(+), 1 deletion(-)
create mode 100644 builtin/am.c
diff --git a/Makefile b/Makefile
index 323c401..57a7c8c 100644
--- a/Makefile
+++ b/Makefile
@@ -466,7 +466,6 @@ TEST_PROGRAMS_NEED_X =
# interactive shell sessions without exporting it.
unexport CDPATH
-SCRIPT_SH += git-am.sh
SCRIPT_SH += git-bisect.sh
SCRIPT_SH += git-difftool--helper.sh
SCRIPT_SH += git-filter-branch.sh
@@ -812,6 +811,7 @@ LIB_OBJS += xdiff-interface.o
LIB_OBJS += zlib.o
BUILTIN_OBJS += builtin/add.o
+BUILTIN_OBJS += builtin/am.o
BUILTIN_OBJS += builtin/annotate.o
BUILTIN_OBJS += builtin/apply.o
BUILTIN_OBJS += builtin/archive.o
diff --git a/builtin.h b/builtin.h
index b87df70..d50c9d1 100644
--- a/builtin.h
+++ b/builtin.h
@@ -30,6 +30,7 @@ extern int textconv_object(const char *path, unsigned mode,
const unsigned char
extern int is_builtin(const char *s);
extern int cmd_add(int argc, const char **argv, const char *prefix);
+extern int cmd_am(int argc, const char **argv, const char *prefix);
extern int cmd_annotate(int argc, const char **argv, const char *prefix);
extern int cmd_apply(int argc, const char **argv, const char *prefix);
extern int cmd_archive(int argc, const char **argv, const char *prefix);
diff --git a/builtin/am.c b/builtin/am.c
new file mode 100644
index 000..6c9
--- /dev/null
+++ b/builtin/am.c
@@ -0,0 +1,167 @@
+/*
+ * Builtin git am
+ *
+ * Based on git-am.sh by Junio C Hamano.
+ */
+#include cache.h
+#include parse-options.h
+#include dir.h
+
+struct am_state {
+ struct strbuf dir;/* state directory path */
+ int cur; /* current patch number */
+ int last; /* last patch number */
+};
+
+/**
+ * Initializes am_state with the default values.
+ */
+static void am_state_init(struct am_state *state)
+{
+ memset(state, 0, sizeof(*state));
+
+ strbuf_init(state-dir, 0);
+}
+
+/**
+ * Release memory allocated by an am_state.
+ */
+static void am_state_release(struct am_state *state)
+{
+ strbuf_release(state-dir);
+}
+
+/**
+ * Returns path relative to the am_state directory.
+ */
+static inline const char *am_path(const struct am_state *state, const char
*path)
+{
+ return mkpath(%s/%s, state-dir.buf, path);
+}
+
+/**
+ * Returns 1 if there is an am session in progress, 0 otherwise.
+ */
+static int am_in_progress(const struct am_state *state)
+{
+ struct stat st;
+
+ if (lstat(state-dir.buf, st) 0 || !S_ISDIR(st.st_mode))
+ return 0;
+ if (lstat(am_path(state, last), st) || !S_ISREG(st.st_mode))
+ return 0;
+ if (lstat(am_path(state, next), st) || !S_ISREG(st.st_mode))
+ return 0;
+ return 1;
+}
+
+/**
+ * Reads the contents of `file`. The third argument can be used to give a hint
+ * about the file size, to avoid reallocs. Returns 0 on success, -1 if the file
+ * does not exist.
+ */
+static int read_state_file(struct strbuf *sb, const char *file, size_t hint) {
+ strbuf_reset(sb);
+
+ if (!strbuf_read_file(sb, file, hint))
+ return 0;
+
+ if (errno == ENOENT)
+ return -1;
+
+ die_errno(_(could not read '%s'), file);
+}
+
+/**
+ * Loads state from disk.
+ */
+static void am_state_load(struct am_state *state)
+{
+ struct strbuf sb = STRBUF_INIT;
+
+ read_state_file(sb, am_path(state, next), 8);
+ state-cur = strtol(sb.buf, NULL, 10);
+
+ read_state_file(sb, am_path(state, last), 8);
+ state-last = strtol(sb.buf, NULL, 10);
+
+ strbuf_release(sb);
+}
+
+/**
+ * Remove the am_state directory.
+ */
+static
[PATCH/WIP 4/8] am: split out mbox/maildir patches with git-mailsplit
git-am.sh supports mbox, stgit and mercurial patches. Re-implement
support for splitting out mbox/maildirs using git-mailsplit, while also
implementing the framework required to support other patch formats in
the future.
Re-implement support for the --patch-format option (since a5a6755
(git-am foreign patch support: introduce patch_format, 2009-05-27)) to
allow the user to choose between the different patch formats.
Signed-off-by: Paul Tan pyoka...@gmail.com
---
builtin/am.c | 104 ---
1 file changed, 100 insertions(+), 4 deletions(-)
diff --git a/builtin/am.c b/builtin/am.c
index 6c9..9c7b058 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -6,11 +6,18 @@
#include cache.h
#include parse-options.h
#include dir.h
+#include run-command.h
+
+enum patch_format {
+ PATCH_FORMAT_UNKNOWN = 0,
+ PATCH_FORMAT_MBOX
+};
struct am_state {
struct strbuf dir;/* state directory path */
int cur; /* current patch number */
int last; /* last patch number */
+ int prec; /* number of digits in patch filename */
};
/**
@@ -21,6 +28,7 @@ static void am_state_init(struct am_state *state)
memset(state, 0, sizeof(*state));
strbuf_init(state-dir, 0);
+ state-prec = 4;
}
/**
@@ -101,13 +109,67 @@ static void am_destroy(const struct am_state *state)
}
/**
+ * Splits out individual patches from `paths`, where each path is either a mbox
+ * file or a Maildir. Return 0 on success, -1 on failure.
+ */
+static int split_patches_mbox(struct am_state *state, struct string_list
*paths)
+{
+ struct child_process cp = CHILD_PROCESS_INIT;
+ struct string_list_item *item;
+ struct strbuf last = STRBUF_INIT;
+
+ cp.git_cmd = 1;
+ argv_array_push(cp.args, mailsplit);
+ argv_array_pushf(cp.args, -d%d, state-prec);
+ argv_array_pushf(cp.args, -o%s, state-dir.buf);
+ argv_array_push(cp.args, -b);
+ argv_array_push(cp.args, --);
+
+ for_each_string_list_item(item, paths)
+ argv_array_push(cp.args, item-string);
+
+ if (capture_command(cp, last, 8))
+ return -1;
+
+ state-cur = 1;
+ state-last = strtol(last.buf, NULL, 10);
+
+ return 0;
+}
+
+/**
+ * Splits out individual patches, of patch_format, contained within paths.
+ * These patches will be stored in the state directory, with each patch's
+ * filename being its index, padded to state-prec digits. state-cur will be
+ * set to the index of the first patch, and state-last will be set to the
+ * index of the last patch. Returns 0 on success, -1 on failure.
+ */
+static int split_patches(struct am_state *state, enum patch_format
patch_format,
+ struct string_list *paths)
+{
+ switch (patch_format) {
+ case PATCH_FORMAT_MBOX:
+ return split_patches_mbox(state, paths);
+ default:
+ die(BUG: invalid patch_format);
+ }
+ return -1;
+}
+
+/**
* Setup a new am session for applying patches
*/
-static void am_setup(struct am_state *state)
+static void am_setup(struct am_state *state, enum patch_format patch_format,
+ struct string_list *paths)
{
if (mkdir(state-dir.buf, 0777) 0 errno != EEXIST)
die_errno(_(failed to create directory '%s'), state-dir.buf);
+ if (split_patches(state, patch_format, paths) 0) {
+ am_destroy(state);
+ die(_(Failed to split patches.));
+ }
+
write_file(am_path(state, next), 1, %d, state-cur);
write_file(am_path(state, last), 1, %d, state-last);
@@ -128,13 +190,32 @@ static void am_next(struct am_state *state)
*/
static void am_run(struct am_state *state)
{
- while (state-cur = state-last)
+ while (state-cur = state-last) {
+ /* patch application not implemented yet */
+
am_next(state);
+ }
am_destroy(state);
}
+/**
+ * parse_options() callback that validates and sets opt-value to the
+ * PATCH_FORMAT_* enum value corresponding to `arg`.
+ */
+static int parse_opt_patchformat(const struct option *opt, const char *arg,
int unset)
+{
+ int *opt_value = opt-value;
+
+ if (!strcmp(arg, mbox))
+ *opt_value = PATCH_FORMAT_MBOX;
+ else
+ return -1;
+ return 0;
+}
+
struct am_state state;
+int opt_patch_format;
static const char * const am_usage[] = {
N_(git am [options] [(mbox|Maildir)...]),
@@ -142,6 +223,8 @@ static const char * const am_usage[] = {
};
static struct option am_options[] = {
+ OPT_CALLBACK(0, patch-format, opt_patch_format, N_(format),
+ N_(format the patch(es) are in), parse_opt_patchformat),
OPT_END()
};
@@ -156,8 +239,21 @@ int cmd_am(int argc, const char **argv, const char
[PATCH/WIP 7/8] am: apply patch with git-apply
Implement applying the patch to the index using git-apply.
Signed-off-by: Paul Tan pyoka...@gmail.com
---
builtin/am.c | 50 +-
1 file changed, 49 insertions(+), 1 deletion(-)
diff --git a/builtin/am.c b/builtin/am.c
index 0b8a42d..7126df3 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -25,6 +25,18 @@ static int is_empty_file(const char *filename)
return !st.st_size;
}
+/**
+ * Returns the first line of msg
+ */
+static const char *firstline(const char *msg)
+{
+ static struct strbuf sb = STRBUF_INIT;
+
+ strbuf_reset(sb);
+ strbuf_add(sb, msg, strchrnul(msg, '\n') - msg);
+ return sb.buf;
+}
+
enum patch_format {
PATCH_FORMAT_UNKNOWN = 0,
PATCH_FORMAT_MBOX
@@ -503,6 +515,29 @@ static int parse_patch(struct am_state *state, const char
*patch)
return 0;
}
+/*
+ * Applies current patch with git-apply. Returns 0 on success, -1 otherwise.
+ */
+static int run_apply(const struct am_state *state)
+{
+ struct child_process cp = CHILD_PROCESS_INIT;
+
+ cp.git_cmd = 1;
+
+ argv_array_push(cp.args, apply);
+ argv_array_push(cp.args, --index);
+ argv_array_push(cp.args, am_path(state, patch));
+
+ if (run_command(cp))
+ return -1;
+
+ /* Reload index as git-apply will have modified it. */
+ discard_cache();
+ read_cache();
+
+ return 0;
+}
+
/**
* Applies all queued patches.
*/
@@ -520,7 +555,20 @@ static void am_run(struct am_state *state)
write_file(am_path(state, final-commit), 1, %s,
state-msg.buf);
write_author_script(state);
- /* patch application not implemented yet */
+ printf_ln(_(Applying: %s), firstline(state-msg.buf));
+
+ if (run_apply(state) 0) {
+ int value;
+
+ printf_ln(_(Patch failed at %s %s), msgnum(state),
+ firstline(state-msg.buf));
+
+ if (!git_config_get_bool(advice.amworkdir, value)
!value)
+ printf_ln(_(The copy of the patch that failed
is found in: %s),
+ am_path(state, patch));
+
+ exit(128);
+ }
next:
am_next(state);
--
2.1.4
--
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/WIP 8/8] am: commit applied patch
Implement do_commit(), which commits the index which contains the
results of applying the patch, along with the extracted commit message
and authorship information.
Signed-off-by: Paul Tan pyoka...@gmail.com
---
builtin/am.c | 46 ++
1 file changed, 46 insertions(+)
diff --git a/builtin/am.c b/builtin/am.c
index 7126df3..3174327 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -8,6 +8,9 @@
#include dir.h
#include run-command.h
#include quote.h
+#include cache-tree.h
+#include refs.h
+#include commit.h
/**
* Returns 1 if the file is empty or does not exist, 0 otherwise.
@@ -539,6 +542,48 @@ static int run_apply(const struct am_state *state)
}
/**
+ * Commits the current index with state-msg as the commit message and
+ * state-author_name, state-author_email and state-author_date as the author
+ * information.
+ */
+static void do_commit(const struct am_state *state)
+{
+ unsigned char tree[20], parent[20], commit[20];
+ unsigned char *ptr;
+ struct commit_list *parents = NULL;
+ const char *reflog_msg, *author;
+ struct strbuf sb = STRBUF_INIT;
+
+ if (write_cache_as_tree(tree, 0, NULL))
+ die(_(git write-tree failed to write a tree));
+
+ if (!get_sha1_commit(HEAD, parent)) {
+ ptr = parent;
+ commit_list_insert(lookup_commit(parent), parents);
+ } else {
+ ptr = NULL;
+ fprintf_ln(stderr, _(applying to an empty history));
+ }
+
+ author = fmt_ident(state-author_name.buf, state-author_email.buf,
+ state-author_date.buf, IDENT_STRICT);
+
+ if (commit_tree(state-msg.buf, state-msg.len, tree, parents, commit,
+ author, NULL))
+ die(_(failed to write commit object));
+
+ reflog_msg = getenv(GIT_REFLOG_ACTION);
+ if (!reflog_msg)
+ reflog_msg = am;
+
+ strbuf_addf(sb, %s: %s, reflog_msg, firstline(state-msg.buf));
+
+ update_ref(sb.buf, HEAD, commit, ptr, 0, UPDATE_REFS_DIE_ON_ERR);
+
+ strbuf_release(sb);
+}
+
+/**
* Applies all queued patches.
*/
static void am_run(struct am_state *state)
@@ -570,6 +615,7 @@ static void am_run(struct am_state *state)
exit(128);
}
+ do_commit(state);
next:
am_next(state);
}
--
2.1.4
--
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/WIP 5/8] am: detect mbox patches
Since 15ced75 (git-am foreign patch support: autodetect some patch
formats, 2009-05-27), git-am.sh is able to autodetect mbox, stgit and
mercurial patches through heuristics.
Re-implement support for autodetecting mbox/maildir files.
Signed-off-by: Paul Tan pyoka...@gmail.com
---
builtin/am.c | 99
1 file changed, 99 insertions(+)
diff --git a/builtin/am.c b/builtin/am.c
index 9c7b058..d589ec5 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -108,6 +108,97 @@ static void am_destroy(const struct am_state *state)
strbuf_release(sb);
}
+/*
+ * Returns 1 if the file looks like a piece of email a-la RFC2822, 0 otherwise.
+ * We check this by grabbing all the non-indented lines and seeing if they look
+ * like they begin with valid header field names.
+ */
+static int is_email(const char *filename)
+{
+ struct strbuf sb = STRBUF_INIT;
+ FILE *fp = xfopen(filename, r);
+ int ret = 1;
+
+ while (!strbuf_getline(sb, fp, '\n')) {
+ const char *x;
+
+ strbuf_rtrim(sb);
+
+ if (!sb.len)
+ break; /* End of header */
+
+ /* Ignore indented folded lines */
+ if (*sb.buf == '\t' || *sb.buf == ' ')
+ continue;
+
+ /* It's a header if it matches the regexp ^[!-9;-~]+: */
+ for (x = sb.buf; *x; x++) {
+ if (('!' = *x *x = '9') || (';' = *x *x =
'~'))
+ continue;
+ if (*x == ':' x != sb.buf)
+ break;
+ ret = 0;
+ goto fail;
+ }
+ }
+
+fail:
+ fclose(fp);
+ strbuf_release(sb);
+ return ret;
+}
+
+/**
+ * Attempts to detect the patch_format of the patches contained in `paths`,
+ * returning the PATCH_FORMAT_* enum value. Returns PATCH_FORMAT_UNKNOWN if
+ * detection fails.
+ */
+static int detect_patch_format(struct string_list *paths)
+{
+ enum patch_format ret = PATCH_FORMAT_UNKNOWN;
+ struct strbuf l1 = STRBUF_INIT;
+ struct strbuf l2 = STRBUF_INIT;
+ struct strbuf l3 = STRBUF_INIT;
+ FILE *fp;
+
+ /*
+* We default to mbox format if input is from stdin and for directories
+*/
+ if (!paths-nr || !strcmp(paths-items-string, -) ||
+ is_directory(paths-items-string)) {
+ strbuf_release(l1);
+ strbuf_release(l2);
+ strbuf_release(l3);
+ return PATCH_FORMAT_MBOX;
+ }
+
+ /*
+* Otherwise, check the first few 3 lines of the first patch, starting
+* from the first non-blank line, to try to detect its format.
+*/
+ fp = xfopen(paths-items-string, r);
+ while (!strbuf_getline(l1, fp, '\n')) {
+ strbuf_trim(l1);
+ if (l1.len)
+ break;
+ }
+ strbuf_getline(l2, fp, '\n');
+ strbuf_trim(l2);
+ strbuf_getline(l3, fp, '\n');
+ strbuf_trim(l3);
+ fclose(fp);
+
+ if (starts_with(l1.buf, From ) || starts_with(l1.buf, From: ))
+ ret = PATCH_FORMAT_MBOX;
+ else if (l1.len l2.len l3.len is_email(paths-items-string))
+ ret = PATCH_FORMAT_MBOX;
+
+ strbuf_release(l1);
+ strbuf_release(l2);
+ strbuf_release(l3);
+ return ret;
+}
+
/**
* Splits out individual patches from `paths`, where each path is either a mbox
* file or a Maildir. Return 0 on success, -1 on failure.
@@ -162,6 +253,14 @@ static int split_patches(struct am_state *state, enum
patch_format patch_format,
static void am_setup(struct am_state *state, enum patch_format patch_format,
struct string_list *paths)
{
+ if (!patch_format)
+ patch_format = detect_patch_format(paths);
+
+ if (!patch_format) {
+ fprintf_ln(stderr, _(Patch format detection failed.));
+ exit(128);
+ }
+
if (mkdir(state-dir.buf, 0777) 0 errno != EEXIST)
die_errno(_(failed to create directory '%s'), state-dir.buf);
--
2.1.4
--
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/WIP 6/8] am: extract patch, message and authorship with git-mailinfo
For the purpose of applying the patch and committing the results,
implement extracting the patch data, commit message and authorship from
an e-mail message using git-mailinfo.
git-mailinfo is run as a separate process, but ideally in the future,
we should be be able to access its functionality directly without
spawning a new process.
Signed-off-by: Paul Tan pyoka...@gmail.com
---
builtin/am.c | 231 +++
1 file changed, 231 insertions(+)
diff --git a/builtin/am.c b/builtin/am.c
index d589ec5..0b8a42d 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -7,6 +7,23 @@
#include parse-options.h
#include dir.h
#include run-command.h
+#include quote.h
+
+/**
+ * Returns 1 if the file is empty or does not exist, 0 otherwise.
+ */
+static int is_empty_file(const char *filename)
+{
+ struct stat st;
+
+ if (stat(filename, st) 0) {
+ if (errno == ENOENT)
+ return 1;
+ die_errno(_(could not stat %s), filename);
+ }
+
+ return !st.st_size;
+}
enum patch_format {
PATCH_FORMAT_UNKNOWN = 0,
@@ -17,6 +34,10 @@ struct am_state {
struct strbuf dir;/* state directory path */
int cur; /* current patch number */
int last; /* last patch number */
+ struct strbuf msg;/* commit message */
+ struct strbuf author_name;/* commit author's name */
+ struct strbuf author_email; /* commit author's email */
+ struct strbuf author_date;/* commit author's date */
int prec; /* number of digits in patch filename */
};
@@ -28,6 +49,10 @@ static void am_state_init(struct am_state *state)
memset(state, 0, sizeof(*state));
strbuf_init(state-dir, 0);
+ strbuf_init(state-msg, 0);
+ strbuf_init(state-author_name, 0);
+ strbuf_init(state-author_email, 0);
+ strbuf_init(state-author_date, 0);
state-prec = 4;
}
@@ -37,6 +62,10 @@ static void am_state_init(struct am_state *state)
static void am_state_release(struct am_state *state)
{
strbuf_release(state-dir);
+ strbuf_release(state-msg);
+ strbuf_release(state-author_name);
+ strbuf_release(state-author_email);
+ strbuf_release(state-author_date);
}
/**
@@ -81,6 +110,92 @@ static int read_state_file(struct strbuf *sb, const char
*file, size_t hint) {
}
/**
+ * Parses the author script `filename`, and sets state-author_name,
+ * state-author_email and state-author_date accordingly. We are strict with
+ * our parsing, as the author script is supposed to be eval'd, and loosely
+ * parsing it may not give the results the user expects.
+ *
+ * The author script is of the format:
+ *
+ * GIT_AUTHOR_NAME='$author_name'
+ * GIT_AUTHOR_EMAIL='$author_email'
+ *GIT_AUTHOR_DATE='$author_date'
+ *
+ * where $author_name, $author_email and $author_date are quoted.
+ */
+static int read_author_script(struct am_state *state)
+{
+ char *value;
+ struct strbuf sb = STRBUF_INIT;
+ const char *filename = am_path(state, author-script);
+ FILE *fp = fopen(filename, r);
+ if (!fp) {
+ if (errno == ENOENT)
+ return 0;
+ die(_(could not open '%s' for reading), filename);
+ }
+
+ if (strbuf_getline(sb, fp, '\n'))
+ return -1;
+ if (!skip_prefix(sb.buf, GIT_AUTHOR_NAME=, (const char**) value))
+ return -1;
+ value = sq_dequote(value);
+ if (!value)
+ return -1;
+ strbuf_addstr(state-author_name, value);
+
+ if (strbuf_getline(sb, fp, '\n'))
+ return -1;
+ if (!skip_prefix(sb.buf, GIT_AUTHOR_EMAIL=, (const char**) value))
+ return -1;
+ value = sq_dequote(value);
+ if (!value)
+ return -1;
+ strbuf_addstr(state-author_email, value);
+
+ if (strbuf_getline(sb, fp, '\n'))
+ return -1;
+ if (!skip_prefix(sb.buf, GIT_AUTHOR_DATE=, (const char**) value))
+ return -1;
+ value = sq_dequote(value);
+ if (!value)
+ return -1;
+ strbuf_addstr(state-author_date, value);
+
+ if (fgetc(fp) != EOF)
+ return -1;
+
+ fclose(fp);
+ strbuf_release(sb);
+ return 0;
+}
+
+/**
+ * Saves state-author_name, state-author_email and state-author_date in
+ * `filename` as an author script, which is the format used by git-am.sh.
+ */
+static void write_author_script(const struct am_state *state)
+{
+ static const char fmt[] = GIT_AUTHOR_NAME='%s'\n
+ GIT_AUTHOR_EMAIL='%s'\n
+ GIT_AUTHOR_DATE='%s'\n;
+ struct strbuf author_name = STRBUF_INIT;
+ struct strbuf author_email = STRBUF_INIT;
+ struct strbuf author_date = STRBUF_INIT;
+
+
Re: [PATCH/WIP 1/8] wrapper: implement xopen()
On Wed, May 27, 2015 at 6:33 AM, Paul Tan pyoka...@gmail.com wrote:
A common usage pattern of open() is to check if it was successful, and
die() if it was not:
int fd = open(path, O_WRONLY | O_CREAT, 0777);
if (fd 0)
die_errno(_(Could not open '%s' for writing.), path);
Implement a wrapper function xopen() that does the above so that we can
save a few lines of code, and make the die() messages consistent.
As a mental todo note for whomever wants to pick up some work: This patch
series indicates to only touch git-am. The first 2 patches introduce
new x-wrappers,
so maybe we'd need to grep through the whole code base and convert the all
these file opening code to the new wrapper.
Signed-off-by: Paul Tan pyoka...@gmail.com
---
git-compat-util.h | 1 +
wrapper.c | 18 ++
2 files changed, 19 insertions(+)
diff --git a/git-compat-util.h b/git-compat-util.h
index 17584ad..9745962 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -718,6 +718,7 @@ extern char *xstrndup(const char *str, size_t len);
extern void *xrealloc(void *ptr, size_t size);
extern void *xcalloc(size_t nmemb, size_t size);
extern void *xmmap(void *start, size_t length, int prot, int flags, int fd,
off_t offset);
+extern int xopen(const char *path, int flags, mode_t mode);
extern ssize_t xread(int fd, void *buf, size_t len);
extern ssize_t xwrite(int fd, const void *buf, size_t len);
extern ssize_t xpread(int fd, void *buf, size_t len, off_t offset);
diff --git a/wrapper.c b/wrapper.c
index c1a663f..971665a 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -189,6 +189,24 @@ void *xcalloc(size_t nmemb, size_t size)
# endif
#endif
+/**
+ * xopen() is the same as open(), but it die()s if the open() fails.
+ */
+int xopen(const char *path, int flags, mode_t mode)
+{
+ int fd;
+
+ assert(path);
+ fd = open(path, flags, mode);
+ if (fd 0) {
+ if ((flags O_WRONLY) || (flags O_RDWR))
+ die_errno(_(could not open '%s' for writing), path);
+ else
+ die_errno(_(could not open '%s' for reading), path);
+ }
+ return fd;
+}
+
/*
* xread() is the same a read(), but it automatically restarts read()
* operations with a recoverable error (EAGAIN and EINTR). xread()
--
2.1.4
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC 1/2] git-rebase -i: Add key word drop to remove a commit
Thank you for reviewing the code. Johannes Schindelinjohannes.schinde...@gmx.de writes: Please note that you can already just comment-out the line if you need to keep a visual trace. Alternatively, you can replace the `pick` command by `noop`. If you really need the `drop` command (with which I am not 100% happy because I already envisage users appending a `drop A` to an edit script pick A; pick B; pick C and expecting A *not to be picked*) It is true that drop has the same effect as noop or commenting, however we thought that drop is more understandable for average users of git. Moreover when using git rebase -i, the 'help' displayed below the list of commits doesn't mention neither the noop command nor the effect of commenting the line (though considering what removing a line does, it can be easily deduced). The drop command was inspired by the drop command from histedit in mercurial. It also has some effects with the second part of this patch (checks removed and/or duplicated commits): if you comment the line, the commit will be considered as removed, thus ending in a warning if the config variable is set to warn/error; however this problem won't appear with noop. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC 1/2] git-rebase -i: Add key word drop to remove a commit
Remi Galan Alfonso remi.galan-alfo...@ensimag.grenoble-inp.fr writes: It also has some effects with the second part of this patch (checks removed and/or duplicated commits): if you comment the line, the commit will be considered as removed, thus ending in a warning if the config variable is set to warn/error; however this problem won't appear with noop. Indeed, that's the whole point of having a drop command. As an advice for your next submission: use git send-email --cover-letter, and explain the overall idea before the patches. I personally prefer drop to noop as a command name: I understand noop as a command without argument (useful to say this is actually an empty list of commands, not an empty file to ask rebase to abort), but I find it weird to write noop sha1 title As Remi wrote, the inspiration comes from Mercurial. Perhaps we should ask on the mercurial ml how happy they are with the name. -- 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
Important Notification
You are required to click on the link to verify your email account because we are upgrading our webmail.http://www.skywap.ro/logo/eupdate/ Webmail Technical Support Copyright 2012. All Rights Reserved -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/WIP PATCH 04/11] upload-pack-2: Implement the version 2 of upload-pack
On Wed, May 27, 2015 at 2:35 AM, Jeff King p...@peff.net wrote:
On Tue, May 26, 2015 at 03:01:08PM -0700, Stefan Beller wrote:
+static void send_capabilities(void)
+{
+ char buf[100];
+
+ while (next_capability(buf))
+ packet_write(1, capability:%s\n, buf);
Like Eric, I find the whole next_capability thing a little ugly. His
suggestion to pass in the parsing state is an improvement, but I wonder
why we need to parse at all. Can we keep the capabilities as:
const char *capabilities[] = {
multi_ack,
thin-pack,
... etc ...
};
and then loop over the array?
Yes, that would be much nicer. I also had this in mind but didn't know
if there was a strong reason for the capabilities to be shoehorned
into a single string as they are currently.
--
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/WIP PATCH 06/11] remote.h: add get_remote_capabilities, request_capabilities
On Wed, May 27, 2015 at 2:50 AM, Jeff King p...@peff.net wrote: On Tue, May 26, 2015 at 11:25:05PM -0400, Eric Sunshine wrote: + len = packet_read(in, src_buf, src_len, + packet_buffer, sizeof(packet_buffer), + PACKET_READ_GENTLE_ON_EOF | + PACKET_READ_CHOMP_NEWLINE); + if (len 0) + die_initial_contact(0); + + if (!len) + break; + + if (len 4 skip_prefix(line, ERR , arg)) The 'len 4' check is needed because there's no guarantee that 'line' is NUL-terminated. Correct? I think this was just blindly copied from get_remote_heads(). And I think that code was being overly paranoid. Ever since f3a3214 (Make send/receive-pack be closer to doing something interesting, 2005-06-29), the pkt-line reader will add an extra NUL to the buffer to ease cases like this. Thanks. I had started digging into packet_read() to determine whether it guaranteed NUL-termination, but didn't get far enough to decide. I agree that if NUL-termination is guaranteed, then the 'len 4' check is superfluous (and confusing, which is why it caught my attention in the first place). -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC 2/2] git rebase -i: Warn removed or dupplicated commits
On Wed, May 27, 2015 at 9:19 AM, Remi Galan Alfonso
remi.galan-alfo...@ensimag.grenoble-inp.fr wrote:
Eric Sunshinesunsh...@sunshineco.com writes:
+ # To uppercase
+ checkLevel=$(echo $checkLevel | tr '[:lower:]' '[:upper:]')
Is there precedence elsewhere for recognizing uppercase and lowercase
variants of config values?
It seems to be commonly used when parsing options in the C files
through strcasecmp. For exemple, in config.c:818 :
if (!strcmp(var, core.safecrlf)) {
if (value !strcasecmp(value, warn)) {
[...]
However we didn't see any precedence in shell files. Do you think we
should remove it?
Precedence in C code is good enough for me, and it makes sense for
your new code to follow suit by being insensitive to case (as you have
already done).
However, it would be a good idea to be consistent in your use of
uppercase/lowercase in the commit message, documentation, and code,
rather than having a mix. I'd suggest sticking with lowercase
throughout since lowercase is more commonly used in the codebase (and
just easier to read).
--
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/WIP PATCH 04/11] upload-pack-2: Implement the version 2 of upload-pack
On Tue, May 26, 2015 at 11:35 PM, Jeff King p...@peff.net wrote:
On Tue, May 26, 2015 at 03:01:08PM -0700, Stefan Beller wrote:
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -716,10 +716,47 @@ static void format_symref_info(struct strbuf *buf,
struct string_list *symref)
strbuf_addf(buf, symref=%s:%s, item-string, (char
*)item-util);
}
-static const char *advertise_capabilities = multi_ack thin-pack side-band
+static char *advertise_capabilities = multi_ack thin-pack side-band
side-band-64k ofs-delta shallow no-progress
include-tag multi_ack_detailed;
If we are upload-pack-2, should we advertise that in the capabilities? I
think it may make things easier later if we try to provide some
opportunistic out-of-band data. E.g., if see tell git-daemon:
git-upload-pack repo\0host=whatever\0\0version=2
how do we know whether version=2 was understood and kicked us into v2
mode, versus an old server that ignored it?
So in my vision we would call git-upload-pack-2 instead of having a version=2.
and if git-upload-pack-2 doesn't exist, the whole conversation is
over, the client
it is left to make up some good error message or retry version 1.
But I think advertising both which versions the server could deal
with, as well as
the currently expected version is a good thing.
capability: can_speak=1,2
capability: speaking_now=2
It also just makes the protocol more self-describing; from the
conversation you can see which version is in use.
+static void send_capabilities(void)
+{
+ char buf[100];
+
+ while (next_capability(buf))
+ packet_write(1, capability:%s\n, buf);
Having a static-sized buffer and then passing it to a function which has
no idea of the buffer size seems like a recipe for a buffer overflow. :)
Yes. All patches I proposed are very brittle. My first goal was to have the
last test passing (an actual fetch with version 2). Now I will start looking at
making things robust, as by now you all seem to not disagree totally.
You are fine here because you are parsing the hard-coded capabilities
string, and we know 100 is enough there. But it's nice to avoid such
magic.
Like Eric, I find the whole next_capability thing a little ugly. His
suggestion to pass in the parsing state is an improvement, but I wonder
why we need to parse at all. Can we keep the capabilities as:
const char *capabilities[] = {
multi_ack,
thin-pack,
... etc ...
};
and then loop over the array? The v1 writer will need to be modified,
but it should be fairly straightforward (loop and add them to the buffer
rather than dumping the whole string).
Thanks for the design guidance! I'll change that.
Also, do we need the capability noise-word?
I thought it opens up a new possible door in the future.
As we ignore anything not starting with capability for now, you
could introduce
your foo and bar ping pong easily and still be version 2 compatible.
S: capability: thin
S: capability: another-capability
S: ping-pong foo
S: done
C: (not having understood ping-pong) just answering with capability: thin
C: done, let's proceed to refs advertisement
The alternative client would do:
C: ping-pong: foo-data1a
S: ping-pong: foo-data1b
C: ping-pong: foo-data2a
C: capability: thin
...
They all have it, except
for:
+ packet_write(1, agent:%s\n, git_user_agent_sanitized());
But isn't that basically a capability, too (I agree it is stretching the
definition of capability, but I think that ship has sailed; the
client's response is not I support this, too but I want to enable
this).
IOW, is there a reason that the initial conversation is not just:
pkt-line(multi_ack);
pkt-line(thin-pack);
...
pkt-line(agent=v2.5.0);
pkt-line();
We already have framing for each capability due to the use of pkt-line.
The capability: is just one more thing the client has to parse past.
The keys are already unique up to any =, so I don't think there is any
ambiguity (e.g., we don't care about capability:agent; we have already
assigned a meaning to the term agent, and will never introduce a
standalone capability with the same name).
It looks clearer to me (we're not wasting band-width), maybe this ping pong
thing can be part of the capabilities announcement too, so we're good this way.
Likewise, if we introduce new protocol items here, the client should
either ignore them (if it does not understand them) or know what to do
with them.
+static void receive_capabilities(void)
+{
+ int done = 0;
+ while (1) {
+ char *line = packet_read_line(0, NULL);
+ if (!line)
+ break;
+ if (starts_with(line, capability:))
+ parse_features(line + strlen(capability:));
+ }
Use:
const char *cap;
if (skip_prefix(line, capability:, cap))
parse_features(cap);
Or better yet, if you take my
[PATCHv3] submodule documentation: Reorder introductory paragraphs
It's better to start the man page with a description of what submodules actually are instead of saying what they are not. Reorder the paragraphs such that the first short paragraph introduces the submodule concept, the second paragraph highlights the usage of the submodule command, the third paragraph giving background information, and finally the fourth paragraph discusing alternatives such as subtrees and remotes, which we don't want to be confused with. This ordering deepens the knowledge on submodules with each paragraph. First the basic questions like How/what will be answered, while the underlying concepts will be taught at a later time. Making sure it is not confused with subtrees and remotes is not really enhancing knowledge of submodules itself, but rather painting the big picture of git concepts, so you could also argue to have it as the second paragraph. Personally I think this may confuse readers, specially newcomers though. Additionally to reordering the paragraphs, they have been slightly reworded. Signed-off-by: Stefan Beller sbel...@google.com --- I think this is the best I can come up with for now. * It still mentions the remotes as a potential explanation mud-hole, but I feel it helps the reader understand submodules a little better. * We also start with a typical git man page intro (Dropping This command does ...) Documentation/git-submodule.txt | 50 ++--- 1 file changed, 22 insertions(+), 28 deletions(-) diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt index 2c25916..2ca1391 100644 --- a/Documentation/git-submodule.txt +++ b/Documentation/git-submodule.txt @@ -25,22 +25,17 @@ SYNOPSIS DESCRIPTION --- -Submodules allow foreign repositories to be embedded within -a dedicated subdirectory of the source tree, always pointed -at a particular commit. +Inspects, updates and manages submodules. -They are not to be confused with remotes, which are meant mainly -for branches of the same project; submodules are meant for -different projects you would like to make part of your source tree, -while the history of the two projects still stays completely -independent and you cannot modify the contents of the submodule -from within the main project. -If you want to merge the project histories and want to treat the -aggregated whole as a single project from then on, you may want to -add a remote for the other project and use the 'subtree' merge strategy, -instead of treating the other project as a submodule. Directories -that come from both projects can be cloned and checked out as a whole -if you choose to go that route. +A Submodule allows you to keep another Git repository in a subdirectory +of your repository. The other repository has its own history, which does not +interfere with the history of the current repository. This can be used to +have external dependencies such as third party libraries for example. + +When cloning or pulling a repository containing submodules however, +these will not be checked out by default; the 'init' and 'update' +subcommands will maintain submodules checked out and at +appropriate revision in your working tree. Submodules are composed from a so-called `gitlink` tree entry in the main repository that refers to a particular commit object @@ -51,19 +46,18 @@ describes the default URL the submodule shall be cloned from. The logical name can be used for overriding this URL within your local repository configuration (see 'submodule init'). -This command will manage the tree entries and contents of the -gitmodules file for you, as well as inspect the status of your -submodules and update them. -When adding a new submodule to the tree, the 'add' subcommand -is to be used. However, when pulling a tree containing submodules, -these will not be checked out by default; -the 'init' and 'update' subcommands will maintain submodules -checked out and at appropriate revision in your working tree. -You can briefly inspect the up-to-date status of your submodules -using the 'status' subcommand and get a detailed overview of the -difference between the index and checkouts using the 'summary' -subcommand. - +Submodules are not to be confused with remotes, which are other +repositories of the same project; submodules are meant for +different projects you would like to make part of your source tree, +while the history of the two projects still stays completely +independent and you cannot modify the contents of the submodule +from within the main project. +If you want to merge the project histories and want to treat the +aggregated whole as a single project from then on, you may want to +add a remote for the other project and use the 'subtree' merge strategy, +instead of treating the other project as a submodule. Directories +that come from both projects can be cloned and checked out as a whole +if you choose to go that route. COMMANDS -- 2.4.1.345.gab207b6.dirty -- To
Re: [PATCH 3/5] verify_lock(): report errors via a strbuf
Michael Haggerty mhag...@alum.mit.edu writes: Instead of writing error messages directly to stderr, write them to a strbuf *err. In lock_ref_sha1_basic(), arrange for these errors to be returned to its caller. I had to scratch my head and view long outside the context before realizing that the caller lock_ref_sha1_basic() already arranges with its caller that errors from it are passed via the strbuf, and this change is just turning verify_lock(), a callee from it, follows that already established pattern. Looks sensible, but the last sentence was misleading at least to me. The caller, lock_ref_sha1_basic(), uses this error reporting convention with all the other callees, and reports its error this way to its callers. perhaps? -- 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 0/5] Fix verify_lock() to report errors via strbuf
Michael Haggerty mhag...@alum.mit.edu writes: The last sentence is nonsense. This patch series relies on lock_ref_sha1_basic() having a strbuf *err parameter, which is only the case since 4a32b2e lock_ref_sha1_basic(): report errors via a struct strbuf *err (2015-05-11) The latter commit is in mh/ref-directory-file (which has now been merged to master, so technically the last sentence is now correct again). [5/5] seems to conflict with the write_ref_sha1() vs write_ref_to_lockfile() updates; I think I can manage, though ;-) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6] send-email: Add sendmail email aliases format
Allen Hubbe alle...@gmail.com writes: Add support for the sendmail email aliases format. Thanks. ** Note: A general 'where to find documentation' paragraph will be added by Junio, appearing either before or after this patch in the series. You didn't have to do this to me; as long as you agree with me that the paragraph is a good thing to have, it is OK (and even more preferable) to include it in this patch. That's called collaboration. If other person's contribution was really significant and the change can stand on its own, then a split two-patch series with the author set to the other person may not be a bad idea, and if other person's contribution was really significant but the change by the other person cannot stand on its own, Helped-by in the log message would be sufficient. My contribution in this case is much less than that. ** Note: A fix to other tests to eliminate the use of tilde for the home dir will be added by Junio, appearing either before or after this patch in the series. That is a sensible thing to do, as it does not relate to this change. Thanks. Will queue and let's start merging this topic to 'next' and down. -- 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/WIP PATCH 06/11] remote.h: add get_remote_capabilities, request_capabilities
On Wed, May 27, 2015 at 01:19:39PM -0400, Eric Sunshine wrote: The 'len 4' check is needed because there's no guarantee that 'line' is NUL-terminated. Correct? I think this was just blindly copied from get_remote_heads(). And I think that code was being overly paranoid. Ever since f3a3214 (Make send/receive-pack be closer to doing something interesting, 2005-06-29), the pkt-line reader will add an extra NUL to the buffer to ease cases like this. Thanks. I had started digging into packet_read() to determine whether it guaranteed NUL-termination, but didn't get far enough to decide. I agree that if NUL-termination is guaranteed, then the 'len 4' check is superfluous (and confusing, which is why it caught my attention in the first place). Yeah, agreed that it should be cleaned up. Interestingly, if you dig on that line, I've touched it several times myself and never noticed this. :) -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/WIP PATCH 04/11] upload-pack-2: Implement the version 2 of upload-pack
On Wed, May 27, 2015 at 01:30:28PM -0400, Eric Sunshine wrote:
Like Eric, I find the whole next_capability thing a little ugly. His
suggestion to pass in the parsing state is an improvement, but I wonder
why we need to parse at all. Can we keep the capabilities as:
const char *capabilities[] = {
multi_ack,
thin-pack,
... etc ...
};
and then loop over the array?
Yes, that would be much nicer. I also had this in mind but didn't know
if there was a strong reason for the capabilities to be shoehorned
into a single string as they are currently.
I don't think there is a good reason, beyond it being the simplest thing
for the current code to work. But as you can see from the existing
packet_write() in upload-pack, it's already going through some
contortions to handle optional capabilities (i.e., capabilities is by
no means the full list anymore).
Doing it item by item will mean we can't use a single packet_write() in
the v1 code, but it's OK to format into a buffer here (we already need
such a buffer for format_symref_info anyway).
-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/WIP PATCH 05/11] transport: add infrastructure to support a protocol version number
On Wed, May 27, 2015 at 12:01:50PM -0700, Stefan Beller wrote: Interesting choice for the short option (-v would be nice, but obviously it is taken). Do we want to delay on claiming the short-and-sweet 'y' until we are sure this is something people will use a lot? In an ideal world, it is not (i.e., auto-upgrade and other tricks become good enough that nobody bothers to specify it manually). [...] Or do you rather hint on dropping the short option at all, and just having NULL in the field? Yes, that's what I was hinting. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] clone: add `--seed` shorthand
Jeff King p...@peff.net writes: On Sun, May 24, 2015 at 12:07:53PM -0700, Junio C Hamano wrote: Jeff King p...@peff.net writes: Having slept on it, I really think --seed should be fetch from the seed into temp refs, and not what I posted earlier. Yeah, I think that is the right way to do it. In the meantime, do you want to pick up patches 1 and 2? I think they are cleanups that stand on their own, whether we do patch 3 or not. Thanks for reminding. Let me take a look. 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 1/2] config: add options to list only variable names
Recenty I created a multi-line branch description with '.' and '='
characters on one of the lines, and noticed that fragments of that line
show up when completing set variable names for 'git config', e.g.:
$ git config --get branch.b.description
Branch description to fool the completion script with a
second line containing dot . and equals = characters.
$ git config --unset TAB
...
second line containing dot . and equals
...
The completion script runs 'git config --list' and processes its output to
strip the values and keep only the variable names. It does so by looking
for lines containing '.' and '=' and outputting everything before the '=',
which was fooled by my multi-line branch description.
A similar issue exists with aliases and pretty format aliases with
multi-line values, but in that case 'git config --get-regexp' is run and
subsequent lines don't have to contain either '.' or '=' to fool the
completion script.
Though 'git config' can produce null-terminated output for newline-safe
parsing, that's of no use in this case, becase we can't cope with nulls in
the shell.
Help the completion script by introducing the '--list-names' and
'--get-names-regexp' options, the names-only equivalents of '--list' and
'--get-regexp', so it doesn't have to separate variable names from their
values anymore.
Signed-off-by: SZEDER Gábor sze...@ira.uka.de
---
Documentation/git-config.txt | 10 +-
builtin/config.c | 22 ++
contrib/completion/git-completion.bash | 4 ++--
t/t1300-repo-config.sh | 22 ++
4 files changed, 51 insertions(+), 7 deletions(-)
diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
index 02ec096..e0d27d5 100644
--- a/Documentation/git-config.txt
+++ b/Documentation/git-config.txt
@@ -16,11 +16,12 @@ SYNOPSIS
'git config' [file-option] [type] [-z|--null] --get-all name [value_regex]
'git config' [file-option] [type] [-z|--null] --get-regexp name_regex
[value_regex]
'git config' [file-option] [type] [-z|--null] --get-urlmatch name URL
+'git config' [file-option] [-z|--null] --get-name-regexp name_regex
'git config' [file-option] --unset name [value_regex]
'git config' [file-option] --unset-all name [value_regex]
'git config' [file-option] --rename-section old_name new_name
'git config' [file-option] --remove-section name
-'git config' [file-option] [-z|--null] -l | --list
+'git config' [file-option] [-z|--null] -l | --list | --list-name
'git config' [file-option] --get-color name [default]
'git config' [file-option] --get-colorbool name [stdout-is-tty]
'git config' [file-option] -e | --edit
@@ -96,6 +97,10 @@ OPTIONS
in which section and variable names are lowercased, but subsection
names are not.
+--get-name-regexp::
+ Like --get-regexp, but shows only matching variable names, not its
+ values.
+
--get-urlmatch name URL::
When given a two-part name section.key, the value for
section.url.key whose url part matches the best to the
@@ -161,6 +166,9 @@ See also FILES.
--list::
List all variables set in config file.
+--list-name::
+ List the names of all variables set in config file.
+
--bool::
'git config' will ensure that the output is true or false
diff --git a/builtin/config.c b/builtin/config.c
index 7188405..38bcf83 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -13,6 +13,7 @@ static char *key;
static regex_t *key_regexp;
static regex_t *regexp;
static int show_keys;
+static int show_only_keys;
static int use_key_regexp;
static int do_all;
static int do_not_match;
@@ -43,6 +44,8 @@ static int respect_includes = -1;
#define ACTION_GET_COLOR (113)
#define ACTION_GET_COLORBOOL (114)
#define ACTION_GET_URLMATCH (115)
+#define ACTION_LIST_NAMES (116)
+#define ACTION_GET_NAME_REGEXP (117)
#define TYPE_BOOL (10)
#define TYPE_INT (11)
@@ -60,6 +63,7 @@ static struct option builtin_config_options[] = {
OPT_BIT(0, get, actions, N_(get value: name [value-regex]),
ACTION_GET),
OPT_BIT(0, get-all, actions, N_(get all values: key
[value-regex]), ACTION_GET_ALL),
OPT_BIT(0, get-regexp, actions, N_(get values for regexp:
name-regex [value-regex]), ACTION_GET_REGEXP),
+ OPT_BIT(0, get-name-regexp, actions, N_(get names for regexp:
name-regex), ACTION_GET_NAME_REGEXP),
OPT_BIT(0, get-urlmatch, actions, N_(get value specific for the
URL: section[.var] URL), ACTION_GET_URLMATCH),
OPT_BIT(0, replace-all, actions, N_(replace all matching variables:
name value [value_regex]), ACTION_REPLACE_ALL),
OPT_BIT(0, add, actions, N_(add a new variable: name value),
ACTION_ADD),
@@ -68,6 +72,7 @@ static struct option builtin_config_options[] = {
OPT_BIT(0, rename-section, actions, N_(rename section: old-name
new-name), ACTION_RENAME_SECTION),
OPT_BIT(0, remove-section, actions, N_(remove a section: name),
[PATCH 2/2] completion: use new 'git config' options to reliably list variable names
List all set config variable names with 'git config --list-names' instead
of '--list' post processing. Similarly, use 'git config
--get-name-regexp' instead of '--get-regexp' to get config variables in a
given section.
Signed-off-by: SZEDER Gábor sze...@ira.uka.de
---
contrib/completion/git-completion.bash | 15 +++
1 file changed, 3 insertions(+), 12 deletions(-)
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index 6abbd56..121aa31 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -744,9 +744,8 @@ __git_compute_porcelain_commands ()
__git_get_config_variables ()
{
local section=$1 i IFS=$'\n'
- for i in $(git --git-dir=$(__gitdir) config --get-regexp
^$section\..* 2/dev/null); do
- i=${i#$section.}
- echo ${i/ */}
+ for i in $(git --git-dir=$(__gitdir) config --get-name-regexp
^$section\..* 2/dev/null); do
+ echo ${i#$section.}
done
}
@@ -1774,15 +1773,7 @@ __git_config_get_set_variables ()
c=$((--c))
done
- git --git-dir=$(__gitdir) config $config_file --list 2/dev/null |
- while read -r line
- do
- case $line in
- *.*=*)
- echo ${line/=*/}
- ;;
- esac
- done
+ git --git-dir=$(__gitdir) config $config_file --list-names 2/dev/null
}
_git_config ()
--
2.4.2.347.ge926c0d
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/4] showing existing ws breakage
Jeff King p...@peff.net writes: In color.diff.*, these are called new, old, and plain. I am of the opinion that context is a far better name than plain, but perhaps we should support both for consistency. Here's a patch for the color.diff side, if we want to go that route. -- 8 -- Subject: diff: accept color.diff.context as a synonym for plain The term plain is a bit ambiguous; let's allow the more specific context, but keep plain around for compatibility. Signed-off-by: Jeff King p...@peff.net --- I didn't bother mentioning the historical plain in the documentation. I don't know if it's better to (for people who find it in the wild and wonder what it means) or if it simply clutters the description. 'plain' does sound a misnomer, as these slot names are about what are painted, not how they are painted. The latter is what their values represent. Whoever named that slot was confused by the fact that 'context' (i.e. what) lines are by default painted in 'plain' color without frills (i.e. how). We usually try to give a brief mention to historical names primarily to silence those who pick up stale information from the Web, get curious, and then complain loudly after finding that we no longer document them even though we keep accepting them silently, so I am somewhat tempted to do this on top. diff --git a/Documentation/config.txt b/Documentation/config.txt index 0a7ffa5..b458590 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -870,7 +870,8 @@ command line with the `--color[=when]` option. color.diff.slot:: Use customized color for diff colorization. `slot` specifies which part of the patch to use the specified color, and is one - of `context` (context text), `meta` (metainformation), `frag` + of `context` (context text - `plain` is a historical synonym), + `meta` (metainformation), `frag` (hunk header), 'func' (function in hunk header), `old` (removed lines), `new` (added lines), `commit` (commit headers), or `whitespace` (highlighting whitespace errors). The values of these variables may be -- 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/WIP 1/8] wrapper: implement xopen()
On 2015-05-27 15.33, Paul Tan wrote:
A common usage pattern of open() is to check if it was successful, and
die() if it was not:
int fd = open(path, O_WRONLY | O_CREAT, 0777);
if (fd 0)
die_errno(_(Could not open '%s' for writing.), path);
Implement a wrapper function xopen() that does the above so that we can
save a few lines of code, and make the die() messages consistent.
Signed-off-by: Paul Tan pyoka...@gmail.com
---
git-compat-util.h | 1 +
wrapper.c | 18 ++
2 files changed, 19 insertions(+)
diff --git a/git-compat-util.h b/git-compat-util.h
index 17584ad..9745962 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -718,6 +718,7 @@ extern char *xstrndup(const char *str, size_t len);
extern void *xrealloc(void *ptr, size_t size);
extern void *xcalloc(size_t nmemb, size_t size);
extern void *xmmap(void *start, size_t length, int prot, int flags, int fd,
off_t offset);
+extern int xopen(const char *path, int flags, mode_t mode);
extern ssize_t xread(int fd, void *buf, size_t len);
extern ssize_t xwrite(int fd, const void *buf, size_t len);
extern ssize_t xpread(int fd, void *buf, size_t len, off_t offset);
diff --git a/wrapper.c b/wrapper.c
index c1a663f..971665a 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -189,6 +189,24 @@ void *xcalloc(size_t nmemb, size_t size)
# endif
#endif
The original open can take 2 or 3 parameters, how about this:
int xopen(const char *path, int oflag, ... )
{
va_list params;
int mode;
int fd;
va_start(params, oflag);
mode = va_arg(params, int);
va_end(params);
fd = open(path, oflag, mode);
+/**
+ * xopen() is the same as open(), but it die()s if the open() fails.
+ */
+int xopen(const char *path, int flags, mode_t mode)
+{
+ int fd;
+
+ assert(path);
+ fd = open(path, flags, mode);
+ if (fd 0) {
+ if ((flags O_WRONLY) || (flags O_RDWR))
+ die_errno(_(could not open '%s' for writing), path);
This is only partly true:
it could be either writing or read write.
I don't know if the info for reading or for writing is needed/helpful at
all,
or if a simple could not open would be enough.
Another thing:
should we handle EINTR ?
(Somewhere in the back of my head I remember that some OS
returned EINTR when handling some foreign file system
Mac OS / NTFS ?)
--
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/WIP PATCH 05/11] transport: add infrastructure to support a protocol version number
Jeff King p...@peff.net writes:
On Tue, May 26, 2015 at 03:01:09PM -0700, Stefan Beller wrote:
+OPT_STRING('y', transport-version, transport_version,
+ N_(transport-version),
+ N_(specify transport version to be used)),
Interesting choice for the short option (-v would be nice, but
obviously it is taken). Do we want to delay on claiming the
short-and-sweet 'y' until we are sure this is something people will use
a lot? In an ideal world, it is not (i.e., auto-upgrade and other tricks
become good enough that nobody bothers to specify it manually).
Yes, just stuff 0 (not NULL but NUL) there; unless we have a very
good reason to believe that the option will be used every day to
toggle per invocation settings, we shouldn't squat on a short and
sweet single letter.
--
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/WIP PATCH 05/11] transport: add infrastructure to support a protocol version number
On Tue, May 26, 2015 at 11:39 PM, Jeff King p...@peff.net wrote:
On Tue, May 26, 2015 at 03:01:09PM -0700, Stefan Beller wrote:
+ OPT_STRING('y', transport-version, transport_version,
+N_(transport-version),
+N_(specify transport version to be used)),
Interesting choice for the short option (-v would be nice, but
obviously it is taken). Do we want to delay on claiming the
short-and-sweet 'y' until we are sure this is something people will use
a lot? In an ideal world, it is not (i.e., auto-upgrade and other tricks
become good enough that nobody bothers to specify it manually).
I made the choice this way:
Oh crap! -v is taken. so which letter is most likely not taken, so I
can move on?
So if you have any other proposal with actual reasons I'd switch in a
heart beat.
I figured the -y is good to testing and debugging, but in an ideal
world we don't
want people messing with the transport option because of automatic upgrades
as you said.
Or do you rather hint on dropping the short option at all, and just having NULL
in the field?
-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC 2/2] git rebase -i: Warn removed or dupplicated commits
Remi Galan Alfonso remi.galan-alfo...@ensimag.grenoble-inp.fr
writes:
Thank you for reviewing the code.
Eric Sunshinesunsh...@sunshineco.com writes:
+ # To uppercase
+ checkLevel=$(echo $checkLevel | tr '[:lower:]' '[:upper:]')
Is there precedence elsewhere for recognizing uppercase and lowercase
variants of config values?
It seems to be commonly used when parsing options in the C files
through strcasecmp. For exemple, in config.c:818 :
if (!strcmp(var, core.safecrlf)) {
if (value !strcasecmp(value, warn)) {
[...]
However we didn't see any precedence in shell files. Do you think we
should remove it?
I think there is a difference between (silently) accepting just to
be lenient and documenting and advocating mixed case uses.
Personally, I'd rather not to see gratuitous flexibility to allow
the same thing spelled in 47 different ways for no good reason.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC 2/2] git rebase -i: Warn removed or dupplicated commits
Matthieu Moy matthieu@grenoble-inp.fr writes: Stephen Kelly steve...@gmail.com writes: Galan Rémi remi.galan-alfonso at ensimag.grenoble-inp.fr writes: Check if commits were removed (i.e. a line was deleted) or dupplicated (e.g. the same commit is picked twice), can print warnings or abort git rebase according to the value of the configuration variable rebase.checkLevel. I sometimes duplicate commits deliberately if I want to split a commit in two. I move a copy up and fix the conflict, and I know that I'll still get the right thing later even if I make a mistake with the conflict resolution. The more I think about it, the more I think we should either not warn at all on duplicate commits, or have a separate config variable. Yeah, I'd say we shouldn't warn, without configuration to keep things simple. It's rare to duplicate by mistake, and when you do so, it's already easy to notice: you get conflicts, and you can git rebase --skip the second occurence. Accidentally dropped commits are another story: it's rather easy to cut-and-forget-to-paste, and the consequence currently is silent data loss ... -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC 1/2] git-rebase -i: Add key word drop to remove a commit
Matthieu Moy matthieu@grenoble-inp.fr writes: I find it weird to write noop sha1 title True, but then it can be spelled # sha1 title too, so do we still want 'drop'? Unless we have a strong reason to believe migrants from Hg cannot be (re)trained, personally, I'd feel that we do not need this 'drop' thing. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC 1/2] git-rebase -i: Add key word drop to remove a commit
Junio C Hamano gits...@pobox.com writes: Matthieu Moy matthieu@grenoble-inp.fr writes: I find it weird to write noop sha1 title True, but then it can be spelled # sha1 title I do find it weird too. # means comment, which means do as if it was not there to me. And in this case it does change the semantics once you activate the safety feature: error out without the # sha1 title, rebase dropping the commit if the comment is present. -- 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 1.5/2] config: add options to list only variable names
Recenty I created a multi-line branch description with '.' and '='
characters on one of the lines, and noticed that fragments of that line
show up when completing set variable names for 'git config', e.g.:
$ git config --get branch.b.description
Branch description to fool the completion script with a
second line containing dot . and equals = characters.
$ git config --unset TAB
...
second line containing dot . and equals
...
The completion script runs 'git config --list' and processes its output to
strip the values and keep only the variable names. It does so by looking
for lines containing '.' and '=' and outputting everything before the '=',
which was fooled by my multi-line branch description.
A similar issue exists with aliases and pretty format aliases with
multi-line values, but in that case 'git config --get-regexp' is run and
subsequent lines don't have to contain either '.' or '=' to fool the
completion script.
Though 'git config' can produce null-terminated output for newline-safe
parsing, that's of no use in this case, becase we can't cope with nulls in
the shell.
Help the completion script by introducing the '--list-names' and
'--get-names-regexp' options, the names-only equivalents of '--list' and
'--get-regexp', so it doesn't have to separate variable names from their
values anymore.
Signed-off-by: SZEDER Gábor sze...@ira.uka.de
---
D'oh, misspelled the option in the docs...
Documentation/git-config.txt | 10 +-
builtin/config.c | 22 ++
contrib/completion/git-completion.bash | 4 ++--
t/t1300-repo-config.sh | 22 ++
4 files changed, 51 insertions(+), 7 deletions(-)
diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
index 02ec096faa..fd519f81d8 100644
--- a/Documentation/git-config.txt
+++ b/Documentation/git-config.txt
@@ -16,11 +16,12 @@ SYNOPSIS
'git config' [file-option] [type] [-z|--null] --get-all name [value_regex]
'git config' [file-option] [type] [-z|--null] --get-regexp name_regex
[value_regex]
'git config' [file-option] [type] [-z|--null] --get-urlmatch name URL
+'git config' [file-option] [-z|--null] --get-name-regexp name_regex
'git config' [file-option] --unset name [value_regex]
'git config' [file-option] --unset-all name [value_regex]
'git config' [file-option] --rename-section old_name new_name
'git config' [file-option] --remove-section name
-'git config' [file-option] [-z|--null] -l | --list
+'git config' [file-option] [-z|--null] -l | --list | --list-names
'git config' [file-option] --get-color name [default]
'git config' [file-option] --get-colorbool name [stdout-is-tty]
'git config' [file-option] -e | --edit
@@ -96,6 +97,10 @@ OPTIONS
in which section and variable names are lowercased, but subsection
names are not.
+--get-name-regexp::
+ Like --get-regexp, but shows only matching variable names, not its
+ values.
+
--get-urlmatch name URL::
When given a two-part name section.key, the value for
section.url.key whose url part matches the best to the
@@ -161,6 +166,9 @@ See also FILES.
--list::
List all variables set in config file.
+--list-names::
+ List the names of all variables set in config file.
+
--bool::
'git config' will ensure that the output is true or false
diff --git a/builtin/config.c b/builtin/config.c
index 7188405f7e..38bcf838c5 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -13,6 +13,7 @@ static char *key;
static regex_t *key_regexp;
static regex_t *regexp;
static int show_keys;
+static int show_only_keys;
static int use_key_regexp;
static int do_all;
static int do_not_match;
@@ -43,6 +44,8 @@ static int respect_includes = -1;
#define ACTION_GET_COLOR (113)
#define ACTION_GET_COLORBOOL (114)
#define ACTION_GET_URLMATCH (115)
+#define ACTION_LIST_NAMES (116)
+#define ACTION_GET_NAME_REGEXP (117)
#define TYPE_BOOL (10)
#define TYPE_INT (11)
@@ -60,6 +63,7 @@ static struct option builtin_config_options[] = {
OPT_BIT(0, get, actions, N_(get value: name [value-regex]),
ACTION_GET),
OPT_BIT(0, get-all, actions, N_(get all values: key
[value-regex]), ACTION_GET_ALL),
OPT_BIT(0, get-regexp, actions, N_(get values for regexp:
name-regex [value-regex]), ACTION_GET_REGEXP),
+ OPT_BIT(0, get-name-regexp, actions, N_(get names for regexp:
name-regex), ACTION_GET_NAME_REGEXP),
OPT_BIT(0, get-urlmatch, actions, N_(get value specific for the
URL: section[.var] URL), ACTION_GET_URLMATCH),
OPT_BIT(0, replace-all, actions, N_(replace all matching variables:
name value [value_regex]), ACTION_REPLACE_ALL),
OPT_BIT(0, add, actions, N_(add a new variable: name value),
ACTION_ADD),
@@ -68,6 +72,7 @@ static struct option builtin_config_options[] = {
OPT_BIT(0, rename-section, actions, N_(rename section: old-name
new-name), ACTION_RENAME_SECTION),
OPT_BIT(0,
[PATCH] p4: Retrieve the right revision of the UTF-16 file
Fixing bug with UTF-16 files when they are retreived by git-p4. It was always getting the tip version of the file and the history of the file was lost. --- git-p4.py |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/git-p4.py b/git-p4.py index cdfa2df..be2c7da 100755 --- a/git-p4.py +++ b/git-p4.py @@ -2098,7 +2098,7 @@ class P4Sync(Command, P4UserMap): # them back too. This is not needed to the cygwin windows version, # just the native NT type. # -text = p4_read_pipe(['print', '-q', '-o', '-', file['depotFile']]) +text = p4_read_pipe(['print', '-q', '-o', '-', %s@%s % (file['depotFile'], file['change']) ]) if p4_version_string().find(/NT) = 0: text = text.replace(\r\n, \n) contents = [ text ] -- 1.7.10.4 -- 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/WIP 3/8] am: implement patch queue mechanism
Paul Tan pyoka...@gmail.com writes: Makefile | 2 +- builtin.h| 1 + builtin/am.c | 167 +++ git.c| 1 + 4 files changed, 170 insertions(+), 1 deletion(-) create mode 100644 builtin/am.c diff --git a/Makefile b/Makefile index 323c401..57a7c8c 100644 --- a/Makefile +++ b/Makefile @@ -466,7 +466,6 @@ TEST_PROGRAMS_NEED_X = # interactive shell sessions without exporting it. unexport CDPATH -SCRIPT_SH += git-am.sh If you are building this new am incrementally (and for something complex like am, that's the only sensible way), perhaps it is a good idea to do the competing implementation trick I suggested earlier when we were discussing your new pull patches, to allow you to keep both versions and switch between them at runtime. That way, you can run tests, both existing ones and new ones you add, on both versions to make sure they behave the same way. -- 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/2] completion: use new 'git config' options to reliably list variable names
On Wed, May 27, 2015 at 10:07:20PM +0200, SZEDER Gábor wrote: List all set config variable names with 'git config --list-names' instead of '--list' post processing. Similarly, use 'git config --get-name-regexp' instead of '--get-regexp' to get config variables in a given section. Signed-off-by: SZEDER Gábor sze...@ira.uka.de --- contrib/completion/git-completion.bash | 15 +++ 1 file changed, 3 insertions(+), 12 deletions(-) Nice diffstat. The less hacky bash parsing we can do, the better. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] config: add options to list only variable names
On Wed, May 27, 2015 at 05:04:38PM -0400, Jeff King wrote: -'git config' [file-option] [-z|--null] -l | --list +'git config' [file-option] [-z|--null] -l | --list | --list-name s/list-name/s/, to match the code (and your commit message). Doh, just saw your 1.5. FWIW, I expected PATCH 1.5/2 to be eh, I should have put this in between patches 1 and 2. I expect a re-roll to be v1.5 (or just v2). This was the only real error in the patch, so your 1.5 looks OK to me. Though I hope you will consider my other suggestions, too. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/5] verify_lock(): report errors via a strbuf
On 05/27/2015 09:48 PM, Junio C Hamano wrote: Michael Haggerty mhag...@alum.mit.edu writes: Instead of writing error messages directly to stderr, write them to a strbuf *err. In lock_ref_sha1_basic(), arrange for these errors to be returned to its caller. I had to scratch my head and view long outside the context before realizing that the caller lock_ref_sha1_basic() already arranges with its caller that errors from it are passed via the strbuf, and this change is just turning verify_lock(), a callee from it, follows that already established pattern. Looks sensible, but the last sentence was misleading at least to me. The caller, lock_ref_sha1_basic(), uses this error reporting convention with all the other callees, and reports its error this way to its callers. perhaps? +1 Thanks for clarifying this sentence. Michael -- Michael Haggerty mhag...@alum.mit.edu -- 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/WIP PATCH 04/11] upload-pack-2: Implement the version 2 of upload-pack
On Wed, May 27, 2015 at 01:45:55PM -0700, Stefan Beller wrote: Right, but I think (and please correct me if there's a case I'm missing) that the behavior is the same whether it is spelled ping-pong or capability:ping-pong. That is, the rule for capability: is if you do not understand it, ignore it and do not mention it in your capabilities; the server is required to assume you were written before that capability was invented. But that is _also_ the rule for ping-pong, I think. The rules are the same, right. But the allowed characters are limited (in theory) as the regular expressions given for the capabilities don't allow for binary data for example, but only well formed ASCII text, space separated. The ping-pong keyword could introduce a binary stream there including line feeds. (Today it sounds like a stupid idea though) Do we need that restriction? IOW, as long as we parse from the start of the packet and give up as soon as we realize it is not a thing we understand, I do not think anybody is hurt by the contents of the item. E.g., if an old client sees: 00XXping-pong=binary goo It knows: 1. The item starts with ping-pong; we don't know what that is, so we never even bother looking at the binary goo. 2. It's in a pkt-line. We read to the end of the packet line and throw the rest of the data away. Now we're synchronized to read the next item. Of course, ping-pong may also introduce another phase to the protocol which is not encapsulated in pkt-lines (e.g., if the data is too big to fit right inside the capability pkt-line, or if it needs a lot of back and forth like ref negotiation). But then we only proceed to that phase if both sides have said I understand ping-pong. So I think we are capable of boot-strapping just about anything using capability negotiation (with the exception of fixing the capability negotiation itself; but even that, we can bootstrap a second more intensive capability negotiation using a capability in the initial list). -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Bug: .gitconfig folder
Jorge grif...@gmx.es writes: If you have a folder named ~/.gitconfig instead of a file with that name, when you try to run some global config editing command it will fail with a wrong error message: fatal: Out of memory? mmap failed: No such device That indeed is a funny error message. How about this patch? -- 8 -- We show that message with die_errno(), but the OS is ought to know why mmap(2) failed much better than we do. There is no reason for us to say Out of memory? here. Note that mmap(2) fails with ENODEV when the file you specify is not something that can be mmap'ed, so you still need to know that No such device can include cases like having a directory when a regular file is expected, but we can expect that a user who creates a directory to a location where a regular file is expected to be would know what s/he is doing, hopefully ;-) sha1_file.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sha1_file.c b/sha1_file.c index ccc6dac..551a9e9 100644 --- a/sha1_file.c +++ b/sha1_file.c @@ -720,7 +720,7 @@ void *xmmap(void *start, size_t length, release_pack_memory(length); ret = mmap(start, length, prot, flags, fd, offset); if (ret == MAP_FAILED) - die_errno(Out of memory? mmap failed); + die_errno(mmap failed); } return ret; } -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/4] showing existing ws breakage
On Wed, May 27, 2015 at 11:57:15AM -0700, Junio C Hamano wrote: -- 8 -- Subject: diff: accept color.diff.context as a synonym for plain The term plain is a bit ambiguous; let's allow the more specific context, but keep plain around for compatibility. Signed-off-by: Jeff King p...@peff.net --- I didn't bother mentioning the historical plain in the documentation. I don't know if it's better to (for people who find it in the wild and wonder what it means) or if it simply clutters the description. 'plain' does sound a misnomer, as these slot names are about what are painted, not how they are painted. The latter is what their values represent. Whoever named that slot was confused by the fact that 'context' (i.e. what) lines are by default painted in 'plain' color without frills (i.e. how). We usually try to give a brief mention to historical names primarily to silence those who pick up stale information from the Web, get curious, and then complain loudly after finding that we no longer document them even though we keep accepting them silently, so I am somewhat tempted to do this on top. Yeah, I waffled on doing it myself. If you take the patch, please do squash that in. Do you want me to also eradicate PLAIN from the code itself? It's a rather simple change, but it does touch a lot of places. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/4] showing existing ws breakage
Jeff King p...@peff.net writes: Do you want me to also eradicate PLAIN from the code itself? It's a rather simple change, but it does touch a lot of places. Nah, that is not user-facing. We do not do s/cache/index/ in the code, either. Besides, I actually find plain much easier to type than context ;-) -- 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/WIP PATCH 04/11] upload-pack-2: Implement the version 2 of upload-pack
On Wed, May 27, 2015 at 1:34 PM, Jeff King p...@peff.net wrote: On Wed, May 27, 2015 at 10:40:37AM -0700, Stefan Beller wrote: If we are upload-pack-2, should we advertise that in the capabilities? I think it may make things easier later if we try to provide some opportunistic out-of-band data. E.g., if see tell git-daemon: git-upload-pack repo\0host=whatever\0\0version=2 how do we know whether version=2 was understood and kicked us into v2 mode, versus an old server that ignored it? So in my vision we would call git-upload-pack-2 instead of having a version=2. and if git-upload-pack-2 doesn't exist, the whole conversation is over, the client it is left to make up some good error message or retry version 1. I'd like for that to be a starting point for us, and then to be able to add-on hints to ease the transition path in whatever way we want. We may even not do that in the long run, but I want to leave the door open if we can. But I think advertising both which versions the server could deal with, as well as the currently expected version is a good thing. capability: can_speak=1,2 capability: speaking_now=2 I was thinking just speaking_now=2, but it probably makes sense to do both. I do not think using it to downgrade will ever be particularly useful (certainly not from v2 to v1, since to understand the flag both sides must be v2 in the first place). But advertising that via the v1 conversation will be a good way to tell the other side that upgrade is possible. If for some reason we discover a flaw in the current version, which makes it unusable (a buffer overflow?, some stupid abuse which makes the capability list huge), you may want to force downgrading (and in the very distant future when we are current on version 4 and have dropped version 1 already, you can only downgrade to 2 and 3, so I can see value in it. Another idea to make it all more future proof: capability: speaking_now=2 must be sent as the first line, so then you can adapt on the client side easily for which version you are listening. Also, do we need the capability noise-word? I thought it opens up a new possible door in the future. As we ignore anything not starting with capability for now, you could introduce your foo and bar ping pong easily and still be version 2 compatible. S: capability: thin S: capability: another-capability S: ping-pong foo S: done C: (not having understood ping-pong) just answering with capability: thin C: done, let's proceed to refs advertisement The alternative client would do: C: ping-pong: foo-data1a S: ping-pong: foo-data1b C: ping-pong: foo-data2a C: capability: thin ... Right, but I think (and please correct me if there's a case I'm missing) that the behavior is the same whether it is spelled ping-pong or capability:ping-pong. That is, the rule for capability: is if you do not understand it, ignore it and do not mention it in your capabilities; the server is required to assume you were written before that capability was invented. But that is _also_ the rule for ping-pong, I think. The rules are the same, right. But the allowed characters are limited (in theory) as the regular expressions given for the capabilities don't allow for binary data for example, but only well formed ASCII text, space separated. The ping-pong keyword could introduce a binary stream there including line feeds. (Today it sounds like a stupid idea though) Eric mentioned the underflow problems here, but I wonder even more: what's wrong with the global ends_with() that we already provide? I was missing knowledge we have that, and apparently I was thinking it's faster to come up with my own version than to look for it. :) Makes sense. :) -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/4] showing existing ws breakage
On Wed, May 27, 2015 at 01:46:26PM -0700, Junio C Hamano wrote:
Jeff King p...@peff.net writes:
Do you want me to also eradicate PLAIN from the code itself? It's a
rather simple change, but it does touch a lot of places.
Nah, that is not user-facing. We do not do s/cache/index/ in the
code, either.
Besides, I actually find plain much easier to type than context ;-)
OK. I just did it while our emails were in flight, so here it is just
for reference.
-- 8 --
Subject: diff.h: rename DIFF_PLAIN color slot to DIFF_CONTEXT
The latter is a much more descriptive name (and we support
color.diff.context now). This also updates the name of any
local variables which were used to store the color.
Signed-off-by: Jeff King p...@peff.net
---
combine-diff.c | 6 +++---
diff.c | 26 +-
diff.h | 2 +-
line-log.c | 6 +++---
4 files changed, 20 insertions(+), 20 deletions(-)
diff --git a/combine-diff.c b/combine-diff.c
index 8eb7278..30c7eb6 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -730,7 +730,7 @@ static void dump_sline(struct sline *sline, const char
*line_prefix,
const char *c_func = diff_get_color(use_color, DIFF_FUNCINFO);
const char *c_new = diff_get_color(use_color, DIFF_FILE_NEW);
const char *c_old = diff_get_color(use_color, DIFF_FILE_OLD);
- const char *c_plain = diff_get_color(use_color, DIFF_PLAIN);
+ const char *c_context = diff_get_color(use_color, DIFF_CONTEXT);
const char *c_reset = diff_get_color(use_color, DIFF_RESET);
if (result_deleted)
@@ -793,7 +793,7 @@ static void dump_sline(struct sline *sline, const char
*line_prefix,
}
if (comment_end)
printf(%s%s %s%s, c_reset,
- c_plain, c_reset,
+ c_context, c_reset,
c_func);
for (i = 0; i comment_end; i++)
putchar(hunk_comment[i]);
@@ -828,7 +828,7 @@ static void dump_sline(struct sline *sline, const char
*line_prefix,
*/
if (!context)
continue;
- fputs(c_plain, stdout);
+ fputs(c_context, stdout);
}
else
fputs(c_new, stdout);
diff --git a/diff.c b/diff.c
index 27bd371..100773f 100644
--- a/diff.c
+++ b/diff.c
@@ -42,7 +42,7 @@ static long diff_algorithm;
static char diff_colors[][COLOR_MAXLEN] = {
GIT_COLOR_RESET,
- GIT_COLOR_NORMAL, /* PLAIN */
+ GIT_COLOR_NORMAL, /* CONTEXT */
GIT_COLOR_BOLD, /* METAINFO */
GIT_COLOR_CYAN, /* FRAGINFO */
GIT_COLOR_RED, /* OLD */
@@ -55,7 +55,7 @@ static char diff_colors[][COLOR_MAXLEN] = {
static int parse_diff_color_slot(const char *var)
{
if (!strcasecmp(var, context) || !strcasecmp(var, plain))
- return DIFF_PLAIN;
+ return DIFF_CONTEXT;
if (!strcasecmp(var, meta))
return DIFF_METAINFO;
if (!strcasecmp(var, frag))
@@ -501,7 +501,7 @@ static void emit_add_line(const char *reset,
static void emit_hunk_header(struct emit_callback *ecbdata,
const char *line, int len)
{
- const char *plain = diff_get_color(ecbdata-color_diff, DIFF_PLAIN);
+ const char *context = diff_get_color(ecbdata-color_diff, DIFF_CONTEXT);
const char *frag = diff_get_color(ecbdata-color_diff, DIFF_FRAGINFO);
const char *func = diff_get_color(ecbdata-color_diff, DIFF_FUNCINFO);
const char *reset = diff_get_color(ecbdata-color_diff, DIFF_RESET);
@@ -518,7 +518,7 @@ static void emit_hunk_header(struct emit_callback *ecbdata,
if (len 10 ||
memcmp(line, atat, 2) ||
!(ep = memmem(line + 2, len - 2, atat, 2))) {
- emit_line(ecbdata-opt, plain, reset, line, len);
+ emit_line(ecbdata-opt, context, reset, line, len);
return;
}
ep += 2; /* skip over @@ */
@@ -540,7 +540,7 @@ static void emit_hunk_header(struct emit_callback *ecbdata,
if (*ep != ' ' *ep != '\t')
break;
if (ep != cp) {
- strbuf_addstr(msgbuf, plain);
+ strbuf_addstr(msgbuf, context);
strbuf_add(msgbuf, cp, ep - cp);
strbuf_addstr(msgbuf, reset);
}
@@ -623,10 +623,10 @@ static void emit_rewrite_lines(struct emit_callback *ecb,
data += len;
}
if (!endp) {
- const char *plain = diff_get_color(ecb-color_diff,
- DIFF_PLAIN);
+
Re: [PATCH v3 0/4] showing existing ws breakage
Jeff King p...@peff.net writes:
On Wed, May 27, 2015 at 01:46:26PM -0700, Junio C Hamano wrote:
Jeff King p...@peff.net writes:
Do you want me to also eradicate PLAIN from the code itself? It's a
rather simple change, but it does touch a lot of places.
Nah, that is not user-facing. We do not do s/cache/index/ in the
code, either.
Besides, I actually find plain much easier to type than context ;-)
OK. I just did it while our emails were in flight, so here it is just
for reference.
I actually like that; the changes are fairly isolated and contained.
-- 8 --
Subject: diff.h: rename DIFF_PLAIN color slot to DIFF_CONTEXT
The latter is a much more descriptive name (and we support
color.diff.context now). This also updates the name of any
local variables which were used to store the color.
Signed-off-by: Jeff King p...@peff.net
---
combine-diff.c | 6 +++---
diff.c | 26 +-
diff.h | 2 +-
line-log.c | 6 +++---
4 files changed, 20 insertions(+), 20 deletions(-)
diff --git a/combine-diff.c b/combine-diff.c
index 8eb7278..30c7eb6 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -730,7 +730,7 @@ static void dump_sline(struct sline *sline, const char
*line_prefix,
const char *c_func = diff_get_color(use_color, DIFF_FUNCINFO);
const char *c_new = diff_get_color(use_color, DIFF_FILE_NEW);
const char *c_old = diff_get_color(use_color, DIFF_FILE_OLD);
- const char *c_plain = diff_get_color(use_color, DIFF_PLAIN);
+ const char *c_context = diff_get_color(use_color, DIFF_CONTEXT);
const char *c_reset = diff_get_color(use_color, DIFF_RESET);
if (result_deleted)
@@ -793,7 +793,7 @@ static void dump_sline(struct sline *sline, const char
*line_prefix,
}
if (comment_end)
printf(%s%s %s%s, c_reset,
- c_plain, c_reset,
+ c_context, c_reset,
c_func);
for (i = 0; i comment_end; i++)
putchar(hunk_comment[i]);
@@ -828,7 +828,7 @@ static void dump_sline(struct sline *sline, const char
*line_prefix,
*/
if (!context)
continue;
- fputs(c_plain, stdout);
+ fputs(c_context, stdout);
}
else
fputs(c_new, stdout);
diff --git a/diff.c b/diff.c
index 27bd371..100773f 100644
--- a/diff.c
+++ b/diff.c
@@ -42,7 +42,7 @@ static long diff_algorithm;
static char diff_colors[][COLOR_MAXLEN] = {
GIT_COLOR_RESET,
- GIT_COLOR_NORMAL, /* PLAIN */
+ GIT_COLOR_NORMAL, /* CONTEXT */
GIT_COLOR_BOLD, /* METAINFO */
GIT_COLOR_CYAN, /* FRAGINFO */
GIT_COLOR_RED, /* OLD */
@@ -55,7 +55,7 @@ static char diff_colors[][COLOR_MAXLEN] = {
static int parse_diff_color_slot(const char *var)
{
if (!strcasecmp(var, context) || !strcasecmp(var, plain))
- return DIFF_PLAIN;
+ return DIFF_CONTEXT;
if (!strcasecmp(var, meta))
return DIFF_METAINFO;
if (!strcasecmp(var, frag))
@@ -501,7 +501,7 @@ static void emit_add_line(const char *reset,
static void emit_hunk_header(struct emit_callback *ecbdata,
const char *line, int len)
{
- const char *plain = diff_get_color(ecbdata-color_diff, DIFF_PLAIN);
+ const char *context = diff_get_color(ecbdata-color_diff, DIFF_CONTEXT);
const char *frag = diff_get_color(ecbdata-color_diff, DIFF_FRAGINFO);
const char *func = diff_get_color(ecbdata-color_diff, DIFF_FUNCINFO);
const char *reset = diff_get_color(ecbdata-color_diff, DIFF_RESET);
@@ -518,7 +518,7 @@ static void emit_hunk_header(struct emit_callback
*ecbdata,
if (len 10 ||
memcmp(line, atat, 2) ||
!(ep = memmem(line + 2, len - 2, atat, 2))) {
- emit_line(ecbdata-opt, plain, reset, line, len);
+ emit_line(ecbdata-opt, context, reset, line, len);
return;
}
ep += 2; /* skip over @@ */
@@ -540,7 +540,7 @@ static void emit_hunk_header(struct emit_callback
*ecbdata,
if (*ep != ' ' *ep != '\t')
break;
if (ep != cp) {
- strbuf_addstr(msgbuf, plain);
+ strbuf_addstr(msgbuf, context);
strbuf_add(msgbuf, cp, ep - cp);
strbuf_addstr(msgbuf, reset);
}
@@ -623,10 +623,10 @@ static void emit_rewrite_lines(struct emit_callback
*ecb,
data += len;
}
if (!endp) {
- const char *plain =
Re: [PATCH 1/2] config: add options to list only variable names
On Wed, May 27, 2015 at 10:07:19PM +0200, SZEDER Gábor wrote:
Help the completion script by introducing the '--list-names' and
'--get-names-regexp' options, the names-only equivalents of '--list' and
'--get-regexp', so it doesn't have to separate variable names from their
values anymore.
Thanks, this sounds like the best solution. It should be a tiny bit more
efficient, too, though I doubt it matters much in practice.
-'git config' [file-option] [-z|--null] -l | --list
+'git config' [file-option] [-z|--null] -l | --list | --list-name
s/list-name/s/, to match the code (and your commit message).
@@ -161,6 +166,9 @@ See also FILES.
--list::
List all variables set in config file.
+--list-name::
+ List the names of all variables set in config file.
Ditto here. Also, now that we have two similar modes, perhaps the
--list description above should become:
List all variables set in config file, along with their values.
@@ -165,7 +170,14 @@ static int collect_config(const char *key_, const char
*value_, void *cb)
ALLOC_GROW(values-items, values-nr + 1, values-alloc);
- return format_config(values-items[values-nr++], key_, value_);
+ if (show_only_keys) {
+ struct strbuf *buf = values-items[values-nr++];
+ strbuf_init(buf, 0);
+ strbuf_addstr(buf, key_);
+ strbuf_addch(buf, term);
+ return 0;
+ } else
+ return format_config(values-items[values-nr++], key_,
value_);
}
Might it flow a little better to always enter format_config, and then
just return early (before writing the value) when show_key_only is set?
cat expect EOF
+beta.noindent
+nextsection.nonewline
+123456.a123
+version.1.2.3eX.alpha
+EOF
+
+test_expect_success 'working --list-names' '
+ git config --list-names output
+ test_cmp expect output
+'
+
+cat expect EOF
We usually avoid the extra space after redirection operators. But we
also usually match existing code. I'm not sure which is more evil in
this case. ;)
+test_expect_success '--get-name-regexp' '
+ git config --get-name-regexp in output
+ test_cmp expect output
+'
This one is the odd man out if you are following existing style,
though.
The rest of the patch looks good to me.
-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] config: add options to list only variable names
Quoting Jeff King p...@peff.net: On Wed, May 27, 2015 at 10:07:19PM +0200, SZEDER Gábor wrote: Help the completion script by introducing the '--list-names' and '--get-names-regexp' options, the names-only equivalents of '--list' and '--get-regexp', so it doesn't have to separate variable names from their values anymore. Thanks, this sounds like the best solution. It should be a tiny bit more efficient, too, though I doubt it matters much in practice. -'git config' [file-option] [-z|--null] -l | --list +'git config' [file-option] [-z|--null] -l | --list | --list-name s/list-name/s/, to match the code (and your commit message). And note how I added an extra 's' to the other option in the commit message! cat expect EOF +beta.noindent +nextsection.nonewline +123456.a123 +version.1.2.3eX.alpha +EOF + +test_expect_success 'working --list-names' ' + git config --list-names output + test_cmp expect output +' + +cat expect EOF We usually avoid the extra space after redirection operators. But we also usually match existing code. I'm not sure which is more evil in this case. ;) +test_expect_success '--get-name-regexp' ' + git config --get-name-regexp in output + test_cmp expect output +' This one is the odd man out if you are following existing style, though. Heh, in both cases I simply copied the existing name-less test, and only adjusted the expected output and the name of the option to test. :) Will reroll. Gábor -- 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/WIP PATCH 04/11] upload-pack-2: Implement the version 2 of upload-pack
On Wed, May 27, 2015 at 10:40:37AM -0700, Stefan Beller wrote: If we are upload-pack-2, should we advertise that in the capabilities? I think it may make things easier later if we try to provide some opportunistic out-of-band data. E.g., if see tell git-daemon: git-upload-pack repo\0host=whatever\0\0version=2 how do we know whether version=2 was understood and kicked us into v2 mode, versus an old server that ignored it? So in my vision we would call git-upload-pack-2 instead of having a version=2. and if git-upload-pack-2 doesn't exist, the whole conversation is over, the client it is left to make up some good error message or retry version 1. I'd like for that to be a starting point for us, and then to be able to add-on hints to ease the transition path in whatever way we want. We may even not do that in the long run, but I want to leave the door open if we can. But I think advertising both which versions the server could deal with, as well as the currently expected version is a good thing. capability: can_speak=1,2 capability: speaking_now=2 I was thinking just speaking_now=2, but it probably makes sense to do both. I do not think using it to downgrade will ever be particularly useful (certainly not from v2 to v1, since to understand the flag both sides must be v2 in the first place). But advertising that via the v1 conversation will be a good way to tell the other side that upgrade is possible. Also, do we need the capability noise-word? I thought it opens up a new possible door in the future. As we ignore anything not starting with capability for now, you could introduce your foo and bar ping pong easily and still be version 2 compatible. S: capability: thin S: capability: another-capability S: ping-pong foo S: done C: (not having understood ping-pong) just answering with capability: thin C: done, let's proceed to refs advertisement The alternative client would do: C: ping-pong: foo-data1a S: ping-pong: foo-data1b C: ping-pong: foo-data2a C: capability: thin ... Right, but I think (and please correct me if there's a case I'm missing) that the behavior is the same whether it is spelled ping-pong or capability:ping-pong. That is, the rule for capability: is if you do not understand it, ignore it and do not mention it in your capabilities; the server is required to assume you were written before that capability was invented. But that is _also_ the rule for ping-pong, I think. Eric mentioned the underflow problems here, but I wonder even more: what's wrong with the global ends_with() that we already provide? I was missing knowledge we have that, and apparently I was thinking it's faster to come up with my own version than to look for it. :) Makes sense. :) -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC 1/2] git-rebase -i: Add key word drop to remove a commit
Matthieu Moy matthieu@grenoble-inp.fr writes: Junio C Hamano gits...@pobox.com writes: Matthieu Moy matthieu@grenoble-inp.fr writes: I find it weird to write noop sha1 title True, but then it can be spelled # sha1 title I do find it weird too. # means comment, which means do as if it was not there to me. And in this case it does change the semantics once you activate the safety feature: error out without the # sha1 title, rebase dropping the commit if the comment is present. Well, I do not agree with the premise that A line was removed, the user may have made a mistake, we need to warn about it is a good idea in the first place. Removing an insn that is not wanted has been the way to skip and not replay a change from the beginning of the time, and users shouldn't be trained into thinking that somehow is a bad practice by having such an option that warns. -- 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/WIP 6/8] am: extract patch, message and authorship with git-mailinfo
Paul Tan pyoka...@gmail.com writes:
@@ -17,6 +34,10 @@ struct am_state {
struct strbuf dir;/* state directory path */
int cur; /* current patch number */
int last; /* last patch number */
+ struct strbuf msg;/* commit message */
+ struct strbuf author_name;/* commit author's name */
+ struct strbuf author_email; /* commit author's email */
+ struct strbuf author_date;/* commit author's date */
int prec; /* number of digits in patch filename */
};
I always get suspicious when structure fields are overly commented,
wondering if it is a sign of naming fields poorly. All of the above
fields look quite self-explanatory and I am not sure if it is worth
having these comments, spending efforts to type SP many times to
align them and all.
By the way, the overall structure of the series look sensible.
+static int read_author_script(struct am_state *state)
+{
+ char *value;
+ struct strbuf sb = STRBUF_INIT;
+ const char *filename = am_path(state, author-script);
+ FILE *fp = fopen(filename, r);
+ if (!fp) {
+ if (errno == ENOENT)
+ return 0;
+ die(_(could not open '%s' for reading), filename);
Hmph, do we want to report with die_errno()?
+ }
+
+ if (strbuf_getline(sb, fp, '\n'))
+ return -1;
+ if (!skip_prefix(sb.buf, GIT_AUTHOR_NAME=, (const char**) value))
This cast is unfortunate; can't value be of const char * type?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/4] showing existing ws breakage
On Wed, May 27, 2015 at 03:22:18AM -0400, Jeff King wrote: In color.diff.*, these are called new, old, and plain. I am of the opinion that context is a far better name than plain, but perhaps we should support both for consistency. Here's a patch for the color.diff side, if we want to go that route. So I had originally thought we would support both names in both places. But since the diff patch we ended up with is basically the real name is context; plain is just a historical anomaly, I do not see any need to support plain in your new whitespace code. I suspect you came to the same conclusion independently, but I wanted to make sure what I had written before didn't leave anybody confused. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] glossary: add remote and submodule
Noticed-by: Philip Oakley philipoak...@iee.org Signed-off-by: Stefan Beller sbel...@google.com --- Documentation/glossary-content.txt | 10 ++ 1 file changed, 10 insertions(+) diff --git a/Documentation/glossary-content.txt b/Documentation/glossary-content.txt index bf383c2..e303135 100644 --- a/Documentation/glossary-content.txt +++ b/Documentation/glossary-content.txt @@ -469,6 +469,11 @@ The most notable example is `HEAD`. def_push,push to describe the mapping between remote def_ref,ref and local ref. +[[def_remote]]remote repository:: + A def_repository,repository which is used to track the same + project but resides somewhere else. To communicate with remotes, + see def_fetch,fetch or def_push,push. + [[def_remote_tracking_branch]]remote-tracking branch:: A def_ref,ref that is used to follow changes from another def_repository,repository. It typically looks like @@ -515,6 +520,11 @@ The most notable example is `HEAD`. is created by giving the `--depth` option to linkgit:git-clone[1], and its history can be later deepened with linkgit:git-fetch[1]. +[[def_submodule]]submodule:: + A def_repository,repository inside another repository. The two + repositories have different history, though the outer repository + knows the commit of the inner repository. + [[def_symref]]symref:: Symbolic reference: instead of containing the def_SHA1,SHA-1 id itself, it is of the format 'ref: refs/some/thing' and when -- 2.4.1.345.gab207b6.dirty -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/WIP PATCH 00/11] Protocol version 2, again!
On Tue, May 26, 2015 at 03:01:04PM -0700, Stefan Beller wrote: Just give us something to play around with - Peff at GitMerge 2015 Sounds like something I would say. The new protocol works just like the old protocol, except for the capabilities negotiation being before any exchange of real data. I like this approach. We all know that one next step is going to be a show me only these refs capability negotiation of some kind. But it's good to keep the version-breaking changes separated from that, and this should be enough to bootstrap that conversation later. If I understand correctly, your proposed protocol allows for a single back-and-forth of capabilities followed by the server speaking the ref advertisement. So it is worth thinking a moment how we might have a more involved conversation before the advertisement if we want to do so in the future. I think in the simplest case, the server claims to understand the foo extension, and then the client says I wish to use foo. And then a rule of foo might be that the conversation continues in some way before sending the advertisement. Like (each line is a pkt-line): ... S: foo S: flush ... C: foo S: Here's some extra foo data. C: Now I respond to that foo data. S: Now the foo conversation is done. Here's the ref advertisement. What if there are multiple such extensions? E.g., if the client asks for both foo and bar, and both require extra back-and-forth messages? Which conversation happens first? Maybe the rule is just whichever one the client asked for first in the capabilities list. And I think in general we want to avoid protocol round-trips if we can (so we'd prefer the client say refspec=refs/heads/*, and not I understand refspecs, too! Let's have a conversation about which ones I'm interested in.). But I think it's worth giving it a little thought to make sure we don't paint ourselves in a corner. My preference for a string suffix instead of a sequential number is motiviated by the discussion of sha1 vs sequential numbers to describe a state of a repository. The main difference here is however how often we expect changes. Version 1 of the protocol is current for 10 years by now, so I do not changes to the protocol number often, which makes it suitable for just having a counter appended to the binary. I think I prefer a number. I'm really hoping that v2 lasts even longer than v1 has, because the capability negotiation should allow us to extend it from within rather than breaking compatibility. As a minor nit, I think I like upload-pack-v2 better than upload-pack-2. But I can live with it either way. :) This series doesn't include an automatic upgrade of the protocol version for later changes if the server supports it, so for now the use of the new protocol needs to be activated manually via setting remote.origin.transportversion. I think that's a good start. Last time we discussed it, I think everybody was more or less on board with client probing (so v1 would start to say btw, I speak v2, and then client would set its remote.origin.transportversion flag). That can come later, and we are not painting ourselves into a corner. I think we also discussed passing the version information out-of-band. So over git-daemon, as git-upload-pack repo\0host=...\0\0version=2, for example. I _think_ we are also fine to build that on top of what you have here. If the version information makes it through to upload-pack, then we can do v2, and if not, we are no worse off than we were. HTTP can do a similar out-of-band thing, but I think ssh is mostly out of luck. The best I could think of was passing an environment variable, but ssh typically only lets through a few variables. We could abuse PATH or something, but that's getting pretty nasty. Anyway, that is all for the future. The only reason I mention it is to make sure that we are not closing any future doors, and I don't think we are. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC 1/2] git-rebase -i: Add key word drop to remove a commit
Hi Rémi,
On 2015-05-26 23:38, Galan Rémi wrote:
Instead of removing a line to remove the commit, you can use the key
word drop (just like pick or edit). It has the same effect as
deleting the line (removing the commit) except that you keep a visual
trace of your actions, allowing a better control and reducing the
possibility of removing a commit by mistake.
Please note that you can already just comment-out the line if you need to keep
a visual trace.
Alternatively, you can replace the `pick` command by `noop`.
If you really need the `drop` command (with which I am not 100% happy because I
already envisage users appending a `drop A` to an edit script pick A; pick B;
pick C and expecting A *not to be picked*), then it is better to just add the
`drop` part to the already existing `noop` clause:
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index f7deeb0..8355be8 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -489,7 +489,7 @@ do_next () {
rm -f $msg $author_script $amend || exit
read -r command sha1 rest $todo
case $command in
- $comment_char*|''|noop)
+ $comment_char*|''|noop|drop)
mark_action_done
;;
pick|p)
Ciao,
Johannes
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/WIP PATCH 04/11] upload-pack-2: Implement the version 2 of upload-pack
On Tue, May 26, 2015 at 03:01:08PM -0700, Stefan Beller wrote:
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -716,10 +716,47 @@ static void format_symref_info(struct strbuf *buf,
struct string_list *symref)
strbuf_addf(buf, symref=%s:%s, item-string, (char
*)item-util);
}
-static const char *advertise_capabilities = multi_ack thin-pack side-band
+static char *advertise_capabilities = multi_ack thin-pack side-band
side-band-64k ofs-delta shallow no-progress
include-tag multi_ack_detailed;
If we are upload-pack-2, should we advertise that in the capabilities? I
think it may make things easier later if we try to provide some
opportunistic out-of-band data. E.g., if see tell git-daemon:
git-upload-pack repo\0host=whatever\0\0version=2
how do we know whether version=2 was understood and kicked us into v2
mode, versus an old server that ignored it?
It also just makes the protocol more self-describing; from the
conversation you can see which version is in use.
+static void send_capabilities(void)
+{
+ char buf[100];
+
+ while (next_capability(buf))
+ packet_write(1, capability:%s\n, buf);
Having a static-sized buffer and then passing it to a function which has
no idea of the buffer size seems like a recipe for a buffer overflow. :)
You are fine here because you are parsing the hard-coded capabilities
string, and we know 100 is enough there. But it's nice to avoid such
magic.
Like Eric, I find the whole next_capability thing a little ugly. His
suggestion to pass in the parsing state is an improvement, but I wonder
why we need to parse at all. Can we keep the capabilities as:
const char *capabilities[] = {
multi_ack,
thin-pack,
... etc ...
};
and then loop over the array? The v1 writer will need to be modified,
but it should be fairly straightforward (loop and add them to the buffer
rather than dumping the whole string).
Also, do we need the capability noise-word? They all have it, except
for:
+ packet_write(1, agent:%s\n, git_user_agent_sanitized());
But isn't that basically a capability, too (I agree it is stretching the
definition of capability, but I think that ship has sailed; the
client's response is not I support this, too but I want to enable
this).
IOW, is there a reason that the initial conversation is not just:
pkt-line(multi_ack);
pkt-line(thin-pack);
...
pkt-line(agent=v2.5.0);
pkt-line();
We already have framing for each capability due to the use of pkt-line.
The capability: is just one more thing the client has to parse past.
The keys are already unique up to any =, so I don't think there is any
ambiguity (e.g., we don't care about capability:agent; we have already
assigned a meaning to the term agent, and will never introduce a
standalone capability with the same name).
Likewise, if we introduce new protocol items here, the client should
either ignore them (if it does not understand them) or know what to do
with them.
+static void receive_capabilities(void)
+{
+ int done = 0;
+ while (1) {
+ char *line = packet_read_line(0, NULL);
+ if (!line)
+ break;
+ if (starts_with(line, capability:))
+ parse_features(line + strlen(capability:));
+ }
Use:
const char *cap;
if (skip_prefix(line, capability:, cap))
parse_features(cap);
Or better yet, if you take my suggestion above:
parse_features(line);
:)
+static int endswith(const char *teststring, const char *ending)
+{
+ int slen = strlen(teststring);
+ int elen = strlen(ending);
+ return !strcmp(teststring + slen - elen, ending);
+}
Eric mentioned the underflow problems here, but I wonder even more:
what's wrong with the global ends_with() that we already provide?
-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/WIP PATCH 05/11] transport: add infrastructure to support a protocol version number
On Tue, May 26, 2015 at 03:01:09PM -0700, Stefan Beller wrote:
+ OPT_STRING('y', transport-version, transport_version,
+N_(transport-version),
+N_(specify transport version to be used)),
Interesting choice for the short option (-v would be nice, but
obviously it is taken). Do we want to delay on claiming the
short-and-sweet 'y' until we are sure this is something people will use
a lot? In an ideal world, it is not (i.e., auto-upgrade and other tricks
become good enough that nobody bothers to specify it manually).
-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/WIP PATCH 06/11] remote.h: add get_remote_capabilities, request_capabilities
On Tue, May 26, 2015 at 03:01:10PM -0700, Stefan Beller wrote:
+void get_remote_capabilities(int in, char *src_buf, size_t src_len)
+{
+ struct strbuf capabilities_string = STRBUF_INIT;
+ for (;;) {
+ int len;
+ char *line = packet_buffer;
+ const char *arg;
+
+ len = packet_read(in, src_buf, src_len,
+ packet_buffer, sizeof(packet_buffer),
+ PACKET_READ_GENTLE_ON_EOF |
+ PACKET_READ_CHOMP_NEWLINE);
+ if (len 0)
+ die_initial_contact(0);
+
+ if (!len)
+ break;
+
+ if (len 4 skip_prefix(line, ERR , arg))
+ die(remote error: %s, arg);
+
+ if (starts_with(line, capability:)) {
+ strbuf_addstr(capabilities_string, line +
strlen(capability:));
+ strbuf_addch(capabilities_string, ' ');
+ }
+ }
I think this is the reverse case of next_capabilities in the upload-pack
side, so I'll make the reverse suggestion. :) Would it make things nicer
if both v1 and v2 parsed the capabilities into a string_list?
+ free(server_capabilities);
+ server_capabilities = xmalloc(capabilities_string.len + 1);
+ server_capabilities = strbuf_detach(capabilities_string, NULL);
Is this xmalloc line left over? The strbuf_detach will write over it.
+ strbuf_release(capabilities_string);
No need to release if we just detached.
+int request_capabilities(int out)
+{
+ fprintf(stderr, request_capabilities\n);
Debug cruft, I presume.
+ // todo: send our capabilities
+ packet_write(out, capability:foo);
No C99 comments. But I think this is just a placeholder. :)
-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/WIP PATCH 07/11] fetch-pack: use the configured transport protocol
On Tue, May 26, 2015 at 03:01:11PM -0700, Stefan Beller wrote:
diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index 4a6b340..32dc8b0 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -127,6 +127,10 @@ int cmd_fetch_pack(int argc, const char **argv, const
char *prefix)
args.update_shallow = 1;
continue;
}
+ if (!strcmp(--transport-version, arg)) {
+ args.version = strtol(arg +
strlen(--transport-version), NULL, 0);
+ continue;
+ }
You strcmp() the arg here, so there can't be anything at arg +
strlen(...), can there? Did you want:
starts_with(arg, --transports-version=)
here? Or better yet, skip_prefix().
- get_remote_heads(fd[0], NULL, 0, ref, 0, NULL, shallow);
+
+ switch (args.version) {
+ default:
+ case 2:
+ get_remote_capabilities(fd[0], NULL, 0);
+ request_capabilities(fd[1]);
+ break;
Should the default case be to complain about an unknown version,
rather than fall-through to 2?
-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 1/4] t4015: modernise style
Move the preparatory steps that create the expected output inside
the test bodies, remove unnecessary blank lines before and after the
test bodies, and drop SP between redirection operator and its target.
Signed-off-by: Junio C Hamano gits...@pobox.com
---
t/t4015-diff-whitespace.sh | 411 +++--
1 file changed, 173 insertions(+), 238 deletions(-)
diff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh
index 604a838..0bfc7ff 100755
--- a/t/t4015-diff-whitespace.sh
+++ b/t/t4015-diff-whitespace.sh
@@ -9,138 +9,144 @@ test_description='Test special whitespace in diff engine.
. ./test-lib.sh
. $TEST_DIRECTORY/diff-lib.sh
-# Ray Lehtiniemi's example
+test_expect_success Ray Lehtiniemi's example '
+ cat -\EOF x
+ do {
+ nothing;
+ } while (0);
+ EOF
+ git update-index --add x
-cat EOF x
-do {
- nothing;
-} while (0);
-EOF
+ cat -\EOF x
+ do
+ {
+ nothing;
+ }
+ while (0);
+ EOF
-git update-index --add x
+ cat -\EOF expect
+ diff --git a/x b/x
+ index adf3937..6edc172 100644
+ --- a/x
+ +++ b/x
+ @@ -1,3 +1,5 @@
+ -do {
+ +do
+ +{
+ nothing;
+ -} while (0);
+ +}
+ +while (0);
+ EOF
-cat EOF x
-do
-{
- nothing;
-}
-while (0);
-EOF
+ git diff out
+ test_cmp expect out
-cat EOF expect
-diff --git a/x b/x
-index adf3937..6edc172 100644
a/x
-+++ b/x
-@@ -1,3 +1,5 @@
--do {
-+do
-+{
-nothing;
--} while (0);
-+}
-+while (0);
-EOF
+ git diff -w out
+ test_cmp expect out
-git diff out
-test_expect_success Ray's example without options 'test_cmp expect out'
+ git diff -b out
+ test_cmp expect out
+'
-git diff -w out
-test_expect_success Ray's example with -w 'test_cmp expect out'
+test_expect_success 'another test, without options' '
+ tr Q \015 -\EOF x
+ whitespace at beginning
+ whitespace change
+ whitespace in the middle
+ whitespace at end
+ unchanged line
+ CR at endQ
+ EOF
-git diff -b out
-test_expect_success Ray's example with -b 'test_cmp expect out'
+ git update-index x
-tr 'Q' '\015' EOF x
-whitespace at beginning
-whitespace change
-whitespace in the middle
-whitespace at end
-unchanged line
-CR at endQ
-EOF
+ tr _ -\EOF x
+ _ whitespace at beginning
+ whitespace change
+ white space in the middle
+ whitespace at end__
+ unchanged line
+ CR at end
+ EOF
-git update-index x
+ tr Q_ \015 -\EOF expect
+ diff --git a/x b/x
+ index d99af23..22d9f73 100644
+ --- a/x
+ +++ b/x
+ @@ -1,6 +1,6 @@
+ -whitespace at beginning
+ -whitespace change
+ -whitespace in the middle
+ -whitespace at end
+ + whitespace at beginning
+ +whitespace change
+ +white space in the middle
+ +whitespace at end__
+unchanged line
+ -CR at endQ
+ +CR at end
+ EOF
-tr '_' ' ' EOF x
- whitespace at beginning
-whitespace change
-white space in the middle
-whitespace at end__
-unchanged line
-CR at end
-EOF
+ git diff out
+ test_cmp expect out
-tr 'Q_' '\015 ' EOF expect
-diff --git a/x b/x
-index d99af23..8b32fb5 100644
a/x
-+++ b/x
-@@ -1,6 +1,6 @@
--whitespace at beginning
--whitespace change
--whitespace in the middle
--whitespace at end
-+ whitespace at beginning
-+whitespace change
-+white space in the middle
-+whitespace at end__
- unchanged line
--CR at endQ
-+CR at end
-EOF
-git diff out
-test_expect_success 'another test, without options' 'test_cmp expect out'
+ expect
+ git diff -w out
+ test_cmp expect out
+
+ git diff -w -b out
+ test_cmp expect out
+
+ git diff -w --ignore-space-at-eol out
+ test_cmp expect out
+
+ git diff -w -b --ignore-space-at-eol out
+ test_cmp expect out
-cat EOF expect
-EOF
-git diff -w out
-test_expect_success 'another test, with -w' 'test_cmp expect out'
-git diff -w -b out
-test_expect_success 'another test, with -w -b' 'test_cmp expect out'
-git diff -w --ignore-space-at-eol out
-test_expect_success 'another test, with -w --ignore-space-at-eol' 'test_cmp
expect out'
-git diff -w -b --ignore-space-at-eol out
-test_expect_success 'another test, with -w -b --ignore-space-at-eol' 'test_cmp
expect out'
-
-tr 'Q_' '\015 ' EOF expect
-diff --git a/x b/x
-index d99af23..8b32fb5 100644
a/x
-+++ b/x
-@@ -1,6 +1,6 @@
--whitespace at beginning
-+ whitespace at beginning
- whitespace change
--whitespace in the middle
-+white space in the middle
- whitespace at end__
- unchanged line
- CR at end
-EOF
-git diff -b out
-test_expect_success 'another test, with -b' 'test_cmp expect out'
-git diff -b --ignore-space-at-eol out
[PATCH v3 4/4] diff.c: --ws-error-highlight=kind option
Traditionally, we only cared about whitespace breakages introduced
in new lines. Some people want to paint whitespace breakages on old
lines, too. When they see a whitespace breakage on a new line, they
can spot the same kind of whitespace breakage on the corresponding
old line and want to say Ah, those breakages are there but they
were inherited from the original, so let's not touch them for now.
Introduce `--ws-error-highlight=kind` option, that lets them pass
a comma separated list of `old`, `new`, and `context` to specify
what lines to highlight whitespace errors on.
Signed-off-by: Junio C Hamano gits...@pobox.com
---
Documentation/diff-options.txt | 10 +
diff.c | 84 +---
diff.h | 5 +++
t/t4015-diff-whitespace.sh | 96 ++
4 files changed, 179 insertions(+), 16 deletions(-)
diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index 6cb083a..85a6547 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -278,6 +278,16 @@ ifndef::git-format-patch[]
initial indent of the line are considered whitespace errors.
Exits with non-zero status if problems are found. Not compatible
with --exit-code.
+
+--ws-error-highlight=kind::
+ Highlight whitespace errors on lines specified by kind
+ in the color specified by `color.diff.whitespace`. kind
+ is a comma separated list of `old`, `new`, `context`. When
+ this option is not given, only whitespace errors in `new`
+ lines are highlighted. E.g. `--ws-error-highlight=new,old`
+ highlights whitespace errors on both deleted and added lines.
+ `all` can be used as a short-hand for `old,new,context`.
+
endif::git-format-patch[]
--full-index::
diff --git a/diff.c b/diff.c
index c575c45..34012b4 100644
--- a/diff.c
+++ b/diff.c
@@ -478,42 +478,57 @@ static int new_blank_line_at_eof(struct emit_callback
*ecbdata, const char *line
return ws_blank_line(line, len, ecbdata-ws_rule);
}
-static void emit_add_line(const char *reset,
- struct emit_callback *ecbdata,
- const char *line, int len)
+static void emit_line_checked(const char *reset,
+ struct emit_callback *ecbdata,
+ const char *line, int len,
+ enum color_diff color,
+ unsigned ws_error_highlight,
+ char sign)
{
- const char *ws = diff_get_color(ecbdata-color_diff, DIFF_WHITESPACE);
- const char *set = diff_get_color(ecbdata-color_diff, DIFF_FILE_NEW);
+ const char *set = diff_get_color(ecbdata-color_diff, color);
+ const char *ws = NULL;
- if (!*ws)
- emit_line_0(ecbdata-opt, set, reset, '+', line, len);
- else if (new_blank_line_at_eof(ecbdata, line, len))
+ if (ecbdata-opt-ws_error_highlight ws_error_highlight) {
+ ws = diff_get_color(ecbdata-color_diff, DIFF_WHITESPACE);
+ if (!*ws)
+ ws = NULL;
+ }
+
+ if (!ws)
+ emit_line_0(ecbdata-opt, set, reset, sign, line, len);
+ else if (sign == '+' new_blank_line_at_eof(ecbdata, line, len))
/* Blank line at EOF - paint '+' as well */
- emit_line_0(ecbdata-opt, ws, reset, '+', line, len);
+ emit_line_0(ecbdata-opt, ws, reset, sign, line, len);
else {
/* Emit just the prefix, then the rest. */
- emit_line_0(ecbdata-opt, set, reset, '+', , 0);
+ emit_line_0(ecbdata-opt, set, reset, sign, , 0);
ws_check_emit(line, len, ecbdata-ws_rule,
ecbdata-opt-file, set, reset, ws);
}
}
-static void emit_del_line(const char *reset,
+static void emit_add_line(const char *reset,
struct emit_callback *ecbdata,
const char *line, int len)
{
- const char *set = diff_get_color(ecbdata-color_diff, DIFF_FILE_OLD);
+ emit_line_checked(reset, ecbdata, line, len,
+ DIFF_FILE_NEW, WSEH_NEW, '+');
+}
- emit_line_0(ecbdata-opt, set, reset, '-', line, len);
+static void emit_del_line(const char *reset,
+ struct emit_callback *ecbdata,
+ const char *line, int len)
+{
+ emit_line_checked(reset, ecbdata, line, len,
+ DIFF_FILE_OLD, WSEH_OLD, '-');
}
static void emit_context_line(const char *reset,
struct emit_callback *ecbdata,
const char *line, int len)
{
- const char *set = diff_get_color(ecbdata-color_diff, DIFF_PLAIN);
-
- emit_line_0(ecbdata-opt, set, reset, ' ', line, len);
+ emit_line_checked(reset,
[PATCH v3 2/4] t4015: separate common setup and per-test expectation
The last two tests in the script were to
- set up color.diff.* slots
- set up an expectation for a single test
- run that test and check the result
but split in a wrong way. It did the first two in the first test
and the third one in the second test. The latter two belong to each
other. This matters when you plan to add more of these tests that
share the common coloring.
While at it, make sure we use a color different from old, which is
also red.
Signed-off-by: Junio C Hamano gits...@pobox.com
---
t/t4015-diff-whitespace.sh | 25 +
1 file changed, 13 insertions(+), 12 deletions(-)
diff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh
index 0bfc7ff..4da30e5 100755
--- a/t/t4015-diff-whitespace.sh
+++ b/t/t4015-diff-whitespace.sh
@@ -810,11 +810,20 @@ test_expect_success 'setup diff colors' '
git config color.diff.old red
git config color.diff.new green
git config color.diff.commit yellow
- git config color.diff.whitespace normal red
+ git config color.diff.whitespace blue
- git config core.autocrlf false
+ git config core.autocrlf false
+'
+
+test_expect_success 'diff that introduces a line with only tabs' '
+ git config core.whitespace blank-at-eol
+ git reset --hard
+ echo test x
+ git commit -m initial x
+ echo {NTN} | tr NT \n\t x
+ git -c color.diff=always diff | test_decode_color current
- cat expected -\EOF
+ cat expected -\EOF
BOLDdiff --git a/x b/xRESET
BOLDindex 9daeafb..2874b91 100644RESET
BOLD--- a/xRESET
@@ -822,18 +831,10 @@ test_expect_success 'setup diff colors' '
CYAN@@ -1 +1,4 @@RESET
testRESET
GREEN+RESETGREEN{RESET
- GREEN+RESETBRED RESET
+ GREEN+RESETBLUE RESET
GREEN+RESETGREEN}RESET
EOF
-'
-test_expect_success 'diff that introduces a line with only tabs' '
- git config core.whitespace blank-at-eol
- git reset --hard
- echo test x
- git commit -m initial x
- echo {NTN} | tr NT \n\t x
- git -c color.diff=always diff | test_decode_color current
test_cmp expected current
'
--
2.4.2-503-g3e4528a
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 0/4] showing existing ws breakage
We paint whitespace breakages in new (i.e. added or updated) lines when showing the git diff output to help people avoid introducing them with their changes. The basic premise is that people would want to avoid touching existing lines only to fix whitespace errors in a patch that does other changes of substance, and that is why we traditionally did not paint whitespace breakages in existing (i.e. deleted or context) lines. However, some people would want to keep existing breakages when they are doing other changes of substance; new lines in such a patch would show existing whitespace breakages painted, and it is not apparent if the breakages were inherited from the original or newly introduced. Christian Brabandt had an interesting idea to help users in this situation; why not give them a mode to paint whitespace breakages in old (i.e. deleted or was replaced) lines, too? http://thread.gmane.org/gmane.comp.version-control.git/269912/focus=269956 This series is a reroll of the previous one http://thread.gmane.org/gmane.comp.version-control.git/269972 which built on Christian'sidea, but with a different implementation (Christian's original painted trailing whitespaces only). The first two patches are unchanged since v2; they are preliminary clean-ups. The third one extends the corresponding patch since v2; not only a helper for deleted lines but also another helper for context lines is added. The fourth one in v2 used a new option --[no-]ws-check-deleted, but in this round a new option --ws-error-highlight=kinds is defined instead. With that, you can say diff --ws-error-highlight=new,old to say I want to see whitespace errors on new and old lines, and diff --ws-error-highlight=new,old,context diff --ws-error-highlight=all can be used to also see whitespace errors on context lines. Being able to see whitespace errors on context lines, i.e. the ones that were there in the original and you left intact, would help you see how prevalent whitespace errors are in the original and hopefully would make it easier for you to decide if a separate preliminary clean-up to only fix these whitespace errors is warranted. Junio C Hamano (4): t4015: modernise style t4015: separate common setup and per-test expectation diff.c: add emit_del_line() and emit_context_line() diff.c: --ws-error-highlight=kind option Documentation/diff-options.txt | 10 + diff.c | 122 -- diff.h | 5 + t/t4015-diff-whitespace.sh | 508 ++--- 4 files changed, 385 insertions(+), 260 deletions(-) -- 2.4.2-503-g3e4528a -- 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/4] diff.c: add emit_del_line() and emit_context_line()
Traditionally, we only had emit_add_line() helper, which knows how
to find and paint whitespace breakages on the given line, because we
only care about whitespace breakages introduced in new lines. The
context lines and old (i.e. deleted) lines are emitted with a
simpler emit_line_0() that paints the entire line in plain or old
colors.
Identify callers of emit_line_0() that show deleted lines and
context lines, have them call new helpers, emit_del_line() and
emit_context_line(), so that we can later tweak what is done to
these two classes of lines.
Signed-off-by: Junio C Hamano gits...@pobox.com
---
diff.c | 50 ++
1 file changed, 38 insertions(+), 12 deletions(-)
diff --git a/diff.c b/diff.c
index d1bd534..c575c45 100644
--- a/diff.c
+++ b/diff.c
@@ -498,6 +498,24 @@ static void emit_add_line(const char *reset,
}
}
+static void emit_del_line(const char *reset,
+ struct emit_callback *ecbdata,
+ const char *line, int len)
+{
+ const char *set = diff_get_color(ecbdata-color_diff, DIFF_FILE_OLD);
+
+ emit_line_0(ecbdata-opt, set, reset, '-', line, len);
+}
+
+static void emit_context_line(const char *reset,
+ struct emit_callback *ecbdata,
+ const char *line, int len)
+{
+ const char *set = diff_get_color(ecbdata-color_diff, DIFF_PLAIN);
+
+ emit_line_0(ecbdata-opt, set, reset, ' ', line, len);
+}
+
static void emit_hunk_header(struct emit_callback *ecbdata,
const char *line, int len)
{
@@ -603,7 +621,6 @@ static void emit_rewrite_lines(struct emit_callback *ecb,
{
const char *endp = NULL;
static const char *nneof = No newline at end of file\n;
- const char *old = diff_get_color(ecb-color_diff, DIFF_FILE_OLD);
const char *reset = diff_get_color(ecb-color_diff, DIFF_RESET);
while (0 size) {
@@ -613,8 +630,7 @@ static void emit_rewrite_lines(struct emit_callback *ecb,
len = endp ? (endp - data + 1) : size;
if (prefix != '+') {
ecb-lno_in_preimage++;
- emit_line_0(ecb-opt, old, reset, '-',
- data, len);
+ emit_del_line(reset, ecb, data, len);
} else {
ecb-lno_in_postimage++;
emit_add_line(reset, ecb, data, len);
@@ -1250,17 +1266,27 @@ static void fn_out_consume(void *priv, char *line,
unsigned long len)
return;
}
- if (line[0] != '+') {
- const char *color =
- diff_get_color(ecbdata-color_diff,
- line[0] == '-' ? DIFF_FILE_OLD :
DIFF_PLAIN);
- ecbdata-lno_in_preimage++;
- if (line[0] == ' ')
- ecbdata-lno_in_postimage++;
- emit_line(ecbdata-opt, color, reset, line, len);
- } else {
+ switch (line[0]) {
+ case '+':
ecbdata-lno_in_postimage++;
emit_add_line(reset, ecbdata, line + 1, len - 1);
+ break;
+ case '-':
+ ecbdata-lno_in_preimage++;
+ emit_del_line(reset, ecbdata, line + 1, len - 1);
+ break;
+ case ' ':
+ ecbdata-lno_in_postimage++;
+ ecbdata-lno_in_preimage++;
+ emit_context_line(reset, ecbdata, line + 1, len - 1);
+ break;
+ default:
+ /* incomplete line at the end */
+ ecbdata-lno_in_preimage++;
+ emit_line(ecbdata-opt,
+ diff_get_color(ecbdata-color_diff, DIFF_PLAIN),
+ reset, line, len);
+ break;
}
}
--
2.4.2-503-g3e4528a
--
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/WIP PATCH 06/11] remote.h: add get_remote_capabilities, request_capabilities
On Tue, May 26, 2015 at 11:25:05PM -0400, Eric Sunshine wrote: + len = packet_read(in, src_buf, src_len, + packet_buffer, sizeof(packet_buffer), + PACKET_READ_GENTLE_ON_EOF | + PACKET_READ_CHOMP_NEWLINE); + if (len 0) + die_initial_contact(0); + + if (!len) + break; + + if (len 4 skip_prefix(line, ERR , arg)) The 'len 4' check is needed because there's no guarantee that 'line' is NUL-terminated. Correct? I think this was just blindly copied from get_remote_heads(). And I think that code was being overly paranoid. Ever since f3a3214 (Make send/receive-pack be closer to doing something interesting, 2005-06-29), the pkt-line reader will add an extra NUL to the buffer to ease cases like this. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/WIP PATCH 08/11] transport: connect_setup appends protocol version number
On Tue, May 26, 2015 at 10:09:45PM -0700, Junio C Hamano wrote:
Stefan Beller sbel...@google.com writes:
On Tue, May 26, 2015 at 3:21 PM, Junio C Hamano gits...@pobox.com wrote:
if (...-version 2) {
... append -%d ...
}
involved.
Oh! I see here you would count the current one as 1, which has no
number extension, and any further would have a -${version}. That
would transport the intention much better I guess.
Yeah, except that I screwed up my comparison. Obviously, I should
have said If version is 2 or later, then append -%d to the name,
otherwise use the name as-is.
FWIW, I had similar head-scratching over Stefan's original. I think it's
OK to say version 1 is magical, and for historical reasons does not
have its number appended. There's no need for us ever to make
upload-pack-1; it just introduces more headaches.
-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/WIP PATCH 08/11] transport: connect_setup appends protocol version number
On Tue, May 26, 2015 at 03:01:12PM -0700, Stefan Beller wrote:
Signed-off-by: Stefan Beller sbel...@google.com
---
transport.c | 18 --
1 file changed, 16 insertions(+), 2 deletions(-)
diff --git a/transport.c b/transport.c
index 3ef15f6..33644a6 100644
--- a/transport.c
+++ b/transport.c
@@ -496,15 +496,29 @@ static int set_git_option(struct git_transport_options
*opts,
static int connect_setup(struct transport *transport, int for_push, int
verbose)
{
struct git_transport_data *data = transport-data;
+ const char *remote_program;
+ char *buf = 0;
Use NULL when you mean a NULL pointer (they're equivalent to the
compiler, but the word is easier to read).
I agree on Eric's naming this to_free (and I consider it idiomatic to
assign them in a chain, like foo = to_free = xmalloc(...), but we
don't always do that).
+ if (transport-smart_options
+ transport-smart_options-transport_version) {
+ buf = xmalloc(strlen(remote_program) + 12);
+ sprintf(buf, %s-%d, remote_program,
+ transport-smart_options-transport_version);
+ remote_program = buf;
+ }
Using xstrfmt can help you avoid magic numbers and repetition,
like:
to_free = xstrfmt(%s-%d,
remote_program,
transport-smart_options-transport_version);
-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/WIP PATCH 09/11] transport: get_refs_via_connect exchanges capabilities before refs.
On Tue, May 26, 2015 at 03:01:13PM -0700, Stefan Beller wrote:
+ switch (version) {
+ default: /*
+ * Configured a protocol version 2?
+ * Try version 2 as it's the most future proof.
+ */
+ /* fall through */
Same comment here as earlier. If we do a v3, it might not be compatible
with v2. Shouldn't we bail if the user asked for it?
+ case 2: /* first talk about capabilities, then get the heads */
+ get_remote_capabilities(data-fd[0], NULL, 0);
+ request_capabilities(data-fd[1]);
+ get_remote_heads(data-fd[0], NULL, 0, refs,
+ for_push ? REF_NORMAL : 0,
+ data-extra_have,
+ data-shallow);
+ break;
+ case 1: /* configured version 1, fall through */
+ case 0: /* unconfigured, use first protocol */
+ get_remote_heads(data-fd[0], NULL, 0, refs,
+ for_push ? REF_NORMAL : 0,
+ data-extra_have,
+ data-shallow);
+ break;
+ }
I actually kind of wonder if we should just die(BUG) here on seeing
0. Is this low level the right place to do the default to v1?
Because eventually we're going to want to default to v2, I would think
(after a few years have passed, at least). It seems like we should be
making that decision centrally when we init the transport options.
-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/WIP PATCH 00/11] Protocol version 2, again!
On Wed, May 27, 2015 at 02:18:18AM -0400, Jeff King wrote: The new protocol works just like the old protocol, except for the capabilities negotiation being before any exchange of real data. I like this approach. [...] So now I've read through all the patches. I still like it. :) There's a lot of work to be done still, but I think this is a great start. Thanks for getting the ball rolling. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/WIP PATCH 10/11] t5544: add a test case for the new protocol
On Tue, May 26, 2015 at 03:01:14PM -0700, Stefan Beller wrote: Signed-off-by: Stefan Beller sbel...@google.com --- t/t5544-fetch-2.sh | 40 1 file changed, 40 insertions(+) create mode 100755 t/t5544-fetch-2.sh Obviously we are not there yet, but a fun test will be to globally bump the transport version to 2 and try to run the test suite. We will also want to test interoperation between v1 and v2. Though the _best_ test of that is not hitting a v2 server configured for v1, but hitting an actual older version of git. Which is outside the scope of the current test suite, as it always operates on a single version. It might be nice to have a separate test suite for doing interoperability, that knows how to build various versions (there's already some prior art in t/perf). I know this isn't the first time this concept has come up. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Redirect git subcommand to itself?
Hi, so I just run into this problem again (which happens to me maybe twice a week): I want to do a git operations, so I type git into my shell, and then I look around what exactly I want to do and usually I find it in the help text of a previous command such as You are currently reverting commit 383c14b. (fix conflicts and run git revert --continue) (use git revert --abort to cancel the revert operation) then I copy the whole operation git revert --abort in this case and paste it to the shell and let go. The result looks like $ git git revert --abort git: 'git' is not a git command. See 'git --help'. Did you mean this? init I wonder if we want to make a git subcommand, which behaves exactly the same as git itself? Then git git git status would just return the same as git status. Thanks, Stefan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Redirect git subcommand to itself?
Stefan Beller sbel...@google.com writes: so I just run into this problem again (which happens to me maybe twice a week): I want to do a git operations, so I type git into my shell, and then I look around what exactly I want to do and usually I find it in the help text of a previous command such as You are currently reverting commit 383c14b. (fix conflicts and run git revert --continue) (use git revert --abort to cancel the revert operation) then I copy the whole operation git revert --abort in this case and paste it to the shell and let go. The result looks like $ git git revert --abort git: 'git' is not a git command. See 'git --help'. Did you mean this? init I wonder if we want to make a git subcommand, which behaves exactly the same as git itself? Then git git git status would just return the same as git status. A few unrelated thoughts. * Perhaps we should omit 'git' from these advice-texts? E.g. use revert --abort to cancel I dunno. * While we bend over backwards to a certain degree to be helpful, I somehow feel making git git a synonym to git is going too far, akin to asking POSIX maintainers to define act, cta, atc, tca, and tac all as synonyms to cat because you often fat-finger when typing cat (yes, tac does something else that is more useful, I know). * You can help yourself with something like this, I suppose: [alias] git = !sh -c 'exec git \$@\' - but I personally feel that it is too ugly to live as part of our official suggestion, so please do not send a patch to add it as a built-in alias ;-). -- 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] glossary: add remote, submodule, superproject
Noticed-by: Philip Oakley philipoak...@iee.org Helped-by: Junio C Hamano gits...@pobox.com Signed-off-by: Stefan Beller sbel...@google.com --- Documentation/glossary-content.txt | 17 + 1 file changed, 17 insertions(+) diff --git a/Documentation/glossary-content.txt b/Documentation/glossary-content.txt index bf383c2..23ab692 100644 --- a/Documentation/glossary-content.txt +++ b/Documentation/glossary-content.txt @@ -469,6 +469,11 @@ The most notable example is `HEAD`. def_push,push to describe the mapping between remote def_ref,ref and local ref. +[[def_remote]]remote repository:: + A def_repository,repository which is used to track the same + project but resides somewhere else. To communicate with remotes, + see def_fetch,fetch or def_push,push. + [[def_remote_tracking_branch]]remote-tracking branch:: A def_ref,ref that is used to follow changes from another def_repository,repository. It typically looks like @@ -515,6 +520,18 @@ The most notable example is `HEAD`. is created by giving the `--depth` option to linkgit:git-clone[1], and its history can be later deepened with linkgit:git-fetch[1]. +[[def_submodule]]submodule:: + A def_repository,repository that holds the history of a + separate project inside another repository (the latter of + which is called def_superproject, superproject). The + containing superproject knows about the names of (but does + not hold copies of) commit objects of the contained submodules. + +[[def_superproject]]superproject:: + A def_repository,repository that references other repositories + inside itself as def_submodule,submodules. The superproject + tracks only the remote and the name of the submodule. + [[def_symref]]symref:: Symbolic reference: instead of containing the def_SHA1,SHA-1 id itself, it is of the format 'ref: refs/some/thing' and when -- 2.4.1.345.gab207b6.dirty -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Redirect git subcommand to itself?
On Wed, May 27, 2015 at 06:53:26PM -0700, Junio C Hamano wrote: I wonder if we want to make a git subcommand, which behaves exactly the same as git itself? Then git git git status would just return the same as git status. A few unrelated thoughts. * Perhaps we should omit 'git' from these advice-texts? E.g. use revert --abort to cancel I dunno. Please don't. You help the set of people who type git and then paste the rest of the command at the expense of people who just paste the whole command. We don't know the relative numbers of those people, but we know there is at least 1 in each group. ;) -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] glossary: add remote and submodule
On Wed, May 27, 2015 at 3:29 PM, Junio C Hamano gits...@pobox.com wrote: Stefan Beller sbel...@google.com writes: Noticed-by: Philip Oakley philipoak...@iee.org Signed-off-by: Stefan Beller sbel...@google.com --- Documentation/glossary-content.txt | 10 ++ 1 file changed, 10 insertions(+) diff --git a/Documentation/glossary-content.txt b/Documentation/glossary-content.txt index bf383c2..e303135 100644 --- a/Documentation/glossary-content.txt +++ b/Documentation/glossary-content.txt @@ -469,6 +469,11 @@ The most notable example is `HEAD`. def_push,push to describe the mapping between remote def_ref,ref and local ref. +[[def_remote]]remote repository:: + A def_repository,repository which is used to track the same + project but resides somewhere else. To communicate with remotes, + see def_fetch,fetch or def_push,push. + OK. @@ -515,6 +520,11 @@ The most notable example is `HEAD`. is created by giving the `--depth` option to linkgit:git-clone[1], and its history can be later deepened with linkgit:git-fetch[1]. +[[def_submodule]]submodule:: + A def_repository,repository inside another repository. The two + repositories have different history, though the outer repository + knows the commit of the inner repository. I'd stress that they are not just different histories (as the 'master' and the 'maint' branches of my project has different histories) but they are separate projects. Perhaps like this? This is a very subtle distinction IMHO, as both master and maint are the same project. Looking from enough distance, it's just the git project without the fine detail of what makes these 2 histories different. I tried coming up with a short paragraph, which may explain my choice of words. But correctness trumps brevity indeed. A repository that holds the history of a separate project inside another repository (the latter of which is called superproject). This is better than what I proposed, but confusing. When naming a project a submodule, my mental standpoint is the superproject. (This project has the submodule foo and bar). But In your description the superproject is called another repository. The containing superproject knows about the names of (but does not hold copies of) commit objects of the contained submodules. That makes sense to point out here. Though should we also introduce superproject now? It is not like that it is strange or unintuitive that the superproject knows about some commits in its submodule. X, though Y however makes it sound as if Y is true despite X. I do not think there is any despite here. -- 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] glossary: add remote and submodule
Stefan Beller sbel...@google.com writes: +[[def_submodule]]submodule:: + A def_repository,repository inside another repository. The two + repositories have different history, though the outer repository + knows the commit of the inner repository. ... But correctness trumps brevity indeed. I do not think the correct way is that much longer, though. A repository inside another repository. The two repositories have different history A repository that holds the history of a separate project inside another repository Heh, they are the same length, no? A repository that holds the history of a separate project inside another repository (the latter of which is called superproject). This is better than what I proposed, but confusing. When naming a project a submodule, my mental standpoint is the superproject. (This project has the submodule foo and bar). But In your description the superproject is called another repository. That is because you are adding an entry for submodule to the glossary, no? I was writing from submodule's point of view, i.e. I (submodule) is inside another repository, and my project is separate from that other repository's. The containing superproject knows about the names of (but does not hold copies of) commit objects of the contained submodules. That makes sense to point out here. Though should we also introduce superproject now? Yes, that is what I was hinting at. -- 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] p4: Retrieve the right revision of the UTF-16 file
Fixing bug with UTF-16 files when they are retrieved by git-p4. It was always getting the tip version of the file and the history of the file was lost. Signed-off-by: Miguel Torroja miguel.torr...@gmail.com --- git-p4.py |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/git-p4.py b/git-p4.py index cdfa2df..be2c7da 100755 --- a/git-p4.py +++ b/git-p4.py @@ -2098,7 +2098,7 @@ class P4Sync(Command, P4UserMap): # them back too. This is not needed to the cygwin windows version, # just the native NT type. # -text = p4_read_pipe(['print', '-q', '-o', '-', file['depotFile']]) +text = p4_read_pipe(['print', '-q', '-o', '-', %s@%s % (file['depotFile'], file['change']) ]) if p4_version_string().find(/NT) = 0: text = text.replace(\r\n, \n) contents = [ text ] -- 1.7.10.4 -- 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] Documentation: include 'merge.branchdesc' for merge and config as well
'merge.branchdesc' is only mentioned in the docs of 'git fmt-merge-msg'. The description of 'merge.log' is already duplicated between 'merge-config.txt' and 'git-fmt-merge-msg.txt'; instead of duplicating the description of another config variable, extract the descriptions of both of these variables from 'git-fmt-merge-msg.txt' into a separate file and include it there and in 'merge-config.txt'. Leave 'merge.summary' only in git-fmt-merge-msg.txt, as it is marked for deprecation. Signed-off-by: SZEDER Gábor sze...@ira.uka.de --- Documentation/fmt-merge-msg-config.txt | 10 ++ Documentation/git-fmt-merge-msg.txt| 12 +--- Documentation/merge-config.txt | 6 +- 3 files changed, 12 insertions(+), 16 deletions(-) create mode 100644 Documentation/fmt-merge-msg-config.txt diff --git a/Documentation/fmt-merge-msg-config.txt b/Documentation/fmt-merge-msg-config.txt new file mode 100644 index 00..c73cfa90b7 --- /dev/null +++ b/Documentation/fmt-merge-msg-config.txt @@ -0,0 +1,10 @@ +merge.branchdesc:: + In addition to branch names, populate the log message with + the branch description text associated with them. Defaults + to false. + +merge.log:: + In addition to branch names, populate the log message with at + most the specified number of one-line descriptions from the + actual commits that are being merged. Defaults to false, and + true is a synonym for 20. diff --git a/Documentation/git-fmt-merge-msg.txt b/Documentation/git-fmt-merge-msg.txt index bb1232a52c..55a9a4b93a 100644 --- a/Documentation/git-fmt-merge-msg.txt +++ b/Documentation/git-fmt-merge-msg.txt @@ -51,17 +51,7 @@ OPTIONS CONFIGURATION - - -merge.branchdesc:: - In addition to branch names, populate the log message with - the branch description text associated with them. Defaults - to false. - -merge.log:: - In addition to branch names, populate the log message with at - most the specified number of one-line descriptions from the - actual commits that are being merged. Defaults to false, and - true is a synonym for 20. +include::fmt-merge-msg-config.txt[] merge.summary:: Synonym to `merge.log`; this is deprecated and will be removed in diff --git a/Documentation/merge-config.txt b/Documentation/merge-config.txt index 8a0e52f8ee..002ca58c21 100644 --- a/Documentation/merge-config.txt +++ b/Documentation/merge-config.txt @@ -26,11 +26,7 @@ merge.ff:: allowed (equivalent to giving the `--ff-only` option from the command line). -merge.log:: - In addition to branch names, populate the log message with at - most the specified number of one-line descriptions from the - actual commits that are being merged. Defaults to false, and - true is a synonym for 20. +include::fmt-merge-msg-config.txt[] merge.renameLimit:: The number of files to consider when performing rename detection -- 2.4.2.349.g6883b65 -- 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] p4: Retrieve the right revision of the UTF-16 file
On 27/05/15 23:31, Miguel Torroja wrote: Fixing bug with UTF-16 files when they are retreived by git-p4. It was always getting the tip version of the file and the history of the file was lost. This looks sensible to me, and seems to work in some simple testing, thanks! Ack. Luke --- git-p4.py |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/git-p4.py b/git-p4.py index cdfa2df..be2c7da 100755 --- a/git-p4.py +++ b/git-p4.py @@ -2098,7 +2098,7 @@ class P4Sync(Command, P4UserMap): # them back too. This is not needed to the cygwin windows version, # just the native NT type. # -text = p4_read_pipe(['print', '-q', '-o', '-', file['depotFile']]) +text = p4_read_pipe(['print', '-q', '-o', '-', %s@%s % (file['depotFile'], file['change']) ]) if p4_version_string().find(/NT) = 0: text = text.replace(\r\n, \n) contents = [ text ] -- 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
Release candidate of Git for Windows 2.x is out
Hi all, I just uploaded release candidates for the upcoming Git for Windows 2.x release. Please find the download link here: https://git-for-windows.github.io/#download There are 32-bit and 64-bit versions both of regular installers and portable installers (portable meaning that they are .7z archives that can be unpacked anywhere and run in place, without any need for running an installer). My projected time line is to hammer out the last kinks until Friday, and then continue after a one-week leave, if needed, and then finally retire msysGit and start the official 2.x release cycle of Git for Windows. If you are running Windows and have a little time to spare, please test this release candidate thoroughly. If you find bugs, please first look at https://github.com/git-for-windows/git/issues (even the closed ones), and comment either on existing tickets or open new ones. It would be even cooler, of course, if you could open Pull Requests with fixes :-) Ciao, Johannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Bug: .gitconfig folder
On Wed, May 27, 2015 at 01:30:29PM -0700, Junio C Hamano wrote: Jorge grif...@gmx.es writes: If you have a folder named ~/.gitconfig instead of a file with that name, when you try to run some global config editing command it will fail with a wrong error message: fatal: Out of memory? mmap failed: No such device That indeed is a funny error message. How about this patch? -- 8 -- We show that message with die_errno(), but the OS is ought to know why mmap(2) failed much better than we do. There is no reason for us to say Out of memory? here. Note that mmap(2) fails with ENODEV when the file you specify is not something that can be mmap'ed, so you still need to know that No such device can include cases like having a directory when a regular file is expected, but we can expect that a user who creates a directory to a location where a regular file is expected to be would know what s/he is doing, hopefully ;-) sha1_file.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sha1_file.c b/sha1_file.c index ccc6dac..551a9e9 100644 --- a/sha1_file.c +++ b/sha1_file.c @@ -720,7 +720,7 @@ void *xmmap(void *start, size_t length, release_pack_memory(length); ret = mmap(start, length, prot, flags, fd, offset); if (ret == MAP_FAILED) - die_errno(Out of memory? mmap failed); + die_errno(mmap failed); } This is definitely an improvement, but the real failing of that error message is that it does not tell us that ~/.gitconfig is the culprit. I don't think we can do much from xmmap, though; it does not have the filename. It would be nice if we got EISDIR from open() in the first place, but I don't think we can implement that efficiently (if we added an xopen that checked that, it would have to stat() every file we opened). -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Bug: .gitconfig folder
Jeff King p...@peff.net writes: -die_errno(Out of memory? mmap failed); +die_errno(mmap failed); This is definitely an improvement, but the real failing of that error message is that it does not tell us that ~/.gitconfig is the culprit. I don't think we can do much from xmmap, though; it does not have the filename. It would be nice if we got EISDIR from open() in the first place, but I don't think we can implement that efficiently (if we added an xopen that checked that, it would have to stat() every file we opened). The patch was meant to be a tongue-in-cheek tangent that is a vast improvement for cases where we absolutely need to use mmap but does not help the OP at all ;-) I do not think there is any need for the config reader to read the existing file via mmap interface; just open it, strbuf_read() the whole thing (and complain when it cannot) and we should be ok. Or do we write back through the mmaped region or something? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/WIP 2/8] wrapper: implement xfopen()
On Wed, May 27, 2015 at 09:33:32PM +0800, Paul Tan wrote:
+/**
+ * xfopen() is the same as fopen(), but it die()s if the fopen() fails.
+ */
+FILE *xfopen(const char *path, const char *mode)
+{
+ FILE *fp;
+
+ assert(path);
+ assert(mode);
+ fp = fopen(path, mode);
+ if (!fp) {
+ if (*mode == 'w' || *mode == 'a')
+ die_errno(_(could not open '%s' for writing), path);
This misses r+. I don't think we use that in our code currently, but
if we're going to introduce a wrapper like this, I think it makes sense
to cover all cases.
-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] p4: Retrieve the right revision of the UTF-16 file
On Wed, May 27, 2015 at 3:04 PM, Luke Diamand l...@diamand.org wrote: On 27/05/15 23:31, Miguel Torroja wrote: Fixing bug with UTF-16 files when they are retreived by git-p4. It was always getting the tip version of the file and the history of the file was lost. This looks sensible to me, and seems to work in some simple testing, thanks! Ack. Luke Thanks; Miguel, please sign-off your patch; otherwise we cannot use it. Thanks. --- git-p4.py |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/git-p4.py b/git-p4.py index cdfa2df..be2c7da 100755 --- a/git-p4.py +++ b/git-p4.py @@ -2098,7 +2098,7 @@ class P4Sync(Command, P4UserMap): # them back too. This is not needed to the cygwin windows version, # just the native NT type. # -text = p4_read_pipe(['print', '-q', '-o', '-', file['depotFile']]) +text = p4_read_pipe(['print', '-q', '-o', '-', %s@%s % (file['depotFile'], file['change']) ]) if p4_version_string().find(/NT) = 0: text = text.replace(\r\n, \n) contents = [ text ] -- 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/WIP 6/8] am: extract patch, message and authorship with git-mailinfo
Paul Tan pyoka...@gmail.com writes:
+static const char *msgnum(const struct am_state *state)
+{
+ static struct strbuf fmt = STRBUF_INIT;
+ static struct strbuf sb = STRBUF_INIT;
+
+ strbuf_reset(fmt);
+ strbuf_addf(fmt, %%0%dd, state-prec);
+
+ strbuf_reset(sb);
+ strbuf_addf(sb, fmt.buf, state-cur);
Hmph, wouldn't (%*d, state-prec, state-cur) work or am I missing
something?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/WIP 6/8] am: extract patch, message and authorship with git-mailinfo
On Wed, May 27, 2015 at 01:44:26PM -0700, Junio C Hamano wrote:
Paul Tan pyoka...@gmail.com writes:
@@ -17,6 +34,10 @@ struct am_state {
struct strbuf dir;/* state directory path */
int cur; /* current patch number */
int last; /* last patch number */
+ struct strbuf msg;/* commit message */
+ struct strbuf author_name;/* commit author's name */
+ struct strbuf author_email; /* commit author's email */
+ struct strbuf author_date;/* commit author's date */
int prec; /* number of digits in patch filename */
};
I always get suspicious when structure fields are overly commented,
wondering if it is a sign of naming fields poorly. All of the above
fields look quite self-explanatory and I am not sure if it is worth
having these comments, spending efforts to type SP many times to
align them and all.
Just my 2 cents, but I think that grouping and overhead comments can
often make things more obvious. For example:
struct am_state {
/* state directory path */
struct strbuf dir;
/*
* current and last patch numbers; are these 1-indexed
* or 0-indexed?
*/
int cur;
int last;
struct strbuf author_name;
struct strbuf author_email;
struct strbuf author_date;
struct strbuf msg;
/* number of digits in patch filename */
int prec;
}
Note that by grouping cur and last, we can discuss them as a group,
and the overhead comment gives us room to mention their shared
properties (my indexing question is a real question, btw, whose answer I
think would be useful to mention in a comment).
Likewise, by grouping the msg strbuf with the author information, it
becomes much more clear that they are all about the commit metadata
(though I would not be opposed to a comment above the whole block,
either).
-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] config: add options to list only variable names
SZEDER Gábor sze...@ira.uka.de writes:
diff --git a/builtin/config.c b/builtin/config.c
index 7188405..38bcf83 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -13,6 +13,7 @@ static char *key;
static regex_t *key_regexp;
static regex_t *regexp;
static int show_keys;
+static int show_only_keys;
Is it just me who thinks this is a strange phrase? Somehow these
words do not roll well on my tongue.
Perhaps static int omit_values? Which would match what this part
does pretty well:
static int show_all_config(const char *key_, const char *value_, void *cb)
{
- if (value_)
+ if (!show_only_keys value_)
That is, if not omitting values and there is a value, then do this.
- return format_config(values-items[values-nr++], key_, value_);
+ if (show_only_keys) {
+ struct strbuf *buf = values-items[values-nr++];
+ strbuf_init(buf, 0);
+ strbuf_addstr(buf, key_);
+ strbuf_addch(buf, term);
+ return 0;
xstrfmt()?
--
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
