* [PATCHv3 2/4] wt-status: Teach sequencer advice to use get_state
From: Phil Hord @ 2012-11-09 18:56 UTC (permalink / raw)
To: git
Cc: phil.hord, Jeff King, Junio C Hamano, konglu, Matthieu Moy,
Kong Lucien, Duperray Valentin, Jonas Franck, Nguy Thomas,
Phil Hord
In-Reply-To: <1352487385-5929-1-git-send-email-hordp@cisco.com>
wt_status_print_state retrieves some sequencer state information via
wt_status_get_state, but other state information it deduces on its
own. Replace these "local knowledge" deductions with wt_status
variables so we can share more common code in the future.
---
wt-status.c | 16 +++++++++-------
wt-status.h | 3 +++
2 files changed, 12 insertions(+), 7 deletions(-)
diff --git a/wt-status.c b/wt-status.c
index 760f52b..a888120 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -781,7 +781,7 @@ static void show_merge_in_progress(struct wt_status *s,
struct wt_status_state *state,
const char *color)
{
- if (has_unmerged(s)) {
+ if (state->has_unmerged) {
status_printf_ln(s, color, _("You have unmerged paths."));
if (advice_status_hints)
status_printf_ln(s, color,
@@ -867,9 +867,7 @@ static void show_rebase_in_progress(struct wt_status *s,
struct wt_status_state *state,
const char *color)
{
- struct stat st;
-
- if (has_unmerged(s)) {
+ if (state->has_unmerged) {
status_printf_ln(s, color, _("You are currently rebasing."));
if (advice_status_hints) {
status_printf_ln(s, color,
@@ -879,12 +877,12 @@ static void show_rebase_in_progress(struct wt_status *s,
status_printf_ln(s, color,
_(" (use \"git rebase --abort\" to check out the original branch)"));
}
- } else if (state->rebase_in_progress || !stat(git_path("MERGE_MSG"), &st)) {
+ } else if (state->rebase_in_progress || state->commit_is_pending) {
status_printf_ln(s, color, _("You are currently rebasing."));
if (advice_status_hints)
status_printf_ln(s, color,
_(" (all conflicts fixed: run \"git rebase --continue\")"));
- } else if (split_commit_in_progress(s)) {
+ } else if (state->split_in_progress) {
status_printf_ln(s, color, _("You are currently splitting a commit during a rebase."));
if (advice_status_hints)
status_printf_ln(s, color,
@@ -907,7 +905,7 @@ static void show_cherry_pick_in_progress(struct wt_status *s,
{
status_printf_ln(s, color, _("You are currently cherry-picking."));
if (advice_status_hints) {
- if (has_unmerged(s))
+ if (state->has_unmerged)
status_printf_ln(s, color,
_(" (fix conflicts and run \"git commit\")"));
else
@@ -955,6 +953,10 @@ static void wt_status_get_state(struct wt_status *s , struct wt_status_state *st
}
if (!stat(git_path("BISECT_LOG"), &st))
state->bisect_in_progress = 1;
+
+ state->has_unmerged = has_unmerged(s);
+ state->split_in_progress = split_commit_in_progress(s);
+ state->commit_is_pending = !stat(git_path("MERGE_MSG"), &st);
}
static void wt_status_print_state(struct wt_status *s)
diff --git a/wt-status.h b/wt-status.h
index 236b41f..0b866a2 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -79,6 +79,9 @@ struct wt_status_state {
int rebase_interactive_in_progress;
int cherry_pick_in_progress;
int bisect_in_progress;
+ int split_in_progress;
+ int has_unmerged;
+ int commit_is_pending;
};
void wt_status_prepare(struct wt_status *s);
--
1.8.0.3.gde9c7d5.dirty
^ permalink raw reply related
* [PATCHv3 1/4] Refactor print_state into get_state
From: Phil Hord @ 2012-11-09 18:56 UTC (permalink / raw)
To: git
Cc: phil.hord, Jeff King, Junio C Hamano, konglu, Matthieu Moy,
Kong Lucien, Duperray Valentin, Jonas Franck, Nguy Thomas,
Phil Hord
In-Reply-To: <1352487385-5929-1-git-send-email-hordp@cisco.com>
Recently git-status learned to display the state of the git
sequencer in long form to help the user remember an interrupted
command. This information is useful to other callers who do
not want it printed in the same way.
Split the new print_state function into separate get_state and
print_state functions so others can get this state info and
share common code.
Signed-off-by: Phil Hord <hordp@cisco.com>
---
wt-status.c | 31 +++++++++++++++++++------------
1 file changed, 19 insertions(+), 12 deletions(-)
diff --git a/wt-status.c b/wt-status.c
index 2a9658b..760f52b 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -928,34 +928,41 @@ static void show_bisect_in_progress(struct wt_status *s,
wt_status_print_trailer(s);
}
-static void wt_status_print_state(struct wt_status *s)
+static void wt_status_get_state(struct wt_status *s , struct wt_status_state *state)
{
- const char *state_color = color(WT_STATUS_HEADER, s);
- struct wt_status_state state;
struct stat st;
- memset(&state, 0, sizeof(state));
+ memset(state, 0, sizeof(*state));
+ /* Determine sequencer activity */
if (!stat(git_path("MERGE_HEAD"), &st)) {
- state.merge_in_progress = 1;
+ state->merge_in_progress = 1;
} else if (!stat(git_path("rebase-apply"), &st)) {
if (!stat(git_path("rebase-apply/applying"), &st)) {
- state.am_in_progress = 1;
+ state->am_in_progress = 1;
if (!stat(git_path("rebase-apply/patch"), &st) && !st.st_size)
- state.am_empty_patch = 1;
+ state->am_empty_patch = 1;
} else {
- state.rebase_in_progress = 1;
+ state->rebase_in_progress = 1;
}
} else if (!stat(git_path("rebase-merge"), &st)) {
if (!stat(git_path("rebase-merge/interactive"), &st))
- state.rebase_interactive_in_progress = 1;
+ state->rebase_interactive_in_progress = 1;
else
- state.rebase_in_progress = 1;
+ state->rebase_in_progress = 1;
} else if (!stat(git_path("CHERRY_PICK_HEAD"), &st)) {
- state.cherry_pick_in_progress = 1;
+ state->cherry_pick_in_progress = 1;
}
if (!stat(git_path("BISECT_LOG"), &st))
- state.bisect_in_progress = 1;
+ state->bisect_in_progress = 1;
+}
+
+static void wt_status_print_state(struct wt_status *s)
+{
+ const char *state_color = color(WT_STATUS_HEADER, s);
+ struct wt_status_state state;
+
+ wt_status_get_state(s, &state);
if (state.merge_in_progress)
show_merge_in_progress(s, &state, state_color);
--
1.8.0.3.gde9c7d5.dirty
^ permalink raw reply related
* [PATCHv3 0/4] git-status short sequencer state info
From: Phil Hord @ 2012-11-09 18:56 UTC (permalink / raw)
To: git
Cc: phil.hord, Jeff King, Junio C Hamano, konglu, Matthieu Moy,
Kong Lucien, Duperray Valentin, Jonas Franck, Nguy Thomas
In-Reply-To: <1351553513-20385-1-git-send-email-hordp@cisco.com>
I think this series is fairly complete at this point. I am still
missing some tests in 4/4, but only because I am not sure how to
induce those conditions. Maybe I should enlist help from $(git-blame).
[PATCHv3 1/4] Refactor print_state into get_state
[PATCHv3 2/4] wt-status: More state retrieval abstraction
[PATCHv3 3/4] git-status: show short sequencer state
[PATCHv3 4/4] Add tests for git-status --sequencer
^ permalink raw reply
* Re: git commit/push can fail silently when clone omits ".git"
From: Heiko Voigt @ 2012-11-09 18:42 UTC (permalink / raw)
To: Jeff King; +Cc: Jeffrey S. Haemer, Jens Lehmann, Git Issues
In-Reply-To: <20121108185643.GN15560@sigill.intra.peff.net>
Hi,
On Thu, Nov 08, 2012 at 01:56:43PM -0500, Jeff King wrote:
> Unfortunately, the patch below which does that seems to make t7407 very
> unhappy. It looks like the submodule test uses "git clone ." and
> "git-submodule add" expects the "/." to still be at the end of the
> configured URL when processing relative submodule paths. I'm not sure if
> that is important, or an unnecessary brittleness in the submodule code.
>
> Jens, Heiko?
After some analysis it seems to me that the test deviates from the
expected behavior. For relative urls we have documented that if we have
a remote in the superproject a relative submodule path is relative to that
remotes url.
In the test super has been cloned from ".". So the tests root directory
should be the directory the submodule path is relative to. That would
be ./submodule (since submodule is also in the root directory) and not
../submodule.
Before your patch a "/." was added to the origin of super and "/." is
currently counted as a path component.
So we have another corner case here: When your superproject was cloned
from "." the urls you currently have to specify with submodule add are
wrong (one ".." to much).
Since this is a change in behaviour I would like to further think about
the implications this brings if we fix this. Not sure how many people
clone from ".". The correct behavior (as documented) is the one you
introduce with your patch. If we decide to fix this we should also correct
the path calculation in git-submodule.sh.
Cheers Heiko
^ permalink raw reply
* Re: [PATCH v2 0/5] push: update remote tags only with force
From: Jeff King @ 2012-11-09 18:38 UTC (permalink / raw)
To: Chris Rorvick
Cc: git, Felipe Contreras, Michael Haggerty, Angelo Borsotti,
Philip Oakley, Johannes Sixt, Kacper Kornet
In-Reply-To: <1352084908-32333-1-git-send-email-chris@rorvick.com>
On Sun, Nov 04, 2012 at 09:08:23PM -0600, Chris Rorvick wrote:
> Patch series to prevent push from updating remote tags w/o forcing them.
> Split out original patch to ease review.
>
> Chris Rorvick (5):
> push: return reject reasons via a mask
> push: add advice for rejected tag reference
> push: flag updates
> push: flag updates that require force
> push: update remote tags only with force
>
> Documentation/git-push.txt | 10 +++++-----
> builtin/push.c | 24 +++++++++++++++---------
> builtin/send-pack.c | 6 ++++++
> cache.h | 7 ++++++-
> remote.c | 39 +++++++++++++++++++++++++++++++--------
> t/t5516-fetch-push.sh | 30 +++++++++++++++++++++++++++++-
> transport-helper.c | 6 ++++++
> transport.c | 25 +++++++++++++++----------
> transport.h | 10 ++++++----
> 9 files changed, 119 insertions(+), 38 deletions(-)
I have not looked carefully at this topic yet, but I did try merging it
to "pu" and found that it had some textual conflicts with the
nd/builtin-to-libgit topic, which moves some builtin/send-pack.c code to
send-pack.c. Since I am graduating that topic to master, I went ahead
and just rebased your topic on top.
If you do a re-roll, please use an updated master, and feel free to
grab (and double-check!) the rebase I am about to send out in 'pu'. I
also included the minor signed/unsigned pointer warning fixup in the
rebase, too.
-Peff
^ permalink raw reply
* Re: [PATCH v2 1/5] push: return reject reasons via a mask
From: Jeff King @ 2012-11-09 18:35 UTC (permalink / raw)
To: Chris Rorvick
Cc: git, Felipe Contreras, Michael Haggerty, Angelo Borsotti,
Philip Oakley, Johannes Sixt, Kacper Kornet
In-Reply-To: <1352084908-32333-2-git-send-email-chris@rorvick.com>
On Sun, Nov 04, 2012 at 09:08:24PM -0600, Chris Rorvick wrote:
> Pass all rejection reasons back from transport_push(). The logic is
> simpler and more flexible with regard to providing useful feedback.
> [...]
>
> void transport_print_push_status(const char *dest, struct ref *refs,
> - int verbose, int porcelain, int *nonfastforward);
> + int verbose, int porcelain, unsigned int *reject_mask);
You missed updating one call-site of this function in
builtin/send-pack.c (gcc -Wall will now complain of the signed/unsigned
mismatch in the passed-in pointer).
-Peff
^ permalink raw reply
* Re: git merge commits are non-deterministic? what changed?
From: Ulrich Spörlein @ 2012-11-09 18:27 UTC (permalink / raw)
To: Jeff King; +Cc: Matthieu Moy, Andreas Schwab, git
In-Reply-To: <20121109161647.GB19725@sigill.intra.peff.net>
On Fri, 2012-11-09 at 11:16:47 -0500, Jeff King wrote:
> On Fri, Nov 09, 2012 at 04:52:48PM +0100, Matthieu Moy wrote:
>
> > Ulrich Spörlein <uqs@spoerlein.net> writes:
> >
> > >> > 2. Why the hell is the commit hash dependent on the ordering of the
> > >> > parent commits? IMHO it should sort the set of parents before
> > >> > calculating the hash ...
> > >>
> > >> What would be the sort key?
> > >
> > > Trivially, the hash of the parents itself. So you'd always get
> > >
> > > ...
> > > parent 0000
> > > parent 1111
> > > parent aaaa
> > > parent ffff
> >
> > That would change the behavior of --first-parent. Or you'd need to
> > compute the sha1 of the sorted list, but keep the unsorted one in the
> > commit. Possible, but weird ;-).
>
> Right. The reason that merge parents are stored in the order given on
> the command line is not random or because it was not considered. It
> encodes a valuable piece of information: did the user merge "foo" into
> "bar", or did they merge "bar" into "foo"?
>
> So I think this discussion is going in the wrong direction; git should
> never sort the parents, because the order is meaningful. The original
> complaint was that a run of svn2git produced different results on two
> different git versions. The important question to me is: did svn2git
> feed the parents to git in the same order?
>
> If it did, and git produced different results, then that is a serious
> bug.
>
> If it did not, then the issue needs to be resolved in svn2git (which
> _may_ want to sort the parents that it feeds to git, but it would depend
> on whether the order it is currently presenting is meaningful).
Yeah, thanks, looks like I have some more work to do. I don't quite get
how it could come up with a different order, seeing that it is using svn
as the base.
Will run some more experiments, thanks for the info so far.
Cheers,
Uli
^ permalink raw reply
* Re: [PATCHv2] replace: parse revision argument for -d
From: Jeff King @ 2012-11-09 16:48 UTC (permalink / raw)
To: Michael J Gruber; +Cc: git
In-Reply-To: <24b0f81315ddab8cc37133d5b3bec8aec90ed652.1351516888.git.git@drmicha.warpmail.net>
On Mon, Oct 29, 2012 at 02:23:27PM +0100, Michael J Gruber wrote:
> 'git replace' parses the revision arguments when it creates replacements
> (so that a sha1 can be abbreviated, e.g.) but not when deleting
> replacements.
>
> Make it parse the arguments to 'replace -d' in the same way.
>
> Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
> ---
> v2 has the simplified error check as per Jeff, and a reworded message.
> Comes with a free test case, too.
I noticed this today in my pile of "to look at" patches. Sorry for being
slow.
> for (p = argv; *p; p++) {
> - if (snprintf(ref, sizeof(ref), "refs/replace/%s", *p)
> - >= sizeof(ref)) {
> - error("replace ref name too long: %.*s...", 50, *p);
> + q = *p;
> + if (get_sha1(q, sha1)) {
> + error("Failed to resolve '%s' as a valid ref.", q);
> had_error = 1;
> continue;
> }
Looks reasonable.
> + q = sha1_to_hex(sha1);
> + snprintf(ref, sizeof(ref), "refs/replace/%s", q);
> if (read_ref(ref, sha1)) {
> - error("replace ref '%s' not found.", *p);
> + error("replace ref '%s' not found.", q);
I worry a little about assuming that "q", which points to a static
internal buffer of sha1_to_hex, is still valid after calling read_ref.
We'll end up in resolve_ref, which might need to do considerable work
(e.g., loading the whole packed refs file). Just grepping for
sha1_to_hex, I don't think it is a problem currently, but it might be
worth copying the value (you could even point into the "ref" buffer to
avoid dealing with an extra allocation).
Other than that, it looks good to me.
-Peff
^ permalink raw reply
* Re: [PATCH v3 2/3] git-submodule foreach: export .gitmodules settings as variables
From: Heiko Voigt @ 2012-11-09 16:45 UTC (permalink / raw)
To: W. Trevor King
Cc: Git, Jeff King, Phil Hord, Shawn Pearce, Jens Lehmann, Nahor
In-Reply-To: <2121ce36cf4eb02385255cbd5b0bbd1dcc803113.1352431675.git.wking@tremily.us>
Hi,
On Thu, Nov 08, 2012 at 10:35:13PM -0500, W. Trevor King wrote:
> From: "W. Trevor King" <wking@tremily.us>
>
> This makes it easy to access per-submodule variables. For example,
>
> git submodule foreach 'git checkout $(git config --file $toplevel/.gitmodules submodule.$name.branch) && git pull'
>
> can now be reduced to
>
> git submodule foreach 'git checkout $submodule_branch && git pull'
What other use cases are there? Would the need for this maybe go away
once you had floating submodules following branches?
The whole thing looks like its adding some complex code which is not so
easy to read. I would like to make sure its worth it.
> diff --git a/git-submodule.sh b/git-submodule.sh
> index bc33112..e4d26f9 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -434,8 +434,24 @@ cmd_foreach()
> clear_local_git_env
> # we make $path available to scripts ...
> path=$sm_path
> +
> + # make all submodule variables available to scripts
> + eval $(
> + git config -f .gitmodules --get-regexp "^submodule\.${name}\..*" |
For completeness you should make the variables possible to override by
repository from the local repository configuration like all other
submodule options that are read directly from .gitmodules.
Cheers Heiko
^ permalink raw reply
* Re: [PATCH] gitweb: make remote_heads config setting work.
From: Jeff King @ 2012-11-09 16:37 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Phil Pennock, git
In-Reply-To: <7vk3tvqthw.fsf@alter.siamese.dyndns.org>
On Thu, Nov 08, 2012 at 08:40:11PM -0800, Junio C Hamano wrote:
> Looking at the code before this part:
>
> if (my ($hi, $mi, $lo) = ($key =~ /^([^.]*)\.(.*)\.([^.]*)$/)) {
> $key = join(".", lc($hi), $mi, lc($lo));
> } else {
> $key = lc($key);
> }
> $key =~ s/^gitweb\.//;
> return if ($key =~ m/\W/);
>
> the new code is munding the $hi and $mi parts, while the mistaken
> configuration this patch is trying to correct is about the $lo part,
> and possibly the $hi part, but never the $mi part.
Good catch. I think the "return" in the existing code suffers from the
same problem: it will bail on non-word characters in the $mi part, but
that part should allow arbitrary characters.
-Peff
^ permalink raw reply
* Re: git-clone and unreliable links?
From: Sitaram Chamarty @ 2012-11-09 16:35 UTC (permalink / raw)
To: Shawn Pearce; +Cc: Josef Wolf, git
In-Reply-To: <CAJo=hJtJPnQFjvz8AAQjf5Rze-_99vF_tYi9CqJ7fiW245Dv1w@mail.gmail.com>
On Wed, Nov 7, 2012 at 9:24 PM, Shawn Pearce <spearce@spearce.org> wrote:
> On Wed, Nov 7, 2012 at 7:35 AM, Josef Wolf <jw@raven.inka.de> wrote:
>> When using git-clone over an unreliable link (say, UMTS) and the network goes
>> down, git-clone deletes everything what was downloaded. When the network goes
>> up again and you restart git-clone, it has to start over from the
>> beginning. Then, eventually, the network goes down again, and everything is
>> deleted again.
>>
>> Is there a way to omit the deleting step, so the second invocation would start
>> where the first invocation was interrupted?
>
> No, because a clone is not resumable.
>
> The best way to obtain a repository over an unstable link is to ask
> the repository owner to make a bundle file with `git bundle create
> --heads --tags` and serve the file using standard HTTP or rsync, which
> are resumable protocols. After you download the file, you can clone or
> fetch from the bundle to initialize your local repository, and then
> run git fetch to incrementally update to anything that is more recent
> than the bundle's creation.
If the server is running gitolite, the admin can set it up so that a
bundle file is automatically created as needed (including "don't do it
more than once per <duration>" logic), and serve it up over rsync
using the same ssh credentials as for access to the repo itself.
However, this is not particularly useful for systems with git://,
although it could certainly be *adapted* for http access.
[Documentation is inline, in src/commands/rsync, for people who wish to know.]
--
Sitaram
^ permalink raw reply
* Re: [PATCH v3 1/3] git-submodule add: Add -r/--record option
From: Heiko Voigt @ 2012-11-09 16:29 UTC (permalink / raw)
To: Junio C Hamano
Cc: W. Trevor King, Git, Jeff King, Phil Hord, Shawn Pearce,
Jens Lehmann, Nahor
In-Reply-To: <7v390jqlep.fsf@alter.siamese.dyndns.org>
Hi,
On Thu, Nov 08, 2012 at 11:34:54PM -0800, Junio C Hamano wrote:
> "W. Trevor King" <wking@tremily.us> writes:
>
> > By remaining agnostic on the variable usage, this patch makes
> > submodule setup more convenient for all parties.
>
> I personally do not think "remaining agnostic on the usage" is a
> good thing, at least for any option to commands at the higher level
> on the stack, such as "git submodule". I am afraid that giving an
> easier way to set up a variable with undefined semantics may make
> setup more confusing for all parties. One party gives one specific
> meaning to the field, while another party uses it for something
> slightly different.
>
> I would not object to "git config submodule.$name.branch $value", on
> the other hand. "git config" can be used to set a piece of data
> that has specific meaning, but as a low-level tool, it is not
> _limited_ to variables that have defined meaning.
I think we should agree on a behavior for this option and implement it
the same time when add learns about it. When we were discussing floating
submodules as an important option for the gerrit people I already started
to implement a proof of concept. Please have a look here:
https://github.com/hvoigt/git/commits/hv/floating_submodules
AFAIK this does not yet implement the same behaviour the gerrit tools
offer for this option. The main reason behind that was because I do not
know the typical workflow behind such an option. So I am open to
changes.
Maybe you can use or base your work on this implementation for submodule
update.
Without submodule update using this option I think it would be better to
implement this option in the tool you are using instead of submodule add.
Everything else feels incomplete to me.
Cheers Heiko
^ permalink raw reply
* Re: bash completion of "addable" files for `git add`
From: Jeff King @ 2012-11-09 16:27 UTC (permalink / raw)
To: Andreas Zeidler; +Cc: git, gitster
In-Reply-To: <5E69B894-C392-4DD5-A4F1-723DA1A3D059@zitc.de>
On Fri, Nov 09, 2012 at 02:22:27PM +0100, Andreas Zeidler wrote:
> so here's a patch adding bash completion on `git add` for modified,
> updated and untracked files. i've also set up a pull request — before
> i found `Documentation/SubmittingPatches`. fwiw, it's at
> https://github.com/git/git/pull/29
Please put cover letter material like this after the "---" divider, or
include a "-- >8 --" scissors line before your commit. Either helps "git
am" realize which content should go into the commit message.
> From cbac6caee7e788569562cb7138eb698cc46a1b8f Mon Sep 17 00:00:00 2001
> From: Andreas Zeidler <az@zitc.de>
> Date: Fri, 9 Nov 2012 13:05:43 +0100
Please omit these lines, as they are redundant with your email header.
> Subject: [PATCH] add bash completion for "addable" files
This one is useful, but it would be even better if it were the subject
of your email. :)
> +__git_addable ()
> +{
> + local dir="$(__gitdir)"
> + if [ -d "$dir" ]; then
> + git --git-dir="$dir" status --short --untracked=all |\
> + egrep '^.[UM?] ' | sed 's/^.. //'
> + return
> + fi
> +}
You would want to use the stable, scriptable "--porcelain" output, so
the completion is not broken by future changes to the "--short" format.
However, I do not think using "git status" is the best option. It
computes three things:
1. The diff between HEAD and index.
2. The diff between index and working tree.
3. The list of untracked files.
But you only care about (2) and (3), so you are wasting time computing
(1). I think the list you want could be generated with:
git diff-files --name-only
git ls-files --exclude-standard -o
and then you do not need to worry about grep or sed at all.
> @@ -810,6 +820,11 @@ _git_add ()
> --ignore-errors --intent-to-add
> "
> return
> + ;;
> + *)
> + __gitcomp "$(__git_addable)"
> + return
> + ;;
I think you'd want to use __gitcomp_nl to handle filenames with spaces.
Speaking of which, the output of status (or of the commands I mentioned)
may have quoting applied to pathnames. I think that is not something we
handle very well right now in the completion, so it may not be worth
worrying about for this particular patch.
Other than those comments, I think the intent of your patch makes a lot
of sense.
-Peff
^ permalink raw reply
* Re: git merge commits are non-deterministic? what changed?
From: Jeff King @ 2012-11-09 16:16 UTC (permalink / raw)
To: Matthieu Moy; +Cc: Ulrich Spörlein, Andreas Schwab, git
In-Reply-To: <vpq390idb8v.fsf@grenoble-inp.fr>
On Fri, Nov 09, 2012 at 04:52:48PM +0100, Matthieu Moy wrote:
> Ulrich Spörlein <uqs@spoerlein.net> writes:
>
> >> > 2. Why the hell is the commit hash dependent on the ordering of the
> >> > parent commits? IMHO it should sort the set of parents before
> >> > calculating the hash ...
> >>
> >> What would be the sort key?
> >
> > Trivially, the hash of the parents itself. So you'd always get
> >
> > ...
> > parent 0000
> > parent 1111
> > parent aaaa
> > parent ffff
>
> That would change the behavior of --first-parent. Or you'd need to
> compute the sha1 of the sorted list, but keep the unsorted one in the
> commit. Possible, but weird ;-).
Right. The reason that merge parents are stored in the order given on
the command line is not random or because it was not considered. It
encodes a valuable piece of information: did the user merge "foo" into
"bar", or did they merge "bar" into "foo"?
So I think this discussion is going in the wrong direction; git should
never sort the parents, because the order is meaningful. The original
complaint was that a run of svn2git produced different results on two
different git versions. The important question to me is: did svn2git
feed the parents to git in the same order?
If it did, and git produced different results, then that is a serious
bug.
If it did not, then the issue needs to be resolved in svn2git (which
_may_ want to sort the parents that it feeds to git, but it would depend
on whether the order it is currently presenting is meaningful).
-Peff
^ permalink raw reply
* Re: Rename edge case...
From: Jeff King @ 2012-11-09 16:09 UTC (permalink / raw)
To: John Szakmeister; +Cc: git
In-Reply-To: <CAEBDL5U+OSTCAqgWoApE_m21Nef24Wqvt78oB6qqV4oEvU0vXQ@mail.gmail.com>
On Fri, Nov 09, 2012 at 04:10:31AM -0500, John Szakmeister wrote:
> I've been browsing StackOverflow answering git-related questions, and
> ran across this one:
> <http://stackoverflow.com/questions/13300675/git-merge-rename-conflict>
>
> It's a bit of an interesting situation. The user did a couple of
> renames in a branch:
> foo.txt => fooOld.txt
> fooNew.txt => foo.txt
>
> Meanwhile, master had an update to fooNew.txt. When the user tried to
> merge master to the branch, it gave a merge conflict saying fooNew.txt
> was deleted, but master tried to update it.
>
> I was a bit surprised that git didn't follow the rename here, though I
> do understand why: git only sees it as a rename if the source
> disappears completely.
Right. If the source didn't go away, it would be a copy. We can do copy
detection, but it is not quite as obvious what a merge should do with a
copy (apply the change to the original? To the copy? In both places? You
would really want hunk-level copy detection for it to make any sense).
Usually git deals with this double-rename case through the use of
"break" or "rewrite" detection. We notice that the old "foo.txt" and the
new "foo.txt" do not look very much like each other, and break the
modification apart into an add and a delete. That makes each side
eligible for rename detection, and we can end up finding the pairs of
renames above.
So in theory it just as simple as a one-liner to turn on break-detection
in merge-recursive. Sadly, that only reveals more issues with how
merge-recursive handles renames. See this thread, which has pointers to
the breakages at the end:
http://thread.gmane.org/gmane.comp.version-control.git/169944
I've become convinced that the best way forward with merge-recursive is
to scrap and rewrite it. It tries to do things in a muddled order, which
makes it very brittle to changes like this. I think it needs to have an
internal representation of the tree that can represent all of the
conflicts, and then follow a few simple phases:
1. "structural" 3-way merge handling renames, breaks, typechanges,
etc. Each path in tree might show things like D/F conflicts, or it
might show content-level merges that still need to happen, even if
the content from those merges is not coming from the same paths in
the source trees.
2. Resolve content-level 3-way merges at each path.
3. Compare the proposed tree to the working tree and list any problems
(e.g., untracked files or local modifications that will be
overwritten).
Right now it tries to do these things interleaved as it processes paths,
and as a result we've had many bugs (e.g., the content-level merge
conflating the content originally at a path and something that was
renamed into place, and missing corner cases where we actually overwrite
untracked files that should be considered precious).
But that is just off the top of my head. I haven't looked at the topic
in quite a while (and I haven't even started working on any such
rewrite).
> So I played locally with a few ideas, and was surprised to find out
> that even breaking up the two renames into two separate commits git
> still didn't follow it.
Right, because the merge only looks at the end points. Try doing a
"diff -M" between your endpoints with and without "-B". We do not have
any double-renames in git.git, but you can find "-B" helping a similar
case: most of a file's content is moved elsewhere, but some small amount
remains. For example, try this in git.git, with and without -B:
git show -M --stat --summary --patch 043a449
It finds the rename only with "-B", which would help a merge (it also
makes the diff shorter and more readable, as you can see what was
changed as the content migrated to the new file).
-Peff
^ permalink raw reply
* Re: git merge commits are non-deterministic? what changed?
From: Matthieu Moy @ 2012-11-09 15:52 UTC (permalink / raw)
To: Ulrich Spörlein; +Cc: Andreas Schwab, git
In-Reply-To: <20121109154245.GP69724@acme.spoerlein.net>
Ulrich Spörlein <uqs@spoerlein.net> writes:
>> > 2. Why the hell is the commit hash dependent on the ordering of the
>> > parent commits? IMHO it should sort the set of parents before
>> > calculating the hash ...
>>
>> What would be the sort key?
>
> Trivially, the hash of the parents itself. So you'd always get
>
> ...
> parent 0000
> parent 1111
> parent aaaa
> parent ffff
That would change the behavior of --first-parent. Or you'd need to
compute the sha1 of the sorted list, but keep the unsorted one in the
commit. Possible, but weird ;-).
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply
* Re: git merge commits are non-deterministic? what changed?
From: Ulrich Spörlein @ 2012-11-09 15:42 UTC (permalink / raw)
To: Andreas Schwab; +Cc: git
In-Reply-To: <m2y5iarf5s.fsf@igel.home>
On Fri, 2012-11-09 at 16:04:31 +0100, Andreas Schwab wrote:
> Ulrich Spörlein <uqs@spoerlein.net> writes:
>
> > Two questions:
> > 1. Can we impose a stable ordering of the commits being recorded in a
> > merge commit? Listing parents in chronological order or something like
> > that.
>
> The order is determined by the order the refs are given to git merge (or
> git commit-tree when using the plumbing).
>
> > 2. Why the hell is the commit hash dependent on the ordering of the
> > parent commits? IMHO it should sort the set of parents before
> > calculating the hash ...
>
> What would be the sort key?
Trivially, the hash of the parents itself. So you'd always get
...
parent 0000
parent 1111
parent aaaa
parent ffff
hth
Uli
^ permalink raw reply
* Re: git merge commits are non-deterministic? what changed?
From: Andreas Schwab @ 2012-11-09 15:04 UTC (permalink / raw)
To: Ulrich Spörlein; +Cc: git
In-Reply-To: <20121109133132.GK69724@acme.spoerlein.net>
Ulrich Spörlein <uqs@spoerlein.net> writes:
> Two questions:
> 1. Can we impose a stable ordering of the commits being recorded in a
> merge commit? Listing parents in chronological order or something like
> that.
The order is determined by the order the refs are given to git merge (or
git commit-tree when using the plumbing).
> 2. Why the hell is the commit hash dependent on the ordering of the
> parent commits? IMHO it should sort the set of parents before
> calculating the hash ...
What would be the sort key?
Andreas.
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
^ permalink raw reply
* Re: RFD: fast-import is picky with author names (and maybe it should - but how much so?)
From: Felipe Contreras @ 2012-11-09 14:34 UTC (permalink / raw)
To: Michael J Gruber; +Cc: Jeff King, Git Mailing List
In-Reply-To: <509CCCBC.8010102@drmicha.warpmail.net>
On Fri, Nov 9, 2012 at 10:28 AM, Michael J Gruber
<git@drmicha.warpmail.net> wrote:
> Hg seems to store just anything in the author field ("committer"). The
> various interfaces that are floating around do some behind-the-back
> conversion to git format. The more conversions they do, the better they
> seem to work (no erroring out) but I'm wondering whether it's really a
> good thing, or whether we should encourage a more diligent approach
> which requires a user to map non-conforming author names wilfully.
So you propose that when somebody does 'git clone hg::hg hg-git' the
thing should fail. I hope you don't think it's too unbecoming for me
to say that I disagree.
IMO it should be git fast-import the one that converts these bad
authors, not every single tool out there. Maybe throw a warning, but
that's all. Or maybe generate a list of bad authors ready to be filled
out. That way when a project is doing a real conversion, say, when
moving to git, they can run the conversion once and see which authors
are bad and not multiple times, each try taking longer than the next.
--
Felipe Contreras
^ permalink raw reply
* Re: Revert option for git add --patch
From: Bogolisk @ 2012-11-09 14:09 UTC (permalink / raw)
To: git
In-Reply-To: <EE89F0A1-1C07-4597-B654-035F657AD09F@me.com>
Jonathon Mah <jmah <at> me.com> writes:
>
> Nathan,
>
> I find myself performing similar actions to you: using git add -p to stage
hunks, sometimes editing the
> staged patch; and keeping mental notes of things I wanted to revert, sometimes
changing them in the editor
> in another window, and sometimes reverting them after the add session with git
checkout -p).
Other front-ends like Egg and Magit has been providing the ability to move hunks
back and forth, for a long time.
^ permalink raw reply
* git merge commits are non-deterministic? what changed?
From: Ulrich Spörlein @ 2012-11-09 13:31 UTC (permalink / raw)
To: git
Hi all,
I'm running a couple of conversions from SVN to git, using a slightly
hacked version of svn2git (because it can cope with multiple branches
and is several orders of magnitude faster than git-svn).
Anyway, when doing some verification runs, using the same version of
svn2git, but different versions of git, I get different commit hashes,
and I tracked it down to the ordering of the parents inside a merge
commit.
version 1.7.9.2
% git show --format=raw e209a83|head
commit e209a83c1e0a387c88a44f3a8f2be2670ed85eae
tree de2d7c6726a45428d4a310da2acd8839daf9f85f
parent 5fba0401c23a594e4ad5e807bf14a5439645a358
parent 25062ba061871945759b3baa833fe64969383e40
parent 89bebeef185ed08424fc548f8569081c6add2439
parent c7d5f60d3a7e2e3c4da23b157c62504667344438
parent e7bc108f0d6a394050818a4af64a59094d3c793e
parent 48231afadc40013e6bfda56b04a11ee3a602598f
author rgrimes <rgrimes@FreeBSD.org> 739897097 +0000
committer rgrimes <rgrimes@FreeBSD.org> 739897097 +0000
vs
git version 1.8.0
% git show --format=raw 42f0fad|head
commit 42f0fadccab6eefc7ffdc1012345b42ad45e36c2
tree de2d7c6726a45428d4a310da2acd8839daf9f85f
parent 5fba0401c23a594e4ad5e807bf14a5439645a358
parent 25062ba061871945759b3baa833fe64969383e40
parent 89bebeef185ed08424fc548f8569081c6add2439
parent 48231afadc40013e6bfda56b04a11ee3a602598f
parent c7d5f60d3a7e2e3c4da23b157c62504667344438
parent e7bc108f0d6a394050818a4af64a59094d3c793e
author rgrimes <rgrimes@FreeBSD.org> 739897097 +0000
committer rgrimes <rgrimes@FreeBSD.org> 739897097 +0000
I haven't verified to see if that ordering is stable within a git
version, but the fact that it changed across versions clearly means that
I cannot depend on this currently (I have never seen this problem in two
years, so I blame git 1.8.0 ...)
Two questions:
1. Can we impose a stable ordering of the commits being recorded in a
merge commit? Listing parents in chronological order or something like
that.
2. Why the hell is the commit hash dependent on the ordering of the
parent commits? IMHO it should sort the set of parents before
calculating the hash ...
Help?
Uli
^ permalink raw reply
* Re: git svn problem, probably a regression
From: Joseph Crowell @ 2012-11-09 13:39 UTC (permalink / raw)
To: git
In-Reply-To: <20121108180657.GM15560@sigill.intra.peff.net>
> > Use of uninitialized value $u in substitution (s///) at
/usr/local/Cellar/git/1.8.0/lib/Git/SVN.pm
> line 106.
> > Use of uninitialized value $u in concatenation (.) or string at
> /usr/local/Cellar/git/1.8.0/lib/Git/SVN.pm line 106.
> > refs/remotes/svn/asset-manager-redesign: 'svn+ssh://<IP address>' not found
in ''
>
> If you have time and can reproduce the bug at will, it would be very
> helpful to use "git-bisect" to pinpoint the exact commit that causes the
> breakage.
>
> -Peff
>
I just encountered the exact same issue while converting an svn repo to git on
Windows through Git Bash. Same error.
^ permalink raw reply
* bash completion of "addable" files for `git add`
From: Andreas Zeidler @ 2012-11-09 13:22 UTC (permalink / raw)
To: git; +Cc: gitster
hi,
i've always been annoyed by having to type — or copy & paste — paths when i wanted to simply add some files to the index. i know about the `add -i` interface, and that improves things a little, but having bash completion for so many other things it still seemed to much hassle.
so here's a patch adding bash completion on `git add` for modified, updated and untracked files. i've also set up a pull request — before i found `Documentation/SubmittingPatches`. fwiw, it's at https://github.com/git/git/pull/29
best regards,
andi
From cbac6caee7e788569562cb7138eb698cc46a1b8f Mon Sep 17 00:00:00 2001
From: Andreas Zeidler <az@zitc.de>
Date: Fri, 9 Nov 2012 13:05:43 +0100
Subject: [PATCH] add bash completion for "addable" files
this adds support for completing only modified, updated and untracked
files on `git add`, since almost always these are what you want to add
to the index.
the list of possible completions is determined using `git status` and
filtered using `egrep` and `sed`.
Signed-off-by: Andreas Zeidler <az@zitc.de>
---
contrib/completion/git-completion.bash | 15 +++++++++++++++
t/t9902-completion.sh | 11 +++++++++++
2 files changed, 26 insertions(+)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index be800e0..4aa0981 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -799,6 +799,16 @@ _git_apply ()
COMPREPLY=()
}
+__git_addable ()
+{
+ local dir="$(__gitdir)"
+ if [ -d "$dir" ]; then
+ git --git-dir="$dir" status --short --untracked=all |\
+ egrep '^.[UM?] ' | sed 's/^.. //'
+ return
+ fi
+}
+
_git_add ()
{
__git_has_doubledash && return
@@ -810,6 +820,11 @@ _git_add ()
--ignore-errors --intent-to-add
"
return
+ ;;
+ *)
+ __gitcomp "$(__git_addable)"
+ return
+ ;;
esac
COMPREPLY=()
}
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index cbd0fb6..a7c81d3 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -288,4 +288,15 @@ test_expect_failure 'complete tree filename with metacharacters' '
EOF
'
+test_expect_success 'add completes untracked and modified names' '
+ echo other content >file1 &&
+ echo content >file2 &&
+ echo more >file3 &&
+ git add file1 &&
+ test_completion_long "git add f" <<-\EOF
+ file2_
+ file3_
+ EOF
+'
+
test_done
--
1.8.0.1.g43af610
--
pgp key at http://zitc.de/pgp - http://wwwkeys.de.pgp.net/
upgrade to plone 4! -- http://plone.org/4
^ permalink raw reply related
* Re: Rename edge case...
From: Johannes Sixt @ 2012-11-09 13:17 UTC (permalink / raw)
To: John Szakmeister; +Cc: Tomas Carnecky, git
In-Reply-To: <CAEBDL5WeQEWdyaJuuNbnnQbbsLYv8NO1ZSj3eHHpjW+ToS9X1A@mail.gmail.com>
Am 11/9/2012 11:25, schrieb John Szakmeister:
> On Fri, Nov 9, 2012 at 4:27 AM, Tomas Carnecky <tomas.carnecky@gmail.com> wrote:
> [snip]
>> When merging two branches, git only looks at the tips. It doesn't inspect
>> their histories to see how the files were moved around. So i doesn't matter
>> whether you rename the files in a single commit or multiple commits. The
>> resulting tree is always the same.
>
> I guess I figured that when I saw the final result, but didn't know if
> there was a way to coax Git into doing a better job here.
If the renames are split in two commits, you can merge the first, and then
the second on top of the result.
-- Hannes
^ permalink raw reply
* Re: [PATCH] cache-tree: invalidate i-t-a paths after writing trees
From: Junio C Hamano @ 2012-11-09 11:57 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: git, Jeff King, Jonathon Mah
In-Reply-To: <1352459040-14452-1-git-send-email-pclouds@gmail.com>
Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
> diff --git a/cache-tree.c b/cache-tree.c
> index 28ed657..30a8018 100644
> --- a/cache-tree.c
> +++ b/cache-tree.c
> @@ -381,6 +381,9 @@ int cache_tree_update(struct cache_tree *it,
> i = update_one(it, cache, entries, "", 0, flags);
> if (i < 0)
> return i;
> + for (i = 0; i < entries; i++)
> + if (cache[i]->ce_flags & CE_INTENT_TO_ADD)
> + cache_tree_invalidate_path(it, cache[i]->name);
> return 0;
> }
I notice there is another special case for CE_REMOVE but there is
nothing that adjusts the cache-tree for these entries in the current
codebase.
I suspect the original code before we (perhaps incorrectly) updated
the code not to error out upon I-T-A entries was fine only because
we do not attempt to fully populate the cache-tree during a merge in
the unpack-trees codepath, which will mark the index entries that
are to be removed with CE_REMOVE in the resulting index.
The solution implemented with this patch will break if we start
updating the cache tree after a successful merge in unpack-trees, I
suspect.
An alternative might be to add a "phoney" bit next to "used" in the
cache_tree structure, mark the cache tree as phoney when we skip an
entry marked as CE_REMOVE or CE_ITA, and make the postprocessing
loop this patch adds aware of that bit, instead of iterating over
the index entries; instead, it would recurse the resulting cache
tree and invalidate parts of the tree that have subtrees with the
"phoney" bit set, or something.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox