Re: Pull is Mostly Evil
Felipe Contreras wrote: >David Lang wrote: >> the vast majority of people here do not take that attitude. > >It's actually the exact opposite. I don't care what is the track record >of the people in the discussion. Ah, yes, like that discussion we once had where you totally didn't run `git log | grep James Denholm` at one point to demonstrate that I had not yet made any contributions,instead of actually engaging in discussion. Oh, wait. >If their argument is good, their argument is good. The problem, though, is that time and time again you've shown that you value your own arguments to the exclusion of all others. You can't tell if someone else's argument is good, because it runs against yours, and yours must be right because you hold it. Regards, James Denholm. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 0/9] Use a structure for object IDs.
On 05/03/2014 10:12 PM, brian m. carlson wrote: > This is a preliminary RFC patch series to move all the relevant uses of > unsigned char [20] to struct object_id. It should not be applied to any > branch yet. > > The goal of this series to improve type-checking in the codebase and to > make it easier to move to a different hash function if the project > decides to do that. This series does not convert all of the codebase, > but only parts. I'm looking for feedback to see if there is consensus > that this is the right direction before investing a large amount of > time. > > Certain parts of the code have to be converted before others to keep the > patch sizes small, maintainable, and bisectable, so functions and > structures that are used across the codebase (e.g. hashcmp and struct > object) will be converted later. Conversion has been done in a roughly > alphabetical order by name of file. > > The constants for raw and hex sizes of SHA-1 values are maintained. > These constants are used where the quantity is the size of an SHA-1 > value, and sizeof(struct object_id) is used wherever memory is to be > allocated. This is done to permit the struct to turn into a union later > if multiple hashes are supported. I left the names at GIT_OID_RAWSZ and > GIT_OID_HEXSZ because that's what libgit2 uses and what Junio seemed to > prefer, but they can be changed later if there's a desire to do that. > > I called the structure member "oid" because it was easily grepable and > distinct from the rest of the codebase. It, too, can be changed if we > decide on a better name. I specifically did not choose "sha1" since it > looks weird to have "sha1->sha1" and I didn't want to rename lots of > variables. That means that we will have sha1->oid all over the place, right? That's unfortunate, because it is exactly backwards from what we would want in a hypothetical future where OIDs are not necessarily SHA-1s. In that future we would certainly have to support SHA-1s in parallel with the new hash. So (in that hypothetical future) we will probably want these expressions to look like oid->sha1, to allow, say, a second struct or union field oid->sha256 [1]. If that future would come to pass, then we would also want to have distinct constants like GIT_SHA1_RAWSZ and GIT_SHA256_RAWSZ rather than the generically-named GIT_OID_RAWSZ. I think that this patch series will improve the code clarity and type safety independent of thoughts about supporting different hash algorithms, so I'm not objecting to your naming decision. But *if* such support is part of your long-term hope, then you might ease the future transition by choosing different names now. (Maybe renaming local variables "sha1 -> oid" might be a handy way of making clear which code has been converted to the new style.) Just to be clear, the above are just some random thoughts for your consideration, but feel free to disregard them. In any case, it sure will be a lot of code churn. If you succeed in this project, then "git blame" will probably consider you the author of about 2/3 of git :-) Michael [1] I'm certainly not advocating that we want to support a different hash, let alone that that hash should be SHA-256; these examples are just for illustration. -- 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
[PATCH 1/3] Revert "make error()'s constant return value more visible"
In recent versions of gcc (4.9.0), we get hundreds of these: advice.c: In function ‘error_resolve_conflict’: advice.c:79:69: warning: right-hand operand of comma expression has no effect [-Wunused-value] error("'%s' is not possible because you have unmerged files.", me); ^ The original patch intended to help in situations like this: if (error(...)) /* do stuff */ However, when there's no conditional statement this gets translated to: (error(..), 1); And the right hand of the expression has no effect. So it looks like gcc is smarter now, and in trying to fix a few warnings we generated hundreds more. This reverts commit e208f9cc7574f5980faba498d0aa30b4defeb34f. Conflicts: git-compat-util.h --- git-compat-util.h | 11 --- usage.c | 1 - 2 files changed, 12 deletions(-) diff --git a/git-compat-util.h b/git-compat-util.h index f6d3a46..b4242e4 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -323,17 +323,6 @@ extern void warning(const char *err, ...) __attribute__((format (printf, 1, 2))) #include #endif /* NO_OPENSSL */ -/* - * Let callers be aware of the constant return value; this can help - * gcc with -Wuninitialized analysis. We restrict this trick to gcc, though, - * because some compilers may not support variadic macros. Since we're only - * trying to help gcc, anyway, it's OK; other compilers will fall back to - * using the function as usual. - */ -#if defined(__GNUC__) && ! defined(__clang__) -#define error(...) (error(__VA_ARGS__), -1) -#endif - extern void set_die_routine(NORETURN_PTR void (*routine)(const char *err, va_list params)); extern void set_error_routine(void (*routine)(const char *err, va_list params)); extern void set_die_is_recursing_routine(int (*routine)(void)); diff --git a/usage.c b/usage.c index ed14645..9d2961e 100644 --- a/usage.c +++ b/usage.c @@ -138,7 +138,6 @@ void NORETURN die_errno(const char *fmt, ...) va_end(params); } -#undef error int error(const char *err, ...) { va_list params; -- 1.9.2+fc1.20.g204a630 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/3] Silence a bunch of format-zero-length warnings
This is in gcc 4.9.0: wt-status.c: In function ‘wt_status_print_unmerged_header’: wt-status.c:191:2: warning: zero-length gnu_printf format string [-Wformat-zero-length] status_printf_ln(s, c, ""); ^ We could pass -Wno-format-zero-length, but it seems compiler-specific flags are frowned upon, so let's just avoid the warning altogether. Signed-off-by: Felipe Contreras --- builtin/commit.c | 2 +- wt-status.c | 22 +++--- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/builtin/commit.c b/builtin/commit.c index 9cfef6c..13b84e4 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -812,7 +812,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix, committer_ident.buf); if (ident_shown) - status_printf_ln(s, GIT_COLOR_NORMAL, ""); + status_printf_ln(s, GIT_COLOR_NORMAL, "%s", ""); saved_color_setting = s->use_color; s->use_color = 0; diff --git a/wt-status.c b/wt-status.c index ec7344e..b8841e1 100644 --- a/wt-status.c +++ b/wt-status.c @@ -188,7 +188,7 @@ static void wt_status_print_unmerged_header(struct wt_status *s) } else { status_printf_ln(s, c, _(" (use \"git add/rm ...\" as appropriate to mark resolution)")); } - status_printf_ln(s, c, ""); + status_printf_ln(s, c, "%s", ""); } static void wt_status_print_cached_header(struct wt_status *s) @@ -204,7 +204,7 @@ static void wt_status_print_cached_header(struct wt_status *s) status_printf_ln(s, c, _(" (use \"git reset %s ...\" to unstage)"), s->reference); else status_printf_ln(s, c, _(" (use \"git rm --cached ...\" to unstage)")); - status_printf_ln(s, c, ""); + status_printf_ln(s, c, "%s", ""); } static void wt_status_print_dirty_header(struct wt_status *s, @@ -223,7 +223,7 @@ static void wt_status_print_dirty_header(struct wt_status *s, status_printf_ln(s, c, _(" (use \"git checkout -- ...\" to discard changes in working directory)")); if (has_dirty_submodules) status_printf_ln(s, c, _(" (commit or discard the untracked or modified content in submodules)")); - status_printf_ln(s, c, ""); + status_printf_ln(s, c, "%s", ""); } static void wt_status_print_other_header(struct wt_status *s, @@ -235,12 +235,12 @@ static void wt_status_print_other_header(struct wt_status *s, if (!s->hints) return; status_printf_ln(s, c, _(" (use \"git %s ...\" to include in what will be committed)"), how); - status_printf_ln(s, c, ""); + status_printf_ln(s, c, "%s", ""); } static void wt_status_print_trailer(struct wt_status *s) { - status_printf_ln(s, color(WT_STATUS_HEADER, s), ""); + status_printf_ln(s, color(WT_STATUS_HEADER, s), "%s", ""); } #define quote_path quote_path_relative @@ -826,7 +826,7 @@ static void wt_status_print_other(struct wt_status *s, string_list_clear(&output, 0); strbuf_release(&buf); conclude: - status_printf_ln(s, GIT_COLOR_NORMAL, ""); + status_printf_ln(s, GIT_COLOR_NORMAL, "%s", ""); } void wt_status_truncate_message_at_cut_line(struct strbuf *buf) @@ -913,7 +913,7 @@ static void wt_status_print_tracking(struct wt_status *s) color_fprintf_ln(s->fp, color(WT_STATUS_HEADER, s), "%c", comment_line_char); else - fprintf_ln(s->fp, ""); + fputs("", s->fp); } static int has_unmerged(struct wt_status *s) @@ -1329,7 +1329,7 @@ void wt_status_print(struct wt_status *s) on_what = _("Not currently on any branch."); } } - status_printf(s, color(WT_STATUS_HEADER, s), ""); + status_printf(s, color(WT_STATUS_HEADER, s), "%s", ""); status_printf_more(s, branch_status_color, "%s", on_what); status_printf_more(s, branch_color, "%s\n", branch_name); if (!s->is_initial) @@ -1342,9 +1342,9 @@ void wt_status_print(struct wt_status *s) free(state.detached_from); if (s->is_initial) { - status_printf_ln(s, color(WT_STATUS_HEADER, s), ""); + status_printf_ln(s, color(WT_STATUS_HEADER, s), "%s", ""); status_printf_ln(s, color(WT_STATUS_HEADER, s), _("Initial commit")); - status_printf_ln(s, color(WT_STATUS_HEADER, s), ""); + status_printf_ln(s, color(WT_STATUS_HEADER, s), "%s", ""); } wt_status_print_updated(s); @@ -1361,7 +1361,7 @@ void wt_status_print(struct wt_status *s) if (s->show_ignored_files) wt_status_print_other(s, &s->ignored, _("Ignored files"), "add -f"); if (advice_status_u_option && 2000 < s->unt
[PATCH 2/3] Revert "silence some -Wuninitialized false positives"
In recent versions of gcc (4.9.0), we get a few of these: notes.c: In function ‘notes_display_config’: notes.c:970:28: warning: right-hand operand of comma expression has no effect [-Wunused-value] config_error_nonbool(k); ^ Previous commit explains the reason. This reverts commit a469a1019352b8efc4bd7003b0bd59eb60fc428c. Conflicts: cache.h parse-options.h --- cache.h | 3 --- config.c| 1 - parse-options.c | 18 +- parse-options.h | 4 4 files changed, 9 insertions(+), 17 deletions(-) diff --git a/cache.h b/cache.h index 107ac61..fae077a 100644 --- a/cache.h +++ b/cache.h @@ -1271,9 +1271,6 @@ extern int check_repository_format_version(const char *var, const char *value, v extern int git_env_bool(const char *, int); extern int git_config_system(void); extern int config_error_nonbool(const char *); -#if defined(__GNUC__) && ! defined(__clang__) -#define config_error_nonbool(s) (config_error_nonbool(s), -1) -#endif extern const char *get_log_output_encoding(void); extern const char *get_commit_output_encoding(void); diff --git a/config.c b/config.c index a30cb5c..0083674 100644 --- a/config.c +++ b/config.c @@ -1870,7 +1870,6 @@ int git_config_rename_section(const char *old_name, const char *new_name) * Call this to report error for your variable that should not * get a boolean value (i.e. "[my] var" means "true"). */ -#undef config_error_nonbool int config_error_nonbool(const char *var) { return error("Missing value for '%s'", var); diff --git a/parse-options.c b/parse-options.c index b536896..2b4b8e5 100644 --- a/parse-options.c +++ b/parse-options.c @@ -19,6 +19,15 @@ int optbug(const struct option *opt, const char *reason) return error("BUG: switch '%c' %s", opt->short_name, reason); } +int opterror(const struct option *opt, const char *reason, int flags) +{ + if (flags & OPT_SHORT) + return error("switch `%c' %s", opt->short_name, reason); + if (flags & OPT_UNSET) + return error("option `no-%s' %s", opt->long_name, reason); + return error("option `%s' %s", opt->long_name, reason); +} + static int get_arg(struct parse_opt_ctx_t *p, const struct option *opt, int flags, const char **arg) { @@ -632,12 +641,3 @@ static int parse_options_usage(struct parse_opt_ctx_t *ctx, return usage_with_options_internal(ctx, usagestr, opts, 0, err); } -#undef opterror -int opterror(const struct option *opt, const char *reason, int flags) -{ - if (flags & OPT_SHORT) - return error("switch `%c' %s", opt->short_name, reason); - if (flags & OPT_UNSET) - return error("option `no-%s' %s", opt->long_name, reason); - return error("option `%s' %s", opt->long_name, reason); -} diff --git a/parse-options.h b/parse-options.h index 3189676..3b140d0 100644 --- a/parse-options.h +++ b/parse-options.h @@ -176,10 +176,6 @@ extern NORETURN void usage_msg_opt(const char *msg, extern int optbug(const struct option *opt, const char *reason); extern int opterror(const struct option *opt, const char *reason, int flags); -#if defined(__GNUC__) && ! defined(__clang__) -#define opterror(o,r,f) (opterror((o),(r),(f)), -1) -#endif - /*- incremental advanced APIs -*/ enum { -- 1.9.2+fc1.20.g204a630 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/3] Fix a ton of compiler warnings
Hi, I'm getting tons and tons of warnings with gcc 4.9.0. Here are the patches to fix them all. Felipe Contreras (3): Revert "make error()'s constant return value more visible" Revert "silence some -Wuninitialized false positives" Silence a bunch of format-zero-length warnings builtin/commit.c | 2 +- cache.h | 3 --- config.c | 1 - git-compat-util.h | 11 --- parse-options.c | 18 +- parse-options.h | 4 usage.c | 1 - wt-status.c | 22 +++--- 8 files changed, 21 insertions(+), 41 deletions(-) -- 1.9.2+fc1.20.g204a630 -- To unsubscribe from this list: send the line "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: Pull is Mostly Evil
Felipe Contreras writes: > David Lang wrote: >> note that this is one person taking the "I don't see any commits from >> you so your opinion doesn't count" attitude. > > Wrong. I said it doesn't count "for the project". There are a number of commits from me that actually count. A few old core performance ones might have actually have affected my carbon footprint noticeably. The one currently in pu will probably not be called often enough for that but will at least have practical consequences. > Do you honestly believe Junio cares about what some random guy on the > list thinks about default aliases? No. Putting aside my code contributions: Git is a comparatively small project, so if the main project you are working on with Git is Git, your experience is limited. So yes, input from people who are _not_ heavy Git developers is important, since the heavy Git developers do not get to see the heavy Git use cases a lot. > It's actually the exact opposite. I don't care what is the track > record of the people in the discussion. If their argument is good, > their argument is good. More like if they are around, they are worth getting plastered with your frustration. > It's the others that focus on the carisma and credentials of the > people in the discussion, rather than the arguments. I think you are confusing inertia with resistance. -- David Kastrup -- To unsubscribe from this list: send the line "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] t3910: show failure of core.precomposeunicode with decomposed filenames
(Sorry for the late reply, I'm handicapped by some local IT problems) Peff, do you know if the fix below helps ? On 2014-04-28 18.16, Jeff King wrote: > If you have existing decomposed filenames in your git > repository (e.g., that were created with older versions of > git that did not precompose unicode), a modern git with > core.precomposeunicode set does not handle them well. Yes. > The problem is that we normalize the paths coming from the > disk into their precomposed form, and then compare them > against the literal bytes in the index. This makes things > better if you have the precomposed form in the index. It > makes things worse if you actually have the decomposed form > in the index. > > As a result, paths with decomposed filenames may have their > precomposed variants listed as untracked files (even though > the precomposed variants do not exist on-disk at all). Yes > This patch just adds a test to demonstrate the breakage. > Some possible fixes are: > > 1. Tell everyone that NFD in the git repo is wrong, and > they should make a new commit to normalize all their > in-repo files to be precomposed. > This is probably not the right thing to do, because it > still doesn't fix checkouts of old history. And it > spreads the problem to people on byte-preserving > filesystems (like ext4), because now they have to start > precomposing their filenames as they are adde to git. (typo: ^added) I'm not sure if I follow. People running ext4 (or Linux in general, or Windows, or Unix) do not suffer from file system "feature" of Mac OS, which accepts precomposed/decomposed Unicode but returns decompomsed. > > 2. Do all index filename comparisons using a UTF-8 aware > comparison function when core.precomposeunicode is set. > This would probably have bad performance, and somewhat > defeats the point of converting the filenames at the > readdir level in the first place. > > 3. Convert index filenames to their precomposed form when > we read the index from disk. This would be efficient, > but we would have to be careful not to write the > precomposed forms back out to disk. Question: How could we be careful? Mac OS writes always decomposed Unicode to disk. (And all other OS tend to use precomposed forms, mainly because the "keyboard driver" generates it.) > > 4. Introduce some infrastructure to efficiently match up > the precomposed/decomposed forms. We already do > something similar for case-insensitive files using > name-hash.c. We might be able to adapt that strategy > here. -- This is my understanding: Some possible fixes are: 1. Accept that NFD in a Git repo which is shared between Mac OS and Linux or Windows is problematic. Whenever core.precomposeunicode = true, do the following: Let Git under Mac OS change all file names in the index into the precomposed form when a new commit is done. This is probably not a wrong thing to do. When the index file is read into memory, precompose the file names and compare them with the precomposed form coming from precompose_utf8_readdir(). This avoids decomposed file names to be reported as untracked by "git status. 2. Do all index filename comparisons under Mac OS X using a UTF-8 aware comparison function regardless if core.precomposeunicode is set. This would probably have bad performance, and somewhat defeats the point of converting the filenames at the readdir level in the first place. (The TC is good) > > Signed-off-by: Jeff King > --- > t/t3910-mac-os-precompose.sh | 10 ++ > 1 file changed, 10 insertions(+) > > diff --git a/t/t3910-mac-os-precompose.sh b/t/t3910-mac-os-precompose.sh > index e4ba601..23aa61e 100755 > --- a/t/t3910-mac-os-precompose.sh > +++ b/t/t3910-mac-os-precompose.sh > @@ -140,6 +140,16 @@ test_expect_success "Add long precomposed filename" ' > git add * && > git commit -m "Long filename" > ' > + > +test_expect_failure 'handle existing decomposed filenames' ' > + echo content >"verbatim.$Adiarnfd" && > + git -c core.precomposeunicode=false add "verbatim.$Adiarnfd" && > + git commit -m "existing decomposed file" && > + >expect && > + git ls-files --exclude-standard -o "verbatim*" >untracked && > + test_cmp expect untracked > +' > + > # Test if the global core.precomposeunicode stops autosensing > # Must be the last test case > test_expect_success "respect git config --global core.precomposeunicode" ' On top of the we need to following: diff --git a/compat/precompose_utf8.c b/compat/precompose_utf8.c index 95fe849..5ed3d17 100644 --- a/compat/precompose_utf8.c +++ b/compat/precompose_utf8.c @@ -57,6 +57,19 @@ void probe_utf8_pathname_composition(char *path, int len) } +char *precompose_str_len(const char *in, size_t insz, int *outsz)
Re: [PATCH 1/9] Define a structure for object IDs.
On 05/03/2014 10:12 PM, brian m. carlson wrote: > Many places throughout the code use "unsigned char [20]" to store object IDs > (SHA-1 values). This leads to lots of hardcoded numbers throughout the > codebase. It also leads to confusion about the purposes of a buffer. > > Introduce a structure for object IDs. This allows us to obtain the benefits > of compile-time checking for misuse. The structure is expected to remain > the same size and have the same alignment requirements on all known > platforms, compared to the array of unsigned char. Please clarify whether you plan to rely on all platforms having "the same size and alignment constraints" for correctness, or whether that observation of the status quo is only meant to reassure us that this change won't cause memory to be wasted on padding. If the former then I would feel very uncomfortable about the change. Otherwise I think it will be a nice improvement in code clarity (and I admire your ambition in taking on this project!) 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: BUG or FEATURE? Use of '/' in branch names
On 05/03/2014 09:35 AM, Dennis Kaarsemaker wrote: > On vr, 2014-05-02 at 15:16 -0700, Jonathan Nieder wrote: >>> $ git checkout -b hotfix/b2 >>> error: unable to resolve reference refs/heads/hotfix/b2: Not a >> directory >>> fatal: Failed to lock ref for update: Not a directory >>> $ >> >> That's an ugly message. I think we can do better. (hint hint) > > 2.0.0-rc2 has a better message already: > > $ git checkout -b hotfix/b2 > error: 'refs/heads/hotfix' exists; cannot create 'refs/heads/hotfix/b2' > fatal: Failed to lock ref for update: Not a directory I was trying to remember when this was changed, but at first I couldn't reproduce the "fixed" error message at all. Finally I figured out that the the error message that you get depends on whether the existing reference is loose: $ bin-wrappers/git checkout -b master/foo error: unable to resolve reference refs/heads/master/foo: Not a directory fatal: Failed to lock ref for update: Not a directory vs. packed: $ bin-wrappers/git checkout -b base/foo error: 'refs/heads/base' exists; cannot create 'refs/heads/base/foo' fatal: Failed to lock ref for update: Not a directory It would be good to make the error message uniform and to document this restriction. 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: Pull is Mostly Evil
David Lang wrote: > note that this is one person taking the "I don't see any commits from > you so your opinion doesn't count" attitude. Wrong. I said it doesn't count "for the project". Do you honestly believe Junio cares about what some random guy on the list thinks about default aliases? No. If he doesn't care what literally everyone thinks about the name "index", why would he care about that random guy? > the vast majority of people here do not take that attitude. It's actually the exact opposite. I don't care what is the track record of the people in the discussion. If their argument is good, their argument is good. It's the others that focus on the carisma and credentials of the people in the discussion, rather than the arguments. -- 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: Pull is Mostly Evil
On Sat, 3 May 2014, David Kastrup wrote: Felipe Contreras writes: David Kastrup wrote: Richard Hansen writes: These three usage patterns are at odds; it's hard to change the default behavior of 'git pull' to favor one usage case without harming another. Perhaps this is why there's so much disagreement about what 'git pull' should do. Should a screwdriver be turning clockwise or counterclockwise by default? There are valid arguments for either. If you don't have anything to contribute don't disturb the people that actually care and are trying to improve Git. Thanks. No need to expand on the welcoming atmosphere here. note that this is one person taking the "I don't see any commits from you so your opinion doesn't count" attitude. the vast majority of people here do not take that attitude. David Lang My heinous plot to subvert the quality of Git has already been thwarted by making sure that its "meritocracy" continues relying only on input from those with an independent income. I'm just sticking around until my current contributions move into master so that I can summarize the resulting low-hanging fruit that the meritorious can then pick at great fanfare. The sooner my work moves from pu into master, the sooner y'all be rid of me.
Re: Pull is Mostly Evil
Richard Hansen wrote: > On 2014-05-03 05:26, Felipe Contreras wrote: > > Richard Hansen wrote: > > > >> I think the fundamental difference is in the relationship between the > >> local and the remote branch (which branch derives from the other). > >> The relationship between the branches determines what the user wants > >> from 'git pull'. > >> > >> In my experience 'git pull' is mostly (only?) used for the following > >> three tasks: > > > > I agree. > > > >> 1. update a local branch to incorporate the latest upstream changes > >> > >> In this case, the local branch (master) is a derivative of the > >> upstream branch (origin/master). The user wants all of the > >> commits in the remote branch to be in the local branch. And the > >> user would like the local changes, if any, to descend from the tip > >> of the remote branch. > > > > My current propsal of making `git pull` by default do --ff-only would > > solve this. > > It would go a long way toward improving the situation, yes. > > > In addition I think by default 'master' should be merged to > > 'origin/master', if say --merge is given. > > This would break cases #2 and #3. (With cases #2 and #3 you want the > fetched branch to be the second parent, not the first.) > > Or are you proposing that pull --merge should reverse the parents if and > only if the remote ref is @{u}? Only if no remote or branch are specified `git pull --merge`. > > > >> For this case, 'git pull --ff-only' followed by 'git rebase -p' > >> works well, as does 'git pull --rebase=preserve' if the user is > >> comfortable rebasing without reviewing the incoming commits first. > > > > I suppose you mean a `git rebase -p` if the `git pull --ff-only` failed. > > Yes. > > > This might be OK on most projects, but not all. > > The rebase only affects the local repository (the commits haven't been > pushed yet or else they'd be in @{u} already), so I'd say it's more of > an individual developer decision than a project decision. > > In my opinion rebase would be the best option here, but if the project > is OK with developers pushing merge or merge-there commits and the > developer isn't yet comfortable with rebasing, then merge is also an > acceptable option. Precisely for that reason. > >> 2. update a published feature branch with the latest changes from its > >> parent branch > > We probably shouldn't change that. > > If we change 'git pull' to default to --ff-only but let 'git pull > $remote [$refspec]' continue to default to --ff then we have two > different behaviors depending on how 'git pull' is invoked. I'm worried > that this would trip up users. I'm not convinced that having two > different behaviors would be bad, but I'm not convinced that it would be > good either. It is the only solution that has been proposed. Moreover, while it's a bit worrisome, it wouldn't create any actual problems. Since `git pull $what` remains the same, there's no problems there. The only change would be on `git pull`. Since most users are not going to do `git pull $what` therefore it would only be a small subset of users that would notice the discrepancy between running with $what, or not. And the only discrepancy they could notice is that when they run `git pull $what` they expect it to be --ff-only, or when the run `git pull` they don't. Only the former could be an issue, but even then, it's highly unlikely that `git pull $what` would ever be a fast-forward. So althought conceptually it doesn't look clean, in reality there wouldn't be any problems. > >> 3. integrate a more-or-less complete feature/fix back into the line > >> of development it forked off of > >> > >> In this case the local branch is a primary line of development and > >> the remote branch contains the derivative work. Think Linus > >> pulling in contributions. Different situations will call for > >> different ways to handle this case, but most will probably want > >> some or all of: > >> > >> * rebase the remote commits onto local HEAD > > > > No. Most people will merge the remote branch as it is. There's no reason > > to rebase, specially if you are creating a merge commit. > > I disagree. I prefer to rebase a topic branch before merging (no-ff) to > the main line of development for a couple of reasons: Well that is *your* preference. Most people would prefer to preserve the history. > * It makes commits easier to review. The review in the vast majority of cases happens *before* the integration. And the problem comes when the integrator makes a mistake, which they inevitable do (we all do), then there's no history about how the conflict was resolved, and what whas the original patch. That's why most people don't do this. > * Rebasing makes the commit history pretty and easier to understand. It is more important to be able to track integration errors than to have a pretty history. That is for most people. I like to have a
[PATCH 0/4] remote-hg: more improvements
Here's a bunch of tests more, and a fixes for Mercurial v3.0. Felipe Contreras (4): remote-hg: add more tests t: remote-hg: add file operation tests t: remote-hg: trivial cleanups and fixes remote-hg: add support for hg v3.0 contrib/remote-helpers/git-remote-hg | 6 +- contrib/remote-helpers/test-hg.sh| 240 ++- 2 files changed, 238 insertions(+), 8 deletions(-) -- 1.9.2+fc1.20.g204a630 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/4] t: remote-hg: trivial cleanups and fixes
There was a broken && chain, and cat is simpler than echo in a subshell. Signed-off-by: Felipe Contreras --- contrib/remote-helpers/test-hg.sh | 14 +++--- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/contrib/remote-helpers/test-hg.sh b/contrib/remote-helpers/test-hg.sh index df09966..9b9468b 100755 --- a/contrib/remote-helpers/test-hg.sh +++ b/contrib/remote-helpers/test-hg.sh @@ -25,7 +25,7 @@ fi check () { echo $3 >expected && - git --git-dir=$1/.git log --format='%s' -1 $2 >actual + git --git-dir=$1/.git log --format='%s' -1 $2 >actual && test_cmp expected actual } @@ -103,12 +103,12 @@ check_push () { } setup () { - ( - echo "[ui]" - echo "username = H G Wells " - echo "[extensions]" - echo "mq =" - ) >>"$HOME"/.hgrc && + cat >> "$HOME"/.hgrc <<-EOF && + [ui] + username = H G Wells + [extensions] + mq = + EOF GIT_AUTHOR_DATE="2007-01-01 00:00:00 +0230" && GIT_COMMITTER_DATE="$GIT_AUTHOR_DATE" && -- 1.9.2+fc1.20.g204a630 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/4] remote-hg: add more tests
Inspired by the tests in gitifyhg. One test is failing, but that's because of a limitation of remote-helpers. Signed-off-by: Felipe Contreras --- contrib/remote-helpers/test-hg.sh | 150 ++ 1 file changed, 150 insertions(+) diff --git a/contrib/remote-helpers/test-hg.sh b/contrib/remote-helpers/test-hg.sh index 7d90056..33a434d 100755 --- a/contrib/remote-helpers/test-hg.sh +++ b/contrib/remote-helpers/test-hg.sh @@ -845,4 +845,154 @@ test_expect_success 'clone remote with generic null bookmark, then push to the b ) ' +test_expect_success 'notes' ' + test_when_finished "rm -rf hgrepo gitrepo" && + + ( + hg init hgrepo && + cd hgrepo && + echo one > content && + hg add content && + hg commit -m one && + echo two > content && + hg commit -m two + ) && + + git clone "hg::hgrepo" gitrepo && + hg -R hgrepo log --template "{node}\n\n" > expected && + git --git-dir=gitrepo/.git log --pretty="tformat:%N" --notes=hg > actual && + test_cmp expected actual +' + +test_expect_failure 'push updates notes' ' + test_when_finished "rm -rf hgrepo gitrepo" && + + ( + hg init hgrepo && + cd hgrepo && + echo one > content && + hg add content && + hg commit -m one + ) && + + git clone "hg::hgrepo" gitrepo && + + ( + cd gitrepo && + echo two > content && + git commit -a -m two + git push + ) && + + hg -R hgrepo log --template "{node}\n\n" > expected && + git --git-dir=gitrepo/.git log --pretty="tformat:%N" --notes=hg > actual && + test_cmp expected actual +' + +test_expect_success 'pull tags' ' + test_when_finished "rm -rf hgrepo gitrepo" && + + ( + hg init hgrepo && + cd hgrepo && + echo one > content && + hg add content && + hg commit -m one + ) && + + git clone "hg::hgrepo" gitrepo && + + (cd hgrepo && hg tag v1.0) && + (cd gitrepo && git pull) && + + echo "v1.0" > expected && + git --git-dir=gitrepo/.git tag > actual && + test_cmp expected actual +' + +test_expect_success 'push merged named branch' ' + test_when_finished "rm -rf hgrepo gitrepo" && + + ( + hg init hgrepo && + cd hgrepo && + echo one > content && + hg add content && + hg commit -m one && + hg branch feature && + echo two > content && + hg commit -m two && + hg update default && + echo three > content && + hg commit -m three + ) && + + ( + git clone "hg::hgrepo" gitrepo && + cd gitrepo && + git merge -m Merge -Xtheirs origin/branches/feature && + git push + ) && + + cat > expected <<-EOF + Merge + three + two + one + EOF + hg -R hgrepo log --template "{desc}\n" > actual && + test_cmp expected actual +' + +test_expect_success 'light tag sets author' ' + test_when_finished "rm -rf hgrepo gitrepo" && + + ( + hg init hgrepo && + cd hgrepo && + echo one > content && + hg add content && + hg commit -m one + ) && + + ( + git clone "hg::hgrepo" gitrepo && + cd gitrepo && + git tag v1.0 && + git push --tags + ) && + + echo "C O Mitter " > expected && + hg -R hgrepo log --template "{author}\n" -r tip > actual && + test_cmp expected actual +' + +test_expect_success 'push tag different branch' ' + test_when_finished "rm -rf hgrepo gitrepo" && + + ( + hg init hgrepo && + cd hgrepo && + echo one > content && + hg add content && + hg commit -m one + hg branch feature && + echo two > content && + hg commit -m two + ) && + + ( + git clone "hg::hgrepo" gitrepo && + cd gitrepo && + git branch && + git checkout branches/feature && + git tag v1.0 && + git push --tags + ) && + + echo feature > expected && + hg -R hgrepo log --template="{branch}\n" -r tip > actual && + test_cmp expected actual +' + test_done -- 1.9.2+fc1.20.g204a630 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/4] t: remote-hg: add file operation tests
Inspired by gitifyhg. Signed-off-by: Felipe Contreras --- contrib/remote-helpers/test-hg.sh | 76 +++ 1 file changed, 76 insertions(+) diff --git a/contrib/remote-helpers/test-hg.sh b/contrib/remote-helpers/test-hg.sh index 33a434d..df09966 100755 --- a/contrib/remote-helpers/test-hg.sh +++ b/contrib/remote-helpers/test-hg.sh @@ -53,6 +53,17 @@ check_bookmark () { fi } +check_files () { + git --git-dir=$1/.git ls-files > actual && + if test $# -gt 1 + then + printf "%s\n" "$2" > expected + else + > expected + fi && + test_cmp expected actual +} + check_push () { expected_ret=$1 ret=0 ref_ret=0 @@ -995,4 +1006,69 @@ test_expect_success 'push tag different branch' ' test_cmp expected actual ' +test_expect_success 'cloning a removed file works' ' + test_when_finished "rm -rf hgrepo gitrepo" && + + ( + hg init hgrepo && + cd hgrepo && + + echo test > test_file && + hg add test_file && + hg commit -m add && + + hg rm test_file && + hg commit -m remove + ) && + + git clone "hg::hgrepo" gitrepo && + check_files gitrepo +' + +test_expect_success 'cloning a file replaced with a directory' ' + test_when_finished "rm -rf hgrepo gitrepo" && + + ( + hg init hgrepo && + cd hgrepo && + + echo test > dir_or_file && + hg add dir_or_file && + hg commit -m add && + + hg rm dir_or_file && + mkdir dir_or_file && + echo test > dir_or_file/test_file && + hg add dir_or_file/test_file && + hg commit -m replase + ) && + + git clone "hg::hgrepo" gitrepo && + check_files gitrepo "dir_or_file/test_file" +' + +test_expect_success 'clone replace directory with a file' ' + test_when_finished "rm -rf hgrepo gitrepo" && + + ( + hg init hgrepo && + cd hgrepo && + + mkdir dir_or_file && + echo test > dir_or_file/test_file && + hg add dir_or_file/test_file && + hg commit -m add && + + hg rm dir_or_file/test_file && + echo test > dir_or_file && + hg add dir_or_file && + hg commit -m add && + + hg rm dir_or_file + ) && + + git clone "hg::hgrepo" gitrepo && + check_files gitrepo "dir_or_file" +' + test_done -- 1.9.2+fc1.20.g204a630 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/4] remote-hg: add support for hg v3.0
Signed-off-by: Felipe Contreras --- contrib/remote-helpers/git-remote-hg | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/contrib/remote-helpers/git-remote-hg b/contrib/remote-helpers/git-remote-hg index 34cda02..8b02803 100755 --- a/contrib/remote-helpers/git-remote-hg +++ b/contrib/remote-helpers/git-remote-hg @@ -13,6 +13,7 @@ # "$GIT_DIR/hg/origin/clone/.hg/". from mercurial import hg, ui, bookmarks, context, encoding, node, error, extensions, discovery, util +from mercurial import changegroup import re import sys @@ -985,7 +986,10 @@ def push_unsafe(repo, remote, parsed_refs, p_revs): if not checkheads(repo, remote, p_revs): return None -cg = repo.getbundle('push', heads=list(p_revs), common=common) +if check_version(3, 0): +cg = changegroup.getbundle(repo, 'push', heads=list(p_revs), common=common) +else: +cg = repo.getbundle('push', heads=list(p_revs), common=common) unbundle = remote.capable('unbundle') if unbundle: -- 1.9.2+fc1.20.g204a630 -- To unsubscribe from this list: send the line "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: Watchman support for git
On Sun, May 4, 2014 at 3:49 AM, David Turner wrote: > On Sat, 2014-05-03 at 15:49 +0700, Duy Nguyen wrote: >> On Sat, May 3, 2014 at 11:39 AM, David Turner >> wrote: >> >> Index v4 and split index (and the following read-cache daemon, >> >> hopefully) >> > >> > Looking at some of the archives for read-cache daemon, it seems to be >> > somewhat similar to watchman, right? But I only saw inotify code; what >> > about Mac OS? Or am I misunderstanding what it is? >> >> It's mentioned in [1], the second paragraph, mostly to hide index I/O >> read cost and the SHA-1 hashing cost in the background. In theory it >> should work on all platforms that support multiple processes and >> efficient IPC. It can help load watchman file cache faster too. > > Yes, that seems like a good idea. > > I actually wrote some of a more-complicated, weirder version of this > idea. In my version, there was a long-running git daemon process that > held the index, the watchman file cache, and also objects loaded from > the object database. Other git commands would then send their > command-line and arguments over to the daemon, which would run the > commands and send stdin/out/err back. Of course, this is complicated > because git commands are designed to run then exit, so they often rely > on variables being initialized to zero, or fail to free memory. I used > the Boehm GC to handle the memory freeing problem. To handle variables > that needed to be reinitialized, I used __attribute__(section...) to put > them all into one section, which I could save on daemon startup and > restore after each command. I also replaced calls to exit() with a > function that called longjmp() so the daemon could survive commands > failing. Had I continued, I would also have had to track open file > descriptors to avoid leaking those. > > This was a giant mess that only sort-of worked: it was difficult to > track down all of the variables that needed to be reinitialized. > > The advantage of my method is that there was somewhat less data to > marshall over IPC, and that objects could be easily cached; the > disadvantage is complexity, massive code changes, and the fact that it > didn't actually totally work at the time I ran out of time. > > So I'm really looking forward to trying your version! Hm.. I may face the same problem if I'm not careful. So far I think the daemon only holds index data (with on-disk format, not in-memory), mainly to cut out SHA-1 hashing cost. This is still at the idea phase for me though, nothing is materialized yet. > I would like to merge the feature into master. It works well for me, > and some of my colleagues who have tried it out. Have you tried to turn watchman on by default, then run it with git test suite? That usually helps. > I can split the vmac patch into two, but one of them will remain quite > large because it contains the code for VMAC and AES, which total a bit > over 100k. Since the list will probably reject that, I'll post a link > to a repository containing the patches. With the read-cache deamon, I think hashing cost is less of an issue, so new hashing algorithm becomes less important. If you store the file cache in the deamon's memory only, there's no need to hash anything. But I guess you already tried this. > I'm not 100% sure how to split the watchman patch up. I could add the > fs_cache code and then separately add the watchman code that populates > the cache. Do you think there is a need to divide it up beyond this? I'll need to have closer look at your patches to give any suggestions. Although if you don't mind waiting a bit, I can try to put my untracked cache patches in good shape (hopefully in 2 weeks), then you can mostly avoid touching dir.c and reuse my work. I backed away from watchman support because I was worried about its overhead (of watchman itself, and git/watchman IPC because it's not designed specifically for git), which led me to try optimizing git as much as possible without watchman first, then see how/if watchman can help on top of that. I still think it's a good approach (maybe because it started to make me doubt if watchman could pull a big performance win on top to justify the changes to support it) -- 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: [RFC PATCH 0/9] Use a structure for object IDs.
On Sat, May 03, 2014 at 08:12:13PM +, brian m. carlson wrote: > This is a preliminary RFC patch series to move all the relevant uses of > unsigned char [20] to struct object_id. It should not be applied to any > branch yet. > > The goal of this series to improve type-checking in the codebase and to > make it easier to move to a different hash function if the project > decides to do that. This series does not convert all of the codebase, > but only parts. I'm looking for feedback to see if there is consensus > that this is the right direction before investing a large amount of > time. I would like to point out that to get something with as few calls as hashclr converted to use struct object_id requires an insane amount of work, because often major parts of several files have to be converted first. So the list should be aware that this will likely be an extensive series, although it is bisectable, so it could theoretically be done in batches. -- 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: [PATCH] subtree/Makefile: Standardize (esp. for packagers)
On 4 May 2014 05:22:48 GMT+10:00, Felipe Contreras wrote: >I think you should take a look at the Makefile of >contrib/remote-helpers. I bet something simple like that would work >just >fine for subtree. The current makefile is simple enough, just quirky and likes to be a special snowflake. 'sall good, the v2 addresses most of my immediate concerns with it. Regards, James Denholm. -- To unsubscribe from this list: send the line "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: Pull is Mostly Evil
On 2014-05-03 05:26, Felipe Contreras wrote: > Richard Hansen wrote: > >> I think the fundamental difference is in the relationship between the >> local and the remote branch (which branch derives from the other). >> The relationship between the branches determines what the user wants >> from 'git pull'. >> >> In my experience 'git pull' is mostly (only?) used for the following >> three tasks: > > I agree. > >> 1. update a local branch to incorporate the latest upstream changes >> >> In this case, the local branch (master) is a derivative of the >> upstream branch (origin/master). The user wants all of the >> commits in the remote branch to be in the local branch. And the >> user would like the local changes, if any, to descend from the tip >> of the remote branch. > > My current propsal of making `git pull` by default do --ff-only would > solve this. It would go a long way toward improving the situation, yes. > In addition I think by default 'master' should be merged to > 'origin/master', if say --merge is given. This would break cases #2 and #3. (With cases #2 and #3 you want the fetched branch to be the second parent, not the first.) Or are you proposing that pull --merge should reverse the parents if and only if the remote ref is @{u}? > >> For this case, 'git pull --ff-only' followed by 'git rebase -p' >> works well, as does 'git pull --rebase=preserve' if the user is >> comfortable rebasing without reviewing the incoming commits first. > > I suppose you mean a `git rebase -p` if the `git pull --ff-only` failed. Yes. > This might be OK on most projects, but not all. The rebase only affects the local repository (the commits haven't been pushed yet or else they'd be in @{u} already), so I'd say it's more of an individual developer decision than a project decision. In my opinion rebase would be the best option here, but if the project is OK with developers pushing merge or merge-there commits and the developer isn't yet comfortable with rebasing, then merge is also an acceptable option. > > What happens after a `git pull --ff-only` fails should be totally > up to the user. I tend to agree, mostly because I want users to have an opportunity to review incoming commits before action is taken. Also, though rebasing would yield the nicest history, some users aren't yet comfortable with rebase. If a project is OK with silly little merge commits from users that aren't comfortable with rebase, then I don't want to force everyone to rebase by default. As an added bonus: Defaulting to --ff-only makes it possible for 'git pull --all' to fast-forward every local branch to their configured upstream, not just the currently checked-out branch. I think this would be a huge usability win. > >> 2. update a published feature branch with the latest changes from its >> parent branch >> >> In this case, the local branch (foo) is a derivative of the >> upstream branch (origin/foo) which is itself a derivative of >> another branch (origin/master). All commits in origin/master >> should be in origin/foo, and ideally all commits unique to >> origin/foo would descend from the tip of origin/master. > > I don't understand why are you tainting the example with 'origin/foo', Originally I didn't have this case in my list, but I added it after thinking about Peff's comment: On 2014-05-02 17:48, Jeff King wrote: > One common workflow for GitHub users is to back-merge master into a > topic, because they want the final "integrated" version on the topic > branch. This almost but doesn't quite fit neatly into the other two cases. It's not case #1 because the shared nature of origin/foo means that rebasing origin/foo onto origin/master is usually bad instead of usually good. It's not case #3 because rebasing origin/master commits onto origin/foo (assuming that the user would usually want to rebase the topic branch when integrating) would definitely be bad. > 'foo' and 'origin/master' are enough for this example. In fact, the > mention of 'origin/master' made it wrong: after the pull not all the > commits of origin/master would be in origin/foo, you need a push for > that. The push of foo to origin/foo was meant to be implied. > We have enough in our plate to taint this with yet another branch > and push. > > For this case `git pull origin master` already work correctly for most > projects. Yes, it does. > We probably shouldn't change that. If we change 'git pull' to default to --ff-only but let 'git pull $remote [$refspec]' continue to default to --ff then we have two different behaviors depending on how 'git pull' is invoked. I'm worried that this would trip up users. I'm not convinced that having two different behaviors would be bad, but I'm not convinced that it would be good either. > >> 3. integrate a more-or-less complete feature/fix back into the line >> of development it forked off of >> >> In this case the local bra
Re: Watchman support for git
On Sat, 2014-05-03 at 15:49 +0700, Duy Nguyen wrote: > On Sat, May 3, 2014 at 11:39 AM, David Turner > wrote: > >> Index v4 and split index (and the following read-cache daemon, > >> hopefully) > > > > Looking at some of the archives for read-cache daemon, it seems to be > > somewhat similar to watchman, right? But I only saw inotify code; what > > about Mac OS? Or am I misunderstanding what it is? > > It's mentioned in [1], the second paragraph, mostly to hide index I/O > read cost and the SHA-1 hashing cost in the background. In theory it > should work on all platforms that support multiple processes and > efficient IPC. It can help load watchman file cache faster too. Yes, that seems like a good idea. I actually wrote some of a more-complicated, weirder version of this idea. In my version, there was a long-running git daemon process that held the index, the watchman file cache, and also objects loaded from the object database. Other git commands would then send their command-line and arguments over to the daemon, which would run the commands and send stdin/out/err back. Of course, this is complicated because git commands are designed to run then exit, so they often rely on variables being initialized to zero, or fail to free memory. I used the Boehm GC to handle the memory freeing problem. To handle variables that needed to be reinitialized, I used __attribute__(section...) to put them all into one section, which I could save on daemon startup and restore after each command. I also replaced calls to exit() with a function that called longjmp() so the daemon could survive commands failing. Had I continued, I would also have had to track open file descriptors to avoid leaking those. This was a giant mess that only sort-of worked: it was difficult to track down all of the variables that needed to be reinitialized. The advantage of my method is that there was somewhat less data to marshall over IPC, and that objects could be easily cached; the disadvantage is complexity, massive code changes, and the fact that it didn't actually totally work at the time I ran out of time. So I'm really looking forward to trying your version! > >> The last line could be a competition between watchman and my coming > >> "untracked cache" series. I expect to cut the number in that line at > >> least in half without external dependency. > > > > I hadn't seen the "untracked cached" work (I actually finished these > > patches a month or so ago but have been waiting for some internal > > reviews before sending them out). Looks interesting. It seems we use a > > similar strategy for handling ignores. > > Yep, mostly the same at the core, except that I exploit directory > mtime while you use inotify. Each approach has its own pros and cons, > I think. Both should face the same traps in caching (e.g. if you "git > rm --cached" a file, that file could be come either untracked, or > ignored). > > >> Patch 2/3 did not seem to make it to the list by the way.. > > > > Thanks for your comments. I just tried again to send patch 2/3. I do > > actually see the CC of it in my @twitter.com mailbox, but I don't see it > > in the archives on the web. Do you know if there is a reason the > > mailing list would reject it? > > Probably its size, 131K, which is also an indicator to split it (and > the third patch) into smaller patches if you want to merge this > feature in master eventually. I would like to merge the feature into master. It works well for me, and some of my colleagues who have tried it out. I can split the vmac patch into two, but one of them will remain quite large because it contains the code for VMAC and AES, which total a bit over 100k. Since the list will probably reject that, I'll post a link to a repository containing the patches. I'm not 100% sure how to split the watchman patch up. I could add the fs_cache code and then separately add the watchman code that populates the cache. Do you think there is a need to divide it up beyond this? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Pull is Mostly Evil
From: "Jonathan Nieder" Sent: Friday, May 02, 2014 11:53 PM Hi, Philip Oakley wrote: That assumes that [git pull] doing something is better than doing nothing, which is appropriate when the costs on either side are roughly similar. I think the conversation's going around in circles. I agree it's going around, but it's a non-exact recurrence. Issues are being surfaced. Potential next steps: a. Documentation or test patch illustrating desired behavior b. More traditional formal design doc explaining desired behavior and the thinking behind it ("problem", "overview of solution", "alternatives rejected", "complications", "example", "open questions"). c. Implementation patch d. Someone takes an existing patch and figures out the next step toward getting it ready for application. My preference is for (a), I guess. I disagree about the leap to the presentation & discussion of a 'solution' in these awkward scenarios (the old joke about "if I were you I wouldn't start from here", when asking for directions tends to apply). This is the same point made by Brooks in the 'Mythical Man Month'. A leap to code is no guarantee of success. The point being that something more concrete (code or a design doc) makes it easier to avoid talking past each other. And having something concrete to edit makes the stakes clearer so people can make it incrementally better without being distracted by unimportant parts. We've had Junio's training wheel, and now Filipe's n'th attempt at code examples, so my bad code wouldn't help ;-). As a systems engineer I've seen these confusions quite a few times in different guises. I tend to fall back to P Checkland's "Systems Thinking, Systems Practice" model of the various processes that have to go on [1] to improve the situation (note he doesn't expect a solved solution in most cases, just an improvement in the situation). At the moment most of the discussion is in the "unstructured" parts of the processes. He also identifies 6 elements 'CATWOE' [2] that need to be considered when studying these problems. Most of the discussion/arguments here are about the different 'Weltanshaung's" (world views) of the contributors. In terms of the new user pull problem, what needs to be modeled is the new user's and their weltanshaung, not how we ('experienced' users?) might 'solve' the problem. The pull problem is, I believe part of the bigger problem of the mind-set shift required for the transition to a DVCS for most new users. Git has grown organically, so still has some soft (unclear) edges, which probably needs more than just a transition plan for Filipe's pull changes, and its choice of the final default (or lack of). For example, if users aren't understanding the differences between remote branches, remote tracking branches, and branches, which is part of the pull problem; have we made it easy for them to understand? [They already have to comprehend the 'staging' concept, so are already cognitively fully loaded]. For the branch type example, some cleaner naming may help, such as: 'remote branch', 'Tracking branch', and '(local) branch', which excludes the noiseword 'remote' from 'Tracking branches' (my deliberate 'T' emphasis). Though that does still leave the confusion between remote servers and remote repos, where the latter may actually be local, and if a file path, be the local '.' repo itself! Thanks and hope that helps, Sorry if this went off at a tangent, but I believe it's important to get to the bottom of the new user problems, which are deeper than just a few command defaults. Jonathan -- Philip -- [1] http://40qx6d15vq6j25i83v3ks8nxfux.wpengine.netdna-cdn.com/files/2012/08/seven-steps2.gif or http://portals.wi.wur.nl/spicad/?Soft_Systems_Methodology Checkland's 7 Steps. [2] CATWOE: customers, actors, transformation, weltanshaung, owners, environment. -- To unsubscribe from this list: send the line "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 8/9] cache-tree: convert struct cache_tree to use object_id
Signed-off-by: brian m. carlson --- builtin/commit.c | 2 +- builtin/fsck.c | 4 ++-- cache-tree.c | 30 +++--- cache-tree.h | 3 ++- merge-recursive.c | 2 +- reachable.c| 2 +- sequencer.c| 2 +- test-dump-cache-tree.c | 4 ++-- 8 files changed, 25 insertions(+), 24 deletions(-) diff --git a/builtin/commit.c b/builtin/commit.c index 9cfef6c..639f843 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -1659,7 +1659,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix) append_merge_tag_headers(parents, &tail); } - if (commit_tree_extended(&sb, active_cache_tree->sha1, parents, sha1, + if (commit_tree_extended(&sb, active_cache_tree->sha1.oid, parents, sha1, author_ident.buf, sign_commit, extra)) { rollback_index_files(); die(_("failed to write commit object")); diff --git a/builtin/fsck.c b/builtin/fsck.c index fc150c8..6854c81 100644 --- a/builtin/fsck.c +++ b/builtin/fsck.c @@ -587,10 +587,10 @@ static int fsck_cache_tree(struct cache_tree *it) fprintf(stderr, "Checking cache tree\n"); if (0 <= it->entry_count) { - struct object *obj = parse_object(it->sha1); + struct object *obj = parse_object(it->sha1.oid); if (!obj) { error("%s: invalid sha1 pointer in cache-tree", - sha1_to_hex(it->sha1)); + sha1_to_hex(it->sha1.oid)); return 1; } obj->used = 1; diff --git a/cache-tree.c b/cache-tree.c index 7fa524a..b7b2d06 100644 --- a/cache-tree.c +++ b/cache-tree.c @@ -219,7 +219,7 @@ int cache_tree_fully_valid(struct cache_tree *it) int i; if (!it) return 0; - if (it->entry_count < 0 || !has_sha1_file(it->sha1)) + if (it->entry_count < 0 || !has_sha1_file(it->sha1.oid)) return 0; for (i = 0; i < it->subtree_nr; i++) { if (!cache_tree_fully_valid(it->down[i]->cache_tree)) @@ -244,7 +244,7 @@ static int update_one(struct cache_tree *it, *skip_count = 0; - if (0 <= it->entry_count && has_sha1_file(it->sha1)) + if (0 <= it->entry_count && has_sha1_file(it->sha1.oid)) return it->entry_count; /* @@ -311,7 +311,7 @@ static int update_one(struct cache_tree *it, struct cache_tree_sub *sub; const char *path, *slash; int pathlen, entlen; - const unsigned char *sha1; + const struct object_id *sha1; unsigned mode; path = ce->name; @@ -327,21 +327,21 @@ static int update_one(struct cache_tree *it, die("cache-tree.c: '%.*s' in '%s' not found", entlen, path + baselen, path); i += sub->count; - sha1 = sub->cache_tree->sha1; + sha1 = &sub->cache_tree->sha1; mode = S_IFDIR; if (sub->cache_tree->entry_count < 0) to_invalidate = 1; } else { - sha1 = ce->sha1; + sha1 = (struct object_id *)ce->sha1; mode = ce->ce_mode; entlen = pathlen - baselen; i++; } - if (mode != S_IFGITLINK && !missing_ok && !has_sha1_file(sha1)) { + if (mode != S_IFGITLINK && !missing_ok && !has_sha1_file(sha1->oid)) { strbuf_release(&buffer); return error("invalid object %06o %s for '%.*s'", - mode, sha1_to_hex(sha1), entlen+baselen, path); + mode, sha1_to_hex(sha1->oid), entlen+baselen, path); } /* @@ -375,8 +375,8 @@ static int update_one(struct cache_tree *it, } if (dryrun) - hash_sha1_file(buffer.buf, buffer.len, tree_type, it->sha1); - else if (write_sha1_file(buffer.buf, buffer.len, tree_type, it->sha1)) { + hash_sha1_file(buffer.buf, buffer.len, tree_type, it->sha1.oid); + else if (write_sha1_file(buffer.buf, buffer.len, tree_type, it->sha1.oid)) { strbuf_release(&buffer); return -1; } @@ -432,7 +432,7 @@ static void write_one(struct strbuf *buffer, struct cache_tree *it, #endif if (0 <= it->entry_count) { - strbuf_add(buffer, it->sha1, 20); + strbuf_add(buffer, it->sha1.oid, GIT_OID_RAWSZ); } for (i = 0; i < it->subtree_nr; i++) { struct cache_tree_sub *down = it->down[i]; @@ -489,7 +4
[PATCH 7/9] bundle.c: convert leaf functions to struct object_id
Signed-off-by: brian m. carlson --- bundle.c | 38 +++--- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/bundle.c b/bundle.c index 1222952..798ba28 100644 --- a/bundle.c +++ b/bundle.c @@ -11,11 +11,11 @@ static const char bundle_signature[] = "# v2 git bundle\n"; -static void add_to_ref_list(const unsigned char *sha1, const char *name, +static void add_to_ref_list(const struct object_id *sha1, const char *name, struct ref_list *list) { ALLOC_GROW(list->list, list->nr + 1, list->alloc); - hashcpy(list->list[list->nr].sha1, sha1); + hashcpy(list->list[list->nr].sha1, sha1->oid); list->list[list->nr].name = xstrdup(name); list->nr++; } @@ -39,7 +39,7 @@ static int parse_bundle_header(int fd, struct bundle_header *header, /* The bundle header ends with an empty line */ while (!strbuf_getwholeline_fd(&buf, fd, '\n') && buf.len && buf.buf[0] != '\n') { - unsigned char sha1[20]; + struct object_id sha1; int is_prereq = 0; if (*buf.buf == '-') { @@ -53,9 +53,9 @@ static int parse_bundle_header(int fd, struct bundle_header *header, * Prerequisites have object name that is optionally * followed by SP and subject line. */ - if (get_sha1_hex(buf.buf, sha1) || - (buf.len > 40 && !isspace(buf.buf[40])) || - (!is_prereq && buf.len <= 40)) { + if (get_sha1_hex(buf.buf, sha1.oid) || + (buf.len > GIT_OID_HEXSZ && !isspace(buf.buf[GIT_OID_HEXSZ])) || + (!is_prereq && buf.len <= GIT_OID_HEXSZ)) { if (report_path) error(_("unrecognized header: %s%s (%d)"), (is_prereq ? "-" : ""), buf.buf, (int)buf.len); @@ -63,9 +63,9 @@ static int parse_bundle_header(int fd, struct bundle_header *header, break; } else { if (is_prereq) - add_to_ref_list(sha1, "", &header->prerequisites); + add_to_ref_list(&sha1, "", &header->prerequisites); else - add_to_ref_list(sha1, buf.buf + 41, &header->references); + add_to_ref_list(&sha1, buf.buf + 41, &header->references); } } @@ -274,16 +274,16 @@ int create_bundle(struct bundle_header *header, const char *path, return -1; rls_fout = xfdopen(rls.out, "r"); while (strbuf_getwholeline(&buf, rls_fout, '\n') != EOF) { - unsigned char sha1[20]; + struct object_id sha1; if (buf.len > 0 && buf.buf[0] == '-') { write_or_die(bundle_fd, buf.buf, buf.len); - if (!get_sha1_hex(buf.buf + 1, sha1)) { - struct object *object = parse_object_or_die(sha1, buf.buf); + if (!get_sha1_hex(buf.buf + 1, sha1.oid)) { + struct object *object = parse_object_or_die(sha1.oid, buf.buf); object->flags |= UNINTERESTING; add_pending_object(&revs, object, buf.buf); } - } else if (!get_sha1_hex(buf.buf, sha1)) { - struct object *object = parse_object_or_die(sha1, buf.buf); + } else if (!get_sha1_hex(buf.buf, sha1.oid)) { + struct object *object = parse_object_or_die(sha1.oid, buf.buf); object->flags |= SHOWN; } } @@ -302,16 +302,16 @@ int create_bundle(struct bundle_header *header, const char *path, for (i = 0; i < revs.pending.nr; i++) { struct object_array_entry *e = revs.pending.objects + i; - unsigned char sha1[20]; + struct object_id sha1; char *ref; const char *display_ref; int flag; if (e->item->flags & UNINTERESTING) continue; - if (dwim_ref(e->name, strlen(e->name), sha1, &ref) != 1) + if (dwim_ref(e->name, strlen(e->name), sha1.oid, &ref) != 1) continue; - if (read_ref_full(e->name, sha1, 1, &flag)) + if (read_ref_full(e->name, sha1.oid, 1, &flag)) flag = 0; display_ref = (flag & REF_ISSYMREF) ? e->name : ref; @@ -342,13 +342,13 @@ int create_bundle(struct bundle_header *header, const char *path, * commit that is referenced by the tag, and not the tag * itself. */ - if (hashcmp(sha1, e->item->sha1)) { +
[RFC PATCH 0/9] Use a structure for object IDs.
This is a preliminary RFC patch series to move all the relevant uses of unsigned char [20] to struct object_id. It should not be applied to any branch yet. The goal of this series to improve type-checking in the codebase and to make it easier to move to a different hash function if the project decides to do that. This series does not convert all of the codebase, but only parts. I'm looking for feedback to see if there is consensus that this is the right direction before investing a large amount of time. Certain parts of the code have to be converted before others to keep the patch sizes small, maintainable, and bisectable, so functions and structures that are used across the codebase (e.g. hashcmp and struct object) will be converted later. Conversion has been done in a roughly alphabetical order by name of file. The constants for raw and hex sizes of SHA-1 values are maintained. These constants are used where the quantity is the size of an SHA-1 value, and sizeof(struct object_id) is used wherever memory is to be allocated. This is done to permit the struct to turn into a union later if multiple hashes are supported. I left the names at GIT_OID_RAWSZ and GIT_OID_HEXSZ because that's what libgit2 uses and what Junio seemed to prefer, but they can be changed later if there's a desire to do that. I called the structure member "oid" because it was easily grepable and distinct from the rest of the codebase. It, too, can be changed if we decide on a better name. I specifically did not choose "sha1" since it looks weird to have "sha1->sha1" and I didn't want to rename lots of variables. Comments? brian m. carlson (9): Define a structure for object IDs. bisect.c: convert to use struct object_id archive.c: convert to use struct object_id zip: use GIT_OID_HEXSZ for trailers branch.c: convert to use struct object_id bulk-checkin.c: convert to use struct object_id bundle.c: convert leaf functions to struct object_id cache-tree: convert struct cache_tree to use object_id diff: convert struct combine_diff_path to object_id archive-zip.c | 4 ++-- archive.c | 16 +++ archive.h | 1 + bisect.c | 30 ++-- branch.c | 16 +++ builtin/commit.c | 2 +- builtin/fsck.c | 4 ++-- bulk-checkin.c | 12 +-- bundle.c | 38 +-- cache-tree.c | 30 ++-- cache-tree.h | 3 ++- combine-diff.c | 54 +- diff-lib.c | 10 +- diff.h | 5 +++-- merge-recursive.c | 2 +- object.h | 13 +++- reachable.c| 2 +- sequencer.c| 2 +- test-dump-cache-tree.c | 4 ++-- 19 files changed, 131 insertions(+), 117 deletions(-) -- 2.0.0.rc0 -- To unsubscribe from this list: send the line "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 9/9] diff: convert struct combine_diff_path to object_id
Signed-off-by: brian m. carlson --- combine-diff.c | 54 +++--- diff-lib.c | 10 +- diff.h | 5 +++-- 3 files changed, 35 insertions(+), 34 deletions(-) diff --git a/combine-diff.c b/combine-diff.c index 24ca7e2..f97eb3a 100644 --- a/combine-diff.c +++ b/combine-diff.c @@ -34,9 +34,9 @@ static struct combine_diff_path *intersect_paths(struct combine_diff_path *curr, memset(p->parent, 0, sizeof(p->parent[0]) * num_parent); - hashcpy(p->sha1, q->queue[i]->two->sha1); + hashcpy(p->sha1.oid, q->queue[i]->two->sha1); p->mode = q->queue[i]->two->mode; - hashcpy(p->parent[n].sha1, q->queue[i]->one->sha1); + hashcpy(p->parent[n].sha1.oid, q->queue[i]->one->sha1); p->parent[n].mode = q->queue[i]->one->mode; p->parent[n].status = q->queue[i]->status; *tail = p; @@ -67,7 +67,7 @@ static struct combine_diff_path *intersect_paths(struct combine_diff_path *curr, continue; } - hashcpy(p->parent[n].sha1, q->queue[i]->one->sha1); + hashcpy(p->parent[n].sha1.oid, q->queue[i]->one->sha1); p->parent[n].mode = q->queue[i]->one->mode; p->parent[n].status = q->queue[i]->status; @@ -274,7 +274,7 @@ static struct lline *coalesce_lines(struct lline *base, int *lenbase, return base; } -static char *grab_blob(const unsigned char *sha1, unsigned int mode, +static char *grab_blob(const struct object_id *sha1, unsigned int mode, unsigned long *size, struct userdiff_driver *textconv, const char *path) { @@ -284,20 +284,20 @@ static char *grab_blob(const unsigned char *sha1, unsigned int mode, if (S_ISGITLINK(mode)) { blob = xmalloc(100); *size = snprintf(blob, 100, -"Subproject commit %s\n", sha1_to_hex(sha1)); - } else if (is_null_sha1(sha1)) { +"Subproject commit %s\n", sha1_to_hex(sha1->oid)); + } else if (is_null_sha1(sha1->oid)) { /* deleted blob */ *size = 0; return xcalloc(1, 1); } else if (textconv) { struct diff_filespec *df = alloc_filespec(path); - fill_filespec(df, sha1, 1, mode); + fill_filespec(df, sha1->oid, 1, mode); *size = fill_textconv(textconv, df, &blob); free_filespec(df); } else { - blob = read_sha1_file(sha1, &type, size); + blob = read_sha1_file(sha1->oid, &type, size); if (type != OBJ_BLOB) - die("object '%s' is not a blob!", sha1_to_hex(sha1)); + die("object '%s' is not a blob!", sha1_to_hex(sha1->oid)); } return blob; } @@ -379,7 +379,7 @@ static void consume_line(void *state_, char *line, unsigned long len) } } -static void combine_diff(const unsigned char *parent, unsigned int mode, +static void combine_diff(const struct object_id *parent, unsigned int mode, mmfile_t *result_file, struct sline *sline, unsigned int cnt, int n, int num_parent, int result_deleted, @@ -904,11 +904,11 @@ static void show_combined_header(struct combine_diff_path *elem, "", elem->path, line_prefix, c_meta, c_reset); printf("%s%sindex ", line_prefix, c_meta); for (i = 0; i < num_parent; i++) { - abb = find_unique_abbrev(elem->parent[i].sha1, + abb = find_unique_abbrev(elem->parent[i].sha1.oid, abbrev); printf("%s%s", i ? "," : "", abb); } - abb = find_unique_abbrev(elem->sha1, abbrev); + abb = find_unique_abbrev(elem->sha1.oid, abbrev); printf("..%s%s\n", abb, c_reset); if (mode_differs) { @@ -981,7 +981,7 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent, /* Read the result of merge first */ if (!working_tree_file) - result = grab_blob(elem->sha1, elem->mode, &result_size, + result = grab_blob(&elem->sha1, elem->mode, &result_size, textconv, elem->path); else { /* Used by diff-tree to read from the working tree */ @@ -1003,12 +1003,12 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent, result = strbuf_detach(&buf, NULL); elem->mode = canon_mode(st.st_mode); } else if (S_ISDIR(st.st_mode)) { - unsigned char sha1[20];
[PATCH 2/9] bisect.c: convert to use struct object_id
Signed-off-by: brian m. carlson --- bisect.c | 30 +++--- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/bisect.c b/bisect.c index d6e851d..fe53214 100644 --- a/bisect.c +++ b/bisect.c @@ -15,7 +15,7 @@ static struct sha1_array good_revs; static struct sha1_array skipped_revs; -static unsigned char *current_bad_sha1; +static struct object_id *current_bad_sha1; static const char *argv_checkout[] = {"checkout", "-q", NULL, "--", NULL}; static const char *argv_show_branch[] = {"show-branch", NULL, NULL}; @@ -403,8 +403,8 @@ static int register_ref(const char *refname, const unsigned char *sha1, int flags, void *cb_data) { if (!strcmp(refname, "bad")) { - current_bad_sha1 = xmalloc(20); - hashcpy(current_bad_sha1, sha1); + current_bad_sha1 = xmalloc(sizeof(*current_bad_sha1)); + hashcpy(current_bad_sha1->oid, sha1); } else if (starts_with(refname, "good-")) { sha1_array_append(&good_revs, sha1); } else if (starts_with(refname, "skip-")) { @@ -563,7 +563,7 @@ static struct commit_list *skip_away(struct commit_list *list, int count) for (i = 0; cur; cur = cur->next, i++) { if (i == index) { - if (hashcmp(cur->item->object.sha1, current_bad_sha1)) + if (hashcmp(cur->item->object.sha1, current_bad_sha1->oid)) return cur; if (previous) return previous; @@ -606,7 +606,7 @@ static void bisect_rev_setup(struct rev_info *revs, const char *prefix, /* rev_argv.argv[0] will be ignored by setup_revisions */ argv_array_push(&rev_argv, "bisect_rev_setup"); - argv_array_pushf(&rev_argv, bad_format, sha1_to_hex(current_bad_sha1)); + argv_array_pushf(&rev_argv, bad_format, sha1_to_hex(current_bad_sha1->oid)); for (i = 0; i < good_revs.nr; i++) argv_array_pushf(&rev_argv, good_format, sha1_to_hex(good_revs.sha1[i])); @@ -627,7 +627,7 @@ static void bisect_common(struct rev_info *revs) } static void exit_if_skipped_commits(struct commit_list *tried, - const unsigned char *bad) + const struct object_id *bad) { if (!tried) return; @@ -636,12 +636,12 @@ static void exit_if_skipped_commits(struct commit_list *tried, "The first bad commit could be any of:\n"); print_commit_list(tried, "%s\n", "%s\n"); if (bad) - printf("%s\n", sha1_to_hex(bad)); + printf("%s\n", sha1_to_hex(bad->oid)); printf("We cannot bisect more!\n"); exit(2); } -static int is_expected_rev(const unsigned char *sha1) +static int is_expected_rev(const struct object_id *sha1) { const char *filename = git_path("BISECT_EXPECTED_REV"); struct stat st; @@ -657,7 +657,7 @@ static int is_expected_rev(const unsigned char *sha1) return 0; if (strbuf_getline(&str, fp, '\n') != EOF) - res = !strcmp(str.buf, sha1_to_hex(sha1)); + res = !strcmp(str.buf, sha1_to_hex(sha1->oid)); strbuf_release(&str); fclose(fp); @@ -718,7 +718,7 @@ static struct commit **get_bad_and_good_commits(int *rev_nr) struct commit **rev = xmalloc(len * sizeof(*rev)); int i, n = 0; - rev[n++] = get_commit_reference(current_bad_sha1); + rev[n++] = get_commit_reference(current_bad_sha1->oid); for (i = 0; i < good_revs.nr; i++) rev[n++] = get_commit_reference(good_revs.sha1[i]); *rev_nr = n; @@ -729,7 +729,7 @@ static struct commit **get_bad_and_good_commits(int *rev_nr) static void handle_bad_merge_base(void) { if (is_expected_rev(current_bad_sha1)) { - char *bad_hex = sha1_to_hex(current_bad_sha1); + char *bad_hex = sha1_to_hex(current_bad_sha1->oid); char *good_hex = join_sha1_array_hex(&good_revs, ' '); fprintf(stderr, "The merge base %s is bad.\n" @@ -749,7 +749,7 @@ static void handle_bad_merge_base(void) static void handle_skipped_merge_base(const unsigned char *mb) { char *mb_hex = sha1_to_hex(mb); - char *bad_hex = sha1_to_hex(current_bad_sha1); + char *bad_hex = sha1_to_hex(current_bad_sha1->oid); char *good_hex = join_sha1_array_hex(&good_revs, ' '); warning("the merge base between %s and [%s] " @@ -780,7 +780,7 @@ static void check_merge_bases(int no_checkout) for (; result; result = result->next) { const unsigned char *mb = result->item->object.sha1; - if (!hashcmp(mb, current_bad_sha1)) { + if (!hashcmp(mb, current_bad_sha1->oid)) { handle_bad_merge_base(); }
[PATCH 6/9] bulk-checkin.c: convert to use struct object_id
Signed-off-by: brian m. carlson --- bulk-checkin.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/bulk-checkin.c b/bulk-checkin.c index 98e651c..92c7b5e 100644 --- a/bulk-checkin.c +++ b/bulk-checkin.c @@ -23,7 +23,7 @@ static struct bulk_checkin_state { static void finish_bulk_checkin(struct bulk_checkin_state *state) { - unsigned char sha1[20]; + struct object_id sha1; struct strbuf packname = STRBUF_INIT; int i; @@ -35,11 +35,11 @@ static void finish_bulk_checkin(struct bulk_checkin_state *state) unlink(state->pack_tmp_name); goto clear_exit; } else if (state->nr_written == 1) { - sha1close(state->f, sha1, CSUM_FSYNC); + sha1close(state->f, sha1.oid, CSUM_FSYNC); } else { - int fd = sha1close(state->f, sha1, 0); - fixup_pack_header_footer(fd, sha1, state->pack_tmp_name, -state->nr_written, sha1, + int fd = sha1close(state->f, sha1.oid, 0); + fixup_pack_header_footer(fd, sha1.oid, state->pack_tmp_name, +state->nr_written, sha1.oid, state->offset); close(fd); } @@ -47,7 +47,7 @@ static void finish_bulk_checkin(struct bulk_checkin_state *state) strbuf_addf(&packname, "%s/pack/pack-", get_object_directory()); finish_tmp_packfile(&packname, state->pack_tmp_name, state->written, state->nr_written, - &state->pack_idx_opts, sha1); + &state->pack_idx_opts, sha1.oid); for (i = 0; i < state->nr_written; i++) free(state->written[i]); -- 2.0.0.rc0 -- To unsubscribe from this list: send the line "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 5/9] branch.c: convert to use struct object_id
Signed-off-by: brian m. carlson --- branch.c | 16 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/branch.c b/branch.c index 660097b..8dc0d49 100644 --- a/branch.c +++ b/branch.c @@ -184,9 +184,9 @@ int validate_new_branchname(const char *name, struct strbuf *ref, if (!attr_only) { const char *head; - unsigned char sha1[20]; + struct object_id sha1; - head = resolve_ref_unsafe("HEAD", sha1, 0, NULL); + head = resolve_ref_unsafe("HEAD", sha1.oid, 0, NULL); if (!is_bare_repository() && head && !strcmp(head, ref->buf)) die(_("Cannot force update the current branch.")); } @@ -228,7 +228,7 @@ void create_branch(const char *head, { struct ref_lock *lock = NULL; struct commit *commit; - unsigned char sha1[20]; + struct object_id sha1; char *real_ref, msg[PATH_MAX + 20]; struct strbuf ref = STRBUF_INIT; int forcing = 0; @@ -248,7 +248,7 @@ void create_branch(const char *head, } real_ref = NULL; - if (get_sha1(start_name, sha1)) { + if (get_sha1(start_name, sha1.oid)) { if (explicit_tracking) { if (advice_set_upstream_failure) { error(_(upstream_missing), start_name); @@ -260,7 +260,7 @@ void create_branch(const char *head, die(_("Not a valid object name: '%s'."), start_name); } - switch (dwim_ref(start_name, strlen(start_name), sha1, &real_ref)) { + switch (dwim_ref(start_name, strlen(start_name), sha1.oid, &real_ref)) { case 0: /* Not branching from any existing branch */ if (explicit_tracking) @@ -281,9 +281,9 @@ void create_branch(const char *head, break; } - if ((commit = lookup_commit_reference(sha1)) == NULL) + if ((commit = lookup_commit_reference(sha1.oid)) == NULL) die(_("Not a valid branch point: '%s'."), start_name); - hashcpy(sha1, commit->object.sha1); + hashcpy(sha1.oid, commit->object.sha1); if (!dont_change_ref) { lock = lock_any_ref_for_update(ref.buf, NULL, 0, NULL); @@ -305,7 +305,7 @@ void create_branch(const char *head, setup_tracking(ref.buf + 11, real_ref, track, quiet); if (!dont_change_ref) - if (write_ref_sha1(lock, sha1, msg) < 0) + if (write_ref_sha1(lock, sha1.oid, msg) < 0) die_errno(_("Failed to write ref")); strbuf_release(&ref); -- 2.0.0.rc0 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/9] archive.c: convert to use struct object_id
Signed-off-by: brian m. carlson --- archive.c | 16 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/archive.c b/archive.c index 3fc0fb2..dba148a 100644 --- a/archive.c +++ b/archive.c @@ -255,7 +255,7 @@ static void parse_treeish_arg(const char **argv, time_t archive_time; struct tree *tree; const struct commit *commit; - unsigned char sha1[20]; + struct object_id sha1; /* Remotes are only allowed to fetch actual refs */ if (remote && !remote_allow_unreachable) { @@ -263,15 +263,15 @@ static void parse_treeish_arg(const char **argv, const char *colon = strchrnul(name, ':'); int refnamelen = colon - name; - if (!dwim_ref(name, refnamelen, sha1, &ref)) + if (!dwim_ref(name, refnamelen, sha1.oid, &ref)) die("no such ref: %.*s", refnamelen, name); free(ref); } - if (get_sha1(name, sha1)) + if (get_sha1(name, sha1.oid)) die("Not a valid object name"); - commit = lookup_commit_reference_gently(sha1, 1); + commit = lookup_commit_reference_gently(sha1.oid, 1); if (commit) { commit_sha1 = commit->object.sha1; archive_time = commit->date; @@ -280,21 +280,21 @@ static void parse_treeish_arg(const char **argv, archive_time = time(NULL); } - tree = parse_tree_indirect(sha1); + tree = parse_tree_indirect(sha1.oid); if (tree == NULL) die("not a tree object"); if (prefix) { - unsigned char tree_sha1[20]; + struct object_id tree_sha1; unsigned int mode; int err; err = get_tree_entry(tree->object.sha1, prefix, -tree_sha1, &mode); +tree_sha1.oid, &mode); if (err || !S_ISDIR(mode)) die("current working directory is untracked"); - tree = parse_tree_indirect(tree_sha1); + tree = parse_tree_indirect(tree_sha1.oid); } ar_args->tree = tree; ar_args->commit_sha1 = commit_sha1; -- 2.0.0.rc0 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/9] Define a structure for object IDs.
Many places throughout the code use "unsigned char [20]" to store object IDs (SHA-1 values). This leads to lots of hardcoded numbers throughout the codebase. It also leads to confusion about the purposes of a buffer. Introduce a structure for object IDs. This allows us to obtain the benefits of compile-time checking for misuse. The structure is expected to remain the same size and have the same alignment requirements on all known platforms, compared to the array of unsigned char. Signed-off-by: brian m. carlson --- object.h | 13 - 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/object.h b/object.h index 6e12f2c..6a9680d 100644 --- a/object.h +++ b/object.h @@ -1,6 +1,17 @@ #ifndef OBJECT_H #define OBJECT_H +/* + * The length in bytes and in hex digits of an object name (SHA-1 value). + * These are the same names used by libgit2. + */ +#define GIT_OID_RAWSZ 20 +#define GIT_OID_HEXSZ 40 + +struct object_id { + unsigned char oid[GIT_OID_RAWSZ]; +}; + struct object_list { struct object *item; struct object_list *next; @@ -49,7 +60,7 @@ struct object { unsigned used : 1; unsigned type : TYPE_BITS; unsigned flags : FLAG_BITS; - unsigned char sha1[20]; + unsigned char sha1[GIT_OID_RAWSZ]; }; extern const char *typename(unsigned int type); -- 2.0.0.rc0 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/9] zip: use GIT_OID_HEXSZ for trailers
The object.h header is included in archive.h for this constant. It will be used by other parts of the archiving code in the future. Signed-off-by: brian m. carlson --- archive-zip.c | 4 ++-- archive.h | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/archive-zip.c b/archive-zip.c index 4bde019..5b9fe42 100644 --- a/archive-zip.c +++ b/archive-zip.c @@ -427,12 +427,12 @@ static void write_zip_trailer(const unsigned char *sha1) copy_le16(trailer.entries, zip_dir_entries); copy_le32(trailer.size, zip_dir_offset); copy_le32(trailer.offset, zip_offset); - copy_le16(trailer.comment_length, sha1 ? 40 : 0); + copy_le16(trailer.comment_length, sha1 ? GIT_OID_HEXSZ : 0); write_or_die(1, zip_dir, zip_dir_offset); write_or_die(1, &trailer, ZIP_DIR_TRAILER_SIZE); if (sha1) - write_or_die(1, sha1_to_hex(sha1), 40); + write_or_die(1, sha1_to_hex(sha1), GIT_OID_HEXSZ); } static void dos_time(time_t *time, int *dos_date, int *dos_time) diff --git a/archive.h b/archive.h index 4a791e1..fd21408 100644 --- a/archive.h +++ b/archive.h @@ -2,6 +2,7 @@ #define ARCHIVE_H #include "pathspec.h" +#include "object.h" struct archiver_args { const char *base; -- 2.0.0.rc0 -- To unsubscribe from this list: send the line "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] subtree/Makefile: Standardize (esp. for packagers)
James Denholm wrote: > Matthew Ogilvie wrote: > > On Sun, Apr 27, 2014 at 12:35:13PM +1000, James Denholm wrote: > >> Jeff King wrote: > > Agreed. It also doesn't help that when subtree patches are proposed > > (especially new features instead of obvious bugs), there often seems > > to be little or no feedback from anyone. > > > > > > Depending on how much time you have: > > > > This may be outside the scope of work you were planning on, > > While current, immediate focus is really just getting the makefile fixed > up and hopefully then have more people package subtree by default, > overall I'll very likely extend that to general work on subtree and such. I think you should take a look at the Makefile of contrib/remote-helpers. I bet something simple like that would work just fine for subtree. -- 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
[PATCH] RelNotes/2.0.0: Grammar and typo fixes
Signed-off-by: Øyvind A. Holm --- Documentation/RelNotes/2.0.0.txt | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Documentation/RelNotes/2.0.0.txt b/Documentation/RelNotes/2.0.0.txt index e6911ac..35e353e 100644 --- a/Documentation/RelNotes/2.0.0.txt +++ b/Documentation/RelNotes/2.0.0.txt @@ -68,7 +68,7 @@ UI, Workflows & Features e.g. "key-id" in "--gpg-sign="). * 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. + "diff" and "grep -p" has been updated to help C++ source better. * "git rebase" learned to interpret a lone "-" as "@{-1}", the branch that we were previously on. @@ -98,7 +98,7 @@ UI, Workflows & Features tree-wide operation even when run inside a subdirectory of a working tree. - * "git add is the same as "git add -A " now. + * "git add " is the same as "git add -A " now. * "core.statinfo" configuration variable, which is a never-advertised synonym to "core.checkstat", has been removed. @@ -137,7 +137,7 @@ UI, Workflows & Features * "git pull" can be told to only accept fast-forward by setting the new "pull.ff" configuration. - * "git reset" learned "-N" option, which does not reset the index + * "git reset" learned the "-N" option, which does not reset the index fully for paths the index knows about but the tree-ish the command resets to does not (these paths are kept as intend-to-add entries). @@ -156,11 +156,11 @@ Performance, Internal Implementation, etc. "easy" interface. * The bitmap-index feature from JGit has been ported, which should - significantly improve performance when serving objects form a + significantly improve performance when serving objects from a repository that uses it. * The way "git log --cc" shows a combined diff against multiple - parents have been optimized. + parents has been optimized. * The prefixcmp() and suffixcmp() functions are gone. Use starts_with() and ends_with(), and also consider if skip_prefix() -- 2.0.0.rc2 -- To unsubscribe from this list: send the line "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] subtree/Makefile: Standardize (esp. for packagers)
Matthew Ogilvie wrote: > On Sun, Apr 27, 2014 at 12:35:13PM +1000, James Denholm wrote: >> Jeff King wrote: > Agreed. It also doesn't help that when subtree patches are proposed > (especially new features instead of obvious bugs), there often seems > to be little or no feedback from anyone. > > > Depending on how much time you have: > > This may be outside the scope of work you were planning on, While current, immediate focus is really just getting the makefile fixed up and hopefully then have more people package subtree by default, overall I'll very likely extend that to general work on subtree and such. > >but > it might be worth grepping through old mailing list archives for > "subtree" patches that haven't been merged, and see if there is > anything worth revisiting/resubmitting. I believe most of the > following (at least) kind of languished and died, often with little > or no real review and feedback: > > (...) > > (I don't know if these are the latest or "best" versions of these, nor > have I really looked at them closely to decide if they are worth > including at all. Be sure to exameine not just the discussion around > the specific patches, but also the other patches in each series...) Yeah, certainly, I'll be sure to have a sticky-beak. Thanks for pointing those out! Regards, James Denholm. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 2/5] contrib/subtree/Makefile: Use GIT-VERSION-FILE
GVF is already being used in most/all other makefiles in the project, and has been for _quite_ a while. Hence, drop file-unique gitver and replace with GIT_VERSION. Signed-off-by: James Denholm --- contrib/subtree/Makefile | 11 --- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/contrib/subtree/Makefile b/contrib/subtree/Makefile index 87797ed..f63334b 100644 --- a/contrib/subtree/Makefile +++ b/contrib/subtree/Makefile @@ -6,7 +6,10 @@ mandir ?= $(prefix)/share/man libexecdir ?= $(prefix)/libexec/git-core man1dir ?= $(mandir)/man1 -gitver ?= $(word 3,$(shell git --version)) +../../GIT-VERSION-FILE: FORCE + $(MAKE) -C ../../ GIT-VERSION-FILE + +-include ../../GIT-VERSION-FILE # this should be set to a 'standard' bsd-type install program INSTALL ?= install @@ -44,11 +47,11 @@ $(GIT_SUBTREE_DOC): $(GIT_SUBTREE_XML) $(GIT_SUBTREE_XML): $(GIT_SUBTREE_TXT) asciidoc -b docbook -d manpage -f $(ASCIIDOC_CONF) \ - -agit_version=$(gitver) $^ + -agit_version=$(GIT_VERSION) $^ $(GIT_SUBTREE_HTML): $(GIT_SUBTREE_TXT) asciidoc -b xhtml11 -d manpage -f $(ASCIIDOC_CONF) \ - -agit_version=$(gitver) $^ + -agit_version=$(GIT_VERSION) $^ test: $(MAKE) -C t/ test @@ -56,3 +59,5 @@ test: clean: rm -f *~ *.xml *.html *.1 rm -rf subproj mainline + +.PHONY: FORCE -- 1.9.2 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 4/5] contrib/subtree/Makefile: Doc-gen rules cleanup
git:Documentation/Makefile establishes asciidoc/xmlto calls as being handled through their appropriate variables, Hence, change to bring into congruency with. Similarly, MANPAGE_XSL exists in git:Documentation/Makefile, while MANPAGE_NORMAL_XSL does not outside contrib/subtree. Hence, replace MANPAGE_NORMAL_XSL with MANPAGE_XSL. Signed-off-by: James Denholm --- contrib/subtree/Makefile | 13 - 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/contrib/subtree/Makefile b/contrib/subtree/Makefile index 579bb51..f3834b5 100644 --- a/contrib/subtree/Makefile +++ b/contrib/subtree/Makefile @@ -14,8 +14,11 @@ man1dir ?= $(mandir)/man1 # this should be set to a 'standard' bsd-type install program INSTALL ?= install -ASCIIDOC_CONF = ../../Documentation/asciidoc.conf -MANPAGE_NORMAL_XSL = ../../Documentation/manpage-normal.xsl +ASCIIDOC = asciidoc +XMLTO= xmlto + +ASCIIDOC_CONF = ../../Documentation/asciidoc.conf +MANPAGE_XSL = ../../Documentation/manpage-normal.xsl GIT_SUBTREE_SH := git-subtree.sh GIT_SUBTREE:= git-subtree @@ -43,14 +46,14 @@ install-man: $(GIT_SUBTREE_DOC) $(INSTALL) -m 644 $^ $(DESTDIR)$(man1dir) $(GIT_SUBTREE_DOC): $(GIT_SUBTREE_XML) - xmlto -m $(MANPAGE_NORMAL_XSL) man $^ + $(XMLTO) -m $(MANPAGE_XSL) man $^ $(GIT_SUBTREE_XML): $(GIT_SUBTREE_TXT) - asciidoc -b docbook -d manpage -f $(ASCIIDOC_CONF) \ + $(ASCIIDOC) -b docbook -d manpage -f $(ASCIIDOC_CONF) \ -agit_version=$(GIT_VERSION) $^ $(GIT_SUBTREE_HTML): $(GIT_SUBTREE_TXT) - asciidoc -b xhtml11 -d manpage -f $(ASCIIDOC_CONF) \ + $(ASCIIDOC) -b xhtml11 -d manpage -f $(ASCIIDOC_CONF) \ -agit_version=$(GIT_VERSION) $^ test: -- 1.9.2 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 3/5] contrib/subtree/Makefile: s/libexecdir/gitexecdir
$(libexecdir) isn't used anywhere else in the project, while $(gitexecdir) is the standard in the other appropriate makefiles. Hence, replace the former with the latter. Signed-off-by: James Denholm --- contrib/subtree/Makefile | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/contrib/subtree/Makefile b/contrib/subtree/Makefile index f63334b..579bb51 100644 --- a/contrib/subtree/Makefile +++ b/contrib/subtree/Makefile @@ -3,7 +3,7 @@ prefix ?= /usr/local mandir ?= $(prefix)/share/man -libexecdir ?= $(prefix)/libexec/git-core +gitexecdir ?= $(prefix)/libexec/git-core man1dir ?= $(mandir)/man1 ../../GIT-VERSION-FILE: FORCE @@ -33,8 +33,8 @@ $(GIT_SUBTREE): $(GIT_SUBTREE_SH) doc: $(GIT_SUBTREE_DOC) $(GIT_SUBTREE_HTML) install: $(GIT_SUBTREE) - $(INSTALL) -d -m 755 $(DESTDIR)$(libexecdir) - $(INSTALL) -m 755 $(GIT_SUBTREE) $(DESTDIR)$(libexecdir) + $(INSTALL) -d -m 755 $(DESTDIR)$(gitexecdir) + $(INSTALL) -m 755 $(GIT_SUBTREE) $(DESTDIR)$(gitexecdir) install-doc: install-man -- 1.9.2 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 5/5] contrib/subtree/Makefile: clean rule cleanup
git:Documentation/Makefile and others establish "RM ?= rm -f" as a convention for rm calls in clean rules, hence follow this convention instead of simply forcing clean to use rm. subproj and mainline no longer need to be removed in clean, as they are no longer created in git:contrib/subtree by "make test". Hence, remove the rm call for those folders. Other makefiles don't remove "*~" files, remove the rm call to prevent unexpected behaviour in the future. Similarly, clean doesn't remove the installable file, so rectify this. Signed-off-by: James Denholm --- Admittedly, git:Makefile does not itself follow the "RM ?= rm -f" setup, instead using "RM = rm -f", but I felt that there were probably $ARCANE_REASONS for this. Also, Peff, you were right about the dirs. contrib/subtree/Makefile | 9 + 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/contrib/subtree/Makefile b/contrib/subtree/Makefile index f3834b5..4f96a24 100644 --- a/contrib/subtree/Makefile +++ b/contrib/subtree/Makefile @@ -11,8 +11,9 @@ man1dir ?= $(mandir)/man1 -include ../../GIT-VERSION-FILE -# this should be set to a 'standard' bsd-type install program -INSTALL ?= install +# These should be set to 'standard' bsd-type programs +INSTALL ?= install +RM ?= rm -f ASCIIDOC = asciidoc XMLTO= xmlto @@ -60,7 +61,7 @@ test: $(MAKE) -C t/ test clean: - rm -f *~ *.xml *.html *.1 - rm -rf subproj mainline + $(RM) $(GIT_SUBTREE) + $(RM) *.xml *.html *.1 .PHONY: FORCE -- 1.9.2 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/5] contrib/subtree/Makefile: scrap unused $(gitdir)
All references were removed in 7ff8463dba0d74fc07a766bed457ae7afcc902b5, but the assignment itself wasn't. Hence, drop gitdir assignment. Signed-off-by: James Denholm --- contrib/subtree/Makefile | 1 - 1 file changed, 1 deletion(-) diff --git a/contrib/subtree/Makefile b/contrib/subtree/Makefile index 4030a16..87797ed 100644 --- a/contrib/subtree/Makefile +++ b/contrib/subtree/Makefile @@ -4,7 +4,6 @@ prefix ?= /usr/local mandir ?= $(prefix)/share/man libexecdir ?= $(prefix)/libexec/git-core -gitdir ?= $(shell git --exec-path) man1dir ?= $(mandir)/man1 gitver ?= $(word 3,$(shell git --version)) -- 1.9.2 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 0/5] contrib/subtree/Makefile: Standardisation pass
contrib/subtree/Makefile is a shambles in regards to it's consistency with other makefiles, which makes subtree overly painful to include in build scripts. The main issues are that calls are made to git itself in the build process, and that a subtree-exclusive variable is used for specifying the exec path. Patches 1/5 through 3/5 resolve these. The "cleanup" fixes (4/5 and 5/5) are based on precedents set by other makefiles across the project. One problem is foreseen: 3/5 will necessitate that package maintainers who already have git-subtree included in their packages update their build-scripts. Signed-off-by: James Denholm Based-on-patch-by: Dan McGee James Denholm (5): contrib/subtree/Makefile: scrap unused $(gitdir) contrib/subtree/Makefile: Use GIT-VERSION-FILE contrib/subtree/Makefile: s/libexecdir/gitexecdir contrib/subtree/Makefile: Doc-gen rules cleanup contrib/subtree/Makefile: clean rule cleanup contrib/subtree/Makefile | 40 1 file changed, 24 insertions(+), 16 deletions(-) -- 1.9.2 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Downloading git source from https://code.google.com/p/git-core/downloads/
Hi, I've used the following link to download git source and corresponding pre-formatted man pages for several months: https://code.google.com/p/git-core/downloads/ However, the latest version available on the site is git-1.9.0 (last updated on 2014-02-14). Is the site still maintained/updated? -- Daniel Villeneuve -- To unsubscribe from this list: send the line "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: Pull is Mostly Evil
Philip Oakley wrote: > From: "Felipe Contreras" > > When doing something is better for the vast majority of people, that's > > what should be done by default, unless the results are catastrophic > > for > > the minority. > > > > Since doing something is not catastrophic to the minority, it follows > > that the default should be to do something. > > It's a simple as that. > > ... which makes it not quite as simple as that ;-) [evidence: the > ongoing dialog among all and sundry] The dialog is not simple becuase it's not easy to make `git pull` do the sensible thing. That doesn't mean `git pull` should do *nothing*, that doesn't follow from the argument above. -- 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: Pull is Mostly Evil
From: "Felipe Contreras" Sent: Saturday, May 03, 2014 12:23 AM Philip Oakley wrote: From: "Felipe Contreras" > So? No defaults can please absolutely everyone, the best anybody > can > do is try to please the majority of people, and merging > fast-forwards only does that. That assumes that doing something is better than doing nothing, When doing something is better for the vast majority of people, that's what should be done by default, unless the results are catastrophic for the minority. Since doing something is not catastrophic to the minority, it follows that the default should be to do something. That 'Since' and 'it follows' are where we have a diverging understanding about the solution approach... It's a simple as that. ... which makes it not quite as simple as that ;-) [evidence: the ongoing dialog among all and sundry] -- Felipe Contreras -- Philip -- To unsubscribe from this list: send the line "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: pull.prompt or other way to slow/disable 'git pull'
W. Trevor King wrote: > On Fri, May 02, 2014 at 05:20:11PM -0500, Felipe Contreras wrote: > > W. Trevor King wrote: > > > > > The 'git pull' (with 'none' mode) explainer just helps retrain folks > > > > > that are already using the current 'git pull' incorrectly. > > > > > > > > If you are going to train them to use a configuration, it should be: > > > > > > > > % git config --global pull.ff false > > > > > > I don't want all pulls to be --no-ff, only pulls from topic branches. > > > > Pulling some branch to a topic branch, or pulling a topic branch to > > another branch? > > The latter. Here's a more detailed list: > > 1. HEAD: an integration branch (master, maint, …) >target: @{upstream}, branch.*.pushremote, and other mirrors >my preferred integration mode: ff-only merge the target `git pull` would do that by default. > 2. HEAD: an integration branch >target: a *different* branch (e.g. maint or feature-x, but not > origin/master or jdoe/master, if HEAD is master) >my preferred integration mode: no-ff merge the target into HEAD. That makes sense, but other people would be OK with a ff merge. > 3. HEAD: a topic branch (e.g. feature-x) >target: a collaborating topic branch (jdoe/feature-x) >my preferred integration mode: ff-only merge the target I don't see why. It will alomst always be non-fast-fowrward, so you should already be prepared for a merge (or rebase). > 4. HEAD: a topic branch (e.g. feature-x) >target: a related topic branch (e.g. jdoe/feature-y) or integration > branch updates used by my feature-x >my preferred integration mode: rebase feature-x onto the target Nah. Most people would prefer a merge. And actually, quite many would want jdoe/feature-y to be rebased on top of feature-x. Either way it would be impossible for Git to figre out what you want to do. > Cases 1 and 2 can usually be distinguished by comparing the > checked-out branch with the branch portion of the remote-tracking > reference), but for folks developing in master, jdoe/master may be a > feature branch (case 2) not a mirror of the maintenance branch (case > 1). I'd say they can be distinguished by what the user typed. > Cases 1 and 3 are the same idea, with any feature branch running long > enough to get collaborators being indistinguishable from an > integration branch except that the latter will eventually be merged > (or dropped) and deleted. Ineed, so why would you want so drastically different behavior? > In the event of non-trivial merge conflicts in case 2, I sometimes > rebase the target onto HEAD and no-ff merge the resulting target'. On > the other hand, sometimes rebasing is not an option. For example, if > I want to merge the target into both master and maint, but master > contains a conflicting commit A: > > -o---o---A---o---B master >|\ >| o---o---C maint > \ > o---D target > > Rebasing would drag A into maint at F: > > -o---o---A---o---B---E master > \ \ / > \ o---D'--- target' > \ \ >o---o---C---F maint > > And I don't want both the pre- and post-rebase versions in my history > at G: > > -o---o---A---o---B---E---G master >|\ \ / / >| \ o---D'--- / target' >| \ / >| o---o---C---F maint > \ / > o---D target > > So I'd just deal with a complicated merge at E: > > -o---o---A---o---B---E---G master >|\ / / >| o---D / target > \ \ / > o---o---C---F-- maint > > Case 4 has similar caveats, since you don't want to rebase feature-x > on top of jdoe/feature-y if there are already other branches based on > the current feature-x that can't (or won't) be rebased. What I do in those cases is do both a merge and a rebase. If I resolved the conflicts correctly in the rebase the result of the merge should be exactly the same. It's not hard because rerere stores the conflict resolutions of the rebase and the merge becomes much simpler. After I'm certain the merge is correct, I remove the temporary rebased branch. Anyway I don't see how is this possibly relevant to the topic at hand. > > Either way, since I think these two are different modes: > > > > 1) git pull > > 2) git pull origin topic > > > > Maybe it would actually make sense to have a configuration specific to > > 2): pull.topicmode. > > I think it makes more sense to just use merge/rebase explicitly, Fine, if you want the user to be explicit, he can be explicit with `git pull --no-ff origin topic`. Problem solved. -- 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: Pull is Mostly Evil
On Fri, May 2, 2014 at 2:13 PM, Junio C Hamano wrote: [snip] > Your earlier long-hand, together with the two examples that pulls > into the same "maint" branch Brian gave us, may give us a better > starting points to think about a saner way. > > To me, the problem sounds like: > > Tutorials of Git often says "use 'git pull' to catch up your > branch with your upstream work and then 'git push' back" (and > worse yet, 'git push' that does not fast-forward suggests doing > so), but 'git pull' that creates a merge in a wrong direction is > not the right thing for many people. Yes, that's a good portion of the problem. > And proposed solutions range from "let's write 'pull' off as a > failed experiment" to "let's forbid any merge made by use of 'pull' > by default, because it is likely that merge may be in reverse". FWIW, at my company, we took another approach. We introduced a `git ffwd` command that fetches from all remotes, and fast-forwards all your local branches that are tracking a remote, and everyone on the team uses it all the time. It should be said this team also likes to use Git bare-metal, because they like knowing how things work out-of-the-box. But they all use the command because it's so convenient. I had started making a C version a while back, but never completed it. I could take a stab at doing so again, if there's interest. -John -- To unsubscribe from this list: send the line "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: Pull is Mostly Evil
Felipe Contreras writes: > David Kastrup wrote: >> Richard Hansen writes: >> >> > These three usage patterns are at odds; it's hard to change the >> > default behavior of 'git pull' to favor one usage case without >> > harming another. Perhaps this is why there's so much disagreement >> > about what 'git pull' should do. >> >> Should a screwdriver be turning clockwise or counterclockwise by >> default? There are valid arguments for either. > > If you don't have anything to contribute don't disturb the people that > actually care and are trying to improve Git. Thanks. No need to expand on the welcoming atmosphere here. My heinous plot to subvert the quality of Git has already been thwarted by making sure that its "meritocracy" continues relying only on input from those with an independent income. I'm just sticking around until my current contributions move into master so that I can summarize the resulting low-hanging fruit that the meritorious can then pick at great fanfare. The sooner my work moves from pu into master, the sooner y'all be rid of me. -- David Kastrup -- To unsubscribe from this list: send the line "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: Pull is Mostly Evil
Richard Hansen wrote: > I think the fundamental difference is in the relationship between the > local and the remote branch (which branch derives from the other). > The relationship between the branches determines what the user wants > from 'git pull'. > > In my experience 'git pull' is mostly (only?) used for the following > three tasks: I agree. > 1. update a local branch to incorporate the latest upstream changes > > In this case, the local branch (master) is a derivative of the > upstream branch (origin/master). The user wants all of the > commits in the remote branch to be in the local branch. And the > user would like the local changes, if any, to descend from the tip > of the remote branch. My current propsal of making `git pull` by default do --ff-only would solve this. In addition I think by default 'master' should be merged to 'origin/master', if say --merge is given. > For this case, 'git pull --ff-only' followed by 'git rebase -p' > works well, as does 'git pull --rebase=preserve' if the user is > comfortable rebasing without reviewing the incoming commits first. I suppose you mean a `git rebase -p` if the `git pull --ff-only` failed. This might be OK on most projects, but not all. What happens after a `git pull --ff-only` fails should be totally up to the user. > 2. update a published feature branch with the latest changes from its > parent branch > > In this case, the local branch (foo) is a derivative of the > upstream branch (origin/foo) which is itself a derivative of > another branch (origin/master). All commits in origin/master > should be in origin/foo, and ideally all commits unique to > origin/foo would descend from the tip of origin/master. I don't understand why are you tainting the example with 'origin/foo', 'foo' and 'origin/master' are enough for this example. In fact, the mention of 'origin/master' made it wrong: after the pull not all the commits of origin/master would be in origin/foo, you need a push for that. We have enough in our plate to taint this with yet another branch and push. For this case `git pull origin master` already work correctly for most projects. We probably shouldn't change that. > 3. integrate a more-or-less complete feature/fix back into the line > of development it forked off of > > In this case the local branch is a primary line of development and > the remote branch contains the derivative work. Think Linus > pulling in contributions. Different situations will call for > different ways to handle this case, but most will probably want > some or all of: > > * rebase the remote commits onto local HEAD No. Most people will merge the remote branch as it is. There's no reason to rebase, specially if you are creating a merge commit. > * merge into local HEAD so that the first parent (if a real merge >and not a ff) is the previous version of the main line of >development and the second parent is the derivative work > * merge --no-ff so that: > - the merge can serve as a cover letter (who reviewed it, > which bug reports were fixed, where the changes came from, > etc.) > - the commits that compose the new topic are grouped together > - the first-parent path represents a series of completed tasks It is very rare that an integrator is even able to do a fast-forward merge anyway. So being explicit about --no-ff might better, but it would hardly make a difference. Either way, a good integrator would configure pull.ff = false. I'd say `git pull origin master` already works fine for this case. -- 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: Pull is Mostly Evil
David Kastrup wrote: > Richard Hansen writes: > > > These three usage patterns are at odds; it's hard to change the > > default behavior of 'git pull' to favor one usage case without > > harming another. Perhaps this is why there's so much disagreement > > about what 'git pull' should do. > > Should a screwdriver be turning clockwise or counterclockwise by > default? There are valid arguments for either. If you don't have anything to contribute don't disturb the people that actually care and are trying to improve Git. Thanks. -- 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: Watchman support for git
On Sat, May 3, 2014 at 11:39 AM, David Turner wrote: >> Index v4 and split index (and the following read-cache daemon, >> hopefully) > > Looking at some of the archives for read-cache daemon, it seems to be > somewhat similar to watchman, right? But I only saw inotify code; what > about Mac OS? Or am I misunderstanding what it is? It's mentioned in [1], the second paragraph, mostly to hide index I/O read cost and the SHA-1 hashing cost in the background. In theory it should work on all platforms that support multiple processes and efficient IPC. It can help load watchman file cache faster too. >> The last line could be a competition between watchman and my coming >> "untracked cache" series. I expect to cut the number in that line at >> least in half without external dependency. > > I hadn't seen the "untracked cached" work (I actually finished these > patches a month or so ago but have been waiting for some internal > reviews before sending them out). Looks interesting. It seems we use a > similar strategy for handling ignores. Yep, mostly the same at the core, except that I exploit directory mtime while you use inotify. Each approach has its own pros and cons, I think. Both should face the same traps in caching (e.g. if you "git rm --cached" a file, that file could be come either untracked, or ignored). >> Patch 2/3 did not seem to make it to the list by the way.. > > Thanks for your comments. I just tried again to send patch 2/3. I do > actually see the CC of it in my @twitter.com mailbox, but I don't see it > in the archives on the web. Do you know if there is a reason the > mailing list would reject it? Probably its size, 131K, which is also an indicator to split it (and the third patch) into smaller patches if you want to merge this feature in master eventually. > At any rate, the contents may be found > at > https://github.com/dturner-tw/git/commit/cf587d54fc72d82a23267348afa2c4b60f14ce51.diff Good enough for me :) > >> initial >> reaction is storing the list of all paths seems too much, but I'll >> need to play with it a bit to understand it. > > I wonder if it would make sense to use the untracked cache as the > storage strategy, but use watchman as the update strategy. I'm afraid not. If a directory mtime is changed, which means files/dirs have been added or deleted, the untracked code would fall back to the opendir/readdir/is_excluded dance again on that directory. If we naively do the same using watchman, we lose its advantage that it knows exactly what files/dirs are added/removed. That kind of knowledge can help speed up the dance, which is not stored anywhere in the untracked cache. We could extend the "read-cache daemon" mentioned above though, to hide all the hard work in the background and present a good view to git: when a file/dir is added, read-cache daemon classifies the new files/dirs as tracked/untracked/ignore and update its untracked cache in memory. When "git status" asks about the index and untracked cache, it will receive the _updated_ cache (not the on disk version any more) with latest dir mtime so git can verify the cache is perfect and skip opendir/ All git does is to write the index down in the end to make the updated data permanent. It sounds interesting. But I'm not so sure if it's worth the complexity. [1] http://article.gmane.org/gmane.comp.version-control.git/247268 -- 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
git subtree pull same remote different prefix problem
Hello, I am currently using git subtree in a project : https://github.com/asmodehn/wkcmake This is basically a set of cmake scripts that help writing convention based (folder hierarchy, etc.) cmake builds. My setup is that "master" has the set of scripts needed. and many other branches ("test" and "tests/*" most notably) have tests projects for these scripts. Travis run build of these to check if the build setup works as expected. The "test" branch has a hierarchy of dependent projects (ProjectA, ProjectB, ProjectC) , all using the wkcmake scripts to build. So I am using git-subtree to pull and push "ProjectA/CMake" ProjectB/Custom CMake" "ProjectC/CMake". I had enough trouble with push to stop using it( merge not happening as I thought it would ). Not sure if I am to blame or not for these... Anyway now, after a commit into the test branch, I instead split the CMake subtree to a new branch, and merge in manually back to master. That seems to work okay so far. However I realized that when I do : git subtree pull -P ProjectA/CMake g...@github.com:asmodehn/ wkcmake.git origin master git subtree pull -P "ProjectB/Custom CMake" g...@github.com:asmodehn/wkcmake.git origin master git subtree pull -P ProjectC/CMake g...@github.com:asmodehn/wkcmake.git origin master I get only one merge commit into the "test" branch. All the other subtree pull returns "Already up to date" even though the content of the working directory in the prefix passed is NOT uptodate. The possible behavior that I could understand would be : - One subtree pull update all prefixes, with only one merge commit, and the prefix doesnt really matter anymore ( could be problematic... ) - Each subtree pull update the appropriate prefix with one merge commit each. So I am curious : I this a scenario that subtree doesnot support ? Am I using it wrong ? Is it a current limitation/bug of subtree that is on the roadmap to be addressed sometime in the future ? Thanks a lot for the help. -- AlexV -- To unsubscribe from this list: send the line "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: Pull is Mostly Evil
Richard Hansen writes: > These three usage patterns are at odds; it's hard to change the > default behavior of 'git pull' to favor one usage case without harming > another. Perhaps this is why there's so much disagreement about what > 'git pull' should do. Should a screwdriver be turning clockwise or counterclockwise by default? There are valid arguments for either. -- David Kastrup -- To unsubscribe from this list: send the line "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: Pull is Mostly Evil
On 2014-05-02 14:13, Junio C Hamano wrote: > Stepping back even further, and thinking what is different between > these two pulls, we notice that the first one is pulling from the > place we push back to. I think the fundamental difference is in the relationship between the local and the remote branch (which branch derives from the other). The relationship between the branches determines what the user wants from 'git pull'. In my experience 'git pull' is mostly (only?) used for the following three tasks: 1. update a local branch to incorporate the latest upstream changes In this case, the local branch (master) is a derivative of the upstream branch (origin/master). The user wants all of the commits in the remote branch to be in the local branch. And the user would like the local changes, if any, to descend from the tip of the remote branch. For this case, 'git pull --ff-only' followed by 'git rebase -p' works well, as does 'git pull --rebase=preserve' if the user is comfortable rebasing without reviewing the incoming commits first. A plain 'git pull' or 'git pull --ff' is suboptimal due to the awkward backwards-parents merge commit. 2. update a published feature branch with the latest changes from its parent branch In this case, the local branch (foo) is a derivative of the upstream branch (origin/foo) which is itself a derivative of another branch (origin/master). All commits in origin/master should be in origin/foo, and ideally all commits unique to origin/foo would descend from the tip of origin/master. The relationship between origin/foo and origin/master is similar to the relationship between master and origin/master in case #1 above, but rebase is frowned upon because the feature branch has been shared with other developers (and the shared repository might reject non-ff updates). This case is sort-of like case #1 above (updating) and sort-of like case #3 below (integrating). For this case, after the local branch foo is updated (case #1 above), 'git pull --ff origin master' or 'git fetch --all && git merge --ff origin/master' work well to update origin/foo. 3. integrate a more-or-less complete feature/fix back into the line of development it forked off of In this case the local branch is a primary line of development and the remote branch contains the derivative work. Think Linus pulling in contributions. Different situations will call for different ways to handle this case, but most will probably want some or all of: * rebase the remote commits onto local HEAD * merge into local HEAD so that the first parent (if a real merge and not a ff) is the previous version of the main line of development and the second parent is the derivative work * merge --no-ff so that: - the merge can serve as a cover letter (who reviewed it, which bug reports were fixed, where the changes came from, etc.) - the commits that compose the new topic are grouped together - the first-parent path represents a series of completed tasks (I prefer to do all three, although I may skip the rebase if the commits came from another public repository so as to not annoy users of that downstream repository.) For this case, 'git pull --no-ff' is better than 'git pull --ff' (for the reasons listed above), but perhaps something more elaborate would be ideal (e.g., rebase there onto here, then merge --no-ff). These three usage patterns are at odds; it's hard to change the default behavior of 'git pull' to favor one usage case without harming another. Perhaps this is why there's so much disagreement about what 'git pull' should do. I see a few ways to improve the situation: 1. Add one or two new commands to split up how the three cases are handled. For example: * Add a new 'git update' command that is friendly for case #1. Update tutorials to recommend 'git update' instead of 'git pull'. It would behave like 'git pull --ff-only' by default. It could behave like 'git pull --rebase[=preserve]' instead, but this has a few downsides: - It doesn't give the user an opportunity to review the incoming commits before rebasing (e.g., to see what sort of conflicts to expect). - It subjects new users to that scary rebase thing before they are prepared to handle it. - The branch to be updated must be checked out. If 'git update' used --ff-only, then 'git update --all' could fast-forward all local branches to their configured upstreams when possible. (How cool would that be?) * Leave 'git pull' and 'git pull $remote [$refspec]' alone -- the current defaults are acceptable (though maybe not ideal) for cases #2 and #3. Another exam
Re: [PATCH 08/12] MINGW: fix main() signature in http-fetch.c and remote-curl.c
On Wed, Apr 30, 2014 at 10:35:56AM +0200, Karsten Blees wrote: > Am 29.04.2014 11:12, schrieb Marat Radchenko: > > On MinGW, compat/mingw.h defines a 'mingw_main' wrapper function. > > Fix `warning: passing argument 2 of 'mingw_main' from incompatible > > pointer type` in http-fetch.c and remote-curl.c by dropping 'const'. > > > > Would you mind cross checking your changes with the msysgit fork? > Fixing the same thing in a slightly different manner will just cause > unnecessary conflicts (i.e. unnecessary work for the Git for Windows > maintainers, as well as for you to come up with an alternate solution > for something that's long been fixed). > > See https://github.com/msysgit/git/commit/9206e7fd (squashed from > https://github.com/msysgit/git/commit/0115ef83 and > https://github.com/msysgit/git/commit/6949537a). 6949537a is just an unnecessary hack and would have to be rewritten no matter who would try to submit it upstream. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: BUG or FEATURE? Use of '/' in branch names
On vr, 2014-05-02 at 15:16 -0700, Jonathan Nieder wrote: > > $ git checkout -b hotfix/b2 > > error: unable to resolve reference refs/heads/hotfix/b2: Not a > directory > > fatal: Failed to lock ref for update: Not a directory > > $ > > That's an ugly message. I think we can do better. (hint hint) 2.0.0-rc2 has a better message already: $ git checkout -b hotfix/b2 error: 'refs/heads/hotfix' exists; cannot create 'refs/heads/hotfix/b2' fatal: Failed to lock ref for update: Not a directory -- Dennis Kaarsemaker www.kaarsemaker.net -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 10/12] MINGW: compat/poll/poll.c: undef NOGDI
On Wed, Apr 30, 2014 at 01:41:25PM +0200, Stepan Kasal wrote: > Hello, > > On Tue, Apr 29, 2014 at 01:12:04PM +0400, Marat Radchenko wrote: > > On MinGW-W64, MsgWaitForMultipleObjects is guarded with #ifndef NOGDI. > > > > Removal -DNOGDI=1 from config.mak.uname has an undesirable effect of > > bringing in wingdi.h with weird #define ERROR 0 that conflicts with > > internal Git enums. So, just #undef NOGDI in compat/poll/poll.c. > > compat/poll/poll.c comes from Gnulib, so it would be better to submit > the patch there and then backport so that the divergence of the two > versions does not get worse. That's why v1 of this patch [1] didn't touch poll.c at all. I don't think it's gnulib problem that combination of two third-parties (git and mingw-w64) set up such conditions where poll.c fails to compile. If one wants to dig deeper, I'd say the problem is in MinGW-W64 headers because their behavior of hiding MsgWaitForMultipleObjects doesn't match behavior of MSVC headers. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html