Re: Problem with Integrated Vim Editor on Win 10
On do, 2016-03-31 at 06:33 +, Peter Olsson wrote: > I'm not sure where we should report this? While the git-for-windows maintainer does read this list, git-for -windows specific issues should be reported in its issue tracker on GitHub. It looks like this issue has been reported already: https://github.com/git-for-windows/git/issues/711 -- Dennis Kaarsemaker http://www.kaarsemaker.net -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Problem with Integrated Vim Editor on Win 10
Zachary Turner google.com> writes: > > Hi, just recently I installed the latest build of Windows 10 of my > machine. This is my second Win10 machine. On the other I am using git > 2.7.0.windows.1 and everything is working just fine. > > On the second machine I am using git 2.8.0.windows.1 and vim does not > work. I sent a bug report to bugs vim.org, but frankly I don't know whose > bug this is, so I'm including it here as well. > > The problem is that vim is just a black screen when git launches it. If I > mash enough keys eventually I see something that resembles vim output at > the bottom, but I can't actually use it. > > I tried going into program files\git\usr\bin and just running vim.exe. > Again, black screen. If I press enter about 10 times I can see the > introduction screen. Then if I press : about 10-20 times it will go into > command mode and a single : appears. after pressing a few more keys all > the rest of the :s appear. Basically, everything is completely unusable. > > I tried downloading vim 7.4 from www.vim.org, and low and behold, it > works. For now I've replaced the copy of vim.exe that ships with git with > the copy from www.vim.org. But this leaves me nervous that something is > seriously wrong. > > Has anyone seen anything like this before, or have any ideas how I might > better diagnose this? > Same problem here, but on Windows 7. I'm pretty sure that this was introduced in version 2.8.0 for Windows. Before upgrading, I used the latest 2.7 version and it worked as expected. I'm not sure where we should report this? /Peter -- 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: Problem with Integrated Vim Editor on Win 10
Zachary Turner google.com> writes: > Has anyone seen anything like this before, or have any ideas how I might > better diagnose this? Not before, but I can confirm this issue on Win10, and reportedly on Win7 as well. Actually I thought this was a Git for Windows specific issue, so I reported it on GitHub; it didn't occur to me that it could be a VIM issue. So I guess you have already diagnosed this better than I did... -- 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 00/21] replacement for dt/refs-backend-lmdb v7 patch 04/33
On Wed, 2016-03-30 at 08:37 +0200, Michael Haggerty wrote: > On 03/29/2016 10:12 PM, David Turner wrote: > > On Sun, 2016-03-27 at 07:22 +0200, Michael Haggerty wrote: > > > On 03/24/2016 07:47 AM, David Turner wrote: > > > > [...] > > > > I incorporated your changes into the lmdb backend. To make > > > > merging > > > > later more convenient, I rebased on top of pu -- I think this > > > > mainly > > > > depends on jk/check-repository-format, but I also included some > > > > fixes > > > > for a couple of tests that had been changed by other patches. > > > > > > I think rebasing changes on top of pu is counterproductive. I > > > believe > > > that Junio had extra work rebasing your earlier series onto a > > > merge > > > of > > > the minimum number of topics that it really depended on. There is > > > no > > > way > > > that he could merge the branch in this form because it would > > > imply > > > merging all of pu. > > > > > > See the zeroth section of SubmittingPatches [1] for the > > > guidelines. > > > > I'm a bit confused because > > [PATCH 18/21] get_default_remote(): remove unneeded flag variable > > > > doesn't do anything on master -- it depends on some patch in pu. > > And > > we definitely want to pick up jk/check-repository-format (which > > doesn't > > include whatever 18/21 depends on). > > > > So what do you think our base should be? > > I think the preference is to base a patch series on the merge of > master > plus the minimum number of topics in pu (ideally, none) that are > "essential" prerequisites of the changes in the patch series. For > example, the version of this patch series that Junio has in his tree > was > based on master + sb/submodule-parallel-update. > > Even if there are minor > conflicts with another in-flight topic, it is easier for Junio to > resolve the conflicts when merging the topics together than to rebase > the patch series over and over as the other patch series evolves. The > goal of this practice is of course to allow patch series to evolve > independently of each other as much as possible. > > Of course if you have insights into nontrivial conflicts between your > patch series and others, it would be helpful to discuss these in your > cover letter. If I am reading this correctly, it looks like your series also has a few more sb submodule patches, e.g. sb/submodule-init, which is responsible for the code that 18/21 depends on. I think jk/check-repository-format is also good to get in first, because it changes the startup sequence a bit and it's a bit tricky to figure out what needs to change in dt/refs-backend-lmdb as a result of it. But I can't just merge jk/check-repository-format on top of 71defe0047 -- some function signatures have changed in the run-command stuff and it seems kind of annoying to fix up. So I propose instead that we just drop 18/21 for now, and use just jk/check-repository-format as the base. Does this seem reasonable to you? -- 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: [PATCHv2 1/4] notes: don't leak memory in git_config_get_notes_strategy
Jeff King writes: > On Wed, Mar 30, 2016 at 02:07:14PM -0700, Junio C Hamano wrote: > >> This is a tangent, but am I the only one who finds that the naming >> of functions in config-get API is confusing? Just wondering if we >> should rename the ones that keeps the memory ownership to the config >> subsystem with s/get/peek/ or something. > > You are definitely not the only one. > > I think the get/peek thing would help, but I also get confused between > git_config_string() and git_config_get_string(). I actually do not have problems with the ones with names that do not have "get" in. git_config_$type() is always a helper function that are designed to be called by config callback to parse (and complain) a pair that is expected to be of $type, and git_$family_config() is always a helper function that are designed to be used as the callback of git_config() to handle the configuration related to the named command family. -- 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] patch-ids: make commit_patch_id() a public helper function
Make commit_patch_id() available to other builtins.
Helped-by: Junio C Hamano
Signed-off-by: Xiaolong Ye
---
patch-ids.c | 2 +-
patch-ids.h | 2 ++
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/patch-ids.c b/patch-ids.c
index b7b3e5a..a4d0016 100644
--- a/patch-ids.c
+++ b/patch-ids.c
@@ -4,7 +4,7 @@
#include "sha1-lookup.h"
#include "patch-ids.h"
-static int commit_patch_id(struct commit *commit, struct diff_options *options,
+int commit_patch_id(struct commit *commit, struct diff_options *options,
unsigned char *sha1)
{
if (commit->parents)
diff --git a/patch-ids.h b/patch-ids.h
index c8c7ca1..eeb56b3 100644
--- a/patch-ids.h
+++ b/patch-ids.h
@@ -13,6 +13,8 @@ struct patch_ids {
struct patch_id_bucket *patches;
};
+int commit_patch_id(struct commit *commit, struct diff_options *options,
+ unsigned char *sha1);
int init_patch_ids(struct patch_ids *);
int free_patch_ids(struct patch_ids *);
struct patch_id *add_commit_patch_id(struct commit *, struct patch_ids *);
--
2.8.0.4.gcb5a9af
--
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 4/4] format-patch: introduce format.base configuration
We can set format.base=auto to record the base commit info automatically,
it is equivalent to set --base=auto in cmdline.
The format.base has lower priority than command line option, so if user
set format.base=auto and pass the command line option in the meantime,
base_commit will be the one passed to command line option.
Signed-off-by: Xiaolong Ye
---
Documentation/git-format-patch.txt | 2 ++
builtin/log.c | 21 ++---
2 files changed, 16 insertions(+), 7 deletions(-)
diff --git a/Documentation/git-format-patch.txt
b/Documentation/git-format-patch.txt
index d8fe651..10149ab 100644
--- a/Documentation/git-format-patch.txt
+++ b/Documentation/git-format-patch.txt
@@ -293,6 +293,8 @@ you can use `--suffix=-patch` to get
`0001-description-of-my-change-patch`.
If set '--base=auto' in cmdline, it will track base commit
automatically,
the base commit will be the merge base of tip commit of the
remote-tracking
branch and revision-range specified in cmdline.
+ If 'format.base=auto' is set in configuration file, it is equivalent
+ to set '--base=auto' in cmdline.
--root::
Treat the revision argument as a , even if it
diff --git a/builtin/log.c b/builtin/log.c
index c5efe73..821a778 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -699,6 +699,7 @@ static int do_signoff;
static const char *signature = git_version_string;
static const char *signature_file;
static int config_cover_letter;
+static int config_base_commit;
static const char *config_output_directory;
enum {
@@ -780,6 +781,12 @@ static int git_format_config(const char *var, const char
*value, void *cb)
}
if (!strcmp(var, "format.outputdirectory"))
return git_config_string(&config_output_directory, var, value);
+ if (!strcmp(var, "format.base")){
+ if (value && !strcasecmp(value, "auto")) {
+ config_base_commit = 1;
+ return 0;
+ }
+ }
return git_log_config(var, value, cb);
}
@@ -1210,7 +1217,12 @@ static void prepare_bases(struct base_tree_info *bases,
DIFF_OPT_SET(&diffopt, RECURSIVE);
diff_setup_done(&diffopt);
- if (!strcmp(base_commit, "auto")) {
+ if (base_commit && strcmp(base_commit, "auto")) {
+ base = lookup_commit_reference_by_name(base_commit);
+ if (!base)
+ die(_("Unknown commit %s"), base_commit);
+ oidcpy(&bases->base_commit, &base->object.oid);
+ } else if ((base_commit && !strcmp(base_commit, "auto")) ||
config_base_commit) {
curr_branch = branch_get(NULL);
upstream = branch_get_upstream(curr_branch, NULL);
if (upstream) {
@@ -1228,11 +1240,6 @@ static void prepare_bases(struct base_tree_info *bases,
"please use git branch --set-upstream-to to track
a remote branch.\n"
"Or you could specify base commit by
--base= manually."));
}
- } else {
- base = lookup_commit_reference_by_name(base_commit);
- if (!base)
- die(_("Unknown commit %s"), base_commit);
- oidcpy(&bases->base_commit, &base->object.oid);
}
init_revisions(&revs, NULL);
@@ -1611,7 +1618,7 @@ int cmd_format_patch(int argc, const char **argv, const
char *prefix)
signature = strbuf_detach(&buf, NULL);
}
- if (base_commit) {
+ if (base_commit || config_base_commit) {
memset(&bases, 0, sizeof(bases));
reset_revision_walk();
prepare_bases(&bases, base_commit, list, nr);
--
2.8.0.4.gcb5a9af
--
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] format-patch: introduce --base=auto option
Introduce --base=auto to record the base commit info automatically, the
base_commit
will be the merge base of tip commit of the upstream branch and revision-range
specified in cmdline.
Helped-by: Junio C Hamano
Helped-by: Wu Fengguang
Signed-off-by: Xiaolong Ye
---
Documentation/git-format-patch.txt | 4
builtin/log.c | 31 +++
2 files changed, 31 insertions(+), 4 deletions(-)
diff --git a/Documentation/git-format-patch.txt
b/Documentation/git-format-patch.txt
index 067d562..d8fe651 100644
--- a/Documentation/git-format-patch.txt
+++ b/Documentation/git-format-patch.txt
@@ -290,6 +290,10 @@ you can use `--suffix=-patch` to get
`0001-description-of-my-change-patch`.
patches for A, B and C, and the identifiers for P, X, Y, Z are appended
at the end of the _first_ message.
+ If set '--base=auto' in cmdline, it will track base commit
automatically,
+ the base commit will be the merge base of tip commit of the
remote-tracking
+ branch and revision-range specified in cmdline.
+
--root::
Treat the revision argument as a , even if it
is just a single commit (that would normally be treated as a
diff --git a/builtin/log.c b/builtin/log.c
index 03cbab0..c5efe73 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1200,6 +1200,9 @@ static void prepare_bases(struct base_tree_info *bases,
struct rev_info revs;
struct diff_options diffopt;
struct object_id *patch_id;
+ struct branch *curr_branch;
+ struct commit_list *base_list;
+ const char *upstream;
unsigned char sha1[20];
int i;
@@ -1207,10 +1210,30 @@ static void prepare_bases(struct base_tree_info *bases,
DIFF_OPT_SET(&diffopt, RECURSIVE);
diff_setup_done(&diffopt);
- base = lookup_commit_reference_by_name(base_commit);
- if (!base)
- die(_("Unknown commit %s"), base_commit);
- oidcpy(&bases->base_commit, &base->object.oid);
+ if (!strcmp(base_commit, "auto")) {
+ curr_branch = branch_get(NULL);
+ upstream = branch_get_upstream(curr_branch, NULL);
+ if (upstream) {
+ if (get_sha1(upstream, sha1))
+ die(_("Failed to resolve '%s' as a valid
ref."), upstream);
+ commit = lookup_commit_or_die(sha1, "upstream base");
+ base_list = get_merge_bases_many(commit, total, list);
+ if (!bases)
+ die(_("Could not find merge base."));
+ base = base_list->item;
+ free_commit_list(base_list);
+ oidcpy(&bases->base_commit, &base->object.oid);
+ } else {
+ die(_("Failed to get upstream, if you want to record
base commit automatically,\n"
+ "please use git branch --set-upstream-to to track
a remote branch.\n"
+ "Or you could specify base commit by
--base= manually."));
+ }
+ } else {
+ base = lookup_commit_reference_by_name(base_commit);
+ if (!base)
+ die(_("Unknown commit %s"), base_commit);
+ oidcpy(&bases->base_commit, &base->object.oid);
+ }
init_revisions(&revs, NULL);
revs.max_parents = 1;
--
2.8.0.4.gcb5a9af
--
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] Add an option to git-format-patch to record base tree info
V3 mainly improves the implementation according to Junio's comments, Changes vs v2 include: - Remove the unnecessary output line "** base-commit-info **". - Improve the traverse logic to handle not only linear topology, but more general cases, it will start revision walk by setting the starting points of the traversal to all elements in the rev list[], and skip the ones in list[], only grab the patch-ids of prerequisite patches. - If --base=auto is set, it will get merge base of upstream and rev range we specified and use it as base commit. If there is no upstream, we just error out and suggest to use set-upstream-to to track a remote branch as upstream. v1 can be found here: http://article.gmane.org/gmane.comp.version-control.git/286873 v2 can be found here: http://article.gmane.org/gmane.comp.version-control.git/289603 Xiaolong Ye (4): patch-ids: make commit_patch_id() a public helper function format-patch: add '--base' option to record base tree info format-patch: introduce --base=auto option format-patch: introduce format.base configuration Documentation/git-format-patch.txt | 31 ++ builtin/log.c | 119 + patch-ids.c| 2 +- patch-ids.h| 2 + 4 files changed, 153 insertions(+), 1 deletion(-) -- 2.8.0.4.gcb5a9af base-commit: 90f7b16b3adc78d4bbabbd426fb69aa78c714f71 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 2/4] format-patch: add '--base' option to record base tree info
Maintainers or third party testers may want to know the exact base tree
the patch series applies to. Teach git format-patch a '--base' option to
record the base tree info and append this information at the end of the
_first_ message (either the cover letter or the first patch in the series).
Helped-by: Junio C Hamano
Helped-by: Wu Fengguang
Signed-off-by: Xiaolong Ye
---
Documentation/git-format-patch.txt | 25 +++
builtin/log.c | 89 ++
2 files changed, 114 insertions(+)
diff --git a/Documentation/git-format-patch.txt
b/Documentation/git-format-patch.txt
index 6821441..067d562 100644
--- a/Documentation/git-format-patch.txt
+++ b/Documentation/git-format-patch.txt
@@ -265,6 +265,31 @@ you can use `--suffix=-patch` to get
`0001-description-of-my-change-patch`.
Output an all-zero hash in each patch's From header instead
of the hash of the commit.
+--base=::
+ Record the base tree information to identify the whole tree
+ the patch series applies to. For example, the patch submitter
+ has a commit history of this shape:
+
+ ---P---X---Y---Z---A---B---C
+
+ where "P" is the well-known public commit (e.g. one in Linus's tree),
+ "X", "Y", "Z" are prerequisite patches in flight, and "A", "B", "C"
+ are the work being sent out, the submitter could say "git format-patch
+ --base=P -3 C" (or variants thereof, e.g. with "--cover" or using
+ "Z..C" instead of "-3 C" to specify the range), and the identifiers
+ for P, X, Y, Z are appended at the end of the _first_ message (either
+ the cover letter or the first patch in the series).
+
+ For non-linear topology, such as
+
+ ---P---X---A---M---C
+ \ /
+Y---Z---B
+
+ the submitter could also use "git format-patch --base=P -3 C" to
generate
+ patches for A, B and C, and the identifiers for P, X, Y, Z are appended
+ at the end of the _first_ message.
+
--root::
Treat the revision argument as a , even if it
is just a single commit (that would normally be treated as a
diff --git a/builtin/log.c b/builtin/log.c
index 0d738d6..03cbab0 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1185,6 +1185,82 @@ static int from_callback(const struct option *opt, const
char *arg, int unset)
return 0;
}
+struct base_tree_info {
+ struct object_id base_commit;
+ int nr_patch_id, alloc_patch_id;
+ struct object_id *patch_id;
+};
+
+static void prepare_bases(struct base_tree_info *bases,
+ const char *base_commit,
+ struct commit **list,
+ int total)
+{
+ struct commit *base = NULL, *commit;
+ struct rev_info revs;
+ struct diff_options diffopt;
+ struct object_id *patch_id;
+ unsigned char sha1[20];
+ int i;
+
+ diff_setup(&diffopt);
+ DIFF_OPT_SET(&diffopt, RECURSIVE);
+ diff_setup_done(&diffopt);
+
+ base = lookup_commit_reference_by_name(base_commit);
+ if (!base)
+ die(_("Unknown commit %s"), base_commit);
+ oidcpy(&bases->base_commit, &base->object.oid);
+
+ init_revisions(&revs, NULL);
+ revs.max_parents = 1;
+ base->object.flags |= UNINTERESTING;
+ add_pending_object(&revs, &base->object, "base");
+ for (i = 0; i < total; i++) {
+ list[i]->object.flags |= 0;
+ add_pending_object(&revs, &list[i]->object, "rev_list");
+ list[i]->util = (void *)1;
+ }
+
+ if (prepare_revision_walk(&revs))
+ die(_("revision walk setup failed"));
+ /*
+* Traverse the prerequisite commits list,
+* get the patch ids and stuff them in bases structure.
+*/
+ while ((commit = get_revision(&revs)) != NULL) {
+ if (commit->util)
+ continue;
+ if (commit_patch_id(commit, &diffopt, sha1))
+ die(_("cannot get patch id"));
+ ALLOC_GROW(bases->patch_id, bases->nr_patch_id + 1,
bases->alloc_patch_id);
+ patch_id = bases->patch_id + bases->nr_patch_id;
+ hashcpy(patch_id->hash, sha1);
+ bases->nr_patch_id++;
+ }
+}
+
+static void print_bases(struct base_tree_info *bases)
+{
+ int i;
+
+ /* Only do this once, either for the cover or for the first one */
+ if (is_null_oid(&bases->base_commit))
+ return;
+
+ /* Show the base commit */
+ printf("base-commit: %s\n", oid_to_hex(&bases->base_commit));
+
+ /* Show the prerequisite patches */
+ for (i = 0; i < bases->nr_patch_id; i++)
+ printf("prerequisite-patch-id: %s\n",
oid_to_hex(&bases->patch_id[i]));
+
+ free(bases->patch_id);
+ bases->nr_patch_id = 0;
+ bases->alloc_patch_id = 0;
+ oidclr(&bases
Re: [PATCHv2 1/4] notes: don't leak memory in git_config_get_notes_strategy
On Wed, Mar 30, 2016 at 02:07:14PM -0700, Junio C Hamano wrote: > This is a tangent, but am I the only one who finds that the naming > of functions in config-get API is confusing? Just wondering if we > should rename the ones that keeps the memory ownership to the config > subsystem with s/get/peek/ or something. You are definitely not the only one. I think the get/peek thing would help, but I also get confused between git_config_string() and git_config_get_string(). -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 2/4] submodule--helper clone: simplify path check
On Wed, Mar 30, 2016 at 5:17 PM, Stefan Beller wrote: > The calling shell code makes sure that `path` is non null and non empty. > (side note: it cannot be null as just three lines before it is passed > to safe_create_leading_directories_const which would crash as you feed > it null). > So we're going to assume that all callers of submodule--helper will provide a non-null non-empty path? Hmm, since we expect only our shell code to really do this... that's probably ok I think. I don't think it can do any real harm should someone outside git call it with a bad path. Thanks, Jake -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/4] submodule--helper: use relative path if possible
As submodules have working directory and their git directory far apart
relative_path(gitdir, work_dir) will not produce a relative path when
git_dir is absolute. When the gitdir is absolute, we need to convert
the workdir to an absolute path as well to compute the relative path.
(e.g. gitdir=/home/user/project/.git/modules/submodule,
workdir=submodule/, relative_dir doesn't take the current working directory
into account, so there is no way for it to know that the correct answer
is indeed "../.git/modules/submodule", if the workdir was given as
/home/user/project/submodule, the answer is obvious.)
Signed-off-by: Stefan Beller
---
builtin/submodule--helper.c | 7 +++
t/t7400-submodule-basic.sh | 2 +-
2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 914e561..0b0fad3 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -159,6 +159,7 @@ static int module_clone(int argc, const char **argv, const
char *prefix)
FILE *submodule_dot_git;
char *sm_gitdir, *cwd, *p;
struct strbuf rel_path = STRBUF_INIT;
+ struct strbuf abs_path = STRBUF_INIT;
struct strbuf sb = STRBUF_INIT;
struct option module_clone_options[] = {
@@ -219,7 +220,12 @@ static int module_clone(int argc, const char **argv, const
char *prefix)
if (!submodule_dot_git)
die_errno(_("cannot open file '%s'"), sb.buf);
+ strbuf_addf(&abs_path, "%s/%s",
+ get_git_work_tree(),
+ path);
fprintf(submodule_dot_git, "gitdir: %s\n",
+ is_absolute_path(sm_gitdir) ?
+ relative_path(sm_gitdir, abs_path.buf, &rel_path) :
relative_path(sm_gitdir, path, &rel_path));
if (fclose(submodule_dot_git))
die(_("could not close file %s"), sb.buf);
@@ -242,6 +248,7 @@ static int module_clone(int argc, const char **argv, const
char *prefix)
relative_path(sb.buf, sm_gitdir, &rel_path));
strbuf_release(&sb);
strbuf_release(&rel_path);
+ strbuf_release(&abs_path);
free(sm_gitdir);
free(cwd);
free(p);
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index fc11809..ea3fabb 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -818,7 +818,7 @@ test_expect_success 'submodule add --name allows to replace
a submodule with ano
)
'
-test_expect_failure 'recursive relative submodules stay relative' '
+test_expect_success 'recursive relative submodules stay relative' '
test_when_finished "rm -rf super clone2 subsub sub3" &&
mkdir subsub &&
(
--
2.5.0.264.g4004fdc.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
[PATCH 3/4] submodule--helper clone: remove double path checking
Just a few lines after the deleted code we call
safe_create_leading_directories_const(path + "/.git")
so the check is done twice without action in between.
Remove the first check.
Signed-off-by: Stefan Beller
---
builtin/submodule--helper.c | 4
1 file changed, 4 deletions(-)
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 88002ca..914e561 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -212,11 +212,7 @@ static int module_clone(int argc, const char **argv, const
char *prefix)
}
/* Write a .git file in the submodule to redirect to the superproject.
*/
- if (safe_create_leading_directories_const(path) < 0)
- die(_("could not create directory '%s'"), path);
-
strbuf_addf(&sb, "%s/.git", path);
-
if (safe_create_leading_directories_const(sb.buf) < 0)
die(_("could not create leading directories of '%s'"), sb.buf);
submodule_dot_git = fopen(sb.buf, "w");
--
2.5.0.264.g4004fdc.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
[PATCH 2/4] submodule--helper clone: simplify path check
The calling shell code makes sure that `path` is non null and non empty.
(side note: it cannot be null as just three lines before it is passed
to safe_create_leading_directories_const which would crash as you feed
it null).
Signed-off-by: Stefan Beller
---
builtin/submodule--helper.c | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index f4c3eff..88002ca 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -215,10 +215,7 @@ static int module_clone(int argc, const char **argv, const
char *prefix)
if (safe_create_leading_directories_const(path) < 0)
die(_("could not create directory '%s'"), path);
- if (path && *path)
- strbuf_addf(&sb, "%s/.git", path);
- else
- strbuf_addstr(&sb, ".git");
+ strbuf_addf(&sb, "%s/.git", path);
if (safe_create_leading_directories_const(sb.buf) < 0)
die(_("could not create leading directories of '%s'"), sb.buf);
--
2.5.0.264.g4004fdc.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
[PATCH 1/4] recursive submodules: test for relative paths
This was reported as a regression at $gmane/290280. The root cause for that bug is in using recursive submodules as their relative path handling seems to be broken in ee8838d (2015-09-08, submodule: rewrite `module_clone` shell function in C). Signed-off-by: Stefan Beller --- t/t7400-submodule-basic.sh | 41 + 1 file changed, 41 insertions(+) diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh index 540771c..fc11809 100755 --- a/t/t7400-submodule-basic.sh +++ b/t/t7400-submodule-basic.sh @@ -818,6 +818,47 @@ test_expect_success 'submodule add --name allows to replace a submodule with ano ) ' +test_expect_failure 'recursive relative submodules stay relative' ' + test_when_finished "rm -rf super clone2 subsub sub3" && + mkdir subsub && + ( + cd subsub && + git init && + >t && + git add t && + git commit -m "initial commit" + ) && + mkdir sub3 && + ( + cd sub3 && + git init && + >t && + git add t && + git commit -m "initial commit" && + git submodule add ../subsub dirdir/subsub && + git commit -m "add submodule subsub" + ) && + mkdir super && + ( + cd super && + git init && + >t && + git add t && + git commit -m "initial commit" && + git submodule add ../sub3 && + git commit -m "add submodule sub" + ) && + git clone super clone2 && + ( + cd clone2 && + git submodule update --init --recursive && + echo "gitdir: ../.git/modules/sub3" >./sub3/.git_expect && + echo "gitdir: ../../../.git/modules/sub3/modules/dirdir/subsub" >./sub3/dirdir/subsub/.git_expect + ) && + test_cmp clone2/sub3/.git_expect clone2/sub3/.git && + test_cmp clone2/sub3/dirdir/subsub/.git_expect clone2/sub3/dirdir/subsub/.git +' + test_expect_success 'submodule add with an existing name fails unless forced' ' ( cd addtest2 && -- 2.5.0.264.g4004fdc.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
[PATCH 0/4] Fix relative path issues in recursive submodules.
It took me longer than expected to fix this bug. It comes with a test and minor refactoring and applies on top of origin/sb/submodule-helper, such that it can be merged into 2.7, 2.8 as well as master. Patch 1 is a test which fails; it has a similar layout as the real failing repository Norio Nomura pointed out, but simplified to the bare essentials for reproduction of the bug. Patch 2&3 are not strictly necessary for fixing the isseu, but it removes stupid code I wrote, so the resulting code looks a little better. Patch 4 fixes the issue by giving more information to relative_path, such that a relative path can be found in all cases. Thanks, Stefan Stefan Beller (4): recursive submodules: test for relative paths submodule--helper clone: simplify path check submodule--helper clone: remove double path checking submodule--helper: use relative path if possible builtin/submodule--helper.c | 16 t/t7400-submodule-basic.sh | 41 + 2 files changed, 49 insertions(+), 8 deletions(-) -- 2.5.0.264.g4004fdc.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
Problem with Integrated Vim Editor on Win 10
Hi, just recently I installed the latest build of Windows 10 of my machine. This is my second Win10 machine. On the other I am using git 2.7.0.windows.1 and everything is working just fine. On the second machine I am using git 2.8.0.windows.1 and vim does not work. I sent a bug report to b...@vim.org, but frankly I don't know whose bug this is, so I'm including it here as well. The problem is that vim is just a black screen when git launches it. If I mash enough keys eventually I see something that resembles vim output at the bottom, but I can't actually use it. I tried going into program files\git\usr\bin and just running vim.exe. Again, black screen. If I press enter about 10 times I can see the introduction screen. Then if I press : about 10-20 times it will go into command mode and a single : appears. after pressing a few more keys all the rest of the :s appear. Basically, everything is completely unusable. I tried downloading vim 7.4 from www.vim.org, and low and behold, it works. For now I've replaced the copy of vim.exe that ships with git with the copy from www.vim.org. But this leaves me nervous that something is seriously wrong. Has anyone seen anything like this before, or have any ideas how I might better diagnose this? -- 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] ident: make the useConfigOnly error messages more informative
On Wed, Mar 30, 2016 at 03:27:05PM -0700, Junio C Hamano wrote:
Marios Titas writes:
- && !(ident_config_given & IDENT_NAME_GIVEN))
- die("user.useConfigOnly set but no name given");
+ && !(ident_config_given & IDENT_NAME_GIVEN)) {
+ fputs(env_hint, stderr);
+ die("no name was given and auto-detection is
disabled
Hmph. I do not think that this is making the message "more
informative".
When a user hits this error, the old message allowed the user to
easily see how to toggle the "disable auto-detection" bit off to let
the code continue by telling the name of the configuration, but the
updated message hides that name, making it harder for the user to
disable the disabling of auto-detection.
I can buy the argument that this change helps the user by making the
message "less" informative, though. By discouraging the users from
toggling the user.useConfigOnly bit off, it indirectly makes the
other option to work around this error condition, i.e. giving a name
more explicitly, more appetizing.
Yeah, maybe informative is not the right word. What I meant is that it
directs the user to do the "git config user.name" thing, which is likely
the most appropriate course of action in this situation. In any event, I
think printing the env_hint message would be really helpful in this
case.
--
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] ident: make the useConfigOnly error messages more informative
Marios Titas writes:
> - && !(ident_config_given & IDENT_NAME_GIVEN))
> - die("user.useConfigOnly set but no name given");
> + && !(ident_config_given & IDENT_NAME_GIVEN)) {
> + fputs(env_hint, stderr);
> + die("no name was given and auto-detection is
> disabled
Hmph. I do not think that this is making the message "more
informative".
When a user hits this error, the old message allowed the user to
easily see how to toggle the "disable auto-detection" bit off to let
the code continue by telling the name of the configuration, but the
updated message hides that name, making it harder for the user to
disable the disabling of auto-detection.
I can buy the argument that this change helps the user by making the
message "less" informative, though. By discouraging the users from
toggling the user.useConfigOnly bit off, it indirectly makes the
other option to work around this error condition, i.e. giving a name
more explicitly, more appetizing.
--
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 00/16] port branch.c to use ref-filter's printing options
Karthik Nayak writes: > I kinda waited before sending this, since there was lot of discussions > happening regarding to GSOC16, didn't want to clutter the mailing list. > > This is part of unification of the commands 'git tag -l, git branch -l > and git for-each-ref'. This ports over branch.c to use ref-filter's > printing options. Overall this was a very pleasant read (I had a few comments but nothing that would make the whole idea of the topic invalidated). Thanks. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 09/16] ref-filter: make "%(symref)" atom work with the ':short' modifier
Karthik Nayak writes:
> The "%(symref)" atom doesn't work when used with the ':short' modifier
> because we strictly match only 'symref' for setting the 'need_symref'
> indicator. Fix this by using comparing with valid_atom rather than used_atom.
>
> Add tests for %(symref) and %(symref:short) while we're here.
>
> Helped-by: Junio C Hamano
> Signed-off-by: Karthik Nayak
> ---
> ref-filter.c| 2 +-
> t/t6300-for-each-ref.sh | 24
> 2 files changed, 25 insertions(+), 1 deletion(-)
>
> diff --git a/ref-filter.c b/ref-filter.c
> index 8c97cdb..5c59b17 100644
> --- a/ref-filter.c
> +++ b/ref-filter.c
> @@ -338,7 +338,7 @@ int parse_ref_filter_atom(const char *atom, const char
> *ep)
> valid_atom[i].parser(&used_atom[at], arg);
> if (*atom == '*')
> need_tagged = 1;
> - if (!strcmp(used_atom[at].name, "symref"))
> + if (!strcmp(valid_atom[i].name, "symref"))
> need_symref = 1;
> return at;
> }
Makes perfect sense.
> diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
> index 2c5f177..b06ea1c 100755
> --- a/t/t6300-for-each-ref.sh
> +++ b/t/t6300-for-each-ref.sh
> @@ -38,6 +38,7 @@ test_atom() {
> case "$1" in
> head) ref=refs/heads/master ;;
>tag) ref=refs/tags/testtag ;;
> + sym) ref=refs/heads/sym ;;
> *) ref=$1 ;;
> esac
An earlier review may have missed this, but I just noticed that the
body of this case/esac is over-indented. Something we can fix later
after the dust settles.
> printf '%s\n' "$3" >expected
> @@ -565,4 +566,27 @@ test_expect_success 'Verify sort with multiple keys' '
> refs/tags/bogo refs/tags/master > actual &&
> test_cmp expected actual
> '
> +
> +test_expect_success 'Add symbolic ref for the following tests' '
> + git symbolic-ref refs/heads/sym refs/heads/master
> +'
> +
> +cat >expected < +refs/heads/master
> +EOF
> +
> +test_expect_success 'Verify usage of %(symref) atom' '
> + git for-each-ref --format="%(symref)" refs/heads/sym > actual &&
> + test_cmp expected actual
> +'
> +
> +cat >expected < +heads/master
> +EOF
> +
> +test_expect_success 'Verify usage of %(symref:short) atom' '
> + git for-each-ref --format="%(symref:short)" refs/heads/sym > actual &&
> + test_cmp expected actual
> +'
> +
> test_done
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 05/16] ref-filter: move get_head_description() from branch.c
Karthik Nayak writes:
> -static char *get_head_description(void)
> -{
> - struct strbuf desc = STRBUF_INIT;
> - struct wt_status_state state;
> - memset(&state, 0, sizeof(state));
> - wt_status_get_state(&state, 1);
> - if (state.rebase_in_progress ||
> - state.rebase_interactive_in_progress)
> - strbuf_addf(&desc, _("(no branch, rebasing %s)"),
> - state.branch);
> - else if (state.bisect_in_progress)
> - strbuf_addf(&desc, _("(no branch, bisect started on %s)"),
> - state.branch);
> - else if (state.detached_from) {
> - /* TRANSLATORS: make sure these match _("HEAD detached at ")
> -and _("HEAD detached from ") in wt-status.c */
> - if (state.detached_at)
> - strbuf_addf(&desc, _("(HEAD detached at %s)"),
> - state.detached_from);
> - else
> - strbuf_addf(&desc, _("(HEAD detached from %s)"),
> - state.detached_from);
> - }
> - else
> - strbuf_addstr(&desc, _("(no branch)"));
> - free(state.branch);
> - free(state.onto);
> - free(state.detached_from);
> - return strbuf_detach(&desc, NULL);
> -}
> -
Hmph, the name used to be a good one within the context of
implementation of "git branch" command, but I have to wonder if it
is a specific enough name in the global context of the entire
project. I also wondered if this sits better in wt-status.c;
doesn't "git status" do something similar?
For now, this should do, as I do not think of anything better.
Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 02/16] ref-filter: include reference to 'used_atom' within 'atom_value'
Karthik Nayak writes:
> Ensure that each 'atom_value' has a reference to its corresponding
> 'used_atom'. This let's us use values within 'used_atom' in the
> 'handler' function.
>
> Hence we can get the %(align) atom's parameters directly from the
> 'used_atom' therefore removing the necessity of passing %(align) atom's
> parameters to 'atom_value'.
>
> This also acts as a preparatory patch for the upcoming patch where we
> introduce %(if:equals=) and %(if:notequals=).
>
> Signed-off-by: Karthik Nayak
> ---
> ref-filter.c | 8 +++-
> 1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/ref-filter.c b/ref-filter.c
> index 41e73f0..12e646c 100644
> --- a/ref-filter.c
> +++ b/ref-filter.c
> @@ -230,11 +230,9 @@ struct ref_formatting_state {
>
> struct atom_value {
> const char *s;
> - union {
> - struct align align;
> - } u;
> void (*handler)(struct atom_value *atomv, struct ref_formatting_state
> *state);
> unsigned long ul; /* used for sorting when not FIELD_STR */
> + struct used_atom *atom;
> };
Makes sense, as this would make what 04/16 does clearer.
--
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 13/16] ref-filter: allow porcelain to translate messages in the output
Karthik Nayak writes:
> +static struct ref_msg {
> + const char *gone;
> + const char *ahead;
> + const char *behind;
> + const char *ahead_behind;
> +} msgs = {
> + "gone",
> + "ahead %d",
> + "behind %d",
> + "ahead %d, behind %d"
> +};
> +
> +void setup_ref_filter_porcelain_msg(void)
> +{
> + msgs.gone = _("gone");
> + msgs.ahead = _("ahead %d");
> + msgs.behind = _("behind %d");
> + msgs.ahead_behind = _("ahead %d, behind %d");
> +}
I do not think this patch is wrong, but I wonder if it would be
sufficient to have a single bit in file-scope static variable and
flip it in setup_ref_filter_porcelain_msg(). I.e.
static int use_porcelain_msg; /* false by default */
void setup_ref_filter_porcelain_msg(void)
{
use_porcelain_msg = 1;
}
static const char *P_(const char *msg)
{
return use_porcelain_msg ? _(msg) : msg;
}
and then mark the message up perhaps like so:
- *s = xstrdup("gone");
+ *s = xstrdup(P_("gone"));
which may make things much simpler.
We'd need to update Makefile to recognize X_() as another keyword;
I'd think something like this should do:
XGETTEXT_FLAGS_C = $(XGETTEXT_FLAGS) --language=C \
---keyword=_ --keyword=N_ --keyword="Q_:1,2"
+--keyword=_ --keyword=N_ --keyword=P_ --keyword="Q_:1,2"
> typedef enum { FIELD_STR, FIELD_ULONG, FIELD_TIME } cmp_type;
>
> struct align {
> @@ -1097,15 +1117,15 @@ static void fill_remote_ref_details(struct used_atom
> *atom, const char *refname,
> else if (atom->u.remote_ref.option == RR_TRACK) {
> if (stat_tracking_info(branch, &num_ours,
> &num_theirs, NULL)) {
> - *s = xstrdup("gone");
> + *s = xstrdup(msgs.gone);
> } else if (!num_ours && !num_theirs)
> *s = "";
> else if (!num_ours)
> - *s = xstrfmt("behind %d", num_theirs);
> + *s = xstrfmt(msgs.behind, num_theirs);
> else if (!num_theirs)
> - *s = xstrfmt("ahead %d", num_ours);
> + *s = xstrfmt(msgs.ahead, num_ours);
> else
> - *s = xstrfmt("ahead %d, behind %d",
> + *s = xstrfmt(msgs.ahead_behind,
>num_ours, num_theirs);
> if (!atom->u.remote_ref.nobracket && *s[0]) {
> const char *to_free = *s;
> diff --git a/ref-filter.h b/ref-filter.h
> index 0014b92..da17145 100644
> --- a/ref-filter.h
> +++ b/ref-filter.h
> @@ -111,5 +111,7 @@ struct ref_sorting *ref_default_sorting(void);
> int parse_opt_merge_filter(const struct option *opt, const char *arg, int
> unset);
> /* Get the current HEAD's description */
> char *get_head_description(void);
> +/* Set up translated strings in the output. */
> +void setup_ref_filter_porcelain_msg(void);
>
> #endif /* REF_FILTER_H */
--
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: [PATCHv2 1/4] notes: don't leak memory in git_config_get_notes_strategy
On Wed, Mar 30, 2016 at 2:07 PM, Junio C Hamano wrote:
> Eric Sunshine writes:
>
>>> diff --git a/builtin/notes.c b/builtin/notes.c
>>> @@ -744,13 +744,14 @@ static int merge_commit(struct notes_merge_options *o)
>>> static int git_config_get_notes_strategy(const char *key,
>>> enum notes_merge_strategy
>>> *strategy)
>>> {
>>> - const char *value;
>>> + char *value;
>>>
>>> - if (git_config_get_string_const(key, &value))
>>> + if (git_config_get_string(key, &value))
>>> return 1;
>>> if (parse_notes_merge_strategy(value, strategy))
>>> git_die_config(key, "unknown notes merge strategy %s",
>>> value);
>>>
>>> + free(value);
>>> return 0;
>>> }
>>
>> Hmm, I thought Peff's suggestion of using git_config_get_value() was
>> accepted as superior since it avoids the allocation altogether, thus
>> no need for free() and no leak.
>
> I agree that this caller can avoid taking ownership of value by
> using git_config_get_value() and that would be a cleaner solution
> here.
>
> This is a tangent, but am I the only one who finds that the naming
> of functions in config-get API is confusing? Just wondering if we
> should rename the ones that keeps the memory ownership to the config
> subsystem with s/get/peek/ or something.
>
I demonstrated the confusion and disability to read and distinguish
between those names clearly already.
So I'd strongly favor a better naming for functions in config.c
--
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: [PATCHv2 1/4] notes: don't leak memory in git_config_get_notes_strategy
Eric Sunshine writes:
>> diff --git a/builtin/notes.c b/builtin/notes.c
>> @@ -744,13 +744,14 @@ static int merge_commit(struct notes_merge_options *o)
>> static int git_config_get_notes_strategy(const char *key,
>> enum notes_merge_strategy *strategy)
>> {
>> - const char *value;
>> + char *value;
>>
>> - if (git_config_get_string_const(key, &value))
>> + if (git_config_get_string(key, &value))
>> return 1;
>> if (parse_notes_merge_strategy(value, strategy))
>> git_die_config(key, "unknown notes merge strategy %s",
>> value);
>>
>> + free(value);
>> return 0;
>> }
>
> Hmm, I thought Peff's suggestion of using git_config_get_value() was
> accepted as superior since it avoids the allocation altogether, thus
> no need for free() and no leak.
I agree that this caller can avoid taking ownership of value by
using git_config_get_value() and that would be a cleaner solution
here.
This is a tangent, but am I the only one who finds that the naming
of functions in config-get API is confusing? Just wondering if we
should rename the ones that keeps the memory ownership to the config
subsystem with s/get/peek/ 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] for-each-ref: fix description of '--contains' in manpage
SZEDER Gábor writes: > 'git for-each-ref's manpage says that '--contains' only lists tags, > but it lists all kinds of refs. > > Signed-off-by: SZEDER Gábor > --- Thanks; will apply on top of 4a71109a (for-each-ref: add '--contains' option, 2015-07-07) so that we can eventually include it in 2.7.x maintenance track. > Documentation/git-for-each-ref.txt | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/Documentation/git-for-each-ref.txt > b/Documentation/git-for-each-ref.txt > index 012e8f9a080d..c52578bb87cc 100644 > --- a/Documentation/git-for-each-ref.txt > +++ b/Documentation/git-for-each-ref.txt > @@ -76,7 +76,7 @@ OPTIONS > specified commit (HEAD if not specified). > > --contains []:: > - Only list tags which contain the specified commit (HEAD if not > + Only list refs which contain the specified commit (HEAD if not > specified). > > FIELD NAMES -- 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] `git submodule update --init --recursive` stores gitdir in full path into `.git` of nested submodules
Stefan Beller writes: > On Wed, Mar 30, 2016 at 9:05 AM, Stefan Beller wrote: >> On Wed, Mar 30, 2016 at 2:03 AM, Norio Nomura wrote: >>> Hi, >>> >>> `git submodule update --init --recursive` stores `gitdir` in full path into >>> `.git` of nested submodules. >>> So, working directory is not portable to another directory. >> >> Are you reporting a regression bug? (Is that a new thing or has it >> always been that way and you just discover that it is unfortunate?) >> Which versions did you test with? > > ➜ 15:34:32 git:(master) git --version > git version 2.8.0 > > at the end of your gist. > The same happens when using 2.7.4, it doesn't happen when using 2.6.6 though. > > It turns out ee8838d (2015-09-08, submodule: rewrite `module_clone` > shell function in C) > broke it. > > I'll look into fixing it. Thanks. This may be an unrelated tangent, but somewhere in "submodule init" codepath seem to turn paths into absolute too aggressively. Merging your recent "path related fix" to 'pu' seems to break some test by showing absolute paths when the test expects to see relative ones. -- 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] diffcore: fix iteration order of identical files during rename detection
SZEDER Gábor writes: > Fill the hashmap with source entries in reverse order to restore the > original exact rename detection behavior. Thanks for digging out and fixing this unintended regression that happened long time ago. Will queue. > > Reported-by: Bill Okara > Signed-off-by: SZEDER Gábor > --- > > Resend of the patch, with a slightly updated commit message, included > in > > http://thread.gmane.org/gmane.comp.version-control.git/287281/focus=287570 > > Being embedded with scissors in an email without Junio among the > recipients on the day the first -rc was tagged... no wonder it flew > below the radar. > > diffcore-rename.c | 6 -- > t/t4001-diff-rename.sh | 11 +++ > 2 files changed, 15 insertions(+), 2 deletions(-) > > diff --git a/diffcore-rename.c b/diffcore-rename.c > index 3b3c1ed535e7..7f03eb5a0404 100644 > --- a/diffcore-rename.c > +++ b/diffcore-rename.c > @@ -340,9 +340,11 @@ static int find_exact_renames(struct diff_options > *options) > int i, renames = 0; > struct hashmap file_table; > > - /* Add all sources to the hash table */ > + /* Add all sources to the hash table in reverse order, because > + * later on they will be retrieved in LIFO order. > + */ > hashmap_init(&file_table, NULL, rename_src_nr); > - for (i = 0; i < rename_src_nr; i++) > + for (i = rename_src_nr-1; i >= 0; i--) > insert_file_table(&file_table, i, rename_src[i].p->one); > > /* Walk the destinations and find best source match */ > diff --git a/t/t4001-diff-rename.sh b/t/t4001-diff-rename.sh > index 2f327b749588..ed90c6c6f984 100755 > --- a/t/t4001-diff-rename.sh > +++ b/t/t4001-diff-rename.sh > @@ -77,6 +77,17 @@ test_expect_success 'favour same basenames even with minor > differences' ' > git show HEAD:path1 | sed "s/15/16/" > subdir/path1 && > git status | test_i18ngrep "renamed: .*path1 -> subdir/path1"' > > +test_expect_success 'two files with same basename and same content' ' > + git reset --hard && > + mkdir -p dir/A dir/B && > + cp path1 dir/A/file && > + cp path1 dir/B/file && > + git add dir && > + git commit -m 2 && > + git mv dir other-dir && > + git status | test_i18ngrep "renamed: .*dir/A/file -> other-dir/A/file" > +' > + > test_expect_success 'setup for many rename source candidates' ' > git reset --hard && > for i in 0 1 2 3 4 5 6 7 8 9; -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/5] t/t5520: test --[no-]autostash with pull.rebase=true
On Wed, Mar 30, 2016 at 3:00 PM, Mehul Jain wrote:
> On Wed, Mar 30, 2016 at 2:46 AM, Eric Sunshine
> wrote:
>> With the exception of the missing --rebase argument, this is exactly
>> the same code as in test_rebase_autostash(), right? Rather than
>> repeating this code yet again, it might be nice to augment that
>> function to accept a (possibly) optional argument controlling whether
>> --rebase is used.
>
> Thanks for the idea. I have come up with something like this:
>
> test_pull () {
> git reset --hard before-rebase &&
> echo dirty >new_file &&
> git add new_file &&
> git pull $@ . copy &&
> test_cmp_rev HEAD^ copy &&
> test "$(cat new_file)" = dirty &&
> test "$(cat file)" = "modified again"
> }
>
> test_pull_fail () {
> git reset --hard before-rebase &&
> echo dirty >new_file &&
> git add new_file &&
> test_must_fail git pull $@ . copy 2>err &&
> test_i18ngrep "uncommitted changes." err
> }
Considering that these are specifically testing behavior related to
autostashing, it might make sense to have "autostash" in the function
names, but that's a very minor point.
> test_expect_success 'pull --rebase succeeds with dirty working
> directory and rebase.autostash set' '
> test_config rebase.autostash true &&
> test_pull --rebase
> '
> [...]
> test_expect_success 'pull --no-autostash & pull.rebase=true' '
> test_config pull.rebase true &&
> test_pull_fail --no-autostash
> '
>
> I'm sorry if this is bit difficult to digest without diff output. I
> just wanted to
> know if the above mention functions looks suitable to you.
This is exactly what I had in mind for simplifying the tests, and it's
perfectly easy to read in this form (a diff would be worse for this
illustration).
One other possibility would be to make this all table-driven by
collecting all of the above state information into a table and then
feeding that into a function (either as its argument list or via
stdin). For instance:
test_autostash <<\-EOF
ok,--rebase,rebase.autostash=true
ok,--rebase --autostash,rebase.autostash=true
ok,--rebase --autostash,rebase.autostash=false
ok,--rebase --autostash,rebase.autostash=
err,--rebase --no-autostash,rebase.autostash=true
err,--rebase --no-autostash,rebase.autostash=false
err,--rebase --no-autostash,rebase.autostash=
ok,--autostash,pull.rebase=true
err,--no-autostash,pull.rebase=true
EOF
The function would loop over the input, split each line apart by
setting IFS=, and then run the test based upon the state information.
"ok" means autostash is expected to succeed, and err means it is
expected to fail. The function would want to specially recognize the
"foo.bar=" in the last argument in order to invoke test_unconfig()
rather than test_config().
However, this may be a case of diminishing returns. The tests as you
illustrated them are sufficiently simple and easy to grok that the
table-driven approach may not add much value (aside from making it
easier to see at a glance if any cases were omitted).
--
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: [PATCHv4 1/6] submodule foreach: correct path display in recursive submodules
Stefan Beller writes: > The `prefix` was put in front of the display path unconditionally. > This is wrong as any relative path computation would need to be at > the front, so include the prefix into the display path. > > The new test replicates the previous test with the difference of executing > from a sub directory. By executing from a sub directory all we would > expect all displayed paths to be prefixed by '../'. > > Prior to this patch the test would report > Entering 'nested1/nested2/../nested3' > instead of the expected > Entering '../nested1/nested2/nested3' > > Signed-off-by: Stefan Beller > --- Definitely easier to read and helps understand what is going on in the changed codepath much better. Very well done. Thanks. > diff --git a/t/t7407-submodule-foreach.sh b/t/t7407-submodule-foreach.sh > index 7ca10b8..776b349 100755 > --- a/t/t7407-submodule-foreach.sh > +++ b/t/t7407-submodule-foreach.sh > @@ -178,6 +178,26 @@ test_expect_success 'test messages from "foreach > --recursive"' ' > ' > > cat > expect < +Entering '../nested1' > +Entering '../nested1/nested2' > +Entering '../nested1/nested2/nested3' > +Entering '../nested1/nested2/nested3/submodule' > +Entering '../sub1' > +Entering '../sub2' > +Entering '../sub3' > +EOF > + > +test_expect_success 'test messages from "foreach --recursive" from > subdirectory' ' > + ( > + cd clone2 && > + mkdir untracked && > + cd untracked && > + git submodule foreach --recursive >../../actual > + ) && > + test_i18ncmp expect actual > +' > + > +cat > expect < nested1-nested1 > nested2-nested2 > nested3-nested3 -- 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: weird diff output?
On Wed, Mar 30, 2016 at 12:31 PM, Jacob Keller wrote: > > If unsure, say Y. > + > +config RMI4_I2C > + tristate "RMI4 I2C Support" > + depends on RMI4_CORE && I2C > + help > + Say Y here if you want to support RMI4 devices connected to an I2C > + bus. > + > + If unsure, say Y. > > after: > > required for all RMI4 device support. > > + If unsure, say Y. > + > +config RMI4_I2C > + tristate "RMI4 I2C Support" > + depends on RMI4_CORE && I2C > + help > + Say Y here if you want to support RMI4 devices connected to an I2C > + bus. > + > If unsure, say Y. The optimum would be: > > If unsure, say Y. > > +config RMI4_I2C > + tristate "RMI4 I2C Support" > + depends on RMI4_CORE && I2C > + help > + Say Y here if you want to support RMI4 devices connected to an I2C > + bus. > + > + If unsure, say Y. > + > config BLA_I2C The overlapping lines: > + > + If unsure, say Y. > + However that broke the lines at the first empty line, not the last as Jeff claimed it. (Could there be a problem in the perl script when empty lines are at the first or last overlapping line?) Thanks for going through examples! (I would, too. But fixing a submodule regression is more important now; I only develop new features when there are no known regressions caused by me) Thanks, Stefan > > So in this particular instance which has multiple blank lines and is a > similar issue as with Stefan's note above, this is where the heuristic > falls apart. At least for C code this is basically vanishingly small > compared to the number of comment header fix ups. > > I think it may be that Stefan's suggestions above may be on the right > track to resolve that too. > > Regards, > Jake -- 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: weird diff output?
On Wed, Mar 30, 2016 at 12:14 PM, Jacob Keller wrote: > I ran this on a few of my local projects and it doesn't seem to > produce any false positives so far. Everything looks good. Of course > this is with just traditional C code. I am currently trying to run > this against the history of Linux as well and see if I can find > anything that seems bad there. > > Thanks, > Jake So far I've only found a single location that ends up looking worse within the Linux kernel. Diffs of some Kbuild settings result in something like before: If unsure, say Y. + +config RMI4_I2C + tristate "RMI4 I2C Support" + depends on RMI4_CORE && I2C + help + Say Y here if you want to support RMI4 devices connected to an I2C + bus. + + If unsure, say Y. after: required for all RMI4 device support. + If unsure, say Y. + +config RMI4_I2C + tristate "RMI4 I2C Support" + depends on RMI4_CORE && I2C + help + Say Y here if you want to support RMI4 devices connected to an I2C + bus. + If unsure, say Y. So in this particular instance which has multiple blank lines and is a similar issue as with Stefan's note above, this is where the heuristic falls apart. At least for C code this is basically vanishingly small compared to the number of comment header fix ups. I think it may be that Stefan's suggestions above may be on the right track to resolve that too. Regards, Jake -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] ident: make the useConfigOnly error messages more informative
The env_hint message applies perfectly to the case when
user.useConfigOnly is set and at least one of the user.name and the
user.email are not provided. Additionally, use a more descriptive error
message when that happens.
Signed-off-by: Marios Titas
---
ident.c | 12
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/ident.c b/ident.c
index 74b2663..4fd82d1 100644
--- a/ident.c
+++ b/ident.c
@@ -352,8 +352,10 @@ const char *fmt_ident(const char *name, const char *email,
int using_default = 0;
if (!name) {
if (strict && ident_use_config_only
- && !(ident_config_given & IDENT_NAME_GIVEN))
- die("user.useConfigOnly set but no name given");
+ && !(ident_config_given & IDENT_NAME_GIVEN)) {
+ fputs(env_hint, stderr);
+ die("no name was given and auto-detection is
disabled");
+ }
name = ident_default_name();
using_default = 1;
if (strict && default_name_is_bogus) {
@@ -375,8 +377,10 @@ const char *fmt_ident(const char *name, const char *email,
if (!email) {
if (strict && ident_use_config_only
- && !(ident_config_given & IDENT_MAIL_GIVEN))
- die("user.useConfigOnly set but no mail given");
+ && !(ident_config_given & IDENT_MAIL_GIVEN)) {
+ fputs(env_hint, stderr);
+ die("no email was given and auto-detection is
disabled");
+ }
email = ident_default_email();
if (strict && default_email_is_bogus) {
fputs(env_hint, stderr);
--
2.8.0
--
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] ident: check for useConfigOnly before auto-detection of name/email
If user.useConfigOnly is set, it does not make sense to try to
auto-detect the name and/or the email. So it's better to do the
useConfigOnly checks first.
Signed-off-by: Marios Titas
---
ident.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/ident.c b/ident.c
index 6e12582..74b2663 100644
--- a/ident.c
+++ b/ident.c
@@ -351,15 +351,15 @@ const char *fmt_ident(const char *name, const char *email,
if (want_name) {
int using_default = 0;
if (!name) {
+ if (strict && ident_use_config_only
+ && !(ident_config_given & IDENT_NAME_GIVEN))
+ die("user.useConfigOnly set but no name given");
name = ident_default_name();
using_default = 1;
if (strict && default_name_is_bogus) {
fputs(env_hint, stderr);
die("unable to auto-detect name (got '%s')",
name);
}
- if (strict && ident_use_config_only
- && !(ident_config_given & IDENT_NAME_GIVEN))
- die("user.useConfigOnly set but no name given");
}
if (!*name) {
struct passwd *pw;
@@ -374,14 +374,14 @@ const char *fmt_ident(const char *name, const char *email,
}
if (!email) {
+ if (strict && ident_use_config_only
+ && !(ident_config_given & IDENT_MAIL_GIVEN))
+ die("user.useConfigOnly set but no mail given");
email = ident_default_email();
if (strict && default_email_is_bogus) {
fputs(env_hint, stderr);
die("unable to auto-detect email address (got '%s')",
email);
}
- if (strict && ident_use_config_only
- && !(ident_config_given & IDENT_MAIL_GIVEN))
- die("user.useConfigOnly set but no mail given");
}
strbuf_reset(&ident);
--
2.8.0
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 2/3] pretty: enable --expand-tabs by default for selected pretty formats
Jeff King writes:
> On Tue, Mar 29, 2016 at 04:15:08PM -0700, Junio C Hamano wrote:
>
>> diff --git a/Documentation/pretty-options.txt
>> b/Documentation/pretty-options.txt
>> index 4fb5c76..23967b6 100644
>> --- a/Documentation/pretty-options.txt
>> +++ b/Documentation/pretty-options.txt
>> @@ -43,10 +43,16 @@ people using 80-column terminals.
>> commit may be copied to the output.
>>
>> --expand-tabs::
>> +--no-expand-tabs::
>> Perform a tab expansion (replace each tab with enough number
>> of spaces to fill to the next display column that is
>> multiple of 8) in the log message before using the message
>> to show in the output.
>> ++
>> +By default, tabs are expanded in pretty formats that indent the log
>> +message by 4 spaces (i.e. 'medium', which is the default, 'full',
>> +and "fuller'). `--no-expand-tabs` option can be used to disable
>> +this.
>
> Mismatched quote types on "fuller".
Thanks.
>> @@ -172,6 +173,7 @@ void get_commit_format(const char *arg, struct rev_info
>> *rev)
>>
>> rev->commit_format = commit_format->format;
>> rev->use_terminator = commit_format->is_tformat;
>> +rev->expand_tabs_in_log = commit_format->expand_tabs_in_log;
>> if (commit_format->format == CMIT_FMT_USERFORMAT) {
>> save_user_format(rev, commit_format->user_format,
>> commit_format->is_tformat);
>
> This feels like the wrong time to set the value in rev_info, as it means
> that:
>
> git log --no-expand-tabs --pretty=full
>
> and
>
> git log --pretty=full --no-expand-tabs
>
> behave differently.
I was sort of hoping that we can get away by defining that "an
explicit --pretty asks for the full behaviour of the format it
specifies, e.g. if you ask --pretty=medium, you are asking for
4-space indented tab-expanded log with the headers at the medium
level of detail".
> The other values set in get_commit_format, like "use_terminator",
> are inherently part of the format, but I don't think this is.
IOW, I was hoping nobody would agree with that and rather everybody
would consider tab-expansion is part of the format.
Let me try your way instead and report how it went when I send out a
reroll.
> Likewise, if we were to eventually add config like "[log]expandtab = 4",
> it should not be overridden by "--pretty=full" (but we probably _would_
> want to have it kick in only for certain formats).
This is exactly why I didn't do a configuration variable, as I think
we can make only 50% of people happy. Some would say "when I
explicitly ask for the "email" format, I expect that expandtab
configuration gets ignored" while others would say "I said I want
expandtab in the configuration no matter what".
--
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: weird diff output?
On Tue, Mar 29, 2016 at 11:05 PM, Jacob Keller wrote: > On Tue, Mar 29, 2016 at 9:55 PM, Jeff King wrote: >> One thing I like to do when playing with new diff ideas is to pipe all >> of "log -p" for a real project through it and see what differences it >> produces. >> > > Great idea! > >> Below is a perl script that implements Stefan's heuristic. I checked its >> output on git.git with: >> >> git log --format='commit %H' -p >old >> perl /path/to/script new >> diff -F ^commit -u old new | less >> >> which shows the differences, with the commit id in the hunk header >> (which makes it easy to "git show $commit | perl /path/to/script" to >> see the new diff with more context. >> > > I'll try to run this against my projects and see what it looks like > to see if I can spot (m)any counter examples, which would indicate > it's a bad idea. I may have some time in the next few days to see how > hard it would be to fully integrate it into the diff machinery too. > > Thanks for the help! > > Regards, > Jake > I ran this on a few of my local projects and it doesn't seem to produce any false positives so far. Everything looks good. Of course this is with just traditional C code. I am currently trying to run this against the history of Linux as well and see if I can find anything that seems bad there. Thanks, Jake -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/5] t/t5520: test --[no-]autostash with pull.rebase=true
Hi Eric,
Thanks for the reviews on this series.
On Wed, Mar 30, 2016 at 2:46 AM, Eric Sunshine wrote:
> With the exception of the missing --rebase argument, this is exactly
> the same code as in test_rebase_autostash(), right? Rather than
> repeating this code yet again, it might be nice to augment that
> function to accept a (possibly) optional argument controlling whether
> --rebase is used.
Thanks for the idea. I have come up with something like this:
* Introduce two function test_pull() and test_pull_fail() in
the place of
test_rebase_autostash() and test_rebase_no_autostash.()
Using these functions we can easily re-write all the 6 tests which
deals with combination of autostash and rebase.autostash. Plus
these functions helped in writing two new tests which deals with
combination of pull.rebase and autostash. Thus reducing the code
base to simpler and fewer lines of code. Also I could re-write one
of the old test to reduce the repetition with them.
Here are the functions and there implementations:
---
test_pull () {
git reset --hard before-rebase &&
echo dirty >new_file &&
git add new_file &&
git pull $@ . copy &&
test_cmp_rev HEAD^ copy &&
test "$(cat new_file)" = dirty &&
test "$(cat file)" = "modified again"
}
test_pull_fail () {
git reset --hard before-rebase &&
echo dirty >new_file &&
git add new_file &&
test_must_fail git pull $@ . copy 2>err &&
test_i18ngrep "uncommitted changes." err
}
test_expect_success 'pull --rebase succeeds with dirty working
directory and rebase.autostash set' '
test_config rebase.autostash true &&
test_pull --rebase
'
test_expect_success "pull --rebase --autostash & rebase.autostash=true" '
test_config rebase.autostash true &&
test_pull --rebase --autostash
'
test_expect_success "pull --rebase --autostash & rebase.autostash=false" '
test_config rebase.autostash false &&
test_pull --rebase --autostash
'
test_expect_success 'pull --rebase: --autostash & rebase.autostash unset' '
test_unconfig rebase.autostash &&
test_pull --rebase --autostash
'
test_expect_success "pull --rebase --no-autostash & rebase.autostash=true" '
test_config rebase.autostash true &&
test_pull_fail --rebase --no-autostash
'
test_expect_success "pull --rebase --no-autostash & rebase.autostash=false" '
test_config rebase.autostash false &&
test_pull_fail --rebase --no-autostash
'
test_expect_success 'pull --rebase --no-autostash & rebase.autostash unset' '
test_unconfig rebase.autostash &&
test_pull_fail --rebase --no-autostash
'
test_expect_success 'pull --autostash & pull.rebase=true' '
test_config pull.rebase true &&
test_pull --autostash
'
test_expect_success 'pull --no-autostash & pull.rebase=true' '
test_config pull.rebase true &&
test_pull_fail --no-autostash
'
---
I'm sorry if this is bit difficult to digest without diff output. I
just wanted to
know if the above mention functions looks suitable to you.
Also I've read your comments on other patches of this series, I will make
changes accordingly ones above mention functions, tests looks fit for a
re-roll.
Thanks,
Mehul
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 1/3] pretty: expand tabs in indented logs to make things line up properly
Eric Sunshine writes: > On Tue, Mar 29, 2016 at 7:15 PM, Junio C Hamano wrote: >> From: Linus Torvalds >> >> A commit log message sometimes tries to line things up using tabs, >> assuming fixed-width font with the standard 8-place tab settings. >> Viewing such a commit however does not work well in "git log", as >> we indent the lines by prefixing 4 spaces in front of them. >> [...] >> Signed-off-by: Linus Torvalds >> Signed-off-by: Junio C Hamano >> --- >> diff --git a/Documentation/pretty-options.txt >> b/Documentation/pretty-options.txt >> @@ -42,6 +42,12 @@ people using 80-column terminals. >> +--expand-tabs:: >> + Perform a tab expansion (replace each tab with enough number >> + of spaces to fill to the next display column that is > > Nit: "enough spaces" or "a sufficient number of spaces". Thanks, will be part of a reroll (when it happens ;-). -- 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 V3] MSVC: Use shipped headers instead of fallback definitions
Sven Strickroth writes: > diff --git a/compat/mingw.h b/compat/mingw.h > index 6b6d695..137f42e 100644 > --- a/compat/mingw.h > +++ b/compat/mingw.h > @@ -415,7 +415,7 @@ int mingw_offset_1st_component(const char *path); > extern void build_libgit_environment(void); > extern const char *program_data_config(void); > #define git_program_data_config program_data_config > -#ifndef __MINGW64_VERSION_MAJOR > +#if !defined(__MINGW64_VERSION_MAJOR) && (!defined(_MSC_VER) || _MSC_VER < > 1800) > #define PRIuMAX "I64u" > #define PRId64 "I64d" > #else I'll wiggle this in, but you seem to be building on top of some unrelated work (please avoid sending such a patch in the future). Thanks, all of you. -- >8 -- From: Sven Strickroth Date: Wed, 30 Mar 2016 13:37:36 +0200 Subject: [PATCH] MSVC: use shipped headers instead of fallback definitions VS2010 comes with stdint.h [1] VS2013 comes with inttypes.h [2] [1] https://stackoverflow.com/a/2628014/3906760 [2] https://blogs.msdn.microsoft.com/vcblog/2013/07/19/c99-library-support-in-visual-studio-2013/ Signed-off-by: Sven Strickroth Acked-by: Johannes Schindelin Acked-by: Sebastian Schuberth Signed-off-by: Junio C Hamano --- compat/mingw.h | 2 +- compat/vcbuild/include/unistd.h | 4 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/compat/mingw.h b/compat/mingw.h index c008694..1de70ff 100644 --- a/compat/mingw.h +++ b/compat/mingw.h @@ -406,7 +406,7 @@ static inline void convert_slashes(char *path) int mingw_offset_1st_component(const char *path); #define offset_1st_component mingw_offset_1st_component #define PATH_SEP ';' -#ifndef __MINGW64_VERSION_MAJOR +#if !defined(__MINGW64_VERSION_MAJOR) && (!defined(_MSC_VER) || _MSC_VER < 1800) #define PRIuMAX "I64u" #define PRId64 "I64d" #else diff --git a/compat/vcbuild/include/unistd.h b/compat/vcbuild/include/unistd.h index c65c2cd..3a959d1 100644 --- a/compat/vcbuild/include/unistd.h +++ b/compat/vcbuild/include/unistd.h @@ -45,11 +45,15 @@ typedef unsigned long long uintmax_t; typedef int64_t off64_t; +#if !defined(_MSC_VER) || _MSC_VER < 1600 #define INTMAX_MIN _I64_MIN #define INTMAX_MAX _I64_MAX #define UINTMAX_MAX _UI64_MAX #define UINT32_MAX 0x /* 4294967295U */ +#else +#include +#endif #define STDIN_FILENO 0 #define STDOUT_FILENO 1 -- 2.8.0-215-gd29a7d9 -- 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 v5] worktree: add: introduce --checkout option
[please don't top-post] On Tue, Mar 29, 2016 at 11:11 PM, Zhang Lei wrote: > Thanks for the review. > Sorry for the patch churn, I wasn't quite familiar with working with > mailing list. No need to apologize. Reviewers understand what it is like being a newcomer and provide additional review comments to help get up to speed. Thanks for working on this enhancement. -- 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 v1 6/7] correct blame for files commited with CRLF
7/7 needs to be amended with something like this,
and the documentation needs an update.
In any case, the user can normalize the repo like this:
$ echo "* text=auto" >>.gitattributes
$ rm .git/index # Remove the index to force Git to
$ git reset # re-scan the working directory
$ git status# Show files that will be normalized
$ git add -u
$ git add .gitattributes
$ git commit -m "Introduce end-of-line normalization"
(or run "dos2unix" filename)
commit a604db36bb946000776514c220964f32979c8756
Author: Torsten Bögershausen
Date: Wed Mar 30 15:53:52 2016 +0200
convert.c: add warning when eol are wrong after checkout
When line endings are not normalized, they may be different after the
next checkout to what is configured.
Add a warning, similar to the CRLF-LF replacement, when a file is commited,
and the line endings are not converted at commit or checkout.
diff --git a/convert.c b/convert.c
index 8266d87..1fddbe8 100644
--- a/convert.c
+++ b/convert.c
@@ -18,6 +18,8 @@
#define CONVERT_STAT_BITS_TXT_CRLF 0x2
#define CONVERT_STAT_BITS_BIN 0x4
+#define CONVERT_STAT_BITS_MIXED (CONVERT_STAT_BITS_TXT_LF |
CONVERT_STAT_BITS_TXT_CRLF)
+
enum crlf_action {
CRLF_UNDEFINED,
CRLF_BINARY,
@@ -279,6 +281,8 @@ static void check_safe_crlf(const char *path, enum
crlf_action crlf_action,
enum safe_crlf checksafe,
unsigned convert_stats, unsigned new_convert_stats)
{
+ enum eol new_eol = output_eol(crlf_action);
+ const char *err_warn_msg = NULL;
if (!checksafe)
return;
if (convert_stats & CONVERT_STAT_BITS_TXT_CRLF &&
@@ -303,6 +307,19 @@ static void check_safe_crlf(const char *path, enum
crlf_action crlf_action,
else /* i.e. SAFE_CRLF_FAIL */
die("LF would be replaced by CRLF in %s", path);
}
+ if ((new_convert_stats & CONVERT_STAT_BITS_MIXED) ==
CONVERT_STAT_BITS_MIXED)
+ err_warn_msg = "mixed eol";
+ else if (new_eol == EOL_LF && new_convert_stats &
CONVERT_STAT_BITS_TXT_CRLF)
+ err_warn_msg = "CRLF";
+
+ if (err_warn_msg) {
+ if (checksafe == SAFE_CRLF_WARN)
+ warning("%s will be present after commit and checkout
in %s.",
+ err_warn_msg, path);
+ else
+ die("%s will be present after commit and checkout in
%s",
+ err_warn_msg, path);
+ }
}
[snip changes in t0027]
--
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: [PATCHv2 3/4] bundle: don't leak an fd in case of early return
On Wed, Mar 30, 2016 at 1:05 PM, Stefan Beller wrote:
> In successful operation `write_pack_data` will close the `bundle_fd`,
> but when we exit early, we need to take care of the file descriptor
> as well as the lock file ourselves. The lock file may be deleted at the
> end of running the program, but we are in library code, so we should
> not rely on that.
>
> Helped-by: Jeff King
> Signed-off-by: Stefan Beller
> ---
> diff --git a/bundle.c b/bundle.c
> @@ -435,30 +436,40 @@ int create_bundle(struct bundle_header *header, const
> char *path,
>
> /* write prerequisites */
> if (compute_and_write_prerequisites(bundle_fd, &revs, argc, argv))
> - return -1;
> + goto err;
>
> argc = setup_revisions(argc, argv, &revs, NULL);
>
> - if (argc > 1)
> - return error(_("unrecognized argument: %s"), argv[1]);
> + if (argc > 1) {
> + ret = error(_("unrecognized argument: %s"), argv[1]);
> + goto err;
> + }
>
> object_array_remove_duplicates(&revs.pending);
>
> ref_count = write_bundle_refs(bundle_fd, &revs);
> if (!ref_count)
> die(_("Refusing to create empty bundle."));
> - else if (ref_count < 0)
> - return -1;
> + else if (ref_count < 0) {
> + if (!bundle_to_stdout)
> + close(bundle_fd);
Why is this close() here considering that it gets closed by the 'err' path?
> + goto err;
> + }
>
> /* write pack */
> if (write_pack_data(bundle_fd, &revs))
> - return -1;
> + goto err;
>
> if (!bundle_to_stdout) {
> if (commit_lock_file(&lock))
> die_errno(_("cannot create '%s'"), path);
> }
> return 0;
> +err:
> + if (!bundle_to_stdout)
> + close(bundle_fd);
> + rollback_lock_file(&lock);
> + 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: [PATCHv2 0/4] Some cleanups
On Wed, Mar 30, 2016 at 10:32:40AM -0700, Stefan Beller wrote: > > I'm OK with all of these as-is, though I did mention a nit in the third > > one. I also like Junio's rewrite instead of using strbuf_list_free. > > I'm fine using the rewritten version instead of using strbuf_list_free. :) > On the third one, there is one case, where we have > > if (..) > return error(_(text)); > > and that is an exit(128); eventually. In the caller perhaps, but isn't that equivalent to: error(_(text)); return -1; ? I think it is OK to make assumptions about error()'s return value; that is what it is there for. -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: [PATCHv2 2/4] abbrev_sha1_in_line: don't leak memory
On Wed, Mar 30, 2016 at 1:05 PM, Stefan Beller wrote: > `split` is of type `struct strbuf **`, which we have a dedicated free > function for, which takes care of freeing all related memory. I think it's important to explain that 'split' and each split[] element were being leaked (despite the existing strbuf_release()) as justification for why this change is beneficial. > Helped-by: Eric Sunshine > Signed-off-by: Stefan Beller > --- > wt-status.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/wt-status.c b/wt-status.c > index ef74864..1ea2ebe 100644 > --- a/wt-status.c > +++ b/wt-status.c > @@ -1063,9 +1063,7 @@ static void abbrev_sha1_in_line(struct strbuf *line) > strbuf_addf(line, "%s", split[i]->buf); > } > } > - for (i = 0; split[i]; i++) > - strbuf_release(split[i]); > - > + strbuf_list_free(split); > } -- 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: [PATCHv2 1/4] notes: don't leak memory in git_config_get_notes_strategy
On Wed, Mar 30, 2016 at 1:05 PM, Stefan Beller wrote:
> `value` is just a temporary scratchpad, so we need to make sure it doesn't
> leak. It is xstrdup'd in `git_config_get_string` and
> `parse_notes_merge_strategy` just compares the string against predefined
> values, so no need to keep it around longer. Make `value` non-const to
> avoid the cast in the free.
>
> Signed-off-by: Stefan Beller
> ---
> diff --git a/builtin/notes.c b/builtin/notes.c
> @@ -744,13 +744,14 @@ static int merge_commit(struct notes_merge_options *o)
> static int git_config_get_notes_strategy(const char *key,
> enum notes_merge_strategy *strategy)
> {
> - const char *value;
> + char *value;
>
> - if (git_config_get_string_const(key, &value))
> + if (git_config_get_string(key, &value))
> return 1;
> if (parse_notes_merge_strategy(value, strategy))
> git_die_config(key, "unknown notes merge strategy %s", value);
>
> + free(value);
> return 0;
> }
Hmm, I thought Peff's suggestion of using git_config_get_value() was
accepted as superior since it avoids the allocation altogether, thus
no need for free() and no leak.
--
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: [PATCHv2 0/4] Some cleanups
On Wed, Mar 30, 2016 at 10:25 AM, Jeff King wrote: > On Wed, Mar 30, 2016 at 10:05:14AM -0700, Stefan Beller wrote: > >> v2: >> Thanks Eric, Jeff, Junio for discussion, I picked up all hints and here >> is a v2. >> >> * drop the overallocation patches (1&2) >> * use git_config_get_string instead of its _const equivalent, such that >> we don't need a cast when freeing in git_config_get_notes_strategy >> * Use strbuf_list_free instead of cooking our own. >> * have a dedicated error exit path in bundle.c, create_bundle > > I'm OK with all of these as-is, though I did mention a nit in the third > one. I also like Junio's rewrite instead of using strbuf_list_free. I'm fine using the rewritten version instead of using strbuf_list_free. :) On the third one, there is one case, where we have if (..) return error(_(text)); and that is an exit(128); eventually. I thought it is worth preserving the difference (as it is a faithful bug fix not a change to make it better or more uniform). Thanks, Stefan > > -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: [PATCHv2 0/4] Some cleanups
On Wed, Mar 30, 2016 at 10:05:14AM -0700, Stefan Beller wrote: > v2: > Thanks Eric, Jeff, Junio for discussion, I picked up all hints and here > is a v2. > > * drop the overallocation patches (1&2) > * use git_config_get_string instead of its _const equivalent, such that > we don't need a cast when freeing in git_config_get_notes_strategy > * Use strbuf_list_free instead of cooking our own. > * have a dedicated error exit path in bundle.c, create_bundle I'm OK with all of these as-is, though I did mention a nit in the third one. I also like Junio's rewrite instead of using strbuf_list_free. -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] `git submodule update --init --recursive` stores gitdir in full path into `.git` of nested submodules
On Wed, Mar 30, 2016 at 9:05 AM, Stefan Beller wrote: > On Wed, Mar 30, 2016 at 2:03 AM, Norio Nomura wrote: >> Hi, >> >> `git submodule update --init --recursive` stores `gitdir` in full path into >> `.git` of nested submodules. >> So, working directory is not portable to another directory. > > Are you reporting a regression bug? (Is that a new thing or has it > always been that way and you just discover that it is unfortunate?) > Which versions did you test with? ➜ 15:34:32 git:(master) git --version git version 2.8.0 at the end of your gist. The same happens when using 2.7.4, it doesn't happen when using 2.6.6 though. It turns out ee8838d (2015-09-08, submodule: rewrite `module_clone` shell function in C) broke it. I'll look into fixing it. Thanks, Stefan > >> >> On following example, `Carthage/Checkouts/Quick/Externals/Nimble/` is nested >> submodule and `Carthage/Checkouts/Quick/Externals/Nimble/.git` contains full >> path. >> https://gist.github.com/norio-nomura/17ce4bdf0151185e77d9b1fcfb5a469d >> >> Thanks, >> -- >> Norio Nomura @norio_nomura >> >> -- >> To unsubscribe from this list: send the line "unsubscribe git" in >> the body of a message to majord...@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 3/4] bundle: don't leak an fd in case of early return
On Wed, Mar 30, 2016 at 10:05:17AM -0700, Stefan Beller wrote: > diff --git a/bundle.c b/bundle.c > index 506ac49..fbc8593 100644 > --- a/bundle.c > +++ b/bundle.c > @@ -407,6 +407,7 @@ int create_bundle(struct bundle_header *header, const > char *path, > int bundle_to_stdout; > int ref_count = 0; > struct rev_info revs; > + int ret = -1; A minor nit, but I don't think we ever put anything but "-1" in this variable. It could go away and we can just "return -1" in the "err" path. -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 4/6] abbrev_sha1_in_line: don't leak memory
On Wed, Mar 30, 2016 at 10:06:41AM -0700, Junio C Hamano wrote:
> Jeff King writes:
>
> > On Tue, Mar 29, 2016 at 09:30:38PM -0400, Eric Sunshine wrote:
> >
> >> The implementation of strbuf_list_free() is this:
> >>
> >> struct strbuf **s = sbs;
> >> while (*s) {
> >> strbuf_release(*s);
> >> free(*s++);
> >> }
> >> free(sbs);
> >>
> >> which means that wt-status.c is leaking not only 'split', but also
> >> each element of split[], right?
> >
> > Yeah, I didn't notice that, but I think you're right.
>
> Correct.
>
> I suspect that we would be better off in the longer term if
> we made conscious effort to deprecate and eradicate the use
> of strbuf_split* API functions. They are easy to write
> crappy code with, inefficient, and error prone. You would
> rarely need to have N resulting pieces as strbufs to be
> easily manipulatable [*1*].
Yeah, I agree that it is a clunky interface, and I would be happy to see
it go away. Many callers would be better off with string_list_split().
When I've looked in the past, though, converting some of the callers was
somewhat non-trivial.
But in this case...
> The function can be written by not using the "split and then
> rebuild" pattern, perhaps like so, and the result is even
> easier to read and understand, I would say. A sample rewrite
> is attached at the end.
I agree that the function is much simpler without it.
> + /* find the second token on the line */
> + cp = strchr(line->buf, ' ');
> + if (!cp)
> + return;
> + cp++;
> + ep = strchr(cp, ' ');
> + if (!ep)
> + return;
Would we ever see "cmd sha1" without a third token? If so, we'd want:
ep = strchrnul(cp, ' ');
-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 5/6] bundle: don't leak an fd in case of early return
Jeff King writes:
>> +if (compute_and_write_prerequisites(bundle_fd, &revs, argc, argv)) {
>> +if (!bundle_to_stdout)
>> +close(bundle_fd);
>> return -1;
>> +}
>
> Makes sense. Should we also be rolling back the lock file? It happens
> automatically at program exit, of course, but we are in library code
> here that should not rely on that.
Yeah, I agree. I suspect that the original wasn't even meant to be
used in a "library-ish" fashion, but as long as we are adding this
close(), we should also be doing the rollback.
Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/6] notes: don't leak memory in git_config_get_notes_strategy
Jeff King writes: > I don't think this is wrong, but would it perhaps be simpler to call > git_config_get_value() in the first place, which does not make a copy of > the string? Yup, I agree. -- 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/6] path.c: allocate enough memory for string
Stefan Beller writes:
> On Tue, Mar 29, 2016 at 5:57 PM, Eric Sunshine
> wrote:
>> On Tue, Mar 29, 2016 at 8:38 PM, Stefan Beller wrote:
>>> `strlen` returns the length of a string without the terminating null byte.
>>> To make sure enough memory is allocated we need to pass `strlen(..) + 1`
>>> to the allocation function.
>>>
>>> Signed-off-by: Stefan Beller
>>> ---
>>> diff --git a/path.c b/path.c
>>> @@ -155,7 +155,7 @@ static struct trie *make_trie_node(const char *key,
>>> void *value)
>>> struct trie *new_node = xcalloc(1, sizeof(*new_node));
>>> new_node->len = strlen(key);
>>> if (new_node->len) {
>>> - new_node->contents = xmalloc(new_node->len);
>>> + new_node->contents = xmalloc(new_node->len + 1);
>>> memcpy(new_node->contents, key, new_node->len);
>>
>> Huh? This is a trie. It never accesses 'contents' as a NUL-terminated
>> string. Plus, no NUL is ever even copied, thus this is just
>> overallocating. How is this an improvement?
>
> By using strlen, I assumed it was a standard C string.
> I missed that, though.
You took hint from a wrong place. You are auditing the destination
buffer, so the correct place to take hint from is the memcpy() that
touches the destination.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git-apply does not work in a sub-directory of a Git repository
Duy Nguyen writes: > But your suggestion is good and I can't think of any better. We could > introduce pathspec as ftiler after "--", but it does not look elegant, > and it overlaps with --include/--exclude. I was imagining that we would allow the magic pathspec syntax used in --include/--exclude command line option parameter. Nobody sane uses glob special characters in their pathnames and those that do deserve whatever breakage that comes to them. > Perhaps we can start to warn people if --include is specified but has > no effect for a cycle or two, then we can do as you suggested? I do not think I'd be against going in that direction. -- 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
[PATCHv2 1/4] notes: don't leak memory in git_config_get_notes_strategy
`value` is just a temporary scratchpad, so we need to make sure it doesn't
leak. It is xstrdup'd in `git_config_get_string` and
`parse_notes_merge_strategy` just compares the string against predefined
values, so no need to keep it around longer. Make `value` non-const to
avoid the cast in the free.
Signed-off-by: Stefan Beller
---
builtin/notes.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/builtin/notes.c b/builtin/notes.c
index ed6f222..6fd058d 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -744,13 +744,14 @@ static int merge_commit(struct notes_merge_options *o)
static int git_config_get_notes_strategy(const char *key,
enum notes_merge_strategy *strategy)
{
- const char *value;
+ char *value;
- if (git_config_get_string_const(key, &value))
+ if (git_config_get_string(key, &value))
return 1;
if (parse_notes_merge_strategy(value, strategy))
git_die_config(key, "unknown notes merge strategy %s", value);
+ free(value);
return 0;
}
--
2.8.0.2.gb331331
--
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
[PATCHv2 2/4] abbrev_sha1_in_line: don't leak memory
`split` is of type `struct strbuf **`, which we have a dedicated free function for, which takes care of freeing all related memory. Helped-by: Eric Sunshine Signed-off-by: Stefan Beller --- wt-status.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/wt-status.c b/wt-status.c index ef74864..1ea2ebe 100644 --- a/wt-status.c +++ b/wt-status.c @@ -1063,9 +1063,7 @@ static void abbrev_sha1_in_line(struct strbuf *line) strbuf_addf(line, "%s", split[i]->buf); } } - for (i = 0; split[i]; i++) - strbuf_release(split[i]); - + strbuf_list_free(split); } static void read_rebase_todolist(const char *fname, struct string_list *lines) -- 2.8.0.2.gb331331 -- 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
[PATCHv2 3/4] bundle: don't leak an fd in case of early return
In successful operation `write_pack_data` will close the `bundle_fd`,
but when we exit early, we need to take care of the file descriptor
as well as the lock file ourselves. The lock file may be deleted at the
end of running the program, but we are in library code, so we should
not rely on that.
Helped-by: Jeff King
Signed-off-by: Stefan Beller
---
bundle.c | 23 +--
1 file changed, 17 insertions(+), 6 deletions(-)
diff --git a/bundle.c b/bundle.c
index 506ac49..fbc8593 100644
--- a/bundle.c
+++ b/bundle.c
@@ -407,6 +407,7 @@ int create_bundle(struct bundle_header *header, const char
*path,
int bundle_to_stdout;
int ref_count = 0;
struct rev_info revs;
+ int ret = -1;
bundle_to_stdout = !strcmp(path, "-");
if (bundle_to_stdout)
@@ -435,30 +436,40 @@ int create_bundle(struct bundle_header *header, const
char *path,
/* write prerequisites */
if (compute_and_write_prerequisites(bundle_fd, &revs, argc, argv))
- return -1;
+ goto err;
argc = setup_revisions(argc, argv, &revs, NULL);
- if (argc > 1)
- return error(_("unrecognized argument: %s"), argv[1]);
+ if (argc > 1) {
+ ret = error(_("unrecognized argument: %s"), argv[1]);
+ goto err;
+ }
object_array_remove_duplicates(&revs.pending);
ref_count = write_bundle_refs(bundle_fd, &revs);
if (!ref_count)
die(_("Refusing to create empty bundle."));
- else if (ref_count < 0)
- return -1;
+ else if (ref_count < 0) {
+ if (!bundle_to_stdout)
+ close(bundle_fd);
+ goto err;
+ }
/* write pack */
if (write_pack_data(bundle_fd, &revs))
- return -1;
+ goto err;
if (!bundle_to_stdout) {
if (commit_lock_file(&lock))
die_errno(_("cannot create '%s'"), path);
}
return 0;
+err:
+ if (!bundle_to_stdout)
+ close(bundle_fd);
+ rollback_lock_file(&lock);
+ return ret;
}
int unbundle(struct bundle_header *header, int bundle_fd, int flags)
--
2.8.0.2.gb331331
--
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
[PATCHv2 4/4] credential-cache, send_request: close fd when done
No need to keep it open any further. Signed-off-by: Stefan Beller --- credential-cache.c | 1 + 1 file changed, 1 insertion(+) diff --git a/credential-cache.c b/credential-cache.c index f4afdc6..86e21de 100644 --- a/credential-cache.c +++ b/credential-cache.c @@ -32,6 +32,7 @@ static int send_request(const char *socket, const struct strbuf *out) write_or_die(1, in, r); got_data = 1; } + close(fd); return got_data; } -- 2.8.0.2.gb331331 -- 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
[PATCHv2 0/4] Some cleanups
v2: Thanks Eric, Jeff, Junio for discussion, I picked up all hints and here is a v2. * drop the overallocation patches (1&2) * use git_config_get_string instead of its _const equivalent, such that we don't need a cast when freeing in git_config_get_notes_strategy * Use strbuf_list_free instead of cooking our own. * have a dedicated error exit path in bundle.c, create_bundle v1: One of my first patches to Git were cleanup patches, and I fell back to my old pattern here, while thinking on how to write better commit messages for the submodule bugfixes I currently have in flight. Just some one liners to not leak memory or file descriptors. They are bundled as a series, but no patch relies on any predessor. This applies on v2.8.0. Thanks, Stefan Stefan Beller (4): notes: don't leak memory in git_config_get_notes_strategy abbrev_sha1_in_line: don't leak memory bundle: don't leak an fd in case of early return credential-cache, send_request: close fd when done builtin/notes.c| 5 +++-- bundle.c | 23 +-- credential-cache.c | 1 + wt-status.c| 4 +--- 4 files changed, 22 insertions(+), 11 deletions(-) -- 2.8.0.2.gb331331 -- 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 4/6] abbrev_sha1_in_line: don't leak memory
Jeff King writes:
> On Tue, Mar 29, 2016 at 09:30:38PM -0400, Eric Sunshine wrote:
>
>> The implementation of strbuf_list_free() is this:
>>
>> struct strbuf **s = sbs;
>> while (*s) {
>> strbuf_release(*s);
>> free(*s++);
>> }
>> free(sbs);
>>
>> which means that wt-status.c is leaking not only 'split', but also
>> each element of split[], right?
>
> Yeah, I didn't notice that, but I think you're right.
Correct.
I suspect that we would be better off in the longer term if
we made conscious effort to deprecate and eradicate the use
of strbuf_split* API functions. They are easy to write
crappy code with, inefficient, and error prone. You would
rarely need to have N resulting pieces as strbufs to be
easily manipulatable [*1*].
The function can be written by not using the "split and then
rebuild" pattern, perhaps like so, and the result is even
easier to read and understand, I would say. A sample rewrite
is attached at the end.
[Footnote]
*1* I wouldn't be this harsh if the function were to fill an
array of pointers or offsets into the original string.
wt-status.c | 43 +++
1 file changed, 19 insertions(+), 24 deletions(-)
diff --git a/wt-status.c b/wt-status.c
index ef74864..4886c35 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1037,35 +1037,30 @@ static int split_commit_in_progress(struct wt_status *s)
*/
static void abbrev_sha1_in_line(struct strbuf *line)
{
- struct strbuf **split;
- int i;
+ const char *cp, *ep, *abbrev;
+ unsigned char sha1[20];
- if (starts_with(line->buf, "exec ") ||
- starts_with(line->buf, "x "))
+ /* the oddball "exec" does not have 40-hex as the second token */
+ if (starts_with(line->buf, "exec ") || starts_with(line->buf, "x "))
return;
- split = strbuf_split_max(line, ' ', 3);
- if (split[0] && split[1]) {
- unsigned char sha1[20];
- const char *abbrev;
+ /* find the second token on the line */
+ cp = strchr(line->buf, ' ');
+ if (!cp)
+ return;
+ cp++;
+ ep = strchr(cp, ' ');
+ if (!ep)
+ return;
- /*
-* strbuf_split_max left a space. Trim it and re-add
-* it after abbreviation.
-*/
- strbuf_trim(split[1]);
- if (!get_sha1(split[1]->buf, sha1)) {
- abbrev = find_unique_abbrev(sha1, DEFAULT_ABBREV);
- strbuf_reset(split[1]);
- strbuf_addf(split[1], "%s ", abbrev);
- strbuf_reset(line);
- for (i = 0; split[i]; i++)
- strbuf_addf(line, "%s", split[i]->buf);
- }
- }
- for (i = 0; split[i]; i++)
- strbuf_release(split[i]);
+ /* is it 40-hex? */
+ if (ep - cp != GIT_SHA1_HEXSZ || get_sha1_hex(cp, sha1))
+ return;
+ /* replace it with its abbreviation */
+ abbrev = find_unique_abbrev(sha1, DEFAULT_ABBREV);
+ strbuf_splice(line, cp - line->buf, GIT_SHA1_HEXSZ,
+ abbrev, strlen(abbrev));
}
static void read_rebase_todolist(const char *fname, struct string_list *lines)
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git show -m with a parent number
Anatoly Borodin writes:
> It's not like `git diff X^2 X` is a big problem, but too much of a
> copypaste.
Brace expansion helps a bit: git diff X{^2,}
Andreas.
--
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
--
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/6] path.c: allocate enough memory for string
On Tue, Mar 29, 2016 at 5:57 PM, Eric Sunshine wrote:
> On Tue, Mar 29, 2016 at 8:38 PM, Stefan Beller wrote:
>> `strlen` returns the length of a string without the terminating null byte.
>> To make sure enough memory is allocated we need to pass `strlen(..) + 1`
>> to the allocation function.
>>
>> Signed-off-by: Stefan Beller
>> ---
>> diff --git a/path.c b/path.c
>> @@ -155,7 +155,7 @@ static struct trie *make_trie_node(const char *key, void
>> *value)
>> struct trie *new_node = xcalloc(1, sizeof(*new_node));
>> new_node->len = strlen(key);
>> if (new_node->len) {
>> - new_node->contents = xmalloc(new_node->len);
>> + new_node->contents = xmalloc(new_node->len + 1);
>> memcpy(new_node->contents, key, new_node->len);
>
> Huh? This is a trie. It never accesses 'contents' as a NUL-terminated
> string. Plus, no NUL is ever even copied, thus this is just
> overallocating. How is this an improvement?
By using strlen, I assumed it was a standard C string.
I missed that, though.
>
>> }
>> new_node->value = value;
>> --
--
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] `git submodule update --init --recursive` stores gitdir in full path into `.git` of nested submodules
On Wed, Mar 30, 2016 at 2:03 AM, Norio Nomura wrote: > Hi, > > `git submodule update --init --recursive` stores `gitdir` in full path into > `.git` of nested submodules. > So, working directory is not portable to another directory. Are you reporting a regression bug? (Is that a new thing or has it always been that way and you just discover that it is unfortunate?) Which versions did you test with? > > On following example, `Carthage/Checkouts/Quick/Externals/Nimble/` is nested > submodule and `Carthage/Checkouts/Quick/Externals/Nimble/.git` contains full > path. > https://gist.github.com/norio-nomura/17ce4bdf0151185e77d9b1fcfb5a469d > > Thanks, > -- > Norio Nomura @norio_nomura > > -- > To unsubscribe from this list: send the line "unsubscribe git" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC/GSOC] Git Beginner | Warnings for potentially destructive commands (v2)
Updated the warning messages, and added warnings for two more commands
(`checkout --` and `stash clear`). The list is now:
$ git rebase
$ git reset --hard
$ git clean -f
$ git gc --prune=now --aggressive
$ git push -f
$ git push remote [+/:]
$ git branch -D
$ git checkout [-p] [] --
$ git stash clear
Looking forward to your comments.
Thanks and regards,
Sidhant Sharma
---
$ ggit rebase
[WARNING] You are about to rebase your commits in $TOPIC_BRANCH onto the
$BASE_BRANCH, which will essentially replay the work done in $TOPIC_BRANCH
since last merge onto $BASE_BRANCH.
For instance, assume the following history exists and the current branch is
master:
o---o---A---B master
\
X---Y topic
After rebasing, the history would be:
o---o---A---B master
\
X'---Y' topic
where X' and Y' are the commits making changes identical to those made by X and
Y respectively.
Rebasing is not usually problematic except in cases when you are rebasing
commits that exist outside your repository (such as on a remote or on someone
else's computer).
$ ggit reset --hard
Resetting to
[WARNING] You are about to hard reset the current HEAD ($CURRENT_BRANCH) to a
previous commit in history, possibly in another branch. This will discard all
changes make thereafter.
For instance, assume the following history exists and the current branch is
master:
o---o---A---B---C---D---E master
After resetting 3 commits:
o---o---A---B master
The commits C, D and E and the changes made by them will be lost.
Note that if you make commits on top of B, you would have rewritten the history
and would have trouble restoring it easily.
You can undo this action by resetting the HEAD to the last commit you were on,
which can be seen by running `git reflog`. The first entry (HEAD{1}) points to
the current HEAD location, second entry (HEAD{1}) points to the last position of
your HEAD and so on.
If you want to reset while retaining the changes made since, use --soft instead
of --hard. This will reset without discarding the changes made in the previous
commits.
$ ggit push --force
Pushing changes to $REMOTE/$BRANCH
[WARNING] You are about to purge commits from the $REMOTE/$BRANCH branch and
overwrite it's history to match yours.
For instance, assume the following history exists where 'origin' is a configured
remote and the current branch is master:
o---o---o---A---B origin/master
\
X---Y---Z your master
After force push:
o---o---o---X---Y---Z origin/master and your master
Commit A and B will be gone. If other people have worked on top of A or B then
they won't be able to merge their changes easily.
To revert this, you would have to force push from a computer that has not yet
pulled the changes you pushed and still has commits A and B as they were in
origin/master previously.
$ ggit push : ( ggit push --delete )
Pushing changes to $REMOTE/$BRANCH
[WARNING] You are about delete a remote branch, which will result in the loss
of commits made on that branch. This may cause a problem if other people are
working on the branch you are deleting, as they would not be able to push or
merge their changes easily.
You can undo this by pushing the same branch to the remote with upstream flag,
that is, by running:
`$ ggit push -u $REMOTE $BRANCH`
This will work unless you have deleted the branch locally as well. In that case,
you need to restore it first.
$ ggit push +
( or ggit push +: )
Pushing changes to $REMOTE/$BRANCH
[WARNING] You are attempting to push changes from $BASE_BRANCH to $TARGET_BRANCH
while allowing non-fast-forward updates. This can leave unreferenced commits
dangling in the origin repository.
For instance, assume the following history exists where 'origin' is a configured
remote and the current branch is master:
o---o---A---B origin/master
\
X---Y---Z your master
State after forced push:
o---o---A---B(unnamed branch)
\
X---Y---Z origin/master and your master
Commits A and B would no longer belong to a branch with a symbolic name, and so
would be unreachable. Also, people who have based their work on A or B would
have to either merge or rebase after you have pushed.
If you wish to keep both the changesets (A,B and X,Y,Z), you may want to use
`merge` or `rebase`.
$ ggit branch -D
( or ggit branch -d -f )
Deleting branch $TARGET_BRANCH
[WARNING] You are about to force delete the $TARGET_BRANCH branch. This will
cause git to delete the branch even if it has not been merged, which may result
in loss of commits. Unless you are confident the branch has been merged, use
`-d` or `--delete` without `--force` which will warn you if the branch has not
been merged.
You can restore a forcefully deleted branch by running:
`$ git branch `
where is the name of the branch you wish to restore and is
the last commit made to the branch you want to res
Re: [PATCH v3] t7012: Implement test for git-checkout
On Wed, Mar 30, 2016 at 9:29 AM, Chhatoi Pritam Baral wrote: > Forgot to mention in the previous message, this is a microproject > for my GSoC '16 application. > > (Is that redundant to mention after v1 and v2? ) Readers of v1 knew this since you mentioned with that version, at least. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] clone: respect configured fetch respecs during initial fetch
Conceptually 'git clone' should behave as if the following commands
were run:
git init
git config ... # set default configuration and origin remote
git fetch
git checkout # unless '--bare' is given
However, that initial 'git fetch' behaves differently from any
subsequent fetches, because it takes only the default fetch refspec
into account and ignores all other fetch refspecs that might have
been explicitly specified on the command line (e.g. 'git -c
remote.origin.fetch= clone' or 'git clone -c ...').
Check whether there are any fetch refspecs configured for the origin
remote and take all of them into account during the initial fetch as
well.
Signed-off-by: SZEDER Gábor
---
Changes since previous (RFC) version:
- new commit message
- additional configured fetch refspecs are taken into account with
'--single-branch' as well
builtin/clone.c | 36
t/t5708-clone-config.sh | 24
2 files changed, 52 insertions(+), 8 deletions(-)
diff --git a/builtin/clone.c b/builtin/clone.c
index 661639255c56..5e2d2c21e456 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -515,7 +515,7 @@ static struct ref *find_remote_branch(const struct ref
*refs, const char *branch
}
static struct ref *wanted_peer_refs(const struct ref *refs,
- struct refspec *refspec)
+ struct refspec *refspec, unsigned int refspec_count)
{
struct ref *head = copy_ref(find_ref_by_name(refs, "HEAD"));
struct ref *local_refs = head;
@@ -536,13 +536,18 @@ static struct ref *wanted_peer_refs(const struct ref
*refs,
warning(_("Could not find remote branch %s to clone."),
option_branch);
else {
- get_fetch_map(remote_head, refspec, &tail, 0);
+ unsigned int i;
+ for (i = 0; i < refspec_count; i++)
+ get_fetch_map(remote_head, &refspec[i], &tail,
0);
/* if --branch=tag, pull the requested tag explicitly */
get_fetch_map(remote_head, tag_refspec, &tail, 0);
}
- } else
- get_fetch_map(refs, refspec, &tail, 0);
+ } else {
+ unsigned int i;
+ for (i = 0; i < refspec_count; i++)
+ get_fetch_map(refs, &refspec[i], &tail, 0);
+ }
if (!option_mirror && !option_single_branch)
get_fetch_map(refs, tag_refspec, &tail, 0);
@@ -840,7 +845,9 @@ int cmd_clone(int argc, const char **argv, const char
*prefix)
int err = 0, complete_refs_before_fetch = 1;
struct refspec *refspec;
- const char *fetch_pattern;
+ unsigned int refspec_count = 1;
+ const char **fetch_patterns;
+ const struct string_list *config_fetch_patterns;
packet_trace_identity("clone");
argc = parse_options(argc, argv, prefix, builtin_clone_options,
@@ -967,9 +974,21 @@ int cmd_clone(int argc, const char **argv, const char
*prefix)
if (option_reference.nr)
setup_reference();
- fetch_pattern = value.buf;
- refspec = parse_fetch_refspec(1, &fetch_pattern);
+ strbuf_addf(&key, "remote.%s.fetch", option_origin);
+ config_fetch_patterns = git_config_get_value_multi(key.buf);
+ if (config_fetch_patterns)
+ refspec_count = 1 + config_fetch_patterns->nr;
+ fetch_patterns = xcalloc(refspec_count, sizeof(*fetch_patterns));
+ fetch_patterns[0] = value.buf;
+ if (config_fetch_patterns) {
+ struct string_list_item *fp;
+ unsigned int i = 1;
+ for_each_string_list_item(fp, config_fetch_patterns)
+ fetch_patterns[i++] = fp->string;
+ }
+ refspec = parse_fetch_refspec(refspec_count, fetch_patterns);
+ strbuf_reset(&key);
strbuf_reset(&value);
remote = remote_get(option_origin);
@@ -1013,7 +1032,7 @@ int cmd_clone(int argc, const char **argv, const char
*prefix)
refs = transport_get_remote_refs(transport);
if (refs) {
- mapped_refs = wanted_peer_refs(refs, refspec);
+ mapped_refs = wanted_peer_refs(refs, refspec, refspec_count);
/*
* transport_get_remote_refs() may return refs with null sha-1
* in mapped_refs (see struct transport->get_refs_list
@@ -1094,6 +1113,7 @@ int cmd_clone(int argc, const char **argv, const char
*prefix)
strbuf_release(&value);
junk_mode = JUNK_LEAVE_ALL;
+ free(fetch_patterns);
free(refspec);
return err;
}
diff --git a/t/t5708-clone-config.sh b/t/t5708-clone-config.sh
index 27d730c0a720..136a8611c7f3 100755
--- a/t/t5708-clone-config.sh
+++ b/t/t5708-clone-config.sh
@@ -37,4 +37,28 @@ test_expect_success 'clone -c config is available during
c
[PATCH] for-each-ref: fix description of '--contains' in manpage
'git for-each-ref's manpage says that '--contains' only lists tags, but it lists all kinds of refs. Signed-off-by: SZEDER Gábor --- Documentation/git-for-each-ref.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt index 012e8f9a080d..c52578bb87cc 100644 --- a/Documentation/git-for-each-ref.txt +++ b/Documentation/git-for-each-ref.txt @@ -76,7 +76,7 @@ OPTIONS specified commit (HEAD if not specified). --contains []:: - Only list tags which contain the specified commit (HEAD if not + Only list refs which contain the specified commit (HEAD if not specified). FIELD NAMES -- 2.8.0.46.gb821760 -- 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] t7012: Implement test for git-checkout
Forgot to mention in the previous message, this is a microproject for my GSoC '16 application. (Is that redundant to mention after v1 and v2? ) On 03/26/2016 10:07 PM, Chhatoi Pritam Baral wrote: > Previously a TODO; add a test for git-checkout skipping a > file with the skip-worktree bit set. > > Signed-off-by: Chhatoi Pritam Baral > --- > > Replaced test_must_fail around grep with '!', as suggested by Eric. > > t/t7012-skip-worktree-writing.sh | 12 +++- > 1 file changed, 11 insertions(+), 1 deletion(-) > > diff --git a/t/t7012-skip-worktree-writing.sh > b/t/t7012-skip-worktree-writing.sh > index 9ceaa40..276f076 100755 > --- a/t/t7012-skip-worktree-writing.sh > +++ b/t/t7012-skip-worktree-writing.sh > @@ -141,6 +141,16 @@ test_expect_success 'git-clean, dirty case' ' > #TODO test_expect_failure 'git-apply removes file' false > #TODO test_expect_failure 'git-mv to skip-worktree' false > #TODO test_expect_failure 'git-mv from skip-worktree' false > -#TODO test_expect_failure 'git-checkout' false > + > +test_expect_success 'git-checkout ignores skip-worktree file' ' > + echo >1 && > + git commit -m "Add files" && > + echo dirty >1 && > + echo dirty >2 && > + git update-index --skip-worktree 1 && > + git checkout -- . && > + grep -q dirty 1 && > + ! grep -q dirty 2 > +' > test_done -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git show -m with a parent number
Hi! Jeff King wrote: > For the first one, showing all diffs is what I want. For the second, it > only makes sense to for the first parent case, as following other > parents would zig-zag through history. Lucky you! :) You probably don't need to inspect 9 month old ex-svn branches with sync (i.e. 'trunk'->'feature') merges *-...-*-...-*-...-*-...-*---trunk \ \ \ \ / *-...-*-...-*-...-*-...-*---feature (Not to forget some funny legacy inter-feature merges.) It's not like `git diff X^2 X` is a big problem, but too much of a copypaste. The other thing that bugs me is that you can easily `git cherry-pick -m 2 X`, but to see the diff that you are going to apply is not that trivial. > But perhaps you have some other use case in mind. In cases like these, I > think it is a good idea to implement the feature, and run with it for a > while, seeing how it can be used. And then if it proves useful, post the > patch to the list describing your experiences. I'll try. BTW, should it be like '-m[=parent]' for consistency, or '-m [parent]' is ok? PS Another idea: '-m parent' makes sence in a normal, 2-branch merge. But what to do in a case of an octopus merge? In a normal case I can treat '-m 2' as 'the diff regarding the second parent', but also as 'the changes contributed by the first parent (plus "evil")'. But with 3 and more branches '-m 3' means 'the changes from 1 and 2'. Would it be possible to get only the contribution from the second or third parent in this case? Yeah, I know, there is `git diff parent1...parent3` etc, but not all the changes from parent3 will always get to the merge commit. -- Mit freundlichen Grüßen, Anatoly Borodin -- 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: [GSoC] A late proposal: a modern send-email
Hi, On Wed, 30 Mar 2016, Ævar Arnfjörð Bjarmason wrote: > Correct me if I'm wrong but don't we basically have 4 kinds of users > using git-send-email: > > 1) Those who get it from a binary Windows package (is it even packaged there?) It is. And reportedly working fine. But in the pre-MSYS2 times it was a major pain in the butt due to the dependencies. Ciao, Dscho
Re: [PATCH 2/2 V3] MSVC: Use shipped headers instead of fallback definitions
Hi Sven, On Wed, 30 Mar 2016, Sven Strickroth wrote: > VS2010 comes with stdint.h [1] > VS2013 comes with inttypes.h [2] > > [1] https://stackoverflow.com/a/2628014/3906760 > [2] > https://blogs.msdn.microsoft.com/vcblog/2013/07/19/c99-library-support-in-visual-studio-2013/ > > Signed-off-by: Sven Strickroth Much, much, much better commit message (even if there is still room for a more pleasant read). ACK Dscho -- 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 V3] MSVC: Use shipped headers instead of fallback definitions
On 3/30/2016 13:37, Sven Strickroth wrote: VS2010 comes with stdint.h [1] VS2013 comes with inttypes.h [2] [1] https://stackoverflow.com/a/2628014/3906760 [2] https://blogs.msdn.microsoft.com/vcblog/2013/07/19/c99-library-support-in-visual-studio-2013/ Signed-off-by: Sven Strickroth ACK. Regards, Sebastian -- 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 V3] MSVC: Use shipped headers instead of fallback definitions
VS2010 comes with stdint.h [1] VS2013 comes with inttypes.h [2] [1] https://stackoverflow.com/a/2628014/3906760 [2] https://blogs.msdn.microsoft.com/vcblog/2013/07/19/c99-library-support-in-visual-studio-2013/ Signed-off-by: Sven Strickroth --- compat/mingw.h | 2 +- compat/vcbuild/include/unistd.h | 4 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/compat/mingw.h b/compat/mingw.h index 6b6d695..137f42e 100644 --- a/compat/mingw.h +++ b/compat/mingw.h @@ -415,7 +415,7 @@ int mingw_offset_1st_component(const char *path); extern void build_libgit_environment(void); extern const char *program_data_config(void); #define git_program_data_config program_data_config -#ifndef __MINGW64_VERSION_MAJOR +#if !defined(__MINGW64_VERSION_MAJOR) && (!defined(_MSC_VER) || _MSC_VER < 1800) #define PRIuMAX "I64u" #define PRId64 "I64d" #else diff --git a/compat/vcbuild/include/unistd.h b/compat/vcbuild/include/unistd.h index c65c2cd..b7cc48c 100644 --- a/compat/vcbuild/include/unistd.h +++ b/compat/vcbuild/include/unistd.h @@ -45,11 +45,15 @@ typedef unsigned long long uintmax_t; typedef int64_t off64_t; +#if !defined(_MSC_VER) || _MSC_VER < 1600 #define INTMAX_MIN _I64_MIN #define INTMAX_MAX _I64_MAX #define UINTMAX_MAX _UI64_MAX #define UINT32_MAX 0x /* 4294967295U */ +#else +#include +#endif #define STDIN_FILENO 0 #define STDOUT_FILENO 1 -- 2.7.4.windows.1 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 06/16] ref-filter: introduce format_ref_array_item()
To allow column display, we will need to first render the output in a
string list to allow print_columns() to compute the proper size of
each column before starting the actual output. Introduce the function
format_ref_array_item() that does the formatting of a ref_array_item
to an strbuf.
show_ref_array_item() is kept as a convenience wrapper around it which
obtains the strbuf and prints it the standard output.
Mentored-by: Christian Couder
Mentored-by: Matthieu Moy
Signed-off-by: Karthik Nayak
---
ref-filter.c | 16
ref-filter.h | 3 +++
2 files changed, 15 insertions(+), 4 deletions(-)
diff --git a/ref-filter.c b/ref-filter.c
index 7004bf0..3bb474f 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1813,10 +1813,10 @@ static void append_literal(const char *cp, const char
*ep, struct ref_formatting
}
}
-void show_ref_array_item(struct ref_array_item *info, const char *format, int
quote_style)
+void format_ref_array_item(struct ref_array_item *info, const char *format,
+ int quote_style, struct strbuf *final_buf)
{
const char *cp, *sp, *ep;
- struct strbuf *final_buf;
struct ref_formatting_state state = REF_FORMATTING_STATE_INIT;
state.quote_style = quote_style;
@@ -1846,9 +1846,17 @@ void show_ref_array_item(struct ref_array_item *info,
const char *format, int qu
}
if (state.stack->prev)
die(_("format: %%(end) atom missing"));
- final_buf = &state.stack->output;
- fwrite(final_buf->buf, 1, final_buf->len, stdout);
+ strbuf_addbuf(final_buf, &state.stack->output);
pop_stack_element(&state.stack);
+}
+
+void show_ref_array_item(struct ref_array_item *info, const char *format, int
quote_style)
+{
+ struct strbuf final_buf = STRBUF_INIT;
+
+ format_ref_array_item(info, format, quote_style, &final_buf);
+ fwrite(final_buf.buf, 1, final_buf.len, stdout);
+ strbuf_release(&final_buf);
putchar('\n');
}
diff --git a/ref-filter.h b/ref-filter.h
index 4aea594..0014b92 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -98,6 +98,9 @@ int parse_ref_filter_atom(const char *atom, const char *ep);
int verify_ref_format(const char *format);
/* Sort the given ref_array as per the ref_sorting provided */
void ref_array_sort(struct ref_sorting *sort, struct ref_array *array);
+/* Based on the given format and quote_style, fill the strbuf */
+void format_ref_array_item(struct ref_array_item *info, const char *format,
+ int quote_style, struct strbuf *final_buf);
/* Print the ref using the given format and quote_style */
void show_ref_array_item(struct ref_array_item *info, const char *format, int
quote_style);
/* Callback function for parsing the sort option */
--
2.7.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 v3 13/16] ref-filter: allow porcelain to translate messages in the output
Introduce setup_ref_filter_porcelain_msg() so that the messages used in
the atom %(upstream:track) can be translated if needed. This is needed
as we port branch.c to use ref-filter's printing API's.
Written-by: Matthieu Moy
Mentored-by: Christian Couder
Mentored-by: Matthieu Moy
Signed-off-by: Karthik Nayak
---
ref-filter.c | 28
ref-filter.h | 2 ++
2 files changed, 26 insertions(+), 4 deletions(-)
diff --git a/ref-filter.c b/ref-filter.c
index 73e0a7f..3435df1 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -15,6 +15,26 @@
#include "version.h"
#include "wt-status.h"
+static struct ref_msg {
+ const char *gone;
+ const char *ahead;
+ const char *behind;
+ const char *ahead_behind;
+} msgs = {
+ "gone",
+ "ahead %d",
+ "behind %d",
+ "ahead %d, behind %d"
+};
+
+void setup_ref_filter_porcelain_msg(void)
+{
+ msgs.gone = _("gone");
+ msgs.ahead = _("ahead %d");
+ msgs.behind = _("behind %d");
+ msgs.ahead_behind = _("ahead %d, behind %d");
+}
+
typedef enum { FIELD_STR, FIELD_ULONG, FIELD_TIME } cmp_type;
struct align {
@@ -1097,15 +1117,15 @@ static void fill_remote_ref_details(struct used_atom
*atom, const char *refname,
else if (atom->u.remote_ref.option == RR_TRACK) {
if (stat_tracking_info(branch, &num_ours,
&num_theirs, NULL)) {
- *s = xstrdup("gone");
+ *s = xstrdup(msgs.gone);
} else if (!num_ours && !num_theirs)
*s = "";
else if (!num_ours)
- *s = xstrfmt("behind %d", num_theirs);
+ *s = xstrfmt(msgs.behind, num_theirs);
else if (!num_theirs)
- *s = xstrfmt("ahead %d", num_ours);
+ *s = xstrfmt(msgs.ahead, num_ours);
else
- *s = xstrfmt("ahead %d, behind %d",
+ *s = xstrfmt(msgs.ahead_behind,
num_ours, num_theirs);
if (!atom->u.remote_ref.nobracket && *s[0]) {
const char *to_free = *s;
diff --git a/ref-filter.h b/ref-filter.h
index 0014b92..da17145 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -111,5 +111,7 @@ struct ref_sorting *ref_default_sorting(void);
int parse_opt_merge_filter(const struct option *opt, const char *arg, int
unset);
/* Get the current HEAD's description */
char *get_head_description(void);
+/* Set up translated strings in the output. */
+void setup_ref_filter_porcelain_msg(void);
#endif /* REF_FILTER_H */
--
2.7.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 v3 09/16] ref-filter: make "%(symref)" atom work with the ':short' modifier
The "%(symref)" atom doesn't work when used with the ':short' modifier
because we strictly match only 'symref' for setting the 'need_symref'
indicator. Fix this by using comparing with valid_atom rather than used_atom.
Add tests for %(symref) and %(symref:short) while we're here.
Helped-by: Junio C Hamano
Signed-off-by: Karthik Nayak
---
ref-filter.c| 2 +-
t/t6300-for-each-ref.sh | 24
2 files changed, 25 insertions(+), 1 deletion(-)
diff --git a/ref-filter.c b/ref-filter.c
index 8c97cdb..5c59b17 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -338,7 +338,7 @@ int parse_ref_filter_atom(const char *atom, const char *ep)
valid_atom[i].parser(&used_atom[at], arg);
if (*atom == '*')
need_tagged = 1;
- if (!strcmp(used_atom[at].name, "symref"))
+ if (!strcmp(valid_atom[i].name, "symref"))
need_symref = 1;
return at;
}
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 2c5f177..b06ea1c 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -38,6 +38,7 @@ test_atom() {
case "$1" in
head) ref=refs/heads/master ;;
tag) ref=refs/tags/testtag ;;
+sym) ref=refs/heads/sym ;;
*) ref=$1 ;;
esac
printf '%s\n' "$3" >expected
@@ -565,4 +566,27 @@ test_expect_success 'Verify sort with multiple keys' '
refs/tags/bogo refs/tags/master > actual &&
test_cmp expected actual
'
+
+test_expect_success 'Add symbolic ref for the following tests' '
+ git symbolic-ref refs/heads/sym refs/heads/master
+'
+
+cat >expected < actual &&
+ test_cmp expected actual
+'
+
+cat >expected < actual &&
+ test_cmp expected actual
+'
+
test_done
--
2.7.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 v3 11/16] ref-filter: introduce refname_atom_parser()
Introduce refname_atom_parser() which will parse the '%(refname)' atom
and store information into the 'used_atom' structure based on the
modifiers used along with the atom.
Signed-off-by: Karthik Nayak
---
ref-filter.c | 70 +---
1 file changed, 39 insertions(+), 31 deletions(-)
diff --git a/ref-filter.c b/ref-filter.c
index 7b35e4f..dc1e404 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -63,6 +63,10 @@ static struct used_atom {
unsigned int length;
} objectname;
enum { S_FULL, S_SHORT } symref;
+ struct {
+ enum { R_NORMAL, R_SHORT, R_STRIP } option;
+ unsigned int strip;
+ } refname;
} u;
} *used_atom;
static int used_atom_cnt, need_tagged, need_symref;
@@ -228,12 +232,27 @@ static void symref_atom_parser(struct used_atom *atom,
const char *arg)
die(_("unrecognized %%(symref) argument: %s"), arg);
}
+static void refname_atom_parser(struct used_atom *atom, const char *arg)
+{
+ if (!arg)
+ atom->u.refname.option = R_NORMAL;
+ else if (!strcmp(arg, "short"))
+ atom->u.refname.option = R_SHORT;
+ else if (skip_prefix(arg, "strip=", &arg)) {
+ atom->u.contents.option = R_STRIP;
+ if (strtoul_ui(arg, 10, &atom->u.refname.strip) ||
+ atom->u.refname.strip <= 0)
+ die(_("positive value expected refname:strip=%s"), arg);
+ } else
+ die(_("unrecognized %%(refname) argument: %s"), arg);
+}
+
static struct {
const char *name;
cmp_type cmp_type;
void (*parser)(struct used_atom *atom, const char *arg);
} valid_atom[] = {
- { "refname" },
+ { "refname", FIELD_STR, refname_atom_parser },
{ "objecttype" },
{ "objectsize", FIELD_ULONG },
{ "objectname", FIELD_STR, objectname_atom_parser },
@@ -1047,21 +1066,16 @@ static inline char *copy_advance(char *dst, const char
*src)
return dst;
}
-static const char *strip_ref_components(const char *refname, const char
*nr_arg)
+static const char *strip_ref_components(const char *refname, unsigned int len)
{
- char *end;
- long nr = strtol(nr_arg, &end, 10);
- long remaining = nr;
+ long remaining = len;
const char *start = refname;
- if (nr < 1 || *end != '\0')
- die(_(":strip= requires a positive integer argument"));
-
while (remaining) {
switch (*start++) {
case '\0':
- die(_("ref '%s' does not have %ld components to
:strip"),
- refname, nr);
+ die("ref '%s' does not have %ud components to :strip",
+ refname, len);
case '/':
remaining--;
break;
@@ -1153,6 +1167,18 @@ static const char *get_symref(struct used_atom *atom,
struct ref_array_item *ref
return ref->symref;
}
+static const char *get_refname(struct used_atom *atom, struct ref_array_item
*ref)
+{
+ if (ref->kind & FILTER_REFS_DETACHED_HEAD)
+ return get_head_description();
+ if (atom->u.refname.option == R_SHORT)
+ return shorten_unambiguous_ref(ref->refname,
warn_ambiguous_refs);
+ else if (atom->u.refname.option == R_STRIP)
+ return strip_ref_components(ref->refname,
atom->u.refname.strip);
+ else
+ return ref->refname;
+}
+
/*
* Parse the object referred by ref, and grab needed value.
*/
@@ -1181,7 +1207,6 @@ static void populate_value(struct ref_array_item *ref)
struct atom_value *v = &ref->value[i];
int deref = 0;
const char *refname;
- const char *formatp;
struct branch *branch = NULL;
v->handler = append_atom;
@@ -1192,11 +1217,9 @@ static void populate_value(struct ref_array_item *ref)
name++;
}
- if (starts_with(name, "refname")) {
- refname = ref->refname;
- if (ref->kind & FILTER_REFS_DETACHED_HEAD)
- refname = get_head_description();
- } else if (starts_with(name, "symref"))
+ if (starts_with(name, "refname"))
+ refname = get_refname(atom, ref);
+ else if (starts_with(name, "symref"))
refname = get_symref(atom, ref);
else if (starts_with(name, "upstream")) {
const char *branch_name;
@@ -1273,21 +1296,6 @@ static void populate_value(struct ref_array_item *ref)
} else
continue;
- formatp = strchr(name, ':');
- if (f
[PATCH v3 14/16] branch, tag: use porcelain output
Call ref-filter's setup_ref_filter_porcelain_msg() to enable translated messages for the %(upstream:tack) atom. Although branch.c doesn't currently use ref-filter's printing API's, this will ensure that when it does in the future patches, we do not need to worry about translation. Written-by: Matthieu Moy Mentored-by: Christian Couder Mentored-by: Matthieu Moy Signed-off-by: Karthik Nayak --- builtin/branch.c | 2 ++ builtin/tag.c| 2 ++ 2 files changed, 4 insertions(+) diff --git a/builtin/branch.c b/builtin/branch.c index 460f32f..8747d82 100644 --- a/builtin/branch.c +++ b/builtin/branch.c @@ -622,6 +622,8 @@ int cmd_branch(int argc, const char **argv, const char *prefix) OPT_END(), }; + setup_ref_filter_porcelain_msg(); + memset(&filter, 0, sizeof(filter)); filter.kind = FILTER_REFS_BRANCHES; filter.abbrev = -1; diff --git a/builtin/tag.c b/builtin/tag.c index 1705c94..82a04ed 100644 --- a/builtin/tag.c +++ b/builtin/tag.c @@ -373,6 +373,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix) OPT_END() }; + setup_ref_filter_porcelain_msg(); + git_config(git_tag_config, sorting_tail); memset(&opt, 0, sizeof(opt)); -- 2.7.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 v3 16/16] branch: implement '--format' option
Implement the '--format' option provided by 'ref-filter'. This lets the
user list branches as per desired format similar to the implementation
in 'git for-each-ref'.
Add tests and documentation for the same.
Mentored-by: Christian Couder
Mentored-by: Matthieu Moy
Signed-off-by: Karthik Nayak
---
Documentation/git-branch.txt | 7 ++-
builtin/branch.c | 14 +-
t/t3203-branch-output.sh | 12
3 files changed, 27 insertions(+), 6 deletions(-)
diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
index 4a7037f..8af132f 100644
--- a/Documentation/git-branch.txt
+++ b/Documentation/git-branch.txt
@@ -12,7 +12,7 @@ SYNOPSIS
[--list] [-v [--abbrev= | --no-abbrev]]
[--column[=] | --no-column]
[(--merged | --no-merged | --contains) []] [--sort=]
- [--points-at ] [...]
+ [--points-at ] [--format=] [...]
'git branch' [--set-upstream | --track | --no-track] [-l] [-f]
[]
'git branch' (--set-upstream-to= | -u ) []
'git branch' --unset-upstream []
@@ -246,6 +246,11 @@ start-point is either a local or remote-tracking branch.
--points-at ::
Only list branches of the given object.
+--format ::
+ A string that interpolates `%(fieldname)` from the object
+ pointed at by a ref being shown. The format is the same as
+ that of linkgit:git-for-each-ref[1].
+
Examples
diff --git a/builtin/branch.c b/builtin/branch.c
index 29cd206..fb05b39 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -27,6 +27,7 @@ static const char * const builtin_branch_usage[] = {
N_("git branch [] [-r] (-d | -D) ..."),
N_("git branch [] (-m | -M) [] "),
N_("git branch [] [-r | -a] [--points-at]"),
+ N_("git branch [] [-r | -a] [--format]"),
NULL
};
@@ -331,14 +332,14 @@ static char *build_format(struct ref_filter *filter, int
maxwidth, const char *r
return strbuf_detach(&fmt, NULL);
}
-static void print_ref_list(struct ref_filter *filter, struct ref_sorting
*sorting)
+static void print_ref_list(struct ref_filter *filter, struct ref_sorting
*sorting, const char *format)
{
int i;
struct ref_array array;
int maxwidth = 0;
const char *remote_prefix = "";
struct strbuf out = STRBUF_INIT;
- char *format;
+ char *to_free = NULL;
/*
* If we are listing more than just remote branches,
@@ -355,7 +356,8 @@ static void print_ref_list(struct ref_filter *filter,
struct ref_sorting *sortin
if (filter->verbose)
maxwidth = calc_maxwidth(&array, strlen(remote_prefix));
- format = build_format(filter, maxwidth, remote_prefix);
+ if (!format)
+ format = to_free = build_format(filter, maxwidth,
remote_prefix);
verify_ref_format(format);
/*
@@ -383,7 +385,7 @@ static void print_ref_list(struct ref_filter *filter,
struct ref_sorting *sortin
}
ref_array_clear(&array);
- free(format);
+ free(to_free);
}
static void rename_branch(const char *oldname, const char *newname, int force)
@@ -483,6 +485,7 @@ int cmd_branch(int argc, const char **argv, const char
*prefix)
enum branch_track track;
struct ref_filter filter;
static struct ref_sorting *sorting = NULL, **sorting_tail = &sorting;
+ const char *format = NULL;
struct option options[] = {
OPT_GROUP(N_("Generic options")),
@@ -523,6 +526,7 @@ int cmd_branch(int argc, const char **argv, const char
*prefix)
OPTION_CALLBACK, 0, "points-at", &filter.points_at,
N_("object"),
N_("print only branches of the object"), 0,
parse_opt_object_name
},
+ OPT_STRING( 0 , "format", &format, N_("format"), N_("format to
use for the output")),
OPT_END(),
};
@@ -583,7 +587,7 @@ int cmd_branch(int argc, const char **argv, const char
*prefix)
if ((filter.kind & FILTER_REFS_BRANCHES) && filter.detached)
filter.kind |= FILTER_REFS_DETACHED_HEAD;
filter.name_patterns = argv;
- print_ref_list(&filter, sorting);
+ print_ref_list(&filter, sorting, format);
print_columns(&output, colopts, NULL);
string_list_clear(&output, 0);
return 0;
diff --git a/t/t3203-branch-output.sh b/t/t3203-branch-output.sh
index 4261403..c33a3f3 100755
--- a/t/t3203-branch-output.sh
+++ b/t/t3203-branch-output.sh
@@ -184,4 +184,16 @@ test_expect_success 'ambiguous branch/tag not marked' '
test_cmp expect actual
'
+test_expect_success 'git branch --format option' '
+ cat >expect <<-\EOF &&
+ Refname is (HEAD detached from fromtag)
+ Refname is refs/heads/ambiguous
+ Refname is refs/heads/branch-one
+ Refname is refs/heads/branch-two
+ Refname is refs/heads/maste
[PATCH v3 15/16] branch: use ref-filter printing APIs
Port branch.c to use ref-filter APIs for printing. This clears out
most of the code used in branch.c for printing and replaces them with
calls made to the ref-filter library.
Introduce build_format() which gets the format required for printing
of refs. Make amendments to print_ref_list() to reflect these changes.
Change calc_maxwidth() to also account for the length of HEAD ref, by
calling ref-filter:get_head_discription().
Also change the test in t6040 to reflect the changes.
Mentored-by: Christian Couder
Mentored-by: Matthieu Moy
Signed-off-by: Karthik Nayak
---
builtin/branch.c | 226 ++-
t/t6040-tracking-info.sh | 2 +-
2 files changed, 66 insertions(+), 162 deletions(-)
diff --git a/builtin/branch.c b/builtin/branch.c
index 8747d82..29cd206 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -35,12 +35,12 @@ static unsigned char head_sha1[20];
static int branch_use_color = -1;
static char branch_colors[][COLOR_MAXLEN] = {
- GIT_COLOR_RESET,
- GIT_COLOR_NORMAL, /* PLAIN */
- GIT_COLOR_RED, /* REMOTE */
- GIT_COLOR_NORMAL, /* LOCAL */
- GIT_COLOR_GREEN,/* CURRENT */
- GIT_COLOR_BLUE, /* UPSTREAM */
+ "%(color:reset)",
+ "%(color:reset)", /* PLAIN */
+ "%(color:red)", /* REMOTE */
+ "%(color:reset)", /* LOCAL */
+ "%(color:green)", /* CURRENT */
+ "%(color:blue)",/* UPSTREAM */
};
enum color_branch {
BRANCH_COLOR_RESET = 0,
@@ -271,157 +271,6 @@ static int delete_branches(int argc, const char **argv,
int force, int kinds,
return(ret);
}
-static void fill_tracking_info(struct strbuf *stat, const char *branch_name,
- int show_upstream_ref)
-{
- int ours, theirs;
- char *ref = NULL;
- struct branch *branch = branch_get(branch_name);
- const char *upstream;
- struct strbuf fancy = STRBUF_INIT;
- int upstream_is_gone = 0;
- int added_decoration = 1;
-
- if (stat_tracking_info(branch, &ours, &theirs, &upstream) < 0) {
- if (!upstream)
- return;
- upstream_is_gone = 1;
- }
-
- if (show_upstream_ref) {
- ref = shorten_unambiguous_ref(upstream, 0);
- if (want_color(branch_use_color))
- strbuf_addf(&fancy, "%s%s%s",
- branch_get_color(BRANCH_COLOR_UPSTREAM),
- ref,
branch_get_color(BRANCH_COLOR_RESET));
- else
- strbuf_addstr(&fancy, ref);
- }
-
- if (upstream_is_gone) {
- if (show_upstream_ref)
- strbuf_addf(stat, _("[%s: gone]"), fancy.buf);
- else
- added_decoration = 0;
- } else if (!ours && !theirs) {
- if (show_upstream_ref)
- strbuf_addf(stat, _("[%s]"), fancy.buf);
- else
- added_decoration = 0;
- } else if (!ours) {
- if (show_upstream_ref)
- strbuf_addf(stat, _("[%s: behind %d]"), fancy.buf,
theirs);
- else
- strbuf_addf(stat, _("[behind %d]"), theirs);
-
- } else if (!theirs) {
- if (show_upstream_ref)
- strbuf_addf(stat, _("[%s: ahead %d]"), fancy.buf, ours);
- else
- strbuf_addf(stat, _("[ahead %d]"), ours);
- } else {
- if (show_upstream_ref)
- strbuf_addf(stat, _("[%s: ahead %d, behind %d]"),
- fancy.buf, ours, theirs);
- else
- strbuf_addf(stat, _("[ahead %d, behind %d]"),
- ours, theirs);
- }
- strbuf_release(&fancy);
- if (added_decoration)
- strbuf_addch(stat, ' ');
- free(ref);
-}
-
-static void add_verbose_info(struct strbuf *out, struct ref_array_item *item,
-struct ref_filter *filter, const char *refname)
-{
- struct strbuf subject = STRBUF_INIT, stat = STRBUF_INIT;
- const char *sub = _(" invalid ref ");
- struct commit *commit = item->commit;
-
- if (!parse_commit(commit)) {
- pp_commit_easy(CMIT_FMT_ONELINE, commit, &subject);
- sub = subject.buf;
- }
-
- if (item->kind == FILTER_REFS_BRANCHES)
- fill_tracking_info(&stat, refname, filter->verbose > 1);
-
- strbuf_addf(out, " %s %s%s",
- find_unique_abbrev(item->commit->object.oid.hash,
filter->abbrev),
- stat.buf, sub);
- strbuf_release(&stat);
- strbuf_release(&subject);
-}
-
-static void format_and_print_ref_item(struct ref_array_item *item, int
maxwidth,
-
[PATCH v3 00/16] port branch.c to use ref-filter's printing options
I kinda waited before sending this, since there was lot of discussions
happening regarding to GSOC16, didn't want to clutter the mailing list.
This is part of unification of the commands 'git tag -l, git branch -l
and git for-each-ref'. This ports over branch.c to use ref-filter's
printing options.
Initially posted here: $(gmane/279226). It was decided that this series
would follow up after refactoring ref-filter parsing mechanism, which
is now merged into master (9606218b32344c5c756f7c29349d3845ef60b80c).
v1 can be found here: $(gmane/288342)
v2 can be found here: $(gmane/288863)
Changes in this version:
1. Only Documentation and commit message changes as suggested by
Jacob in v2.
Thanks to Jacob for his suggestions on the previous iteration.
Karthik Nayak (16):
ref-filter: implement %(if), %(then), and %(else) atoms
ref-filter: include reference to 'used_atom' within 'atom_value'
ref-filter: implement %(if:equals=) and
%(if:notequals=)
ref-filter: modify "%(objectname:short)" to take length
ref-filter: move get_head_description() from branch.c
ref-filter: introduce format_ref_array_item()
ref-filter: make %(upstream:track) prints "[gone]" for invalid
upstreams
ref-filter: add support for %(upstream:track,nobracket)
ref-filter: make "%(symref)" atom work with the ':short' modifier
ref-filter: introduce symref_atom_parser()
ref-filter: introduce refname_atom_parser()
ref-filter: add support for %(refname:dir) and %(refname:base)
ref-filter: allow porcelain to translate messages in the output
branch, tag: use porcelain output
branch: use ref-filter printing APIs
branch: implement '--format' option
Documentation/git-branch.txt | 7 +-
Documentation/git-for-each-ref.txt | 63 +-
builtin/branch.c | 267 ++
builtin/tag.c | 2 +
ref-filter.c | 447 +++--
ref-filter.h | 7 +
t/t3203-branch-output.sh | 12 +
t/t6040-tracking-info.sh | 2 +-
t/t6300-for-each-ref.sh| 40 +++-
t/t6302-for-each-ref-filter.sh | 88
10 files changed, 657 insertions(+), 278 deletions(-)
Interdiff:
diff --git a/Documentation/git-for-each-ref.txt
b/Documentation/git-for-each-ref.txt
index 0d7d80f..578bbd1 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -109,10 +109,6 @@ objectsize::
objectname::
The object name (aka SHA-1).
For a non-ambiguous abbreviation of the object name append `:short`.
- For an abbreviation of the object name with desired length append
- `:short=`, where the minimum length is MINIMUM_ABBREV. The
- length may be exceeded to ensure unique object names.
-
upstream::
The name of a local ref which can be considered ``upstream''
@@ -120,11 +116,12 @@ upstream::
`refname` above. Additionally respects `:track` to show
"[ahead N, behind M]" and `:trackshort` to show the terse
version: ">" (ahead), "<" (behind), "<>" (ahead and behind),
- or "=" (in sync). `:track` also prints "[gone]" whenever
- unknown upstream ref is encountered. Append `:track,nobracket`
- to show tracking information without brackets (i.e "ahead N,
- behind M"). Has no effect if the ref does not have tracking
- information associated with it.
+ or "=" (in sync). Has no effect if the ref does not have
+ tracking information associated with it. `:track` also prints
+ "[gone]" whenever unknown upstream ref is encountered. Append
+ `:track,nobracket` to show tracking information without
+ brackets (i.e "ahead N, behind M"). Has no effect if the ref
+ does not have tracking information associated with it.
push::
The name of a local ref which represents the `@{push}` location
--
2.7.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 v3 12/16] ref-filter: add support for %(refname:dir) and %(refname:base)
Add the options `:dir` and `:base` to the %(refname) atom. The `:dir`
option gives the directory (the part after $GIT_DIR/) of the ref without
the refname. The `:base` option gives the base directory of the given
ref (i.e. the directory following $GIT_DIR/refs/).
Add tests and documentation for the same.
Signed-off-by: Karthik Nayak
---
Documentation/git-for-each-ref.txt | 4 +++-
ref-filter.c | 28 +---
t/t6300-for-each-ref.sh| 2 ++
3 files changed, 30 insertions(+), 4 deletions(-)
diff --git a/Documentation/git-for-each-ref.txt
b/Documentation/git-for-each-ref.txt
index be763c4..0d7d80f 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -96,7 +96,9 @@ refname::
slash-separated path components from the front of the refname
(e.g., `%(refname:strip=2)` turns `refs/tags/foo` into `foo`.
`` must be a positive integer. If a displayed ref has fewer
- components than ``, the command aborts with an error.
+ components than ``, the command aborts with an error. For the base
+ directory of the ref (i.e. foo in refs/foo/bar/boz) append
+ `:base`. For the entire directory path append `:dir`.
objecttype::
The type of the object (`blob`, `tree`, `commit`, `tag`).
diff --git a/ref-filter.c b/ref-filter.c
index dc1e404..73e0a7f 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -64,7 +64,7 @@ static struct used_atom {
} objectname;
enum { S_FULL, S_SHORT } symref;
struct {
- enum { R_NORMAL, R_SHORT, R_STRIP } option;
+ enum { R_BASE, R_DIR, R_NORMAL, R_SHORT, R_STRIP }
option;
unsigned int strip;
} refname;
} u;
@@ -243,7 +243,11 @@ static void refname_atom_parser(struct used_atom *atom,
const char *arg)
if (strtoul_ui(arg, 10, &atom->u.refname.strip) ||
atom->u.refname.strip <= 0)
die(_("positive value expected refname:strip=%s"), arg);
- } else
+ } else if (!strcmp(arg, "dir"))
+ atom->u.contents.option = R_DIR;
+ else if (!strcmp(arg, "base"))
+ atom->u.contents.option = R_BASE;
+ else
die(_("unrecognized %%(refname) argument: %s"), arg);
}
@@ -1175,7 +1179,25 @@ static const char *get_refname(struct used_atom *atom,
struct ref_array_item *re
return shorten_unambiguous_ref(ref->refname,
warn_ambiguous_refs);
else if (atom->u.refname.option == R_STRIP)
return strip_ref_components(ref->refname,
atom->u.refname.strip);
- else
+ else if (atom->u.refname.option == R_BASE) {
+ const char *sp, *ep;
+
+ if (skip_prefix(ref->refname, "refs/", &sp)) {
+ ep = strchr(sp, '/');
+ if (!ep)
+ return "";
+ return xstrndup(sp, ep - sp);
+ }
+ return "";
+ } else if (atom->u.refname.option == R_DIR) {
+ const char *sp, *ep;
+
+ sp = ref->refname;
+ ep = strrchr(sp, '/');
+ if (!ep)
+ return "";
+ return xstrndup(sp, ep - sp);
+ } else
return ref->refname;
}
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index b06ea1c..36d32d7 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -53,6 +53,8 @@ test_atom head refname refs/heads/master
test_atom head refname:short master
test_atom head refname:strip=1 heads/master
test_atom head refname:strip=2 master
+test_atom head refname:dir refs/heads
+test_atom head refname:base heads
test_atom head upstream refs/remotes/origin/master
test_atom head upstream:short origin/master
test_atom head push refs/remotes/myfork/master
--
2.7.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 v3 07/16] ref-filter: make %(upstream:track) prints "[gone]" for invalid upstreams
Borrowing from branch.c's implementation print "[gone]" whenever an
unknown upstream ref is encountered instead of just ignoring it.
This makes sure that when branch.c is ported over to using ref-filter
APIs for printing, this feature is not lost.
Make changes to t/t6300-for-each-ref.sh and
Documentation/git-for-each-ref.txt to reflect this change.
Mentored-by: Christian Couder
Mentored-by: Matthieu Moy
Helped-by : Jacob Keller
Signed-off-by: Karthik Nayak
---
Documentation/git-for-each-ref.txt | 3 ++-
ref-filter.c | 4 +++-
t/t6300-for-each-ref.sh| 2 +-
3 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/Documentation/git-for-each-ref.txt
b/Documentation/git-for-each-ref.txt
index d3223a2..85ac2a8 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -119,7 +119,8 @@ upstream::
"[ahead N, behind M]" and `:trackshort` to show the terse
version: ">" (ahead), "<" (behind), "<>" (ahead and behind),
or "=" (in sync). Has no effect if the ref does not have
- tracking information associated with it.
+ tracking information associated with it. `:track` also prints
+ "[gone]" whenever unknown upstream ref is encountered.
push::
The name of a local ref which represents the `@{push}` location
diff --git a/ref-filter.c b/ref-filter.c
index 3bb474f..4d7e0c0 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1049,8 +1049,10 @@ static void fill_remote_ref_details(struct used_atom
*atom, const char *refname,
*s = shorten_unambiguous_ref(refname, warn_ambiguous_refs);
else if (atom->u.remote_ref == RR_TRACK) {
if (stat_tracking_info(branch, &num_ours,
- &num_theirs, NULL))
+ &num_theirs, NULL)) {
+ *s = "[gone]";
return;
+ }
if (!num_ours && !num_theirs)
*s = "";
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 2be0a3f..a92b36f 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -382,7 +382,7 @@ test_expect_success 'Check that :track[short] cannot be
used with other atoms' '
test_expect_success 'Check that :track[short] works when upstream is invalid' '
cat >expected <<-\EOF &&
-
+ [gone]
EOF
test_when_finished "git config branch.master.merge refs/heads/master" &&
--
2.7.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 v3 05/16] ref-filter: move get_head_description() from branch.c
Move the implementation of get_head_description() from branch.c to
ref-filter. This gives a description of the HEAD ref if called. This
is used as the refname for the HEAD ref whenever the
FILTER_REFS_DETACHED_HEAD option is used. Make it public because we
need it to calculate the length of the HEAD refs description in
branch.c:calc_maxwidth() when we port branch.c to use ref-filter
APIs.
Mentored-by: Christian Couder
Mentored-by: Matthieu Moy
Signed-off-by: Karthik Nayak
---
builtin/branch.c | 31 ---
ref-filter.c | 38 --
ref-filter.h | 2 ++
3 files changed, 38 insertions(+), 33 deletions(-)
diff --git a/builtin/branch.c b/builtin/branch.c
index 7b45b6b..460f32f 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -355,37 +355,6 @@ static void add_verbose_info(struct strbuf *out, struct
ref_array_item *item,
strbuf_release(&subject);
}
-static char *get_head_description(void)
-{
- struct strbuf desc = STRBUF_INIT;
- struct wt_status_state state;
- memset(&state, 0, sizeof(state));
- wt_status_get_state(&state, 1);
- if (state.rebase_in_progress ||
- state.rebase_interactive_in_progress)
- strbuf_addf(&desc, _("(no branch, rebasing %s)"),
- state.branch);
- else if (state.bisect_in_progress)
- strbuf_addf(&desc, _("(no branch, bisect started on %s)"),
- state.branch);
- else if (state.detached_from) {
- /* TRANSLATORS: make sure these match _("HEAD detached at ")
- and _("HEAD detached from ") in wt-status.c */
- if (state.detached_at)
- strbuf_addf(&desc, _("(HEAD detached at %s)"),
- state.detached_from);
- else
- strbuf_addf(&desc, _("(HEAD detached from %s)"),
- state.detached_from);
- }
- else
- strbuf_addstr(&desc, _("(no branch)"));
- free(state.branch);
- free(state.onto);
- free(state.detached_from);
- return strbuf_detach(&desc, NULL);
-}
-
static void format_and_print_ref_item(struct ref_array_item *item, int
maxwidth,
struct ref_filter *filter, const char
*remote_prefix)
{
diff --git a/ref-filter.c b/ref-filter.c
index 17f781d..7004bf0 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -13,6 +13,7 @@
#include "utf8.h"
#include "git-compat-util.h"
#include "version.h"
+#include "wt-status.h"
typedef enum { FIELD_STR, FIELD_ULONG, FIELD_TIME } cmp_type;
@@ -1077,6 +1078,37 @@ static void fill_remote_ref_details(struct used_atom
*atom, const char *refname,
*s = refname;
}
+char *get_head_description(void)
+{
+ struct strbuf desc = STRBUF_INIT;
+ struct wt_status_state state;
+ memset(&state, 0, sizeof(state));
+ wt_status_get_state(&state, 1);
+ if (state.rebase_in_progress ||
+ state.rebase_interactive_in_progress)
+ strbuf_addf(&desc, _("(no branch, rebasing %s)"),
+ state.branch);
+ else if (state.bisect_in_progress)
+ strbuf_addf(&desc, _("(no branch, bisect started on %s)"),
+ state.branch);
+ else if (state.detached_from) {
+ /* TRANSLATORS: make sure these match _("HEAD detached at ")
+ and _("HEAD detached from ") in wt-status.c */
+ if (state.detached_at)
+ strbuf_addf(&desc, _("(HEAD detached at %s)"),
+ state.detached_from);
+ else
+ strbuf_addf(&desc, _("(HEAD detached from %s)"),
+ state.detached_from);
+ }
+ else
+ strbuf_addstr(&desc, _("(no branch)"));
+ free(state.branch);
+ free(state.onto);
+ free(state.detached_from);
+ return strbuf_detach(&desc, NULL);
+}
+
/*
* Parse the object referred by ref, and grab needed value.
*/
@@ -1116,9 +1148,11 @@ static void populate_value(struct ref_array_item *ref)
name++;
}
- if (starts_with(name, "refname"))
+ if (starts_with(name, "refname")) {
refname = ref->refname;
- else if (starts_with(name, "symref"))
+ if (ref->kind & FILTER_REFS_DETACHED_HEAD)
+ refname = get_head_description();
+ } else if (starts_with(name, "symref"))
refname = ref->symref ? ref->symref : "";
else if (starts_with(name, "upstream")) {
const char *branch_name;
diff --git a/ref-filter.h b/ref-filter.h
index 14d435e..4aea594 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -106,5 +106,7 @@ int
[PATCH v3 03/16] ref-filter: implement %(if:equals=) and %(if:notequals=)
Implement %(if:equals=) wherein the if condition is only
satisfied if the value obtained between the %(if:...) and %(then) atom
is the same as the given ''.
Similarly, implement (if:notequals=) wherein the if condition
is only satisfied if the value obtained between the %(if:...) and
%(then) atom is differnt from the given ''.
This is done by introducing 'if_atom_parser()' which parses the given
%(if) atom and then stores the data in used_atom which is later passed
on to the used_atom of the %(then) atom, so that it can do the required
comparisons.
Add tests and Documentation for the same.
Mentored-by: Christian Couder
Mentored-by: Matthieu Moy
Signed-off-by: Karthik Nayak
---
Documentation/git-for-each-ref.txt | 3 +++
ref-filter.c | 43 +-
t/t6302-for-each-ref-filter.sh | 18
3 files changed, 59 insertions(+), 5 deletions(-)
diff --git a/Documentation/git-for-each-ref.txt
b/Documentation/git-for-each-ref.txt
index d048561..e1b1a66 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -152,6 +152,9 @@ if::
evaluating the string before %(then), this is useful when we
use the %(HEAD) atom which prints either "*" or " " and we
want to apply the 'if' condition only on the 'HEAD' ref.
+ Append ":equals=" or ":notequals=" to compare
+ the value between the %(if:...) and %(then) atoms with the
+ given string.
In addition to the above, for commit and tag objects, the header
field names (`tree`, `parent`, `object`, `type`, and `tag`) can
diff --git a/ref-filter.c b/ref-filter.c
index 12e646c..857a8b5 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -22,6 +22,8 @@ struct align {
};
struct if_then_else {
+ const char *if_equals,
+ *not_equals;
unsigned int then_atom_seen : 1,
else_atom_seen : 1,
condition_satisfied : 1;
@@ -49,6 +51,10 @@ static struct used_atom {
enum { C_BARE, C_BODY, C_BODY_DEP, C_LINES, C_SIG,
C_SUB } option;
unsigned int nlines;
} contents;
+ struct {
+ const char *if_equals,
+ *not_equals;
+ } if_then_else;
enum { O_FULL, O_SHORT } objectname;
} u;
} *used_atom;
@@ -169,6 +175,19 @@ static void align_atom_parser(struct used_atom *atom,
const char *arg)
string_list_clear(¶ms, 0);
}
+static void if_atom_parser(struct used_atom *atom, const char *arg)
+{
+ if (!arg)
+ return;
+ else if (skip_prefix(arg, "equals=", &atom->u.if_then_else.if_equals))
+;
+ else if (skip_prefix(arg, "notequals=",
&atom->u.if_then_else.not_equals))
+ ;
+ else
+ die(_("unrecognized %%(if) argument: %s"), arg);
+}
+
+
static struct {
const char *name;
cmp_type cmp_type;
@@ -209,7 +228,7 @@ static struct {
{ "color", FIELD_STR, color_atom_parser },
{ "align", FIELD_STR, align_atom_parser },
{ "end" },
- { "if" },
+ { "if", FIELD_STR, if_atom_parser },
{ "then" },
{ "else" },
};
@@ -410,6 +429,9 @@ static void if_atom_handler(struct atom_value *atomv,
struct ref_formatting_stat
struct ref_formatting_stack *new;
struct if_then_else *if_then_else = xcalloc(sizeof(struct
if_then_else), 1);
+ if_then_else->if_equals = atomv->atom->u.if_then_else.if_equals;
+ if_then_else->not_equals = atomv->atom->u.if_then_else.not_equals;
+
push_stack_element(&state->stack);
new = state->stack;
new->at_end = if_then_else_handler;
@@ -441,10 +463,17 @@ static void then_atom_handler(struct atom_value *atomv,
struct ref_formatting_st
die(_("format: %%(then) atom used after %%(else)"));
if_then_else->then_atom_seen = 1;
/*
-* If there exists non-empty string between the 'if' and
-* 'then' atom then the 'if' condition is satisfied.
+* If the 'equals' or 'notequals' attribute is used then
+* perform the required comparison. If not, only non-empty
+* strings satisfy the 'if' condition.
*/
- if (cur->output.len && !is_empty(cur->output.buf))
+ if (if_then_else->if_equals) {
+ if (!strcmp(if_then_else->if_equals, cur->output.buf))
+ if_then_else->condition_satisfied = 1;
+ } else if (if_then_else->not_equals) {
+ if (strcmp(if_then_else->not_equals, cur->output.buf))
+ if_then_else->condition_satisfied = 1;
+ } else if (cur->output.len && !is_empty(cur->output.buf))
if_then_else->condition_satisfied = 1;
strbuf_reset(&cur->output);
}
@@ -1137,7 +1166,11 @@ static void populate_value(struct ref_array_item *ref)
} else i
[PATCH v3 08/16] ref-filter: add support for %(upstream:track,nobracket)
Add support for %(upstream:track,nobracket) which will print the
tracking information without the brackets (i.e. "ahead N, behind M").
This is needed when we port branch.c to use ref-filter's printing APIs.
Add test and documentation for the same.
Mentored-by: Christian Couder
Mentored-by: Matthieu Moy
Signed-off-by: Karthik Nayak
---
Documentation/git-for-each-ref.txt | 8 +++--
ref-filter.c | 67 +-
t/t6300-for-each-ref.sh| 2 ++
3 files changed, 51 insertions(+), 26 deletions(-)
diff --git a/Documentation/git-for-each-ref.txt
b/Documentation/git-for-each-ref.txt
index 85ac2a8..be763c4 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -118,9 +118,11 @@ upstream::
`refname` above. Additionally respects `:track` to show
"[ahead N, behind M]" and `:trackshort` to show the terse
version: ">" (ahead), "<" (behind), "<>" (ahead and behind),
- or "=" (in sync). Has no effect if the ref does not have
- tracking information associated with it. `:track` also prints
- "[gone]" whenever unknown upstream ref is encountered.
+ or "=" (in sync). `:track` also prints "[gone]" whenever
+ unknown upstream ref is encountered. Append `:track,nobracket`
+ to show tracking information without brackets (i.e "ahead N,
+ behind M"). Has no effect if the ref does not have tracking
+ information associated with it.
push::
The name of a local ref which represents the `@{push}` location
diff --git a/ref-filter.c b/ref-filter.c
index 4d7e0c0..8c97cdb 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -46,8 +46,10 @@ static struct used_atom {
union {
char color[COLOR_MAXLEN];
struct align align;
- enum { RR_NORMAL, RR_SHORTEN, RR_TRACK, RR_TRACKSHORT }
- remote_ref;
+ struct {
+ enum { RR_NORMAL, RR_SHORTEN, RR_TRACK, RR_TRACKSHORT }
option;
+ unsigned int nobracket: 1;
+ } remote_ref;
struct {
enum { C_BARE, C_BODY, C_BODY_DEP, C_LINES, C_SIG,
C_SUB } option;
unsigned int nlines;
@@ -75,16 +77,33 @@ static void color_atom_parser(struct used_atom *atom, const
char *color_value)
static void remote_ref_atom_parser(struct used_atom *atom, const char *arg)
{
- if (!arg)
- atom->u.remote_ref = RR_NORMAL;
- else if (!strcmp(arg, "short"))
- atom->u.remote_ref = RR_SHORTEN;
- else if (!strcmp(arg, "track"))
- atom->u.remote_ref = RR_TRACK;
- else if (!strcmp(arg, "trackshort"))
- atom->u.remote_ref = RR_TRACKSHORT;
- else
- die(_("unrecognized format: %%(%s)"), atom->name);
+ struct string_list params = STRING_LIST_INIT_DUP;
+ int i;
+
+ if (!arg) {
+ atom->u.remote_ref.option = RR_NORMAL;
+ return;
+ }
+
+ atom->u.remote_ref.nobracket = 0;
+ string_list_split(¶ms, arg, ',', -1);
+
+ for (i = 0; i < params.nr; i++) {
+ const char *s = params.items[i].string;
+
+ if (!strcmp(s, "short"))
+ atom->u.remote_ref.option = RR_SHORTEN;
+ else if (!strcmp(s, "track"))
+ atom->u.remote_ref.option = RR_TRACK;
+ else if (!strcmp(s, "trackshort"))
+ atom->u.remote_ref.option = RR_TRACKSHORT;
+ else if (!strcmp(s, "nobracket"))
+ atom->u.remote_ref.nobracket = 1;
+ else
+ die(_("unrecognized format: %%(%s)"), atom->name);
+ }
+
+ string_list_clear(¶ms, 0);
}
static void body_atom_parser(struct used_atom *atom, const char *arg)
@@ -1045,25 +1064,27 @@ static void fill_remote_ref_details(struct used_atom
*atom, const char *refname,
struct branch *branch, const char **s)
{
int num_ours, num_theirs;
- if (atom->u.remote_ref == RR_SHORTEN)
+ if (atom->u.remote_ref.option == RR_SHORTEN)
*s = shorten_unambiguous_ref(refname, warn_ambiguous_refs);
- else if (atom->u.remote_ref == RR_TRACK) {
+ else if (atom->u.remote_ref.option == RR_TRACK) {
if (stat_tracking_info(branch, &num_ours,
&num_theirs, NULL)) {
- *s = "[gone]";
- return;
- }
-
- if (!num_ours && !num_theirs)
+ *s = xstrdup("gone");
+ } else if (!num_ours && !num_theirs)
*s = "";
else if (!num_ours)
- *s = xstrfmt("[behind %d]", num_theirs);
+ *s = xstrfmt("behind %d", num_theirs);
[PATCH v3 04/16] ref-filter: modify "%(objectname:short)" to take length
Add support for %(objectname:short=) which would print the
abbreviated unique objectname of given length. When no length is
specified, the length is 'DEFAULT_ABBREV'. The minimum length is
'MINIMUM_ABBREV'. The length may be exceeded to ensure that the provided
object name is unique.
Add tests and documentation for the same.
Mentored-by: Christian Couder
Mentored-by: Matthieu Moy
Helped-by: Jacob Keller
Signed-off-by: Karthik Nayak
---
Documentation/git-for-each-ref.txt | 4
ref-filter.c | 25 +++--
t/t6300-for-each-ref.sh| 10 ++
3 files changed, 33 insertions(+), 6 deletions(-)
diff --git a/Documentation/git-for-each-ref.txt
b/Documentation/git-for-each-ref.txt
index e1b1a66..d3223a2 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -107,6 +107,10 @@ objectsize::
objectname::
The object name (aka SHA-1).
For a non-ambiguous abbreviation of the object name append `:short`.
+ For an abbreviation of the object name with desired length append
+ `:short=`, where the minimum length is MINIMUM_ABBREV. The
+ length may be exceeded to ensure unique object names.
+
upstream::
The name of a local ref which can be considered ``upstream''
diff --git a/ref-filter.c b/ref-filter.c
index 857a8b5..17f781d 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -55,7 +55,10 @@ static struct used_atom {
const char *if_equals,
*not_equals;
} if_then_else;
- enum { O_FULL, O_SHORT } objectname;
+ struct {
+ enum { O_FULL, O_LENGTH, O_SHORT } option;
+ unsigned int length;
+ } objectname;
} u;
} *used_atom;
static int used_atom_cnt, need_tagged, need_symref;
@@ -118,10 +121,17 @@ static void contents_atom_parser(struct used_atom *atom,
const char *arg)
static void objectname_atom_parser(struct used_atom *atom, const char *arg)
{
if (!arg)
- atom->u.objectname = O_FULL;
+ atom->u.objectname.option = O_FULL;
else if (!strcmp(arg, "short"))
- atom->u.objectname = O_SHORT;
- else
+ atom->u.objectname.option = O_SHORT;
+ else if (skip_prefix(arg, "short=", &arg)) {
+ atom->u.contents.option = O_LENGTH;
+ if (strtoul_ui(arg, 10, &atom->u.objectname.length) ||
+ atom->u.objectname.length == 0)
+ die(_("positive value expected objectname:short=%s"),
arg);
+ if (atom->u.objectname.length < MINIMUM_ABBREV)
+ atom->u.objectname.length = MINIMUM_ABBREV;
+ } else
die(_("unrecognized %%(objectname) argument: %s"), arg);
}
@@ -591,12 +601,15 @@ static int grab_objectname(const char *name, const
unsigned char *sha1,
struct atom_value *v, struct used_atom *atom)
{
if (starts_with(name, "objectname")) {
- if (atom->u.objectname == O_SHORT) {
+ if (atom->u.objectname.option == O_SHORT) {
v->s = xstrdup(find_unique_abbrev(sha1,
DEFAULT_ABBREV));
return 1;
- } else if (atom->u.objectname == O_FULL) {
+ } else if (atom->u.objectname.option == O_FULL) {
v->s = xstrdup(sha1_to_hex(sha1));
return 1;
+ } else if (atom->u.objectname.option == O_LENGTH) {
+ v->s = xstrdup(find_unique_abbrev(sha1,
atom->u.objectname.length));
+ return 1;
} else
die("BUG: unknown %%(objectname) option");
}
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 19a2823..2be0a3f 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -60,6 +60,8 @@ test_atom head objecttype commit
test_atom head objectsize 171
test_atom head objectname $(git rev-parse refs/heads/master)
test_atom head objectname:short $(git rev-parse --short refs/heads/master)
+test_atom head objectname:short=1 $(git rev-parse --short=1 refs/heads/master)
+test_atom head objectname:short=10 $(git rev-parse --short=10
refs/heads/master)
test_atom head tree $(git rev-parse refs/heads/master^{tree})
test_atom head parent ''
test_atom head numparent 0
@@ -99,6 +101,8 @@ test_atom tag objecttype tag
test_atom tag objectsize 154
test_atom tag objectname $(git rev-parse refs/tags/testtag)
test_atom tag objectname:short $(git rev-parse --short refs/tags/testtag)
+test_atom head objectname:short=1 $(git rev-parse --short=1 refs/heads/master)
+test_atom head objectname:short=10 $(git rev-parse --short=10
refs/heads/master)
test_atom tag tree ''
test_atom tag parent ''
test_atom tag numparent ''
@@ -164,6 +168,12 @@ test_expect_success 'Check invalid for
[PATCH v3 10/16] ref-filter: introduce symref_atom_parser()
Introduce symref_atom_parser() which will parse the '%(symref)' atom and
store information into the 'used_atom' structure based on the modifiers
used along with the atom.
Signed-off-by: Karthik Nayak
---
ref-filter.c | 25 +++--
1 file changed, 23 insertions(+), 2 deletions(-)
diff --git a/ref-filter.c b/ref-filter.c
index 5c59b17..7b35e4f 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -62,6 +62,7 @@ static struct used_atom {
enum { O_FULL, O_LENGTH, O_SHORT } option;
unsigned int length;
} objectname;
+ enum { S_FULL, S_SHORT } symref;
} u;
} *used_atom;
static int used_atom_cnt, need_tagged, need_symref;
@@ -217,6 +218,15 @@ static void if_atom_parser(struct used_atom *atom, const
char *arg)
die(_("unrecognized %%(if) argument: %s"), arg);
}
+static void symref_atom_parser(struct used_atom *atom, const char *arg)
+{
+ if (!arg)
+ atom->u.symref = S_FULL;
+ else if (!strcmp(arg, "short"))
+ atom->u.symref = S_SHORT;
+ else
+ die(_("unrecognized %%(symref) argument: %s"), arg);
+}
static struct {
const char *name;
@@ -252,7 +262,7 @@ static struct {
{ "contents", FIELD_STR, contents_atom_parser },
{ "upstream", FIELD_STR, remote_ref_atom_parser },
{ "push", FIELD_STR, remote_ref_atom_parser },
- { "symref" },
+ { "symref", FIELD_STR, symref_atom_parser },
{ "flag" },
{ "HEAD" },
{ "color", FIELD_STR, color_atom_parser },
@@ -1132,6 +1142,17 @@ char *get_head_description(void)
return strbuf_detach(&desc, NULL);
}
+static const char *get_symref(struct used_atom *atom, struct ref_array_item
*ref)
+{
+ if (!ref->symref)
+ return "";
+ else if (atom->u.symref == S_SHORT)
+ return shorten_unambiguous_ref(ref->symref,
+ warn_ambiguous_refs);
+ else
+ return ref->symref;
+}
+
/*
* Parse the object referred by ref, and grab needed value.
*/
@@ -1176,7 +1197,7 @@ static void populate_value(struct ref_array_item *ref)
if (ref->kind & FILTER_REFS_DETACHED_HEAD)
refname = get_head_description();
} else if (starts_with(name, "symref"))
- refname = ref->symref ? ref->symref : "";
+ refname = get_symref(atom, ref);
else if (starts_with(name, "upstream")) {
const char *branch_name;
/* only local branches may have an upstream */
--
2.7.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 v3 01/16] ref-filter: implement %(if), %(then), and %(else) atoms
Implement %(if), %(then) and %(else) atoms. Used as
%(if)...%(then)...%(end) or %(if)...%(then)...%(else)...%(end). If the
format string between %(if) and %(then) expands to an empty string, or
to only whitespaces, then the whole %(if)...%(end) expands to the string
following %(then). Otherwise, it expands to the string following
%(else), if any. Nesting of this construct is possible.
This is in preparation for porting over `git branch -l` to use
ref-filter APIs for printing.
Add Documentation and tests regarding the same.
Mentored-by: Christian Couder
Mentored-by: Matthieu Moy
Signed-off-by: Karthik Nayak
---
Documentation/git-for-each-ref.txt | 45 +++--
ref-filter.c | 133 +++--
t/t6302-for-each-ref-filter.sh | 70 +++
3 files changed, 237 insertions(+), 11 deletions(-)
diff --git a/Documentation/git-for-each-ref.txt
b/Documentation/git-for-each-ref.txt
index 012e8f9..d048561 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -141,10 +141,17 @@ align::
"width=" and/or "position=" prefixes may be omitted, and bare
and used instead. For instance,
`%(align:,)`. If the contents length is more
- than the width then no alignment is performed. If used with
- '--quote' everything in between %(align:...) and %(end) is
- quoted, but if nested then only the topmost level performs
- quoting.
+ than the width then no alignment is performed.
+
+if::
+ Used as %(if)...%(then)...(%end) or
+ %(if)...%(then)...%(else)...%(end). If there is an atom with
+ value or string literal after the %(if) then everything after
+ the %(then) is printed, else if the %(else) atom is used, then
+ everything after %(else) is printed. We ignore space when
+ evaluating the string before %(then), this is useful when we
+ use the %(HEAD) atom which prints either "*" or " " and we
+ want to apply the 'if' condition only on the 'HEAD' ref.
In addition to the above, for commit and tag objects, the header
field names (`tree`, `parent`, `object`, `type`, and `tag`) can
@@ -181,6 +188,20 @@ As a special case for the date-type fields, you may
specify a format for
the date by adding `:` followed by date format name (see the
values the `--date` option to linkgit::git-rev-list[1] takes).
+Some atoms like %(align) and %(if) always require a matching %(end).
+We call them "opening atoms" and sometimes denote them as %($open).
+
+When a scripting language specific quoting is in effect (i.e. one of
+`--shell`, `--perl`, `--python`, `--tcl` is used), except for opening
+atoms, replacement from every %(atom) is quoted when and only when it
+appears at the top-level (that is, when it appears outside
+%($open)...%(end)).
+
+When a scripting language specific quoting is in effect, everything
+between a top-level opening atom and its matching %(end) is evaluated
+according to the semantics of the opening atom and its result is
+quoted.
+
EXAMPLES
@@ -268,6 +289,22 @@ eval=`git for-each-ref --shell --format="$fmt" \
eval "$eval"
+
+An example to show the usage of %(if)...%(then)...%(else)...%(end).
+This prefixes the current branch with a star.
+
+
+git for-each-ref --format="%(if)%(HEAD)%(then)* %(else)
%(end)%(refname:short)" refs/heads/
+
+
+
+An example to show the usage of %(if)...%(then)...%(end).
+This prints the authorname, if present.
+
+
+git for-each-ref --format="%(refname)%(if)%(authorname)%(then)
%(color:red)Authored by: %(authorname)%(end)"
+
+
SEE ALSO
linkgit:git-show-ref[1]
diff --git a/ref-filter.c b/ref-filter.c
index bc551a7..41e73f0 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -21,6 +21,12 @@ struct align {
unsigned int width;
};
+struct if_then_else {
+ unsigned int then_atom_seen : 1,
+ else_atom_seen : 1,
+ condition_satisfied : 1;
+};
+
/*
* An atom is a valid field atom listed below, possibly prefixed with
* a "*" to denote deref_tag().
@@ -203,6 +209,9 @@ static struct {
{ "color", FIELD_STR, color_atom_parser },
{ "align", FIELD_STR, align_atom_parser },
{ "end" },
+ { "if" },
+ { "then" },
+ { "else" },
};
#define REF_FORMATTING_STATE_INIT { 0, NULL }
@@ -210,7 +219,7 @@ static struct {
struct ref_formatting_stack {
struct ref_formatting_stack *prev;
struct strbuf output;
- void (*at_end)(struct ref_formatting_stack *stack);
+ void (*at_end)(struct ref_formatting_stack **stack);
void *at_end_data;
};
@@ -343,13 +352,14 @@ static void pop_stack_element(struct ref_formatting_stack
**stack)
*stack = prev;
}
-static void end_align_handler(struct ref_formatting_stack *stack)
+static void end_align_handler(struct ref_formatting_stack **stack)
{
-
[PATCH v3 02/16] ref-filter: include reference to 'used_atom' within 'atom_value'
Ensure that each 'atom_value' has a reference to its corresponding
'used_atom'. This let's us use values within 'used_atom' in the
'handler' function.
Hence we can get the %(align) atom's parameters directly from the
'used_atom' therefore removing the necessity of passing %(align) atom's
parameters to 'atom_value'.
This also acts as a preparatory patch for the upcoming patch where we
introduce %(if:equals=) and %(if:notequals=).
Signed-off-by: Karthik Nayak
---
ref-filter.c | 8 +++-
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/ref-filter.c b/ref-filter.c
index 41e73f0..12e646c 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -230,11 +230,9 @@ struct ref_formatting_state {
struct atom_value {
const char *s;
- union {
- struct align align;
- } u;
void (*handler)(struct atom_value *atomv, struct ref_formatting_state
*state);
unsigned long ul; /* used for sorting when not FIELD_STR */
+ struct used_atom *atom;
};
/*
@@ -370,7 +368,7 @@ static void align_atom_handler(struct atom_value *atomv,
struct ref_formatting_s
push_stack_element(&state->stack);
new = state->stack;
new->at_end = end_align_handler;
- new->at_end_data = &atomv->u.align;
+ new->at_end_data = &atomv->atom->u.align;
}
static void if_then_else_handler(struct ref_formatting_stack **stack)
@@ -1069,6 +1067,7 @@ static void populate_value(struct ref_array_item *ref)
struct branch *branch = NULL;
v->handler = append_atom;
+ v->atom = atom;
if (*name == '*') {
deref = 1;
@@ -1133,7 +1132,6 @@ static void populate_value(struct ref_array_item *ref)
v->s = " ";
continue;
} else if (starts_with(name, "align")) {
- v->u.align = atom->u.align;
v->handler = align_atom_handler;
continue;
} else if (!strcmp(name, "end")) {
--
2.7.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: git-apply does not work in a sub-directory of a Git repository
On Thu, Mar 24, 2016 at 11:50 PM, Junio C Hamano wrote:
> So a better alternative may be to conditionally disable the "Paths
> outside are not touched regardless of --include" logic, i.e. we
> exclude paths outside by default just as before, but if there is at
> least one explicit "--include" given, we skip this "return 0".
>
> That way, we do not have to commit to turning --include/--exclude to
> pathspec (which I agree is a huge change in behaviour that may not
> be a good idea) and we do not have to add "--full-tree" that is only
> understood by "apply" but not other commands that operate on the
> current directory by default.
Suppose I don't like git-apply's default behavior, I make an alias.ap
= "apply --include=*". So far so good, but when I want to limit paths
back to "subdir" (it does not have to be the same as cwd), how do I do
it without typing resorting to typing "git apply" explicitly ? I don't
see an option to cancel out --include=*. For "git ap --exclude=*
--include=subdir" to have that effect, we need to change
for (i = 0; i < limit_by_name.nr; i++) {
in use_patch() to
for (i = limit_by_name.nr - 1; i >= 0; i--) {
Simple change, but not exactly harmless.
Off topic, but --include/--exclude should be able to deal with
relative path like --include=../*.c or --include=./*. I guess nobody
has complained about it, so it's not needed.
--
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
AW: [PATCH] Fix http-backend reading till EOF, ignoring CONTENT_LENGTH, violating rfc3875 -- WAS: Problem with git-http-backend.exe as iis cgi
> -Ursprüngliche Nachricht-
> Von: Jeff King [mailto:p...@peff.net]
> Gesendet: Dienstag, 29. März 2016 22:14
> An: Florian Manschwetus
> Cc: Chris Packham; Konstantin Khomoutov; git@vger.kernel.org
> Betreff: Re: [PATCH] Fix http-backend reading till EOF, ignoring
> CONTENT_LENGTH, violating rfc3875 -- WAS: Problem with git-http-
> backend.exe as iis cgi
>
> On Tue, Mar 29, 2016 at 10:38:23AM +, Florian Manschwetus wrote:
>
> > > | A request-body is supplied with the request if the CONTENT_LENGTH
> > > | is not NULL. The server MUST make at least that many bytes
> > > | available for the script to read. The server MAY signal an
> > > | end-of-file condition after CONTENT_LENGTH bytes have been read or
> > > | it MAY supply extension data. Therefore, the script MUST NOT
> > > | attempt to read more than CONTENT_LENGTH bytes, even if more data
> > > | is available. However, it is not obliged to read any of the data.
> > >
> > > So yes, if Git currently reads until EOF, it's an error.
> > > The correct way would be:
> > >
> > > 1) Check to see if the CONTENT_LENGTH variable is available in the
> > >environment. If no, read nothing.
> > >
> > > 2) Otherwise read as many bytes it specifies, and no more.
> > >
> > > 1. https://www.ietf.org/rfc/rfc3875
>
> I don't think the second part of (1) will work very well if the client sends a
> chunked transfer-encoding (which git will do if the input is large). In such a
> case the server would either have to buffer the entire input to find its
> length,
> or stream the data to the CGI without setting $CONTENT_LENGTH. At least
> some servers choose the latter (including Apache).
>
> > diff --git a/http-backend.c b/http-backend.c index 8870a26..94976df
> > 100644
> > --- a/http-backend.c
> > +++ b/http-backend.c
> > @@ -277,16 +277,32 @@ static struct rpc_service *select_service(const
> char *name)
> > */
> > static ssize_t read_request(int fd, unsigned char **out) {
> > - size_t len = 0, alloc = 8192;
> > - unsigned char *buf = xmalloc(alloc);
> > + unsigned char *buf = null;
> > ...
>
> git-am complained that your patch did not apply, but after writing something
> similar locally, I found that t5551.25 hangs indefinitely.
> Which is not surprising. Most tests are doing very limited ref negotiation, so
> the content that hits read_request() here is small, and we send it in a single
> write with a content-length header. But t5551.25 uses a much bigger
> workload, which causes the client to use a chunked transfer-encoding, and
> this code to refuse to read anything (and then the protocol stalls, as we are
> waiting for the client to say something).
>
> So I think you'd want to take a missing CONTENT_LENGTH as a hint to read
> until EOF.
>
> That also raises another issue: what happens in the paths that don't hit
> read_request()? We may also process input via:
>
> - inflate_request(), if the client gzipped it; for well-formed input,
> I think we'll stop reading when the gzip stream tells us there is no
> more data, but a malformed one would have us reading until EOF,
> regardless of what $CONTENT_LENGTH says.
>
> - for input which we expect to be large (like incoming packfiles for a
> push), buffer_input will be unset, and we will pass the descriptor
> directly to a sub-program like git-index-pack. Again, for
> well-formed input it would read just the packfile, but it may
> actually continue to EOF.
>
> So I don't think your patch is covering all cases.
>
> -Peff
After additional analysis it turned out, that in the case you mentioned, at
least IIS, sets CONTENT_LENGTH to -1 resulting in the current behavior of
git-http-backend being sufficient in this situation.
Therefore I refactored the code again a bit, to match up the behavior I
currently fake by using some bash magic...
From ccd6c88e39a850b253979b785463719cdc0fa1e2 Mon Sep 17 00:00:00 2001
From: manschwetus
Date: Tue, 29 Mar 2016 12:16:21 +0200
Subject: [PATCH 1/2] Fix http-backend reading till EOF, ignoring
CONTENT_LENGTH, violating rfc3875
Signed-off-by: Florian Manschwetus
---
http-backend.c | 48 +++-
1 file changed, 31 insertions(+), 17 deletions(-)
diff --git a/http-backend.c b/http-backend.c
index 8870a26..94976df 100644
--- a/http-backend.c
+++ b/http-backend.c
@@ -277,16 +277,32 @@ static struct rpc_service *select_service(const char
*name)
*/
static ssize_t read_request(int fd, unsigned char **out)
{
- size_t len = 0, alloc = 8192;
- unsigned char *buf = xmalloc(alloc);
+ unsigned char *buf = null;
+ size_t len = 0;
+ /* get request size */
+ size_t req_len = git_env_ulong("CONTENT_LENGTH",
+ 0);
+
+ /* check request size */
+ if (max_request_buffer < req_len) {
+ die("request was larger than our maximum size (%lu);"
+ " try setting GIT_HTTP_MAX_RE
[BUG] `git submodule update --init --recursive` stores gitdir in full path into `.git` of nested submodules
Hi, `git submodule update --init --recursive` stores `gitdir` in full path into `.git` of nested submodules. So, working directory is not portable to another directory. On following example, `Carthage/Checkouts/Quick/Externals/Nimble/` is nested submodule and `Carthage/Checkouts/Quick/Externals/Nimble/.git` contains full path. https://gist.github.com/norio-nomura/17ce4bdf0151185e77d9b1fcfb5a469d Thanks, -- Norio Nomura @norio_nomura -- 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 V2] MSVC: VS2013 comes with inttypes.h
On 3/29/2016 19:23, Sven Strickroth wrote: > --- a/compat/mingw.h > +++ b/compat/mingw.h > @@ -415,7 +415,7 @@ int mingw_offset_1st_component(const char *path); > extern void build_libgit_environment(void); > extern const char *program_data_config(void); > #define git_program_data_config program_data_config > -#ifndef __MINGW64_VERSION_MAJOR > +#if !defined(__MINGW64_VERSION_MAJOR) && (!defined(_MSC_VER) || _MSC_VER < > 1800) > #define PRIuMAX "I64u" > #define PRId64 "I64d" > #else ACK for this part. For reference see [1]. > diff --git a/compat/vcbuild/include/unistd.h b/compat/vcbuild/include/unistd.h > index c65c2cd..b7cc48c 100644 > --- a/compat/vcbuild/include/unistd.h > +++ b/compat/vcbuild/include/unistd.h > @@ -45,11 +45,15 @@ typedef unsigned long long uintmax_t; > > typedef int64_t off64_t; > > +#if !defined(_MSC_VER) || _MSC_VER < 1800 > #define INTMAX_MIN _I64_MIN > #define INTMAX_MAX _I64_MAX > #define UINTMAX_MAX _UI64_MAX > > #define UINT32_MAX 0x /* 4294967295U */ > +#else > +#include > +#endif If we would do "#include " here instead, we could lower the _MSC_VER requirement to at least 1700. According to the comment at [2] we could lower it even to 1600. Also the original code is missing a single space after "#include". [1] https://blogs.msdn.microsoft.com/vcblog/2013/07/19/c99-library-support-in-visual-studio-2013/ [2] https://stackoverflow.com/questions/126279/c99-stdint-h-header-and-ms-visual-studio#comment4620359_126279 Regards, Sebastian -- 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] diffcore: fix iteration order of identical files during rename detection
If the two paths 'dir/A/file' and 'dir/B/file' have identical content and the parent directory is renamed, e.g. 'git mv dir other-dir', then diffcore reports the following exact renames: renamed:dir/B/file -> other-dir/A/file renamed:dir/A/file -> other-dir/B/file While technically not wrong, this is confusing not only for the user, but also for git commands that make decisions based on rename information, e.g. 'git log --follow other-dir/A/file' follows 'dir/B/file' past the rename. This behavior is a side effect of commit v2.0.0-rc4~8^2~14 (diffcore-rename.c: simplify finding exact renames, 2013-11-14): the hashmap storing sources returns entries from the same bucket, i.e. sources matching the current destination, in LIFO order. Thus the iteration first examines 'other-dir/A/file' and 'dir/B/file' and, upon finding identical content and basename, reports an exact rename. Other hashmap users are apparently happy with the current iteration order over the entries of a bucket. Changing the iteration order would risk upsetting other hashmap users and would increase the memory footprint of each bucket by a pointer to the tail element. Fill the hashmap with source entries in reverse order to restore the original exact rename detection behavior. Reported-by: Bill Okara Signed-off-by: SZEDER Gábor --- Resend of the patch, with a slightly updated commit message, included in http://thread.gmane.org/gmane.comp.version-control.git/287281/focus=287570 Being embedded with scissors in an email without Junio among the recipients on the day the first -rc was tagged... no wonder it flew below the radar. diffcore-rename.c | 6 -- t/t4001-diff-rename.sh | 11 +++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/diffcore-rename.c b/diffcore-rename.c index 3b3c1ed535e7..7f03eb5a0404 100644 --- a/diffcore-rename.c +++ b/diffcore-rename.c @@ -340,9 +340,11 @@ static int find_exact_renames(struct diff_options *options) int i, renames = 0; struct hashmap file_table; - /* Add all sources to the hash table */ + /* Add all sources to the hash table in reverse order, because +* later on they will be retrieved in LIFO order. +*/ hashmap_init(&file_table, NULL, rename_src_nr); - for (i = 0; i < rename_src_nr; i++) + for (i = rename_src_nr-1; i >= 0; i--) insert_file_table(&file_table, i, rename_src[i].p->one); /* Walk the destinations and find best source match */ diff --git a/t/t4001-diff-rename.sh b/t/t4001-diff-rename.sh index 2f327b749588..ed90c6c6f984 100755 --- a/t/t4001-diff-rename.sh +++ b/t/t4001-diff-rename.sh @@ -77,6 +77,17 @@ test_expect_success 'favour same basenames even with minor differences' ' git show HEAD:path1 | sed "s/15/16/" > subdir/path1 && git status | test_i18ngrep "renamed: .*path1 -> subdir/path1"' +test_expect_success 'two files with same basename and same content' ' + git reset --hard && + mkdir -p dir/A dir/B && + cp path1 dir/A/file && + cp path1 dir/B/file && + git add dir && + git commit -m 2 && + git mv dir other-dir && + git status | test_i18ngrep "renamed: .*dir/A/file -> other-dir/A/file" +' + test_expect_success 'setup for many rename source candidates' ' git reset --hard && for i in 0 1 2 3 4 5 6 7 8 9; -- 2.8.0.46.gb821760 -- 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: [Outreachy] Git remote whitelist/blacklist
Hi Elena, I thought a bit more about your proposal. The Outreachy internship is scheduled for 3 months and I think you would be able to come up with a "Git remote whitelist/blacklist" implementation that the Git list considers to accept within a month. In that case it would be good if you would have a few extra ideas in your Outreachy proposal that you could tackle afterwards. These extras could be extensions to the "whitelist/blacklist" project or a contribution to a completely different part of Git. According to the Outreachy page [1] you can still change your application until April 22. In addition I wonder if you would be willing to start slowly with the "drafting the implementation" task even before April 22. I, and AFAIK the majority of the other people on the list, do not work full time on Git. That means some email responses might be delayed for reasons unrelated to Git. Therefore I think you will have a better experience if you work with a steady slow pace instead of high burst of list activity :-) Cheers, Lars [1] https://wiki.gnome.org/Outreachy#Submit_an_Application > On 29 Mar 2016, at 21:24, Lars Schneider wrote: > > Hi Elena, > > sorry for the late reply. I think it is great that you are interested in the > Git remote whitelist/blacklist idea. Unfortunately I don't have any > experience with Outreachy and I wonder what kind of feedback you are looking > for. > > Thanks, > Lars > > >> On 26 Mar 2016, at 13:15, elena petrashen wrote: >> >> Hi everyone, >> >> I think I will submit the application as it is now, but still >> it would be great to get feedback on it, as I don't think >> there was no reply because everything was perfect :( >> >> Thank you! And have an awesome weekend. >> >> On Thu, Mar 24, 2016 at 5:50 PM, elena petrashen >> wrote: >>> Hi, >>> >>> I'm thinking of applying to Outreachy program this round with Git >>> and the project I'm really interested in is "Git remote whitelist/blacklist" >>> project (http://git.github.io/SoC-2016-Ideas/). >>> I have drafted the description/timeline for this project >>> and it would be great to get feedback/suggestions. >>> (I'm actually a bit confused about the scale of this. The >>> Outreachy application doesn't ask for "proposal" in the way >>> GSoC seems to, but merely requests "details and the timeline", >>> so I'm not sure whether the shorter description of what's planned >>> is expected or should I go deeper in detail. I apologize if I >>> chose a wrong approach.) >>> >>> Thank you! >>> > What project(s) are you interested in (these can be in the >>> same or different organizations)? >>> My preferred project to work on is Git remote whitelist/blacklist >>> project listed on http://git.github.io/SoC-2016-Ideas/. I'm really >>> interested in doing this project as I think this kind of effort is >>> really important: I recently started using git myself, and sometimes >>> I was really scared to push something to the location I was not >>> supposed to push to. I would really appreciate the opportunity in >>> participating in making git a bit more newbie-friendly. >>> > Who is a possible mentor for the project you are most interested in? >>> Lars Schneider >>> > Please describe the details and the timeline of the work you >>> plan to accomplish on the project you are most interested in (discuss >>> these first with the mentor of the project): >>> The goal is to provide a safer environment for newcomers to Git to >>> enabling the possibility to modify git config, adding there "allowed" >>> and "denied" remotes for pushing. Code, tests, and documentation >>> are to be created. >>> >>> Timeline: >>> 0. Analysis >>> Apr 22 - May 22 - studying the current code and drafting the >>> implementation proposal >>> 1. Design >>> a. May 22-June 1 - discussion with the mentor regarding the task, >>> presenting the approach and amending it per mentor's feedback >>> b. June 1st-June 15th - communicating with the community >>> regarding the suggested changes and agreeing on logic, scope >>> and format of changes. >>> 2. Development >>> c. June 15th-July 1st - submitting code for the first basic version, >>> amending it according to the feedback >>> d. July 1st - July 15th - extending the code to cover all of the >>> agreed scope >>> e. July 15th - Aug 1st - finalizing full coverage with tests and >>> documentation >>> 3. Evaluation >>> f. Aug 1st - Aug 23rd - adding nice-to-have features and other >>> suggestion by the community >> -- >> To unsubscribe from this list: send the line "unsubscribe git" in >> the body of a message to majord...@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] MSVC: vsnprintf in Visual Studio 2015 doesn't need SNPRINTF_SIZE_CORR any more
On Wed, Mar 30, 2016 at 9:49 AM, Johannes Schindelin wrote: >> > #ifndef SNPRINTF_SIZE_CORR >> > -#if defined(WIN32) && (!defined(__GNUC__) || __GNUC__ < 4) >> > +#if defined(WIN32) && (!defined(__GNUC__) || __GNUC__ < 4) && >> > (!defined(_MSC_VER) || _MSC_VER < 1900) >> > #define SNPRINTF_SIZE_CORR 1 >> > #else >> > #define SNPRINTF_SIZE_CORR 0 >> >> I wonder if the logic is (and was) sensible here. We assume that every >> non-__GNUC__ and non-_MSC_VER compiler on Windows requires the >> correction. Wouldn't it make sense to not assume requiring the >> correction unless we know the compiler has this bug? That is, >> shouldn't this better say >> >> #if defined(WIN32) && (defined(__GNUC__) && __GNUC__ < 4) || >> (defined(_MSC_VER) && _MSC_VER < 1900)) >> #define SNPRINTF_SIZE_CORR 1 >> #else >> #define SNPRINTF_SIZE_CORR 0 > > Since the standard on Windows always was MS Visual C, it should be assumed > that compilers *other* than GCC followed Visual C's lead. > > Of course, evidence speaks louder than assumptions. > > Therefore I would prefer to keep the current version, at least until we > encounter a case where it is incorrect. Fine with me. It's probably better not to change the logic as we wouldn't know whether this would break things for some exotic compiler currently in use to compile Git. Also ACK from my side on the path then. -- Sebastian Schuberth -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] MSVC: vsnprintf in Visual Studio 2015 doesn't need SNPRINTF_SIZE_CORR any more
Hi Sven & Sebastian, On Tue, 29 Mar 2016, Sebastian Schuberth wrote: > On Tue, Mar 29, 2016 at 9:13 PM, Sven Strickroth wrote: ACK on the patch. > > diff --git a/compat/snprintf.c b/compat/snprintf.c > > index 42ea1ac..0b11688 100644 > > --- a/compat/snprintf.c > > +++ b/compat/snprintf.c > > @@ -9,7 +9,7 @@ > > * always have room for a trailing NUL byte. > > */ > > #ifndef SNPRINTF_SIZE_CORR > > -#if defined(WIN32) && (!defined(__GNUC__) || __GNUC__ < 4) > > +#if defined(WIN32) && (!defined(__GNUC__) || __GNUC__ < 4) && > > (!defined(_MSC_VER) || _MSC_VER < 1900) > > #define SNPRINTF_SIZE_CORR 1 > > #else > > #define SNPRINTF_SIZE_CORR 0 > > I wonder if the logic is (and was) sensible here. We assume that every > non-__GNUC__ and non-_MSC_VER compiler on Windows requires the > correction. Wouldn't it make sense to not assume requiring the > correction unless we know the compiler has this bug? That is, > shouldn't this better say > > #if defined(WIN32) && (defined(__GNUC__) && __GNUC__ < 4) || > (defined(_MSC_VER) && _MSC_VER < 1900)) > #define SNPRINTF_SIZE_CORR 1 > #else > #define SNPRINTF_SIZE_CORR 0 Since the standard on Windows always was MS Visual C, it should be assumed that compilers *other* than GCC followed Visual C's lead. Of course, evidence speaks louder than assumptions. Therefore I would prefer to keep the current version, at least until we encounter a case where it is incorrect. Thanks, 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
