Re: Pull is Mostly Evil

2014-05-03 Thread James Denholm
Felipe Contreras wrote:
>David Lang wrote:
>> the vast majority of people here do not take that attitude.
>
>It's actually the exact opposite. I don't care what is the track record
>of the people in the discussion.

Ah, yes, like that discussion we once had where you totally
didn't run `git log | grep James Denholm` at one point to demonstrate that I 
had not yet made any
contributions,instead of actually engaging in discussion. Oh,
wait.

>If their argument is good, their argument is good.

The problem, though, is that time and time again you've
shown that you value your own arguments to the exclusion
of all others. You can't tell if someone else's argument is
 good, because it runs against yours, and yours must be
right because you hold it.

Regards,
James Denholm.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC PATCH 0/9] Use a structure for object IDs.

2014-05-03 Thread Michael Haggerty
On 05/03/2014 10:12 PM, brian m. carlson wrote:
> This is a preliminary RFC patch series to move all the relevant uses of
> unsigned char [20] to struct object_id.  It should not be applied to any
> branch yet.
> 
> The goal of this series to improve type-checking in the codebase and to
> make it easier to move to a different hash function if the project
> decides to do that.  This series does not convert all of the codebase,
> but only parts.  I'm looking for feedback to see if there is consensus
> that this is the right direction before investing a large amount of
> time.
> 
> Certain parts of the code have to be converted before others to keep the
> patch sizes small, maintainable, and bisectable, so functions and
> structures that are used across the codebase (e.g. hashcmp and struct
> object) will be converted later.  Conversion has been done in a roughly
> alphabetical order by name of file.
> 
> The constants for raw and hex sizes of SHA-1 values are maintained.
> These constants are used where the quantity is the size of an SHA-1
> value, and sizeof(struct object_id) is used wherever memory is to be
> allocated.  This is done to permit the struct to turn into a union later
> if multiple hashes are supported.  I left the names at GIT_OID_RAWSZ and
> GIT_OID_HEXSZ because that's what libgit2 uses and what Junio seemed to
> prefer, but they can be changed later if there's a desire to do that.
> 
> I called the structure member "oid" because it was easily grepable and
> distinct from the rest of the codebase.  It, too, can be changed if we
> decide on a better name.  I specifically did not choose "sha1" since it
> looks weird to have "sha1->sha1" and I didn't want to rename lots of
> variables.

That means that we will have sha1->oid all over the place, right?
That's unfortunate, because it is exactly backwards from what we would
want in a hypothetical future where OIDs are not necessarily SHA-1s.  In
that future we would certainly have to support SHA-1s in parallel with
the new hash.  So (in that hypothetical future) we will probably want
these expressions to look like oid->sha1, to allow, say, a second struct
or union field oid->sha256 [1].

If that future would come to pass, then we would also want to have
distinct constants like GIT_SHA1_RAWSZ and GIT_SHA256_RAWSZ rather than
the generically-named GIT_OID_RAWSZ.

I think that this patch series will improve the code clarity and type
safety independent of thoughts about supporting different hash
algorithms, so I'm not objecting to your naming decision.  But *if* such
support is part of your long-term hope, then you might ease the future
transition by choosing different names now.

(Maybe renaming local variables "sha1 -> oid" might be a handy way of
making clear which code has been converted to the new style.)

Just to be clear, the above are just some random thoughts for your
consideration, but feel free to disregard them.

In any case, it sure will be a lot of code churn.  If you succeed in
this project, then "git blame" will probably consider you the author of
about 2/3 of git :-)

Michael

[1] I'm certainly not advocating that we want to support a different
hash, let alone that that hash should be SHA-256; these examples are
just for illustration.

-- 
Michael Haggerty
mhag...@alum.mit.edu
http://softwareswirl.blogspot.com/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 1/3] Revert "make error()'s constant return value more visible"

2014-05-03 Thread Felipe Contreras
In recent versions of gcc (4.9.0), we get hundreds of these:

  advice.c: In function ‘error_resolve_conflict’:
  advice.c:79:69: warning: right-hand operand of comma expression has no effect 
[-Wunused-value]
error("'%s' is not possible because you have unmerged files.", me);
 ^
The original patch intended to help in situations like this:

  if (error(...))
/* do stuff */

However, when there's no conditional statement this gets translated to:

  (error(..), 1);

And the right hand of the expression has no effect.

So it looks like gcc is smarter now, and in trying to fix a few warnings
we generated hundreds more.

This reverts commit e208f9cc7574f5980faba498d0aa30b4defeb34f.

Conflicts:
git-compat-util.h
---
 git-compat-util.h | 11 ---
 usage.c   |  1 -
 2 files changed, 12 deletions(-)

diff --git a/git-compat-util.h b/git-compat-util.h
index f6d3a46..b4242e4 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -323,17 +323,6 @@ extern void warning(const char *err, ...) 
__attribute__((format (printf, 1, 2)))
 #include 
 #endif /* NO_OPENSSL */
 
-/*
- * Let callers be aware of the constant return value; this can help
- * gcc with -Wuninitialized analysis. We restrict this trick to gcc, though,
- * because some compilers may not support variadic macros. Since we're only
- * trying to help gcc, anyway, it's OK; other compilers will fall back to
- * using the function as usual.
- */
-#if defined(__GNUC__) && ! defined(__clang__)
-#define error(...) (error(__VA_ARGS__), -1)
-#endif
-
 extern void set_die_routine(NORETURN_PTR void (*routine)(const char *err, 
va_list params));
 extern void set_error_routine(void (*routine)(const char *err, va_list 
params));
 extern void set_die_is_recursing_routine(int (*routine)(void));
diff --git a/usage.c b/usage.c
index ed14645..9d2961e 100644
--- a/usage.c
+++ b/usage.c
@@ -138,7 +138,6 @@ void NORETURN die_errno(const char *fmt, ...)
va_end(params);
 }
 
-#undef error
 int error(const char *err, ...)
 {
va_list params;
-- 
1.9.2+fc1.20.g204a630

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 3/3] Silence a bunch of format-zero-length warnings

2014-05-03 Thread Felipe Contreras
This is in gcc 4.9.0:

  wt-status.c: In function ‘wt_status_print_unmerged_header’:
  wt-status.c:191:2: warning: zero-length gnu_printf format string 
[-Wformat-zero-length]
status_printf_ln(s, c, "");
^

We could pass -Wno-format-zero-length, but it seems compiler-specific
flags are frowned upon, so let's just avoid the warning altogether.

Signed-off-by: Felipe Contreras 
---
 builtin/commit.c |  2 +-
 wt-status.c  | 22 +++---
 2 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index 9cfef6c..13b84e4 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -812,7 +812,7 @@ static int prepare_to_commit(const char *index_file, const 
char *prefix,
committer_ident.buf);
 
if (ident_shown)
-   status_printf_ln(s, GIT_COLOR_NORMAL, "");
+   status_printf_ln(s, GIT_COLOR_NORMAL, "%s", "");
 
saved_color_setting = s->use_color;
s->use_color = 0;
diff --git a/wt-status.c b/wt-status.c
index ec7344e..b8841e1 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -188,7 +188,7 @@ static void wt_status_print_unmerged_header(struct 
wt_status *s)
} else {
status_printf_ln(s, c, _("  (use \"git add/rm ...\" as 
appropriate to mark resolution)"));
}
-   status_printf_ln(s, c, "");
+   status_printf_ln(s, c, "%s", "");
 }
 
 static void wt_status_print_cached_header(struct wt_status *s)
@@ -204,7 +204,7 @@ static void wt_status_print_cached_header(struct wt_status 
*s)
status_printf_ln(s, c, _("  (use \"git reset %s ...\" to 
unstage)"), s->reference);
else
status_printf_ln(s, c, _("  (use \"git rm --cached ...\" 
to unstage)"));
-   status_printf_ln(s, c, "");
+   status_printf_ln(s, c, "%s", "");
 }
 
 static void wt_status_print_dirty_header(struct wt_status *s,
@@ -223,7 +223,7 @@ static void wt_status_print_dirty_header(struct wt_status 
*s,
status_printf_ln(s, c, _("  (use \"git checkout -- ...\" to 
discard changes in working directory)"));
if (has_dirty_submodules)
status_printf_ln(s, c, _("  (commit or discard the untracked or 
modified content in submodules)"));
-   status_printf_ln(s, c, "");
+   status_printf_ln(s, c, "%s", "");
 }
 
 static void wt_status_print_other_header(struct wt_status *s,
@@ -235,12 +235,12 @@ static void wt_status_print_other_header(struct wt_status 
*s,
if (!s->hints)
return;
status_printf_ln(s, c, _("  (use \"git %s ...\" to include in 
what will be committed)"), how);
-   status_printf_ln(s, c, "");
+   status_printf_ln(s, c, "%s", "");
 }
 
 static void wt_status_print_trailer(struct wt_status *s)
 {
-   status_printf_ln(s, color(WT_STATUS_HEADER, s), "");
+   status_printf_ln(s, color(WT_STATUS_HEADER, s), "%s", "");
 }
 
 #define quote_path quote_path_relative
@@ -826,7 +826,7 @@ static void wt_status_print_other(struct wt_status *s,
string_list_clear(&output, 0);
strbuf_release(&buf);
 conclude:
-   status_printf_ln(s, GIT_COLOR_NORMAL, "");
+   status_printf_ln(s, GIT_COLOR_NORMAL, "%s", "");
 }
 
 void wt_status_truncate_message_at_cut_line(struct strbuf *buf)
@@ -913,7 +913,7 @@ static void wt_status_print_tracking(struct wt_status *s)
color_fprintf_ln(s->fp, color(WT_STATUS_HEADER, s), "%c",
 comment_line_char);
else
-   fprintf_ln(s->fp, "");
+   fputs("", s->fp);
 }
 
 static int has_unmerged(struct wt_status *s)
@@ -1329,7 +1329,7 @@ void wt_status_print(struct wt_status *s)
on_what = _("Not currently on any branch.");
}
}
-   status_printf(s, color(WT_STATUS_HEADER, s), "");
+   status_printf(s, color(WT_STATUS_HEADER, s), "%s", "");
status_printf_more(s, branch_status_color, "%s", on_what);
status_printf_more(s, branch_color, "%s\n", branch_name);
if (!s->is_initial)
@@ -1342,9 +1342,9 @@ void wt_status_print(struct wt_status *s)
free(state.detached_from);
 
if (s->is_initial) {
-   status_printf_ln(s, color(WT_STATUS_HEADER, s), "");
+   status_printf_ln(s, color(WT_STATUS_HEADER, s), "%s", "");
status_printf_ln(s, color(WT_STATUS_HEADER, s), _("Initial 
commit"));
-   status_printf_ln(s, color(WT_STATUS_HEADER, s), "");
+   status_printf_ln(s, color(WT_STATUS_HEADER, s), "%s", "");
}
 
wt_status_print_updated(s);
@@ -1361,7 +1361,7 @@ void wt_status_print(struct wt_status *s)
if (s->show_ignored_files)
wt_status_print_other(s, &s->ignored, _("Ignored 
files"), "add -f");
if (advice_status_u_option && 2000 < s->unt

[PATCH 2/3] Revert "silence some -Wuninitialized false positives"

2014-05-03 Thread Felipe Contreras
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

2014-05-03 Thread Felipe Contreras
Hi,

I'm getting tons and tons of warnings with gcc 4.9.0. Here are the patches to
fix them all.


Felipe Contreras (3):
  Revert "make error()'s constant return value more visible"
  Revert "silence some -Wuninitialized false positives"
  Silence a bunch of format-zero-length warnings

 builtin/commit.c  |  2 +-
 cache.h   |  3 ---
 config.c  |  1 -
 git-compat-util.h | 11 ---
 parse-options.c   | 18 +-
 parse-options.h   |  4 
 usage.c   |  1 -
 wt-status.c   | 22 +++---
 8 files changed, 21 insertions(+), 41 deletions(-)

-- 
1.9.2+fc1.20.g204a630

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Pull is Mostly Evil

2014-05-03 Thread David Kastrup
Felipe Contreras  writes:

> David Lang wrote:
>> note that this is one person taking the "I don't see any commits from
>> you so your opinion doesn't count" attitude.
>
> Wrong. I said it doesn't count "for the project".

There are a number of commits from me that actually count.  A few old
core performance ones might have actually have affected my carbon
footprint noticeably.  The one currently in pu will probably not be
called often enough for that but will at least have practical
consequences.

> Do you honestly believe Junio cares about what some random guy on the
> list thinks about default aliases? No.

Putting aside my code contributions: Git is a comparatively small
project, so if the main project you are working on with Git is Git, your
experience is limited.  So yes, input from people who are _not_ heavy
Git developers is important, since the heavy Git developers do not get
to see the heavy Git use cases a lot.

> It's actually the exact opposite. I don't care what is the track
> record of the people in the discussion. If their argument is good,
> their argument is good.

More like if they are around, they are worth getting plastered with your
frustration.

> It's the others that focus on the carisma and credentials of the
> people in the discussion, rather than the arguments.

I think you are confusing inertia with resistance.

-- 
David Kastrup
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] t3910: show failure of core.precomposeunicode with decomposed filenames

2014-05-03 Thread Torsten Bögershausen
(Sorry for the late reply, I'm handicapped by some local IT problems)
Peff, do you know if the fix below helps ?

On 2014-04-28 18.16, Jeff King wrote:
> If you have existing decomposed filenames in your git
> repository (e.g., that were created with older versions of
> git that did not precompose unicode), a modern git with
> core.precomposeunicode set does not handle them well.
Yes.
> The problem is that we normalize the paths coming from the
> disk into their precomposed form, and then compare them
> against the literal bytes in the index. This makes things
> better if you have the precomposed form in the index. It
> makes things worse if you actually have the decomposed form
> in the index.
>
> As a result, paths with decomposed filenames may have their
> precomposed variants listed as untracked files (even though
> the precomposed variants do not exist on-disk at all).
Yes
> This patch just adds a test to demonstrate the breakage.
> Some possible fixes are:
>
>   1. Tell everyone that NFD in the git repo is wrong, and
>  they should make a new commit to normalize all their
>  in-repo files to be precomposed.
>  This is probably not the right thing to do, because it
>  still doesn't fix checkouts of old history. And it
>  spreads the problem to people on byte-preserving
>  filesystems (like ext4), because now they have to start
>  precomposing their filenames as they are adde to git.
 (typo:  
^added)
I'm not sure if I follow. People running ext4 (or Linux in general,
or Windows, or Unix) do not suffer from file system
"feature" of Mac OS, which accepts precomposed/decomposed Unicode
but returns decompomsed.
>
>   2. Do all index filename comparisons using a UTF-8 aware
>  comparison function when core.precomposeunicode is set.
>  This would probably have bad performance, and somewhat
>  defeats the point of converting the filenames at the
>  readdir level in the first place.
>
>   3. Convert index filenames to their precomposed form when
>  we read the index from disk. This would be efficient,
>  but we would have to be careful not to write the
>  precomposed forms back out to disk.
Question:
How could we be careful?
Mac OS writes always decomposed Unicode to disk.
(And all other OS tend to use precomposed forms, mainly because the "keyboard
driver" generates it.)
>
>   4. Introduce some infrastructure to efficiently match up
>  the precomposed/decomposed forms. We already do
>  something similar for case-insensitive files using
>  name-hash.c. We might be able to adapt that strategy
>  here.
--

This is my understanding:
Some possible fixes are:

  1. Accept that NFD in a Git repo which is shared between Mac OS
 and Linux or Windows is problematic.
 Whenever core.precomposeunicode = true, do the following:
 Let Git under Mac OS change all file names in the index
 into the precomposed form when a new commit is done.
 This is probably not a wrong thing to do.

 When the index file is read into memory, precompose the file names and 
compare
 them with the precomposed form coming from precompose_utf8_readdir().
 This avoids decomposed file names to be reported as untracked by "git 
status.

  2. Do all index filename comparisons under Mac OS X using a UTF-8 aware
 comparison function regardless if core.precomposeunicode is set.
 This would probably have bad performance, and somewhat
 defeats the point of converting the filenames at the
 readdir level in the first place.

(The TC is good)

>
> Signed-off-by: Jeff King 
> ---
>  t/t3910-mac-os-precompose.sh | 10 ++
>  1 file changed, 10 insertions(+)
>
> diff --git a/t/t3910-mac-os-precompose.sh b/t/t3910-mac-os-precompose.sh
> index e4ba601..23aa61e 100755
> --- a/t/t3910-mac-os-precompose.sh
> +++ b/t/t3910-mac-os-precompose.sh
> @@ -140,6 +140,16 @@ test_expect_success "Add long precomposed filename" '
>   git add * &&
>   git commit -m "Long filename"
>  '
> +
> +test_expect_failure 'handle existing decomposed filenames' '
> + echo content >"verbatim.$Adiarnfd" &&
> + git -c core.precomposeunicode=false add "verbatim.$Adiarnfd" &&
> + git commit -m "existing decomposed file" &&
> + >expect &&
> + git ls-files --exclude-standard -o "verbatim*" >untracked &&
> + test_cmp expect untracked
> +'
> +
>  # Test if the global core.precomposeunicode stops autosensing
>  # Must be the last test case
>  test_expect_success "respect git config --global core.precomposeunicode" '
On top of the we need to following:

diff --git a/compat/precompose_utf8.c b/compat/precompose_utf8.c
index 95fe849..5ed3d17 100644
--- a/compat/precompose_utf8.c
+++ b/compat/precompose_utf8.c
@@ -57,6 +57,19 @@ void probe_utf8_pathname_composition(char *path, int len)
 }
 
 
+char *precompose_str_len(const char *in, size_t insz, int *outsz)

Re: [PATCH 1/9] Define a structure for object IDs.

2014-05-03 Thread Michael Haggerty
On 05/03/2014 10:12 PM, brian m. carlson wrote:
> Many places throughout the code use "unsigned char [20]" to store object IDs
> (SHA-1 values).  This leads to lots of hardcoded numbers throughout the
> codebase.  It also leads to confusion about the purposes of a buffer.
> 
> Introduce a structure for object IDs.  This allows us to obtain the benefits
> of compile-time checking for misuse.  The structure is expected to remain
> the same size and have the same alignment requirements on all known
> platforms, compared to the array of unsigned char.

Please clarify whether you plan to rely on all platforms having "the
same size and alignment constraints" for correctness, or whether that
observation of the status quo is only meant to reassure us that this
change won't cause memory to be wasted on padding.

If the former then I would feel very uncomfortable about the change.
Otherwise I think it will be a nice improvement in code clarity (and I
admire your ambition in taking on this project!)

Michael

-- 
Michael Haggerty
mhag...@alum.mit.edu
http://softwareswirl.blogspot.com/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: BUG or FEATURE? Use of '/' in branch names

2014-05-03 Thread Michael Haggerty
On 05/03/2014 09:35 AM, Dennis Kaarsemaker wrote:
> On vr, 2014-05-02 at 15:16 -0700, Jonathan Nieder wrote:
>>> $ git checkout -b hotfix/b2
>>> error: unable to resolve reference refs/heads/hotfix/b2: Not a
>> directory
>>> fatal: Failed to lock ref for update: Not a directory
>>> $
>>
>> That's an ugly message.  I think we can do better. (hint hint)
> 
> 2.0.0-rc2 has a better message already:
> 
> $ git checkout -b hotfix/b2
> error: 'refs/heads/hotfix' exists; cannot create 'refs/heads/hotfix/b2'
> fatal: Failed to lock ref for update: Not a directory

I was trying to remember when this was changed, but at first I couldn't
reproduce the "fixed" error message at all.  Finally I figured out that
the the error message that you get depends on whether the existing
reference is loose:

$ bin-wrappers/git checkout -b master/foo
error: unable to resolve reference refs/heads/master/foo: Not a
directory
fatal: Failed to lock ref for update: Not a directory

vs. packed:

$ bin-wrappers/git checkout -b base/foo
error: 'refs/heads/base' exists; cannot create 'refs/heads/base/foo'
fatal: Failed to lock ref for update: Not a directory

It would be good to make the error message uniform and to document this
restriction.

Michael

-- 
Michael Haggerty
mhag...@alum.mit.edu
http://softwareswirl.blogspot.com/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Pull is Mostly Evil

2014-05-03 Thread Felipe Contreras
David Lang wrote:
> note that this is one person taking the "I don't see any commits from
> you so your opinion doesn't count" attitude.

Wrong. I said it doesn't count "for the project". Do you honestly
believe Junio cares about what some random guy on the list thinks about
default aliases? No.

If he doesn't care what literally everyone thinks about the name
"index", why would he care about that random guy?

> the vast majority of people here do not take that attitude.

It's actually the exact opposite. I don't care what is the track record
of the people in the discussion. If their argument is good, their
argument is good.

It's the others that focus on the carisma and credentials of the people
in the discussion, rather than the arguments.

-- 
Felipe Contreras
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Pull is Mostly Evil

2014-05-03 Thread David Lang

On Sat, 3 May 2014, David Kastrup wrote:


Felipe Contreras  writes:


David Kastrup wrote:

Richard Hansen  writes:


These three usage patterns are at odds; it's hard to change the
default behavior of 'git pull' to favor one usage case without
harming another.  Perhaps this is why there's so much disagreement
about what 'git pull' should do.


Should a screwdriver be turning clockwise or counterclockwise by
default?  There are valid arguments for either.


If you don't have anything to contribute don't disturb the people that
actually care and are trying to improve Git. Thanks.


No need to expand on the welcoming atmosphere here.


note that this is one person taking the "I don't see any commits from you so 
your opinion doesn't count" attitude.


the vast majority of people here do not take that attitude.

David Lang


 My heinous plot to
subvert the quality of Git has already been thwarted by making sure that
its "meritocracy" continues relying only on input from those with an
independent income.  I'm just sticking around until my current
contributions move into master so that I can summarize the resulting
low-hanging fruit that the meritorious can then pick at great fanfare.

The sooner my work moves from pu into master, the sooner y'all be rid of
me.



Re: Pull is Mostly Evil

2014-05-03 Thread Felipe Contreras
Richard Hansen wrote:
> On 2014-05-03 05:26, Felipe Contreras wrote:
> > Richard Hansen wrote:
> > 
> >> I think the fundamental difference is in the relationship between the
> >> local and the remote branch (which branch derives from the other).
> >> The relationship between the branches determines what the user wants
> >> from 'git pull'.
> >>
> >> In my experience 'git pull' is mostly (only?) used for the following
> >> three tasks:
> > 
> > I agree.
> > 
> >>  1. update a local branch to incorporate the latest upstream changes
> >>
> >> In this case, the local branch (master) is a derivative of the
> >> upstream branch (origin/master).  The user wants all of the
> >> commits in the remote branch to be in the local branch.  And the
> >> user would like the local changes, if any, to descend from the tip
> >> of the remote branch.
> > 
> > My current propsal of making `git pull` by default do --ff-only would
> > solve this.
> 
> It would go a long way toward improving the situation, yes.
> 
> > In addition I think by default 'master' should be merged to
> > 'origin/master', if say --merge is given.
> 
> This would break cases #2 and #3.  (With cases #2 and #3 you want the
> fetched branch to be the second parent, not the first.)
> 
> Or are you proposing that pull --merge should reverse the parents if and
> only if the remote ref is @{u}?

Only if no remote or branch are specified `git pull --merge`.

> > 
> >> For this case, 'git pull --ff-only' followed by 'git rebase -p'
> >> works well, as does 'git pull --rebase=preserve' if the user is
> >> comfortable rebasing without reviewing the incoming commits first.
> > 
> > I suppose you mean a `git rebase -p` if the `git pull --ff-only` failed.
> 
> Yes.
> 
> > This might be OK on most projects, but not all.
> 
> The rebase only affects the local repository (the commits haven't been
> pushed yet or else they'd be in @{u} already), so I'd say it's more of
> an individual developer decision than a project decision.
> 
> In my opinion rebase would be the best option here, but if the project
> is OK with developers pushing merge or merge-there commits and the
> developer isn't yet comfortable with rebasing, then merge is also an
> acceptable option.

Precisely for that reason.

> >>  2. update a published feature branch with the latest changes from its
> >> parent branch

> > We probably shouldn't change that.
> 
> If we change 'git pull' to default to --ff-only but let 'git pull
> $remote [$refspec]' continue to default to --ff then we have two
> different behaviors depending on how 'git pull' is invoked.  I'm worried
> that this would trip up users.  I'm not convinced that having two
> different behaviors would be bad, but I'm not convinced that it would be
> good either.

It is the only solution that has been proposed.

Moreover, while it's a bit worrisome, it wouldn't create any actual
problems. Since `git pull $what` remains the same, there's no problems
there. The only change would be on `git pull`.

Since most users are not going to do `git pull $what` therefore it would
only be a small subset of users that would notice the discrepancy
between running with $what, or not. And the only discrepancy they could
notice is that when they run `git pull $what` they expect it to be
--ff-only, or when the run `git pull` they don't. Only the former could
be an issue, but even then, it's highly unlikely that `git pull $what`
would ever be a fast-forward.

So althought conceptually it doesn't look clean, in reality there
wouldn't be any problems.

> >>  3. integrate a more-or-less complete feature/fix back into the line
> >> of development it forked off of
> >>
> >> In this case the local branch is a primary line of development and
> >> the remote branch contains the derivative work.  Think Linus
> >> pulling in contributions.  Different situations will call for
> >> different ways to handle this case, but most will probably want
> >> some or all of:
> >>
> >>  * rebase the remote commits onto local HEAD
> > 
> > No. Most people will merge the remote branch as it is. There's no reason
> > to rebase, specially if you are creating a merge commit.
> 
> I disagree.  I prefer to rebase a topic branch before merging (no-ff) to
> the main line of development for a couple of reasons:

Well that is *your* preference. Most people would prefer to preserve the
history.

>   * It makes commits easier to review.

The review in the vast majority of cases happens *before* the
integration.

And the problem comes when the integrator makes a mistake, which they
inevitable do (we all do), then there's no history about how the
conflict was resolved, and what whas the original patch.

That's why most people don't do this.

>   * Rebasing makes the commit history pretty and easier to understand.

It is more important to be able to track integration errors than to have
a pretty history. That is for most people.

I like to have a

[PATCH 0/4] remote-hg: more improvements

2014-05-03 Thread Felipe Contreras
Here's a bunch of tests more, and a fixes for Mercurial v3.0.


Felipe Contreras (4):
  remote-hg: add more tests
  t: remote-hg: add file operation tests
  t: remote-hg: trivial cleanups and fixes
  remote-hg: add support for hg v3.0

 contrib/remote-helpers/git-remote-hg |   6 +-
 contrib/remote-helpers/test-hg.sh| 240 ++-
 2 files changed, 238 insertions(+), 8 deletions(-)

-- 
1.9.2+fc1.20.g204a630

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 3/4] t: remote-hg: trivial cleanups and fixes

2014-05-03 Thread Felipe Contreras
There was a broken && chain, and cat is simpler than echo in a subshell.

Signed-off-by: Felipe Contreras 
---
 contrib/remote-helpers/test-hg.sh | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/contrib/remote-helpers/test-hg.sh 
b/contrib/remote-helpers/test-hg.sh
index df09966..9b9468b 100755
--- a/contrib/remote-helpers/test-hg.sh
+++ b/contrib/remote-helpers/test-hg.sh
@@ -25,7 +25,7 @@ fi
 
 check () {
echo $3 >expected &&
-   git --git-dir=$1/.git log --format='%s' -1 $2 >actual
+   git --git-dir=$1/.git log --format='%s' -1 $2 >actual &&
test_cmp expected actual
 }
 
@@ -103,12 +103,12 @@ check_push () {
 }
 
 setup () {
-   (
-   echo "[ui]"
-   echo "username = H G Wells "
-   echo "[extensions]"
-   echo "mq ="
-   ) >>"$HOME"/.hgrc &&
+   cat >> "$HOME"/.hgrc <<-EOF &&
+   [ui]
+   username = H G Wells 
+   [extensions]
+   mq =
+   EOF
 
GIT_AUTHOR_DATE="2007-01-01 00:00:00 +0230" &&
GIT_COMMITTER_DATE="$GIT_AUTHOR_DATE" &&
-- 
1.9.2+fc1.20.g204a630

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 1/4] remote-hg: add more tests

2014-05-03 Thread Felipe Contreras
Inspired by the tests in gitifyhg.

One test is failing, but that's because of a limitation of
remote-helpers.

Signed-off-by: Felipe Contreras 
---
 contrib/remote-helpers/test-hg.sh | 150 ++
 1 file changed, 150 insertions(+)

diff --git a/contrib/remote-helpers/test-hg.sh 
b/contrib/remote-helpers/test-hg.sh
index 7d90056..33a434d 100755
--- a/contrib/remote-helpers/test-hg.sh
+++ b/contrib/remote-helpers/test-hg.sh
@@ -845,4 +845,154 @@ test_expect_success 'clone remote with generic null 
bookmark, then push to the b
)
 '
 
+test_expect_success 'notes' '
+   test_when_finished "rm -rf hgrepo gitrepo" &&
+
+   (
+   hg init hgrepo &&
+   cd hgrepo &&
+   echo one > content &&
+   hg add content &&
+   hg commit -m one &&
+   echo two > content &&
+   hg commit -m two
+   ) &&
+
+   git clone "hg::hgrepo" gitrepo &&
+   hg -R hgrepo log --template "{node}\n\n" > expected &&
+   git --git-dir=gitrepo/.git log --pretty="tformat:%N" --notes=hg > 
actual &&
+   test_cmp expected actual
+'
+
+test_expect_failure 'push updates notes' '
+   test_when_finished "rm -rf hgrepo gitrepo" &&
+
+   (
+   hg init hgrepo &&
+   cd hgrepo &&
+   echo one > content &&
+   hg add content &&
+   hg commit -m one
+   ) &&
+
+   git clone "hg::hgrepo" gitrepo &&
+
+   (
+   cd gitrepo &&
+   echo two > content &&
+   git commit -a -m two
+   git push
+   ) &&
+
+   hg -R hgrepo log --template "{node}\n\n" > expected &&
+   git --git-dir=gitrepo/.git log --pretty="tformat:%N" --notes=hg > 
actual &&
+   test_cmp expected actual
+'
+
+test_expect_success 'pull tags' '
+   test_when_finished "rm -rf hgrepo gitrepo" &&
+
+   (
+   hg init hgrepo &&
+   cd hgrepo &&
+   echo one > content &&
+   hg add content &&
+   hg commit -m one
+   ) &&
+
+   git clone "hg::hgrepo" gitrepo &&
+
+   (cd hgrepo && hg tag v1.0) &&
+   (cd gitrepo && git pull) &&
+
+   echo "v1.0" > expected &&
+   git --git-dir=gitrepo/.git tag > actual &&
+   test_cmp expected actual
+'
+
+test_expect_success 'push merged named branch' '
+   test_when_finished "rm -rf hgrepo gitrepo" &&
+
+   (
+   hg init hgrepo &&
+   cd hgrepo &&
+   echo one > content &&
+   hg add content &&
+   hg commit -m one &&
+   hg branch feature &&
+   echo two > content &&
+   hg commit -m two &&
+   hg update default &&
+   echo three > content &&
+   hg commit -m three
+   ) &&
+
+   (
+   git clone "hg::hgrepo" gitrepo &&
+   cd gitrepo &&
+   git merge -m Merge -Xtheirs origin/branches/feature &&
+   git push
+   ) &&
+
+   cat > expected <<-EOF
+   Merge
+   three
+   two
+   one
+   EOF
+   hg -R hgrepo log --template "{desc}\n" > actual &&
+   test_cmp expected actual
+'
+
+test_expect_success 'light tag sets author' '
+   test_when_finished "rm -rf hgrepo gitrepo" &&
+
+   (
+   hg init hgrepo &&
+   cd hgrepo &&
+   echo one > content &&
+   hg add content &&
+   hg commit -m one
+   ) &&
+
+   (
+   git clone "hg::hgrepo" gitrepo &&
+   cd gitrepo &&
+   git tag v1.0 &&
+   git push --tags
+   ) &&
+
+   echo "C O Mitter " > expected &&
+   hg -R hgrepo log --template "{author}\n" -r tip > actual &&
+   test_cmp expected actual
+'
+
+test_expect_success 'push tag different branch' '
+   test_when_finished "rm -rf hgrepo gitrepo" &&
+
+   (
+   hg init hgrepo &&
+   cd hgrepo &&
+   echo one > content &&
+   hg add content &&
+   hg commit -m one
+   hg branch feature &&
+   echo two > content &&
+   hg commit -m two
+   ) &&
+
+   (
+   git clone "hg::hgrepo" gitrepo &&
+   cd gitrepo &&
+   git branch &&
+   git checkout branches/feature &&
+   git tag v1.0 &&
+   git push --tags
+   ) &&
+
+   echo feature > expected &&
+   hg -R hgrepo log --template="{branch}\n" -r tip > actual &&
+   test_cmp expected actual
+'
+
 test_done
-- 
1.9.2+fc1.20.g204a630

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 2/4] t: remote-hg: add file operation tests

2014-05-03 Thread Felipe Contreras
Inspired by gitifyhg.

Signed-off-by: Felipe Contreras 
---
 contrib/remote-helpers/test-hg.sh | 76 +++
 1 file changed, 76 insertions(+)

diff --git a/contrib/remote-helpers/test-hg.sh 
b/contrib/remote-helpers/test-hg.sh
index 33a434d..df09966 100755
--- a/contrib/remote-helpers/test-hg.sh
+++ b/contrib/remote-helpers/test-hg.sh
@@ -53,6 +53,17 @@ check_bookmark () {
fi
 }
 
+check_files () {
+   git --git-dir=$1/.git ls-files > actual &&
+   if test $# -gt 1
+   then
+   printf "%s\n" "$2" > expected
+   else
+   > expected
+   fi &&
+   test_cmp expected actual
+}
+
 check_push () {
expected_ret=$1 ret=0 ref_ret=0
 
@@ -995,4 +1006,69 @@ test_expect_success 'push tag different branch' '
test_cmp expected actual
 '
 
+test_expect_success 'cloning a removed file works' '
+   test_when_finished "rm -rf hgrepo gitrepo" &&
+
+   (
+   hg init hgrepo &&
+   cd hgrepo &&
+
+   echo test > test_file &&
+   hg add test_file &&
+   hg commit -m add &&
+
+   hg rm test_file &&
+   hg commit -m remove
+   ) &&
+
+   git clone "hg::hgrepo" gitrepo &&
+   check_files gitrepo
+'
+
+test_expect_success 'cloning a file replaced with a directory' '
+   test_when_finished "rm -rf hgrepo gitrepo" &&
+
+   (
+   hg init hgrepo &&
+   cd hgrepo &&
+
+   echo test > dir_or_file &&
+   hg add dir_or_file &&
+   hg commit -m add &&
+
+   hg rm dir_or_file &&
+   mkdir dir_or_file &&
+   echo test > dir_or_file/test_file &&
+   hg add dir_or_file/test_file &&
+   hg commit -m replase
+   ) &&
+
+   git clone "hg::hgrepo" gitrepo &&
+   check_files gitrepo "dir_or_file/test_file"
+'
+
+test_expect_success 'clone replace directory with a file' '
+   test_when_finished "rm -rf hgrepo gitrepo" &&
+
+   (
+   hg init hgrepo &&
+   cd hgrepo &&
+
+   mkdir dir_or_file &&
+   echo test > dir_or_file/test_file &&
+   hg add dir_or_file/test_file &&
+   hg commit -m add &&
+
+   hg rm dir_or_file/test_file &&
+   echo test > dir_or_file &&
+   hg add dir_or_file &&
+   hg commit -m add &&
+
+   hg rm dir_or_file
+   ) &&
+
+   git clone "hg::hgrepo" gitrepo &&
+   check_files gitrepo "dir_or_file"
+'
+
 test_done
-- 
1.9.2+fc1.20.g204a630

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 4/4] remote-hg: add support for hg v3.0

2014-05-03 Thread Felipe Contreras
Signed-off-by: Felipe Contreras 
---
 contrib/remote-helpers/git-remote-hg | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/contrib/remote-helpers/git-remote-hg 
b/contrib/remote-helpers/git-remote-hg
index 34cda02..8b02803 100755
--- a/contrib/remote-helpers/git-remote-hg
+++ b/contrib/remote-helpers/git-remote-hg
@@ -13,6 +13,7 @@
 # "$GIT_DIR/hg/origin/clone/.hg/".
 
 from mercurial import hg, ui, bookmarks, context, encoding, node, error, 
extensions, discovery, util
+from mercurial import changegroup
 
 import re
 import sys
@@ -985,7 +986,10 @@ def push_unsafe(repo, remote, parsed_refs, p_revs):
 if not checkheads(repo, remote, p_revs):
 return None
 
-cg = repo.getbundle('push', heads=list(p_revs), common=common)
+if check_version(3, 0):
+cg = changegroup.getbundle(repo, 'push', heads=list(p_revs), 
common=common)
+else:
+cg = repo.getbundle('push', heads=list(p_revs), common=common)
 
 unbundle = remote.capable('unbundle')
 if unbundle:
-- 
1.9.2+fc1.20.g204a630

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Watchman support for git

2014-05-03 Thread Duy Nguyen
On Sun, May 4, 2014 at 3:49 AM, David Turner  wrote:
> On Sat, 2014-05-03 at 15:49 +0700, Duy Nguyen wrote:
>> On Sat, May 3, 2014 at 11:39 AM, David Turner  
>> wrote:
>> >> Index v4 and split index (and the following read-cache daemon,
>> >> hopefully)
>> >
>> > Looking at some of the archives for read-cache daemon, it seems to be
>> > somewhat similar to watchman, right?  But I only saw inotify code; what
>> > about Mac OS?  Or am I misunderstanding what it is?
>>
>> It's mentioned in [1], the second paragraph, mostly to hide index I/O
>> read cost and the SHA-1 hashing cost in the background. In theory it
>> should work on all platforms that support multiple processes and
>> efficient IPC. It can help load watchman file cache faster too.
>
> Yes, that seems like a good idea.
>
> I actually wrote some of a more-complicated, weirder version of this
> idea.  In my version, there was a long-running git daemon process that
> held the index, the watchman file cache, and also objects loaded from
> the object database.  Other git commands would then send their
> command-line and arguments over to the daemon, which would run the
> commands and send stdin/out/err back.  Of course, this is complicated
> because git commands are designed to run then exit, so they often rely
> on variables being initialized to zero, or fail to free memory.  I used
> the Boehm GC to handle the memory freeing problem.  To handle variables
> that needed to be reinitialized, I used __attribute__(section...) to put
> them all into one section, which I could save on daemon startup and
> restore after each command.  I also replaced calls to exit() with a
> function that called longjmp() so the daemon could survive commands
> failing.  Had I continued, I would also have had to track open file
> descriptors to avoid leaking those.
>
> This was a giant mess that only sort-of worked: it was difficult to
> track down all of the variables that needed to be reinitialized.
>
> The advantage of my method is that there was somewhat less data to
> marshall over IPC, and that objects could be easily cached; the
> disadvantage is complexity, massive code changes, and the fact that it
> didn't actually totally work at the time I ran out of time.
>
> So I'm really looking forward to trying your version!

Hm.. I may face the same problem if I'm not careful. So far I think
the daemon only holds index data (with on-disk format, not in-memory),
mainly to cut out SHA-1 hashing cost. This is still at the idea phase
for me though, nothing is materialized yet.

> I would like to merge the feature into master.  It works well for me,
> and some of my colleagues who have tried it out.

Have you tried to turn watchman on by default, then run it with git
test suite? That usually helps.

> I can split the vmac patch into two, but one of them will remain quite
> large because it contains the code for VMAC and AES, which total a bit
> over 100k.  Since the list will probably reject that, I'll post a link
> to a repository containing the patches.

With the read-cache deamon, I think hashing cost is less of an issue,
so new hashing algorithm becomes less important. If you store the file
cache in the deamon's memory only, there's no need to hash anything.
But I guess you already tried this.

> I'm not 100% sure how to split the watchman patch up.  I could add the
> fs_cache code and then separately add the watchman code that populates
> the cache.  Do you think there is a need to divide it up beyond this?

I'll need to have closer look at your patches to give any suggestions.
Although if you don't mind waiting a bit, I can try to put my
untracked cache patches in good shape (hopefully in 2 weeks), then you
can mostly avoid touching dir.c and reuse my work.

I backed away from watchman support because I was worried about its
overhead (of watchman itself, and git/watchman IPC because it's not
designed specifically for git), which led me to try optimizing git as
much as possible without watchman first, then see how/if watchman can
help on top of that. I still think it's a good approach (maybe because
it started to make me doubt if watchman could pull a big performance
win on top to justify the changes to support it)
-- 
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC PATCH 0/9] Use a structure for object IDs.

2014-05-03 Thread brian m. carlson
On Sat, May 03, 2014 at 08:12:13PM +, brian m. carlson wrote:
> This is a preliminary RFC patch series to move all the relevant uses of
> unsigned char [20] to struct object_id.  It should not be applied to any
> branch yet.
> 
> The goal of this series to improve type-checking in the codebase and to
> make it easier to move to a different hash function if the project
> decides to do that.  This series does not convert all of the codebase,
> but only parts.  I'm looking for feedback to see if there is consensus
> that this is the right direction before investing a large amount of
> time.

I would like to point out that to get something with as few calls as
hashclr converted to use struct object_id requires an insane amount of
work, because often major parts of several files have to be converted
first.  So the list should be aware that this will likely be an
extensive series, although it is bisectable, so it could theoretically
be done in batches.

-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187


signature.asc
Description: Digital signature


Re: [PATCH] subtree/Makefile: Standardize (esp. for packagers)

2014-05-03 Thread James Denholm
On 4 May 2014 05:22:48 GMT+10:00, Felipe Contreras  
wrote:
>I think you should take a look at the Makefile of
>contrib/remote-helpers. I bet something simple like that would work
>just
>fine for subtree.

The current makefile is simple enough, just quirky and likes
to be a special snowflake. 'sall good, the v2 addresses most
of my immediate concerns with it.

Regards,
James Denholm.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Pull is Mostly Evil

2014-05-03 Thread Richard Hansen
On 2014-05-03 05:26, Felipe Contreras wrote:
> Richard Hansen wrote:
> 
>> I think the fundamental difference is in the relationship between the
>> local and the remote branch (which branch derives from the other).
>> The relationship between the branches determines what the user wants
>> from 'git pull'.
>>
>> In my experience 'git pull' is mostly (only?) used for the following
>> three tasks:
> 
> I agree.
> 
>>  1. update a local branch to incorporate the latest upstream changes
>>
>> In this case, the local branch (master) is a derivative of the
>> upstream branch (origin/master).  The user wants all of the
>> commits in the remote branch to be in the local branch.  And the
>> user would like the local changes, if any, to descend from the tip
>> of the remote branch.
> 
> My current propsal of making `git pull` by default do --ff-only would
> solve this.

It would go a long way toward improving the situation, yes.

> In addition I think by default 'master' should be merged to
> 'origin/master', if say --merge is given.

This would break cases #2 and #3.  (With cases #2 and #3 you want the
fetched branch to be the second parent, not the first.)

Or are you proposing that pull --merge should reverse the parents if and
only if the remote ref is @{u}?

> 
>> For this case, 'git pull --ff-only' followed by 'git rebase -p'
>> works well, as does 'git pull --rebase=preserve' if the user is
>> comfortable rebasing without reviewing the incoming commits first.
> 
> I suppose you mean a `git rebase -p` if the `git pull --ff-only` failed.

Yes.

> This might be OK on most projects, but not all.

The rebase only affects the local repository (the commits haven't been
pushed yet or else they'd be in @{u} already), so I'd say it's more of
an individual developer decision than a project decision.

In my opinion rebase would be the best option here, but if the project
is OK with developers pushing merge or merge-there commits and the
developer isn't yet comfortable with rebasing, then merge is also an
acceptable option.

> 
> What happens after a `git pull --ff-only` fails should be totally
> up to the user.

I tend to agree, mostly because I want users to have an opportunity to
review incoming commits before action is taken.  Also, though rebasing
would yield the nicest history, some users aren't yet comfortable with
rebase.  If a project is OK with silly little merge commits from users
that aren't comfortable with rebase, then I don't want to force everyone
to rebase by default.

As an added bonus:  Defaulting to --ff-only makes it possible for 'git
pull --all' to fast-forward every local branch to their configured
upstream, not just the currently checked-out branch.  I think this would
be a huge usability win.

> 
>>  2. update a published feature branch with the latest changes from its
>> parent branch
>>
>> In this case, the local branch (foo) is a derivative of the
>> upstream branch (origin/foo) which is itself a derivative of
>> another branch (origin/master).  All commits in origin/master
>> should be in origin/foo, and ideally all commits unique to
>> origin/foo would descend from the tip of origin/master.
> 
> I don't understand why are you tainting the example with 'origin/foo',

Originally I didn't have this case in my list, but I added it after
thinking about Peff's comment:

  On 2014-05-02 17:48, Jeff King wrote:
  > One common workflow for GitHub users is to back-merge master into a
  > topic, because they want the final "integrated" version on the topic
  > branch.

This almost but doesn't quite fit neatly into the other two cases.  It's
not case #1 because the shared nature of origin/foo means that rebasing
origin/foo onto origin/master is usually bad instead of usually good.
It's not case #3 because rebasing origin/master commits onto origin/foo
(assuming that the user would usually want to rebase the topic branch
when integrating) would definitely be bad.

> 'foo' and 'origin/master' are enough for this example. In fact, the
> mention of 'origin/master' made it wrong: after the pull not all the
> commits of origin/master would be in origin/foo, you need a push for
> that.

The push of foo to origin/foo was meant to be implied.

> We have enough in our plate to taint this with yet another branch
> and push.
> 
> For this case `git pull origin master` already work correctly for most
> projects.

Yes, it does.

> We probably shouldn't change that.

If we change 'git pull' to default to --ff-only but let 'git pull
$remote [$refspec]' continue to default to --ff then we have two
different behaviors depending on how 'git pull' is invoked.  I'm worried
that this would trip up users.  I'm not convinced that having two
different behaviors would be bad, but I'm not convinced that it would be
good either.

> 
>>  3. integrate a more-or-less complete feature/fix back into the line
>> of development it forked off of
>>
>> In this case the local bra

Re: Watchman support for git

2014-05-03 Thread David Turner
On Sat, 2014-05-03 at 15:49 +0700, Duy Nguyen wrote:
> On Sat, May 3, 2014 at 11:39 AM, David Turner  
> wrote:
> >> Index v4 and split index (and the following read-cache daemon,
> >> hopefully)
> >
> > Looking at some of the archives for read-cache daemon, it seems to be
> > somewhat similar to watchman, right?  But I only saw inotify code; what
> > about Mac OS?  Or am I misunderstanding what it is?
> 
> It's mentioned in [1], the second paragraph, mostly to hide index I/O
> read cost and the SHA-1 hashing cost in the background. In theory it
> should work on all platforms that support multiple processes and
> efficient IPC. It can help load watchman file cache faster too.

Yes, that seems like a good idea.

I actually wrote some of a more-complicated, weirder version of this
idea.  In my version, there was a long-running git daemon process that
held the index, the watchman file cache, and also objects loaded from
the object database.  Other git commands would then send their
command-line and arguments over to the daemon, which would run the
commands and send stdin/out/err back.  Of course, this is complicated
because git commands are designed to run then exit, so they often rely
on variables being initialized to zero, or fail to free memory.  I used
the Boehm GC to handle the memory freeing problem.  To handle variables
that needed to be reinitialized, I used __attribute__(section...) to put
them all into one section, which I could save on daemon startup and
restore after each command.  I also replaced calls to exit() with a
function that called longjmp() so the daemon could survive commands
failing.  Had I continued, I would also have had to track open file
descriptors to avoid leaking those.

This was a giant mess that only sort-of worked: it was difficult to
track down all of the variables that needed to be reinitialized. 

The advantage of my method is that there was somewhat less data to
marshall over IPC, and that objects could be easily cached; the
disadvantage is complexity, massive code changes, and the fact that it
didn't actually totally work at the time I ran out of time. 

So I'm really looking forward to trying your version!

> >> The last line could be a competition between watchman and my coming
> >> "untracked cache" series. I expect to cut the number in that line at
> >> least in half without external dependency.
> >
> > I hadn't seen the "untracked cached" work (I actually finished these
> > patches a month or so ago but have been waiting for some internal
> > reviews before sending them out).  Looks interesting.  It seems we use a
> > similar strategy for handling ignores.
> 
> Yep, mostly the same at the core, except that I exploit directory
> mtime while you use inotify. Each approach has its own pros and cons,
> I think. Both should face the same traps in caching (e.g. if you "git
> rm --cached" a file, that file could be come either untracked, or
> ignored).
> 
> >> Patch 2/3 did not seem to make it to the list by the way..
> >
> > Thanks for your comments.  I just tried again to send patch 2/3.  I do
> > actually see the CC of it in my @twitter.com mailbox, but I don't see it
> > in the archives on the web.  Do you know if there is a reason the
> > mailing list would reject it?
> 
> Probably its size, 131K, which is also an indicator to split it (and
> the third patch) into smaller patches if you want to merge this
> feature in master eventually.

I would like to merge the feature into master.  It works well for me,
and some of my colleagues who have tried it out.

I can split the vmac patch into two, but one of them will remain quite
large because it contains the code for VMAC and AES, which total a bit
over 100k.  Since the list will probably reject that, I'll post a link
to a repository containing the patches.

I'm not 100% sure how to split the watchman patch up.  I could add the
fs_cache code and then separately add the watchman code that populates
the cache.  Do you think there is a need to divide it up beyond this?

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Pull is Mostly Evil

2014-05-03 Thread Philip Oakley

From: "Jonathan Nieder" 
Sent: Friday, May 02, 2014 11:53 PM

Hi,

Philip Oakley wrote:


That assumes that [git pull] doing something is better than doing
nothing,
which is appropriate when the costs on either side are roughly
similar.


I think the conversation's going around in circles.


I agree it's going around, but it's a non-exact recurrence. Issues are
being surfaced.


Potential next steps:

a. Documentation or test patch illustrating desired behavior

b. More traditional formal design doc explaining desired behavior and
   the thinking behind it ("problem", "overview of solution",
   "alternatives rejected", "complications", "example", "open
   questions").

c. Implementation patch

d. Someone takes an existing patch and figures out the next step
   toward getting it ready for application.

My preference is for (a), I guess.


I disagree about the leap to the presentation & discussion of a
'solution' in these awkward scenarios (the old joke about "if I were you
I wouldn't start from here", when asking for directions tends to apply).
This is the same point made by Brooks in the 'Mythical Man Month'. A
leap to code is no guarantee of success.



The point being that something more concrete (code or a design doc)
makes it easier to avoid talking past each other.  And having
something concrete to edit makes the stakes clearer so people can make
it incrementally better without being distracted by unimportant parts.


We've had Junio's training wheel, and now Filipe's n'th attempt at code
examples, so my bad code wouldn't help ;-). As a systems engineer I've
seen these confusions quite a few times in different guises.

I tend to fall back to P Checkland's "Systems Thinking, Systems
Practice" model of the various processes that have to go on [1] to
improve the situation (note he doesn't expect a solved solution in most
cases, just an improvement in the situation). At the moment most of the
discussion is in the "unstructured" parts of the processes. He also
identifies 6 elements 'CATWOE' [2] that need to be considered when
studying these problems.

Most of the discussion/arguments here are about the different
'Weltanshaung's" (world views) of the contributors.

In terms of the new user pull problem, what needs to be modeled is the
new user's and their weltanshaung, not how we ('experienced' users?)
might 'solve' the problem.

The pull problem is, I believe part of the bigger problem of the
mind-set shift required for the transition to a DVCS for most new users.
Git has grown organically, so still has some soft (unclear) edges, which
probably needs more than just a transition plan for Filipe's pull
changes, and its choice of the final default (or lack of).

For example, if users aren't understanding the differences between
remote branches, remote tracking branches, and branches, which is part
of the pull problem; have we made it easy for them to understand? [They
already have to comprehend the 'staging' concept, so are already
cognitively fully loaded].

For the branch type example, some cleaner naming may help, such as:
'remote branch', 'Tracking branch', and '(local) branch', which excludes
the noiseword 'remote' from 'Tracking branches' (my deliberate 'T'
emphasis). Though that does still leave the confusion between remote
servers and remote repos, where the latter may actually be local, and if
a file path, be the local '.' repo itself!



Thanks and hope that helps,


Sorry if this went off at a tangent, but I believe it's important to get
to the bottom of the new user problems, which are deeper than just a few 
command defaults.



Jonathan
--


Philip
--
[1]
http://40qx6d15vq6j25i83v3ks8nxfux.wpengine.netdna-cdn.com/files/2012/08/seven-steps2.gif
or http://portals.wi.wur.nl/spicad/?Soft_Systems_Methodology Checkland's
7 Steps.

[2] CATWOE: customers, actors, transformation, weltanshaung, owners,
environment.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 8/9] cache-tree: convert struct cache_tree to use object_id

2014-05-03 Thread brian m. carlson
Signed-off-by: brian m. carlson 
---
 builtin/commit.c   |  2 +-
 builtin/fsck.c |  4 ++--
 cache-tree.c   | 30 +++---
 cache-tree.h   |  3 ++-
 merge-recursive.c  |  2 +-
 reachable.c|  2 +-
 sequencer.c|  2 +-
 test-dump-cache-tree.c |  4 ++--
 8 files changed, 25 insertions(+), 24 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index 9cfef6c..639f843 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1659,7 +1659,7 @@ int cmd_commit(int argc, const char **argv, const char 
*prefix)
append_merge_tag_headers(parents, &tail);
}
 
-   if (commit_tree_extended(&sb, active_cache_tree->sha1, parents, sha1,
+   if (commit_tree_extended(&sb, active_cache_tree->sha1.oid, parents, 
sha1,
 author_ident.buf, sign_commit, extra)) {
rollback_index_files();
die(_("failed to write commit object"));
diff --git a/builtin/fsck.c b/builtin/fsck.c
index fc150c8..6854c81 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -587,10 +587,10 @@ static int fsck_cache_tree(struct cache_tree *it)
fprintf(stderr, "Checking cache tree\n");
 
if (0 <= it->entry_count) {
-   struct object *obj = parse_object(it->sha1);
+   struct object *obj = parse_object(it->sha1.oid);
if (!obj) {
error("%s: invalid sha1 pointer in cache-tree",
- sha1_to_hex(it->sha1));
+ sha1_to_hex(it->sha1.oid));
return 1;
}
obj->used = 1;
diff --git a/cache-tree.c b/cache-tree.c
index 7fa524a..b7b2d06 100644
--- a/cache-tree.c
+++ b/cache-tree.c
@@ -219,7 +219,7 @@ int cache_tree_fully_valid(struct cache_tree *it)
int i;
if (!it)
return 0;
-   if (it->entry_count < 0 || !has_sha1_file(it->sha1))
+   if (it->entry_count < 0 || !has_sha1_file(it->sha1.oid))
return 0;
for (i = 0; i < it->subtree_nr; i++) {
if (!cache_tree_fully_valid(it->down[i]->cache_tree))
@@ -244,7 +244,7 @@ static int update_one(struct cache_tree *it,
 
*skip_count = 0;
 
-   if (0 <= it->entry_count && has_sha1_file(it->sha1))
+   if (0 <= it->entry_count && has_sha1_file(it->sha1.oid))
return it->entry_count;
 
/*
@@ -311,7 +311,7 @@ static int update_one(struct cache_tree *it,
struct cache_tree_sub *sub;
const char *path, *slash;
int pathlen, entlen;
-   const unsigned char *sha1;
+   const struct object_id *sha1;
unsigned mode;
 
path = ce->name;
@@ -327,21 +327,21 @@ static int update_one(struct cache_tree *it,
die("cache-tree.c: '%.*s' in '%s' not found",
entlen, path + baselen, path);
i += sub->count;
-   sha1 = sub->cache_tree->sha1;
+   sha1 = &sub->cache_tree->sha1;
mode = S_IFDIR;
if (sub->cache_tree->entry_count < 0)
to_invalidate = 1;
}
else {
-   sha1 = ce->sha1;
+   sha1 = (struct object_id *)ce->sha1;
mode = ce->ce_mode;
entlen = pathlen - baselen;
i++;
}
-   if (mode != S_IFGITLINK && !missing_ok && !has_sha1_file(sha1)) 
{
+   if (mode != S_IFGITLINK && !missing_ok && 
!has_sha1_file(sha1->oid)) {
strbuf_release(&buffer);
return error("invalid object %06o %s for '%.*s'",
-   mode, sha1_to_hex(sha1), entlen+baselen, path);
+   mode, sha1_to_hex(sha1->oid), entlen+baselen, 
path);
}
 
/*
@@ -375,8 +375,8 @@ static int update_one(struct cache_tree *it,
}
 
if (dryrun)
-   hash_sha1_file(buffer.buf, buffer.len, tree_type, it->sha1);
-   else if (write_sha1_file(buffer.buf, buffer.len, tree_type, it->sha1)) {
+   hash_sha1_file(buffer.buf, buffer.len, tree_type, it->sha1.oid);
+   else if (write_sha1_file(buffer.buf, buffer.len, tree_type, 
it->sha1.oid)) {
strbuf_release(&buffer);
return -1;
}
@@ -432,7 +432,7 @@ static void write_one(struct strbuf *buffer, struct 
cache_tree *it,
 #endif
 
if (0 <= it->entry_count) {
-   strbuf_add(buffer, it->sha1, 20);
+   strbuf_add(buffer, it->sha1.oid, GIT_OID_RAWSZ);
}
for (i = 0; i < it->subtree_nr; i++) {
struct cache_tree_sub *down = it->down[i];
@@ -489,7 +4

[PATCH 7/9] bundle.c: convert leaf functions to struct object_id

2014-05-03 Thread brian m. carlson
Signed-off-by: brian m. carlson 
---
 bundle.c | 38 +++---
 1 file changed, 19 insertions(+), 19 deletions(-)

diff --git a/bundle.c b/bundle.c
index 1222952..798ba28 100644
--- a/bundle.c
+++ b/bundle.c
@@ -11,11 +11,11 @@
 
 static const char bundle_signature[] = "# v2 git bundle\n";
 
-static void add_to_ref_list(const unsigned char *sha1, const char *name,
+static void add_to_ref_list(const struct object_id *sha1, const char *name,
struct ref_list *list)
 {
ALLOC_GROW(list->list, list->nr + 1, list->alloc);
-   hashcpy(list->list[list->nr].sha1, sha1);
+   hashcpy(list->list[list->nr].sha1, sha1->oid);
list->list[list->nr].name = xstrdup(name);
list->nr++;
 }
@@ -39,7 +39,7 @@ static int parse_bundle_header(int fd, struct bundle_header 
*header,
/* The bundle header ends with an empty line */
while (!strbuf_getwholeline_fd(&buf, fd, '\n') &&
   buf.len && buf.buf[0] != '\n') {
-   unsigned char sha1[20];
+   struct object_id sha1;
int is_prereq = 0;
 
if (*buf.buf == '-') {
@@ -53,9 +53,9 @@ static int parse_bundle_header(int fd, struct bundle_header 
*header,
 * Prerequisites have object name that is optionally
 * followed by SP and subject line.
 */
-   if (get_sha1_hex(buf.buf, sha1) ||
-   (buf.len > 40 && !isspace(buf.buf[40])) ||
-   (!is_prereq && buf.len <= 40)) {
+   if (get_sha1_hex(buf.buf, sha1.oid) ||
+   (buf.len > GIT_OID_HEXSZ && 
!isspace(buf.buf[GIT_OID_HEXSZ])) ||
+   (!is_prereq && buf.len <= GIT_OID_HEXSZ)) {
if (report_path)
error(_("unrecognized header: %s%s (%d)"),
  (is_prereq ? "-" : ""), buf.buf, 
(int)buf.len);
@@ -63,9 +63,9 @@ static int parse_bundle_header(int fd, struct bundle_header 
*header,
break;
} else {
if (is_prereq)
-   add_to_ref_list(sha1, "", 
&header->prerequisites);
+   add_to_ref_list(&sha1, "", 
&header->prerequisites);
else
-   add_to_ref_list(sha1, buf.buf + 41, 
&header->references);
+   add_to_ref_list(&sha1, buf.buf + 41, 
&header->references);
}
}
 
@@ -274,16 +274,16 @@ int create_bundle(struct bundle_header *header, const 
char *path,
return -1;
rls_fout = xfdopen(rls.out, "r");
while (strbuf_getwholeline(&buf, rls_fout, '\n') != EOF) {
-   unsigned char sha1[20];
+   struct object_id sha1;
if (buf.len > 0 && buf.buf[0] == '-') {
write_or_die(bundle_fd, buf.buf, buf.len);
-   if (!get_sha1_hex(buf.buf + 1, sha1)) {
-   struct object *object = 
parse_object_or_die(sha1, buf.buf);
+   if (!get_sha1_hex(buf.buf + 1, sha1.oid)) {
+   struct object *object = 
parse_object_or_die(sha1.oid, buf.buf);
object->flags |= UNINTERESTING;
add_pending_object(&revs, object, buf.buf);
}
-   } else if (!get_sha1_hex(buf.buf, sha1)) {
-   struct object *object = parse_object_or_die(sha1, 
buf.buf);
+   } else if (!get_sha1_hex(buf.buf, sha1.oid)) {
+   struct object *object = parse_object_or_die(sha1.oid, 
buf.buf);
object->flags |= SHOWN;
}
}
@@ -302,16 +302,16 @@ int create_bundle(struct bundle_header *header, const 
char *path,
 
for (i = 0; i < revs.pending.nr; i++) {
struct object_array_entry *e = revs.pending.objects + i;
-   unsigned char sha1[20];
+   struct object_id sha1;
char *ref;
const char *display_ref;
int flag;
 
if (e->item->flags & UNINTERESTING)
continue;
-   if (dwim_ref(e->name, strlen(e->name), sha1, &ref) != 1)
+   if (dwim_ref(e->name, strlen(e->name), sha1.oid, &ref) != 1)
continue;
-   if (read_ref_full(e->name, sha1, 1, &flag))
+   if (read_ref_full(e->name, sha1.oid, 1, &flag))
flag = 0;
display_ref = (flag & REF_ISSYMREF) ? e->name : ref;
 
@@ -342,13 +342,13 @@ int create_bundle(struct bundle_header *header, const 
char *path,
 * commit that is referenced by the tag, and not the tag
 * itself.
 */
-   if (hashcmp(sha1, e->item->sha1)) {
+ 

[RFC PATCH 0/9] Use a structure for object IDs.

2014-05-03 Thread brian m. carlson
This is a preliminary RFC patch series to move all the relevant uses of
unsigned char [20] to struct object_id.  It should not be applied to any
branch yet.

The goal of this series to improve type-checking in the codebase and to
make it easier to move to a different hash function if the project
decides to do that.  This series does not convert all of the codebase,
but only parts.  I'm looking for feedback to see if there is consensus
that this is the right direction before investing a large amount of
time.

Certain parts of the code have to be converted before others to keep the
patch sizes small, maintainable, and bisectable, so functions and
structures that are used across the codebase (e.g. hashcmp and struct
object) will be converted later.  Conversion has been done in a roughly
alphabetical order by name of file.

The constants for raw and hex sizes of SHA-1 values are maintained.
These constants are used where the quantity is the size of an SHA-1
value, and sizeof(struct object_id) is used wherever memory is to be
allocated.  This is done to permit the struct to turn into a union later
if multiple hashes are supported.  I left the names at GIT_OID_RAWSZ and
GIT_OID_HEXSZ because that's what libgit2 uses and what Junio seemed to
prefer, but they can be changed later if there's a desire to do that.

I called the structure member "oid" because it was easily grepable and
distinct from the rest of the codebase.  It, too, can be changed if we
decide on a better name.  I specifically did not choose "sha1" since it
looks weird to have "sha1->sha1" and I didn't want to rename lots of
variables.

Comments?

brian m. carlson (9):
  Define a structure for object IDs.
  bisect.c: convert to use struct object_id
  archive.c: convert to use struct object_id
  zip: use GIT_OID_HEXSZ for trailers
  branch.c: convert to use struct object_id
  bulk-checkin.c: convert to use struct object_id
  bundle.c: convert leaf functions to struct object_id
  cache-tree: convert struct cache_tree to use object_id
  diff: convert struct combine_diff_path to object_id

 archive-zip.c  |  4 ++--
 archive.c  | 16 +++
 archive.h  |  1 +
 bisect.c   | 30 ++--
 branch.c   | 16 +++
 builtin/commit.c   |  2 +-
 builtin/fsck.c |  4 ++--
 bulk-checkin.c | 12 +--
 bundle.c   | 38 +--
 cache-tree.c   | 30 ++--
 cache-tree.h   |  3 ++-
 combine-diff.c | 54 +-
 diff-lib.c | 10 +-
 diff.h |  5 +++--
 merge-recursive.c  |  2 +-
 object.h   | 13 +++-
 reachable.c|  2 +-
 sequencer.c|  2 +-
 test-dump-cache-tree.c |  4 ++--
 19 files changed, 131 insertions(+), 117 deletions(-)

-- 
2.0.0.rc0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 9/9] diff: convert struct combine_diff_path to object_id

2014-05-03 Thread brian m. carlson
Signed-off-by: brian m. carlson 
---
 combine-diff.c | 54 +++---
 diff-lib.c | 10 +-
 diff.h |  5 +++--
 3 files changed, 35 insertions(+), 34 deletions(-)

diff --git a/combine-diff.c b/combine-diff.c
index 24ca7e2..f97eb3a 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -34,9 +34,9 @@ static struct combine_diff_path *intersect_paths(struct 
combine_diff_path *curr,
memset(p->parent, 0,
   sizeof(p->parent[0]) * num_parent);
 
-   hashcpy(p->sha1, q->queue[i]->two->sha1);
+   hashcpy(p->sha1.oid, q->queue[i]->two->sha1);
p->mode = q->queue[i]->two->mode;
-   hashcpy(p->parent[n].sha1, q->queue[i]->one->sha1);
+   hashcpy(p->parent[n].sha1.oid, q->queue[i]->one->sha1);
p->parent[n].mode = q->queue[i]->one->mode;
p->parent[n].status = q->queue[i]->status;
*tail = p;
@@ -67,7 +67,7 @@ static struct combine_diff_path *intersect_paths(struct 
combine_diff_path *curr,
continue;
}
 
-   hashcpy(p->parent[n].sha1, q->queue[i]->one->sha1);
+   hashcpy(p->parent[n].sha1.oid, q->queue[i]->one->sha1);
p->parent[n].mode = q->queue[i]->one->mode;
p->parent[n].status = q->queue[i]->status;
 
@@ -274,7 +274,7 @@ static struct lline *coalesce_lines(struct lline *base, int 
*lenbase,
return base;
 }
 
-static char *grab_blob(const unsigned char *sha1, unsigned int mode,
+static char *grab_blob(const struct object_id *sha1, unsigned int mode,
   unsigned long *size, struct userdiff_driver *textconv,
   const char *path)
 {
@@ -284,20 +284,20 @@ static char *grab_blob(const unsigned char *sha1, 
unsigned int mode,
if (S_ISGITLINK(mode)) {
blob = xmalloc(100);
*size = snprintf(blob, 100,
-"Subproject commit %s\n", sha1_to_hex(sha1));
-   } else if (is_null_sha1(sha1)) {
+"Subproject commit %s\n", 
sha1_to_hex(sha1->oid));
+   } else if (is_null_sha1(sha1->oid)) {
/* deleted blob */
*size = 0;
return xcalloc(1, 1);
} else if (textconv) {
struct diff_filespec *df = alloc_filespec(path);
-   fill_filespec(df, sha1, 1, mode);
+   fill_filespec(df, sha1->oid, 1, mode);
*size = fill_textconv(textconv, df, &blob);
free_filespec(df);
} else {
-   blob = read_sha1_file(sha1, &type, size);
+   blob = read_sha1_file(sha1->oid, &type, size);
if (type != OBJ_BLOB)
-   die("object '%s' is not a blob!", sha1_to_hex(sha1));
+   die("object '%s' is not a blob!", 
sha1_to_hex(sha1->oid));
}
return blob;
 }
@@ -379,7 +379,7 @@ static void consume_line(void *state_, char *line, unsigned 
long len)
}
 }
 
-static void combine_diff(const unsigned char *parent, unsigned int mode,
+static void combine_diff(const struct object_id *parent, unsigned int mode,
 mmfile_t *result_file,
 struct sline *sline, unsigned int cnt, int n,
 int num_parent, int result_deleted,
@@ -904,11 +904,11 @@ static void show_combined_header(struct combine_diff_path 
*elem,
 "", elem->path, line_prefix, c_meta, c_reset);
printf("%s%sindex ", line_prefix, c_meta);
for (i = 0; i < num_parent; i++) {
-   abb = find_unique_abbrev(elem->parent[i].sha1,
+   abb = find_unique_abbrev(elem->parent[i].sha1.oid,
 abbrev);
printf("%s%s", i ? "," : "", abb);
}
-   abb = find_unique_abbrev(elem->sha1, abbrev);
+   abb = find_unique_abbrev(elem->sha1.oid, abbrev);
printf("..%s%s\n", abb, c_reset);
 
if (mode_differs) {
@@ -981,7 +981,7 @@ static void show_patch_diff(struct combine_diff_path *elem, 
int num_parent,
 
/* Read the result of merge first */
if (!working_tree_file)
-   result = grab_blob(elem->sha1, elem->mode, &result_size,
+   result = grab_blob(&elem->sha1, elem->mode, &result_size,
   textconv, elem->path);
else {
/* Used by diff-tree to read from the working tree */
@@ -1003,12 +1003,12 @@ static void show_patch_diff(struct combine_diff_path 
*elem, int num_parent,
result = strbuf_detach(&buf, NULL);
elem->mode = canon_mode(st.st_mode);
} else if (S_ISDIR(st.st_mode)) {
-   unsigned char sha1[20];

[PATCH 2/9] bisect.c: convert to use struct object_id

2014-05-03 Thread brian m. carlson
Signed-off-by: brian m. carlson 
---
 bisect.c | 30 +++---
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/bisect.c b/bisect.c
index d6e851d..fe53214 100644
--- a/bisect.c
+++ b/bisect.c
@@ -15,7 +15,7 @@
 static struct sha1_array good_revs;
 static struct sha1_array skipped_revs;
 
-static unsigned char *current_bad_sha1;
+static struct object_id *current_bad_sha1;
 
 static const char *argv_checkout[] = {"checkout", "-q", NULL, "--", NULL};
 static const char *argv_show_branch[] = {"show-branch", NULL, NULL};
@@ -403,8 +403,8 @@ static int register_ref(const char *refname, const unsigned 
char *sha1,
int flags, void *cb_data)
 {
if (!strcmp(refname, "bad")) {
-   current_bad_sha1 = xmalloc(20);
-   hashcpy(current_bad_sha1, sha1);
+   current_bad_sha1 = xmalloc(sizeof(*current_bad_sha1));
+   hashcpy(current_bad_sha1->oid, sha1);
} else if (starts_with(refname, "good-")) {
sha1_array_append(&good_revs, sha1);
} else if (starts_with(refname, "skip-")) {
@@ -563,7 +563,7 @@ static struct commit_list *skip_away(struct commit_list 
*list, int count)
 
for (i = 0; cur; cur = cur->next, i++) {
if (i == index) {
-   if (hashcmp(cur->item->object.sha1, current_bad_sha1))
+   if (hashcmp(cur->item->object.sha1, 
current_bad_sha1->oid))
return cur;
if (previous)
return previous;
@@ -606,7 +606,7 @@ static void bisect_rev_setup(struct rev_info *revs, const 
char *prefix,
 
/* rev_argv.argv[0] will be ignored by setup_revisions */
argv_array_push(&rev_argv, "bisect_rev_setup");
-   argv_array_pushf(&rev_argv, bad_format, sha1_to_hex(current_bad_sha1));
+   argv_array_pushf(&rev_argv, bad_format, 
sha1_to_hex(current_bad_sha1->oid));
for (i = 0; i < good_revs.nr; i++)
argv_array_pushf(&rev_argv, good_format,
 sha1_to_hex(good_revs.sha1[i]));
@@ -627,7 +627,7 @@ static void bisect_common(struct rev_info *revs)
 }
 
 static void exit_if_skipped_commits(struct commit_list *tried,
-   const unsigned char *bad)
+   const struct object_id *bad)
 {
if (!tried)
return;
@@ -636,12 +636,12 @@ static void exit_if_skipped_commits(struct commit_list 
*tried,
   "The first bad commit could be any of:\n");
print_commit_list(tried, "%s\n", "%s\n");
if (bad)
-   printf("%s\n", sha1_to_hex(bad));
+   printf("%s\n", sha1_to_hex(bad->oid));
printf("We cannot bisect more!\n");
exit(2);
 }
 
-static int is_expected_rev(const unsigned char *sha1)
+static int is_expected_rev(const struct object_id *sha1)
 {
const char *filename = git_path("BISECT_EXPECTED_REV");
struct stat st;
@@ -657,7 +657,7 @@ static int is_expected_rev(const unsigned char *sha1)
return 0;
 
if (strbuf_getline(&str, fp, '\n') != EOF)
-   res = !strcmp(str.buf, sha1_to_hex(sha1));
+   res = !strcmp(str.buf, sha1_to_hex(sha1->oid));
 
strbuf_release(&str);
fclose(fp);
@@ -718,7 +718,7 @@ static struct commit **get_bad_and_good_commits(int *rev_nr)
struct commit **rev = xmalloc(len * sizeof(*rev));
int i, n = 0;
 
-   rev[n++] = get_commit_reference(current_bad_sha1);
+   rev[n++] = get_commit_reference(current_bad_sha1->oid);
for (i = 0; i < good_revs.nr; i++)
rev[n++] = get_commit_reference(good_revs.sha1[i]);
*rev_nr = n;
@@ -729,7 +729,7 @@ static struct commit **get_bad_and_good_commits(int *rev_nr)
 static void handle_bad_merge_base(void)
 {
if (is_expected_rev(current_bad_sha1)) {
-   char *bad_hex = sha1_to_hex(current_bad_sha1);
+   char *bad_hex = sha1_to_hex(current_bad_sha1->oid);
char *good_hex = join_sha1_array_hex(&good_revs, ' ');
 
fprintf(stderr, "The merge base %s is bad.\n"
@@ -749,7 +749,7 @@ static void handle_bad_merge_base(void)
 static void handle_skipped_merge_base(const unsigned char *mb)
 {
char *mb_hex = sha1_to_hex(mb);
-   char *bad_hex = sha1_to_hex(current_bad_sha1);
+   char *bad_hex = sha1_to_hex(current_bad_sha1->oid);
char *good_hex = join_sha1_array_hex(&good_revs, ' ');
 
warning("the merge base between %s and [%s] "
@@ -780,7 +780,7 @@ static void check_merge_bases(int no_checkout)
 
for (; result; result = result->next) {
const unsigned char *mb = result->item->object.sha1;
-   if (!hashcmp(mb, current_bad_sha1)) {
+   if (!hashcmp(mb, current_bad_sha1->oid)) {
handle_bad_merge_base();
} 

[PATCH 6/9] bulk-checkin.c: convert to use struct object_id

2014-05-03 Thread brian m. carlson
Signed-off-by: brian m. carlson 
---
 bulk-checkin.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/bulk-checkin.c b/bulk-checkin.c
index 98e651c..92c7b5e 100644
--- a/bulk-checkin.c
+++ b/bulk-checkin.c
@@ -23,7 +23,7 @@ static struct bulk_checkin_state {
 
 static void finish_bulk_checkin(struct bulk_checkin_state *state)
 {
-   unsigned char sha1[20];
+   struct object_id sha1;
struct strbuf packname = STRBUF_INIT;
int i;
 
@@ -35,11 +35,11 @@ static void finish_bulk_checkin(struct bulk_checkin_state 
*state)
unlink(state->pack_tmp_name);
goto clear_exit;
} else if (state->nr_written == 1) {
-   sha1close(state->f, sha1, CSUM_FSYNC);
+   sha1close(state->f, sha1.oid, CSUM_FSYNC);
} else {
-   int fd = sha1close(state->f, sha1, 0);
-   fixup_pack_header_footer(fd, sha1, state->pack_tmp_name,
-state->nr_written, sha1,
+   int fd = sha1close(state->f, sha1.oid, 0);
+   fixup_pack_header_footer(fd, sha1.oid, state->pack_tmp_name,
+state->nr_written, sha1.oid,
 state->offset);
close(fd);
}
@@ -47,7 +47,7 @@ static void finish_bulk_checkin(struct bulk_checkin_state 
*state)
strbuf_addf(&packname, "%s/pack/pack-", get_object_directory());
finish_tmp_packfile(&packname, state->pack_tmp_name,
state->written, state->nr_written,
-   &state->pack_idx_opts, sha1);
+   &state->pack_idx_opts, sha1.oid);
for (i = 0; i < state->nr_written; i++)
free(state->written[i]);
 
-- 
2.0.0.rc0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 5/9] branch.c: convert to use struct object_id

2014-05-03 Thread brian m. carlson
Signed-off-by: brian m. carlson 
---
 branch.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/branch.c b/branch.c
index 660097b..8dc0d49 100644
--- a/branch.c
+++ b/branch.c
@@ -184,9 +184,9 @@ int validate_new_branchname(const char *name, struct strbuf 
*ref,
 
if (!attr_only) {
const char *head;
-   unsigned char sha1[20];
+   struct object_id sha1;
 
-   head = resolve_ref_unsafe("HEAD", sha1, 0, NULL);
+   head = resolve_ref_unsafe("HEAD", sha1.oid, 0, NULL);
if (!is_bare_repository() && head && !strcmp(head, ref->buf))
die(_("Cannot force update the current branch."));
}
@@ -228,7 +228,7 @@ void create_branch(const char *head,
 {
struct ref_lock *lock = NULL;
struct commit *commit;
-   unsigned char sha1[20];
+   struct object_id sha1;
char *real_ref, msg[PATH_MAX + 20];
struct strbuf ref = STRBUF_INIT;
int forcing = 0;
@@ -248,7 +248,7 @@ void create_branch(const char *head,
}
 
real_ref = NULL;
-   if (get_sha1(start_name, sha1)) {
+   if (get_sha1(start_name, sha1.oid)) {
if (explicit_tracking) {
if (advice_set_upstream_failure) {
error(_(upstream_missing), start_name);
@@ -260,7 +260,7 @@ void create_branch(const char *head,
die(_("Not a valid object name: '%s'."), start_name);
}
 
-   switch (dwim_ref(start_name, strlen(start_name), sha1, &real_ref)) {
+   switch (dwim_ref(start_name, strlen(start_name), sha1.oid, &real_ref)) {
case 0:
/* Not branching from any existing branch */
if (explicit_tracking)
@@ -281,9 +281,9 @@ void create_branch(const char *head,
break;
}
 
-   if ((commit = lookup_commit_reference(sha1)) == NULL)
+   if ((commit = lookup_commit_reference(sha1.oid)) == NULL)
die(_("Not a valid branch point: '%s'."), start_name);
-   hashcpy(sha1, commit->object.sha1);
+   hashcpy(sha1.oid, commit->object.sha1);
 
if (!dont_change_ref) {
lock = lock_any_ref_for_update(ref.buf, NULL, 0, NULL);
@@ -305,7 +305,7 @@ void create_branch(const char *head,
setup_tracking(ref.buf + 11, real_ref, track, quiet);
 
if (!dont_change_ref)
-   if (write_ref_sha1(lock, sha1, msg) < 0)
+   if (write_ref_sha1(lock, sha1.oid, msg) < 0)
die_errno(_("Failed to write ref"));
 
strbuf_release(&ref);
-- 
2.0.0.rc0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 3/9] archive.c: convert to use struct object_id

2014-05-03 Thread brian m. carlson
Signed-off-by: brian m. carlson 
---
 archive.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/archive.c b/archive.c
index 3fc0fb2..dba148a 100644
--- a/archive.c
+++ b/archive.c
@@ -255,7 +255,7 @@ static void parse_treeish_arg(const char **argv,
time_t archive_time;
struct tree *tree;
const struct commit *commit;
-   unsigned char sha1[20];
+   struct object_id sha1;
 
/* Remotes are only allowed to fetch actual refs */
if (remote && !remote_allow_unreachable) {
@@ -263,15 +263,15 @@ static void parse_treeish_arg(const char **argv,
const char *colon = strchrnul(name, ':');
int refnamelen = colon - name;
 
-   if (!dwim_ref(name, refnamelen, sha1, &ref))
+   if (!dwim_ref(name, refnamelen, sha1.oid, &ref))
die("no such ref: %.*s", refnamelen, name);
free(ref);
}
 
-   if (get_sha1(name, sha1))
+   if (get_sha1(name, sha1.oid))
die("Not a valid object name");
 
-   commit = lookup_commit_reference_gently(sha1, 1);
+   commit = lookup_commit_reference_gently(sha1.oid, 1);
if (commit) {
commit_sha1 = commit->object.sha1;
archive_time = commit->date;
@@ -280,21 +280,21 @@ static void parse_treeish_arg(const char **argv,
archive_time = time(NULL);
}
 
-   tree = parse_tree_indirect(sha1);
+   tree = parse_tree_indirect(sha1.oid);
if (tree == NULL)
die("not a tree object");
 
if (prefix) {
-   unsigned char tree_sha1[20];
+   struct object_id tree_sha1;
unsigned int mode;
int err;
 
err = get_tree_entry(tree->object.sha1, prefix,
-tree_sha1, &mode);
+tree_sha1.oid, &mode);
if (err || !S_ISDIR(mode))
die("current working directory is untracked");
 
-   tree = parse_tree_indirect(tree_sha1);
+   tree = parse_tree_indirect(tree_sha1.oid);
}
ar_args->tree = tree;
ar_args->commit_sha1 = commit_sha1;
-- 
2.0.0.rc0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 1/9] Define a structure for object IDs.

2014-05-03 Thread brian m. carlson
Many places throughout the code use "unsigned char [20]" to store object IDs
(SHA-1 values).  This leads to lots of hardcoded numbers throughout the
codebase.  It also leads to confusion about the purposes of a buffer.

Introduce a structure for object IDs.  This allows us to obtain the benefits
of compile-time checking for misuse.  The structure is expected to remain
the same size and have the same alignment requirements on all known
platforms, compared to the array of unsigned char.

Signed-off-by: brian m. carlson 
---
 object.h | 13 -
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/object.h b/object.h
index 6e12f2c..6a9680d 100644
--- a/object.h
+++ b/object.h
@@ -1,6 +1,17 @@
 #ifndef OBJECT_H
 #define OBJECT_H
 
+/*
+ * The length in bytes and in hex digits of an object name (SHA-1 value).
+ * These are the same names used by libgit2.
+ */
+#define GIT_OID_RAWSZ 20
+#define GIT_OID_HEXSZ 40
+
+struct object_id {
+   unsigned char oid[GIT_OID_RAWSZ];
+};
+
 struct object_list {
struct object *item;
struct object_list *next;
@@ -49,7 +60,7 @@ struct object {
unsigned used : 1;
unsigned type : TYPE_BITS;
unsigned flags : FLAG_BITS;
-   unsigned char sha1[20];
+   unsigned char sha1[GIT_OID_RAWSZ];
 };
 
 extern const char *typename(unsigned int type);
-- 
2.0.0.rc0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 4/9] zip: use GIT_OID_HEXSZ for trailers

2014-05-03 Thread brian m. carlson
The object.h header is included in archive.h for this constant.  It will be
used by other parts of the archiving code in the future.

Signed-off-by: brian m. carlson 
---
 archive-zip.c | 4 ++--
 archive.h | 1 +
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/archive-zip.c b/archive-zip.c
index 4bde019..5b9fe42 100644
--- a/archive-zip.c
+++ b/archive-zip.c
@@ -427,12 +427,12 @@ static void write_zip_trailer(const unsigned char *sha1)
copy_le16(trailer.entries, zip_dir_entries);
copy_le32(trailer.size, zip_dir_offset);
copy_le32(trailer.offset, zip_offset);
-   copy_le16(trailer.comment_length, sha1 ? 40 : 0);
+   copy_le16(trailer.comment_length, sha1 ? GIT_OID_HEXSZ : 0);
 
write_or_die(1, zip_dir, zip_dir_offset);
write_or_die(1, &trailer, ZIP_DIR_TRAILER_SIZE);
if (sha1)
-   write_or_die(1, sha1_to_hex(sha1), 40);
+   write_or_die(1, sha1_to_hex(sha1), GIT_OID_HEXSZ);
 }
 
 static void dos_time(time_t *time, int *dos_date, int *dos_time)
diff --git a/archive.h b/archive.h
index 4a791e1..fd21408 100644
--- a/archive.h
+++ b/archive.h
@@ -2,6 +2,7 @@
 #define ARCHIVE_H
 
 #include "pathspec.h"
+#include "object.h"
 
 struct archiver_args {
const char *base;
-- 
2.0.0.rc0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] subtree/Makefile: Standardize (esp. for packagers)

2014-05-03 Thread Felipe Contreras
James Denholm wrote:
> Matthew Ogilvie  wrote:
> > On Sun, Apr 27, 2014 at 12:35:13PM +1000, James Denholm wrote:
> >> Jeff King  wrote:
> > Agreed.  It also doesn't help that when subtree patches are proposed
> > (especially new features instead of obvious bugs), there often seems
> > to be little or no feedback from anyone.
> >
> > 
> > Depending on how much time you have:
> >
> > This may be outside the scope of work you were planning on,
> 
> While current, immediate focus is really just getting the makefile fixed
> up and hopefully then have more people package subtree by default,
> overall I'll very likely extend that to general work on subtree and such.

I think you should take a look at the Makefile of
contrib/remote-helpers. I bet something simple like that would work just
fine for subtree.

-- 
Felipe Contreras
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] RelNotes/2.0.0: Grammar and typo fixes

2014-05-03 Thread Øyvind A . Holm
Signed-off-by: Øyvind A. Holm 
---
 Documentation/RelNotes/2.0.0.txt | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/Documentation/RelNotes/2.0.0.txt b/Documentation/RelNotes/2.0.0.txt
index e6911ac..35e353e 100644
--- a/Documentation/RelNotes/2.0.0.txt
+++ b/Documentation/RelNotes/2.0.0.txt
@@ -68,7 +68,7 @@ UI, Workflows & Features
e.g. "key-id" in "--gpg-sign=").
 
  * The pattern to find where the function begins in C/C++ used in
-   "diff" and "grep -p" have been updated to help C++ source better.
+   "diff" and "grep -p" has been updated to help C++ source better.
 
  * "git rebase" learned to interpret a lone "-" as "@{-1}", the
branch that we were previously on.
@@ -98,7 +98,7 @@ UI, Workflows & Features
tree-wide operation even when run inside a subdirectory of a
working tree.
 
- * "git add  is the same as "git add -A " now.
+ * "git add " is the same as "git add -A " now.
 
  * "core.statinfo" configuration variable, which is a
never-advertised synonym to "core.checkstat", has been removed.
@@ -137,7 +137,7 @@ UI, Workflows & Features
  * "git pull" can be told to only accept fast-forward by setting the
new "pull.ff" configuration.
 
- * "git reset" learned "-N" option, which does not reset the index
+ * "git reset" learned the "-N" option, which does not reset the index
fully for paths the index knows about but the tree-ish the command
resets to does not (these paths are kept as intend-to-add entries).
 
@@ -156,11 +156,11 @@ Performance, Internal Implementation, etc.
"easy" interface.
 
  * The bitmap-index feature from JGit has been ported, which should
-   significantly improve performance when serving objects form a
+   significantly improve performance when serving objects from a
repository that uses it.
 
  * The way "git log --cc" shows a combined diff against multiple
-   parents have been optimized.
+   parents has been optimized.
 
  * The prefixcmp() and suffixcmp() functions are gone.  Use
starts_with() and ends_with(), and also consider if skip_prefix()
-- 
2.0.0.rc2

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] subtree/Makefile: Standardize (esp. for packagers)

2014-05-03 Thread James Denholm
Matthew Ogilvie  wrote:
> On Sun, Apr 27, 2014 at 12:35:13PM +1000, James Denholm wrote:
>> Jeff King  wrote:
> Agreed.  It also doesn't help that when subtree patches are proposed
> (especially new features instead of obvious bugs), there often seems
> to be little or no feedback from anyone.
>
> 
> Depending on how much time you have:
>
> This may be outside the scope of work you were planning on,

While current, immediate focus is really just getting the makefile fixed
up and hopefully then have more people package subtree by default,
overall I'll very likely extend that to general work on subtree and such.

>   
>but
> it might be worth grepping through old mailing list archives for
> "subtree" patches that haven't been merged, and see if there is
> anything worth revisiting/resubmitting.  I believe most of the
> following (at least) kind of languished and died, often with little
> or no real review and feedback:
>
> (...)
>
> (I don't know if these are the latest or "best" versions of these, nor
> have I really looked at them closely to decide if they are worth
> including at all.  Be sure to exameine not just the discussion around
> the specific patches, but also the other patches in each series...)

Yeah, certainly, I'll be sure to have a sticky-beak. Thanks for pointing
those out!

Regards,
James Denholm.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 2/5] contrib/subtree/Makefile: Use GIT-VERSION-FILE

2014-05-03 Thread James Denholm
GVF is already being used in most/all other makefiles in the project,
and has been for _quite_ a while. Hence, drop file-unique gitver and
replace with GIT_VERSION.

Signed-off-by: James Denholm 
---
 contrib/subtree/Makefile | 11 ---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/contrib/subtree/Makefile b/contrib/subtree/Makefile
index 87797ed..f63334b 100644
--- a/contrib/subtree/Makefile
+++ b/contrib/subtree/Makefile
@@ -6,7 +6,10 @@ mandir ?= $(prefix)/share/man
 libexecdir ?= $(prefix)/libexec/git-core
 man1dir ?= $(mandir)/man1
 
-gitver ?= $(word 3,$(shell git --version))
+../../GIT-VERSION-FILE: FORCE
+   $(MAKE) -C ../../ GIT-VERSION-FILE
+
+-include ../../GIT-VERSION-FILE
 
 # this should be set to a 'standard' bsd-type install program
 INSTALL ?= install
@@ -44,11 +47,11 @@ $(GIT_SUBTREE_DOC): $(GIT_SUBTREE_XML)
 
 $(GIT_SUBTREE_XML): $(GIT_SUBTREE_TXT)
asciidoc -b docbook -d manpage -f $(ASCIIDOC_CONF) \
-   -agit_version=$(gitver) $^
+   -agit_version=$(GIT_VERSION) $^
 
 $(GIT_SUBTREE_HTML): $(GIT_SUBTREE_TXT)
asciidoc -b xhtml11 -d manpage -f $(ASCIIDOC_CONF) \
-   -agit_version=$(gitver) $^
+   -agit_version=$(GIT_VERSION) $^
 
 test:
$(MAKE) -C t/ test
@@ -56,3 +59,5 @@ test:
 clean:
rm -f *~ *.xml *.html *.1
rm -rf subproj mainline
+
+.PHONY: FORCE
-- 
1.9.2

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 4/5] contrib/subtree/Makefile: Doc-gen rules cleanup

2014-05-03 Thread James Denholm
git:Documentation/Makefile establishes asciidoc/xmlto calls as being
handled through their appropriate variables, Hence, change to bring into
congruency with.

Similarly, MANPAGE_XSL exists in git:Documentation/Makefile, while
MANPAGE_NORMAL_XSL does not outside contrib/subtree. Hence, replace
MANPAGE_NORMAL_XSL with MANPAGE_XSL.

Signed-off-by: James Denholm 
---
 contrib/subtree/Makefile | 13 -
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/contrib/subtree/Makefile b/contrib/subtree/Makefile
index 579bb51..f3834b5 100644
--- a/contrib/subtree/Makefile
+++ b/contrib/subtree/Makefile
@@ -14,8 +14,11 @@ man1dir ?= $(mandir)/man1
 # this should be set to a 'standard' bsd-type install program
 INSTALL ?= install
 
-ASCIIDOC_CONF  = ../../Documentation/asciidoc.conf
-MANPAGE_NORMAL_XSL =  ../../Documentation/manpage-normal.xsl
+ASCIIDOC = asciidoc
+XMLTO= xmlto
+
+ASCIIDOC_CONF = ../../Documentation/asciidoc.conf
+MANPAGE_XSL   = ../../Documentation/manpage-normal.xsl
 
 GIT_SUBTREE_SH := git-subtree.sh
 GIT_SUBTREE:= git-subtree
@@ -43,14 +46,14 @@ install-man: $(GIT_SUBTREE_DOC)
$(INSTALL) -m 644 $^ $(DESTDIR)$(man1dir)
 
 $(GIT_SUBTREE_DOC): $(GIT_SUBTREE_XML)
-   xmlto -m $(MANPAGE_NORMAL_XSL)  man $^
+   $(XMLTO) -m $(MANPAGE_XSL) man $^
 
 $(GIT_SUBTREE_XML): $(GIT_SUBTREE_TXT)
-   asciidoc -b docbook -d manpage -f $(ASCIIDOC_CONF) \
+   $(ASCIIDOC) -b docbook -d manpage -f $(ASCIIDOC_CONF) \
-agit_version=$(GIT_VERSION) $^
 
 $(GIT_SUBTREE_HTML): $(GIT_SUBTREE_TXT)
-   asciidoc -b xhtml11 -d manpage -f $(ASCIIDOC_CONF) \
+   $(ASCIIDOC) -b xhtml11 -d manpage -f $(ASCIIDOC_CONF) \
-agit_version=$(GIT_VERSION) $^
 
 test:
-- 
1.9.2

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 3/5] contrib/subtree/Makefile: s/libexecdir/gitexecdir

2014-05-03 Thread James Denholm
$(libexecdir) isn't used anywhere else in the project, while
$(gitexecdir) is the standard in the other appropriate makefiles. Hence,
replace the former with the latter.

Signed-off-by: James Denholm 
---
 contrib/subtree/Makefile | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/contrib/subtree/Makefile b/contrib/subtree/Makefile
index f63334b..579bb51 100644
--- a/contrib/subtree/Makefile
+++ b/contrib/subtree/Makefile
@@ -3,7 +3,7 @@
 
 prefix ?= /usr/local
 mandir ?= $(prefix)/share/man
-libexecdir ?= $(prefix)/libexec/git-core
+gitexecdir ?= $(prefix)/libexec/git-core
 man1dir ?= $(mandir)/man1
 
 ../../GIT-VERSION-FILE: FORCE
@@ -33,8 +33,8 @@ $(GIT_SUBTREE): $(GIT_SUBTREE_SH)
 doc: $(GIT_SUBTREE_DOC) $(GIT_SUBTREE_HTML)
 
 install: $(GIT_SUBTREE)
-   $(INSTALL) -d -m 755 $(DESTDIR)$(libexecdir)
-   $(INSTALL) -m 755 $(GIT_SUBTREE) $(DESTDIR)$(libexecdir)
+   $(INSTALL) -d -m 755 $(DESTDIR)$(gitexecdir)
+   $(INSTALL) -m 755 $(GIT_SUBTREE) $(DESTDIR)$(gitexecdir)
 
 install-doc: install-man
 
-- 
1.9.2

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 5/5] contrib/subtree/Makefile: clean rule cleanup

2014-05-03 Thread James Denholm
git:Documentation/Makefile and others establish "RM ?= rm -f" as a
convention for rm calls in clean rules, hence follow this convention
instead of simply forcing clean to use rm.

subproj and mainline no longer need to be removed in clean, as they are
no longer created in git:contrib/subtree by "make test". Hence, remove
the rm call for those folders.

Other makefiles don't remove "*~" files, remove the rm call to prevent
unexpected behaviour in the future. Similarly, clean doesn't remove the
installable file, so rectify this.

Signed-off-by: James Denholm 
---

Admittedly, git:Makefile does not itself follow the "RM ?= rm -f"
setup, instead using "RM = rm -f", but I felt that there were probably
$ARCANE_REASONS for this.

Also, Peff, you were right about the dirs.

 contrib/subtree/Makefile | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/contrib/subtree/Makefile b/contrib/subtree/Makefile
index f3834b5..4f96a24 100644
--- a/contrib/subtree/Makefile
+++ b/contrib/subtree/Makefile
@@ -11,8 +11,9 @@ man1dir ?= $(mandir)/man1
 
 -include ../../GIT-VERSION-FILE
 
-# this should be set to a 'standard' bsd-type install program
-INSTALL ?= install
+# These should be set to 'standard' bsd-type programs
+INSTALL  ?= install
+RM   ?= rm -f
 
 ASCIIDOC = asciidoc
 XMLTO= xmlto
@@ -60,7 +61,7 @@ test:
$(MAKE) -C t/ test
 
 clean:
-   rm -f *~ *.xml *.html *.1
-   rm -rf subproj mainline
+   $(RM) $(GIT_SUBTREE)
+   $(RM) *.xml *.html *.1
 
 .PHONY: FORCE
-- 
1.9.2

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 1/5] contrib/subtree/Makefile: scrap unused $(gitdir)

2014-05-03 Thread James Denholm
All references were removed in 7ff8463dba0d74fc07a766bed457ae7afcc902b5,
but the assignment itself wasn't. Hence, drop gitdir assignment.

Signed-off-by: James Denholm 
---
 contrib/subtree/Makefile | 1 -
 1 file changed, 1 deletion(-)

diff --git a/contrib/subtree/Makefile b/contrib/subtree/Makefile
index 4030a16..87797ed 100644
--- a/contrib/subtree/Makefile
+++ b/contrib/subtree/Makefile
@@ -4,7 +4,6 @@
 prefix ?= /usr/local
 mandir ?= $(prefix)/share/man
 libexecdir ?= $(prefix)/libexec/git-core
-gitdir ?= $(shell git --exec-path)
 man1dir ?= $(mandir)/man1
 
 gitver ?= $(word 3,$(shell git --version))
-- 
1.9.2

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 0/5] contrib/subtree/Makefile: Standardisation pass

2014-05-03 Thread James Denholm
contrib/subtree/Makefile is a shambles in regards to it's consistency
with other makefiles, which makes subtree overly painful to include in
build scripts.

The main issues are that calls are made to git itself in the build
process, and that a subtree-exclusive variable is used for specifying
the exec path. Patches 1/5 through 3/5 resolve these.

The "cleanup" fixes (4/5 and 5/5) are based on precedents set by other
makefiles across the project.

One problem is foreseen: 3/5 will necessitate that package maintainers
who already have git-subtree included in their packages update their
build-scripts.

Signed-off-by: James Denholm 
Based-on-patch-by: Dan McGee 

James Denholm (5):
  contrib/subtree/Makefile: scrap unused $(gitdir)
  contrib/subtree/Makefile: Use GIT-VERSION-FILE
  contrib/subtree/Makefile: s/libexecdir/gitexecdir
  contrib/subtree/Makefile: Doc-gen rules cleanup
  contrib/subtree/Makefile: clean rule cleanup

 contrib/subtree/Makefile | 40 
 1 file changed, 24 insertions(+), 16 deletions(-)

-- 
1.9.2

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Downloading git source from https://code.google.com/p/git-core/downloads/

2014-05-03 Thread Daniel Villeneuve

Hi,

I've used the following link to download git source and corresponding 
pre-formatted man pages for several months:


https://code.google.com/p/git-core/downloads/

However, the latest version available on the site is git-1.9.0 (last 
updated on 2014-02-14).


Is the site still maintained/updated?
--
Daniel Villeneuve

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Pull is Mostly Evil

2014-05-03 Thread Felipe Contreras
Philip Oakley wrote:
> From: "Felipe Contreras" 
> > When doing something is better for the vast majority of people, that's
> > what should be done by default, unless the results are catastrophic 
> > for
> > the minority.
> >
> > Since doing something is not catastrophic to the minority, it follows
> > that the default should be to do something.

> > It's a simple as that.
> 
> ... which makes it not quite as simple as that ;-) [evidence: the 
> ongoing dialog among all and sundry]

The dialog is not simple becuase it's not easy to make `git pull` do the
sensible thing. That doesn't mean `git pull` should do *nothing*, that
doesn't follow from the argument above.

-- 
Felipe Contreras
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Pull is Mostly Evil

2014-05-03 Thread Philip Oakley

From: "Felipe Contreras" 
Sent: Saturday, May 03, 2014 12:23 AM

Philip Oakley wrote:

From: "Felipe Contreras" 
> So? No defaults can please absolutely everyone, the best anybody 
> can

> do is try to please the majority of people, and merging
> fast-forwards only does that.

That assumes that doing something is better than doing nothing,


When doing something is better for the vast majority of people, that's
what should be done by default, unless the results are catastrophic 
for

the minority.

Since doing something is not catastrophic to the minority, it follows
that the default should be to do something.

That 'Since' and 'it follows' are where we have a diverging 
understanding about the solution approach...



It's a simple as that.


... which makes it not quite as simple as that ;-) [evidence: the 
ongoing dialog among all and sundry]


--
Felipe Contreras
--


Philip 


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: pull.prompt or other way to slow/disable 'git pull'

2014-05-03 Thread Felipe Contreras
W. Trevor King wrote:
> On Fri, May 02, 2014 at 05:20:11PM -0500, Felipe Contreras wrote:
> > W. Trevor King wrote:
> > > > > The 'git pull' (with 'none' mode) explainer just helps retrain folks
> > > > > that are already using the current 'git pull' incorrectly.
> > > > 
> > > > If you are going to train them to use a configuration, it should be:
> > > > 
> > > > % git config --global pull.ff false
> > > 
> > > I don't want all pulls to be --no-ff, only pulls from topic branches.
> > 
> > Pulling some branch to a topic branch, or pulling a topic branch to
> > another branch?
> 
> The latter.  Here's a more detailed list:
> 
> 1. HEAD: an integration branch (master, maint, …)
>target: @{upstream}, branch.*.pushremote, and other mirrors
>my preferred integration mode: ff-only merge the target

`git pull` would do that by default.

> 2. HEAD: an integration branch
>target: a *different* branch (e.g. maint or feature-x, but not
>  origin/master or jdoe/master, if HEAD is master)
>my preferred integration mode: no-ff merge the target into HEAD.

That makes sense, but other people would be OK with a ff merge.

> 3. HEAD: a topic branch (e.g. feature-x)
>target: a collaborating topic branch (jdoe/feature-x)
>my preferred integration mode: ff-only merge the target

I don't see why. It will alomst always be non-fast-fowrward, so you
should already be prepared for a merge (or rebase).

> 4. HEAD: a topic branch (e.g. feature-x)
>target: a related topic branch (e.g. jdoe/feature-y) or integration
>  branch updates used by my feature-x
>my preferred integration mode: rebase feature-x onto the target

Nah. Most people would prefer a merge. And actually, quite many would
want jdoe/feature-y to be rebased on top of feature-x.

Either way it would be impossible for Git to figre out what you want to
do.

> Cases 1 and 2 can usually be distinguished by comparing the
> checked-out branch with the branch portion of the remote-tracking
> reference), but for folks developing in master, jdoe/master may be a
> feature branch (case 2) not a mirror of the maintenance branch (case
> 1).

I'd say they can be distinguished by what the user typed.
 
> Cases 1 and 3 are the same idea, with any feature branch running long
> enough to get collaborators being indistinguishable from an
> integration branch except that the latter will eventually be merged
> (or dropped) and deleted.

Ineed, so why would you want so drastically different behavior?
 
> In the event of non-trivial merge conflicts in case 2, I sometimes
> rebase the target onto HEAD and no-ff merge the resulting target'.  On
> the other hand, sometimes rebasing is not an option.  For example, if
> I want to merge the target into both master and maint, but master
> contains a conflicting commit A:
> 
>   -o---o---A---o---B  master
>|\
>| o---o---C  maint
> \
>  o---D  target
> 
> Rebasing would drag A into maint at F:
> 
>   -o---o---A---o---B---E  master
> \   \ /
>  \   o---D'---  target'
>   \   \
>o---o---C---F  maint
> 
> And I don't want both the pre- and post-rebase versions in my history
> at G:
> 
>   -o---o---A---o---B---E---G  master
>|\   \ /   /
>| \   o---D'---   /  target'
>|  \ /
>|   o---o---C---F  maint
> \ /
>  o---D  target
> 
> So I'd just deal with a complicated merge at E:
> 
>   -o---o---A---o---B---E---G  master
>|\ /   /
>| o---D   /  target
> \   \   /
>  o---o---C---F--  maint
> 
> Case 4 has similar caveats, since you don't want to rebase feature-x
> on top of jdoe/feature-y if there are already other branches based on
> the current feature-x that can't (or won't) be rebased.

What I do in those cases is do both a merge and a rebase. If I resolved
the conflicts correctly in the rebase the result of the merge should be
exactly the same. It's not hard because rerere stores the conflict
resolutions of the rebase and the merge becomes much simpler. After I'm
certain the merge is correct, I remove the temporary rebased branch.

Anyway I don't see how is this possibly relevant to the topic at hand.

> > Either way, since I think these two are different modes:
> > 
> >   1) git pull
> >   2) git pull origin topic
> > 
> > Maybe it would actually make sense to have a configuration specific to
> > 2): pull.topicmode.
> 
> I think it makes more sense to just use merge/rebase explicitly,

Fine, if you want the user to be explicit, he can be explicit with
`git pull --no-ff origin topic`. Problem solved.

-- 
Felipe Contreras--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Pull is Mostly Evil

2014-05-03 Thread John Szakmeister
On Fri, May 2, 2014 at 2:13 PM, Junio C Hamano  wrote:
[snip]
> Your earlier long-hand, together with the two examples that pulls
> into the same "maint" branch Brian gave us, may give us a better
> starting points to think about a saner way.
>
> To me, the problem sounds like:
>
> Tutorials of Git often says "use 'git pull' to catch up your
> branch with your upstream work and then 'git push' back" (and
> worse yet, 'git push' that does not fast-forward suggests doing
> so), but 'git pull' that creates a merge in a wrong direction is
> not the right thing for many people.

Yes, that's a good portion of the problem.

> And proposed solutions range from "let's write 'pull' off as a
> failed experiment" to "let's forbid any merge made by use of 'pull'
> by default, because it is likely that merge may be in reverse".

FWIW, at my company, we took another approach.  We introduced a `git
ffwd` command that fetches from all remotes, and fast-forwards all
your local branches that are tracking a remote, and everyone on the
team uses it all the time.  It should be said this team also likes to
use Git bare-metal, because they like knowing how things work
out-of-the-box.  But they all use the command because it's so
convenient.

I had started making a C version a while back, but never completed it.
 I could take a stab at doing so again, if there's interest.

-John
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Pull is Mostly Evil

2014-05-03 Thread David Kastrup
Felipe Contreras  writes:

> David Kastrup wrote:
>> Richard Hansen  writes:
>> 
>> > These three usage patterns are at odds; it's hard to change the
>> > default behavior of 'git pull' to favor one usage case without
>> > harming another.  Perhaps this is why there's so much disagreement
>> > about what 'git pull' should do.
>> 
>> Should a screwdriver be turning clockwise or counterclockwise by
>> default?  There are valid arguments for either.
>
> If you don't have anything to contribute don't disturb the people that
> actually care and are trying to improve Git. Thanks.

No need to expand on the welcoming atmosphere here.  My heinous plot to
subvert the quality of Git has already been thwarted by making sure that
its "meritocracy" continues relying only on input from those with an
independent income.  I'm just sticking around until my current
contributions move into master so that I can summarize the resulting
low-hanging fruit that the meritorious can then pick at great fanfare.

The sooner my work moves from pu into master, the sooner y'all be rid of
me.

-- 
David Kastrup
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Pull is Mostly Evil

2014-05-03 Thread Felipe Contreras
Richard Hansen wrote:

> I think the fundamental difference is in the relationship between the
> local and the remote branch (which branch derives from the other).
> The relationship between the branches determines what the user wants
> from 'git pull'.
> 
> In my experience 'git pull' is mostly (only?) used for the following
> three tasks:

I agree.

>  1. update a local branch to incorporate the latest upstream changes
> 
> In this case, the local branch (master) is a derivative of the
> upstream branch (origin/master).  The user wants all of the
> commits in the remote branch to be in the local branch.  And the
> user would like the local changes, if any, to descend from the tip
> of the remote branch.

My current propsal of making `git pull` by default do --ff-only would
solve this. In addition I think by default 'master' should be merged to
'origin/master', if say --merge is given.

> For this case, 'git pull --ff-only' followed by 'git rebase -p'
> works well, as does 'git pull --rebase=preserve' if the user is
> comfortable rebasing without reviewing the incoming commits first.

I suppose you mean a `git rebase -p` if the `git pull --ff-only` failed.
This might be OK on most projects, but not all.

What happens after a `git pull --ff-only` fails should be totally
up to the user.

>  2. update a published feature branch with the latest changes from its
> parent branch
> 
> In this case, the local branch (foo) is a derivative of the
> upstream branch (origin/foo) which is itself a derivative of
> another branch (origin/master).  All commits in origin/master
> should be in origin/foo, and ideally all commits unique to
> origin/foo would descend from the tip of origin/master.

I don't understand why are you tainting the example with 'origin/foo',
'foo' and 'origin/master' are enough for this example. In fact, the
mention of 'origin/master' made it wrong: after the pull not all the
commits of origin/master would be in origin/foo, you need a push for
that. We have enough in our plate to taint this with yet another branch
and push.

For this case `git pull origin master` already work correctly for most
projects. We probably shouldn't change that.

>  3. integrate a more-or-less complete feature/fix back into the line
> of development it forked off of
> 
> In this case the local branch is a primary line of development and
> the remote branch contains the derivative work.  Think Linus
> pulling in contributions.  Different situations will call for
> different ways to handle this case, but most will probably want
> some or all of:
> 
>  * rebase the remote commits onto local HEAD

No. Most people will merge the remote branch as it is. There's no reason
to rebase, specially if you are creating a merge commit.

>  * merge into local HEAD so that the first parent (if a real merge
>and not a ff) is the previous version of the main line of
>development and the second parent is the derivative work
>  * merge --no-ff so that:
> - the merge can serve as a cover letter (who reviewed it,
>   which bug reports were fixed, where the changes came from,
>   etc.)
> - the commits that compose the new topic are grouped together
> - the first-parent path represents a series of completed tasks

It is very rare that an integrator is even able to do a fast-forward
merge anyway. So being explicit about --no-ff might better, but it would
hardly make a difference. Either way, a good integrator would configure
pull.ff = false.

I'd say `git pull origin master` already works fine for this case.

-- 
Felipe Contreras
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Pull is Mostly Evil

2014-05-03 Thread Felipe Contreras
David Kastrup wrote:
> Richard Hansen  writes:
> 
> > These three usage patterns are at odds; it's hard to change the
> > default behavior of 'git pull' to favor one usage case without
> > harming another.  Perhaps this is why there's so much disagreement
> > about what 'git pull' should do.
> 
> Should a screwdriver be turning clockwise or counterclockwise by
> default?  There are valid arguments for either.

If you don't have anything to contribute don't disturb the people that
actually care and are trying to improve Git. Thanks.

-- 
Felipe Contreras
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Watchman support for git

2014-05-03 Thread Duy Nguyen
On Sat, May 3, 2014 at 11:39 AM, David Turner  wrote:
>> Index v4 and split index (and the following read-cache daemon,
>> hopefully)
>
> Looking at some of the archives for read-cache daemon, it seems to be
> somewhat similar to watchman, right?  But I only saw inotify code; what
> about Mac OS?  Or am I misunderstanding what it is?

It's mentioned in [1], the second paragraph, mostly to hide index I/O
read cost and the SHA-1 hashing cost in the background. In theory it
should work on all platforms that support multiple processes and
efficient IPC. It can help load watchman file cache faster too.

>> The last line could be a competition between watchman and my coming
>> "untracked cache" series. I expect to cut the number in that line at
>> least in half without external dependency.
>
> I hadn't seen the "untracked cached" work (I actually finished these
> patches a month or so ago but have been waiting for some internal
> reviews before sending them out).  Looks interesting.  It seems we use a
> similar strategy for handling ignores.

Yep, mostly the same at the core, except that I exploit directory
mtime while you use inotify. Each approach has its own pros and cons,
I think. Both should face the same traps in caching (e.g. if you "git
rm --cached" a file, that file could be come either untracked, or
ignored).

>> Patch 2/3 did not seem to make it to the list by the way..
>
> Thanks for your comments.  I just tried again to send patch 2/3.  I do
> actually see the CC of it in my @twitter.com mailbox, but I don't see it
> in the archives on the web.  Do you know if there is a reason the
> mailing list would reject it?

Probably its size, 131K, which is also an indicator to split it (and
the third patch) into smaller patches if you want to merge this
feature in master eventually.

>   At any rate, the contents may be found
> at
> https://github.com/dturner-tw/git/commit/cf587d54fc72d82a23267348afa2c4b60f14ce51.diff

Good enough for me :)

>
>> initial
>> reaction is storing the list of all paths seems too much, but I'll
>> need to play with it a bit to understand it.
>
> I wonder if it would make sense to use the untracked cache as the
> storage strategy, but use watchman as the update strategy.

I'm afraid not. If a directory mtime is changed, which means
files/dirs have been added or deleted, the untracked code would fall
back to the opendir/readdir/is_excluded dance again on that directory.
If we naively do the same using watchman, we lose its advantage that
it knows exactly what files/dirs are added/removed. That kind of
knowledge can help speed up the dance, which is not stored anywhere in
the untracked cache.

We could extend the "read-cache daemon" mentioned above though, to
hide all the hard work in the background and present a good view to
git: when a file/dir is added, read-cache daemon classifies the new
files/dirs as tracked/untracked/ignore and update its untracked cache
in memory. When "git status" asks about the index and untracked cache,
it will receive the _updated_ cache (not the on disk version any more)
with latest dir mtime so git can verify the cache is perfect and skip
opendir/ All git does is to write the index down in the end to
make the updated data permanent. It sounds interesting. But I'm not so
sure if it's worth the complexity.

[1] http://article.gmane.org/gmane.comp.version-control.git/247268
-- 
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


git subtree pull same remote different prefix problem

2014-05-03 Thread Asmodehn Shade
Hello,

I am currently using git subtree in a project :
https://github.com/asmodehn/wkcmake

This is basically a set of cmake scripts that help writing convention
based (folder hierarchy, etc.) cmake builds.

My setup is that "master" has the set of scripts needed. and many
other branches ("test" and "tests/*" most notably) have tests projects
for these scripts. Travis run build of these to check if the build
setup works as expected.

The "test" branch has a hierarchy of dependent projects (ProjectA,
ProjectB, ProjectC) , all using the wkcmake scripts to build.

So I am using git-subtree to pull and push  "ProjectA/CMake"
ProjectB/Custom CMake" "ProjectC/CMake".

I had enough trouble with push to stop using it( merge not happening
as I thought it would ). Not sure if I am to blame or not for these...
Anyway now, after a commit into the test branch, I instead split the
CMake subtree to a new branch, and merge in manually back to master.
That seems to work okay so far.

However I realized that when I do :
 git subtree pull -P ProjectA/CMake g...@github.com:asmodehn/
wkcmake.git origin master
 git subtree pull -P "ProjectB/Custom CMake"
g...@github.com:asmodehn/wkcmake.git origin master
 git subtree pull -P ProjectC/CMake
g...@github.com:asmodehn/wkcmake.git origin master

I get only one merge commit into the "test" branch. All the other
subtree pull returns "Already up to date" even though the content of
the working directory in the prefix passed is NOT uptodate.

The possible behavior that I could understand would be :
- One subtree pull update all prefixes, with only one merge commit,
and the prefix doesnt really matter anymore ( could be problematic...
)
- Each subtree pull update the appropriate prefix with one merge commit each.

So I am curious : I this a scenario that subtree doesnot support ?
Am I using it wrong ?
Is it a current limitation/bug of subtree that is on the roadmap to be
addressed sometime in the future ?

Thanks a lot for the help.
--
AlexV
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Pull is Mostly Evil

2014-05-03 Thread David Kastrup
Richard Hansen  writes:

> These three usage patterns are at odds; it's hard to change the
> default behavior of 'git pull' to favor one usage case without harming
> another.  Perhaps this is why there's so much disagreement about what
> 'git pull' should do.

Should a screwdriver be turning clockwise or counterclockwise by
default?  There are valid arguments for either.

-- 
David Kastrup
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Pull is Mostly Evil

2014-05-03 Thread Richard Hansen
On 2014-05-02 14:13, Junio C Hamano wrote:
> Stepping back even further, and thinking what is different between
> these two pulls, we notice that the first one is pulling from the
> place we push back to.

I think the fundamental difference is in the relationship between the
local and the remote branch (which branch derives from the other).
The relationship between the branches determines what the user wants
from 'git pull'.

In my experience 'git pull' is mostly (only?) used for the following
three tasks:

 1. update a local branch to incorporate the latest upstream changes

In this case, the local branch (master) is a derivative of the
upstream branch (origin/master).  The user wants all of the
commits in the remote branch to be in the local branch.  And the
user would like the local changes, if any, to descend from the tip
of the remote branch.

For this case, 'git pull --ff-only' followed by 'git rebase -p'
works well, as does 'git pull --rebase=preserve' if the user is
comfortable rebasing without reviewing the incoming commits first.
A plain 'git pull' or 'git pull --ff' is suboptimal due to the
awkward backwards-parents merge commit.

 2. update a published feature branch with the latest changes from its
parent branch

In this case, the local branch (foo) is a derivative of the
upstream branch (origin/foo) which is itself a derivative of
another branch (origin/master).  All commits in origin/master
should be in origin/foo, and ideally all commits unique to
origin/foo would descend from the tip of origin/master.

The relationship between origin/foo and origin/master is similar
to the relationship between master and origin/master in case #1
above, but rebase is frowned upon because the feature branch has
been shared with other developers (and the shared repository might
reject non-ff updates).

This case is sort-of like case #1 above (updating) and sort-of
like case #3 below (integrating).

For this case, after the local branch foo is updated (case #1
above), 'git pull --ff origin master' or 'git fetch --all && git
merge --ff origin/master' work well to update origin/foo.

 3. integrate a more-or-less complete feature/fix back into the line
of development it forked off of

In this case the local branch is a primary line of development and
the remote branch contains the derivative work.  Think Linus
pulling in contributions.  Different situations will call for
different ways to handle this case, but most will probably want
some or all of:

 * rebase the remote commits onto local HEAD
 * merge into local HEAD so that the first parent (if a real merge
   and not a ff) is the previous version of the main line of
   development and the second parent is the derivative work
 * merge --no-ff so that:
- the merge can serve as a cover letter (who reviewed it,
  which bug reports were fixed, where the changes came from,
  etc.)
- the commits that compose the new topic are grouped together
- the first-parent path represents a series of completed tasks

(I prefer to do all three, although I may skip the rebase if the
commits came from another public repository so as to not annoy
users of that downstream repository.)

For this case, 'git pull --no-ff' is better than 'git pull --ff'
(for the reasons listed above), but perhaps something more
elaborate would be ideal (e.g., rebase there onto here, then merge
--no-ff).

These three usage patterns are at odds; it's hard to change the
default behavior of 'git pull' to favor one usage case without harming
another.  Perhaps this is why there's so much disagreement about what
'git pull' should do.

I see a few ways to improve the situation:

  1. Add one or two new commands to split up how the three cases are
 handled.  For example:

  * Add a new 'git update' command that is friendly for case
#1.  Update tutorials to recommend 'git update' instead of
'git pull'.  It would behave like 'git pull --ff-only' by
default.

It could behave like 'git pull --rebase[=preserve]' instead,
but this has a few downsides:
 - It doesn't give the user an opportunity to review the
   incoming commits before rebasing (e.g., to see what sort of
   conflicts to expect).
 - It subjects new users to that scary rebase thing before
   they are prepared to handle it.
 - The branch to be updated must be checked out.  If 'git
   update' used --ff-only, then 'git update --all' could
   fast-forward all local branches to their configured
   upstreams when possible.  (How cool would that be?)

  * Leave 'git pull' and 'git pull $remote [$refspec]' alone --
the current defaults are acceptable (though maybe not ideal)
for cases #2 and #3.

 Another exam

Re: [PATCH 08/12] MINGW: fix main() signature in http-fetch.c and remote-curl.c

2014-05-03 Thread Marat Radchenko
On Wed, Apr 30, 2014 at 10:35:56AM +0200, Karsten Blees wrote:
> Am 29.04.2014 11:12, schrieb Marat Radchenko:
> > On MinGW, compat/mingw.h defines a 'mingw_main' wrapper function.
> > Fix `warning: passing argument 2 of 'mingw_main' from incompatible
> > pointer type` in http-fetch.c and remote-curl.c by dropping 'const'.
> > 
> 
> Would you mind cross checking your changes with the msysgit fork?
> Fixing the same thing in a slightly different manner will just cause
> unnecessary conflicts (i.e. unnecessary work for the Git for Windows
> maintainers, as well as for you to come up with an alternate solution
> for something that's long been fixed).
>
> See https://github.com/msysgit/git/commit/9206e7fd (squashed from
> https://github.com/msysgit/git/commit/0115ef83 and
> https://github.com/msysgit/git/commit/6949537a). 

6949537a is just an unnecessary hack and would have to be rewritten
no matter who would try to submit it upstream.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: BUG or FEATURE? Use of '/' in branch names

2014-05-03 Thread Dennis Kaarsemaker
On vr, 2014-05-02 at 15:16 -0700, Jonathan Nieder wrote:
> > $ git checkout -b hotfix/b2
> > error: unable to resolve reference refs/heads/hotfix/b2: Not a
> directory
> > fatal: Failed to lock ref for update: Not a directory
> > $
> 
> That's an ugly message.  I think we can do better. (hint hint)

2.0.0-rc2 has a better message already:

$ git checkout -b hotfix/b2
error: 'refs/heads/hotfix' exists; cannot create 'refs/heads/hotfix/b2'
fatal: Failed to lock ref for update: Not a directory


-- 
Dennis Kaarsemaker
www.kaarsemaker.net

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 10/12] MINGW: compat/poll/poll.c: undef NOGDI

2014-05-03 Thread Marat Radchenko
On Wed, Apr 30, 2014 at 01:41:25PM +0200, Stepan Kasal wrote:
> Hello,
> 
> On Tue, Apr 29, 2014 at 01:12:04PM +0400, Marat Radchenko wrote:
> > On MinGW-W64, MsgWaitForMultipleObjects is guarded with #ifndef NOGDI.
> > 
> > Removal -DNOGDI=1 from config.mak.uname has an undesirable effect of
> > bringing in wingdi.h with weird #define ERROR 0 that conflicts with
> > internal Git enums. So, just #undef NOGDI in compat/poll/poll.c.
> 
> compat/poll/poll.c comes from Gnulib, so it would be better to submit
> the patch there and then backport so that the divergence of the two
> versions does not get worse.

That's why v1 of this patch [1] didn't touch poll.c at all.
I don't think it's gnulib problem that combination of two third-parties
(git and mingw-w64) set up such conditions where poll.c fails to compile.

If one wants to dig deeper, I'd say the problem is in MinGW-W64 headers
because their behavior of hiding MsgWaitForMultipleObjects doesn't
match behavior of MSVC headers.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html