Re: [PATCH 2/3] i18n: Only extract comments marked by special tag
2014-04-18 2:08 GMT+08:00 Junio C Hamano gits...@pobox.com:
Jiang Xin worldhello@gmail.com writes:
When extract l10n messages, we use --add-comments option to keep
comments right above the l10n messages for references. But sometimes
irrelevant comments are also extracted. For example in the following
code block, the comment in line 2 will be extracted as comment for the
l10n message in line 3, but obviously it's wrong.
{ OPTION_CALLBACK, 0, ignore-removal, addremove_explicit,
NULL /* takes no arguments */,
N_(ignore paths removed in the working tree (same as
--no-all)),
PARSE_OPT_NOARG, ignore_removal_cb },
Since almost all comments for l10n translators are marked with the same
prefix (tag): TRANSLATORS:, it's safe to only extract comments with
this special tag. I.E. it's better to call xgettext as:
xgettext --add-comments=TRANSLATORS: ...
Also tweaks the multi-line comment in init-db.c, to make it start with
the proper tag, not * TRANSLATORS: (which has a star before the tag).
Hmph.
I am not very happy with this change, as it would force us to
special case Translators comment to follow a non-standard
multi-line comment formatting convention. Is there a way to tell
xgettext to accept both of these forms?
/* TRANSLATORS: this is a short comment to help you */
_(foo bar);
/*
* TRANSLATORS: this comment is to help you, but it is
* a lot longer to fit on just a single line.
*/
_(bar baz);
We can not provide multiple `--add-comments=TAG` options to xgettext,
because xgettext holds the tag in one string, not in a list:
/* Tag used in comment of prevailing domain. */
static char *comment_tag;
So if we won't change our multi-line comments for translators, must
hack gettext in some ways.
There maybe 3 ways to hack gettext:
1. When matching comments against TAG, using strstr not strncmp.
2360 /* When the comment tag is seen, it drags in not
only the line
2361which it starts, but all remaining comment lines. */
2362 if (add_all_remaining_comments
2363 || (add_all_remaining_comments =
2364 (comment_tag != NULL
2365 strncmp (s, comment_tag, strlen
(comment_tag)) == 0)))
2. Add a extension to in-comment xgettext instructions.
There is a undocumented feature in xgettext: User can provide
instructions (prefixed by xgettext:) in comments, such as:
/*
* xgettext: fuzzy possible-c-format no-wrap
* other comments...
*/
But it does not help much, unless we hack xgettext to extend this
hidden feature. I.E. Add an additional flag to support unconditionally
reference to the commit block. Like:
/*
* xgettext: comments
* TRANSLATORS: this comment is to help you, but it is
* a lot longer to fit on just a single line.
*/
_(bar baz);
3. Hack the parser for comments in gettext-tools/src/x-c.c (maybe
function phase4_getc()) to support various multi-line comments style,
such as:
/*
* TRANSLATORS: this comment is to help you, but it is
* a lot longer to fit on just a single line.
*/
/*
** TRANSLATORS: this comment is to help you, but it is
** a lot longer to fit on just a single line.
*/
/
* TRANSLATORS: this comment is to help you, but it is *
* a lot longer to fit on just a single line. *
/
I CC this mail to the gettext mailing list. Full thread see:
* http://thread.gmane.org/gmane.comp.version-control.git/246390/focus=246431
--
Jiang Xin
--
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] Update SVN.pm
Hello, cc'ing Roman, the original author. (I should have done that in the first post, sorry. I have also forwarded him another mail from this thread, asking him for author's sign off.) On Thu, Apr 17, 2014 at 10:39:49AM -0700, Junio C Hamano wrote: Stepan Kasal ka...@ucw.cz writes: On Wed, Apr 16, 2014 at 12:13:21PM -0700, Junio C Hamano wrote: Interesting. What other strange forms can they record in their repositories, I have to wonder. Can they do 2014-01-07T5:8:6.048176Z for example? Roman Belinsky, the author of this fix, witnessed after large scale conversion that the problem happens with the hour part only. Is this large scale conversion done from a SVN repository that is created by bog standard SVN, or something else? I don't know. Roman? How certain are we that this hour part is broken is the only kind of breakage in timestamps we would encouter? I would say we can be certain, as Roman said that the same PC that inserts the timestamp with one-digit hours does not misformat minutes. (Still cited from the same discussion https://github.com/msysgit/git/pull/126#discussion_r9661916 ) We do not have code review for that bug, as far as I know, but this is a natural bug: a reasonably looking time 5:08:09.048176 is used in format %sT%s [...] and by being slightly more lenient than necessary to cover one observed case that triggered the patch, we can cover SVN repositories broken in a similar but slightly different way. I second that, in general. But my guess is that this particular similar but slightly different breakage will never appear, so the self-documenting original fix wins for me. Especially given that this regexp matching is not used for finding a timestamp from random places [...] I agree that the broader regexp is not dangerous in this context. So it seems to be no big issue either way. Thanks for taking this so carefully, Stepan -- 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] blame: add correct paddings in time_buf for align
When show blame information with relative time, the UTF-8 characters in
`time_str` will break the alignment of columns after the date field.
This is because the `time_buf` padding with spaces should have a
constant display width, not a fixed strlen size. So we should call
utf8_strwidth() instead of strlen() for calibration.
Signed-off-by: Jiang Xin worldhello@gmail.com
---
Before applying this patch:
5817da01 builtin-blame.c (Pierre Habouzit 6 年前
21) #include parse-options.h
ffaf9cc0 builtin-blame.c (Geoffrey Thomas 5 年前
22) #include utf8.h
3b8a12e8 builtin/blame.c (Axel Bonnet 3 年 10 个月之前
23) #include userdiff.h
25ed3412 builtin/blame.c (Bo Yang 1 年 1 个月之前
24) #include line-range.h
58dbfa2e builtin/blame.c (Eric Sunshine 9 个月之前
25) #include line-log.h
cee7f245 builtin-pickaxe.c (Junio C Hamano 8 年前
26)
After applying this patch:
5817da01 builtin-blame.c (Pierre Habouzit 6 年前
21) #include parse-options.h
ffaf9cc0 builtin-blame.c (Geoffrey Thomas 5 年前
22) #include utf8.h
3b8a12e8 builtin/blame.c (Axel Bonnet 3 年 10 个月之前
23) #include userdiff.h
25ed3412 builtin/blame.c (Bo Yang 1 年 1 个月之前
24) #include line-range.h
58dbfa2e builtin/blame.c (Eric Sunshine 9 个月之前
25) #include line-log.h
cee7f245 builtin-pickaxe.c (Junio C Hamano 8 年前
26)
builtin/blame.c | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/builtin/blame.c b/builtin/blame.c
index 88cb799..c8f6647 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -1564,12 +1564,19 @@ static const char *format_time(unsigned long time,
const char *tz_str,
else {
const char *time_str;
int time_len;
+ int time_col;
int tz;
tz = atoi(tz_str);
time_str = show_date(time, tz, blame_date_mode);
time_len = strlen(time_str);
memcpy(time_buf, time_str, time_len);
- memset(time_buf + time_len, ' ', blame_date_width - time_len);
+ /*
+* Add space paddings to time_buf to display a fixed width
+* string, and use time_col for display width calibration.
+*/
+ time_col = utf8_strwidth(time_str);
+ memset(time_buf + time_len, ' ', blame_date_width - time_col);
+ *(time_buf + time_len + blame_date_width - time_col) = 0;
}
return time_buf;
}
--
1.9.2.474.g17b2a16
--
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/2] add: add --ignore-submodules[=when] parameter
Am 13.04.2014 00:45, schrieb Ronald Weiss:
Allow ignoring submodules (or not) by command line switch, like diff
and status do.
Git add currently doesn't honor ignore from .gitmodules or .git/config,
which is related functionality, however I'd like to change that in
another patch, coming soon.
I think we should drop this paragraph from this commit message. Though
I believe it's helpful to add this information after the --- below
to inform readers of the list of your future plans.
This commit is also a prerequisite for the next one in series, which
adds the --ignore-submodules switch to git commit.
But this information definitely belongs here.
That's why signature
of function add_files_to_cache was changed.
But that's necessary for this patch too, no?
That also requires compilo fixes in checkout.c and commit.c
Sorry, but I don't know what a compilo fix is ;-) .. I suspect you
mean that add_files_to_cache() needs a new NULL argument in some
places. What about dropping the last two sentences and adding
something like Add a new argument to add_files_to_cache() to pass
the command line option and update all other call sites to pass
NULL instead. to the first paragraph?
Apart from that and the flags of the test (see below) this patch
is looking good to me.
Signed-off-by: Ronald Weiss weiss.ron...@gmail.com
---
Documentation/git-add.txt| 7 ++-
builtin/add.c| 16 --
builtin/checkout.c | 2 +-
builtin/commit.c | 2 +-
cache.h | 2 +-
t/t3704-add-ignore-submodules.sh | 45
6 files changed, 68 insertions(+), 6 deletions(-)
create mode 100644 t/t3704-add-ignore-submodules.sh
diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt
index 9631526..b2c936f 100644
--- a/Documentation/git-add.txt
+++ b/Documentation/git-add.txt
@@ -11,7 +11,7 @@ SYNOPSIS
'git add' [-n] [-v] [--force | -f] [--interactive | -i] [--patch | -p]
[--edit | -e] [--[no-]all | --[no-]ignore-removal | [--update | -u]]
[--intent-to-add | -N] [--refresh] [--ignore-errors]
[--ignore-missing]
- [--] [pathspec...]
+ [--ignore-submodules[=when]] [--] [pathspec...]
DESCRIPTION
---
@@ -164,6 +164,11 @@ for git add --no-all pathspec..., i.e. ignored
removed files.
be ignored, no matter if they are already present in the work
tree or not.
+--ignore-submodules[=when]::
+ Can be used to override any settings of the 'submodule.*.ignore'
+ option in linkgit:git-config[1] or linkgit:gitmodules[5].
+ when can be either none or all, which is the default.
+
\--::
This option can be used to separate command-line options from
the list of files, (useful when filenames might be mistaken
diff --git a/builtin/add.c b/builtin/add.c
index 459208a..85f2110 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -83,7 +83,8 @@ static void update_callback(struct diff_queue_struct *q,
}
int add_files_to_cache(const char *prefix,
-const struct pathspec *pathspec, int flags)
+const struct pathspec *pathspec, int flags,
+const char *ignore_submodules_arg)
{
struct update_callback_data data;
struct rev_info rev;
@@ -99,6 +100,12 @@ int add_files_to_cache(const char *prefix,
rev.diffopt.format_callback = update_callback;
rev.diffopt.format_callback_data = data;
rev.max_count = 0; /* do not compare unmerged paths with stage #2 */
+
+ if (ignore_submodules_arg) {
+ DIFF_OPT_SET(rev.diffopt, OVERRIDE_SUBMODULE_CONFIG);
+ handle_ignore_submodules_arg(rev.diffopt,
ignore_submodules_arg);
+ }
+
run_diff_files(rev, DIFF_RACY_IS_MODIFIED);
return !!data.add_errors;
}
@@ -237,6 +244,8 @@ static int ignore_add_errors, intent_to_add,
ignore_missing;
static int addremove = ADDREMOVE_DEFAULT;
static int addremove_explicit = -1; /* unspecified */
+static char *ignore_submodule_arg;
+
static int ignore_removal_cb(const struct option *opt, const char *arg, int
unset)
{
/* if we are told to ignore, we are not adding removals */
@@ -262,6 +271,9 @@ static struct option builtin_add_options[] = {
OPT_BOOL( 0 , refresh, refresh_only, N_(don't add, only refresh the
index)),
OPT_BOOL( 0 , ignore-errors, ignore_add_errors, N_(just skip files
which cannot be added because of errors)),
OPT_BOOL( 0 , ignore-missing, ignore_missing, N_(check if - even
missing - files are ignored in dry run)),
+ { OPTION_STRING, 0, ignore-submodules, ignore_submodule_arg,
N_(when),
+ N_(ignore changes to submodules, optional when: all, none. (Default:
all)),
+ PARSE_OPT_OPTARG, NULL, (intptr_t)all },
OPT_END(),
};
@@ -434,7 +446,7 @@ int cmd_add(int argc, const char **argv, const char
Re: [PATCH v4 2/2] commit: add --ignore-submodules[=when] parameter
Am 13.04.2014 00:49, schrieb Ronald Weiss:
Allow ignoring submodules (or not) by command line switch, like diff
and status do.
Git commit honors the 'ignore' setting from .gitmodules or .git/config,
but didn't allow to override it from command line.
This patch depends on Jens Lehmann's patch commit -m: commit staged
submodules regardless of ignore config. Without it,
commit -m --ignore-submodules would not work and tests introduced
here would fail.
Apart from the flags of the test (see below) I get three failures when
running t7513. And I'm wondering why you are using test_might_fail
there, shouldn't we know exactly if a commit should succeed or not
and test exactly for that?
Signed-off-by: Ronald Weiss weiss.ron...@gmail.com
---
Documentation/git-commit.txt| 7
builtin/commit.c| 8 +++-
t/t7513-commit-ignore-submodules.sh | 78
+
3 files changed, 92 insertions(+), 1 deletion(-)
create mode 100644 t/t7513-commit-ignore-submodules.sh
diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index 0bbc8f5..de0e8fe 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt
@@ -13,6 +13,7 @@ SYNOPSIS
[-F file | -m msg] [--reset-author] [--allow-empty]
[--allow-empty-message] [--no-verify] [-e] [--author=author]
[--date=date] [--cleanup=mode] [--[no-]status]
+[--ignore-submodules[=when]]
[-i | -o] [-S[key-id]] [--] [file...]
DESCRIPTION
@@ -277,6 +278,12 @@ The possible options are:
The default can be changed using the status.showUntrackedFiles
configuration variable documented in linkgit:git-config[1].
+--ignore-submodules[=when]::
+ Can be used to override any settings of the 'submodule.*.ignore'
+ option in linkgit:git-config[1] or linkgit:gitmodules[5].
+ when can be either none, dirty, untracked or all, which
+ is the default.
+
-v::
--verbose::
Show unified diff between the HEAD commit and what
diff --git a/builtin/commit.c b/builtin/commit.c
index a148e28..8c4d05e 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -361,7 +361,7 @@ static char *prepare_index(int argc, const char **argv,
const char *prefix,
*/
if (all || (also pathspec.nr)) {
fd = hold_locked_index(index_lock, 1);
- add_files_to_cache(also ? prefix : NULL, pathspec, 0, NULL);
+ add_files_to_cache(also ? prefix : NULL, pathspec, 0,
ignore_submodule_arg);
refresh_cache_or_die(refresh_flags);
update_main_cache_tree(WRITE_TREE_SILENT);
if (write_cache(fd, active_cache, active_nr) ||
@@ -1526,6 +1526,9 @@ int cmd_commit(int argc, const char **argv, const char
*prefix)
OPT_BOOL(0, amend, amend, N_(amend previous commit)),
OPT_BOOL(0, no-post-rewrite, no_post_rewrite, N_(bypass
post-rewrite hook)),
{ OPTION_STRING, 'u', untracked-files, untracked_files_arg,
N_(mode), N_(show untracked files, optional modes: all, normal, no.
(Default: all)), PARSE_OPT_OPTARG, NULL, (intptr_t)all },
+ { OPTION_STRING, 0, ignore-submodules, ignore_submodule_arg,
N_(when),
+ N_(ignore changes to submodules, optional when: all, none.
(Default: all)),
+ PARSE_OPT_OPTARG, NULL, (intptr_t)all },
/* end commit contents options */
OPT_HIDDEN_BOOL(0, allow-empty, allow_empty,
@@ -1564,6 +1567,9 @@ int cmd_commit(int argc, const char **argv, const char
*prefix)
argc = parse_and_validate_options(argc, argv, builtin_commit_options,
builtin_commit_usage,
prefix, current_head, s);
+
+ s.ignore_submodule_arg = ignore_submodule_arg;
+
if (dry_run)
return dry_run_commit(argc, argv, prefix, current_head, s);
index_file = prepare_index(argc, argv, prefix, current_head, 0);
diff --git a/t/t7513-commit-ignore-submodules.sh
b/t/t7513-commit-ignore-submodules.sh
new file mode 100644
Flags: 100755
index 000..52ab37d
--- /dev/null
+++ b/t/t7513-commit-ignore-submodules.sh
@@ -0,0 +1,78 @@
+#!/bin/sh
+#
+# Copyright (c) 2014 Ronald Weiss
+#
+
+test_description='Test of git commit --ignore-submodules'
+
+. ./test-lib.sh
+
+test_expect_success 'create submodule' '
+ test_create_repo sm (
+ cd sm
+ foo
+ git add foo
+ git commit -m Add foo
+ )
+ git submodule add ./sm
+ git commit -m Add sm
+'
+
+update_sm () {
+ (cd sm
+ echo bar foo
+ git add foo
+ git commit -m Updated foo
+ )
+}
+
+test_expect_success 'commit -a --ignore-submodules=all ignores dirty
submodule' '
+ update_sm
+ test_must_fail git commit -a
Re: [PATCH v2.1] commit: add --ignore-submodules[=when] parameter
Am 13.04.2014 01:41, schrieb Ronald Weiss:
On 8. 4. 2014 20:26, Jens Lehmann wrote:
Am 07.04.2014 23:46, schrieb Ronald Weiss:
Then, on top of that, I'll prepare patches for add to honor ignore
from .gitmodules, and -f implying --ignore-submodules. That might need
more discussion, let's see.
Makes sense.
I thought more about that, and also played with the code a bit.
First, I was confused when I wrote that git add doesn't honor
submodules' ignore setting only from .gitmodules, but it does from
.git/config. It doesn't, from neither. Sorry for the confusion. However,
that doesn't change anything on the fact that it would be nice if add
would honor the ignore setting, from both places.
Ok, thanks for digging deeper here.
Second, there are some differences between adding standard ignored
files, and ignored submodules:
1) Already tracked files are never ignored, regardless of .gitignore.
However, tracked submodules should be ignored by add -u, if told so
by their ignore setting.
2) So .gitignore seems to only do something when adding new files to
the repo. However, when adding new submodules, they are probably never
ignored (setting the ignore setting for non existent submodule seems
like non-sense, although possible).
What about diff.ignoreSubmodules=all?
3) Ignored files can be ignored less explicitely (in global gitignore,
or using a wildcard, or by ignoring parent folder). So it makes sense
to warn the user if he tries to explicitely add an ignored file, as he
might not be aware that the file is ignored. Submodules, however, can
only be ignored explicitely. And when user explicitely specifies the
submodule in an add command, he most probably really wants to add it,
so I don't see the point in warning him and requiring the -f option.
But we do have diff.ignoreSubmodules=all, so I do not agree with
your conclusion.
So, I think that the use cases are completely different, for submodules
and ignored files. So trying to make add behave the same for both, might
not be that good idea.
I would propose - let's make add honor the ignore setting by just
parsing if from config like the other commands do, and pass it to
underlying diff invocations. And at the same the, let's override it for
submodules explicitely specified on the command line, to never ignore
such submodules, without requiring the -f option. That seems to be
pretty easy, see below.
What about doing that only when '-f' is given? Would that do what we
want?
diff --git a/builtin/add.c b/builtin/add.c
index 85f2110..f19e6c8 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -284,6 +284,10 @@ static int add_config(const char *var, const char
*value, void *cb)
ignore_add_errors = git_config_bool(var, value);
return 0;
}
+
+ if (starts_with(var, submodule.))
+ return parse_submodule_config_option(var, value);
+
return git_default_config(var, value, cb);
}
@@ -320,6 +324,7 @@ int cmd_add(int argc, const char **argv, const char
*prefix)
char *seen = NULL;
git_config(add_config, NULL);
+ gitmodules_config();
Wrong order, gitmodules_config() must be called before git_config()
to make the .gitmodules settings overridable by the users config.
argc = parse_options(argc, argv, prefix, builtin_add_options,
builtin_add_usage, PARSE_OPT_KEEP_ARGV0);
@@ -425,6 +430,7 @@ int cmd_add(int argc, const char **argv, const char
*prefix)
PATHSPEC_EXCLUDE);
for (i = 0; i pathspec.nr; i++) {
+ int cachepos;
const char *path = pathspec.items[i].match;
if (pathspec.items[i].magic PATHSPEC_EXCLUDE)
continue;
@@ -440,6 +446,18 @@ int cmd_add(int argc, const char **argv, const char
*prefix)
die(_(pathspec '%s' did not match any
files),
pathspec.items[i].original);
}
+
+ /* disable ignore setting for any submodules specified
explicitly in the pathspec */
+ if (path[0]
+ (cachepos = cache_name_pos(path,
pathspec.items[i].len)) = 0
+ S_ISGITLINK(active_cache[cachepos]-ce_mode)) {
+ char *optname;
+ int optnamelen = pathspec.items[i].len + 17;
+ optname = xcalloc(optnamelen + 1, 1);
+ snprintf(optname, optnamelen + 1,
submodule.%s.ignore, path);
+ parse_submodule_config_option(optname, none);
We should use dirty instead of none here. Modifications inside
the submodule cannot be added without committing them there first
and using none might incur an extra preformance penalty for
looking
Re: [git] [RFC/PATCH 2/4] Submodules: Add the lib-submodule-update.sh test library
Am 17.04.2014 23:55, schrieb W. Trevor King: On Thu, Apr 17, 2014 at 11:08:06PM +0200, Jens Lehmann wrote: Am 17.04.2014 18:41, schrieb W. Trevor King: On Tue, Mar 25, 2014 at 06:05:05PM +0100, Jens Lehmann wrote: *) When a submodule is replaced with a tracked file of the same name the submodule work tree including any local modifications (and even the whole history if it uses a .git directory instead of a gitfile!) is simply removed. … I think the first bug really needs to be fixed, as that behavior is extremely nasty. We should always protect work tree modifications (unless forced) and *never* remove a .git directory (even when forced). I think this should be covered by the usual “don't allow checkouts from dirty workdirs unless the dirty-ing changes are easily applied to the target tree”. Nope, the target tree will be removed completely and everything in it is silently nuked. It should be allowed with '-f', but only if the submodule contains a gitfile, and never if it contains a .git directory (which is just what we do for rm too). I think it's not covered *now* because of a flaw in our “are dirty-ing changes easily applied to the target tree” detection logic, and the solution should involve updating that logic to hit on this case. Yup. b) recursive checkout is the place to consistently care about submodule modifications (the submodule script doesn't do that and it is impossible to change that without causing trouble to a lot of users. I agree that the submodule script is not the place for this, and the core checkout code is. I'd like checkouts to always be recursive, and see --[no-]recurse-submodules as a finger-breaking stop-gap until we can complete that transition for checkout, bisect, merge, reset, and other work-tree altering commands. I think this is one reason why some folks prefer the stiffer joints you get from a subtree approach. We are definitely in the same boat here :-) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Re: [RFC/PATCH 2/4] Submodules: Add the lib-submodule-update.sh test library
On Thu, Apr 17, 2014 at 11:30:04PM +0200, Jens Lehmann wrote: - Heiko's config cache for submodules patch Needed for my recursive checkout series to populate new submodules. Which will be followed by my recursive fetch series, which is also the last part of what started with this: http://thread.gmane.org/gmane.comp.version-control.git/217018 and is described here: https://github.com/hvoigt/git/wiki/submodule-fetch-config Cheers Heiko -- 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
Project idea: strbuf allocation modes
I like that strbuf is getting used more than it used to, and I think
that is a good general trend to help git rid of and/or avoid buffer
overflows and arbitrary limits on strings.
It is unfortunate that it is currently impossible to use a strbuf
without doing a memory allocation. So code like
void f()
{
char path[PATH_MAX];
...
}
typically gets turned into either
void f()
{
struct strbuf path;
strbuf_add(path, ...); -- does a malloc
...
strbuf_release(path);-- does a free
}
which costs extra memory allocations, or
void f()
{
static struct strbuf path;
strbuf_add(path, ...);
...
strbuf_setlen(path, 0);
}
which, by using a static variable, avoids most of the malloc/free
overhead, but makes the function unsafe to use recursively or from
multiple threads.
The Idea
I would like to see strbuf enhanced to allow it to use memory that it
doesn't own (for example, stack-allocated memory), while (optionally)
allowing it to switch over to using allocated memory if the string grows
past the size of the pre-allocated buffer.
The way I envision this, strbuf would gain a flags field with two bits:
STRBUF_OWNS_MEMORY
The memory pointed to by buf is owned by this strbuf. If this
bit is not set, then the memory should never be freed, and
(among other things) strbuf_detach() must always call xstrcpy().
STRBUF_FIXED_MEMORY
This strbuf can use the memory of length alloc at buf however
it likes. But it must never free() or realloc() the memory or
move the string. Essentially, buf should be treated as a
constant pointer. If the string overgrows the buffer, die().
The different bit combinations would have the following effects:
flags == ~STRBUF_OWNS_MEMORY | ~STRBUF_FIXED_MEMORY
The strbuf doesn't own the current memory, but it is allowed to
grow the buffer into other (allocated) memory. When needed,
malloc() new memory, copy the old string to the new memory,
and clear this bit -- but *don't* free the old memory. Note
that STRBUF_INIT, when buf is initialized to point at
strbuf_slopbuf, would use this flag setting. This flag
combination could be used to wrap a stack-allocated buffer
without preventing the string from growing beyond the
size of the buffer:
void f()
{
char path_buf[PATH_MAX];
struct strbuf path;
strbuf_wrap_preallocated(path, path_buf, 0, sizeof(path));
...
strbuf_release(path);
}
(Probably the boilerplate could be reduced by using macros.)
flags == STRBUF_OWNS_MEMORY | ~STRBUF_FIXED_MEMORY
This gives the current behavior of a non-empty strbuf.
flags == ~STRBUF_OWNS_MEMORY | STRBUF_FIXED_MEMORY
This would make it possible to use a strbuf to wrap a
fixed-size buffer that belongs to somebody else and must not
be moved. For example,
void f(char *path_buf, size_t path_buf_len)
{
struct strbuf path;
strbuf_wrap_fixed(path, path_buf,
strlen(path_buf), path_buf_len);
...
/*
* no strbuf_release() required here, but if called it
* is a NOOP
*/
}
(Probably the boilerplate could be reduced by using macros.)
It would also be possible to use this mode to dynamically
allocate a strbuf along with space for its string (i.e., to
make do with one allocation instead of two):
struct strbuf sb = xmalloc(sizeof(strbuf) + len + 1);
sb.alloc = len + 1;
sb.len = len;
sb.flags = STRBUF_FIXED_MEMORY;
sb.buf = (void *)sb + sizeof(strbuf);
*sb.buf = '\0';
If this is considered useful, it should of course be offered
via a function.
flags == STRBUF_OWNS_MEMORY | STRBUF_FIXED_MEMORY
This combination seems less useful, but I leave it here for
completeness. It is basically a strbuf whose length is
constrained to its currently-allocated size.
The nice thing is that I believe this new functionality could be
implemented without slowing down the most-used strbuf functions. For
example, none of the inline functions would have to be changed. The
flags would only have to be checked when doing memory-related operations
like strbuf_grow(), strbuf_detach(), etc.
Anyway, I just wanted to get the idea out there. I'd like to get
feedback about whether people think this is a good idea. If so, maybe
I'll add it to the wiki as a suggested project.
I've wanted to work on this myself, but realistically I'm not going to
have time in the near future. It might be a good Ensimag project or a
future GSoC project (though it might be a bit
Store refreshed stat info in a separate file?
With git status, writing refreshed index takes 252ms per total 1s, 361s/1.4s, 86ms/360ms on gentoo-x86, webkit and linux-2.6 respectively (*). It's takes a significant amount of time from git status. And this happens whenever you touch a single tracked file, then do git status. We tried to solve this with index v5, but it's been years(?) since its start as a GSoC project. So I'm thinking of another way around.. The major cost of writing an index is the SHA-1 hashing. The bigger the written part is, the higher cost we pay. So what if we write stat-only data to a separate file? Think of it as an index extension, only it stays outside the index. On webkit with 182k files, the stat data size would be about 6MB (its index v4 is 15M for comparison). But with stat-only we could employ some cheap but efficient compressing, sd_dev, sd_uid and sd_gid are likely the same for every entry. And we could store the stat data of updated entries only. So I'm hoping to get that 6MB down to a few hundred KBs. That makes hashing lightning fast. So the idea is, when we do refresh, we note what entry has stat updated. Then we write $GIT_DIR/index.stat (and leave $GIT_DIR/index alone), which is a valid index except that it has zero entries and a only one (new) extension storing (maybe compressed) stat data of updated entries. The extension also contains the trailing SHA-1 of $GIT_DIR/index for verification later. When we read $GIT_DIR/index, we check for the existence of index.stat. If it does and its attached SHA-1 matches, we overwrite some stat data with the info from index.stat. Back to the original question, I'm hoping to reduce some significant numbers above to less than 10ms with this. So I see all good points but no bad ones. Time to ask git@vger to give some. I'm actually trying this idea in my untracked cache because I can't afford to lose 50% of the gain from untracked cache, just because I have to save some bits in the giant $GIT_DIR/index and take the cost of rehashing. (*) this is with the untracked cache enabled and total time is about 40% less than upstream git status. The numbers against upstream git status are actually less signficant. But I have to think positive that one day untracked cache may be merged :) -- 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
Re: [PATCH] blame: add correct paddings in time_buf for align
On Fri, Apr 18, 2014 at 3:44 PM, Jiang Xin worldhello@gmail.com wrote:
When show blame information with relative time, the UTF-8 characters in
`time_str` will break the alignment of columns after the date field.
This is because the `time_buf` padding with spaces should have a
constant display width, not a fixed strlen size. So we should call
utf8_strwidth() instead of strlen() for calibration.
Signed-off-by: Jiang Xin worldhello@gmail.com
---
Before applying this patch:
5817da01 builtin-blame.c (Pierre Habouzit 6 年前
21) #include parse-options.h
ffaf9cc0 builtin-blame.c (Geoffrey Thomas 5 年前
22) #include utf8.h
3b8a12e8 builtin/blame.c (Axel Bonnet 3 年 10 个月之前
23) #include userdiff.h
25ed3412 builtin/blame.c (Bo Yang 1 年 1 个月之前
24) #include line-range.h
58dbfa2e builtin/blame.c (Eric Sunshine 9 个月之前
25) #include line-log.h
cee7f245 builtin-pickaxe.c (Junio C Hamano 8 年前
26)
After applying this patch:
5817da01 builtin-blame.c (Pierre Habouzit 6 年前
21) #include parse-options.h
ffaf9cc0 builtin-blame.c (Geoffrey Thomas 5 年前
22) #include utf8.h
3b8a12e8 builtin/blame.c (Axel Bonnet 3 年 10 个月之前
23) #include userdiff.h
25ed3412 builtin/blame.c (Bo Yang 1 年 1 个月之前
24) #include line-range.h
58dbfa2e builtin/blame.c (Eric Sunshine 9 个月之前
25) #include line-log.h
cee7f245 builtin-pickaxe.c (Junio C Hamano 8 年前
26)
The numbers 21..26 still do not look aligned, both in gmail raw
message view and gmane.
builtin/blame.c | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/builtin/blame.c b/builtin/blame.c
index 88cb799..c8f6647 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -1564,12 +1564,19 @@ static const char *format_time(unsigned long time,
const char *tz_str,
else {
const char *time_str;
int time_len;
+ int time_col;
int tz;
tz = atoi(tz_str);
time_str = show_date(time, tz, blame_date_mode);
time_len = strlen(time_str);
memcpy(time_buf, time_str, time_len);
- memset(time_buf + time_len, ' ', blame_date_width - time_len);
+ /*
+* Add space paddings to time_buf to display a fixed width
+* string, and use time_col for display width calibration.
+*/
+ time_col = utf8_strwidth(time_str);
+ memset(time_buf + time_len, ' ', blame_date_width - time_col);
+ *(time_buf + time_len + blame_date_width - time_col) = 0;
And you may want to turn time_buf[128] to strbuf as well while you're at it.
}
return time_buf;
}
--
1.9.2.474.g17b2a16
--
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
--
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
Re: [PATCH] tag: add -i and --introduced modifier for --contains
Jeff King p...@peff.net writes: ---A---B---C-D---E---F (maint, v3.4) \ \ / \ ---G-H---I (master, v4.0) \ / / --J--- The fix is J, and it got merged up to maint at D, and to master at H. v4.0 does not contain v3.4. What's the best description of J? By the rules above, we hit the third rule pick the closest. Which means we choose v3.4 or v4.0 based solely on how many commits are between the topic's merge and the tag release. Which has nothing at all to do with the topic itself. Even if J..F and J..I were of the same hop-count, there is no fundamental reason to choose one over the other. What is best at that point depends on what the user wants to see. - Luis's case that started this thread may want to favor v3.4 if only because that sounds the smaller, even though v3.4 and v4.0 in the illustration cannot be compared. - I think the closest we have had is primarily a heuristic to favour the result that is textually shorter. - And as I alluded to, which one has the earliest timestamp?, is another valid question to ask. In other words, there is no single correct answer, once you have multiple canidates that are all valid from topological point of view. In this case we'd show v4.0 (because J-H-I is shorter than J-D-E-F). But I suspect most users would want to know v3.4, because they want to know the oldest release they can move up to that contains the commit. But that notion of oldness is not conveyed by the graph above; it's only an artifact of the tag names. Yes, exactly. -- 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/3] i18n: Only extract comments marked by special tag
Jiang Xin worldhello@gmail.com writes: I am not very happy with this change, as it would force us to special case Translators comment to follow a non-standard multi-line comment formatting convention. Is there a way to tell xgettext to accept both of these forms? /* TRANSLATORS: this is a short comment to help you */ _(foo bar); /* * TRANSLATORS: this comment is to help you, but it is * a lot longer to fit on just a single line. */ _(bar baz); We can not provide multiple `--add-comments=TAG` options to xgettext, because xgettext holds the tag in one string, not in a list: /* Tag used in comment of prevailing domain. */ static char *comment_tag; So if we won't change our multi-line comments for translators, must hack gettext in some ways. There maybe 3 ways to hack gettext: ... I CC this mail to the gettext mailing list. Full thread see: * http://thread.gmane.org/gmane.comp.version-control.git/246390/focus=246431 This is one of these times when I find myself very fortunate for being surrounded by competent contributors with good tastes, which I may not deserve ;-) Thanks for being thorough. Having said that, it is only just a single comment, and it is too much hassle to even think about what to do in the meantime while we wait until such a change happens and an updated version of gettext reaches everybody. Let's take 2/3 as-is. Documentation/CodingGuidelines may want to have a sentence of two to explain this, though. Documentation/CodingGuidelines | 9 + 1 file changed, 9 insertions(+) diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines index dab5c61..b367a85 100644 --- a/Documentation/CodingGuidelines +++ b/Documentation/CodingGuidelines @@ -159,10 +159,19 @@ For C programs: - Multi-line comments include their delimiters on separate lines from the text. E.g. /* * A very long * multi-line comment. */ + Note however that a multi-line comment that explains a translatable + string to translators uses a different convention of starting with a + magic token TRANSLATORS: immediately after the opening delimiter, + and without an asterisk at the beginning of each line. E.g. + + /* TRANSLATORS: here is a comment that explains the string + to be translated, that follows immediately after it */ + _(Here is a translatable string explained by the above.); + - Double negation is often harder to understand than no negation at all. -- 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] blame: add correct paddings in time_buf for align
Duy Nguyen pclo...@gmail.com writes: On Fri, Apr 18, 2014 at 3:44 PM, Jiang Xin worldhello@gmail.com wrote: When show blame information with relative time, the UTF-8 characters in `time_str` will break the alignment of columns after the date field. This is because the `time_buf` padding with spaces should have a constant display width, not a fixed strlen size. So we should call utf8_strwidth() instead of strlen() for calibration. Signed-off-by: Jiang Xin worldhello@gmail.com --- Before applying this patch: 5817da01 builtin-blame.c (Pierre Habouzit 6 年前 21) #include parse-options.h ffaf9cc0 builtin-blame.c (Geoffrey Thomas 5 年前 22) #include utf8.h 3b8a12e8 builtin/blame.c (Axel Bonnet 3 年 10 个月之前 23) #include userdiff.h 25ed3412 builtin/blame.c (Bo Yang 1 年 1 个月之前 24) #include line-range.h 58dbfa2e builtin/blame.c (Eric Sunshine 9 个月之前 25) #include line-log.h cee7f245 builtin-pickaxe.c (Junio C Hamano 8 年前 26) After applying this patch: 5817da01 builtin-blame.c (Pierre Habouzit 6 年前 21) #include parse-options.h ffaf9cc0 builtin-blame.c (Geoffrey Thomas 5 年前 22) #include utf8.h 3b8a12e8 builtin/blame.c (Axel Bonnet 3 年 10 个月之前 23) #include userdiff.h 25ed3412 builtin/blame.c (Bo Yang 1 年 1 个月之前 24) #include line-range.h 58dbfa2e builtin/blame.c (Eric Sunshine 9 个月之前 25) #include line-log.h cee7f245 builtin-pickaxe.c (Junio C Hamano 8 年前 26) The numbers 21..26 still do not look aligned, both in gmail raw message view and gmane. Interesting. In my GNUS/Emacs on a fixed-column terminal where an CJK occupies two display columns, they do look aligned, but in my Chrome showing news.gmane.org/gmane.comp.version-control.git/, they do look jagged. When these lines shown in the browser get copied and pasted to gedit, they still look jagged, but after saving it to a file and catting it to the same fixed-column terminal, they are perfectly aligned. Font issues, I guess? + /* +* Add space paddings to time_buf to display a fixed width +* string, and use time_col for display width calibration. +*/ + time_col = utf8_strwidth(time_str); + memset(time_buf + time_len, ' ', blame_date_width - time_col); + *(time_buf + time_len + blame_date_width - time_col) = 0; And you may want to turn time_buf[128] to strbuf as well while you're at it. Good eyes. -- 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: Project idea: strbuf allocation modes
Michael Haggerty mhag...@alum.mit.edu writes: The Idea I would like to see strbuf enhanced to allow it to use memory that it doesn't own (for example, stack-allocated memory), while (optionally) allowing it to switch over to using allocated memory if the string grows past the size of the pre-allocated buffer. I'd like to see these characteristics, but I would want to see that this is done entirely internally inside the strbuf implementation without any API impact, other than the initialisation. I do not think the current API contract is too rigid to allow us doing so. - The API users may peek strbuf.buf in-place until they perform an operation that makes it longer (at which point the .buf pointer may point at a new piece of memory). - The API users may strbuf_detach() to obtain a piece of memory that belongs to them (at which point the strbuf becomes empty), hence needs to be freed by the callers. As long as the above two promises are kept intact, it is all internal to the strbuf implementation, current iteration of which does not have any initial (possibly static) allocation other than the fixed terminating NUL, but your updated one may take a caller supplied piece of memory that is designed to outlive the strbuf itself as its initial allocation and the memory ownership can be left as an internal implementation detail to the strbuf without bothering the callers. -- 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: Store refreshed stat info in a separate file?
Duy Nguyen pclo...@gmail.com writes: The major cost of writing an index is the SHA-1 hashing. The bigger the written part is, the higher cost we pay. So what if we write stat-only data to a separate file? Think of it as an index extension, only it stays outside the index. On webkit with 182k files, the stat data size would be about 6MB (its index v4 is 15M for comparison). But with stat-only we could employ some cheap but efficient compressing, sd_dev, sd_uid and sd_gid are likely the same for every entry. And we could store the stat data of updated entries only. So I'm hoping to get that 6MB down to a few hundred KBs. That makes hashing lightning fast. It is perfectly OK to store your verbose stat data after deflating it in the index as an index extension, so storing 6MB that can be compressed efficiently without compressing is dumb applies whether the result is stored in the index or in a separate file, I would think. Having said that, I do not think there is a fundamental reason why the stat data has to live inside the same index file. A separate file is just fine, as long as you can reliably detect that they went out of sync for whatever reason (e.g. the index proper updated, a stale stat file left beind), and storing the trailer checksum from the corresponding index in this new file is an obvious and good solution. I am not sure if that should be called index.stat, though. It is more about untracked files. The stat data for cached paths are in the index proper, so what you are adding is not what we would call stat info when we talk about the index. -- 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/3] i18n: Only extract comments marked by special tag
Junio C Hamano gits...@pobox.com writes: Documentation/CodingGuidelines may want to have a sentence of two to explain this, though. After re-reading what I sent out, I realized that the way I singled out multi-line comments was misleading. Here is an updated version. -- 8 -- Subject: [PATCH] i18n: mention TRANSLATORS: marker in Documentation/CodingGuidelines These comments have to have TRANSLATORS: at the very beginning and have to deviate from the usual multi-line comment formatting convention. Signed-off-by: Junio C Hamano gits...@pobox.com --- Documentation/CodingGuidelines | 10 ++ 1 file changed, 10 insertions(+) diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines index dab5c61..f9b8bff 100644 --- a/Documentation/CodingGuidelines +++ b/Documentation/CodingGuidelines @@ -164,6 +164,16 @@ For C programs: * multi-line comment. */ + Note however that a comment that explains a translatable string to + translators uses a convention of starting with a magic token + TRANSLATORS: immediately after the opening delimiter, even when + it spans multiple lines. We do not add an asterisk at the beginning + of each line, either. E.g. + + /* TRANSLATORS: here is a comment that explains the string + to be translated, that follows immediately after it */ + _(Here is a translatable string explained by the above.); + - Double negation is often harder to understand than no negation at all. -- 1.9.2-651-g78816bc -- 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
[ANNOUNCE] Git v2.0.0-rc0
An early preview release Git v2.0.0-rc0 is now available for
testing at the usual places.
A major version bump between v1.x.x series and the upcoming v2.0.0
means there are a handful of backward incompatible UI improvements,
but for most people, all the tricky preparation for the transition
would have been already done for you and the upcoming release just
flips the default. Unless you were living in a cave and have stayed
with an ancient version of Git (e.g. one before 1.8.2 that was
released more than a year ago) for all these times, that is---those
of you may want to double check the backward compatibility notes
section at the beginning of the draft release notes.
The tarballs are found at:
https://www.kernel.org/pub/software/scm/git/testing/
The following public repositories all have a copy of the 'v2.0.0-rc0'
tag and the 'master' branch that the tag points at:
url = https://kernel.googlesource.com/pub/scm/git/git
url = git://repo.or.cz/alt-git.git
url = https://code.google.com/p/git-core/
url = git://git.sourceforge.jp/gitroot/git-core/git.git
url = git://git-core.git.sourceforge.net/gitroot/git-core/git-core
url = https://github.com/gitster/git
Git v2.0 Release Notes (draft)
==
Backward compatibility notes
When git push [$there] does not say what to push, we have used the
traditional matching semantics so far (all your branches were sent
to the remote as long as there already are branches of the same name
over there). In Git 2.0, the default is now the simple semantics,
which pushes:
- only the current branch to the branch with the same name, and only
when the current branch is set to integrate with that remote
branch, if you are pushing to the same remote as you fetch from; or
- only the current branch to the branch with the same name, if you
are pushing to a remote that is not where you usually fetch from.
You can use the configuration variable push.default to change
this. If you are an old-timer who wants to keep using the
matching semantics, you can set the variable to matching, for
example. Read the documentation for other possibilities.
When git add -u and git add -A are run inside a subdirectory
without specifying which paths to add on the command line, they
operate on the entire tree for consistency with git commit -a and
other commands (these commands used to operate only on the current
subdirectory). Say git add -u . or git add -A . if you want to
limit the operation to the current directory.
git add path is the same as git add -A path now, so that
git add dir/ will notice paths you removed from the directory and
record the removal. In older versions of Git, git add path used
to ignore removals. You can say git add --ignore-removal path to
add only added or modified paths in path, if you really want to.
The -q option to git diff-files, which does *NOT* mean quiet,
has been removed (it told Git to ignore deletion, which you can do
with git diff-files --diff-filter=d).
git request-pull lost a few heuristics that often led to mistakes.
Updates since v1.9 series
-
UI, Workflows Features
* The multi-mail post-receive hook (in contrib/) has been updated
to a more recent version from the upstream.
* git gc --aggressive learned --depth option and
gc.aggressiveDepth configuration variable to allow use of a less
insane depth than the built-in default value of 250.
* git log learned the --show-linear-break option to show where a
single strand-of-pearls is broken in its output.
* The rev-parse --parseopt mechanism used by scripted Porcelains to
parse command line options and to give help text learned to take
the argv-help (the placeholder string for an option parameter,
e.g. key-id in --gpg-sign=key-id).
* The pattern to find where the function begins in C/C++ used in
diff and grep -p have been updated to help C++ source better.
* git rebase learned to interpret a lone - as @{-1}, the
branch that we were previously on.
* git commit --cleanup=mode learned a new mode, scissors.
* git tag --list output can be sorted using version sort with
--sort=version:refname.
* Discard the accumulated heuristics to guess from which branch the
result wants to be pulled from and make sure what the end user
specified is not second-guessed by git request-pull, to avoid
mistakes. When you pushed out your 'master' branch to your public
repository as 'for-linus', use the new master:for-linus syntax to
denote the branch to be pulled.
* git grep learned to behave in a way similar to native grep when
-h (no header) and -c (count) options are given.
* transport-helper, fast-import and fast-export have been updated to
allow the ref mapping and ref deletion in a way similar to the
natively supported transports.
* The simple mode is the default for git push.
* git add -u and git add -A, when run without any pathspec, is
What's cooking in git.git (Apr 2014, #06; Fri, 18)
Here are the topics that have been cooking. Commits prefixed with
'-' are only in 'pu' (proposed updates) while commits prefixed with
'+' are in 'next'.
The tip of the 'master' branch is at v2.0.0-rc0, an early preview
release. There are a handful of topics still in 'next' (and a few
that are not in 'next') that I'd like to have in v2.0.0-rc1, but
other than that, this hopefully is fairly a close approximation of
the upcoming release.
Also I am still waiting for acks for a few topics before merging
them to 'next'. I would feel it would be good if we can merge them
early to 'next' for wider and longer exposure and some of them may
even deserve to be in -rc1, but as none of them is a regression fix,
it is also OK to wait until 2.0 final.
You can find the changes described here in the integration branches
of the repositories listed at
http://git-blame.blogspot.com/p/git-public-repositories.html
--
[Graduated to master]
* fc/complete-aliased-push (2014-04-09) 1 commit
(merged to 'next' on 2014-04-16 at ab798d1)
+ completion: fix completing args of aliased push, fetch, etc.
* fc/prompt-zsh-read-from-file (2014-04-14) 1 commit
(merged to 'next' on 2014-04-16 at 0e5fec0)
+ prompt: fix missing file errors in zsh
* fc/remote-helper-fixes (2014-04-14) 5 commits
(merged to 'next' on 2014-04-16 at 180482e)
+ remote-bzr: trivial test fix
+ remote-bzr: include authors field in pushed commits
+ remote-bzr: add support for older versions
+ remote-hg: always normalize paths
+ remote-helpers: allow all tests running from any dir
* jk/config-die-bad-number-noreturn (2014-04-16) 1 commit
(merged to 'next' on 2014-04-16 at 4d49036)
+ config.c: mark die_bad_number as NORETURN
Squelch a false compiler warning from older gcc.
--
[Stalled]
* fc/publish-vs-upstream (2014-04-11) 8 commits
- sha1_name: add support for @{publish} marks
- sha1_name: simplify track finding
- sha1_name: cleanup interpret_branch_name()
- branch: display publish branch
- push: add --set-publish option
- branch: add --set-publish-to option
- Add concept of 'publish' branch
- t5516 (fetch-push): fix test restoration
Add branch@{publish}; it seems that this is somewhat different from
Ram and Peff started working on. There are still many discussion
messages going back and forth but not updates to the patches.
Seems to have some interactions to break t5516 when merged to 'pu'.
* tr/merge-recursive-index-only (2014-02-05) 3 commits
- merge-recursive: -Xindex-only to leave worktree unchanged
- merge-recursive: internal flag to avoid touching the worktree
- merge-recursive: remove dead conditional in update_stages()
(this branch is used by tr/remerge-diff.)
Will hold.
* tr/remerge-diff (2014-02-26) 5 commits
. log --remerge-diff: show what the conflict resolution changed
. name-hash: allow dir hashing even when !ignore_case
. merge-recursive: allow storing conflict hunks in index
. revision: fold all merge diff variants into an enum merge_diff_mode
. combine-diff: do not pass revs-dense_combined_merges redundantly
(this branch uses tr/merge-recursive-index-only.)
log -p output learns a new way to let users inspect a merge
commit by showing the differences between the automerged result
with conflicts the person who recorded the merge would have seen
and the final conflict resolution that was recorded in the merge.
Needs to be rebased, now kb/fast-hashmap topic is in.
* bc/blame-crlf-test (2014-02-18) 1 commit
- blame: add a failing test for a CRLF issue.
I have a feeling that a fix for this should be fairly isolated and
trivial (it should be just the matter of paying attention to the
crlf settings when synthesizing the fake commit)---perhaps somebody
can squash in a fix to this?
* jk/makefile (2014-02-05) 16 commits
- FIXUP
- move LESS/LV pager environment to Makefile
- Makefile: teach scripts to include make variables
- FIXUP
- Makefile: auto-build C strings from make variables
- Makefile: drop *_SQ variables
- FIXUP
- Makefile: add c-quote helper function
- Makefile: introduce sq function for shell-quoting
- Makefile: always create files via make-var
- Makefile: store GIT-* sentinel files in MAKE/
- Makefile: prefer printf to echo for GIT-*
- Makefile: use tempfile/mv strategy for GIT-*
- Makefile: introduce make-var helper function
- Makefile: fix git-instaweb dependency on gitweb
- Makefile: drop USE_GETTEXT_SCHEME from GIT-CFLAGS
Simplify the Makefile rules and macros that exist primarily for
quoting purposes, and make it easier to robustly express the
dependency rules.
Expecting a reroll.
* po/everyday-doc (2014-01-27) 1 commit
- Make 'git help everyday' work
This may make the said command to emit something, but the source is
not meant to be formatted into a manual pages to begin with, and
also its contents are a bit stale. It may be a good first
user.signingkey without gpg? (using s/mime or ssh?)
Dear Git Community I've spent almost a day to find an answer to this question: We already have trusted Certificates from a CA. Can we use them instead of an additional PGP key? We already have: - s/mime certificate - web server ssl/tls certificate - XMPP Jabber ssl/tls certificate - Object Code Signing certificate Or if we have to use a new pgp key: can we sign it using any of our certificates? Thanks a lot for any hint in advance!, kind regards, Thomas -- 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: Project idea: strbuf allocation modes
On 04/18/2014 07:21 PM, Junio C Hamano wrote: Michael Haggerty mhag...@alum.mit.edu writes: The Idea I would like to see strbuf enhanced to allow it to use memory that it doesn't own (for example, stack-allocated memory), while (optionally) allowing it to switch over to using allocated memory if the string grows past the size of the pre-allocated buffer. I'd like to see these characteristics, but I would want to see that this is done entirely internally inside the strbuf implementation without any API impact, other than the initialisation. I do not think the current API contract is too rigid to allow us doing so. - The API users may peek strbuf.buf in-place until they perform an operation that makes it longer (at which point the .buf pointer may point at a new piece of memory). - The API users may strbuf_detach() to obtain a piece of memory that belongs to them (at which point the strbuf becomes empty), hence needs to be freed by the callers. As long as the above two promises are kept intact, it is all internal to the strbuf implementation, current iteration of which does not have any initial (possibly static) allocation other than the fixed terminating NUL, but your updated one may take a caller supplied piece of memory that is designed to outlive the strbuf itself as its initial allocation and the memory ownership can be left as an internal implementation detail to the strbuf without bothering the callers. I think that's exactly what I described. The only thing that would have to change in the strbuf behavior (aside from initialization) is that a buffer-growing operation might die if the string would grow outside of the bounds of the existing buffer when STRBUF_FIXED_MEMORY is set. Michael -- Michael Haggerty mhag...@alum.mit.edu http://softwareswirl.blogspot.com/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: user.signingkey without gpg? (using s/mime or ssh?)
On Fri, Apr 18, 2014 at 10:04:50PM +0200, Thomas Schittli wrote: We already have trusted Certificates from a CA. Can we use them instead of an additional PGP key? Git wants a key that can be used by GnuPG, and X.509 certificates can't be. It invokes the gpg binary that's in your path, so X.509 integration isn't possible unless gpg learns about it. We already have: - s/mime certificate - web server ssl/tls certificate - XMPP Jabber ssl/tls certificate - Object Code Signing certificate Or if we have to use a new pgp key: can we sign it using any of our certificates? Only in the sense that you can sign any arbitrary piece of text or data with your certificates. -- brian m. carlson / brian with sandals: Houston, Texas, US +1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187 signature.asc Description: Digital signature
Refactoring hardcoded SHA-1 constants
SHA-1 is clearly on its way out. I know that there has been discussion in the past about moving to different algorithms. I'm not interested in having that discussion now. I'd like to introduce a set of preprocessor constants that we'd use instead of hard-coded 20s and 40s everywhere. That way, if we decide to support a different algorithm, doing so becomes significantly easier. These would be used in new code. After that, I'd like to slowly start moving existing code to use these constants. I would also like to consider, as a third step, turning all of the unsigned char[20] uses into a struct containing unsigned char[20] as its only member, like libgit2 does. This disambiguates arbitrary byte data from byte sequences being used to represent an SHA-1 ID. I'm hoping the first suggestion will be mostly unobjectionable, as having hard-coded constants is widely considered bad programming practice. I suspect the latter two will be more controversial. These changes, if implemented, would be done by hand, not by machine, and they would be bisectable. Comments? -- brian m. carlson / brian with sandals: Houston, Texas, US +1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187 signature.asc Description: Digital signature
Re: Refactoring hardcoded SHA-1 constants
Hi, brian m. carlson wrote: I'd like to introduce a set of preprocessor constants that we'd use instead of hard-coded 20s and 40s everywhere. Lukewarm on that. It's hard to do consistently and unless they're named well it can be harder to know what something like BINARY_OBJECT_NAME_LENGTH means than plain '20' when first reading. [...] I would also like to consider, as a third step, turning all of the unsigned char[20] uses into a struct containing unsigned char[20] as its only member, like libgit2 does. That would be very welcome! It's a nice way to steer people toward hashcmp using the type system, and it makes it possible to use a union to enforce alignment later if measurements show benefit. Thanks, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] tag: add -i and --introduced modifier for --contains
On Thu, Apr 17, 2014 at 10:04 AM, Junio C Hamano gits...@pobox.com wrote: Luis R. Rodriguez mcg...@do-not-panic.com writes: And between v3.4 and v3.5-rc1, the latter is a closer anchor point for that commit (v3.5-rc1 only needs about 200 hops to reach the commit, while from v3.4 you would need close to 500 hops), Ah! Thanks for explaining this mysterious puzzle to me. I'm a bit perplexed why still. Can I trouble you for a little elaboration here? Junio gives a great huge example Phew! Thanks for the elaborate explanation, this makes perfect sense now! Now, as to what *SHOULD* happen, I think the above exercise shows us a way to define what the desired semantics is, without resorting to heuristics (e.g. which tag has older timestamp? or which tag's name sorts older under Linux version naming convention?). I think ultimately this reveals that given that tags *can* be arbitrary and subjective, and given that clocks can also pretty much arbitrary 'git describe --contains' can and probably only should do best effort (TM) and perhaps one thing to help is documenting this issue well and provide a set of best practices that are supported for tagging schemes. I can't describe how many libraries I've reviewed about software versioning schemes and most of them support a huge array of things, and funny enough the Linux versioning scheme, was not supported well, for something so simple as versioning sort. This is ultimately why I had to implement my own sort solution on rel-html. If we agree on this we could just for example take on the Linux versioning scheme as an emum and document that well both on code and a wiki. More on this below. With regards to timestamps: care must be taken given that we'd be assuming that clocks are synchronized, this can likely yield incorrect results on a distributed development environment with different time zones, and it can also be easily cheated, which is why I was concerned over using timestamps. Its still certainly something that can be considered, but I've heard enough rants of a few maintainers about crazy dates on patches which makes me believe this could actually be an issue, specially if we speed up development and need higher degree of resolution. I know the above example but its perhaps worth mentioning how Linux does not follow the above development model for merging stable fixes or changes though, but it does not prevent folks from branching off of older tags to do development which Linux will then pull. In Ingo's case the issue then points then I think to another mild issue -- the commit was developed on a v3.3 based tag, which is why 'git describe --first-parent c5905afb' yields v3.3-rc1-41-gc5905af and not v3.4, which *can also* be a bit perplexing if one does not understand the above example you provided can be used for a development work flow for code sent out to Linus. That said then, since we don't follow the model you laid out it still reveals another issue, and I am not yet sure I still understand why --contains yields a v3.5 tag in that case since we ensured commits on v3.5 were already piled up on older releases, or were being introduced newly on its own release. It smells to me that the commit's first parent (which can be anything) is used somehow here as a shortcut ? This doesn't mean we can't use the work flow above for merging changes from say a v3.4.x onto a v3.5 -- but we don't -- and perhaps as part of the documentation about a scheme for Linux, we should advise against such practices. In any case the closest thing I see we can use upstream on Linux is 'git cherry-pick -x commit-id' but Greg doesn't seem to use this and instead appends the commit with the respective commit ID of the upstream gitsum. Both strategies yield different commit IDs anyway, so neither practice should interrupt the 'git describe --contains' practice. In the stable branches to find out when a commit was introduced one would not rely on the commit ID on the stable branch but instead of the commit ID of the 'upstream reference'. Commit A can be described in terms of both v3.4 and v9.0, And in the real example case, why *would* c5905afb' be be described in terms of v3.5 instead of v3.4 ? and it may be closer to v9.0 than v3.4, and under that definition we pick the closest tag, the current describe --contains behaviour may be correct, but from the human point of view, it is *WRONG*. Yeap, if a development work flow does not follow a strict pattern (maybe a .git/config variable?) perhaps 'git describe --contains' should spit out a the few tags it does have? It is wrong because v9.0 can reach v3.4. So perhaps the rule should be updated to do something like: - find candidate tags that can be used to describe --contains the commit A, yielding v3.4, v3.5 (not shown), and v9.0; Sure. - among the candidate tags, cull the ones that contain another candidate tag, rejecting v3.5 (not shown) and v9.0; Sounds good to me but that seems
Re: [PATCH] tag: add -i and --introduced modifier for --contains
Luis R. Rodriguez mcg...@do-not-panic.com writes: I think ultimately this reveals that given that tags *can* be arbitrary and subjective,... Yes; see the part at the bottom. Commit A can be described in terms of both v3.4 and v9.0, And in the real example case, why *would* c5905afb' be be described in terms of v3.5 instead of v3.4 ? I am not interested in graphing that particular history between v3.4 and v3.5 myself. If you are interested, I already gave you enough information on how to figure that out. - find candidate tags that can be used to describe --contains the commit A, yielding v3.4, v3.5 (not shown), and v9.0; - among the candidate tags, cull the ones that contain another candidate tag, rejecting v3.5 (not shown) and v9.0; - among the surviving tags, pick the closest. Hmm? Sounds good to me! Not so fast ;-) My other message to Peff in response to his another example has an updated position on this. Reject candidates that can reach other candidates is universally correct, but after that point, there are at least three but probably more options that suit preference of different people and project to break ties: - Your case that started this thread may want to favor v3.4 if only because that v3.4 _sounds_ smaller than v4.0 (in Peff's example), even when v3.4 and v4.0 do not have ancestry relationship. - The closest we have had is a heuristic to produce a result that is textually shorter. - And as I alluded to, which one has the earliest timestamp?, is another valid question to ask. And there may be more to appear. A new command line option (and possibly a new configuration) to choose from these three (and more heuristics that will be added later) would be necessary. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 6/9] branch: display publish branch
Jeff King wrote:
On Sat, Apr 12, 2014 at 10:05:15AM -0500, Felipe Contreras wrote:
As you can see; some branches are published, others are not. The ones that
are
not published don't have a @{publish}, and `git branch -v` doesn't show
them.
Why is that hard to understand?
Do you ever push the unpublished branches anywhere at all? If not, then
you would not have a tracking branch. E.g., git _would_ push to remote
gh, branch refs/heads/topic, but there is no remote tracking branch
refs/remotes/gh/topic, because you have never actually pushed there.
So there is no @{publish} branch.
Or do you have some branches in a state where they are pushed, but not
published? It wasn't clear to me from your example if your pu or
dev/remote/hg-extra ever get pushed.
Sometimes I do push these branches, but I don't understand what you mean by
pushed state. When you push something no states are changed. Say I do:
% git push tmp wip-feature
% git push backup wip-feature
So I pushed a branch to two different repositories, the former one might not
even exist any more. Who cares? No states have changed. The fact that this
branch was pushed doesn't change anything about the nature of the branch.
I do not use git branch -v myself,
Me neither (at least not the upstream version).
so I don't personally care that much how it behaves. But I do use a separate
script that does the same thing, and I would want it to show the ahead/behind
relationship between each branch and where it would be pushed to (and as I
said, I define mine with refspecs). Right now it uses nasty hackery to guess
at where things will be pushed, but ideally it would ask git via @{push} or
some similar mechanism.
Yes, but you can push a branch to many locations, to which one should the
script show the tracking information? IMO it should be the one location you
explicitly configured, and in the case of wip-feature is no location.
If the former (you do not actually push them), then I think the semantics I
am looking for and the ones you want would coincide. If not, then I think we
are really talking about two different things.
I ask again what is so difficult about the notion that there are two kinds of
branches?
A)
% git checkout ready-feature
% git push tmp ready-feature
% git push -p github ready-feature
% git push backup ready-feature
B)
% git checkout wip-feature
% git push tmp wip-feature
% git push backup wip-feature
In a haste these branches might not look very different, but conceptually they
are. One has a location where it is publicly visible, and where you wish to
push regularly, the other one doesn't.
Whether you push a branch or not is not really important, it's whether or not
the branch has a *special* place where you push to.
--
Felipe Contreras
--
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 v9 0/6] transport-helper: fixes
Junio C Hamano wrote: Felipe Contreras felipe.contre...@gmail.com writes: These patches add support for remote helpers --force, --dry-run, and reporting forced update. Changes since v8: --- a/transport-helper.c +++ b/transport-helper.c @@ -734,7 +734,7 @@ static int push_update_ref_status(struct strbuf *buf, } (*ref)-status = status; - (*ref)-forced_update = forced; + (*ref)-forced_update |= forced; (*ref)-remote_status = msg; return !(status == REF_STATUS_OK); } Hmph, isn't v8 already in 'master' as of 90e6255a (Merge branch 'fc/transport-helper-fixes', 2014-03-18)? I think I saw gitk report Branches: remotes/origin/pu, but OK. -- Felipe Contreras -- 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: Refactoring hardcoded SHA-1 constants
brian m. carlson wrote: I'd like to introduce a set of preprocessor constants that we'd use instead of hard-coded 20s and 40s everywhere. I agree that it would help code clarity to have symbolic constants for these numbers. On 04/19/2014 12:40 AM, Jonathan Nieder wrote: Lukewarm on that. It's hard to do consistently and unless they're named well it can be harder to know what something like BINARY_OBJECT_NAME_LENGTH means than plain '20' when first reading. OK, so let's see if we can name them well. (Though I think if the names come into wide use then we'll get familiar with them quickly.) libgit2 seems to use the name oid (for object ID) where we tend to use sha1 or name. That might be a good convention for us to move towards. Their constants are called GIT_OID_RAWSZ (== 20) and GIT_OID_HEXSZ (== 40). They don't exactly roll off the tongue, I'll admit. We wouldn't need a GIT_ prefix, I think, since our code is not meant to be used as a library. Let the brainstorming (and bikeshedding) begin! 1. GIT_OID_RAWSZ / GIT_OID_HEXSZ 2. OID_RAWSZ / OID_HEXSZ 3. OID_BINARY_LEN / OID_ASCII_LEN 4. BINARY_OID_LEN / ASCII_OID_LEN Michael -- Michael Haggerty mhag...@alum.mit.edu http://softwareswirl.blogspot.com/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] blame: add correct paddings in time_buf for align
On Sat, Apr 19, 2014 at 12:08 AM, Junio C Hamano gits...@pobox.com wrote: Duy Nguyen pclo...@gmail.com writes: On Fri, Apr 18, 2014 at 3:44 PM, Jiang Xin worldhello@gmail.com wrote: When show blame information with relative time, the UTF-8 characters in `time_str` will break the alignment of columns after the date field. This is because the `time_buf` padding with spaces should have a constant display width, not a fixed strlen size. So we should call utf8_strwidth() instead of strlen() for calibration. Signed-off-by: Jiang Xin worldhello@gmail.com --- Before applying this patch: 5817da01 builtin-blame.c (Pierre Habouzit 6 年前 21) #include parse-options.h ffaf9cc0 builtin-blame.c (Geoffrey Thomas 5 年前 22) #include utf8.h 3b8a12e8 builtin/blame.c (Axel Bonnet 3 年 10 个月之前 23) #include userdiff.h 25ed3412 builtin/blame.c (Bo Yang 1 年 1 个月之前 24) #include line-range.h 58dbfa2e builtin/blame.c (Eric Sunshine 9 个月之前 25) #include line-log.h cee7f245 builtin-pickaxe.c (Junio C Hamano 8 年前 26) After applying this patch: 5817da01 builtin-blame.c (Pierre Habouzit 6 年前 21) #include parse-options.h ffaf9cc0 builtin-blame.c (Geoffrey Thomas 5 年前 22) #include utf8.h 3b8a12e8 builtin/blame.c (Axel Bonnet 3 年 10 个月之前 23) #include userdiff.h 25ed3412 builtin/blame.c (Bo Yang 1 年 1 个月之前 24) #include line-range.h 58dbfa2e builtin/blame.c (Eric Sunshine 9 个月之前 25) #include line-log.h cee7f245 builtin-pickaxe.c (Junio C Hamano 8 年前 26) The numbers 21..26 still do not look aligned, both in gmail raw message view and gmane. Interesting. In my GNUS/Emacs on a fixed-column terminal where an CJK occupies two display columns, they do look aligned, but in my Chrome showing news.gmane.org/gmane.comp.version-control.git/, they do look jagged. When these lines shown in the browser get copied and pasted to gedit, they still look jagged, but after saving it to a file and catting it to the same fixed-column terminal, they are perfectly aligned. Font issues, I guess? Emacs/x11, gedit and xterm all show aligned listings. So yes probably font issues. And because it works fine in text-based environment or editors. I think we're good. -- 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
Re: Refactoring hardcoded SHA-1 constants
On Sat, Apr 19, 2014 at 7:06 AM, Michael Haggerty mhag...@alum.mit.edu wrote: Let the brainstorming (and bikeshedding) begin! 1. GIT_OID_RAWSZ / GIT_OID_HEXSZ 2. OID_RAWSZ / OID_HEXSZ 3. OID_BINARY_LEN / OID_ASCII_LEN 4. BINARY_OID_LEN / ASCII_OID_LEN 5. sizeof(oid) / ASCII_OID_LEN -- 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
Re: Project idea: strbuf allocation modes
Michael Haggerty mhag...@alum.mit.edu writes: On 04/18/2014 07:21 PM, Junio C Hamano wrote: Michael Haggerty mhag...@alum.mit.edu writes: The Idea I would like to see strbuf enhanced to allow it to use memory that it doesn't own (for example, stack-allocated memory), while (optionally) allowing it to switch over to using allocated memory if the string grows past the size of the pre-allocated buffer. I'd like to see these characteristics, but I would want to see that this is done entirely internally inside the strbuf implementation without any API impact, other than the initialisation. I do not ... I think that's exactly what I described. The only thing that would have to change in the strbuf behavior (aside from initialization) is that a buffer-growing operation might die if the string would grow outside of the bounds of the existing buffer when STRBUF_FIXED_MEMORY is set. Yup, we are in agreement ;-) More seriously, sorry for sounding as if I was objecting. I should have edited s/but/and/ before sending my response out. 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: Refactoring hardcoded SHA-1 constants
Duy Nguyen pclo...@gmail.com writes:
On Sat, Apr 19, 2014 at 7:06 AM, Michael Haggerty mhag...@alum.mit.edu
wrote:
Let the brainstorming (and bikeshedding) begin!
1. GIT_OID_RAWSZ / GIT_OID_HEXSZ
2. OID_RAWSZ / OID_HEXSZ
3. OID_BINARY_LEN / OID_ASCII_LEN
4. BINARY_OID_LEN / ASCII_OID_LEN
5. sizeof(oid) / ASCII_OID_LEN
Can we safely assume sizeof(struct { uchar oid[20]; }) is 20, or on
some 64-bit platforms do we have to worry about 8-byte alignment?
In any case, if the pair of names in 1. are what is already used, I
do not see a need for us to waste time bikeshedding at all.
In an ideal world, an implementation of cmd_foo() that defines a
variable to hold an object name and then calls into libgit.a
services may link in the future with a different implementation of
the services that are currently offered by libgit.a but are
reimplemented on top of libgit2, right? Having the same name would
help us, not hurt us, with that direction in some future, no?
--
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 v9 0/6] transport-helper: fixes
Felipe Contreras felipe.contre...@gmail.com writes: Junio C Hamano wrote: Felipe Contreras felipe.contre...@gmail.com writes: These patches add support for remote helpers --force, --dry-run, and reporting forced update. Changes since v8: --- a/transport-helper.c +++ b/transport-helper.c @@ -734,7 +734,7 @@ static int push_update_ref_status(struct strbuf *buf, } (*ref)-status = status; - (*ref)-forced_update = forced; + (*ref)-forced_update |= forced; (*ref)-remote_status = msg; return !(status == REF_STATUS_OK); } Hmph, isn't v8 already in 'master' as of 90e6255a (Merge branch 'fc/transport-helper-fixes', 2014-03-18)? I think I saw gitk report Branches: remotes/origin/pu, but OK. I don't get that but part, but as I said, if what you wanted to apply has further updates relative to what we have, please send in incremental. The patches did not apply cleanly near the tip of 'master', so I didn't check what could be missing myself. 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: Refactoring hardcoded SHA-1 constants
On Sat, Apr 19, 2014 at 8:06 AM, Junio C Hamano gits...@pobox.com wrote:
Duy Nguyen pclo...@gmail.com writes:
On Sat, Apr 19, 2014 at 7:06 AM, Michael Haggerty mhag...@alum.mit.edu
wrote:
Let the brainstorming (and bikeshedding) begin!
1. GIT_OID_RAWSZ / GIT_OID_HEXSZ
2. OID_RAWSZ / OID_HEXSZ
3. OID_BINARY_LEN / OID_ASCII_LEN
4. BINARY_OID_LEN / ASCII_OID_LEN
5. sizeof(oid) / ASCII_OID_LEN
Can we safely assume sizeof(struct { uchar oid[20]; }) is 20, or on
some 64-bit platforms do we have to worry about 8-byte alignment?
That's an interesting point. sizeof(that struct) on x86-64 returns 20.
I haven't checked about other platforms.
We have some ondisk structures around that contain unsigned char [20]
if I remember correctly. Those would need to be handled with care
during the conversion if this becomes a real issue.
--
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
Re: [ANNOUNCE] Git v2.0.0-rc0
On Fri, Apr 18, 2014 at 9:37 PM, Junio C Hamano gits...@pobox.com wrote: An early preview release Git v2.0.0-rc0 is now available for testing at the usual places. This is supposed to have _all_ the v2.0 topics, correct? I'm unable to find the commit that actually _changes_ the default prefix for git svn (as announced in Documentation/git-svn.txt and the release notes for v1.8.5 and v1.9.0). For reference, it was posted as patch 3/3 back in October: http://thread.gmane.org/gmane.comp.version-control.git/232761/focus=235900 Very sorry for not discovering this earlier. ...Johan -- Johan Herland, jo...@herland.net www.herland.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: [PATCH 5/5] completion: fix completion of certain aliases
Junio C Hamano wrote: Gábor Szeder sze...@ira.uka.de writes: words[] is just fine, we never modify it after it is filled by _get_comp_words_by_ref() at the very beginning. Hmph. I would have understood if the latter were we never look at it (to decide what to do). we never modify it does not sound like an enough justification behind words[] is just fine---maybe I am not reading you correctly. The root of the problem is that the expected position of the name of the git command in __git_complete_remote_or_refspec() is hardcoded as words[1], but that is not the case when: 1) it's an alias, as in Felipe's example: git p oriTAB, because while the index is ok, the content is not. 2) in presence of options of the main git command: git -c foo=bar push oriTAB, because the index is off. 3) the command is a shell alias for which the user explicitly set the completion function with __git_complete() (at his own risk): alias gp=git push; __git_complete gp _git_push; gp oriTAB Neither the index nor the content are ok. Fixing the hard-coded indexing would only solve 2) but not 1) and 3), as it obviously couldn't turn the git or shell alias into a git command on its own. Felipe's patch only deals with 1), as it only kicks in in case of a git alias. Which is the far more common use-case, and the problem I've seen people report multiple times in multiple media. Yeah, do completions for commands (not just for the ones that use remote-or-refspec Felipe's patch addresses) have trouble with the latter two in general? If that is the case,... Communicating the name of the git command to __git_complete_remote_or_refspec() by its callers via a new variable as suggested by Junio, or perhaps by an additional parameter to the function is IMHO the right thing to do, because, unless I'm missing something, it would make all three cases work. ... while the above analysis may be correct, taking Felipe's patch to address only (1) and leaving a solution to the more general words[1] problem for other patches on top might not be too bad an approach. Unless (A) remote-or-refspec thing is the primary offender, and other commands do not suffer from the words[1] problem, in which case I tend to agree that an additional parameter would be the way to go (there are only a few callers of the function); or Since I already fixed the problem (all 3) years ago[1], you can take a look at the patch and see which commands which use a hard-coded index value might have real issues. My guess is that it's more than just remote-or-refspec, but I don't have the incentive to look deeper into this (the issue is solved for me). (B) even if words[1] problem is more widespread, such a more general solution to all three issues can be coded cleanly and quickly, without having to have Felipe's patch as a stop-gap measure. I already sent two patches, one that solves all the problems, and one that solves the most important one. I would gladly revisit my old patch and see which of those changes are really necessary and solve a real issue, *if* I knew the resulting patch wouldn't get the same road-blockers as the old one did; namely being held to higher standards than the current code. I say it's more important to fix the real issues real people have than hold on to arbitrary standards which might force the bug to remain present for years just because some variable was not documented in the original patch (just like all other variables in the script)... But that's just me. [1] http://thread.gmane.org/gmane.comp.version-control.git/197226 -- Felipe Contreras-- 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
