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: [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 p...@peff.net
---
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)
+{
+char *prec_str = NULL;
+if (precomposed_unicode != 1)
+return NULL;
+
Re: Pull is Mostly Evil
Felipe Contreras felipe.contre...@gmail.com 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
[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 felipe.contre...@gmail.com
---
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 file...\ 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 file...\ to
unstage)), s-reference);
else
status_printf_ln(s, c, _( (use \git rm --cached file...\
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 -- file...\ 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 file...\ 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-untracked_in_ms) {
- status_printf_ln(s,
[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
[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 openssl/x509v3.h
#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
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
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: Pull is Mostly Evil
James Denholm nod.h...@gmail.com writes: 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. It's called an ad hominem attack, and it's a very common and very effective rhetorical device. Cf URL:http://thread.gmane.org/gmane.comp.version-control.git/246598/focus=247002 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. If he considered others capable of independent thought, would he call out their imperviousness to rhetorics as a deficiency? -- 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-03 23:08, Felipe Contreras wrote:
Richard Hansen wrote:
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`.
OK. Let me summarize to make sure I understand your full proposal:
1. if plain 'git pull', default to --ff-only
2. if 'git pull --merge', default to --ff. If the local branch can't
be fast-forwarded to the upstream branch, then create a merge
commit where the local branch is the *second* parent, not the first
3. if 'git pull $remote [$refspec]', default to --merge --ff. If the
local branch can't be fast-forwarded to the remote branch, then
create a merge commit where the remote branch is the second parent
(the current behavior)
Is that accurate?
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.
It's not the only proposal -- I proposed a few alternatives in my
earlier email (though not in the form of code), and others have too. In
particular:
* create a new 'git integrate' command/alias that behaves like 'git
pull --no-ff'
* change 'git pull' and 'git pull $remote [$refspec]' to do --ff-only
by default
Another option that I just thought of: Instead of your proposed
pull.mode and branch.name.pullmode, add the following two sets of configs:
* pull.updateMode, branch.name.pullUpdateMode:
The default mode to use when running 'git pull' without naming a
remote repository or when the named remote branch is @{u}. Valid
options: ff-only (default), merge-ff, merge-ff-there, merge-no-ff,
merge-no-ff-there, rebase, rebase-here, rebase-here-then-merge-no-ff
* pull.integrateMode, branch.name.pullIntegrateMode:
The default mode to use when running 'git pull $remote [$refspec]'
when '$remote [$refspec]' is not @{u}. Valid options are the same
as those for pull.updateMode. Default is merge-ff.
This gives the default split behavior as you propose, but the user can
reconfigure to suit personal preference (and we can easily change the
default for one or the other if there's too much outcry).
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.
Yes, it might not be a problem, but I'm still nervous. I'd need more
input (e.g., user survey, broad mailing list consensus, long beta test
period, decree by a benevolent dictator) before I'd be comfortable with it.
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.
Probably. My point is that the behavior should be configurable, and I'd
like that particular behavior to be one of the options (but not the
default -- that wouldn't be appropriate).
* It makes commits easier to review.
The review in the vast majority of cases happens *before* the
integration.
True, although even when review happens before integration there is
value in making code archeology easier.
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.
Good point, although if I was the integrator and there was a
particularly hairy conflict I'd still rebase but
Re: A failing attempt to use Git in a centralized environment
On Wed, Apr 30, 2014 at 1:15 PM, Geert Bosch bos...@mac.com wrote: On Apr 28, 2014, at 02:29, Marat Radchenko ma...@slonopotamus.org wrote: In short: 1. Hack, hack, hack 2. Commit 3. Push, woops, reject (non-ff) 4. Pull 5. Push Just do pull --rebase? This is essentially the same as what SVN used to do in your setup. That's not necessarily a good solution either. For teams that don't use rebase, it can leave them with their newly committed stuff now rebased on the work from upstream--duplicating commits without understanding why and where they came from, especially if other branches were built on top of that one. I agree in concept, but in practice it can be quite confusing. :-( -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: [RFC PATCH 0/9] Use a structure for object IDs.
Am 04.05.2014 08:35, schrieb Michael Haggerty: On 05/03/2014 10:12 PM, brian m. carlson wrote: 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? Only during the transition period. When all functions that currently take unsigned char[20] are converted to struct object_id *, this additional dereferences go away again. -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/9] Define a structure for object IDs.
Am 04.05.2014 08:07, schrieb Michael Haggerty: On 05/03/2014 10:12 PM, brian m. carlson wrote: 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. I think that a compiler that has different size and alignment requirements for the proposed struct object_id and an unsigned char[20] would, strictly speaking, not be a C compiler. -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] remote-hg: add more tests
On Sat, May 3, 2014 at 10:16 PM, Felipe Contreras
felipe.contre...@gmail.com wrote:
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 felipe.contre...@gmail.com
---
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
Broken -chain.
+ 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 commit...@example.com 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
--
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
James Denholm wrote: 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. You mean this thread[1] in which I sent 14 mails directly to you? Yeah, I din't engage in that discussion at all! And the point I was making is that if I manage to show the community was wrong in some thing (as you claimed), that community wouldn't include you. 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. I can show you evidence of how that's a blatant lie. Just two days ago I changed my mind because somebody provided a good argument. But I'm not going to bother any more with you, you are just spreading lies and tainting the discussion. [1] http://thread.gmane.org/gmane.comp.version-control.git/247188/focus=247584 -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/9] Define a structure for object IDs.
Johannes Sixt j...@kdbg.org writes: Am 04.05.2014 08:07, schrieb Michael Haggerty: On 05/03/2014 10:12 PM, brian m. carlson wrote: 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. I think that a compiler that has different size and alignment requirements for the proposed struct object_id and an unsigned char[20] would, strictly speaking, not be a C compiler. Huh? How so? There is no warranty as far as I know that a structure with only a single member has the same size and alignment requirements as the single member would have. There is also no guarantee as far as I know that anything but element dereference is a valid means of converting access to a struct to access to a sole element. -- 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: [msysGit] Re: #178 parsing of pretty=format:%an %ad causes fatal: bad revision '%ad'
Hi, Interesting discussion. However, the example below of three-spaces between %an and %ad in the example below resulted in the formatting of the output with the three spaces, but no dq's. Original #178 content G:\ws_test_env\GIT_TESTBED_TMP\fest-swing-1.xgit log --all --pretty=format:%an%ad -- pom.xml Mon Nov 23 03:09:17 2009 + Mon Nov 23 02:42:24 2009 + This added to my confusion as by right dq within dq should be formatted. (Yea right, these days its needs to be escaped. But haven't tried that.) In summary so far, it would appear that the --pretty. needs to be contained in double-quotes as a whole. This was the solution I applied to my problem. In the discussions I've seen more information requested as to the arguments provided to the execution class. I solved this issue as I made it work by experiment. I format the argument as a whole and don't have the space. IE. pretty=format:name:%an%nauthor:%ad%n. Regards -Original Message- From: Erik Faye-Lund Sent: Friday, May 02, 2014 2:23 PM To: Jonathan Nieder Cc: Dave Bradley ; GIT Mailing-list ; msysGit Subject: Re: [msysGit] Re: #178 parsing of pretty=format:%an %ad causes fatal: bad revision '%ad' On Fri, May 2, 2014 at 7:23 PM, Jonathan Nieder jrnie...@gmail.com wrote: (resending with the correct address for the Git for Windows developers. Sorry for the noise.) Hi Dave, Dave Bradley wrote: G:\ws_test_env\GIT_TESTBED_TMP\fest-swing-1.xgit log --all --pretty=format:%an %ad -- pom.xml Mon Nov 23 03:09:17 2009 + Mon Nov 23 02:42:24 2009 + G:\ws_test_env\GIT_TESTBED_TMP\fest-swing-1.xgit log --all --pretty=format:%an %ad -- pom.xml fatal: bad revision '%ad' On Linux, this example gets passed to git as six arguments: log --all --pretty=format:%an %ad -- pom.xml As does it on Windows. -- 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:
On 2014-05-03 23:08, Felipe Contreras wrote:
Richard Hansen wrote:
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`.
OK. Let me summarize to make sure I understand your full proposal:
1. if plain 'git pull', default to --ff-only
2. if 'git pull --merge', default to --ff. If the local branch can't
be fast-forwarded to the upstream branch, then create a merge
commit where the local branch is the *second* parent, not the first
3. if 'git pull $remote [$refspec]', default to --merge --ff. If the
local branch can't be fast-forwarded to the remote branch, then
create a merge commit where the remote branch is the second parent
(the current behavior)
Is that accurate?
Yes, that is accurate. Note that 3. is the current behavior.
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.
It's not the only proposal -- I proposed a few alternatives in my
earlier email (though not in the form of code), and others have too. In
particular:
* create a new 'git integrate' command/alias that behaves like 'git
pull --no-ff'
Yeah but that's for a different issue altogheter. I doesn't solve the
problems in 1. nor 2. nor 3.
* change 'git pull' and 'git pull $remote [$refspec]' to do --ff-only
by default
Another option that I just thought of: Instead of your proposed
pull.mode and branch.name.pullmode, add the following two sets of configs:
* pull.updateMode, branch.name.pullUpdateMode:
The default mode to use when running 'git pull' without naming a
remote repository or when the named remote branch is @{u}. Valid
options: ff-only (default), merge-ff, merge-ff-there, merge-no-ff,
merge-no-ff-there, rebase, rebase-here, rebase-here-then-merge-no-ff
Those are way too many options to be able to sensibly explain them.
* pull.integrateMode, branch.name.pullIntegrateMode:
The default mode to use when running 'git pull $remote [$refspec]'
when '$remote [$refspec]' is not @{u}. Valid options are the same
as those for pull.updateMode. Default is merge-ff.
This gives the default split behavior as you propose, but the user can
reconfigure to suit personal preference (and we can easily change the
default for one or the other if there's too much outcry).
If we reduce the number of options to begin with (more can be added
later), then it might make sense to have these two options.
However, that doesn't change the proposal you described above (1. 2.
3.).
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.
Yes, it might not be a problem, but I'm still nervous. I'd need more
input (e.g., user survey, broad mailing list consensus, long beta test
period, decree by a benevolent dictator) before I'd be comfortable with it.
The user surveys are not happening any more. The results were ignored by
the developers anyway.
Mailing list consensus might be possible, but that wouldn't tell us
much.
There's something we can do, and let me clarify my proposal. What you
described above is what I think should happen eventually, however, we
can start by doing something like what my patch series is doing; issue a
warning that the merge is not fast-forward and things might change in
the future.
If people find this behavior confusing they will complain in the mailing
list. Although I suspect it would be for other reasons, not the 'git
pull'/'git pull $there' division. Either way we would see in the
discussion.
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
Re: Pull is Mostly Evil
On 4 May 2014 19:51:09 GMT+10:00, Felipe Contreras felipe.contre...@gmail.com wrote: James Denholm wrote: 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. You mean this thread[1] in which I sent 14 mails directly to you? Yeah, I din't engage in that discussion at all! Yeah, you didn't. Instead you danced, but I guess it's really all said and done now so eh, have your point. 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. I can show you evidence of how that's a blatant lie. Just two days ago I changed my mind because somebody provided a good argument. And I can show you evidence of you being indiscourseable on the topic of your pet proposals, but I won't, because you're indiscourseable on the meta similarly. But I'm not going to bother any more with you, you are just spreading lies and tainting the discussion. Well, maybe we'll see what other folks think. 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: [PATCH 1/9] Define a structure for object IDs.
Johannes Sixt j...@kdbg.org writes: I think that a compiler that has different size and alignment requirements for the proposed struct object_id and an unsigned char[20] would, strictly speaking, not be a C compiler. Unlike arrays, a struct can have arbitrary internal padding. It is perfectly compliant (and even reasonable) to make struct object_id require 8 byte alignment, adding 4 bytes of padding at the end. Andreas. -- Andreas Schwab, sch...@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 And now for something completely different. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Pull is Mostly Evil
James Denholm nod.h...@gmail.com writes: On 4 May 2014 19:51:09 GMT+10:00, Felipe Contreras felipe.contre...@gmail.com wrote: But I'm not going to bother any more with you, you are just spreading lies and tainting the discussion. Well, maybe we'll see what other folks think. According to whose summary? URL:https://www.youtube.com/watch?v=2eMkth8FWno -- 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
On 2014-04-30 16.57, Torsten Bögershausen wrote: There is something wrong with the patch, (test suite hangs or TC fail), so I need to com back later. Sorry for the noise. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/9] Define a structure for object IDs.
On Sun, May 4, 2014 at 1:07 PM, Michael Haggerty mhag...@alum.mit.edu wrote: 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. It's not just about wasted padding. Some structs, like ondisk_cache_entry, reflect on-disk format. Padding means breakage. But I don't think this will be a big issue because we can detect if padding happens, abort to force the user to complain, and deal with it on case-by-case basis. -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/9] Define a structure for object IDs.
On Sun, May 04, 2014 at 08:07:26AM +0200, Michael Haggerty wrote: 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. I plan to write the code portably. My statement was basically that I don't expect this to result in more memory being used. I don't even plan to write the code assuming that offsetof(struct object_id, oid) is 0. I have owned SPARC systems, and I have experienced plenty of aggravation with code that makes unportable alignment assumptions. I don't want to make that mistake myself. -- 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 1/9] Define a structure for object IDs.
brian m. carlson sand...@crustytoothpaste.net writes: I don't even plan to write the code assuming that offsetof(struct object_id, oid) is 0. This is guaranteed by the C standard, though. Andreas. -- Andreas Schwab, sch...@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 And now for something completely different. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/9] Define a structure for object IDs.
Andreas Schwab sch...@linux-m68k.org writes: brian m. carlson sand...@crustytoothpaste.net writes: I don't even plan to write the code assuming that offsetof(struct object_id, oid) is 0. This is guaranteed by the C standard, though. Any reference? -- 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
[PATCH] Bump core.deltaBaseCacheLimit to 96m
The default of 16m causes serious thrashing for large delta chains combined with large files. Here are some benchmarks (pu variant of git blame): time git blame -C src/xdisp.c /dev/null for a repository of Emacs repacked with git gc --aggressive (v1.9, resulting in a window size of 250) located on an SSD drive. The file in question has about 3 lines, 1Mb of size, and a history with about 2500 commits. 16m (previous default): real3m33.936s user2m15.396s sys 1m17.352s 32m: real3m1.319s user2m8.660s sys 0m51.904s 64m: real2m20.636s user1m55.780s sys 0m23.964s 96m: real2m5.668s user1m50.784s sys 0m14.288s 128m: real2m4.337s user1m50.764s sys 0m12.832s 192m: real2m3.567s user1m49.508s sys 0m13.312s Signed-off-by: David Kastrup d...@gnu.org --- Documentation/config.txt | 2 +- environment.c| 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index 1932e9b..21a3c86 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -489,7 +489,7 @@ core.deltaBaseCacheLimit:: to avoid unpacking and decompressing frequently used base objects multiple times. + -Default is 16 MiB on all platforms. This should be reasonable +Default is 96 MiB on all platforms. This should be reasonable for all users/operating systems, except on the largest projects. You probably do not need to adjust this value. + diff --git a/environment.c b/environment.c index 5c4815d..37354c8 100644 --- a/environment.c +++ b/environment.c @@ -37,7 +37,7 @@ int core_compression_seen; int fsync_object_files; size_t packed_git_window_size = DEFAULT_PACKED_GIT_WINDOW_SIZE; size_t packed_git_limit = DEFAULT_PACKED_GIT_LIMIT; -size_t delta_base_cache_limit = 16 * 1024 * 1024; +size_t delta_base_cache_limit = 96 * 1024 * 1024; unsigned long big_file_threshold = 512 * 1024 * 1024; const char *pager_program; int pager_use_color = 1; -- 1.9.1 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/9] Define a structure for object IDs.
David Kastrup d...@gnu.org writes: Andreas Schwab sch...@linux-m68k.org writes: brian m. carlson sand...@crustytoothpaste.net writes: I don't even plan to write the code assuming that offsetof(struct object_id, oid) is 0. This is guaranteed by the C standard, though. Any reference? §6.7.2.1#15 Andreas. -- Andreas Schwab, sch...@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 And now for something completely different. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/9] Define a structure for object IDs.
Andreas Schwab sch...@linux-m68k.org writes: David Kastrup d...@gnu.org writes: Andreas Schwab sch...@linux-m68k.org writes: brian m. carlson sand...@crustytoothpaste.net writes: I don't even plan to write the code assuming that offsetof(struct object_id, oid) is 0. This is guaranteed by the C standard, though. Any reference? §6.7.2.1#15 More like #13. I am pretty sure, however, that this has not always been the case. -- 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: Downloading git source from https://code.google.com/p/git-core/downloads/
Daniel Villeneuve dvillene...@kronos.com writes: 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? No: google is closing downloads on google code, we had to move somewhere else. It's still here: https://www.kernel.org/pub/software/scm/git/ and also there: https://github.com/git/git/releases -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 0/9] Use a structure for object IDs.
On Sun, May 04, 2014 at 08:35:00AM +0200, Michael Haggerty wrote: On 05/03/2014 10:12 PM, brian m. carlson wrote: 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]. As Johannes pointed out, only during the transition period. 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. You have a point. I'll make the change. 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. It is an eventual goal, but without this series, it's not even worth discussing since it's too hard to implement. Even if that doesn't happen, my hope is that we'll at least improve the safety of the code and hopefully avoid a bug or two out of it. (Maybe renaming local variables sha1 - oid might be a handy way of making clear which code has been converted to the new style.) This is a good idea as well. I'll walk through the patches and fix that. Just to be clear, the above are just some random thoughts for your consideration, but feel free to disregard them. I appreciate the well-thought-out response. -- 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 1/9] Define a structure for object IDs.
David Kastrup d...@gnu.org writes: Andreas Schwab sch...@linux-m68k.org writes: David Kastrup d...@gnu.org writes: Andreas Schwab sch...@linux-m68k.org writes: brian m. carlson sand...@crustytoothpaste.net writes: I don't even plan to write the code assuming that offsetof(struct object_id, oid) is 0. This is guaranteed by the C standard, though. Any reference? §6.7.2.1#15 More like #13. I don't know what you mean, but this is a C11 reference. I am pretty sure, however, that this has not always been the case. You are wrong. Andreas. -- Andreas Schwab, sch...@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 And now for something completely different. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: pull.prompt or other way to slow/disable 'git pull'
On Sat, May 03, 2014 at 04:50:52AM -0500, Felipe Contreras wrote: Either way it would be impossible for Git to figre out what you want to do. That's my point. The details of my particular workflow are unimportant. Anyway I don't see how is this possibly relevant to the topic at hand. I'm trying to motivate a way to slow/disable 'git pull', which I see as orthogonal to your push to change the default configuration. I thought describing my workflow in more detail would help clarify why… 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. … this global pull.ff config was not a solution. 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. That's certainly explicit, but some folks are in the habit of just running 'git pull' (regardless of which branch they happen to be on) without thinking “Where am I, what am I integrating, and how should I integrate it?”. As I claimed earlier: On Thu, May 01, 2014 at 06:10:04PM -0700, W. Trevor King wrote [1]: Folks who are setting any ff options don't need any of these training wheels. My proposed --prompt behavior is for folks who think “I often run this command without thinking it through all the way. I'm also not used to reading Git's output and using 'reset --hard' with the reflog to reverse changes. Instead of trusting me to only say what I mean or leaving me to recover from mistakes, please tell me what's about to change and let me opt out if I've changed my mind.” In the messages following that, you seemed to agree that such folks existed [2], and suggested I use pull.mode=fetch-only [3] or pull.ff=false [4] or pull.topicargs='--merge --no-ff' [5]. Now we agree (I think? Based on your “it would be impossible for Git…” quoted above) that you can have a sane workflow for which no pull-strategy default will always do the right thing. We just disagree (I think) on what to do about it. I'm suggesting pull.prompt, pull.mode=none, or some other way to slow/disable 'git pull' while folks retrain themselves. You're suggesting (I think? Based on your 'git pull --no-ff origin topic' quoted above) that folks just skip right to remembering which ff options to use in which situations. Do you feel folks won't need a way to slow/disable 'git pull' while they build the ff options and their project's recommended workflow into their own practice? Or do you agree that they will need some kind of helper for the transition, and just feel that git.prompt is the wrong helper? Cheers, Trevor [1]: http://article.gmane.org/gmane.comp.version-control.git/247917 [2]: http://article.gmane.org/gmane.comp.version-control.git/247919 On Thu, May 01, 2014 at 08:14:29PM -0500, Felipe Contreras wrote: W. Trevor King wrote: Folks who are setting any ff options don't need any of these training wheels. Indeed. [3]: http://article.gmane.org/gmane.comp.version-control.git/247957 On Fri, May 02, 2014 at 02:13:25PM -0500, Felipe Contreras wrote: W. Trevor King wrote: On Fri, May 02, 2014 at 01:55:36PM -0500, Felipe Contreras wrote: W. Trevor King wrote: But once such folks are identified, you just have to convince them (once) to set the pull.prompt config. That's a lot easier than convincing them (for every pull) to set the appropriate ff flag. It wouldn't matter if by the default non-fast-forward merges are rejected. It would matter if you didn't want them making non-fast-forward merges (e.g. for explicitly-merged topic branches). It would matter almost exactly zero. And just as they can do pull.promot = true, they can do pull.mode = fetch-only. [4]: http://article.gmane.org/gmane.comp.version-control.git/247986 On Fri, May 02, 2014 at 04:18:57PM -0500, Felipe Contreras wrote: W. Trevor King wrote: Saying “that's unlikely to happen” doesn't solve the problem that some newcomers have trouble matching their project's desired workflow. % git config --global pull.ff false Done. [5]: http://article.gmane.org/gmane.comp.version-control.git/247998 On Fri, May 02, 2014 at 05:20:11PM -0500, Felipe Contreras
Re: [PATCH 10/12] MINGW: compat/poll/poll.c: undef NOGDI
Hello Marat, On Sat, May 03, 2014 at 11:00:51AM +0400, Marat Radchenko wrote: On Wed, Apr 30, 2014 at 01:41:25PM +0200, Stepan Kasal wrote: 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 [..] compat/poll/poll.c comes from Gnulib, so it would be better to submit [..] That's why v1 of this patch [1] didn't touch poll.c at all. ouch! It looks like you everyone sending you elsewhere. I apologize for being part of that. (I was not aware about the previous version.) 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. Well, yes and no: gnulib is mostly a collection of compatibility reimplementaions of functions that should be available on an ideal system. 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. Thank you very much for this analysis. It enables us to redirect you the third time: to report this as a bug in MinGW-W64 ! ;-) Seriously, it looks you found the best description of the problem, and it would be nice if you could modify your patch so that it is really a work around: it would be in effect only for MinGW-W64, and the comment would explain that this is a hack to work around the bug. If you manage to change the defs for poll.c without changing its content, no one could tell you to report to gnulib first. OTOH, if MsgWaitForMultipleObjects is present ustream (in gnulib's poll.c, sorry that I cannot check right now), it still might be better to submit the work-around there first. Thanks for your work, Stepan Kasal -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] Silence a bunch of format-zero-length warnings
On Sun, May 04, 2014 at 01:12:55AM -0500, Felipe Contreras wrote: 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. I believe these warnings existed before GCC 4.9 as well, but I'm not opposed to the change. -- 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: Pull is Mostly Evil
On 2014-05-04 06:17, Felipe Contreras wrote:
Richard Hansen wrote:
On 2014-05-03 23:08, Felipe Contreras wrote:
It is the only solution that has been proposed.
It's not the only proposal -- I proposed a few alternatives in my
earlier email (though not in the form of code), and others have too. In
particular:
* create a new 'git integrate' command/alias that behaves like 'git
pull --no-ff'
Yeah but that's for a different issue altogheter. I doesn't solve the
problems in 1. nor 2. nor 3.
'git integrate' would handle usage cases #2 (update a published branch
to its parent branch) and #3 (integrate a completed task into the main
line of development), making it feasible to change 'git pull' and 'git
pull $remote [$refspec]' to default to --ff-only to handle usage case #1
(update local branch to @{u}).
* change 'git pull' and 'git pull $remote [$refspec]' to do --ff-only
by default
Another option that I just thought of: Instead of your proposed
pull.mode and branch.name.pullmode, add the following two sets of configs:
* pull.updateMode, branch.name.pullUpdateMode:
The default mode to use when running 'git pull' without naming a
remote repository or when the named remote branch is @{u}. Valid
options: ff-only (default), merge-ff, merge-ff-there, merge-no-ff,
merge-no-ff-there, rebase, rebase-here, rebase-here-then-merge-no-ff
Those are way too many options to be able to sensibly explain them.
Certainly this is too many options for a first patch series, but I don't
think they're unexplainable. (I listed a bunch of options because I was
trying to envision where this might take us in the long run.)
For the first patch series, I'd expect: merge (which uses the merge.ff
option to determine whether to ff, ff-only, or no-ff), rebase, and ff-only.
Later ff-only would be made the default.
Later some or all of the other options would be added depending on user
interest.
* pull.integrateMode, branch.name.pullIntegrateMode:
The default mode to use when running 'git pull $remote [$refspec]'
when '$remote [$refspec]' is not @{u}. Valid options are the same
as those for pull.updateMode. Default is merge-ff.
This gives the default split behavior as you propose, but the user can
reconfigure to suit personal preference (and we can easily change the
default for one or the other if there's too much outcry).
If we reduce the number of options to begin with (more can be added
later),
yup
then it might make sense to have these two options.
However, that doesn't change the proposal you described above (1. 2.
3.).
Not sure what you mean. I oulined three usage cases:
#1 update local branch to @{u}
#2 update a published branch to its parent branch
#3 integrate a completed task into the main line of development
Having these two sets of options (updateMode and integrateMode) would
make it possible to configure plain 'git pull' to handle usage case #1
and 'git pull $remote [$refspec]' to handle usage cases #2 and #3.
Or the user could configure 'git pull' and 'git pull $remote [$refspec]'
to behave the same, in case they find the different behaviors to be too
confusing.
There's something we can do, and let me clarify my proposal. What you
described above is what I think should happen eventually, however, we
can start by doing something like what my patch series is doing; issue a
warning that the merge is not fast-forward and things might change in
the future.
OK, let me rephrase to make sure I understand:
1. leave the default behavior as-is for now (merge with local
branch the first parent)
2. add --merge argument
3. add ff-only setting
4. plan to eventually change the plain 'git pull' default to ff-only,
but don't change the default yet
5. add a warning if the plain 'git pull' is a non-ff
6. wait and see how users react. If they're OK with it, switch the
default of the plain 'git pull' to ff-only.
Is that accurate? If so, sounds OK to me.
If people find this behavior confusing they will complain in the mailing
list.
true
Although I suspect it would be for other reasons, not the 'git
pull'/'git pull $there' division.
probably
Either way we would see in the discussion.
sounds good to me
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
Re: [PATCH 1/9] Define a structure for object IDs.
Am 04.05.2014 12:55, schrieb Andreas Schwab: Johannes Sixt j...@kdbg.org writes: I think that a compiler that has different size and alignment requirements for the proposed struct object_id and an unsigned char[20] would, strictly speaking, not be a C compiler. Unlike arrays, a struct can have arbitrary internal padding. It is perfectly compliant (and even reasonable) to make struct object_id require 8 byte alignment, adding 4 bytes of padding at the end. Isn't internal padding only allowed between members to achieve correct alignment of later members, and at the end only sufficient padding so that members are aligned correctly when the struct is part of an array? The former would not be the case because there is only one member, and the latter is not the case because a char or array of char does not have alignment requirement? -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] Silence a bunch of format-zero-length warnings
brian m. carlson wrote: On Sun, May 04, 2014 at 01:12:55AM -0500, Felipe Contreras wrote: 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. I believe these warnings existed before GCC 4.9 as well, but I'm not opposed to the change. Yes, I'm aware of that. I didn't say they started to happen in 4.9, I said they happen in 4.9. -- Felipe Contreras-- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 10/12] MINGW: compat/poll/poll.c: undef NOGDI
Marat Radchenko wrote: 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. I agree with that. Can you file a bug report? -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 10/12] MINGW: compat/poll/poll.c: undef NOGDI
On Sun, May 04, 2014 at 08:52:44PM +0200, Stepan Kasal wrote: Thank you very much for this analysis. It enables us to redirect you the third time: to report this as a bug in MinGW-W64 ! ;-) I'll report this to MinGW-W64 soon, though even if/when they fix the issue on their side, I'd still like to have a workaround in Git to be able to use older MinGW-W64 versions that didn't receive a fix. Seriously, it looks you found the best description of the problem, and it would be nice if you could modify your patch so that it is really a work around: it would be in effect only for MinGW-W64, and the comment would explain that this is a hack to work around the bug. Workarounds do not have to be ugly and full of #ifdef's. If you manage to change the defs for poll.c without changing its content, no one could tell you to report to gnulib first. v1 does exactly this. OTOH, if MsgWaitForMultipleObjects is present ustream (in gnulib's poll.c, sorry that I cannot check right now), it still might be better to submit the work-around there first. Workaround is just don't pass -DNOGDI on MinGW-W64 if you want MsgWaitForMultipleObjects, there's nothing to send to gnulib. After all, was there a strong reason why Git started passing it? What is there was no option to disable part of windows.h? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: pull.prompt or other way to slow/disable 'git pull'
W. Trevor King wrote: Do you feel folks won't need a way to slow/disable 'git pull' while they build the ff options and their project's recommended workflow into their own practice? That's right. Or do you agree that they will need some kind of helper for the transition, and just feel that git.prompt is the wrong helper? I feel helpers are good when we are transitioning from an established Git behavior to a new one. Or when the operation is potentially dangerous. But a fast-forward merge is not dangerous, an in fact it's what the vast majority of people would want. Even more, I'm now feeling confident I will be able to put a proposal that allow a simple configuration to fulfill the need of these users without affecting anyone else negatively. -- 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
Richard Hansen wrote:
On 2014-05-04 06:17, Felipe Contreras wrote:
Richard Hansen wrote:
On 2014-05-03 23:08, Felipe Contreras wrote:
It is the only solution that has been proposed.
It's not the only proposal -- I proposed a few alternatives in my
earlier email (though not in the form of code), and others have too. In
particular:
* create a new 'git integrate' command/alias that behaves like 'git
pull --no-ff'
Yeah but that's for a different issue altogheter. I doesn't solve the
problems in 1. nor 2. nor 3.
'git integrate' would handle usage cases #2 (update a published branch
to its parent branch) and #3 (integrate a completed task into the main
line of development),
But these cases are completely different. One should reverse the
parents, the other one not.
I feel if a new command is to be added, it should be the one that is
introducing the brand new behavior: switching the parents. So it would
be appropriate for 1. and 2.
* change 'git pull' and 'git pull $remote [$refspec]' to do --ff-only
by default
Another option that I just thought of: Instead of your proposed
pull.mode and branch.name.pullmode, add the following two sets of
configs:
* pull.updateMode, branch.name.pullUpdateMode:
The default mode to use when running 'git pull' without naming a
remote repository or when the named remote branch is @{u}. Valid
options: ff-only (default), merge-ff, merge-ff-there, merge-no-ff,
merge-no-ff-there, rebase, rebase-here, rebase-here-then-merge-no-ff
Those are way too many options to be able to sensibly explain them.
Certainly this is too many options for a first patch series, but I don't
think they're unexplainable. (I listed a bunch of options because I was
trying to envision where this might take us in the long run.)
Actually I think they are too many for any point in time.
Maybe pull.updateArgs would make more sense.
For the first patch series, I'd expect: merge (which uses the merge.ff
option to determine whether to ff, ff-only, or no-ff), rebase, and ff-only.
Seems sensible.
then it might make sense to have these two options.
However, that doesn't change the proposal you described above (1. 2.
3.).
Not sure what you mean. I oulined three usage cases:
#1 update local branch to @{u}
#2 update a published branch to its parent branch
#3 integrate a completed task into the main line of development
Having these two sets of options (updateMode and integrateMode) would
make it possible to configure plain 'git pull' to handle usage case #1
and 'git pull $remote [$refspec]' to handle usage cases #2 and #3.
Not if by default they are already handled.
There's something we can do, and let me clarify my proposal. What you
described above is what I think should happen eventually, however, we
can start by doing something like what my patch series is doing; issue a
warning that the merge is not fast-forward and things might change in
the future.
OK, let me rephrase to make sure I understand:
1. leave the default behavior as-is for now (merge with local
branch the first parent)
2. add --merge argument
3. add ff-only setting
4. plan to eventually change the plain 'git pull' default to ff-only,
but don't change the default yet
5. add a warning if the plain 'git pull' is a non-ff
6. wait and see how users react. If they're OK with it, switch the
default of the plain 'git pull' to ff-only.
Is that accurate? If so, sounds OK to me.
That is what my patch series is doing already, basically.
The new warning I'm proposing would be for the split behavior of 'git
merge' and 'git merge $there'. Which is what is worrysome.
mode = rebase-here-then-merge-no-ff would do what I described
I think that mode is way too specific to be useful for most people.
--
Felipe Contreras
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/9] Define a structure for object IDs.
Johannes Sixt j...@kdbg.org writes: Isn't internal padding only allowed between members to achieve correct alignment of later members, and at the end only sufficient padding so that members are aligned correctly when the struct is part of an array? The standard allows arbitrary internal padding, it doesn't have to be minimal. Andreas. -- Andreas Schwab, sch...@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 And now for something completely different. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Summary of the problems with git pull
Hi,
There has been a lot of discussion about why `git pull` is broken for so
many many workflows: [1][2][3][4][5], even as far back as [6].
Many issues has been brought up, and many proposed solutions, probably
too many for most people properly digest them, so here I'll try to
synthesize them.
Mainly there are two core problems with `git pull`:
1) Many times it does a merge when it's not when people want
2) Many times it does a merge opposite of the desired direction
The issue comes in trying to solve these problems in a way that would
not affect anybody negatively.
The simplest way to fix these issues unobtrusively would be to add
configurations for the new behavior. However, this wouldn't solve the
problem because as it has been discussed the vast majority of people
doing these bad merges are not advanced users, so they wouldn't
activate these options.
What is needed is to change the default behavior, but in a way that is
not obtrusive, and with backwards compatibility configuration.
The most concrete proposal is my patch series that puts everything in
place to enable fast-forward merged by default only. However, that still
doesn't solve the problem of the ordering of parents. Normally these two
issues would be independent of each other, but after further analysis I
think they are not.
== Two different kinds of pulls ==
It has become clear that the `git pull` command is in fact used to do
two very different things:
1) Update the current branch
Most people do `git pull` like they would do `svn update`; to update
their current branch.
2) Merge a remote branch
There are many use-cases of this.
The agreement so far is that 1) is the one that is broken, that is; a)
by default only fast-forwards should be allowed, and b) when a merge
happens the parents should be reverted (merge 'master' to
'origin/master').
It would be possible to differentiate these kinds by saying a `git pull`
that doesn't specify the location can be assumed as 1), and a
`git pull remote branch` that does as 2). Another possibility is to
assume only the upstream branch corresponds to 1).
Unfortunately it's the feeling of many people that this solution is not
clean, because it's two very different behaviors for the same command.
Furthermore even deeper analysis of the use-cases demonstrates there
would be a need to have different configurations for the different modes
(e.g. pull.updateMode and pull.integrateMode).
== git update ==
Another proposed solution is to have a new command: `git update`. This
command would be similar to `git pull --ff-only` by default, but it
could be configured to do merges instead, and when doing so in reverse.
An interesting side-effect of this new command is that it opens the
possibility of thinking about `git update --rebase` vs.
`git pull --rebase`.
For example:
a) git update --merge
Merge HEAD into @{upstream}
b) git update --rebase
Rebase HEAD onto @{upstream}
c) git pull --merge github john
Merge github/john into HEAD
d) git pull --rebase github john
Rebase github/john onto HEAD
Notice how the relationships between them are nice and consistent. Also,
d) was not possible before.
This solves essentially all the issues people presented in their
use-cases, except the differentiation between merging topic and upstream
branches, for which we might want to add different options in
`git pull`, but that's independent from the issues I mentioned at the
beginning.
Nothing changes for the users of `git pull`, except that perhaps we
would want --rebase to work in reverse, since the current
`git pull --rebase` would already rebase HEAD onto @{upstream}.
Personally my next step would be to port the changes I did for
pull.mode = merge-ff-only into a new command `git update`, which would
probably be a copy-paste from `git pull` and see how that turns out. In
addition, when running `git pull` without arguments we might want to add
a temporary notice explaining that perhaps the user wanted to type
`git update` instead.
Cheers.
[1] http://article.gmane.org/gmane.comp.version-control.git/233554
[2] http://article.gmane.org/gmane.comp.version-control.git/234295
[3] http://article.gmane.org/gmane.comp.version-control.git/225146
[4] http://article.gmane.org/gmane.comp.version-control.git/247237
[5] http://article.gmane.org/gmane.comp.version-control.git/247939
[6] http://article.gmane.org/gmane.comp.version-control.git/130819
--
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
material for git training sessions/presentations
Hi, I know there are a few people on this list that do git training in various forms. At $dayjob I've been asked to run a few training sessions in house. The initial audience is SW developers so they are fairly clued up on VCS concepts and most have some experience (although some not positive) with git. Eventually this may also include some QA folks who are writing/maintaining test suites who might be less clued up on VCSes in general. I know if I googled for git tutorials I'll find a bunch and I can probably write a few myself but does anyone have any advice from training sessions they've run about how best to present the subject matter. Particularly to a fairly savy audience who may have developed some bad habits. My plan was to try and have a few PCs/laptops handy and try to make it a little interactive. Also if anyone has any presentations I could use under a CC-BY-SA (or other liberal license) as a basis for any material I produce that would save me starting from scratch. Thanks, Chris -- 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: material for git training sessions/presentations
The GitHub training team has all of their materials open sourced under a CC BY 3.0 license. They're all written in Markdown and hosted on GitHub. You can check them out here, including going through an online rendering of the materials: http://training.github.com/kit/ Scott On Sun, May 4, 2014 at 9:18 PM, Chris Packham judge.pack...@gmail.com wrote: Hi, I know there are a few people on this list that do git training in various forms. At $dayjob I've been asked to run a few training sessions in house. The initial audience is SW developers so they are fairly clued up on VCS concepts and most have some experience (although some not positive) with git. Eventually this may also include some QA folks who are writing/maintaining test suites who might be less clued up on VCSes in general. I know if I googled for git tutorials I'll find a bunch and I can probably write a few myself but does anyone have any advice from training sessions they've run about how best to present the subject matter. Particularly to a fairly savy audience who may have developed some bad habits. My plan was to try and have a few PCs/laptops handy and try to make it a little interactive. Also if anyone has any presentations I could use under a CC-BY-SA (or other liberal license) as a basis for any material I produce that would save me starting from scratch. Thanks, Chris -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: material for git training sessions/presentations
Scott Chacon wrote: The GitHub training team has all of their materials open sourced under a CC BY 3.0 license. They're all written in Markdown and hosted on GitHub. You can check them out here, including going through an online rendering of the materials: http://training.github.com/kit/ Very nice! I'm skimming through the contents and I noticed you mention 'color.ui = auto' a lot. There's no need for that, it has been the default since v1.8.4. Cheers. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/5] contrib/subtree/Makefile: Standardisation pass
On Sat, May 03, 2014 at 10:49:30PM +1000, James Denholm wrote: 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. Thanks, these all look sane to me (I do not use subtree, but since it's just about Makefiles, it was pretty easy to review). One problem is foreseen: 3/5 will necessitate that package maintainers who already have git-subtree included in their packages update their build-scripts. I think that's probably OK. We strive for backwards compatibility in the tool itself, but refactoring Makefiles in contrib/ affects a pretty limited audience. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 5/5] contrib/subtree/Makefile: clean rule cleanup
On Sat, May 03, 2014 at 10:49:35PM +1000, James Denholm wrote: 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 I do not think BSD-ism matters for rm, as it works pretty much the same everywhere. install, on the other hand, is a bit weirder between systems. So you might want to leave that comment as-is. OTOH, we do not even bother with such a comment in the main Makefile. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] Silence a bunch of format-zero-length warnings
On Sun, May 04, 2014 at 07:01:22PM +, brian m. carlson wrote: On Sun, May 04, 2014 at 01:12:55AM -0500, Felipe Contreras wrote: 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. I believe these warnings existed before GCC 4.9 as well, but I'm not opposed to the change. Yeah, this started last summer when we added __attribute__((format)) to the status_printf_ln calls, and I posted essentially the same patch. We kind of waffled between eh, just set -Wno-format-zero-length and doing something, and ended up at the former. I'd be fine with doing it this way; we're not likely to add a lot of new callsites that would make it a hassle to keep up with. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Pull is Mostly Evil
On 2014-05-04 17:13, Felipe Contreras wrote: Richard Hansen wrote: On 2014-05-04 06:17, Felipe Contreras wrote: Richard Hansen wrote: On 2014-05-03 23:08, Felipe Contreras wrote: It is the only solution that has been proposed. It's not the only proposal -- I proposed a few alternatives in my earlier email (though not in the form of code), and others have too. In particular: * create a new 'git integrate' command/alias that behaves like 'git pull --no-ff' Yeah but that's for a different issue altogheter. I doesn't solve the problems in 1. nor 2. nor 3. 'git integrate' would handle usage cases #2 (update a published branch to its parent branch) and #3 (integrate a completed task into the main line of development), But these cases are completely different. One should reverse the parents, the other one not. No -- for both #2 and #3 I want the remote branch to be merged into the local branch. In the example I gave for use case #2, foo is a local branch with origin/foo as the configured upstream and origin/foo was forked off of origin/master. Someone pushed new stuff to origin/master, and the user wants the new stuff to also be in origin/foo. So the user does this: git checkout foo git pull --ff-only # this is use case #1 git pull origin master # this is use case #2 git push The merge commit created by 'git pull origin master' should have origin/master as the second parent, not the first. -Richard -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] Revert make error()'s constant return value more visible
On Sun, May 04, 2014 at 01:12:53AM -0500, Felipe Contreras wrote:
So it looks like gcc is smarter now, and in trying to fix a few warnings
we generated hundreds more.
This reverts commit e208f9cc7574f5980faba498d0aa30b4defeb34f.
And now we've gone the other way, and re-enabled the initial warnings.
Can we come up with a solution that helps both cases?
I could not find a way to annotate a value as maybe unused, but we can
hide it inside a function, like:
-- 8 --
Subject: inline error() function
The error() function does two things: it prints an error,
and it returns -1 as a convenience to allow:
return error(foo);
Commit e208f9c converted this to a macro to make the
constant return value more visible to the compiler. However,
recent versions of gcc complain when error is used in a void
context, as the constant -1 ends up unused.
Instead, let's convert error() to a static inline, which
should accomplish the same thing without the extra warnings
(because gcc will not warn about unused return values unless
warn_unused_result is specified for the particular
function).
Signed-off-by: Jeff King p...@peff.net
---
git-compat-util.h | 20
usage.c | 8 +---
2 files changed, 13 insertions(+), 15 deletions(-)
diff --git a/git-compat-util.h b/git-compat-util.h
index f6d3a46..3aef0d3 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -310,7 +310,6 @@ extern NORETURN void usage(const char *err);
extern NORETURN void usagef(const char *err, ...) __attribute__((format
(printf, 1, 2)));
extern NORETURN void die(const char *err, ...) __attribute__((format (printf,
1, 2)));
extern NORETURN void die_errno(const char *err, ...) __attribute__((format
(printf, 1, 2)));
-extern int error(const char *err, ...) __attribute__((format (printf, 1, 2)));
extern void warning(const char *err, ...) __attribute__((format (printf, 1,
2)));
#ifndef NO_OPENSSL
@@ -325,14 +324,19 @@ extern void warning(const char *err, ...)
__attribute__((format (printf, 1, 2)))
/*
* 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.
+ * gcc with -Wuninitialized analysis.
*/
-#if defined(__GNUC__) ! defined(__clang__)
-#define error(...) (error(__VA_ARGS__), -1)
-#endif
+__attribute__((format (printf, 1, 0)))
+extern void error_impl(const char *err, va_list params);
+__attribute__((format (printf, 1, 2)))
+static inline int error(const char *err, ...)
+{
+va_list params;
+va_start(params, err);
+error_impl(err, params);
+va_end(params);
+return -1;
+}
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));
diff --git a/usage.c b/usage.c
index ed14645..5456e4b 100644
--- a/usage.c
+++ b/usage.c
@@ -138,15 +138,9 @@ void NORETURN die_errno(const char *fmt, ...)
va_end(params);
}
-#undef error
-int error(const char *err, ...)
+void error_impl(const char *err, va_list params)
{
- va_list params;
-
- va_start(params, err);
error_routine(err, params);
- va_end(params);
- return -1;
}
void warning(const char *warn, ...)
--
2.0.0.rc1.436.g03cb729
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] Revert make error()'s constant return value more visible
Jeff King wrote: On Sun, May 04, 2014 at 01:12:53AM -0500, Felipe Contreras wrote: So it looks like gcc is smarter now, and in trying to fix a few warnings we generated hundreds more. This reverts commit e208f9cc7574f5980faba498d0aa30b4defeb34f. And now we've gone the other way, and re-enabled the initial warnings. Can we come up with a solution that helps both cases? What initial warnings? As I explained already I don't get any warnings with this patch series in gcc 4.9.0. -- 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
Richard Hansen wrote: On 2014-05-04 17:13, Felipe Contreras wrote: Richard Hansen wrote: On 2014-05-04 06:17, Felipe Contreras wrote: Richard Hansen wrote: On 2014-05-03 23:08, Felipe Contreras wrote: It is the only solution that has been proposed. It's not the only proposal -- I proposed a few alternatives in my earlier email (though not in the form of code), and others have too. In particular: * create a new 'git integrate' command/alias that behaves like 'git pull --no-ff' Yeah but that's for a different issue altogheter. I doesn't solve the problems in 1. nor 2. nor 3. 'git integrate' would handle usage cases #2 (update a published branch to its parent branch) and #3 (integrate a completed task into the main line of development), But these cases are completely different. One should reverse the parents, the other one not. No -- for both #2 and #3 I want the remote branch to be merged into the local branch. I didn't mean #2 and #3, I meant (#1) vs. (#2, #3). -- 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
