* Re: Git Gems
From: Neal Kreitzinger @ 2011-11-18 0:49 UTC (permalink / raw)
To: Hilco Wijbenga; +Cc: Git Users
In-Reply-To: <CAE1pOi1gyshz_502NQvLNAByfwiYXW2fzA+EnGKz8tuFrCpkxg@mail.gmail.com>
On 11/16/2011 5:18 PM, Hilco Wijbenga wrote:
> Hi all,
>
> Just today, I found out about 'git add -p'. I had never even thought
> of this but, now that I know, I can't imagine life without it. :-)
> Actually, it's a bit scary to note that the Git devs apparently aren't
> just telepathic but they can read my thoughts even before I think
> them. ;-)
>
> All kidding aside, I'm starting to wonder which other Git Gems I don't
> know about. For me, the list of Git Gems would include Git's Bash
> command line prompt, 'git add -p', 'git rebase', 'git commit --amend',
> and 'git-new-workdir'. I realize there are plenty of books and such
> out there but I'm really just looking for a list of Git commands
> and/or options that are worth looking into. Finding out more about a
> particular command/script/option is easy, realizing that a particular
> one is the one you need is not. Especially, if you don't even know you
> have a problem.
>
> As an example, 'git rebase' didn't really seem useful until I
> understood (well, more or less) what it did. Until then, I just
> naively assumed that merge commits and non-linear history were
> something you simply had to live with. I'm guessing that, like me, a
> lot of people come to Git with quite a few assumptions and
> preconceived notions due to their exposure to other SCM tools. :-(
>
This book: http://progit.org/ will show you many gems if you take the
time to peruse it.
v/r,
neal
^ permalink raw reply
* Re: [PATCH 6/8] Convert resolve_ref_unsafe+xstrdup to resolve_ref
From: Nguyen Thai Ngoc Duy @ 2011-11-18 0:57 UTC (permalink / raw)
To: Ramkumar Ramachandra
Cc: git, Junio C Hamano, Jeff King, Michael Haggerty, Jonathan Nieder
In-Reply-To: <CALkWK0=h2Q1VTt5AwbBMaZgCYNEkZG5vQGoG=SDBMqYexhJjGA@mail.gmail.com>
2011/11/17 Ramkumar Ramachandra <artagnon@gmail.com>:
> Hi Nguyễn,
>
> Nguyễn Thái Ngọc Duy wrote:
>> diff --git a/builtin/checkout.c b/builtin/checkout.c
>> index 2b8e73b..6efb1cf 100644
>> --- a/builtin/checkout.c
>> +++ b/builtin/checkout.c
>> @@ -696,15 +696,14 @@ static int switch_branches(struct checkout_opts *opts, struct branch_info *new)
>> {
>> int ret = 0;
>> struct branch_info old;
>> + char *path;
>> unsigned char rev[20];
>> int flag;
>> memset(&old, 0, sizeof(old));
>> - old.path = xstrdup(resolve_ref_unsafe("HEAD", rev, 0, &flag));
>> + old.path = path = resolve_ref("HEAD", rev, 0, &flag);
>> old.commit = lookup_commit_reference_gently(rev, 1);
>> - if (!(flag & REF_ISSYMREF)) {
>> - free((char *)old.path);
>> + if (!(flag & REF_ISSYMREF))
>> old.path = NULL;
>> - }
>>
>> if (old.path && !prefixcmp(old.path, "refs/heads/"))
>> old.name = old.path + strlen("refs/heads/");
>
> This caught my eye immediately: you introduced a new "path" variable.
> Let's scroll ahead and see why.
>
>> @@ -718,8 +717,10 @@ static int switch_branches(struct checkout_opts *opts, struct branch_info *new)
>> }
>>
>> ret = merge_working_tree(opts, &old, new);
>> - if (ret)
>> + if (ret) {
>> + free(path);
>> return ret;
>> + }
>>
>> if (!opts->quiet && !old.path && old.commit && new->commit != old.commit)
>> orphaned_commit_warning(old.commit);
>> @@ -727,7 +728,7 @@ static int switch_branches(struct checkout_opts *opts, struct branch_info *new)
>> update_refs_for_switch(opts, &old, new);
>>
>> ret = post_checkout_hook(old.commit, new->commit, 1);
>> - free((char *)old.path);
>> + free(path);
>> return ret || opts->writeout_error;
>> }
>
> Before application of your patch, if !(flag & REF_ISSYMREF) then
> old.path is set to NULL and this free() would've read free(NULL). Was
> this codepath ever reached, and did you fix a bug by introducing the
> new "path" variable, or was it never reached but you introduced the
> new variable for clarity anyway? Either case is worth mentioning in
> the commit message, I think.
free(NULL) is OK if I remember correctly, so it's not really a bug.
Although I do plug a memory leak when merge_working_tree() returns
non-zero.
--
Duy
^ permalink raw reply
* Re: [PATCH 8/8] Enable GIT_DEBUG_MEMCHECK on git_pathname()
From: Nguyen Thai Ngoc Duy @ 2011-11-18 1:12 UTC (permalink / raw)
To: Jeff King
Cc: Ramkumar Ramachandra, git, Junio C Hamano, Michael Haggerty,
Jonathan Nieder
In-Reply-To: <20111117134201.GA30718@sigill.intra.peff.net>
On Thu, Nov 17, 2011 at 8:42 PM, Jeff King <peff@peff.net> wrote:
> On Thu, Nov 17, 2011 at 04:05:52PM +0530, Ramkumar Ramachandra wrote:
>
>> The macros __FILE__, __LINE__ and __VA_ARGS__ are gcc-specific
>> extensions, no? I was curious to see if some other parts of Git are
>> using this: a quick grep returns mailmap.c and notes-merge.c. They
>> both use __VA_ARGS__ it for debugging purposes. So, nothing new.
>
> All three are in C99. I'm pretty sure __FILE__ and __LINE__ were
> available in C89, but I only have a copy of C99 handy these days.
> Variable-argument macros were definitely introduced in C99 (and were a
> gcc extension for a while before then).
>
>> What happens if GIT_DEBUG_MEMCHECK is set, but I'm not using gcc?
>> Also, it's probably worth mentioning in the commit message that this
>> debugging trick is gcc-specific.
>
> Older compilers will probably barf on the variable-argument macros.
Anyway to detect if __VA_ARGS__ is supported at compile time? I guess
#ifdef __GNUC__ is the last resort.
notes-merge.c introduces __VA_ARGS__ since v1.7.4 so we may want to do
something there too.
--
Duy
^ permalink raw reply
* Re: [PATCH 8/8] Enable GIT_DEBUG_MEMCHECK on git_pathname()
From: Jeff King @ 2011-11-18 1:27 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy
Cc: Ramkumar Ramachandra, git, Junio C Hamano, Michael Haggerty,
Jonathan Nieder
In-Reply-To: <CACsJy8A25SyLVKv8GwkYaHBJwU5tHqgdJK6L-upF9HWseFzCtQ@mail.gmail.com>
On Fri, Nov 18, 2011 at 08:12:27AM +0700, Nguyen Thai Ngoc Duy wrote:
> > Older compilers will probably barf on the variable-argument macros.
>
> Anyway to detect if __VA_ARGS__ is supported at compile time? I guess
> #ifdef __GNUC__ is the last resort.
You can check "#if __STDC_VERSION__ >= 19901L", but that will of course
only tell you whether you have C99; older gcc (and possibly other
compilers) supported __VA_ARGS__ even before it was standardized.
But more annoying is that there isn't a great fallback to __VA_ARGS__.
If you can't use it, then every callsite has to have the same number of
arguments. So it's not like you can localize the fallback code to just
the definition.
Unless you really need macro-like behavior, you're probably better off
using a variadic function and making it a static inline on platforms
which can do so.
> notes-merge.c introduces __VA_ARGS__ since v1.7.4 so we may want to do
> something there too.
I hadn't noticed. That definitely violates our usual rules about
portability. That usage can easily be turned into an inline function.
However, since nobody has complained in the past year, it makes me
wonder if we are overly conservative (my guess is that people on crazy
old compilers just don't keep up with git. Which maybe means they aren't
worth worrying about. But who knows).
-Peff
^ permalink raw reply
* Re: [PATCH 8/8] Enable GIT_DEBUG_MEMCHECK on git_pathname()
From: Jonathan Nieder @ 2011-11-18 1:27 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy
Cc: Jeff King, Ramkumar Ramachandra, git, Junio C Hamano,
Michael Haggerty, Johan Herland
In-Reply-To: <CACsJy8A25SyLVKv8GwkYaHBJwU5tHqgdJK6L-upF9HWseFzCtQ@mail.gmail.com>
Nguyen Thai Ngoc Duy wrote:
> Anyway to detect if __VA_ARGS__ is supported at compile time? I guess
> #ifdef __GNUC__ is the last resort.
Why would one want to do that? Either your feature requires C99 (so the
build can error out if __VA_ARGS__ support is missing) or it doesn't.
> notes-merge.c introduces __VA_ARGS__ since v1.7.4 so we may want to do
> something there too.
Good catch. How about this patch? It seems I forgot to send it out
last May and it has been rotting in my local tree ever since.
-- >8 --
Subject: notes merge: eliminate OUTPUT macro
The macro is variadic, which breaks support for pre-C99 compilers,
and it hides an "if", which can make code hard to understand on
first reading if some arguments have side-effects.
The OUTPUT macro seems to have been inspired by the "output" function
from merge-recursive. But that function in merge-recursive exists to
indent output based on the level of recursion and there is no similar
justification for such a function in "notes merge".
Noticed with 'make CC="gcc -std=c89 -pedantic"':
notes-merge.c:24:22: warning: anonymous variadic macros were introduced in C99 [-Wvariadic-macros]
Encouraged-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
notes-merge.c | 104 +++++++++++++++++++++++++++++++++-----------------------
1 files changed, 61 insertions(+), 43 deletions(-)
diff --git a/notes-merge.c b/notes-merge.c
index baaf31f4..d0e5034d 100644
--- a/notes-merge.c
+++ b/notes-merge.c
@@ -21,14 +21,6 @@ void init_notes_merge_options(struct notes_merge_options *o)
o->verbosity = NOTES_MERGE_VERBOSITY_DEFAULT;
}
-#define OUTPUT(o, v, ...) \
- do { \
- if ((o)->verbosity >= (v)) { \
- printf(__VA_ARGS__); \
- puts(""); \
- } \
- } while (0)
-
static int path_to_sha1(const char *path, unsigned char *sha1)
{
char hex_sha1[40];
@@ -392,21 +384,26 @@ static int merge_one_change_manual(struct notes_merge_options *o,
strbuf_addf(&(o->commit_msg), "\t%s\n", sha1_to_hex(p->obj));
- OUTPUT(o, 2, "Auto-merging notes for %s", sha1_to_hex(p->obj));
+ if (o->verbosity >= 2)
+ printf("Auto-merging notes for %s\n", sha1_to_hex(p->obj));
check_notes_merge_worktree(o);
if (is_null_sha1(p->local)) {
/* D/F conflict, checkout p->remote */
assert(!is_null_sha1(p->remote));
- OUTPUT(o, 1, "CONFLICT (delete/modify): Notes for object %s "
- "deleted in %s and modified in %s. Version from %s "
- "left in tree.", sha1_to_hex(p->obj), lref, rref, rref);
+ if (o->verbosity >= 1)
+ printf("CONFLICT (delete/modify): Notes for object %s "
+ "deleted in %s and modified in %s. Version from %s "
+ "left in tree.\n",
+ sha1_to_hex(p->obj), lref, rref, rref);
write_note_to_worktree(p->obj, p->remote);
} else if (is_null_sha1(p->remote)) {
/* D/F conflict, checkout p->local */
assert(!is_null_sha1(p->local));
- OUTPUT(o, 1, "CONFLICT (delete/modify): Notes for object %s "
- "deleted in %s and modified in %s. Version from %s "
- "left in tree.", sha1_to_hex(p->obj), rref, lref, lref);
+ if (o->verbosity >= 1)
+ printf("CONFLICT (delete/modify): Notes for object %s "
+ "deleted in %s and modified in %s. Version from %s "
+ "left in tree.\n",
+ sha1_to_hex(p->obj), rref, lref, lref);
write_note_to_worktree(p->obj, p->local);
} else {
/* "regular" conflict, checkout result of ll_merge() */
@@ -415,8 +412,9 @@ static int merge_one_change_manual(struct notes_merge_options *o,
reason = "add/add";
assert(!is_null_sha1(p->local));
assert(!is_null_sha1(p->remote));
- OUTPUT(o, 1, "CONFLICT (%s): Merge conflict in notes for "
- "object %s", reason, sha1_to_hex(p->obj));
+ if (o->verbosity >= 1)
+ printf("CONFLICT (%s): Merge conflict in notes for "
+ "object %s\n", reason, sha1_to_hex(p->obj));
ll_merge_in_worktree(o, p);
}
@@ -438,24 +436,30 @@ static int merge_one_change(struct notes_merge_options *o,
case NOTES_MERGE_RESOLVE_MANUAL:
return merge_one_change_manual(o, p, t);
case NOTES_MERGE_RESOLVE_OURS:
- OUTPUT(o, 2, "Using local notes for %s", sha1_to_hex(p->obj));
+ if (o->verbosity >= 2)
+ printf("Using local notes for %s\n",
+ sha1_to_hex(p->obj));
/* nothing to do */
return 0;
case NOTES_MERGE_RESOLVE_THEIRS:
- OUTPUT(o, 2, "Using remote notes for %s", sha1_to_hex(p->obj));
+ if (o->verbosity >= 2)
+ printf("Using remote notes for %s\n",
+ sha1_to_hex(p->obj));
if (add_note(t, p->obj, p->remote, combine_notes_overwrite))
die("BUG: combine_notes_overwrite failed");
return 0;
case NOTES_MERGE_RESOLVE_UNION:
- OUTPUT(o, 2, "Concatenating local and remote notes for %s",
- sha1_to_hex(p->obj));
+ if (o->verbosity >= 2)
+ printf("Concatenating local and remote notes for %s\n",
+ sha1_to_hex(p->obj));
if (add_note(t, p->obj, p->remote, combine_notes_concatenate))
die("failed to concatenate notes "
"(combine_notes_concatenate)");
return 0;
case NOTES_MERGE_RESOLVE_CAT_SORT_UNIQ:
- OUTPUT(o, 2, "Concatenating unique lines in local and remote "
- "notes for %s", sha1_to_hex(p->obj));
+ if (o->verbosity >= 2)
+ printf("Concatenating unique lines in local and remote "
+ "notes for %s\n", sha1_to_hex(p->obj));
if (add_note(t, p->obj, p->remote, combine_notes_cat_sort_uniq))
die("failed to concatenate notes "
"(combine_notes_cat_sort_uniq)");
@@ -518,8 +522,9 @@ static int merge_from_diffs(struct notes_merge_options *o,
conflicts = merge_changes(o, changes, &num_changes, t);
free(changes);
- OUTPUT(o, 4, "Merge result: %i unmerged notes and a %s notes tree",
- conflicts, t->dirty ? "dirty" : "clean");
+ if (o->verbosity >= 4)
+ printf("Merge result: %i unmerged notes and a %s notes tree\n",
+ conflicts, t->dirty ? "dirty" : "clean");
return conflicts ? -1 : 1;
}
@@ -616,33 +621,40 @@ int notes_merge(struct notes_merge_options *o,
if (!bases) {
base_sha1 = null_sha1;
base_tree_sha1 = EMPTY_TREE_SHA1_BIN;
- OUTPUT(o, 4, "No merge base found; doing history-less merge");
+ if (o->verbosity >= 4)
+ printf("No merge base found; doing history-less merge\n");
} else if (!bases->next) {
base_sha1 = bases->item->object.sha1;
base_tree_sha1 = bases->item->tree->object.sha1;
- OUTPUT(o, 4, "One merge base found (%.7s)",
- sha1_to_hex(base_sha1));
+ if (o->verbosity >= 4)
+ printf("One merge base found (%.7s)\n",
+ sha1_to_hex(base_sha1));
} else {
/* TODO: How to handle multiple merge-bases? */
base_sha1 = bases->item->object.sha1;
base_tree_sha1 = bases->item->tree->object.sha1;
- OUTPUT(o, 3, "Multiple merge bases found. Using the first "
- "(%.7s)", sha1_to_hex(base_sha1));
+ if (o->verbosity >= 3)
+ printf("Multiple merge bases found. Using the first "
+ "(%.7s)\n", sha1_to_hex(base_sha1));
}
- OUTPUT(o, 4, "Merging remote commit %.7s into local commit %.7s with "
- "merge-base %.7s", sha1_to_hex(remote->object.sha1),
- sha1_to_hex(local->object.sha1), sha1_to_hex(base_sha1));
+ if (o->verbosity >= 4)
+ printf("Merging remote commit %.7s into local commit %.7s with "
+ "merge-base %.7s\n", sha1_to_hex(remote->object.sha1),
+ sha1_to_hex(local->object.sha1),
+ sha1_to_hex(base_sha1));
if (!hashcmp(remote->object.sha1, base_sha1)) {
/* Already merged; result == local commit */
- OUTPUT(o, 2, "Already up-to-date!");
+ if (o->verbosity >= 2)
+ printf("Already up-to-date!\n");
hashcpy(result_sha1, local->object.sha1);
goto found_result;
}
if (!hashcmp(local->object.sha1, base_sha1)) {
/* Fast-forward; result == remote commit */
- OUTPUT(o, 2, "Fast-forward");
+ if (o->verbosity >= 2)
+ printf("Fast-forward\n");
hashcpy(result_sha1, remote->object.sha1);
goto found_result;
}
@@ -684,8 +696,9 @@ int notes_merge_commit(struct notes_merge_options *o,
int path_len = strlen(path), i;
const char *msg = strstr(partial_commit->buffer, "\n\n");
- OUTPUT(o, 3, "Committing notes in notes merge worktree at %.*s",
- path_len - 1, path);
+ if (o->verbosity >= 3)
+ printf("Committing notes in notes merge worktree at %.*s\n",
+ path_len - 1, path);
if (!msg || msg[2] == '\0')
die("partial notes commit has empty message");
@@ -700,7 +713,9 @@ int notes_merge_commit(struct notes_merge_options *o,
unsigned char obj_sha1[20], blob_sha1[20];
if (ent->len - path_len != 40 || get_sha1_hex(relpath, obj_sha1)) {
- OUTPUT(o, 3, "Skipping non-SHA1 entry '%s'", ent->name);
+ if (o->verbosity >= 3)
+ printf("Skipping non-SHA1 entry '%s'\n",
+ ent->name);
continue;
}
@@ -712,14 +727,16 @@ int notes_merge_commit(struct notes_merge_options *o,
if (add_note(partial_tree, obj_sha1, blob_sha1, NULL))
die("Failed to add resolved note '%s' to notes tree",
ent->name);
- OUTPUT(o, 4, "Added resolved note for object %s: %s",
- sha1_to_hex(obj_sha1), sha1_to_hex(blob_sha1));
+ if (o->verbosity >= 4)
+ printf("Added resolved note for object %s: %s\n",
+ sha1_to_hex(obj_sha1), sha1_to_hex(blob_sha1));
}
create_notes_commit(partial_tree, partial_commit->parents, msg,
result_sha1);
- OUTPUT(o, 4, "Finalized notes merge commit: %s",
- sha1_to_hex(result_sha1));
+ if (o->verbosity >= 4)
+ printf("Finalized notes merge commit: %s\n",
+ sha1_to_hex(result_sha1));
free(path);
return 0;
}
@@ -731,7 +748,8 @@ int notes_merge_abort(struct notes_merge_options *o)
int ret;
strbuf_addstr(&buf, git_path(NOTES_MERGE_WORKTREE));
- OUTPUT(o, 3, "Removing notes merge worktree at %s", buf.buf);
+ if (o->verbosity >= 3)
+ printf("Removing notes merge worktree at %s\n", buf.buf);
ret = remove_dir_recursively(&buf, 0);
strbuf_release(&buf);
return ret;
--
1.7.8.rc2
^ permalink raw reply related
* Re: [PATCH/RFC] introduce strbuf_addpath()
From: Nguyen Thai Ngoc Duy @ 2011-11-18 1:42 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Ramkumar Ramachandra, Junio C Hamano, Git List
In-Reply-To: <20111116215004.GA29872@elie.hsd1.il.comcast.net>
On Wed, Nov 16, 2011 at 03:50:04PM -0600, Jonathan Nieder wrote:
> strbuf_addpath() is like git_path_unsafe(), except instead of
> returning its own buffer, it appends its result to a buffer provided
> by the caller.
>
> Benefits:
>
> - Since it uses a caller-supplied buffer, unlike git_path_unsafe(),
> there is no risk that one call will clobber the result from
> another.
>
> - Unlike git_pathdup(), it does not need to waste time allocating
> memory in the middle of your tight loop over refs.
>
> - The size of the result is not limited to PATH_MAX.
>
> Caveat: the size of its result is not limited to PATH_MAX. Existing
> code might be relying on git_path*() to produce a result that is safe
> to copy to a PATH_MAX-sized buffer. Be careful.
>
> This patch introduces the strbuf_addpath() function and converts a few
> existing users of the strbuf_addstr(git_path(...)) idiom to
> demonstrate the API.
>
> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
> ---
> Jonathan Nieder wrote:
>
> > I think if I ran the world, the fundamental operation would be
> > strbuf_addpath().
>
> Like this, maybe.
>
If I ran the world, I would change get_pathname() to return "struct
strbuf *" instead and change "char *git_path(..)" to "const char *git_path(...)".
Code paths that want to modify git_path() return value could
just use strbuf_addpath()
I did try to turn git_path to return const char *, the following patch
seems to make it build without any warnings
-- 8< --
diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index c6bc8eb..fd7c682 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -1041,7 +1041,7 @@ struct ref *fetch_pack(struct fetch_pack_args *my_args,
if (args.depth > 0) {
struct cache_time mtime;
struct strbuf sb = STRBUF_INIT;
- char *shallow = git_path("shallow");
+ const char *shallow = git_path("shallow");
int fd;
mtime.sec = st.st_mtime;
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 91731b9..261f1a0 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -367,7 +367,8 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
char note[1024];
const char *what, *kind;
struct ref *rm;
- char *url, *filename = dry_run ? "/dev/null" : git_path("FETCH_HEAD");
+ const char *filename = dry_run ? "/dev/null" : git_path("FETCH_HEAD");
+ char *url;
fp = fopen(filename, "a");
if (!fp)
@@ -647,7 +648,7 @@ static void check_not_current_branch(struct ref *ref_map)
static int truncate_fetch_head(void)
{
- char *filename = git_path("FETCH_HEAD");
+ const char *filename = git_path("FETCH_HEAD");
FILE *fp = fopen(filename, "w");
if (!fp)
diff --git a/builtin/fsck.c b/builtin/fsck.c
index 0e0e17a..f9624d7 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -215,12 +215,12 @@ static void check_unreachable_object(struct object *obj)
printf("dangling %s %s\n", typename(obj->type),
sha1_to_hex(obj->sha1));
if (write_lost_and_found) {
- char *filename = git_path("lost-found/%s/%s",
+ const char *filename = git_path("lost-found/%s/%s",
obj->type == OBJ_COMMIT ? "commit" : "other",
sha1_to_hex(obj->sha1));
FILE *f;
- if (safe_create_leading_directories(filename)) {
+ if (safe_create_leading_directories_const(filename)) {
error("Could not create lost-found");
return;
}
diff --git a/builtin/remote.c b/builtin/remote.c
index 583eec9..0662d37 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -587,7 +587,7 @@ static int migrate_file(struct remote *remote)
{
struct strbuf buf = STRBUF_INIT;
int i;
- char *path = NULL;
+ const char *path = NULL;
strbuf_addf(&buf, "remote.%s.url", remote->name);
for (i = 0; i < remote->url_nr; i++)
diff --git a/fast-import.c b/fast-import.c
index 8d8ea3c..4293a9f 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -403,7 +403,7 @@ static void dump_marks_helper(FILE *, uintmax_t, struct mark_set *);
static void write_crash_report(const char *err)
{
- char *loc = git_path("fast_import_crash_%"PRIuMAX, (uintmax_t) getpid());
+ const char *loc = git_path("fast_import_crash_%"PRIuMAX, (uintmax_t) getpid());
FILE *rpt = fopen(loc, "w");
struct branch *b;
unsigned long lu;
diff --git a/notes-merge.c b/notes-merge.c
index e33c2c9..8da2e0a 100644
--- a/notes-merge.c
+++ b/notes-merge.c
@@ -288,7 +288,7 @@ static void check_notes_merge_worktree(struct notes_merge_options *o)
"(%s exists).", git_path("NOTES_MERGE_*"));
}
- if (safe_create_leading_directories(git_path(
+ if (safe_create_leading_directories_const(git_path(
NOTES_MERGE_WORKTREE "/.test")))
die_errno("unable to create directory %s",
git_path(NOTES_MERGE_WORKTREE));
@@ -303,8 +303,8 @@ static void write_buf_to_worktree(const unsigned char *obj,
const char *buf, unsigned long size)
{
int fd;
- char *path = git_path(NOTES_MERGE_WORKTREE "/%s", sha1_to_hex(obj));
- if (safe_create_leading_directories(path))
+ const char *path = git_path(NOTES_MERGE_WORKTREE "/%s", sha1_to_hex(obj));
+ if (safe_create_leading_directories_const(path))
die_errno("unable to create directory for '%s'", path);
if (file_exists(path))
die("found existing file at '%s'", path);
diff --git a/refs.c b/refs.c
index 62d8a37..7469cf1 100644
--- a/refs.c
+++ b/refs.c
@@ -1189,7 +1189,7 @@ int dwim_log(const char *str, int len, unsigned char *sha1, char **log)
static struct ref_lock *lock_ref_sha1_basic(const char *ref, const unsigned char *old_sha1, int flags, int *type_p)
{
- char *ref_file;
+ const char *ref_file;
const char *orig_ref = ref;
struct ref_lock *lock;
int last_errno = 0;
@@ -1250,7 +1250,7 @@ static struct ref_lock *lock_ref_sha1_basic(const char *ref, const unsigned char
if ((flags & REF_NODEREF) && (type & REF_ISSYMREF))
lock->force_write = 1;
- if (safe_create_leading_directories(ref_file)) {
+ if (safe_create_leading_directories_const(ref_file)) {
last_errno = errno;
error("unable to create directory for %s", ref_file);
goto error_return;
@@ -1411,7 +1411,7 @@ int rename_ref(const char *oldref, const char *newref, const char *logmsg)
}
}
- if (log && safe_create_leading_directories(git_path("logs/%s", newref))) {
+ if (log && safe_create_leading_directories_const(git_path("logs/%s", newref))) {
error("unable to create directory for %s", newref);
goto rollback;
}
-- 8< --
^ permalink raw reply related
* Re: [PATCH 8/8] Enable GIT_DEBUG_MEMCHECK on git_pathname()
From: Jonathan Nieder @ 2011-11-18 1:36 UTC (permalink / raw)
To: Jeff King
Cc: Nguyen Thai Ngoc Duy, Ramkumar Ramachandra, git, Junio C Hamano,
Michael Haggerty
In-Reply-To: <20111118012715.GA7826@sigill.intra.peff.net>
Jeff King wrote:
> On Fri, Nov 18, 2011 at 08:12:27AM +0700, Nguyen Thai Ngoc Duy wrote:
>> notes-merge.c introduces __VA_ARGS__ since v1.7.4 so we may want to do
>> something there too.
>
> I hadn't noticed. That definitely violates our usual rules about
> portability. That usage can easily be turned into an inline function.
> However, since nobody has complained in the past year, it makes me
> wonder if we are overly conservative (my guess is that people on crazy
> old compilers just don't keep up with git. Which maybe means they aren't
> worth worrying about. But who knows).
I suspect we didn't notice because MSVC 2005 and later have some
(limited, as far as I can tell from web searches) support for
__VA_ARGS__.
^ permalink raw reply
* [PATCH] Use macro define to replace magic character
From: Zheng Liu @ 2011-11-18 1:43 UTC (permalink / raw)
To: git; +Cc: Zheng Liu
We should use macro define rather than magic character to make more readable
Signed-off-by: Zheng Liu <wenqing.lz@taobao.com>
---
merge-recursive.c | 2 +-
tree-diff.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/merge-recursive.c b/merge-recursive.c
index cc664c3..96457bc 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -503,7 +503,7 @@ static struct string_list *get_renames(struct merge_options *o,
struct string_list_item *item;
struct rename *re;
struct diff_filepair *pair = diff_queued_diff.queue[i];
- if (pair->status != 'R') {
+ if (pair->status != DIFF_STATUS_RENAMED) {
diff_free_filepair(pair);
continue;
}
diff --git a/tree-diff.c b/tree-diff.c
index b3cc2e4..d52c440 100644
--- a/tree-diff.c
+++ b/tree-diff.c
@@ -225,7 +225,7 @@ static void try_to_follow_renames(struct tree_desc *t1, struct tree_desc *t2, co
* diff_queued_diff, we will also use that as the path in
* the future!
*/
- if ((p->status == 'R' || p->status == 'C') &&
+ if ((p->status == DIFF_STATUS_RENAMED || p->status == DIFF_STATUS_COPIED) &&
!strcmp(p->two->path, opt->pathspec.raw[0])) {
/* Switch the file-pairs around */
q->queue[i] = choice;
--
1.7.5.4
^ permalink raw reply related
* [PATCH] Remove useless code about diffcore_count_changes()
From: Zheng Liu @ 2011-11-18 1:43 UTC (permalink / raw)
To: git; +Cc: Zheng Liu
We don't need to check return value because it always returns zero in
diffcore_count_changes().
Signed-off-by: Zheng Liu <wenqing.lz@taobao.com>
---
diffcore-break.c | 7 ++-----
diffcore-rename.c | 7 ++-----
2 files changed, 4 insertions(+), 10 deletions(-)
diff --git a/diffcore-break.c b/diffcore-break.c
index 44f8678..402bba1 100644
--- a/diffcore-break.c
+++ b/diffcore-break.c
@@ -68,11 +68,8 @@ static int should_break(struct diff_filespec *src,
if (max_size < MINIMUM_BREAK_SIZE)
return 0; /* we do not break too small filepair */
- if (diffcore_count_changes(src, dst,
- &src->cnt_data, &dst->cnt_data,
- 0,
- &src_copied, &literal_added))
- return 0;
+ diffcore_count_changes(src, dst, &src->cnt_data, &dst->cnt_data, 0,
+ &src_copied, &literal_added);
/* sanity */
if (src->size < src_copied)
diff --git a/diffcore-rename.c b/diffcore-rename.c
index f639601..969482f 100644
--- a/diffcore-rename.c
+++ b/diffcore-rename.c
@@ -182,11 +182,8 @@ static int estimate_similarity(struct diff_filespec *src,
delta_limit = (unsigned long)
(base_size * (MAX_SCORE-minimum_score) / MAX_SCORE);
- if (diffcore_count_changes(src, dst,
- &src->cnt_data, &dst->cnt_data,
- delta_limit,
- &src_copied, &literal_added))
- return 0;
+ diffcore_count_changes(src, dst, &src->cnt_data, &dst->cnt_data,
+ delta_limit, &src_copied, &literal_added);
/* How similar are they?
* what percentage of material in dst are from source?
--
1.7.5.4
^ permalink raw reply related
* Re: [PATCH 8/8] Enable GIT_DEBUG_MEMCHECK on git_pathname()
From: Nguyen Thai Ngoc Duy @ 2011-11-18 1:50 UTC (permalink / raw)
To: Jeff King
Cc: Ramkumar Ramachandra, git, Junio C Hamano, Michael Haggerty,
Jonathan Nieder
In-Reply-To: <20111118012715.GA7826@sigill.intra.peff.net>
On Fri, Nov 18, 2011 at 8:27 AM, Jeff King <peff@peff.net> wrote:
> On Fri, Nov 18, 2011 at 08:12:27AM +0700, Nguyen Thai Ngoc Duy wrote:
>
>> > Older compilers will probably barf on the variable-argument macros.
>>
>> Anyway to detect if __VA_ARGS__ is supported at compile time? I guess
>> #ifdef __GNUC__ is the last resort.
>
> You can check "#if __STDC_VERSION__ >= 19901L", but that will of course
> only tell you whether you have C99; older gcc (and possibly other
> compilers) supported __VA_ARGS__ even before it was standardized.
>
> But more annoying is that there isn't a great fallback to __VA_ARGS__.
> If you can't use it, then every callsite has to have the same number of
> arguments. So it's not like you can localize the fallback code to just
> the definition.
>
> Unless you really need macro-like behavior, you're probably better off
> using a variadic function and making it a static inline on platforms
> which can do so.
I need to save __FILE__ and __LINE__ of call site, inline functions
probably don't help.
>> notes-merge.c introduces __VA_ARGS__ since v1.7.4 so we may want to do
>> something there too.
>
> I hadn't noticed. That definitely violates our usual rules about
> portability. That usage can easily be turned into an inline function.
> However, since nobody has complained in the past year, it makes me
> wonder if we are overly conservative (my guess is that people on crazy
> old compilers just don't keep up with git. Which maybe means they aren't
> worth worrying about. But who knows).
For the record, Sun C compiler 5.9 seems to support it.
--
Duy
^ permalink raw reply
* Re: [PATCH 8/8] Enable GIT_DEBUG_MEMCHECK on git_pathname()
From: Jeff King @ 2011-11-18 2:06 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy
Cc: Ramkumar Ramachandra, git, Junio C Hamano, Michael Haggerty,
Jonathan Nieder
In-Reply-To: <CACsJy8CB6VXjyC-M4C9qGm-n73Kuf1Q0SbH4Ync5Osts-uufQQ@mail.gmail.com>
On Fri, Nov 18, 2011 at 08:50:20AM +0700, Nguyen Thai Ngoc Duy wrote:
> > Unless you really need macro-like behavior, you're probably better off
> > using a variadic function and making it a static inline on platforms
> > which can do so.
>
> I need to save __FILE__ and __LINE__ of call site, inline functions
> probably don't help.
Yeah, you'd have to pass them in to the function. Which of course you
can't wrap with a macro, because the whole thing is variadic.
-Peff
^ permalink raw reply
* A flaw in dep generation with gcc -MMD?
From: Nguyen Thai Ngoc Duy @ 2011-11-18 2:24 UTC (permalink / raw)
To: Git Mailing List
Hi,
My builtin/.depend/add.o.d says
add.o: .... cache.h ...
Shouldn't it be "builtin/add.o: ... cache.h ..."? I tried to touch
cache.h and "make builtin/add.o". It did not remake builtin/add.o. If
I modify add.o.d by hand and remake, it works.
--
Duy
^ permalink raw reply
* Re: [PATCH 0/3] avoiding unintended consequences of git_path() usage
From: Nguyen Thai Ngoc Duy @ 2011-11-18 3:33 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Ramkumar Ramachandra, Junio C Hamano, Git List
In-Reply-To: <20111116075955.GB13706@elie.hsd1.il.comcast.net>
On Wed, Nov 16, 2011 at 01:59:55AM -0600, Jonathan Nieder wrote:
> Ramkumar Ramachandra wrote:
> > Junio C Hamano wrote:
>
> >> Or perhaps http://thread.gmane.org/gmane.comp.version-control.git/184963/focus=185436
> >
> > I noticed that sha1_to_hex() also operates like this. The
> > resolve_ref() function is really important, but using the same
> > technique for these tiny functions is probably an overkill
>
> I don't follow. Do you mean that not being confusing is overkill,
> because the function is small that no one will bother to look up the
> right semantics? Wait, that sentence didn't come out the way I
> wanted. ;-)
>
> Jokes aside, here's a rough series to do the git_path ->
> git_path_unsafe renaming. While writing it, I noticed a couple of
> bugs, hence the two patches before the last one. Patch 2 is the more
> interesting one.
Or perhaps we can use per-file buffer rings instead of a global one.
This means git_path() can only interfere another one in the same file,
making the interaction simpler and hopefully simple enough for reviewers
to catch 90% bugs, therefore safe enough to avoid the _unsafe suffix.
Adding static variable declaration in cache.h is ugly, but that could be
moved to a separate header file.
diff --git a/cache.h b/cache.h
index 2e6ad36..437bc3a 100644
--- a/cache.h
+++ b/cache.h
@@ -660,9 +660,13 @@ extern char *git_snpath(char *buf, size_t n, const char *fmt, ...)
extern char *git_pathdup(const char *fmt, ...)
__attribute__((format (printf, 1, 2)));
+#define git_path(...) git_path_1(pathname_array[3 & ++pathname_index], __VA_ARGS__)
+static char pathname_array[4][PATH_MAX];
+static int pathname_index;
+
/* Return a statically allocated filename matching the sha1 signature */
extern char *mkpath(const char *fmt, ...) __attribute__((format (printf, 1, 2)));
-extern char *git_path(const char *fmt, ...) __attribute__((format (printf, 1, 2)));
+extern char *git_path_1(char *pathname, const char *fmt, ...) __attribute__((format (printf, 2, 3)));
extern char *git_path_submodule(const char *path, const char *fmt, ...)
__attribute__((format (printf, 2, 3)));
diff --git a/path.c b/path.c
index b6f71d1..3c95db1 100644
--- a/path.c
+++ b/path.c
@@ -101,10 +101,9 @@ char *mkpath(const char *fmt, ...)
return cleanup_path(pathname);
}
-char *git_path(const char *fmt, ...)
+char *git_path_1(char *pathname, const char *fmt, ...)
{
const char *git_dir = get_git_dir();
- char *pathname = get_pathname();
va_list args;
unsigned len;
^ permalink raw reply related
* Re: A flaw in dep generation with gcc -MMD?
From: Jonathan Nieder @ 2011-11-18 3:41 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy; +Cc: Git Mailing List
In-Reply-To: <CACsJy8BZMDyf4MCiKxPJ5Z+XS+C-MC82SpMFyWgiXmb9xCnScw@mail.gmail.com>
Nguyen Thai Ngoc Duy wrote:
> My builtin/.depend/add.o.d says
>
> add.o: .... cache.h ...
Interesting. What compiler do you use?
Thanks for finding it,
Jonathan
$ head -1 builtin/.depend/add.o.d
builtin/add.o: builtin/add.c cache.h git-compat-util.h compat/bswap.h \
$ gcc --version | head -1
gcc (Debian 4.6.2-4) 4.6.2
^ permalink raw reply
* Re: A flaw in dep generation with gcc -MMD?
From: Nguyen Thai Ngoc Duy @ 2011-11-18 3:56 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Git Mailing List
In-Reply-To: <20111118034142.GA25228@elie.hsd1.il.comcast.net>
On Fri, Nov 18, 2011 at 10:41 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Nguyen Thai Ngoc Duy wrote:
>
>> My builtin/.depend/add.o.d says
>>
>> add.o: .... cache.h ...
>
> Interesting. What compiler do you use?
$ gcc --version
gcc (Gentoo 4.4.4-r2 p1.2, pie-0.4.5) 4.4.4
> $ head -1 builtin/.depend/add.o.d
> builtin/add.o: builtin/add.c cache.h git-compat-util.h compat/bswap.h \
> $ gcc --version | head -1
> gcc (Debian 4.6.2-4) 4.6.2
Hmm.. I guess it's my compiler's fault then. Next question, how can I
disable this feature?
--
Duy
^ permalink raw reply
* Re: Git Gems
From: Ramkumar Ramachandra @ 2011-11-18 4:38 UTC (permalink / raw)
To: Hilco Wijbenga; +Cc: Git List
In-Reply-To: <CAE1pOi1gyshz_502NQvLNAByfwiYXW2fzA+EnGKz8tuFrCpkxg@mail.gmail.com>
Hi Hilco,
Hilco Wijbenga wrote:
> [CC: Git Users <git@vger.kernel.org>]
For the record, this is the official Git list: both "developers" and
"users" hang out here.
> [...]
> As an example, 'git rebase' didn't really seem useful until I
> understood (well, more or less) what it did. Until then, I just
> naively assumed that merge commits and non-linear history were
> something you simply had to live with. I'm guessing that, like me, a
> lot of people come to Git with quite a few assumptions and
> preconceived notions due to their exposure to other SCM tools. :-(
I'm not sure how a listing is going to help; nevertheless, here are a
few of the lesser-known features of the top of my head (in no
particular order): rerere, attributes, replace refs, filter-branch,
blame's -C and -M switches, log's -S switch, custom diff drivers,
bundle, submodule, stash, notes, and reflog.
[You can find more by digging through the sources]
Which brings us to an interesting aside: unlike many other SCMs which
have a definite and finite set of features, git is really just a
toolkit that grows everyday- various people use various subsets and
write up their own custom scripts to help them automate tasks. The
"git rev-list/ git rev-parse/ git cat-file" is an awesome trio to
start writing shell scripts with :)
Cheers.
-- Ram
^ permalink raw reply
* Re: A flaw in dep generation with gcc -MMD?
From: Miles Bader @ 2011-11-18 4:49 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy; +Cc: Jonathan Nieder, Git Mailing List
In-Reply-To: <CACsJy8A44PFtYrm8NQU+48sVkOe8mjJyO9opO5-TwRtAd-TKsQ@mail.gmail.com>
Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:
>> Interesting. What compiler do you use?
>
> $ gcc --version
> gcc (Gentoo 4.4.4-r2 p1.2, pie-0.4.5) 4.4.4
FWIW, gcc 4.4.6 on debian does the correct thing too...
-Miles
--
Yo mama's so fat when she gets on an elevator it HAS to go down.
^ permalink raw reply
* [PATCH] Makefile: add option to disable automatic dependency generation
From: Jonathan Nieder @ 2011-11-18 4:57 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy; +Cc: Git Mailing List, Fredrik Kuivinen
In-Reply-To: <CACsJy8A44PFtYrm8NQU+48sVkOe8mjJyO9opO5-TwRtAd-TKsQ@mail.gmail.com>
Duy noticed that now that the COMPUTE_HEADER_DEPENDENCIES feature is
turned on automatically for compilers that support it (see
v1.7.8-rc0~142^2~1, 2011-08-18), there is no easy way to force it off.
For example, setting COMPUTE_HEADER_DEPENDENCIES to the empty string
in config.mak just tells the makefile to treat it as undefined and
run a test command to see if the -MMD option is supported.
Introduce a new NO_COMPUTE_HEADER_DEPENDENCIES variable that forces
the feature off. The new semantics:
- If NO_COMPUTE_HEADER_DEPENDENCIES is set to a nonempty string,
the -MMD option will not be used. The build relies on hard-coded
dependencies in the "ifndef USE_COMPUTED_HEADER_DEPENDENCIES"
section of the Makefile instead.
- If COMPUTE_HEADER_DEPENDENCIES is empty and NO_COMPUTE_... is
nonempty, the build uses gcc's on-the-fly dependency generation
feature.
- If neither is nonempty, the makefile runs a quick test command
to decide whether the compiler supports the -MMD option and
whether to enable this feature.
Inspired-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Nguyen Thai Ngoc Duy wrote:
> Hmm.. I guess it's my compiler's fault then. Next question, how can I
> disable this feature?
"make COMPUTE_HEADER_DEPENDENCIES=" works. But there's no way aside
from "override COMPUTE_HEADER_DEPENDENCIES=" to do that in config.mak.
And yuck.
By the way, I'm not convinced it's your compiler's fault. It might be
my compiler's fault. More reading to do...
Makefile | 15 ++++++++++++++-
1 files changed, 14 insertions(+), 1 deletions(-)
diff --git a/Makefile b/Makefile
index ee34eab8..e4a658f6 100644
--- a/Makefile
+++ b/Makefile
@@ -250,6 +250,13 @@ all::
# DEFAULT_EDITOR='$GIT_FALLBACK_EDITOR',
# DEFAULT_EDITOR='"C:\Program Files\Vim\gvim.exe" --nofork'
#
+# Define COMPUTE_HEADER_DEPENDENCIES if your compiler supports the -MMD option
+# and you want to avoid rebuilding objects when an unrelated header file
+# changes.
+#
+# Define NO_COMPUTE_HEADER_DEPENDENCIES if you want to disable automatic
+# dependency generation even though your compiler is detected to support it.
+#
# Define CHECK_HEADER_DEPENDENCIES to check for problems in the hard-coded
# dependency rules.
#
@@ -1245,10 +1252,16 @@ endif
endif
ifdef CHECK_HEADER_DEPENDENCIES
+NO_COMPUTE_HEADER_DEPENDENCIES = YesPlease
+endif
+
+ifdef NO_COMPUTE_HEADER_DEPENDENCIES
COMPUTE_HEADER_DEPENDENCIES =
USE_COMPUTED_HEADER_DEPENDENCIES =
-else
+endif
+
ifndef COMPUTE_HEADER_DEPENDENCIES
+ifndef NO_COMPUTE_HEADER_DEPENDENCIES
dep_check = $(shell $(CC) $(ALL_CFLAGS) \
-c -MF /dev/null -MMD -MP -x c /dev/null -o /dev/null 2>&1; \
echo $$?)
--
1.7.8.rc2
^ permalink raw reply related
* Re: [PATCH] Makefile: add option to disable automatic dependency generation
From: Jonathan Nieder @ 2011-11-18 5:00 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy; +Cc: Git Mailing List, Fredrik Kuivinen
In-Reply-To: <20111118045742.GA25145@elie.hsd1.il.comcast.net>
Jonathan Nieder wrote:
> - If COMPUTE_HEADER_DEPENDENCIES is empty and NO_COMPUTE_... is
> nonempty, the build uses gcc's on-the-fly dependency generation
> feature.
Erm. This should say:
"- If COMPUTE_HEADER_DEPENDENCIES is nonempty and NO_COMPUTE_... is
empty,"
Sorry for the noise.
^ permalink raw reply
* test t/t1304-default-acl.sh
From: Scott Parrish @ 2011-11-18 5:10 UTC (permalink / raw)
To: Git Mailing List
Hi folks,
This test case fails on FreeBSD 8.2. Specifically, the use of the "d[efault]:" syntax in the ACL tag is invalid on FreeBSD. The manpage indicates that default ACL's should be set via the "-d" command line option to setfacl. See http://www.FreeBSD.org/cgi/man.cgi?query=setfacl&apropos=0&sektion=0&manpath=FreeBSD+8.2-RELEASE&arch=default&format=html for details.
Note that the test used "-d" at one time but appears to have been changed to use "d:" to accommodate Solaris where "-d" has the effect of deleting ACL entries. The change was made on this commit: https://github.com/gitster/git/commit/db826571e4099066fe44233d95642591016c831b
So I don't think it is as straight-forward as reverting to the use of "-d".
Thoughts?
Cheers,
-Scott
^ permalink raw reply
* Re: t/t1304: avoid -d option to setfacl (db82657)
From: Brandon Casey @ 2011-11-18 5:37 UTC (permalink / raw)
To: wsp; +Cc: git
In-Reply-To: <gitster/git/commit/db826571e4099066fe44233d95642591016c831b/729354@github.com>
[cc git@vger.kernel.org since that is the appropriate place for this discusion]
On Thu, Nov 17, 2011 at 8:59 PM, wsp
<reply+c-729354-3460aca0fa61e627f9d1a271cf70a99d5c1e7e4e-921167@reply.github.com>
wrote:
> Could this test case be reviewed again? It fails on FreeBSD where the appropriate way to specify default ACL's is with the "-d" option. The "d[efault]:" syntax is invalid on FreeBSD.
Well, I'm not sure there is a right answer here.
Linux accepts -d or "d[efault]"
Solaris only accepts "d[efault]"
FreeBSD only accepts -d
a quick search shows z/OS only accepts "d[efault]"
other?
I think most everything else rolls their own implementation into chmod
or chacl or something like that.
The abandoned POSIX draft does actually specify the FreeBSD behavior.
So I think it's kind of a toss-up. Which option we choose should
probably depend on whether we get more test coverage by using the
"d[efault]" notation or by using the -d option. That depends on
whether there are more Solaris users compiling git or whether there
are more FreeBSD users. I don't know the answer to that either. I
tend to think there are very few of either.
-Brandon
^ permalink raw reply
* Re: [PATCH] Makefile: add option to disable automatic dependency generation
From: Junio C Hamano @ 2011-11-18 6:12 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Nguyen Thai Ngoc Duy, Git Mailing List, Fredrik Kuivinen
In-Reply-To: <20111118045742.GA25145@elie.hsd1.il.comcast.net>
Jonathan Nieder <jrnieder@gmail.com> writes:
> Duy noticed that now that the COMPUTE_HEADER_DEPENDENCIES feature is
> turned on automatically for compilers that support it (see
> v1.7.8-rc0~142^2~1, 2011-08-18), there is no easy way to force it off.
> For example, setting COMPUTE_HEADER_DEPENDENCIES to the empty string
> in config.mak just tells the makefile to treat it as undefined and
> run a test command to see if the -MMD option is supported.
>
> Introduce a new NO_COMPUTE_HEADER_DEPENDENCIES variable that forces
> the feature off.
Eek. At least at the end user UI level, couldn't we do this as a tristate?
E.g. "YesPlease" (or anything that begins with Y if you are ambitious) to
explicitly enable, empty (or "auto") to autodetect, and anything else to
decline?
Even better, couldn't we either (1) rearrange .dep/ files somehow, so that
compiler difference does not matter, or (2) have dep_check to perform a
trial run to detect versions of compilers that produce the output that we
cannot use?
^ permalink raw reply
* Re: [PATCH 8/8] Enable GIT_DEBUG_MEMCHECK on git_pathname()
From: Johan Herland @ 2011-11-18 6:16 UTC (permalink / raw)
To: Jonathan Nieder
Cc: Nguyen Thai Ngoc Duy, Jeff King, Ramkumar Ramachandra, git,
Junio C Hamano, Michael Haggerty
In-Reply-To: <20111118012746.GA22485@elie.hsd1.il.comcast.net>
On Fri, Nov 18, 2011 at 02:27, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Subject: notes merge: eliminate OUTPUT macro
>
> The macro is variadic, which breaks support for pre-C99 compilers,
> and it hides an "if", which can make code hard to understand on
> first reading if some arguments have side-effects.
>
> The OUTPUT macro seems to have been inspired by the "output" function
> from merge-recursive. But that function in merge-recursive exists to
> indent output based on the level of recursion and there is no similar
> justification for such a function in "notes merge".
>
> Noticed with 'make CC="gcc -std=c89 -pedantic"':
>
> notes-merge.c:24:22: warning: anonymous variadic macros were introduced in C99 [-Wvariadic-macros]
>
> Encouraged-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Acked-by: Johan Herland <johan@herland.net>
--
Johan Herland, <johan@herland.net>
www.herland.net
^ permalink raw reply
* Re: [PATCH] Makefile: add option to disable automatic dependency generation
From: Jonathan Nieder @ 2011-11-18 6:21 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Nguyen Thai Ngoc Duy, Git Mailing List, Fredrik Kuivinen
In-Reply-To: <7vty62klg9.fsf@alter.siamese.dyndns.org>
Junio C Hamano wrote:
> Eek. At least at the end user UI level, couldn't we do this as a tristate?
> E.g. "YesPlease" (or anything that begins with Y if you are ambitious) to
> explicitly enable, empty (or "auto") to autodetect, and anything else to
> decline?
Ah, I didn't mind the UI so much. Handling
COMPUTE_HEADER_DEPENDENCIES_FORCE = (yes | no | auto)
should be doable. I'd suggest making any other value error out, so
typos don't result in mysterious behavior.
> Even better, couldn't we either (1) rearrange .dep/ files somehow, so that
> compiler difference does not matter
Yes, I'm working on an incantation all the compilers like (it
shouldn't be hard --- adding an -MQ option should be enough, but I
want to understand the bug first). But even with such a fix, I think
it will be important to have a way to turn the feature off. When
someone using a compiler without -MMD support reports a bug, wouldn't
it be nice to be able to reproduce it?
^ permalink raw reply
* Re: [PATCH 8/8] Enable GIT_DEBUG_MEMCHECK on git_pathname()
From: Jonathan Nieder @ 2011-11-18 6:52 UTC (permalink / raw)
To: Johan Herland
Cc: Nguyen Thai Ngoc Duy, Jeff King, Ramkumar Ramachandra, git,
Junio C Hamano, Michael Haggerty
In-Reply-To: <CALKQrgfTKmSd8se3n3xq89SXRmNPm3qz3Ckv2mUghot8kStKxA@mail.gmail.com>
Johan Herland wrote:
> Acked-by: Johan Herland <johan@herland.net>
Thanks for looking it over.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox