Re: [PATCH 5/6] status: do not depend on flaky reflog messages
Junio C Hamano wrote:
>> What is wrong with git describe? Is this cheaper, or am I missing something?
>
> I think what you are missing is that the "detached from" is not
> about your current HEAD after you flipped it around with many resets
> and commits. It is about what tag or what specific commit you
> detached your HEAD at originally.
No, it is about what tag of specific commit you detached your HEAD
from, *without using checkout*. If you used checkout, you'd get the
"detached at" message, and I haven't changed that.
> "You can ask the same to describe" is wrong and is not a valid
> argument. Once you replace the HEAD^ with a distant commit
> (e.g. HEAD~400) in the third step in the example, "describe" will
> not talk about v1.8.2 at all.
You're missing the point: how do I, as the end-user, detach my HEAD
*without using checkout*? The hypothetical example you have given is:
$ git checkout HEAD^
$ git update-ref HEAD $(git rev-parse HEAD~400)
Which end-user executes that?
> Your argument can be that it is not a useful piece of information,
> and as you can probably guess from my "I wouldn't be surprised"
> above, I am not sure how useful it would be and in what situation
> [*1*].
Precisely. It is a poorly thought-out feature that locks things up
too tightly, and makes life hell for contributors. It must therefore
be removed.
> But the original commit thought it was necessary and that was done
> for a reason; we need to be careful here. With a good justification
> why it is not necessary (or misleading to the user), I do not think
> we cannot change it.
We cannot reverse-engineer intents. All we can do is look at the
evidence in front of us. Read the commit message, and look at the
newly added test. There is absolutely no indication about why this
"detached from" is useful, and where.
> $ git rebase master
> ... replays some but stops
> $ git status
>
> currently uses that "HEAD detached from" codepath, but I think that
> is a mistake. We could not just tell the HEAD is detached, but the
> reason _why_ the HEAD is detached (i.e. we are in the middle of a
> rebase). The prompt script can do it, "status" should be able to do
> the same, and do a lot more sensible thing than unconditionally
> showing that "HEAD detached from" and then say "You are currently
> rebasing" on a separate line. Most likely we do not want to even
> say "Not currently on any branch" but just say "You are currently
> rebasing branch X on top of Y" (and perhaps "N commits remaining to
> be replayed").
That information is available in .git/{rebase-apply,rebase-merge}, and
your suggestion pertains to improving show_rebase_in_progress(). The
first line is about the state of HEAD, and is completely orthogonal to
the issue at hand.
artagnon|rebase-rev-failure|REBASE-i 2/3:~/src/git$ git status
# HEAD detached from a7e9fd4
# You are currently editing a commit while rebasing branch
'rebase-rev-failure' on '9926f66'.
#
nothing to commit, working directory clean
That first line about "HEAD detached from ..." is completely useless.
And yes, my prompt is more useful. No prizes for guessing how often I
use the long form of git status.
> *1* One thing I could think of is to start sightseeing or (more
> realistically) manually bisecting at a given release point,
> reset the detached HEAD around several times, and then want to
> be reminded where the session started from. I do not think it
> is particularly a very good example, though.
The example you have given now is:
$ git checkout @^
# or whatever bisect command to detach HEAD
$ git reset @~3
...
$ git reset @^
...
$ git reset @~5
# when was HEAD originally detached?
Yes, it is a contrived example where this feature arguably has some
utility. Is it worth putting the information in the status for such
an esoteric example? If one really wants this information, they can
open up the reflog and grep for "checkout: ".
--
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/2] [submodule] handle multibyte characters in name
Many "git submodule" operations do not work on a submodule at a path whose
name is not in ASCII.
This is because "git ls-files" is used to find which paths are bound to
submodules to the current working tree, and the output is C-quoted by default
for non ASCII pathnames.
Tell "git ls-files" to not C-quote its output, which is easier than unwrapping
C-quote ourselves.
Solution-suggested-by: Junio C Hamano
Signed-off-by: Fredrik Gustafsson
---
git-submodule.sh | 2 +-
t/t7400-submodule-basic.sh | 12
2 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/git-submodule.sh b/git-submodule.sh
index 79bfaac..bad051e 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -113,7 +113,7 @@ resolve_relative_url ()
module_list()
{
(
- git ls-files --error-unmatch --stage -- "$@" ||
+ git -c core.quotepath=false ls-files --error-unmatch --stage --
"$@" ||
echo "unmatched pathspec exists"
) |
perl -e '
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index ff26535..d5743ee 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -868,4 +868,16 @@ test_expect_success 'submodule deinit fails when submodule
has a .git directory
test -n "$(git config --get-regexp "submodule\.example\.")"
'
+test_expect_success 'submodule with strange name works "å äö"' '
+ mkdir "å äö" &&
+ (
+ cd "å äö" &&
+ git init &&
+ touch sub
+ git add sub
+ git commit -m "init sub"
+ )
+ git submodule add "/å äö" &&
+ test -n "$(git submodule | grep "å äö")"
+'
test_done
--
1.8.3.1.381.g2ab719e.dirty
--
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/2] [submodule] Replace perl-code with sh
This will prevent a fork and makes the code similair to the rest of the
file.
In the long term git-submodule.sh needs to use something else than sh to
handle newline in filenames (and therefore needs to use a language that
accepts \0 in strings). However I don't think that keeping that small
perl-part will ease any rewrite.
Signed-off-by: Fredrik Gustafsson
---
git-submodule.sh | 51 ++-
1 file changed, 22 insertions(+), 29 deletions(-)
diff --git a/git-submodule.sh b/git-submodule.sh
index bad051e..be96934 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -112,38 +112,31 @@ resolve_relative_url ()
#
module_list()
{
+ null_sha1=
+ unmerged=
(
git -c core.quotepath=false ls-files --error-unmatch --stage --
"$@" ||
- echo "unmatched pathspec exists"
+ echo "#unmatched"
) |
- perl -e '
- my %unmerged = ();
- my ($null_sha1) = ("0" x 40);
- my @out = ();
- my $unmatched = 0;
- while () {
- if (/^unmatched pathspec/) {
- $unmatched = 1;
- next;
- }
- chomp;
- my ($mode, $sha1, $stage, $path) =
- /^([0-7]+) ([0-9a-f]{40}) ([0-3])\t(.*)$/;
- next unless $mode eq "16";
- if ($stage ne "0") {
- if (!$unmerged{$path}++) {
- push @out, "$mode $null_sha1 U\t$path\n";
- }
- next;
- }
- push @out, "$_\n";
- }
- if ($unmatched) {
- print "#unmatched\n";
- } else {
- print for (@out);
- }
- '
+ while read mode sha1 stage path
+ do
+ if test $mode = "#unmatched"
+ then
+ echo "#unmatched"
+ elif test $mode = "16"
+ then
+ if test $stage != "0"
+ then
+ if test "$unmerged" != "$path"
+ then
+ echo "$mode $null_sha1 U $path"
+ fi
+ unmerged="$path"
+ else
+ echo "$mode $sha1 $stage $path"
+ fi
+ fi
+ done
}
die_if_unmatched ()
--
1.8.3.1.381.g2ab719e.dirty
--
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/2] module_list enhancements
The first iteration can be found here: http://thread.gmane.org/gmane.comp.version-control.git/227572/ The errors in the first patch was a faulty test. I also applied Junios solution in the first patch, it was nicer. Fredrik Gustafsson (2): [submodule] handle multibyte characters in name [submodule] Replace perl-code with sh git-submodule.sh | 53 -- t/t7400-submodule-basic.sh | 12 +++ 2 files changed, 35 insertions(+), 30 deletions(-) -- 1.8.3.1.381.g2ab719e.dirty -- 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] unpack_entry: do not die when we fail to apply a delta
On Thu, 13 Jun 2013, Jeff King wrote:
> When we try to load an object from disk and fail, our
> general strategy is to see if we can get it from somewhere
> else (e.g., a loose object). That lets users fix corruption
> problems by copying known-good versions of objects into the
> object database.
>
> We already handle the case where we were not able to read
> the delta from disk. However, when we find that the delta we
> read does not apply, we simply die. This case is harder to
> trigger, as corruption in the delta data itself would
> trigger a crc error from zlib. However, a corruption that
> pointed us at the wrong delta base might cause it.
>
> We can do the same "fail and try to find the object
> elsewhere" trick instead of dying. This not only gives us a
> chance to recover, but also puts us on code paths that will
> alert the user to the problem (with the current message,
> they do not even know which sha1 caused the problem).
>
> Signed-off-by: Jeff King
That makes sense.
Could you produce a test case to go along with this change?
> ---
> I needed this earlier today to recover from a corrupted packfile (I
> fortunately had an older version of the repo in backups). Still tracking
> down the exact nature of the corruption.
>
> sha1_file.c | 11 ++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/sha1_file.c b/sha1_file.c
> index 5c08701..d458708 100644
> --- a/sha1_file.c
> +++ b/sha1_file.c
> @@ -2135,8 +2135,17 @@ void *unpack_entry(struct packed_git *p, off_t
> obj_offset,
> data = patch_delta(base, base_size,
> delta_data, delta_size,
> &size);
> +
> + /*
> + * We could not apply the delta; warn the user, but keep going.
> + * Our failure will be noticed either in the next iteration of
> + * the loop, or if this is the final delta, in the caller when
> + * we return NULL. Those code paths will take care of making
> + * a more explicit warning and retrying with another copy of
> + * the object.
> + */
> if (!data)
> - die("failed to apply delta");
> + error("failed to apply delta");
>
> free(delta_data);
> }
> --
> 1.8.3.rc2.14.g7eee6b3
>
--
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: git stash while pending merge should not be allowed
On 06/07/13 11:47, Junio C Hamano wrote: Scott McPeak writes: I suggest that this problem could easily have been avoided if "git stash" refused to run with a pending merge (present MERGE_HEAD file), since this is crucial repository state that it does not save. This seems similar to what "git cherry-pick" does. Sounds senslbe. What do we want to see happen in other states, in which Git gives control back to the user asking for help before moving forward (e.g. am, rebase, cherry-pick, revert)? If you're asking me, I don't know. My first thought is if there is any pending state that "stash" doesn't save, stash should refuse to run. But I don't know know very much about some of those commands. -Scott -- 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] unpack_entry: do not die when we fail to apply a delta
When we try to load an object from disk and fail, our
general strategy is to see if we can get it from somewhere
else (e.g., a loose object). That lets users fix corruption
problems by copying known-good versions of objects into the
object database.
We already handle the case where we were not able to read
the delta from disk. However, when we find that the delta we
read does not apply, we simply die. This case is harder to
trigger, as corruption in the delta data itself would
trigger a crc error from zlib. However, a corruption that
pointed us at the wrong delta base might cause it.
We can do the same "fail and try to find the object
elsewhere" trick instead of dying. This not only gives us a
chance to recover, but also puts us on code paths that will
alert the user to the problem (with the current message,
they do not even know which sha1 caused the problem).
Signed-off-by: Jeff King
---
I needed this earlier today to recover from a corrupted packfile (I
fortunately had an older version of the repo in backups). Still tracking
down the exact nature of the corruption.
sha1_file.c | 11 ++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/sha1_file.c b/sha1_file.c
index 5c08701..d458708 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -2135,8 +2135,17 @@ void *unpack_entry(struct packed_git *p, off_t
obj_offset,
data = patch_delta(base, base_size,
delta_data, delta_size,
&size);
+
+ /*
+* We could not apply the delta; warn the user, but keep going.
+* Our failure will be noticed either in the next iteration of
+* the loop, or if this is the final delta, in the caller when
+* we return NULL. Those code paths will take care of making
+* a more explicit warning and retrying with another copy of
+* the object.
+*/
if (!data)
- die("failed to apply delta");
+ error("failed to apply delta");
free(delta_data);
}
--
1.8.3.rc2.14.g7eee6b3
--
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] am: handle stray $dotest directory case
Ramkumar Ramachandra writes: >> But that is entirely an independent issue (I am going to agree with >> you in the end). > > Exactly. It might be nice to fix those two things (are there any > observed bugs?), but it is entirely orthogonal to our issue. OK. >> That is a correct observation. But it needed a bit of thinking to >> reach your conclusion that special casing this and handling --abort >> in a new different codepath is the right solution. > > Yeah, the commit message is lacking. OK. I'll queue this on 'pu', not in 'next' so that it can be rerolled. > I'll probably work on these follow-ups in the morning. Thanks for poking. Thanks. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] rebase: guard against missing files in read_basic_state()
Ramkumar Ramachandra writes:
> Signed-off-by: Ramkumar Ramachandra
This does not affect correctness; i.e. head_name=$(cat that-file)
will error out if the file is missing, right?
A more troublesome is that nobody seems to check the return value of
this function. If head-name, onto or orig-head is missing, is that
an error condition that should make the callers of read_basic_state
stop and refuse to proceed?
The way the && cascade is used seems to indicate that, but up to the
point where it sents $verbose. If and only if head-name, onto, orig-head
and quiet can be read in state-dir, verbose in state-dir is checked
and only then $verbose is set.
Martin, this seems to be from your series around early Feburary
2011. Do you recall why these checks are cascaded this way?
I do not offhand think of a good reason.
> ---
> git-rebase.sh | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/git-rebase.sh b/git-rebase.sh
> index d0c11a9..2122fe0 100755
> --- a/git-rebase.sh
> +++ b/git-rebase.sh
> @@ -84,6 +84,8 @@ keep_empty=
> test "$(git config --bool rebase.autosquash)" = "true" && autosquash=t
>
> read_basic_state () {
> + test -f "$state_dir/head-name" &&
> + test -f "$state_dir/onto" &&
> head_name=$(cat "$state_dir"/head-name) &&
> onto=$(cat "$state_dir"/onto) &&
> # We always write to orig-head, but interactive rebase used to write to
--
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] reset: trivial refactoring
Martin von Zweigbergk writes:
> ...the line after this one reads
>
>err = reset_index(sha1, MIXED, quiet);
>
> ? I don't know what the consequence of not calling prime_cache_tree()
> would be, though.
It does not affect correctness, but makes the subsequent "git
status", the part that internally computes "diff-cache --index" to
see what changes have been added to the index, more costly.
After doing "reset --hard $commit" or just "reset $commit", we know
that the contents of the index must match $commit^{tree}, and
writing out any subpart of the index that corresponds to a directory
(including the top-level one, i.e. the whole index) must match the
corresponding subtree of $commit^{tree}. And that is why we prime
the cache-tree that was discarded by unpack_trees() at the very end.
Then incremental "git add" to the resulting index after that can
invalidate only the parts of the index and cache-tree while
relieving the next "write-tree" (most often done by the next "git
commit") from having to compute the tree objects for parts of the
index that have not been touched since the "reset" operation.
I do not use "reset --keep $commit" very often myself, but IIRC, it
is like "checkout $commit" (and not "checkout -m $commit") in that
the resulting index matches $commit^{tree}, so I think priming the
cache-tree just like --hard/--mixed is the right thing to do.
--
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] reset: trivial refactoring
On Thu, Jun 13, 2013 at 4:13 PM, Martin von Zweigbergk
wrote:
> On Thu, Jun 13, 2013 at 11:15 AM, Felipe Contreras
> wrote:
>> @@ -82,7 +82,7 @@ static int reset_index(const unsigned char *sha1, int
>> reset_type, int quiet)
>> if (unpack_trees(nr, desc, &opts))
>> return -1;
>>
>> - if (reset_type == MIXED || reset_type == HARD) {
>> + if (reset_type == HARD) {
>
> Are you sure that this can not be reached given that...
>
>> @@ -323,8 +323,11 @@ int cmd_reset(int argc, const char **argv, const char
>> *prefix)
>> struct lock_file *lock = xcalloc(1, sizeof(struct
>> lock_file));
>> int newfd = hold_locked_index(lock, 1);
>> if (reset_type == MIXED) {
>> + int flags = quiet ? REFRESH_QUIET :
>> REFRESH_IN_PORCELAIN;
>> if (read_from_tree(pathspec, sha1))
>> return 1;
>> + refresh_index(&the_index, flags, NULL, NULL,
>> + _("Unstaged changes after reset:"));
>> } else {
>> int err = reset_index(sha1, reset_type, quiet);
>> if (reset_type == KEEP && !err)
>
> ...the line after this one reads
>
>err = reset_index(sha1, MIXED, quiet);
That's true. Only the rest of the patch makes sense then. It seems
there should be a way to have a single call reset_index(KEEP), so we
don't have to call again with MIXED, but perhaps that's for later.
--
Felipe Contreras
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] reset: trivial refactoring
On Thu, Jun 13, 2013 at 11:15 AM, Felipe Contreras
wrote:
> @@ -82,7 +82,7 @@ static int reset_index(const unsigned char *sha1, int
> reset_type, int quiet)
> if (unpack_trees(nr, desc, &opts))
> return -1;
>
> - if (reset_type == MIXED || reset_type == HARD) {
> + if (reset_type == HARD) {
Are you sure that this can not be reached given that...
> @@ -323,8 +323,11 @@ int cmd_reset(int argc, const char **argv, const char
> *prefix)
> struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
> int newfd = hold_locked_index(lock, 1);
> if (reset_type == MIXED) {
> + int flags = quiet ? REFRESH_QUIET :
> REFRESH_IN_PORCELAIN;
> if (read_from_tree(pathspec, sha1))
> return 1;
> + refresh_index(&the_index, flags, NULL, NULL,
> + _("Unstaged changes after reset:"));
> } else {
> int err = reset_index(sha1, reset_type, quiet);
> if (reset_type == KEEP && !err)
...the line after this one reads
err = reset_index(sha1, MIXED, quiet);
? I don't know what the consequence of not calling prime_cache_tree()
would be, though.
The merging of the two if blocks looks good. Thanks.
--
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 5/6] status: do not depend on flaky reflog messages
Ramkumar Ramachandra writes: > To be clear: the problem is not the feature, but rather in the > _implementation_ of the feature. OK, but are we discussing the same "feature" (see below)? >> You were at 1.8.2 but no longer are, so in the following sequence: >> >> $ git checkout v1.8.2 >> $ git status >> $ git reset --hard HEAD^ >> $ git status >> >> the former would say "detached at v1.8.2" while the latter should >> *not*, because we are no longer at v1.8.2. "detached from v1.8.2" >> is too subtle a way to express the state, and is confusing, but I >> would not be surprised if people find it useful to be able to learn >> "v1.8.2" even after you strayed away. > > What is wrong with git describe? Is this cheaper, or am I missing something? I think what you are missing is that the "detached from" is not about your current HEAD after you flipped it around with many resets and commits. It is about what tag or what specific commit you detached your HEAD at originally. "You can ask the same to describe" is wrong and is not a valid argument. Once you replace the HEAD^ with a distant commit (e.g. HEAD~400) in the third step in the example, "describe" will not talk about v1.8.2 at all. Your argument can be that it is not a useful piece of information, and as you can probably guess from my "I wouldn't be surprised" above, I am not sure how useful it would be and in what situation [*1*]. But the original commit thought it was necessary and that was done for a reason; we need to be careful here. With a good justification why it is not necessary (or misleading to the user), I do not think we cannot change it. As various tools that use detached "intermediate" states leave enough clues for "status" to notice what is going on, I actually think it is a mistake to focus on what happens when we are in such a detached "intermediate" status with this codepath. For example: $ git rebase master ... replays some but stops $ git status currently uses that "HEAD detached from" codepath, but I think that is a mistake. We could not just tell the HEAD is detached, but the reason _why_ the HEAD is detached (i.e. we are in the middle of a rebase). The prompt script can do it, "status" should be able to do the same, and do a lot more sensible thing than unconditionally showing that "HEAD detached from" and then say "You are currently rebasing" on a separate line. Most likely we do not want to even say "Not currently on any branch" but just say "You are currently rebasing branch X on top of Y" (and perhaps "N commits remaining to be replayed"). [Footnote] *1* One thing I could think of is to start sightseeing or (more realistically) manually bisecting at a given release point, reset the detached HEAD around several times, and then want to be reminded where the session started from. I do not think it is particularly a very good example, though. -- 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] match-trees: factor out fill_tree_desc_strict
On Thu, Jun 13, 2013 at 2:19 PM, René Scharfe wrote: > Deduplicate code by moving tree_desc initialtization into a helper s/initialtization/initialization/ > function, fill_tree_desc_strict. It is like fill_tree_descriptor, > except that it only accepts tree hashes and no tree references (tags, > commits). No functional change. > > Signed-off-by: René Scharfe -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/6] t/checkout-last: checkout - doesn't work after rebase
Ramkumar Ramachandra writes:
> Junio C Hamano wrote:
>> Why two?
>
> What breaks checkout - is the initial HEAD detachment (which writes
> that "checkout: " message), before anything else happens. None of
> , , and make any difference: I'm testing
> exactly the code that I patched.
>
> I have recently been told that I should be testing "end-user behavior"
> by treating the programs as black-boxes, instead of "implementation".
> What is your opinion on the issue? Should I write more tests?
Of course, the behaviours that should be observable by end-users
need be spelled out. Also, an impementation detail that cannot be
observed or make any difference to the end-user experience should
not be cast in stone.
The guideline is a good one, but you need to realize that there is a
difference between "do not test implementation details" and "do not
look at implementation when designing tests". Only the former is
necessary and correct: it lets you avoid over-specifying the
behaviour.
Sometimes you can trivially tell that some obvious implementations
that may be different from what you have can break the expectation
you are setting. For example, in v1.5.0, "rebase A B" literally ran
"git-checkout B" as the first thing, and if you want the end-user to
expect that @{-$n} does not resolve to B because such internal
"checkout" is not an user action, covering "rebase A B", even if you
know that the current implemention happens to be immune to such a
form of breakage, would be a good way to future-proof your code.
That still is testing the behaviour. You are just taking advantage
of the fact that you know the implementation and can anticipate how
future changes by careless people could break the behaviour.
--
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] am: handle stray $dotest directory case
Junio C Hamano wrote: > Hmph, when did ORIG_HEAD set, and what commit does it point at? By some unrelated previous operation (eg. pull, rebase, merge). The point is that at any point in "normal operation", ORIG_HEAD exists, and usually points to @~N, for some N. If I rm .git/ORIG_HEAD by hand, the am --abort obviously errors out: $ git am --abort fatal: Not a valid object name ORIG_HEAD fatal: ambiguous argument 'ORIG_HEAD': unknown revision or path not in the working tree. Use '--' to separate paths from revisions, like this: 'git [...] -- [...]' > As "git am" reading from stdin, waiting, hasn't moved HEAD yet at > all, I think two things need to happen to fix that: > > (1) at around the call to check_patch_format and split_patches, > clear ORIG_HEAD (this may have to be done only !$rebasing, > though). > > (2) safe_to_abort() should make sure ORIG_HEAD exists; otherwise it > is unsafe. > > But that is entirely an independent issue (I am going to agree with > you in the end). Exactly. It might be nice to fix those two things (are there any observed bugs?), but it is entirely orthogonal to our issue. > That is a correct observation. But it needed a bit of thinking to > reach your conclusion that special casing this and handling --abort > in a new different codepath is the right solution. Yeah, the commit message is lacking. > How would "am --skip", "am --resolved", or "am anothermbox" behave > in this "we already have $dotest because the user started one > session but killed it" case, which used to be covered by -d $dotest > alone but now flows to the other side of the if/else/fi codepath? > Do they need a similar treatment, or would they naturally error out > as they should? am --skip and am --resolved will error out, saying "Resolve operation not in progress, we are not resuming". The message needs to be tweaked. am anothermbox will work just fine, implicitly overwriting the existing $dotest directory. But yeah, we could explicitly remove the directory and allow the mkdir -p to re-create it. I'll probably work on these follow-ups in the morning. Thanks for poking. -- 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 0/3] Refactor useful notes functions into notes-utils.[ch]
On Thu, Jun 13, 2013 at 1:16 PM, Felipe Contreras wrote: > On Thu, Jun 13, 2013 at 12:24 PM, Junio C Hamano wrote: >> But the thing is, that majority is what writes the majority of the >> code and does the majority of the reviews, so as maintainer I *do* >> have to give their opinion a lot of weight, not to mention my own >> opinion about how to help keep the community the most productive. > > Indeed, but that doesn't make it a fact. It remains an opinion. And just to make it clear, I didn't deny you are the only one with commit access, and therefore you make all the shots. You made a decision, fine, I never said you can't do that. What I said is that you should not use words to imply that your *opinion* is a fact. The fact that you make a decision doesn't make your opinion a fact, and the fact that many people share your opinion doesn't make it a fact either. So, instead of saying: "Just one side being right, and the other side continuing to repeat nonsense without listening." You should say: "Simply a matter of disagreement where the code belongs." -- Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/6] status: do not depend on flaky reflog messages
Junio C Hamano wrote:
>> At this point, the utility of such a message is in question.
>
> You can question, but I am not convinced the answer is an
> unambiguous "not useful"
I am not arguing for an unambiguous "not useful". I am arguing for a
practical compromise: this patch locks things up too tightly, and
makes life hell for contributors who want to improve reflog messages.
To be clear: the problem is not the feature, but rather in the
_implementation_ of the feature.
> You were at 1.8.2 but no longer are, so in the following sequence:
>
> $ git checkout v1.8.2
> $ git status
> $ git reset --hard HEAD^
> $ git status
>
> the former would say "detached at v1.8.2" while the latter should
> *not*, because we are no longer at v1.8.2. "detached from v1.8.2"
> is too subtle a way to express the state, and is confusing, but I
> would not be surprised if people find it useful to be able to learn
> "v1.8.2" even after you strayed away.
What is wrong with git describe? Is this cheaper, or am I missing something?
>> Moreover,
>> there are several tests in t/status-help that explicitly rely on rebase
>> writing "checkout: " messages to the reflog. As evidenced by the
>> failing tests in t/checkout-last, these messages are completely
>> unintended and flaky.
>
> The above only helps to convince me that "rebase should not affect
> what the last checked-out branch was by letting 'checkout' it
> internally calls to write reflog entries for it" With patches 6, 2,
> and 3, I thought you fixed that issue.
I also thought of ignoring the first line in the actual output ("HEAD
detached from/to"), and comparing the rest to make the tests pass. At
that point, you start to wonder: what is this fantastic feature that
we are bending over backwards for?
--
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] am: handle stray $dotest directory case
Ramkumar Ramachandra writes:
> Junio C Hamano wrote:
>> Perhaps _that_ guarding condition is what needs
>> to be fixed, by reverting it back to just "does $dotest exist?"
>>
>> Adding a single case workaround smells like a band-aid to me.
>
> Like I pointed out earlier, the original codepath is not equipped to
> handle this case. A "normal" git am --abort runs:
>
> git read-tree --reset -u HEAD ORIG_HEAD
> git reset ORIG_HEAD
Hmph, when did ORIG_HEAD set, and what commit does it point at?
As "git am" reading from stdin, waiting, hasn't moved HEAD yet at
all, I think two things need to happen to fix that:
(1) at around the call to check_patch_format and split_patches,
clear ORIG_HEAD (this may have to be done only !$rebasing,
though).
(2) safe_to_abort() should make sure ORIG_HEAD exists; otherwise it
is unsafe.
But that is entirely an independent issue (I am going to agree with
you in the end).
> blowing away the top commit in the scenario you outlined.
>
> This happens because that codepath incorrectly believes that an am is
> "in progress". What this means is that it believes that some of the
> am code actually got executed in the previous run, setting ORIG_HEAD
> etc. In your scenario
>
> % git am
> ^C
>
> nothing is executed, and a stray directory is left behind.
That is a correct observation. But it needed a bit of thinking to
reach your conclusion that special casing this and handling --abort
in a new different codepath is the right solution.
> If anything, I think the check for $dotest/{next,last} has made the
> code more robust by correctly verifying that some code did get
> executed, and that an am is indeed in progress. The bug you have
> outlined is equivalent to:
>
> % mkdir .git/rebase-apply
> % git am --abort
Yes. Or a previous "git am" run lost "$dotest/last" by a bug and
then the user asked to "am --abort". Either case, the best you can
do is probably to blow away .git/rebase-apply directory.
How would "am --skip", "am --resolved", or "am anothermbox" behave
in this "we already have $dotest because the user started one
session but killed it" case, which used to be covered by -d $dotest
alone but now flows to the other side of the if/else/fi codepath?
Do they need a similar treatment, or would they naturally error out
as they should?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/6] t/checkout-last: checkout - doesn't work after rebase
Junio C Hamano wrote: > Why two? What breaks checkout - is the initial HEAD detachment (which writes that "checkout: " message), before anything else happens. None of , , and make any difference: I'm testing exactly the code that I patched. I have recently been told that I should be testing "end-user behavior" by treating the programs as black-boxes, instead of "implementation". What is your opinion on the issue? Should I write more tests? -- 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] use logical OR (||) instead of binary OR (|) in logical context
The compiler can short-circuit the evaluation of conditions strung
together with logical OR operators instead of computing the resulting
bitmask with binary ORs. More importantly, this patch makes the
intent of the changed code clearer, because the logical context (as
opposed to binary context) becomes immediately obvious.
While we're at it, simplify the check for patch->is_rename in
builtin/apply.c a bit; it can only be 0 or 1, so we don't need a
comparison operator.
Signed-off-by: René Scharfe
---
builtin/apply.c | 2 +-
builtin/ls-files.c | 8
builtin/merge-base.c | 2 +-
3 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index 30eefc3..faf8e30 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -3525,7 +3525,7 @@ static int check_patch(struct patch *patch)
ok_if_exists = 0;
if (new_name &&
- ((0 < patch->is_new) | (0 < patch->is_rename) | patch->is_copy)) {
+ ((0 < patch->is_new) || patch->is_rename || patch->is_copy)) {
int err = check_to_create(new_name, ok_if_exists);
if (err && threeway) {
diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index 2202072..87f3b33 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -219,7 +219,7 @@ static void show_files(struct dir_struct *dir)
if (show_killed)
show_killed_files(dir);
}
- if (show_cached | show_stage) {
+ if (show_cached || show_stage) {
for (i = 0; i < active_nr; i++) {
struct cache_entry *ce = active_cache[i];
if ((dir->flags & DIR_SHOW_IGNORED) &&
@@ -233,7 +233,7 @@ static void show_files(struct dir_struct *dir)
(ce_skip_worktree(ce) ? tag_skip_worktree :
tag_cached), ce);
}
}
- if (show_deleted | show_modified) {
+ if (show_deleted || show_modified) {
for (i = 0; i < active_nr; i++) {
struct cache_entry *ce = active_cache[i];
struct stat st;
@@ -571,8 +571,8 @@ int cmd_ls_files(int argc, const char **argv, const char
*cmd_prefix)
die("ls-files --ignored needs some exclude pattern");
/* With no flags, we default to showing the cached files */
- if (!(show_stage | show_deleted | show_others | show_unmerged |
- show_killed | show_modified | show_resolve_undo))
+ if (!(show_stage || show_deleted || show_others || show_unmerged ||
+ show_killed || show_modified || show_resolve_undo))
show_cached = 1;
if (max_prefix)
diff --git a/builtin/merge-base.c b/builtin/merge-base.c
index 1bc7991..0c4cd2f 100644
--- a/builtin/merge-base.c
+++ b/builtin/merge-base.c
@@ -107,7 +107,7 @@ int cmd_merge_base(int argc, const char **argv, const char
*prefix)
argc = parse_options(argc, argv, prefix, options, merge_base_usage, 0);
if (!octopus && !reduce && argc < 2)
usage_with_options(merge_base_usage, options);
- if (is_ancestor && (show_all | octopus | reduce))
+ if (is_ancestor && (show_all || octopus || reduce))
die("--is-ancestor cannot be used with other options");
if (is_ancestor)
return handle_is_ancestor(argc, argv);
--
1.8.3
--
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] match-trees: factor out fill_tree_desc_strict
Deduplicate code by moving tree_desc initialtization into a helper
function, fill_tree_desc_strict. It is like fill_tree_descriptor,
except that it only accepts tree hashes and no tree references (tags,
commits). No functional change.
Signed-off-by: René Scharfe
---
match-trees.c | 44 +++-
1 file changed, 19 insertions(+), 25 deletions(-)
diff --git a/match-trees.c b/match-trees.c
index 2bb734d..7873cde 100644
--- a/match-trees.c
+++ b/match-trees.c
@@ -47,6 +47,22 @@ static int score_matches(unsigned mode1, unsigned mode2,
const char *path)
return score;
}
+static void *fill_tree_desc_strict(struct tree_desc *desc,
+ const unsigned char *hash)
+{
+ void *buffer;
+ enum object_type type;
+ unsigned long size;
+
+ buffer = read_sha1_file(hash, &type, &size);
+ if (!buffer)
+ die("unable to read tree (%s)", sha1_to_hex(hash));
+ if (type != OBJ_TREE)
+ die("%s is not a tree", sha1_to_hex(hash));
+ init_tree_desc(desc, buffer, size);
+ return buffer;
+}
+
static int base_name_entries_compare(const struct name_entry *a,
const struct name_entry *b)
{
@@ -61,23 +77,10 @@ static int score_trees(const unsigned char *hash1, const
unsigned char *hash2)
{
struct tree_desc one;
struct tree_desc two;
- void *one_buf, *two_buf;
+ void *one_buf = fill_tree_desc_strict(&one, hash1);
+ void *two_buf = fill_tree_desc_strict(&two, hash2);
int score = 0;
- enum object_type type;
- unsigned long size;
- one_buf = read_sha1_file(hash1, &type, &size);
- if (!one_buf)
- die("unable to read tree (%s)", sha1_to_hex(hash1));
- if (type != OBJ_TREE)
- die("%s is not a tree", sha1_to_hex(hash1));
- init_tree_desc(&one, one_buf, size);
- two_buf = read_sha1_file(hash2, &type, &size);
- if (!two_buf)
- die("unable to read tree (%s)", sha1_to_hex(hash2));
- if (type != OBJ_TREE)
- die("%s is not a tree", sha1_to_hex(hash2));
- init_tree_desc(&two, two_buf, size);
for (;;) {
struct name_entry e1, e2;
int got_entry_from_one = tree_entry(&one, &e1);
@@ -124,16 +127,7 @@ static void match_trees(const unsigned char *hash1,
int recurse_limit)
{
struct tree_desc one;
- void *one_buf;
- enum object_type type;
- unsigned long size;
-
- one_buf = read_sha1_file(hash1, &type, &size);
- if (!one_buf)
- die("unable to read tree (%s)", sha1_to_hex(hash1));
- if (type != OBJ_TREE)
- die("%s is not a tree", sha1_to_hex(hash1));
- init_tree_desc(&one, one_buf, size);
+ void *one_buf = fill_tree_desc_strict(&one, hash1);
while (one.size) {
const char *path;
--
1.8.3
--
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] rebase: use peel_committish() where appropriate
The failing tests in t/rebase and t/rebase-interactive pass as a result.
Signed-off-by: Ramkumar Ramachandra
---
git-rebase.sh | 6 +++---
t/t3400-rebase.sh | 2 +-
t/t3404-rebase-interactive.sh | 2 +-
3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/git-rebase.sh b/git-rebase.sh
index d0c11a9..28e8d47 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -434,7 +434,7 @@ then
shift
;;
esac
- upstream=`git rev-parse --verify "${upstream_name}^0"` ||
+ upstream=$(peel_committish "${upstream_name}") ||
die "$(eval_gettext "invalid upstream \$upstream_name")"
upstream_arg="$upstream_name"
else
@@ -470,7 +470,7 @@ case "$onto_name" in
fi
;;
*)
- onto=$(git rev-parse --verify "${onto_name}^0") ||
+ onto=$(peel_committish "$onto_name") ||
die "$(eval_gettext "Does not point to a valid commit: \$onto_name")"
;;
esac
@@ -490,7 +490,7 @@ case "$#" in
orig_head=$(git rev-parse -q --verify "refs/heads/$1")
then
head_name="refs/heads/$1"
- elif orig_head=$(git rev-parse -q --verify "$1")
+ elif orig_head=$(peel_committish "$1" quiet)
then
head_name="detached HEAD"
else
diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
index 890f159..272f0f5 100755
--- a/t/t3400-rebase.sh
+++ b/t/t3400-rebase.sh
@@ -88,7 +88,7 @@ test_expect_success 'rebase fast-forward to master' '
test_i18ngrep "Fast-forwarded HEAD to my-topic-branch" out
'
-test_expect_failure 'rebase against revision specified as :/quuxery' '
+test_expect_success 'rebase against revision specified as :/quuxery' '
git checkout my-topic-branch^ &&
sha1=$(git rev-parse ":/Add B") &&
git rebase $sha1 &&
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index ca4ee92..c9a5d56 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -947,7 +947,7 @@ test_expect_success 'rebase -i respects core.commentchar' '
test B = $(git cat-file commit HEAD^ | sed -ne \$p)
'
-test_expect_failure 'rebase -i against revision specified as :/quuxery' '
+test_expect_success 'rebase -i against revision specified as :/quuxery' '
git checkout branch1 &&
sha1=$(git rev-parse ":/J") &&
git rebase $sha1 &&
--
1.8.3.1.381.g31c8856.dirty
--
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/3] sh-setup: add new peel_committish() helper
The normal way to check whether a certain revision resolves to a valid
commit is:
$ git rev-parse --verify $REV^0
Unfortunately, this does not work when $REV is of the type :/quuxery.
Write a helper to work around this limitation.
Suggested-by: Junio C Hamano
Signed-off-by: Ramkumar Ramachandra
---
git-sh-setup.sh | 13 +
1 file changed, 13 insertions(+)
diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index 2f78359..6ae19a6 100644
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh
@@ -313,3 +313,16 @@ then
}
: ${GIT_OBJECT_DIRECTORY="$GIT_DIR/objects"}
fi
+
+peel_committish () {
+ test $# -gt 1 && quiet="--quiet" || quiet=""
+ case "$1" in
+ :/*)
+ peeltmp=$(git rev-parse --verify $quiet "$1") &&
+ git rev-parse --verify "${peeltmp}^0"
+ ;;
+ *)
+ git rev-parse --verify "${1}^0"
+ ;;
+ esac
+}
--
1.8.3.1.381.g31c8856.dirty
--
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] t/rebase: add failing tests for a peculiar revision
The following commands fail, even if :/quuxery resolves to a perfectly
valid commit:
$ git rebase :/quuxery
$ git rebase -i :/quuxery
This is because rebase [-i] attempts to rev-parse ${REV}^0 to verify
that the given revision resolves to a commit. Add tests to document
these failures.
Signed-off-by: Ramkumar Ramachandra
---
t/t3400-rebase.sh | 8
t/t3404-rebase-interactive.sh | 8
2 files changed, 16 insertions(+)
diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
index b58fa1a..890f159 100755
--- a/t/t3400-rebase.sh
+++ b/t/t3400-rebase.sh
@@ -88,6 +88,14 @@ test_expect_success 'rebase fast-forward to master' '
test_i18ngrep "Fast-forwarded HEAD to my-topic-branch" out
'
+test_expect_failure 'rebase against revision specified as :/quuxery' '
+ git checkout my-topic-branch^ &&
+ sha1=$(git rev-parse ":/Add B") &&
+ git rebase $sha1 &&
+ git checkout my-topic-branch^ &&
+ git rebase ":/Add B"
+'
+
test_expect_success 'the rebase operation should not have destroyed author
information' '
! (git log | grep "Author:" | grep "<>")
'
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 79e8d3c..ca4ee92 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -947,4 +947,12 @@ test_expect_success 'rebase -i respects core.commentchar' '
test B = $(git cat-file commit HEAD^ | sed -ne \$p)
'
+test_expect_failure 'rebase -i against revision specified as :/quuxery' '
+ git checkout branch1 &&
+ sha1=$(git rev-parse ":/J") &&
+ git rebase $sha1 &&
+ git checkout branch1 &&
+ git rebase ":/J"
+'
+
test_done
--
1.8.3.1.381.g31c8856.dirty
--
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] reset: trivial refactoring
After commit 3fde386 (reset [--mixed]: use diff-based reset whether or
not pathspec was given), some code can't be reached, and other code can
be moved to the 'reset_type == MIXED' check.
Let's remove the check that can't be reached, and move the code is
specific to MIXED.
Signed-off-by: Felipe Contreras
---
builtin/reset.c | 11 ---
1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index 6032131..68739ba 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -82,7 +82,7 @@ static int reset_index(const unsigned char *sha1, int
reset_type, int quiet)
if (unpack_trees(nr, desc, &opts))
return -1;
- if (reset_type == MIXED || reset_type == HARD) {
+ if (reset_type == HARD) {
tree = parse_tree_indirect(sha1);
prime_cache_tree(&active_cache_tree, tree);
}
@@ -323,8 +323,11 @@ int cmd_reset(int argc, const char **argv, const char
*prefix)
struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
int newfd = hold_locked_index(lock, 1);
if (reset_type == MIXED) {
+ int flags = quiet ? REFRESH_QUIET :
REFRESH_IN_PORCELAIN;
if (read_from_tree(pathspec, sha1))
return 1;
+ refresh_index(&the_index, flags, NULL, NULL,
+ _("Unstaged changes after reset:"));
} else {
int err = reset_index(sha1, reset_type, quiet);
if (reset_type == KEEP && !err)
@@ -333,12 +336,6 @@ int cmd_reset(int argc, const char **argv, const char
*prefix)
die(_("Could not reset index file to revision
'%s'."), rev);
}
- if (reset_type == MIXED) { /* Report what has not been updated.
*/
- int flags = quiet ? REFRESH_QUIET :
REFRESH_IN_PORCELAIN;
- refresh_index(&the_index, flags, NULL, NULL,
- _("Unstaged changes after reset:"));
- }
-
if (write_cache(newfd, active_cache, active_nr) ||
commit_locked_index(lock))
die(_("Could not write new index file."));
--
1.8.3.698.g079b096
--
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 0/3] Refactor useful notes functions into notes-utils.[ch]
On Thu, Jun 13, 2013 at 12:24 PM, Junio C Hamano wrote: > Felipe Contreras writes: > >> On Wed, Jun 12, 2013 at 3:02 PM, Junio C Hamano wrote: >> >>> The proposed patch was rejected on the basis that it was organized >>> the code in a wrong way. And your patch shows how it should be >>> done. >> >> In your opinion. >> >> The fact that nobody outside of 'git' will ever use >> init_copy_notes_for_rewrite() still remains. Therefore this >> "organization" is wrong. > > That is a fact? No, it's not. > It is your opinion on what might happen in the future. That's right, an informed opinion. > And you ignored external projects that may want to link with > libgit.a, Like which project? Moreover: % find /opt/git -name '*.a' Returns nothing. The cannot link to libgit.a, and besides, we don't provide a public API at all. > and closed the door for future improvements. Johan's > implementation has the same effect of allowing sequencer.c to call > these functions without doing so. That would be closing the door to ghosts. Do you want to bet? Five years from now nobody will be using init_copy_notes_for_rewrite(). You loose, we move it to builtin/lib.a, you win, we don't. > Anyway, I have a more important thing to say. > > You sometimes identify the right problem to tackle, but often the > discussions on your patches go in a wrong direction that does not > help solving the original problem at all. So what? I'm a human, am I not allowed to make mistakes? You make mistakes too. > The two examples I can immediately recall offhand are: > > (1) a possible "blame" enhancement, where gitk, that currently runs > two passes of it to identify where each line ultimately came > from and to identify where each line was moved to the current > place, could ask it to learn both with a single run. Yes, *I ACKNOWLEDGED* the direction was not the right one, and I didn't have the time nor the patience to go into such a tedious direction. >From my recommended guideline: * Accept comments on your reviews gracefully. If the original patch submitter doesn't agree with your review, don't take offense. Don't assume the submitter has to automatically modify the patches according to your comments, or even necessarily seek a compromise. The submitter is entitled to his opinion, and so are you. Also, remember that each person has their own priorities in life, and it might take time before the submitter has time to implement the changes, if ever. The changes you request might be beyond the time the submitter is willing to spend, and it's OK for him to decide to drop the patches as a result. You can help by picking the patches yourself in those situations. > (2) refactoring builtin/notes.c to make it possible for sequencer > machinery can also call useful helper functions buried in it. You are wrong. My patches did solve the original problem, I know because I was the one that found the original problem. > The solution to these problems is > for contributors and reviewers to _collaborate_ to come up with a > better end result, which is often different from both the original > patch and the suggestions in the initial review. Collaboration requires both sides to work on the problem. Not one side pointing fingers and the other side doing all the work. > When it is your patch, however, we repeatedly saw that the review > process got derailed in the middle. When working collaboratively it's fine to disagree, and it's fine to have two sides come up with two different patches. If you disagree with the other side, send a patch that does it properly. If the other side doesn't do *exactly* what you want, that's not the review process being derailed. > If there is no will to collaborate on the contributor's end, > however, and the primary thing the contributor wants to do is to > endlessly argue, the efforts by reviewers are all wasted. We do not > get anywhere. In order to have and endless argument, *both sides* need to be engaged in the argument. If you decide that a disagreement has been reached, the argument ends in a disagreement. > That is how I perceive what happens to many of your patches. I am > sure you will say "that is your opinion", but I do not think I am > alone. The opinion of a billion people is still an opinion. > But the thing is, that majority is what writes the majority of the > code and does the majority of the reviews, so as maintainer I *do* > have to give their opinion a lot of weight, not to mention my own > opinion about how to help keep the community the most productive. Indeed, but that doesn't make it a fact. It remains an opinion. > And I have to conclude that the cost of having to deal with you > outweighs the benefit the project gets out of having you around. > Therefore I have ask you to leave and not bother us anymore. We shall see. [1] http://article.gmane.org/gmane.comp.version-control.git/225325 -- Felipe Contreras -- To unsubscribe from this list: send the line
[PATCH 0/3] Support :/quuxery in rebase (early preview)
Hi, So this is a series to make git rebase [-i] :/quuxery possible. It is an early preview, because I have not tested that :/quuxery works as the , , and . Thanks. Ramkumar Ramachandra (3): t/rebase: add failing tests for a peculiar revision sh-setup: add new peel_committish() helper rebase: use peel_committish() where appropriate git-rebase.sh | 6 +++--- git-sh-setup.sh | 13 + t/t3400-rebase.sh | 8 t/t3404-rebase-interactive.sh | 8 4 files changed, 32 insertions(+), 3 deletions(-) -- 1.8.3.1.381.g31c8856.dirty -- 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 5/6] status: do not depend on flaky reflog messages
Junio C Hamano wrote: > [...] I'd just like to point out quickly that I first attempted to write 6/6 without this patch. It is absolutely impossible, because the "detached HEAD from/to" messages no longer make any sense when checkout starts respecting GIT_REFLOG_ACTION. At that point, I'm was just monkeying around the trash-directory running describes to somehow try and make the expected output equal to the actual output. There was no method to the madness, and I was literally losing my mind. This is _the_ patch that makes this series possible. If you want to be convinced, please attempt to drop this patch and fix the tests in 6/6. You will see what I mean. -- 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 5/6] status: do not depend on flaky reflog messages
Ramkumar Ramachandra writes: > b397ea4 (status: show more info than "currently not on any branch", > 2013-03-13) made the output of 'git status' richer in the case of a > detached HEAD. Before this patch, with a detached HEAD: > > $ git status > # Not currently on any branch. > > After the patch: > > $ git checkout v1.8.2 > # HEAD is now detached > $ git status > # HEAD detached at v1.8.2. > > It works by digging the reflog for the most recent message of the form > "checkout: moving from to ". It then asserts that HEAD and > "" are the same, and displays this message. When they aren't equal, > it displays: > > $ git status > # HEAD detached from fe11db. > > At this point, the utility of such a message is in question. You can question, but I am not convinced the answer is an unambiguous "not useful" You were at 1.8.2 but no longer are, so in the following sequence: $ git checkout v1.8.2 $ git status $ git reset --hard HEAD^ $ git status the former would say "detached at v1.8.2" while the latter should *not*, because we are no longer at v1.8.2. "detached from v1.8.2" is too subtle a way to express the state, and is confusing, but I would not be surprised if people find it useful to be able to learn "v1.8.2" even after you strayed away. > Moreover, > there are several tests in t/status-help that explicitly rely on rebase > writing "checkout: " messages to the reflog. As evidenced by the > failing tests in t/checkout-last, these messages are completely > unintended and flaky. The above only helps to convince me that "rebase should not affect what the last checked-out branch was by letting 'checkout' it internally calls to write reflog entries for it" With patches 6, 2, and 3, I thought you fixed that issue. So I am not convinced that is a good argument to justify to regress "HEAD detached from" message to "Not on any branch". At least, not just yet. > This issue is not isolated to rebase at all. Several other scripts like > bisect write (confusing) "checkout: " messages to the reflog. Fixing > them is left as an exercise to other contributors. Any scripted Porcelain that use "checkout", with patch 6, should be able to do so with reflog-action environment variable, right? And 2 and 3 are examples of such fixes to two of them, which argues even more strongly that 6 should be earier in the series, I think. > > Signed-off-by: Ramkumar Ramachandra > --- > t/t7512-status-help.sh | 24 +++- > wt-status.c| 11 --- > 2 files changed, 15 insertions(+), 20 deletions(-) > > diff --git a/t/t7512-status-help.sh b/t/t7512-status-help.sh > index bf08d4e..ed9d57c 100755 > --- a/t/t7512-status-help.sh > +++ b/t/t7512-status-help.sh > @@ -188,10 +188,9 @@ test_expect_success 'status when rebasing -i in edit > mode' ' > export FAKE_LINES && > test_when_finished "git rebase --abort" && > ONTO=$(git rev-parse --short HEAD~2) && > - TGT=$(git rev-parse --short two_rebase_i) && > git rebase -i HEAD~2 && > cat >expected <<-EOF && > - # HEAD detached from $TGT > + # Not currently on any branch. > # You are currently editing a commit while rebasing branch > '\''rebase_i_edit'\'' on '\''$ONTO'\''. > # (use "git commit --amend" to amend the current commit) > # (use "git rebase --continue" once you are satisfied with your > changes) > @@ -246,11 +245,10 @@ test_expect_success 'status after editing the last > commit with --amend during a > export FAKE_LINES && > test_when_finished "git rebase --abort" && > ONTO=$(git rev-parse --short HEAD~3) && > - TGT=$(git rev-parse --short three_amend) && > git rebase -i HEAD~3 && > git commit --amend -m "foo" && > cat >expected <<-EOF && > - # HEAD detached from $TGT > + # Not currently on any branch. > # You are currently editing a commit while rebasing branch > '\''amend_last'\'' on '\''$ONTO'\''. > # (use "git commit --amend" to amend the current commit) > # (use "git rebase --continue" once you are satisfied with your > changes) > @@ -280,7 +278,7 @@ test_expect_success 'status: (continue first edit) second > edit' ' > git rebase -i HEAD~3 && > git rebase --continue && > cat >expected <<-EOF && > - # HEAD detached from $ONTO > + # Not currently on any branch. > # You are currently editing a commit while rebasing branch > '\''several_edits'\'' on '\''$ONTO'\''. > # (use "git commit --amend" to amend the current commit) > # (use "git rebase --continue" once you are satisfied with your > changes) > @@ -302,7 +300,7 @@ test_expect_success 'status: (continue first edit) second > edit and split' ' > git rebase --continue && > git reset HEAD^ && > cat >expected <<-EOF && > - # HEAD detached from $ONTO > + # Not currently on any branch. > # You are currently splitting a com
Re: [PATCH 2/6] rebase: prepare to write reflog message for checkout
Junio C Hamano wrote: > [...] Will fix those. > I suspect doing 6/6 before this patch may make more sense. Yeah, I'd done it like that in the original (early preview thing). Allow me to explain why I flipped the ordering. The problem I am facing is that 6/6 causes very major breakages, and 5/6 attempts to minimize that fallout and make life for 6/6 easier. The problem with putting this patch (and the rebase -i) after those two is simple: it calls set_reflog_action, but never explicitly indicates that it wants to set the reflog message for checkout. As a result, the reflog messages are merely accidental and will look like: rebase rebase -i (start) in both the critical patches (5/6 and 6/6). This was an absolute debugging disaster for me, and I didn't know what wt-status was trying to tell me with its cryptic "detached HEAD to" and "detached HEAD from" messages. Makes sense? -- 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 4/6] wt-status: remove unused field in grab_1st_switch_cbdata
Ramkumar Ramachandra writes:
> The struct grab_1st_switch_cbdata has the field "found", which is set in
> grab_1st_switch() when a match is found. This information is redundant
> and unused by any caller: the return value of the function serves to
> communicate this information anyway. Remove the field.
>
> Signed-off-by: Ramkumar Ramachandra
> ---
Good.
> wt-status.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/wt-status.c b/wt-status.c
> index bf84a86..2511270 100644
> --- a/wt-status.c
> +++ b/wt-status.c
> @@ -1035,7 +1035,6 @@ got_nothing:
> }
>
> struct grab_1st_switch_cbdata {
> - int found;
> struct strbuf buf;
> unsigned char nsha1[20];
> };
> @@ -1059,7 +1058,6 @@ static int grab_1st_switch(unsigned char *osha1,
> unsigned char *nsha1,
> for (end = target; *end && *end != '\n'; end++)
> ;
> strbuf_add(&cb->buf, target, end - target);
> - cb->found = 1;
> return 1;
> }
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/6] rebase -i: prepare to write reflog message for checkout
The same comment as 2/6 applies to this one. What these two do makes sense, but I think having 6/6 before them would make the series easier to follow. -- 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 2/6] rebase: prepare to write reflog message for checkout
Ramkumar Ramachandra writes: > rebase should never write "checkout: " messages to the reflog, since it > is highly confusing to the end user, and breaks > grab_nth_branch_checkout(), as demonstrated by failing tests > in t/checkout-last. "breaks" because "the branch flipping rebase internally does is not 'checkout' as far as the end user is concerned", which essentially is the tautology with the first part "should never write". I would say The branch flipping rebase internally does is not 'checkout' as far as the end user is concerned; rebase should never write "checkout: " messages to the reflog. Instead it should say "rebase: checkout master". > Set a sensible GIT_REFLOG_ACTION: checkout does not > respect GIT_REFLOG_ACTION yet, but this defect will be addressed in a > future patch. That is not a defect; it did not have to so far, until this patch. I suspect doing 6/6 before this patch may make more sense. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/6] t/checkout-last: checkout - doesn't work after rebase
Ramkumar Ramachandra writes: > The following command > > $ git checkout - > > does not work as expected after a rebase. Every kind of rebase must > behave in the exactly same way: for the purposes of checkout -, the > rebase event should be inconsequential. > > Add two failing tests documenting this bug: one for a normal rebase, and > another for an interactive rebase. Why two? After the discussion, I would have expected to see the two argument form: git rebase [-i] master other started on the 'other' branch and also started on a branch that is not 'master' or 'other', also be tested to specify the desired behaviour in these cases. > > Signed-off-by: Ramkumar Ramachandra > --- > t/t2012-checkout-last.sh | 16 > 1 file changed, 16 insertions(+) > > diff --git a/t/t2012-checkout-last.sh b/t/t2012-checkout-last.sh > index b44de9d..ae6d319 100755 > --- a/t/t2012-checkout-last.sh > +++ b/t/t2012-checkout-last.sh > @@ -116,4 +116,20 @@ test_expect_success 'master...' ' > test "z$(git rev-parse --verify HEAD)" = "z$(git rev-parse --verify > master^)" > ' > > +test_expect_failure '"checkout -" works after a rebase' ' > + git checkout master && > + git checkout other && > + git rebase master && > + git checkout - && > + test "z$(git symbolic-ref HEAD)" = "zrefs/heads/master" > +' > + > +test_expect_failure '"checkout -" works after an interactive rebase' ' > + git checkout master && > + git checkout other && > + git rebase -i master && > + git checkout - && > + test "z$(git symbolic-ref HEAD)" = "zrefs/heads/master" > +' > + > test_done -- 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 2/4] push: make upstream, simple work with pushdefault
Junio C Hamano wrote: > Did you mean "I'm still resisting, but after reading [...] I think > it makes sense"? If so, please discard my question. Sorry about the lack of clarity. I agreed with most of what you said, and I outlined how we could possibly turn it into an implementation. Still haven't found a solution. -- 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 2/4] push: make upstream, simple work with pushdefault
Junio C Hamano writes: > Ramkumar Ramachandra writes: > >> Junio C Hamano wrote: >> >>> [...] >> >> Okay, so what you're saying makes sense. > > That is not a very useful style of quoting; what did you just agree to? Ahh. I took your response as "I'm still resisting [to what was quoted before it]. But to the part [...] that I am not quoting I would agree", hence my question. Did you mean "I'm still resisting, but after reading [...] I think it makes sense"? If so, please discard my question. -- 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] am: handle stray $dotest directory case
Junio C Hamano wrote:
> Perhaps _that_ guarding condition is what needs
> to be fixed, by reverting it back to just "does $dotest exist?"
>
> Adding a single case workaround smells like a band-aid to me.
Like I pointed out earlier, the original codepath is not equipped to
handle this case. A "normal" git am --abort runs:
git read-tree --reset -u HEAD ORIG_HEAD
git reset ORIG_HEAD
blowing away the top commit in the scenario you outlined.
This happens because that codepath incorrectly believes that an am is
"in progress". What this means is that it believes that some of the
am code actually got executed in the previous run, setting ORIG_HEAD
etc. In your scenario
% git am
^C
nothing is executed, and a stray directory is left behind.
If anything, I think the check for $dotest/{next,last} has made the
code more robust by correctly verifying that some code did get
executed, and that an am is indeed in progress. The bug you have
outlined is equivalent to:
% mkdir .git/rebase-apply
% git am --abort
Therefore, the fix is to treat it as exactly that: a stray directory
that needs to be cleaned up in the codepath that treats it as a "fresh
run"; not going through the "normal" am --abort logic and blowing away
the top commit.
Makes sense?
--
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 RFC] show-branch: use pager
Øystein Walle writes: > This is for consistency with other porcelain commands such as 'log'. > > Signed-off-by: Øystein Walle > --- > The rationale for this patch I hope is consicely explained in the commit > message. I was rather surprised it didn't use a pager as I've gotten used to > it > for most commands. > > I marked this as an RFC because of Jeff King's comments in > daa0c3d97 where I got the impression this this might not be a good idea. > However I haven't found any bugs and all the tests pass. The tests are run largely without tty to allow them to run unattended, aren't they? I think it makes a lot of sense to use pager by default for the normal show-branch output. I however do not think pager should apply to other modes (e.g. --independent, --merge-base). But the use of these other modes are meant to be on the upstream side of a pipe or to be written out to a file, so a blanket call to setup_pager() before you even discover what mode we are in would not hurt in practice. -- 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 0/3] Refactor useful notes functions into notes-utils.[ch]
Felipe Contreras writes: > On Wed, Jun 12, 2013 at 3:02 PM, Junio C Hamano wrote: > >> The proposed patch was rejected on the basis that it was organized >> the code in a wrong way. And your patch shows how it should be >> done. > > In your opinion. > > The fact that nobody outside of 'git' will ever use > init_copy_notes_for_rewrite() still remains. Therefore this > "organization" is wrong. That is a fact? It is your opinion on what might happen in the future. And you ignored external projects that may want to link with libgit.a, and closed the door for future improvements. Johan's implementation has the same effect of allowing sequencer.c to call these functions without doing so. Anyway, I have a more important thing to say. You sometimes identify the right problem to tackle, but often the discussions on your patches go in a wrong direction that does not help solving the original problem at all. The two examples I can immediately recall offhand are: (1) a possible "blame" enhancement, where gitk, that currently runs two passes of it to identify where each line ultimately came from and to identify where each line was moved to the current place, could ask it to learn both with a single run. (2) refactoring builtin/notes.c to make it possible for sequencer machinery can also call useful helper functions buried in it. but I am sure other reviewers can recall other instances in the recent past. Your patches were wrong in both cases, but that is not an issue. If you are not familiar with the area you are trying to improve, it is understandable that initial attempts may try to solve the right problem in a wrong way. That is perfectly normal. That is what the patch review process is there to help. Reviewers who are more familiar with the area (either the code flow and data structure used in blame, or how the object files are laid out in the source tree and the build procedure is designed to link them to which binary) can point the contributor in a direction that would take us to a better result in the end. During the discussion, it may turn out that reviewers have overlooked issues that also need to be addressed, or there may be further adjustments needed that are initially overlooked by everyone. The solution to these problems is for contributors and reviewers to _collaborate_ to come up with a better end result, which is often different from both the original patch and the suggestions in the initial review. When it is your patch, however, we repeatedly saw that the review process got derailed in the middle. The reviewers tried to reach a good end result in the same way as they interact with other contributors, i.e. by showing a way they think is better, trying to make the contributor realize why it is better by rephrasing and coming up with other examples. This iteration takes a lot of resources, but the reviewers are hoping that we will see a good result at the end of the review and everybody wins. They are trying to collaborate. If there is no will to collaborate on the contributor's end, however, and the primary thing the contributor wants to do is to endlessly argue, the efforts by reviewers are all wasted. We do not get anywhere. That is how I perceive what happens to many of your patches. I am sure you will say "that is your opinion", but I do not think I am alone. And I am also sure you will then say "majority is not always right". But the thing is, that majority is what writes the majority of the code and does the majority of the reviews, so as maintainer I *do* have to give their opinion a lot of weight, not to mention my own opinion about how to help keep the community the most productive. And I have to conclude that the cost of having to deal with you outweighs the benefit the project gets out of having you around. Therefore I have ask you to leave and not bother us anymore. Goodbye. -- 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] am: handle stray $dotest directory case
Ramkumar Ramachandra writes:
> The following bug has been observed since rr/rebase-autostash:
>
> $ git am # no input file
> ^C
> $ git am --abort
> Resolve operation not in progress, we are not resuming.
>
> This happens because the following test fails:
>
> test -d "$dotest" && test -f "$dotest/last" && test -f "$dotest/next"
>
> and am precludes the possibility of a stray $dotest directory
> existing (when $dotest/{last,next} are not present).
Why did the original code sequence that read:
if test -d "$dotest"
then
... handle skip, resolved, abort, because
... these can be run ONLY when we know we have
... started an "am" session.
... catch new "git am mbox" invocation and error
... out, because that should not be allowed when
... we know we have started an "am" session.
had to change its guarding condition to
if test -d "$dotest" && test -f "$dotest/last" && test -f "$dotest/next"
in the first place? Perhaps _that_ guarding condition is what needs
to be fixed, by reverting it back to just "does $dotest exist?"
Adding a single case workaround smells like a band-aid to me.
> Fix the bug by checking for a stray $dotest directory explicitly and
> removing it on --abort.
>
> Reported-by: Junio C Hamano
> Signed-off-by: Ramkumar Ramachandra
> ---
> git-am.sh | 5 +
> 1 file changed, 5 insertions(+)
>
> diff --git a/git-am.sh b/git-am.sh
> index 1cf3d1d..f46a123 100755
> --- a/git-am.sh
> +++ b/git-am.sh
> @@ -506,6 +506,11 @@ then
> esac
> rm -f "$dotest/dirtyindex"
> else
> + # Possible stray $dotest directory
> + if test -d "$dotest" && test t = "$abort"; then
> + clean_abort
> + fi
> +
> # Make sure we are not given --skip, --resolved, nor --abort
> test "$skip$resolved$abort" = "" ||
> die "$(gettext "Resolve operation not in progress, we are not
> resuming.")"
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] rebase -i: write better reflog messages for start
Junio C Hamano wrote: > But what does it have to do with rebase polluting the reflog? See the series I just posted. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] rebase -i: write better reflog messages for start
Ramkumar Ramachandra writes: > Ramkumar Ramachandra wrote: >> t/status-help. Looks seriously unrelated, and I'm breaking my head >> over it. Any clues? > > Damn it! A recent commit is responsible for this avalanche in test > breakages: b397ea (status: show more info than "currently not on any > branch", 2013-03-13). It re-implements a backward version of > grab_nth_branch_switch(): grab_1st_switch() essentially _relies_ on > the random unintended pollution that rebase writes to the reflog to > print a more useful (?) status :/ After "git checkout v1.3.0", it is reasonable to expect that you can tell what you checked out and what state you are in. If you then made a few commits or resetted to some other commit, it is debatable if "detached from v1.3.0" is useful or the subtle difference between "detached at" vs "detached from" is confusing. But what does it have to do with rebase polluting the reflog? -- 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 2/4] push: make upstream, simple work with pushdefault
Ramkumar Ramachandra writes: > Junio C Hamano wrote: > >> [...] > > Okay, so what you're saying makes sense. That is not a very useful style of quoting; what did you just agree to? I think we should step back and clarify the "to which repository the push goes" and "what branch(es) in that chosen repository is updated". The former is determined by your original "triangular" topic in the recent world order. The push.default specifies the logic/algorithm to decide the latter, when there is no stronger configuration is given (e.g. the push refspecs in remote.*.push, and branch.*.push). > - current: push "$(HEAD)". No restrictions on destination. This updates the branch with the same name the current branch on the pushing side. > - matching: push ":" to the destination specified by the current > branch. This updates the branches in the destination repository, for which branches with the same name exists on the pushing side. > - upstream: In the special case when fetch source is equal to push > destination, push "$(HEAD):$(branch.$(HEAD).merge)". Otherwise, > fallback to current. Useful in central workflows. That looks to me as an inconsistent description. If you are not pushing to where you fetched from, that is not even central. This is mean to update the branch that is fetched from and is integrated with the current branch with the tip of the current branch, so the branch at the destination repository that gets updated is branch.$current.merge. It further means that the repository being pushed to must be the same as the repository we fetch from; otherwise it is an error. > - simple: [still haven't thought about what to do with this; I'm > generally not in favor of artificially crippling functionality by > erroring out] In a central workflow (i.e. repository we fetch from to update the current branch is the same as the repository we push the tip of this branch to), this works the same as upstream, but the configured branch.$current.merge has to be the same as the name of the current branch in the local repository; otherwise it is an error. In a triangular workflow TBD, but I think doing the same as current may be a good starting point. > Just like upstream respects branch..merge, current respects > branch..push, making branch-level ref mapping in triangular > workflows possible. I do not know we want to make branch.*.push linked to current. If it is set, shouldn't that apply when push.default is "matching" and other values? That is why I threw it in the same category as the traditional push refspecs in remote.*.push in the early part of this message. -- 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] rebase: finish_rebase() in noop rebase
In the following case $ git rebase master Current branch autostash-fix is up to date. the autostash is not applied automatically, because this codepath forgets to call finish_rebase(). Fix this. Also add a test to guard against regressions. Signed-off-by: Ramkumar Ramachandra --- git-rebase.sh | 1 + t/t3420-rebase-autostash.sh | 11 +++ 2 files changed, 12 insertions(+) diff --git a/git-rebase.sh b/git-rebase.sh index 154d4be..2d5c2bd 100755 --- a/git-rebase.sh +++ b/git-rebase.sh @@ -547,6 +547,7 @@ then # Lazily switch to the target branch if needed... test -z "$switch_to" || git checkout "$switch_to" -- say "$(eval_gettext "Current branch \$branch_name is up to date.")" + finish_rebase exit 0 else say "$(eval_gettext "Current branch \$branch_name is up to date, rebase forced.")" diff --git a/t/t3420-rebase-autostash.sh b/t/t3420-rebase-autostash.sh index 1bde007..90eb264 100755 --- a/t/t3420-rebase-autostash.sh +++ b/t/t3420-rebase-autostash.sh @@ -152,6 +152,17 @@ test_expect_success "rebase: fast-forward rebase" ' git checkout feature-branch ' +test_expect_success "rebase: noop rebase" ' + test_config rebase.autostash true && + git reset --hard && + git checkout -b same-feature-branch feature-branch && + test_when_finished git branch -D same-feature-branch && + echo dirty >>file1 && + git rebase feature-branch && + grep dirty file1 && + git checkout feature-branch +' + testrebase "" .git/rebase-apply testrebase " --merge" .git/rebase-merge testrebase " --interactive" .git/rebase-merge -- 1.8.3.1.381.gf08dd97.dirty -- 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] rebase: guard against missing files in read_basic_state()
Signed-off-by: Ramkumar Ramachandra
---
git-rebase.sh | 2 ++
1 file changed, 2 insertions(+)
diff --git a/git-rebase.sh b/git-rebase.sh
index d0c11a9..2122fe0 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -84,6 +84,8 @@ keep_empty=
test "$(git config --bool rebase.autosquash)" = "true" && autosquash=t
read_basic_state () {
+ test -f "$state_dir/head-name" &&
+ test -f "$state_dir/onto" &&
head_name=$(cat "$state_dir"/head-name) &&
onto=$(cat "$state_dir"/onto) &&
# We always write to orig-head, but interactive rebase used to write to
--
1.8.3.1.381.gf08dd97.dirty
--
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 couple of edge cases in autostash
Hi, I apologize for having missed these two trivial cases in the original series. Ramkumar Ramachandra (3): rebase: guard against missing files in read_basic_state() rebase: finish_rebase() in fast-forward rebase rebase: finish_rebase() in noop rebase git-rebase.sh | 4 t/t3420-rebase-autostash.sh | 22 ++ 2 files changed, 26 insertions(+) -- 1.8.3.1.381.gf08dd97.dirty -- 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/3] rebase: finish_rebase() in fast-forward rebase
In the following case
$ git rebase master
Fast-forwarded autostash-fix to master.
The autostash is not applied automatically, because this codepath
forgets to call finish_rebase(). Fix this. Also add a test to guard
against regressions.
Signed-off-by: Ramkumar Ramachandra
---
git-rebase.sh | 1 +
t/t3420-rebase-autostash.sh | 11 +++
2 files changed, 12 insertions(+)
diff --git a/git-rebase.sh b/git-rebase.sh
index 2122fe0..154d4be 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -579,6 +579,7 @@ if test "$mb" = "$orig_head"
then
say "$(eval_gettext "Fast-forwarded \$branch_name to \$onto_name.")"
move_to_original_branch
+ finish_rebase
exit 0
fi
diff --git a/t/t3420-rebase-autostash.sh b/t/t3420-rebase-autostash.sh
index 479cbb2..1bde007 100755
--- a/t/t3420-rebase-autostash.sh
+++ b/t/t3420-rebase-autostash.sh
@@ -141,6 +141,17 @@ testrebase() {
'
}
+test_expect_success "rebase: fast-forward rebase" '
+ test_config rebase.autostash true &&
+ git reset --hard &&
+ git checkout -b behind-feature-branch feature-branch~1 &&
+ test_when_finished git branch -D behind-feature-branch &&
+ echo dirty >>file1 &&
+ git rebase feature-branch &&
+ grep dirty file1 &&
+ git checkout feature-branch
+'
+
testrebase "" .git/rebase-apply
testrebase " --merge" .git/rebase-merge
testrebase " --interactive" .git/rebase-merge
--
1.8.3.1.381.gf08dd97.dirty
--
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] prompt: squelch error output from cat
SZEDER Gábor wrote: > Just curious: when do those files don't exist? When using an older > version of git with a newer prompt, obviously, but are there other > cases? # On terminal one $ git rebase --interactive master # Ignore editor, and open terminal two cat: .git/rebase-merge/msgnum: No such file or directory cat: .git/rebase-merge/end: No such file or directory $ -- 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] prompt: squelch error output from cat
Hi,
On Thu, Jun 13, 2013 at 07:16:49PM +0530, Ramkumar Ramachandra wrote:
> The files $g/rebase-{merge,apply}/{head-name,msgnum,end} are not
> guaranteed to exist. When attempting to cat them, squelch the error
> output to get rid of messages like these:
>
> cat: .git/rebase-merge/msgnum: No such file or directory
> cat: .git/rebase-merge/end: No such file or directory
>
> Signed-off-by: Ramkumar Ramachandra
Makes sense.
Just curious: when do those files don't exist? When using an older
version of git with a newer prompt, obviously, but are there other
cases?
Thanks,
Gábor
--
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: slow process of post-receive script on a remote (samba) share
Tamas Csabina writes:
> Meanwhile I`ve figured it out that the sluggish post-receive execution
> was due to a (mis)-configuration in the samba share where the remote
> repository is hosted. These are:
> oplocks = No
> level2 oplocks = No
[...]
> Now, do I have to worry about allowing oplocks on the remote
> repository from the git point of view? Thinking about concurrent push
> operations from different developers?
>From a brief glance at the relevant docs [1], it would seem that oplocks
are actually just an implementation detail for safe (in the context of
parallel access) client caching. So they should be fully transparent to
any application usage. However, the docs do mention that you may be in
trouble if the connection to the server times out.
That being said, some FSes see more usage and thus have been tested more
in this context, and git does tend to show some pretty weird issues on
broken network FSes (one such case: Lustre[2]).
In addition, there are some known races w.r.t. the handling of refs, and
of pruning, if you run git-gc while concurrent pushes are going on.
Jeff King and Michael Haggerty are currently working on getting them
fixed, see e.g. [3]. To see these, you'll have to hit the repo much
harder than a small team can.
So it *should* work, at least if you disable gc.auto and run git-gc
manually at some safe time. But I wouldn't be surprised if there are
bugs lurking in the context of Windows usage on a Samba-hosted repo,
which sounds like a very rare combination.
And in any case, don't take my word for it; if your life or company
depends on this, you'll need to do your own testing to ensure that it
holds up.
Oh, and why do it that way? You would most likely get much better
performance out of it if you hosted the repo over ssh (e.g. with
gitolite[4]) or a smart-http server, since the expensive operations (and
they are *expensive*) would be completely local to the server. The
tradeoff there is that it also shifts a lot of CPU work to the server,
but if you can afford it, you should see a great speedup especially when
fetching large amounts of data (e.g. at cloen time).
[1]
http://www.samba.org/samba/docs/man/Samba-HOWTO-Collection/locking.html#id2615667
[2] http://thread.gmane.org/gmane.comp.version-control.git/212109
[3] http://thread.gmane.org/gmane.comp.version-control.git/223299
[4] http://gitolite.com/gitolite/
--
Thomas Rast
trast@{inf,student}.ethz.ch
--
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] am: handle stray $dotest directory case
The following bug has been observed since rr/rebase-autostash:
$ git am # no input file
^C
$ git am --abort
Resolve operation not in progress, we are not resuming.
This happens because the following test fails:
test -d "$dotest" && test -f "$dotest/last" && test -f "$dotest/next"
and am precludes the possibility of a stray $dotest directory
existing (when $dotest/{last,next} are not present).
Fix the bug by checking for a stray $dotest directory explicitly and
removing it on --abort.
Reported-by: Junio C Hamano
Signed-off-by: Ramkumar Ramachandra
---
git-am.sh | 5 +
1 file changed, 5 insertions(+)
diff --git a/git-am.sh b/git-am.sh
index 1cf3d1d..f46a123 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -506,6 +506,11 @@ then
esac
rm -f "$dotest/dirtyindex"
else
+ # Possible stray $dotest directory
+ if test -d "$dotest" && test t = "$abort"; then
+ clean_abort
+ fi
+
# Make sure we are not given --skip, --resolved, nor --abort
test "$skip$resolved$abort" = "" ||
die "$(gettext "Resolve operation not in progress, we are not
resuming.")"
--
1.8.3.1.379.ged35616
--
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: slow process of post-receive script on a remote (samba) share
Hi Thomas,
Thanks for the reply.
The script is a bash script, just to mention.
Meanwhile I`ve figured it out that the sluggish post-receive execution
was due to a (mis)-configuration in the samba share where the remote
repository is hosted. These are:
oplocks = No
level2 oplocks = No
Removing these from the share`s section in the smb.conf solved the
issue, and the push process is taking up around 4 seconds, which I
think is reliable.
Now, do I have to worry about allowing oplocks on the remote
repository from the git point of view? Thinking about concurrent push
operations from different developers?
Tamas
On 13 June 2013 14:19, Thomas Rast wrote:
> Tamas Csabina writes:
>
>> I am using Git bash from version 1.8.3.msysgit.0, on a Windows 7x64 PC.
>> I have an issue with executing git push if I have a post-receive
>> script configured.
>> The content of the script is not really important, as if I have a
>> script that contains only commented out lines (around 70 lines), my
>> git push command is delayed with around 5 seconds.
>>
>>
>> I`ve tested the script on another PC and it is working fine. No delay
>> at all. So there are some issues on my PC regarding how git processes
>> remote scripts.
>>
>> I took a wireshark trace with 2 scenarios on my PC:
>>
>> 1. just execute `cat \post-receive` command in the git
>> bash
>> 2. did a `real` git push
>>
>> Results of the wireshark traces shows:
>>
>> 1. Read AndX Request, FID: 0x228f, 1024 bytes at offset 0 (1024 bytes
>> at time, always)
>> 2. Read AndX Request, FID: 0x21c9, 1 byte at offset 0 (1 byte, always)
>>
>> Conclusion:
>> git push command reads the post-receive script in 1 byte chunks, which
>> dramatically slows down the execution process.
>
> git doesn't read the script; the interpreter does. In the case of a
> script, the interpreter is specified in the #! line at the top; in the
> case of a binary executable, it is specified within the executable (and
> for linux, is usually /lib/ld-linux.so.2).
>
> Exactly the same should happen if you run the hook manually, so you can
> try that to debug it.
>
> Note also that Weird Things(tm) relating to SIGPIPE may happen if you
> don't read your input. Even if you are only fooling around for testing,
> a post-receive hook must consume its input, e.g., by discarding it with
> 'cat >/dev/null'.
>
> --
> Thomas Rast
> trast@{inf,student}.ethz.ch
--
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] prompt: squelch error output from cat
The files $g/rebase-{merge,apply}/{head-name,msgnum,end} are not
guaranteed to exist. When attempting to cat them, squelch the error
output to get rid of messages like these:
cat: .git/rebase-merge/msgnum: No such file or directory
cat: .git/rebase-merge/end: No such file or directory
Signed-off-by: Ramkumar Ramachandra
---
contrib/completion/git-prompt.sh | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index 86a4f3f..07a6218 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -347,9 +347,9 @@ __git_ps1 ()
local step=""
local total=""
if [ -d "$g/rebase-merge" ]; then
- b="$(cat "$g/rebase-merge/head-name")"
- step=$(cat "$g/rebase-merge/msgnum")
- total=$(cat "$g/rebase-merge/end")
+ b="$(cat "$g/rebase-merge/head-name" 2>/dev/null)"
+ step=$(cat "$g/rebase-merge/msgnum" 2>/dev/null)
+ total=$(cat "$g/rebase-merge/end" 2>/dev/null)
if [ -f "$g/rebase-merge/interactive" ]; then
r="|REBASE-i"
else
@@ -357,10 +357,10 @@ __git_ps1 ()
fi
else
if [ -d "$g/rebase-apply" ]; then
- step=$(cat "$g/rebase-apply/next")
- total=$(cat "$g/rebase-apply/last")
+ step=$(cat "$g/rebase-apply/next" 2>/dev/null)
+ total=$(cat "$g/rebase-apply/last" 2>/dev/null)
if [ -f "$g/rebase-apply/rebasing" ]; then
- b="$(cat "$g/rebase-apply/head-name")"
+ b="$(cat "$g/rebase-apply/head-name"
2>/dev/null)"
r="|REBASE"
elif [ -f "$g/rebase-apply/applying" ]; then
r="|AM"
--
1.8.3.1.384.g7cec0b4
--
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] Documentation/CommunityGuidelines
On Thu, Jun 13, 2013 at 5:19 AM, Thomas Adam wrote: > It's a point on which one is never going to win, because no matter what one > says, it'll just get twisted round in such a way that one then ends up > questioning their own words, and their own conduct, and that's bad, because > there never was anything wrong with them to begin with. Perhaps because you are actually wrong. In the words of Tyrion Lannister: "Why do you want me to shut up? Am I starting to make sense?" Questioning our own ideas is the hallmark of a rational person. > So when you realise this point, it becomes almost impossible to proceed > further with any kind of discussion, because even the technical points of > discussion then end up being lost in a tirade of needless side-stepping > discussion. You start the side-stepping the moment you say "I don't like your tone", which is precisely why one should concentrate on the argument being made, and not *how* it's being made. I'm not the only one that things that way, read the extremely useful article from Paul Graham[1]. It is you the one that is against concentrating on the technical points of the discussion. > That is why I think this is the wrong thing to do. If you are suggesting punitive measures, let me remind you that any modern society follows principles established in the Magna Carta eight hundred years ago. Before being punished by the state, every person has the right to a speedy trial, and the trial of course has to be based on *the written law*. If we don't have by-laws, you cannot be blamed to have violated them, and you are even against guidelines, so on what basis are you going to determine that somebody has acted in an illicit way? The opinion of a single dictator? Mob rule? It doesn't matter how you cut it, that would not be the rule of law[2], a concept that has been in the civilized world even longer, for thousands of years. [1] http://www.paulgraham.com/disagree.html [2] http://en.wikipedia.org/wiki/Rule_of_law -- 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 2/6] rebase: prepare to write reflog message for checkout
rebase should never write "checkout: " messages to the reflog, since it is highly confusing to the end user, and breaks grab_nth_branch_checkout(), as demonstrated by failing tests in t/checkout-last. Set a sensible GIT_REFLOG_ACTION: checkout does not respect GIT_REFLOG_ACTION yet, but this defect will be addressed in a future patch. When the defect is addressed, rebase will write the following line to the reflog when started: rebase: checkout master This is much better than the confusing message it currently writes: checkout: moving from master to 1462b67 Signed-off-by: Ramkumar Ramachandra --- git-rebase.sh | 2 ++ 1 file changed, 2 insertions(+) diff --git a/git-rebase.sh b/git-rebase.sh index d0c11a9..6587019 100755 --- a/git-rebase.sh +++ b/git-rebase.sh @@ -568,6 +568,8 @@ test "$type" = interactive && run_specific_rebase # Detach HEAD and reset the tree say "$(gettext "First, rewinding head to replay your work on top of it...")" + +GIT_REFLOG_ACTION="$GIT_REFLOG_ACTION: checkout $onto_name" git checkout -q "$onto^0" || die "could not detach HEAD" git update-ref ORIG_HEAD $orig_head -- 1.8.3.1.384.g7cec0b4 -- 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/6] Fix git checkout - with rebase
Hi, I'm happy to report that I have found a reasonable solution to the problem: see [5/6]. The larger problem still persists: in my opinion, b397ea4 takes the wrong approach to the problem it is attempting to solve; nobody cares _how_ I got to a detached HEAD state; what is important is that I'm stuck in such a state and need useful information. The correct approach, in my opinion, is already taken by my prompt: use git describe. artagnon|checkout-dash=$ git checkout @~1 artagnon|(checkout-dash~1):~/src/git$ Now compare this with the approach taken by the patch: artagnon|(checkout-dash~1):~/src/git$ git status # HEAD detached at 7aa7992 nothing added to commit but untracked files present Completely useless. Unfortunately, it is too late to revert b397ea4, as too much stuff already depends on it now (see builtin/branch.c for example). Reworking the code to use describe is not an easy task at all: describe has no exposed API, and is polluted with die() statements. Nevertheless, it can be a fruitful exercise for someone who is willing to take on the challenge. Thanks. Ramkumar Ramachandra (6): t/checkout-last: checkout - doesn't work after rebase rebase: prepare to write reflog message for checkout rebase -i: prepare to write reflog message for checkout wt-status: remove unused field in grab_1st_switch_cbdata status: do not depend on flaky reflog messages checkout: respect GIT_REFLOG_ACTION builtin/checkout.c | 11 --- git-rebase--interactive.sh | 2 ++ git-rebase.sh | 2 ++ t/t2012-checkout-last.sh | 16 t/t7512-status-help.sh | 37 + wt-status.c| 13 - 6 files changed, 49 insertions(+), 32 deletions(-) -- 1.8.3.1.384.g7cec0b4 -- 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/6] t/checkout-last: checkout - doesn't work after rebase
The following command $ git checkout - does not work as expected after a rebase. Every kind of rebase must behave in the exactly same way: for the purposes of checkout -, the rebase event should be inconsequential. Add two failing tests documenting this bug: one for a normal rebase, and another for an interactive rebase. Signed-off-by: Ramkumar Ramachandra --- t/t2012-checkout-last.sh | 16 1 file changed, 16 insertions(+) diff --git a/t/t2012-checkout-last.sh b/t/t2012-checkout-last.sh index b44de9d..ae6d319 100755 --- a/t/t2012-checkout-last.sh +++ b/t/t2012-checkout-last.sh @@ -116,4 +116,20 @@ test_expect_success 'master...' ' test "z$(git rev-parse --verify HEAD)" = "z$(git rev-parse --verify master^)" ' +test_expect_failure '"checkout -" works after a rebase' ' + git checkout master && + git checkout other && + git rebase master && + git checkout - && + test "z$(git symbolic-ref HEAD)" = "zrefs/heads/master" +' + +test_expect_failure '"checkout -" works after an interactive rebase' ' + git checkout master && + git checkout other && + git rebase -i master && + git checkout - && + test "z$(git symbolic-ref HEAD)" = "zrefs/heads/master" +' + test_done -- 1.8.3.1.384.g7cec0b4 -- 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/6] status: do not depend on flaky reflog messages
b397ea4 (status: show more info than "currently not on any branch", 2013-03-13) made the output of 'git status' richer in the case of a detached HEAD. Before this patch, with a detached HEAD: $ git status # Not currently on any branch. After the patch: $ git checkout v1.8.2 # HEAD is now detached $ git status # HEAD detached at v1.8.2. It works by digging the reflog for the most recent message of the form "checkout: moving from to ". It then asserts that HEAD and "" are the same, and displays this message. When they aren't equal, it displays: $ git status # HEAD detached from fe11db. At this point, the utility of such a message is in question. Moreover, there are several tests in t/status-help that explicitly rely on rebase writing "checkout: " messages to the reflog. As evidenced by the failing tests in t/checkout-last, these messages are completely unintended and flaky. Relying on them only makes it harder to improve the reflog messages written by scripts. As a reasonable compromise, remove the logic to display the "HEAD detached from ..." message, and fallback to "Not currently on any branch." in this case. Update the tests, giving scripts some breathing space. This issue is not isolated to rebase at all. Several other scripts like bisect write (confusing) "checkout: " messages to the reflog. Fixing them is left as an exercise to other contributors. Signed-off-by: Ramkumar Ramachandra --- t/t7512-status-help.sh | 24 +++- wt-status.c| 11 --- 2 files changed, 15 insertions(+), 20 deletions(-) diff --git a/t/t7512-status-help.sh b/t/t7512-status-help.sh index bf08d4e..ed9d57c 100755 --- a/t/t7512-status-help.sh +++ b/t/t7512-status-help.sh @@ -188,10 +188,9 @@ test_expect_success 'status when rebasing -i in edit mode' ' export FAKE_LINES && test_when_finished "git rebase --abort" && ONTO=$(git rev-parse --short HEAD~2) && - TGT=$(git rev-parse --short two_rebase_i) && git rebase -i HEAD~2 && cat >expected <<-EOF && - # HEAD detached from $TGT + # Not currently on any branch. # You are currently editing a commit while rebasing branch '\''rebase_i_edit'\'' on '\''$ONTO'\''. # (use "git commit --amend" to amend the current commit) # (use "git rebase --continue" once you are satisfied with your changes) @@ -246,11 +245,10 @@ test_expect_success 'status after editing the last commit with --amend during a export FAKE_LINES && test_when_finished "git rebase --abort" && ONTO=$(git rev-parse --short HEAD~3) && - TGT=$(git rev-parse --short three_amend) && git rebase -i HEAD~3 && git commit --amend -m "foo" && cat >expected <<-EOF && - # HEAD detached from $TGT + # Not currently on any branch. # You are currently editing a commit while rebasing branch '\''amend_last'\'' on '\''$ONTO'\''. # (use "git commit --amend" to amend the current commit) # (use "git rebase --continue" once you are satisfied with your changes) @@ -280,7 +278,7 @@ test_expect_success 'status: (continue first edit) second edit' ' git rebase -i HEAD~3 && git rebase --continue && cat >expected <<-EOF && - # HEAD detached from $ONTO + # Not currently on any branch. # You are currently editing a commit while rebasing branch '\''several_edits'\'' on '\''$ONTO'\''. # (use "git commit --amend" to amend the current commit) # (use "git rebase --continue" once you are satisfied with your changes) @@ -302,7 +300,7 @@ test_expect_success 'status: (continue first edit) second edit and split' ' git rebase --continue && git reset HEAD^ && cat >expected <<-EOF && - # HEAD detached from $ONTO + # Not currently on any branch. # You are currently splitting a commit while rebasing branch '\''several_edits'\'' on '\''$ONTO'\''. # (Once your working directory is clean, run "git rebase --continue") # @@ -329,7 +327,7 @@ test_expect_success 'status: (continue first edit) second edit and amend' ' git rebase --continue && git commit --amend -m "foo" && cat >expected <<-EOF && - # HEAD detached from $ONTO + # Not currently on any branch. # You are currently editing a commit while rebasing branch '\''several_edits'\'' on '\''$ONTO'\''. # (use "git commit --amend" to amend the current commit) # (use "git rebase --continue" once you are satisfied with your changes) @@ -351,7 +349,7 @@ test_expect_success 'status: (amend first edit) second edit' ' git commit --amend -m "a" && git rebase --continue && cat >expected <<-EOF && - # HEAD detached from $ONTO + # Not currently on any branch. # You are currently editing a commit while rebasing branch '\''several_edits'\''
[PATCH 4/6] wt-status: remove unused field in grab_1st_switch_cbdata
The struct grab_1st_switch_cbdata has the field "found", which is set in
grab_1st_switch() when a match is found. This information is redundant
and unused by any caller: the return value of the function serves to
communicate this information anyway. Remove the field.
Signed-off-by: Ramkumar Ramachandra
---
wt-status.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/wt-status.c b/wt-status.c
index bf84a86..2511270 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1035,7 +1035,6 @@ got_nothing:
}
struct grab_1st_switch_cbdata {
- int found;
struct strbuf buf;
unsigned char nsha1[20];
};
@@ -1059,7 +1058,6 @@ static int grab_1st_switch(unsigned char *osha1, unsigned
char *nsha1,
for (end = target; *end && *end != '\n'; end++)
;
strbuf_add(&cb->buf, target, end - target);
- cb->found = 1;
return 1;
}
--
1.8.3.1.384.g7cec0b4
--
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/6] rebase -i: prepare to write reflog message for checkout
Interactive rebase should never write "checkout: " messages to the reflog, since it is highly confusing to the end user, and breaks grab_nth_branch_checkout(), as demonstrated by failing tests in t/checkout-last. Set a sensible GIT_REFLOG_ACTION: checkout does not respect GIT_REFLOG_ACTION yet, but this defect will be addressed in a future patch. When the defect is addressed, rebase -i will write the following line to the reflog when started: rebase -i (start): checkout master This is much better than the confusing message it currently writes: checkout: moving from master to 1462b67 Signed-off-by: Ramkumar Ramachandra --- git-rebase--interactive.sh | 2 ++ 1 file changed, 2 insertions(+) diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh index f953d8d..0f04425 100644 --- a/git-rebase--interactive.sh +++ b/git-rebase--interactive.sh @@ -838,6 +838,7 @@ comment_for_reflog start if test ! -z "$switch_to" then + GIT_REFLOG_ACTION="$GIT_REFLOG_ACTION: checkout $switch_to" output git checkout "$switch_to" -- || die "Could not checkout $switch_to" fi @@ -981,6 +982,7 @@ has_action "$todo" || test -d "$rewritten" || test -n "$force_rebase" || skip_unnecessary_picks +GIT_REFLOG_ACTION="$GIT_REFLOG_ACTION: checkout $onto_name" output git checkout $onto || die_abort "could not detach HEAD" git update-ref ORIG_HEAD $orig_head do_rest -- 1.8.3.1.384.g7cec0b4 -- 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 6/6] checkout: respect GIT_REFLOG_ACTION
GIT_REFLOG_ACTION is an environment variable specifying the reflog
message to write after an action is completed. Several other commands
including merge, reset, and commit respect it.
Fix the failing tests in t/checkout-last by making checkout respect it
too. You can now expect
$ git checkout -
to work as expected after any rebase operation.
Also update the tests in t/status-help that rely on rebase writing
"checkout: " messages to the reflog.
Signed-off-by: Ramkumar Ramachandra
---
builtin/checkout.c | 11 ---
t/t2012-checkout-last.sh | 4 ++--
t/t7512-status-help.sh | 13 ++---
3 files changed, 16 insertions(+), 12 deletions(-)
diff --git a/builtin/checkout.c b/builtin/checkout.c
index f5b50e5..1e2af85 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -587,7 +587,7 @@ static void update_refs_for_switch(const struct
checkout_opts *opts,
struct branch_info *new)
{
struct strbuf msg = STRBUF_INIT;
- const char *old_desc;
+ const char *old_desc, *reflog_msg;
if (opts->new_branch) {
if (opts->new_orphan_branch) {
if (opts->new_branch_log && !log_all_ref_updates) {
@@ -620,8 +620,13 @@ static void update_refs_for_switch(const struct
checkout_opts *opts,
old_desc = old->name;
if (!old_desc && old->commit)
old_desc = sha1_to_hex(old->commit->object.sha1);
- strbuf_addf(&msg, "checkout: moving from %s to %s",
- old_desc ? old_desc : "(invalid)", new->name);
+
+ reflog_msg = getenv("GIT_REFLOG_ACTION");
+ if (!reflog_msg)
+ strbuf_addf(&msg, "checkout: moving from %s to %s",
+ old_desc ? old_desc : "(invalid)", new->name);
+ else
+ strbuf_insert(&msg, 0, reflog_msg, strlen(reflog_msg));
if (!strcmp(new->name, "HEAD") && !new->path && !opts->force_detach) {
/* Nothing to do. */
diff --git a/t/t2012-checkout-last.sh b/t/t2012-checkout-last.sh
index ae6d319..336b6f2 100755
--- a/t/t2012-checkout-last.sh
+++ b/t/t2012-checkout-last.sh
@@ -116,7 +116,7 @@ test_expect_success 'master...' '
test "z$(git rev-parse --verify HEAD)" = "z$(git rev-parse --verify
master^)"
'
-test_expect_failure '"checkout -" works after a rebase' '
+test_expect_success '"checkout -" works after a rebase' '
git checkout master &&
git checkout other &&
git rebase master &&
@@ -124,7 +124,7 @@ test_expect_failure '"checkout -" works after a rebase' '
test "z$(git symbolic-ref HEAD)" = "zrefs/heads/master"
'
-test_expect_failure '"checkout -" works after an interactive rebase' '
+test_expect_success '"checkout -" works after an interactive rebase' '
git checkout master &&
git checkout other &&
git rebase -i master &&
diff --git a/t/t7512-status-help.sh b/t/t7512-status-help.sh
index ed9d57c..f8661e4 100755
--- a/t/t7512-status-help.sh
+++ b/t/t7512-status-help.sh
@@ -77,7 +77,7 @@ test_expect_success 'status when rebase in progress before
resolving conflicts'
ONTO=$(git rev-parse --short HEAD^^) &&
test_must_fail git rebase HEAD^ --onto HEAD^^ &&
cat >expected <<-EOF &&
- # HEAD detached at $ONTO
+ # Not currently on any branch.
# You are currently rebasing branch '\''rebase_conflicts'\'' on
'\''$ONTO'\''.
# (fix conflicts and then run "git rebase --continue")
# (use "git rebase --skip" to skip this patch)
@@ -104,7 +104,7 @@ test_expect_success 'status when rebase in progress before
rebase --continue' '
echo three >main.txt &&
git add main.txt &&
cat >expected <<-EOF &&
- # HEAD detached at $ONTO
+ # Not currently on any branch.
# You are currently rebasing branch '\''rebase_conflicts'\'' on
'\''$ONTO'\''.
# (all conflicts fixed: run "git rebase --continue")
#
@@ -136,7 +136,7 @@ test_expect_success 'status during rebase -i when conflicts
unresolved' '
ONTO=$(git rev-parse --short rebase_i_conflicts) &&
test_must_fail git rebase -i rebase_i_conflicts &&
cat >expected <<-EOF &&
- # HEAD detached at $ONTO
+ # Not currently on any branch.
# You are currently rebasing branch '\''rebase_i_conflicts_second'\''
on '\''$ONTO'\''.
# (fix conflicts and then run "git rebase --continue")
# (use "git rebase --skip" to skip this patch)
@@ -162,7 +162,7 @@ test_expect_success 'status during rebase -i after
resolving conflicts' '
test_must_fail git rebase -i rebase_i_conflicts &&
git add main.txt &&
cat >expected <<-EOF &&
- # HEAD detached at $ONTO
+ # Not currently on any branch.
# You are currently rebasing branch '\''rebase_i_conflicts_second'\''
on '\''$ONTO'\''.
# (all conflicts fixed: run "git rebase --continue")
#
@@ -215,9 +215,8
Re: [PATCH 2/3] Move copy_note_for_rewrite + friends from builtin/notes.c to notes-utils.c
On Thu, Jun 13, 2013 at 1:45 AM, Andreas Krey wrote: > On Wed, 12 Jun 2013 13:28:05 +, Felipe Contreras wrote: > ... >> And you are >> doing that with the express purpose of annoying. > > Where did 'assume good faith' go to today? Did you read the last part? "This does not mean that one should continue to assume good faith when there's evidence to the contrary." That being said, my evidence was not solid, and while there is still the possibility that he was indeed acting in good faith, I've received no response from him, and Junio has committed the change without any mentioning of where the idea come from. Either way, I bet you my good faith suggestion will *not* end up in the official guidelines, nor will any suggestion of mine. -- Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC 3/4] git-mw: Adding git-mw.perl script
benoit.per...@ensimag.fr writes: > From: Benoit Person > > This script will be used for all tools and command related to a mediawiki > remote. In this commit we introduce the tool, the way it parses argument > and subcommands and an example of subcommand: "help". It also updates > the Makefile so that the new tool is installed properly. How does the "make" Vs "make install" work? How does a developer run the tool without installing? I first tried: $ ../../bin-wrappers/git mw git: 'mw' is not a git command. See 'git --help'. Then, this first seem OK: $ ./git-mw usage: git mw git mw commands are: HelpDisplay help information about git mw Preview Parse and render local file into HTML BUT, this will take the installed GitMediawiki.pm if it is available, and we don't want this (if one hacks GitMediawiki.pm locally, one wants the new hacked to be taken into account without "make install"ing it). To understand better how it works, try adding this in git-mw.perl: print "$_\n" for @INC; I get this: /home/moy/local/usr-squeeze/share/perl/5.14.2 /home/moy/local/usr-squeeze/src/MediaWiki-API-0.39/blib/lib /etc/perl /usr/local/lib/perl/5.14.2 /usr/local/share/perl/5.14.2 /usr/lib/perl5 /usr/share/perl5 /usr/lib/perl/5.14 /usr/share/perl/5.14 /usr/local/lib/site_perl . The '.' is there, but it comes after the hardcoded /home/moy/local/usr-squeeze/share/perl/5.14.2 (which has to comes first, to let the install version be robust to whatever comes after). I think you need an equivalent of Git's toplevel bin-wrappers/git, or perhaps use the same bin-wrapper/git but let "make install" in contrib/mw-to-git/ install GitMediawiki.pm in perl/blib/lib BTW, I just noticed we had a Git::SVN, so perhaps GitMediawiki should be Git::MediaWiki. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: slow process of post-receive script on a remote (samba) share
Tamas Csabina writes:
> I am using Git bash from version 1.8.3.msysgit.0, on a Windows 7x64 PC.
> I have an issue with executing git push if I have a post-receive
> script configured.
> The content of the script is not really important, as if I have a
> script that contains only commented out lines (around 70 lines), my
> git push command is delayed with around 5 seconds.
>
>
> I`ve tested the script on another PC and it is working fine. No delay
> at all. So there are some issues on my PC regarding how git processes
> remote scripts.
>
> I took a wireshark trace with 2 scenarios on my PC:
>
> 1. just execute `cat \post-receive` command in the git
> bash
> 2. did a `real` git push
>
> Results of the wireshark traces shows:
>
> 1. Read AndX Request, FID: 0x228f, 1024 bytes at offset 0 (1024 bytes
> at time, always)
> 2. Read AndX Request, FID: 0x21c9, 1 byte at offset 0 (1 byte, always)
>
> Conclusion:
> git push command reads the post-receive script in 1 byte chunks, which
> dramatically slows down the execution process.
git doesn't read the script; the interpreter does. In the case of a
script, the interpreter is specified in the #! line at the top; in the
case of a binary executable, it is specified within the executable (and
for linux, is usually /lib/ld-linux.so.2).
Exactly the same should happen if you run the hook manually, so you can
try that to debug it.
Note also that Weird Things(tm) relating to SIGPIPE may happen if you
don't read your input. Even if you are only fooling around for testing,
a post-receive hook must consume its input, e.g., by discarding it with
'cat >/dev/null'.
--
Thomas Rast
trast@{inf,student}.ethz.ch
--
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: New feature discussion: git rebase --status
Le 2013-06-13 07:52, Antoine Pelisse a écrit : On Thu, Jun 13, 2013 at 12:19 AM, Junio C Hamano wrote: Antoine Pelisse writes: Maybe we can display previous and next commits to provide some context. Like we do for diff. For example: $ git status # HEAD detached from ecb9f3e # Already applied 330 patches (displaying next 3): # b170635... my_commit_message # b170635... my_commit_message # b170635... my_commit_message # Already applied 119 (displaying last 3) # b170635... my_commit_message # b170635... my_commit_message # b170635... my_commit_message I think you meant one of them to be # Still to be applied 119 (showing the first 3) instead. Of course, I am not sure if it is worth 8 lines, especially given that "git log --oneline -$n" would give you "Already applied" part that is beyond what will be shown in this message easily if you wanted to. So it might be enough to show "The one that has last been replayed" (aka "HEAD") and "The one you are in the middle of replaying". That's very true. The piece of information that is hard to get is "what's left to be done". So something like this would make sense: $ git status # HEAD detached from ecb9f3e # You are currently editing a832578... my_commit_message [120/450] while rebasing. # 320 patches left to apply (showing next 3): # b170635... my_commit_message # b170635... my_commit_message # b170635... my_commit_message # (use "git commit --amend" to amend the current commit) # (use "git rebase --continue" once you are satisfied with your changes) So that's 4 extra lines compared to current output. But should we make it a default ? Personally I believe we should not make it the default output. Currently, the output I'm working on is the following: $ git status # HEAD detached from ecb9f3e # You are currently editing a832578 while rebasing branch 'split-rm-v7' on 'ecb9f3e'. # (use "git commit --amend" to amend the current commit) # (use "git rebase --continue" once you are satisfied with your changes) $ git status --rebase-todo # HEAD detached from ecb9f3e # You are currently editing a832578 while rebasing branch 'split-rm-v7' on 'ecb9f3e'. # Still 2 patches left to apply: # e a832578 rm: better error message on failure for multiple files # e fd0330b rm: introduce advice.rmHints to shorten messages # (use "git commit --amend" to amend the current commit) # (use "git rebase --continue" once you are satisfied with your changes) I'm still unsure about the name of the flag, I chose that one accordingly to .git/merge-rebase/rebase-todo -- Mathieu Liénard--Mayor, 2nd year at Grenoble INP - ENSIMAG (+33)6 80 56 30 02 -- 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/RFC 4/4] git-mw: Adding preview tool in git-mw.perl
benoit.per...@ensimag.fr writes:
> From: Benoit Person
>
> This final commit adds the preview subcommand to git mw. It works as such:
> 1- Find the remote name of the current branch's upstream and check if it's a
> mediawiki one.
> 1b- If it's not found or if it's not a mediawiki one. It will list all the
> mediawiki remotes configured and ask the user to replay the command with the
> --remote option set.
> 2- Parse the content of the local file (or blob) (given as argument) using
> the distant mediawiki's API
> 3- Retrieve the current page on the distant mediawiki
> 4- Replaces all content in that page with the newly parsed one
> 5- Convert relative links into absolute
> 6- Save the result on disk
>
> The command accepts those options:
> --autoload | -a tries to launch the newly generated file in the user's
> default browser (using git web--browse)
> --remote | -r provides a way to select the distant mediawiki in which
> the user wants to preview his file (or blob)
> --output | -o enables the user to choose the output filename. Default
> output filename is based on the input filename in which
> the extension '.mw' is replaced with '.html'
> --blob | -b tells the script that the last argument is a blob and not
> a filename
A commit messages that answers the "what?" and "how?" questions (as
opposed to "why?") is always suspicious: doesn't the message belong
elsewhere?
Here, you have a nice user documentation for command-line options, and
the actual user doc is much poorer:
> +sub preview_help {
> + print <<'END';
> +usage: git mw preview [--remote|-r ] [--autoload|-a]
> + [--output|-o ]
> +
> +-r, --remoteSpecify which mediawiki should be used
> +-o, --outputName of the output file
> +-a, --autoload Autoload the page in your default web browser
> +END
(shorter description, missing --blob)
> + } else { # file mode
> + if (! -e $file_name) {
> + die "File $file_name does not exists \n";
We're just setting a convention to use ${var} in string interpolation
(Celestin's perlcritic patch series), so better do it right now ;-).
Did you try "make perlcritic" on your code?
> + # Default preview_file_name is file_name with .html ext
> + if ($preview_file_name eq '') {
EMPTY ?
> + if ($remote_name eq '') {
EMPTY ?
> + # Load template page
> + $template = get("$remote_url/index.php?title=$wiki_page_name")
> + or die "You need to create $wiki_page_name before previewing
> it";
I got hit again by the HTTPS certificate validation failure. It would
make sense to have a more detailed error message, including the URL,
because having the same error:
You need to create Accueil before previewing it at
/home/moy/local/usr-wheezy/libexec/git-core/git-mw line 182.
for any kind of HTTP failure is a painful. Doesn't "get" return an HTTP
code? If so, your message would make sense for 404 errors, but not for
the others.
> + $mw_content_text = $html_tree->look_down('id', 'mw-content-text');
Unfortunately, this doesn't seem standard. It doesn't work on
https://ensiwiki.ensimag.fr/index.php/Accueil at least (which is my main
use-case :-( ).
At least, you should check $mw_content_text and have a nice error
message here. As much as possible, you should allow a way to solve it
(make the lookup configurable in .git/config, or allow the user to
specify an arbitrary HTML template to plug onto, or display the raw,
incomplete, HTML).
I replaced 'mw-content-text' with 'bodyContent' and it worked.
Then I got
Wide character in print at /usr/lib/perl/5.14/IO/Handle.pm line 159.
but the file was generated. There are encoding problems: the title says
"Le Wiki des étudiants et enseignants" (it should be a É).
I guess you fed the API with an improper encoding (double UTF-8
encoding, or UTF-8 announced as latin-1 or so), and the API returned you
some hard-coded, badly encoded, rendered HTML.
> @@ -41,6 +241,7 @@ usage: git mw
>
> git mw commands are:
> HelpDisplay help information about git mw
> +Preview Parse and render local file into HTML
> END
Lower-case help and preview.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC 1/4] git-mw: Introduction of GitMediawiki.pm
benoit.per...@ensimag.fr writes: > +install_pm: > + cp $(GIT_MEDIAWIKI_PM) $(INSTLIBDIR) Better use "install", which is roughly like "cp". Also, this fails if the target dir does not exist, ie. if one did not run "make install" at the toplevel Git. It's OK, but perhaps you should add a comment in the Makefile like '# Run "make install" from Git's toplevel before using this'. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-remote-mediawiki: remove hardcoded version number in the test suite
benoit.per...@ensimag.fr writes: > Updates the code to make it more easy to switch mediawiki version when > testing. Before that, the version number was partly hardcoded, partly > in a var. This is obviously good. > Maybe I should add a warning that the installation procedure may not work > in the future ? It seems to work for the range 1.19.X - 1.21.X though :) ? > Should I also update the version number to the latest one (1.21.1) ? This kind of remarks does not belong to the commit message, it should be below the --- (before the diffstat). And yes, I think a comment saying "# Versions foo, bar and boz have been tested" or so would be welcome. Thanks, -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] git-remote-mediawiki: remove hardcoded version number in the test suite
From: Benoit Person
Updates the code to make it more easy to switch mediawiki version when
testing. Before that, the version number was partly hardcoded, partly
in a var.
Maybe I should add a warning that the installation procedure may not work
in the future ? It seems to work for the range 1.19.X - 1.21.X though :) ?
Should I also update the version number to the latest one (1.21.1) ?
Signed-off-by: Benoit Person
Signed-off-by: Matthieu Moy
---
contrib/mw-to-git/t/test-gitmw-lib.sh | 19 ++-
contrib/mw-to-git/t/test.config | 3 ++-
2 files changed, 12 insertions(+), 10 deletions(-)
diff --git a/contrib/mw-to-git/t/test-gitmw-lib.sh
b/contrib/mw-to-git/t/test-gitmw-lib.sh
index 3b2cfac..bb76cee 100755
--- a/contrib/mw-to-git/t/test-gitmw-lib.sh
+++ b/contrib/mw-to-git/t/test-gitmw-lib.sh
@@ -336,20 +336,21 @@ wiki_install () {
fi
# Fetch MediaWiki's archive if not already present in the TMP directory
+ MW_FILENAME="mediawiki-$MW_VERSION_MAJOR.$MW_VERSION_MINOR.tar.gz"
cd "$TMP"
- if [ ! -f "$MW_VERSION.tar.gz" ] ; then
- echo "Downloading $MW_VERSION sources ..."
- wget
"http://download.wikimedia.org/mediawiki/1.19/mediawiki-1.19.0.tar.gz"; ||
+ if [ ! -f $MW_FILENAME ] ; then
+ echo "Downloading $MW_VERSION_MAJOR.$MW_VERSION_MINOR sources
..."
+ wget
"http://download.wikimedia.org/mediawiki/$MW_VERSION_MAJOR/$MW_FILENAME"; ||
error "Unable to download "\
- "http://download.wikimedia.org/mediawiki/1.19/"\
- "mediawiki-1.19.0.tar.gz. "\
+
"http://download.wikimedia.org/mediawiki/$MW_VERSION_MAJOR/"\
+ "$MW_FILENAME. "\
"Please fix your connection and launch the script
again."
- echo "$MW_VERSION.tar.gz downloaded in `pwd`. "\
+ echo "$MW_FILENAME downloaded in `pwd`. "\
"You can delete it later if you want."
else
- echo "Reusing existing $MW_VERSION.tar.gz downloaded in `pwd`."
+ echo "Reusing existing $MW_FILENAME downloaded in `pwd`."
fi
- archive_abs_path=$(pwd)/"$MW_VERSION.tar.gz"
+ archive_abs_path=$(pwd)/$MW_FILENAME
cd "$WIKI_DIR_INST/$WIKI_DIR_NAME/" ||
error "can't cd to $WIKI_DIR_INST/$WIKI_DIR_NAME/"
tar xzf "$archive_abs_path" --strip-components=1 ||
@@ -431,5 +432,5 @@ wiki_delete () {
# Delete the wiki's SQLite database
rm -f "$TMP/$DB_FILE" || error "Database $TMP/$DB_FILE could not be
deleted."
rm -f "$FILES_FOLDER/$DB_FILE"
- rm -rf "$TMP/$MW_VERSION"
+ rm -rf "$TMP/mediawiki-$MW_VERSION_MAJOR.$MW_VERSION_MINOR.tar.gz"
}
diff --git a/contrib/mw-to-git/t/test.config b/contrib/mw-to-git/t/test.config
index 958b37b..f835dcc 100644
--- a/contrib/mw-to-git/t/test.config
+++ b/contrib/mw-to-git/t/test.config
@@ -30,6 +30,7 @@ WEB_WWW=$WEB/www
# The variables below are used by the script to install a wiki.
# You should not modify these unless you are modifying the script itself.
-MW_VERSION=mediawiki-1.19.0
+MW_VERSION_MAJOR=1.20
+MW_VERSION_MINOR=0
FILES_FOLDER=install-wiki
DB_INSTALL_SCRIPT=db_install.php
--
1.8.3.GIT
--
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/RFC V2 0/4] git-remote-mediawiki: new tool to preview local changes without pushing
benoit.per...@ensimag.fr writes: > For now, this PATCH/RFC is based on the 'next' branch merged with the > bp/mediawiki-credential patch. For the final version, I will try > to rebase it on celestin's work with perlcritic. Actually, it seems based on an old "next" branch. This hunk --- a/contrib/mw-to-git/git-remote-mediawiki.perl +++ b/contrib/mw-to-git/git-remote-mediawiki.perl @@ -13,6 +13,8 @@ use strict; use MediaWiki::API; +use Git; Fails to apply because "use Git;" is already there in today's next. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] rebase -i: write better reflog messages for start
Ramkumar Ramachandra wrote: > t/status-help. Looks seriously unrelated, and I'm breaking my head > over it. Any clues? Damn it! A recent commit is responsible for this avalanche in test breakages: b397ea (status: show more info than "currently not on any branch", 2013-03-13). It re-implements a backward version of grab_nth_branch_switch(): grab_1st_switch() essentially _relies_ on the random unintended pollution that rebase writes to the reflog to print a more useful (?) status :/ I have no choice but to completely redo this bit, and update all the tests. Let me know if there is some easy way to work around this that I'm missing. Thanks. -- 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] Documentation/CommunityGuidelines
Hello, On Mon, Jun 10, 2013 at 06:58:47PM +0530, Ramkumar Ramachandra wrote: > I've tried to write down a bare minimum, without restating the obvious. [...] I often come across so-called "community guidelines" in other projects---some of which adhere to them quite strictly, and others simply document something for the curious. But usually the reason for their existence in the first place are tell-tale signs of trying to fix a problem at the wrong end, and I believe this is what is about to happen if a document such as this ever becomes official. There's no disputing the fact that over the last few weeks, FC's behaviour has been called in to question. He's managed to rub a lot of core people up the wrong way, and in doing so has deliberately side-stepped that problem by doing the one thing which puts anyone trying to raise that point muted; by assuming that he's right. It's a point on which one is never going to win, because no matter what one says, it'll just get twisted round in such a way that one then ends up questioning their own words, and their own conduct, and that's bad, because there never was anything wrong with them to begin with. So when you realise this point, it becomes almost impossible to proceed further with any kind of discussion, because even the technical points of discussion then end up being lost in a tirade of needless side-stepping discussion. It's a bullying tactic on the part of FC which means he'll do any, and everything, to get his own way. So I say to all those seasoned reviewers out there on this list not to put up with it. There comes a point, regardless of how useful someone's contribution may be, that if the barrier to entry is so high that any kind of criticism or comment made against code comes with a massive chance of having to defend yourself against innocence on the part of the reviewer, that those contributions should be shelved. I've seen also another yardstick used to defend FC's behaviour, and that is one of "commits within the last three months". That count is completely meaningless, since the review process is always going to be the same. So these guidelines gain the community nothing, and only serve to punish those who are already following them, without them being written down, because the root-cause of the problem is still here, and isn't going to go away, no matter how much referring to these guidelines might help. That is why I think this is the wrong thing to do. -- Thomas Adam -- 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/RFC 4/4] git-mw: Adding preview tool in git-mw.perl
From: Benoit Person
This final commit adds the preview subcommand to git mw. It works as such:
1- Find the remote name of the current branch's upstream and check if it's a
mediawiki one.
1b- If it's not found or if it's not a mediawiki one. It will list all the
mediawiki remotes configured and ask the user to replay the command with the
--remote option set.
2- Parse the content of the local file (or blob) (given as argument) using
the distant mediawiki's API
3- Retrieve the current page on the distant mediawiki
4- Replaces all content in that page with the newly parsed one
5- Convert relative links into absolute
6- Save the result on disk
The command accepts those options:
--autoload | -a tries to launch the newly generated file in the user's
default browser (using git web--browse)
--remote | -r provides a way to select the distant mediawiki in which
the user wants to preview his file (or blob)
--output | -o enables the user to choose the output filename. Default
output filename is based on the input filename in which
the extension '.mw' is replaced with '.html'
--blob | -b tells the script that the last argument is a blob and not
a filename
Signed-off-by: Benoit Person
Signed-off-by: Matthieu Moy
---
contrib/mw-to-git/git-mw.perl | 203 +-
1 file changed, 202 insertions(+), 1 deletion(-)
diff --git a/contrib/mw-to-git/git-mw.perl b/contrib/mw-to-git/git-mw.perl
index a2f0aa1..79c6cd0 100644
--- a/contrib/mw-to-git/git-mw.perl
+++ b/contrib/mw-to-git/git-mw.perl
@@ -12,10 +12,37 @@ use strict;
use warnings;
use Getopt::Long;
+use URI::URL qw(url);
+use IO::File;
+use LWP::Simple;
+use HTML::TreeBuilder;
+
+use Git;
+use MediaWiki::API;
+use GitMediawiki qw(smudge_filename connect_maybe);
+
+#preview parameters
+my $file_name;
+my $remote_name = '';
+my $preview_file_name = '';
+my $autoload = 0;
+my $blob = 0;
+sub file {
+ $file_name = shift;
+ return $file_name;
+}
my %commands = (
'help' =>
- [\&help, {}, \&help]
+ [\&help, {}, \&help],
+ 'preview' =>
+ [\&preview, {
+ '<>' => \&file,
+ 'output|o=s' => \$preview_file_name,
+ 'remote|r=s' => \$remote_name,
+ 'autoload|a' => \$autoload,
+ 'blob|b' => \$blob
+ }, \&preview_help]
);
# Search for sub-command
@@ -33,6 +60,179 @@ GetOptions( %{$cmd->[1]},
# Launch command
&{$cmd->[0]};
+# Preview Functions
+
+sub preview_help {
+ print <<'END';
+usage: git mw preview [--remote|-r ] [--autoload|-a]
+ [--output|-o ]
+
+-r, --remoteSpecify which mediawiki should be used
+-o, --outputName of the output file
+-a, --autoload Autoload the page in your default web browser
+END
+ exit;
+}
+
+sub preview {
+ my $wiki;
+ my ($remote_url, $wiki_page_name);
+ my ($content, $content_tree, $template, $html_tree, $mw_content_text);
+ my $file_content;
+
+ # file_name argumeent is mandatory
+ if (! defined $file_name) {
+ die "File not set, see `git mw help` \n";
+ }
+
+ if ($blob) { # blob mode
+ $blob = $file_name;
+ if ($blob =~ /(.+):(.+)/) {
+ $file_name = $2;
+ }
+ } else { # file mode
+ if (! -e $file_name) {
+ die "File $file_name does not exists \n";
+ }
+ }
+
+ # Default preview_file_name is file_name with .html ext
+ if ($preview_file_name eq '') {
+ $preview_file_name = $file_name;
+ $preview_file_name =~ s/\.[^.]+$/.html/;
+ }
+
+ # Transform file_name into a mediawiki page name
+ $wiki_page_name = smudge_filename($file_name);
+ $wiki_page_name =~ s/\.[^.]+$//;
+
+ if ($remote_name eq '') {
+ # Search current branch upstream branch remote
+ $remote_name = git_cmd_try {
+ my $current_branch =
+ Git::command_oneline('symbolic-ref', '--short',
'HEAD');
+ Git::config("branch.$current_branch.remote") }
+ "%s failed w/ code %d";
+
+ if ($remote_name) {
+ $remote_url = mediawiki_remote_url_maybe($remote_name);
+ }
+
+ # Search all possibles mediawiki remotes
+ if (! $remote_url) {
+ my @remotes = git_cmd_try {
+ Git::command('remote'); }
+ "%s failed w/ code%d";
+
+ my @valid_remotes = ();
+ foreach my $remote (@remotes) {
+
[PATCH/RFC 3/4] git-mw: Adding git-mw.perl script
From: Benoit Person
This script will be used for all tools and command related to a mediawiki
remote. In this commit we introduce the tool, the way it parses argument
and subcommands and an example of subcommand: "help". It also updates
the Makefile so that the new tool is installed properly.
Signed-off-by: Benoit Person
Signed-off-by: Matthieu Moy
---
contrib/mw-to-git/Makefile| 7 ---
contrib/mw-to-git/git-mw.perl | 46 +++
2 files changed, 50 insertions(+), 3 deletions(-)
create mode 100644 contrib/mw-to-git/git-mw.perl
diff --git a/contrib/mw-to-git/Makefile b/contrib/mw-to-git/Makefile
index fe30e7f..c0633b1 100644
--- a/contrib/mw-to-git/Makefile
+++ b/contrib/mw-to-git/Makefile
@@ -6,6 +6,7 @@
GIT_MEDIAWIKI_PM=GitMediawiki.pm
SCRIPT_PERL=git-remote-mediawiki.perl
+SCRIPT_PERL+=git-mw.perl
GIT_ROOT_DIR=../..
HERE=contrib/mw-to-git/
@@ -19,14 +20,14 @@ install_pm:
cp $(GIT_MEDIAWIKI_PM) $(INSTLIBDIR)
build:
- $(MAKE) -C $(GIT_ROOT_DIR) SCRIPT_PERL=$(SCRIPT_PERL_FULL) \
+ $(MAKE) -C $(GIT_ROOT_DIR) SCRIPT_PERL="$(SCRIPT_PERL_FULL)" \
build-perl-script
install: install_pm
- $(MAKE) -C $(GIT_ROOT_DIR) SCRIPT_PERL=$(SCRIPT_PERL_FULL) \
+ $(MAKE) -C $(GIT_ROOT_DIR) SCRIPT_PERL="$(SCRIPT_PERL_FULL)" \
install-perl-script
clean:
- $(MAKE) -C $(GIT_ROOT_DIR) SCRIPT_PERL=$(SCRIPT_PERL_FULL) \
+ $(MAKE) -C $(GIT_ROOT_DIR) SCRIPT_PERL="$(SCRIPT_PERL_FULL)" \
clean-perl-script
rm $(INSTLIBDIR)/$(GIT_MEDIAWIKI_PM)
\ No newline at end of file
diff --git a/contrib/mw-to-git/git-mw.perl b/contrib/mw-to-git/git-mw.perl
new file mode 100644
index 000..a2f0aa1
--- /dev/null
+++ b/contrib/mw-to-git/git-mw.perl
@@ -0,0 +1,46 @@
+#!/usr/bin/perl
+
+# Copyright (C) 2013
+# Benoit Person
+# Celestin Matte
+# License: GPL v2 or later
+
+# Set of tools for git repo with a mediawiki remote.
+# Documentation & bugtracker: https://github.com/moy/Git-Mediawiki/
+
+use strict;
+use warnings;
+
+use Getopt::Long;
+
+my %commands = (
+ 'help' =>
+ [\&help, {}, \&help]
+);
+
+# Search for sub-command
+my $cmd = $commands{'help'};
+for (my $i = 0; $i < @ARGV; $i++) {
+ if (defined $commands{$ARGV[$i]}) {
+ $cmd = $commands{$ARGV[$i]};
+ splice @ARGV, $i, 1;
+ last;
+ }
+};
+GetOptions( %{$cmd->[1]},
+ 'help|h' => \&{$cmd->[2]});
+
+# Launch command
+&{$cmd->[0]};
+
+## Help Functions
##
+
+sub help {
+ print <<'END';
+usage: git mw
+
+git mw commands are:
+HelpDisplay help information about git mw
+END
+ exit;
+}
\ No newline at end of file
--
1.8.3.GIT
--
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/RFC 1/4] git-mw: Introduction of GitMediawiki.pm
From: Benoit Person
The GitMediawiki.pm goal is to share code betwwen several scripts in
Git-Mediawiki (for now, git-mw.perl introduced in this patch serie and
git-remote-mediawiki.perl)
Signed-off-by: Benoit Person
Signed-off-by: Matthieu Moy
---
contrib/mw-to-git/GitMediawiki.pm | 24
contrib/mw-to-git/Makefile| 19 +--
2 files changed, 41 insertions(+), 2 deletions(-)
create mode 100644 contrib/mw-to-git/GitMediawiki.pm
diff --git a/contrib/mw-to-git/GitMediawiki.pm
b/contrib/mw-to-git/GitMediawiki.pm
new file mode 100644
index 000..8a0ffc7
--- /dev/null
+++ b/contrib/mw-to-git/GitMediawiki.pm
@@ -0,0 +1,24 @@
+package GitMediawiki;
+
+use 5.008;
+use strict;
+use Git;
+
+BEGIN {
+
+our ($VERSION, @ISA, @EXPORT, @EXPORT_OK);
+
+# Totally unstable API.
+$VERSION = '0.01';
+
+require Exporter;
+
+@ISA = qw(Exporter);
+
+@EXPORT = ();
+
+# Methods which can be called as standalone functions as well:
+@EXPORT_OK = ();
+}
+
+1; # Famous last words
\ No newline at end of file
diff --git a/contrib/mw-to-git/Makefile b/contrib/mw-to-git/Makefile
index f149719..fe30e7f 100644
--- a/contrib/mw-to-git/Makefile
+++ b/contrib/mw-to-git/Makefile
@@ -4,14 +4,29 @@
#
## Build git-remote-mediawiki
+GIT_MEDIAWIKI_PM=GitMediawiki.pm
SCRIPT_PERL=git-remote-mediawiki.perl
GIT_ROOT_DIR=../..
HERE=contrib/mw-to-git/
SCRIPT_PERL_FULL=$(patsubst %,$(HERE)/%,$(SCRIPT_PERL))
+INSTLIBDIR=$(shell $(MAKE) -C $(GIT_ROOT_DIR)/perl \
+-s --no-print-directory instlibdir)
all: build
-build install clean:
+install_pm:
+ cp $(GIT_MEDIAWIKI_PM) $(INSTLIBDIR)
+
+build:
+ $(MAKE) -C $(GIT_ROOT_DIR) SCRIPT_PERL=$(SCRIPT_PERL_FULL) \
+build-perl-script
+
+install: install_pm
+ $(MAKE) -C $(GIT_ROOT_DIR) SCRIPT_PERL=$(SCRIPT_PERL_FULL) \
+install-perl-script
+
+clean:
$(MAKE) -C $(GIT_ROOT_DIR) SCRIPT_PERL=$(SCRIPT_PERL_FULL) \
-$@-perl-script
+clean-perl-script
+ rm $(INSTLIBDIR)/$(GIT_MEDIAWIKI_PM)
\ No newline at end of file
--
1.8.3.GIT
--
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/RFC 2/4] git-mw: Moving some functions from git-remote-mediawiki.perl to GitMediawiki.pm
From: Benoit Person
Moving mediawiki_clean_filename, mediawiki_smudge_filename and mw_connect_maybe
Since we have a clean namespace in a perl module, we also rename them into more
concise ones (clean_filename, smudge_filename and connect_maybe). It also
delete the side effects of mw_connect_maybe requiring it to be called with
parameters and returning the new instance of the Mediawiki::Api if it needs to
be created
Signed-off-by: Benoit Person
Signed-off-by: Matthieu Moy
---
contrib/mw-to-git/GitMediawiki.pm | 72 +-
contrib/mw-to-git/git-remote-mediawiki.perl | 80 +
2 files changed, 84 insertions(+), 68 deletions(-)
diff --git a/contrib/mw-to-git/GitMediawiki.pm
b/contrib/mw-to-git/GitMediawiki.pm
index 8a0ffc7..acf4e43 100644
--- a/contrib/mw-to-git/GitMediawiki.pm
+++ b/contrib/mw-to-git/GitMediawiki.pm
@@ -18,7 +18,77 @@ require Exporter;
@EXPORT = ();
# Methods which can be called as standalone functions as well:
-@EXPORT_OK = ();
+@EXPORT_OK = qw(clean_filename smudge_filename connect_maybe);
+}
+
+# Mediawiki filenames can contain forward slashes. This variable decides by
which pattern they should be replaced
+use constant SLASH_REPLACEMENT => '%2F';
+
+sub clean_filename {
+ my $filename = shift;
+ $filename =~ s{@{[SLASH_REPLACEMENT]}}{/}g;
+ # [, ], |, {, and } are forbidden by MediaWiki, even URL-encoded.
+ # Do a variant of URL-encoding, i.e. looks like URL-encoding,
+ # but with _ added to prevent MediaWiki from thinking this is
+ # an actual special character.
+ $filename =~ s/[\[\]\{\}\|]/sprintf("_%%_%x", ord($&))/ge;
+ # If we use the uri escape before
+ # we should unescape here, before anything
+
+ return $filename;
+}
+
+sub smudge_filename {
+ my $filename = shift;
+ $filename =~ s{/}{@{[SLASH_REPLACEMENT]}}g;
+ $filename =~ s/ /_/g;
+ # Decode forbidden characters encoded in clean_filename
+ $filename =~ s/_%_([0-9a-fA-F][0-9a-fA-F])/sprintf('%c', hex($1))/ge;
+ return $filename;
+}
+
+sub connect_maybe {
+ my $wiki = shift;
+ if ($wiki) {
+ return $wiki;
+ }
+
+ my $remote_name = shift;
+ my $remote_url = shift;
+ my ($wiki_login, $wiki_password, $wiki_domain);
+
+ git_cmd_try {
+ $wiki_login = Git::config("remote.${remote_name}.mwLogin");
+ $wiki_password =
Git::config("remote.${remote_name}.mwPassword");
+ $wiki_domain = Git::config("remote.${remote_name}.mwDomain");}
+ "%s failed w/ code %d";
+
+ $wiki = MediaWiki::API->new;
+ $wiki->{config}->{api_url} = "${remote_url}/api.php";
+ if ($wiki_login) {
+ my %credential = (
+ 'url' => $remote_url,
+ 'username' => $wiki_login,
+ 'password' => $wiki_password
+ );
+ Git::credential(\%credential);
+ my $request = {lgname => $credential{username},
+ lgpassword => $credential{password},
+ lgdomain => $wiki_domain};
+ if ($wiki->login($request)) {
+ Git::credential(\%credential, 'approve');
+ print {*STDERR} qq(Logged in mediawiki user
"$credential{username}".\n);
+ } else {
+ print {*STDERR} qq(Failed to log in mediawiki user
"$credential{username}" on ${remote_url}\n);
+ print {*STDERR} ' (error ' .
+ $wiki->{error}->{code} . ': ' .
+ $wiki->{error}->{details} . ")\n";
+ Git::credential(\%credential, 'reject');
+ exit 1;
+ }
+ }
+
+ return $wiki;
}
1; # Famous last words
\ No newline at end of file
diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl
b/contrib/mw-to-git/git-remote-mediawiki.perl
index be17e89..d679035 100755
--- a/contrib/mw-to-git/git-remote-mediawiki.perl
+++ b/contrib/mw-to-git/git-remote-mediawiki.perl
@@ -13,6 +13,8 @@
use strict;
use MediaWiki::API;
+use Git;
+use GitMediawiki qw(clean_filename smudge_filename connect_maybe);
use DateTime::Format::ISO8601;
# By default, use UTF-8 to communicate with Git and the user
@@ -24,9 +26,6 @@ use IPC::Open2;
use warnings;
-# Mediawiki filenames can contain forward slashes. This variable decides by
which pattern they should be replaced
-use constant SLASH_REPLACEMENT => "%2F";
-
# It's not always possible to delete pages (may require some
# privileges). Deleted pages are replaced with this content.
use constant DELETED_CONTENT => "[[Category:Deleted]]\n";
@@ -159,36 +158,6 @@ while () {
# MediaWiki API instance, created lazily.
my $mediawiki;
-sub mw_connect_maybe {
- if ($mediawiki) {
- return;
- }
-
[PATCH/RFC V2 0/4] git-remote-mediawiki: new tool to preview local changes without pushing
From: Benoit Person The #7 issue on git-mediawiki's issue tracker [1] states that the ability to preview content without pushing would be a nice thing to have. This serie is a second attempt to achieve it: - It adds a new GitMediawiki.pm package to share code between the new tool and `git-remote-mediawiki.perl`. (PATCH 1 & 2) - It creates a new "meta"-command `git mw` with subcommand handling (PATCH 3) - It adds a new subcommand named `preview` to `git mw` (PATCH 4) changes from the V0: - add new package GitMediawiki - move some of git-remote-mediawiki functions into the package - update git-remote-mediawiki to use those "moved" functions - add a hacky-way to install it in the Makefile - use it in the new git mw tool - add a way to give to the preview tool blobs as argument - add a fallback when the upstream's branch remote is not a mediawiki remote - update the `autoload` option to use `git web--browse` and not `xdg-open` - update the way we find the upstream's branch remote name For now, this PATCH/RFC is based on the 'next' branch merged with the bp/mediawiki-credential patch. For the final version, I will try to rebase it on celestin's work with perlcritic. [1] https://github.com/moy/Git-Mediawiki/issues/7 Benoit Person (4): git-mw: Introduction of GitMediawiki.pm git-mw: Moving some functions from git-remote-mediawiki.perl to GitMediawiki.pm git-mw: Adding git-mw.perl script git-mw: Adding preview tool in git-mw.perl contrib/mw-to-git/GitMediawiki.pm | 94 +++ contrib/mw-to-git/Makefile | 22 ++- contrib/mw-to-git/git-mw.perl | 247 contrib/mw-to-git/git-remote-mediawiki.perl | 80 ++--- 4 files changed, 373 insertions(+), 70 deletions(-) create mode 100644 contrib/mw-to-git/GitMediawiki.pm create mode 100644 contrib/mw-to-git/git-mw.perl -- 1.8.3.GIT -- 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 jx/clean-interactive] t0060: skip a few relative_path tests on Windows
2013/6/13 Johannes Sixt : > From: Johannes Sixt > > The bash on Windows rewrites paths that look like absolute POSIX paths > when they are a command-line argument of a regular Windows program, such > as git and the test helpers. As a consequence, the actual tests performed > are not what the tests scripts expect. > > The tests that need *not* be skipped are those where the two paths passed > to 'test-path-utils relative_path' have the same prefix and the result is > expected to be a relative path. This is because the rewriting changes > "/a/b" to "D:/Src/MSysGit/a/b", and when both inputs are extended the same > way, this just cancels out in the relative path computation. > > Signed-off-by: Johannes Sixt > --- > t/t0060-path-utils.sh | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > Thank you for pointing out this cross-platform issue. I test your patch on Mac OS X, but not test it on msys yet. Since this issue is from the very first commit of this series and this series are still in pu, so I move your patch as patch 02/16 of this series. You can get the update series in my clone on GitHub: $ git remote add jiangxin git://github.com/jiangxin/git.git $ git fetch jiangxin jx/clean-interactive $ git log --oneline -16 jiangxin/jx/clean-interactive c646c test: add t7301 for git-clean--interactive 92d4a git-clean: add documentation for interactive git-clean 22e3d git-clean: add ask each interactive action 5fcb8 git-clean: add select by numbers interactive action 77ef8e git-clean: add filter by pattern interactive action 02327 git-clean: use a git-add-interactive compatible UI 2322 git-clean: add colors to interactive git-clean 06fad git-clean: show items of del_list in columns 6ae8b git-clean: add support for -i/--interactive 1eeb5 git-clean: refactor git-clean into two phases 3f903 Refactor write_name_quoted_relative, remove unused params 8ccdf Refactor quote_path_relative, remove unused params 13da5e quote.c: remove path_relative, use relative_path instead 1208ee path.c: refactor relative_path(), not only strip prefix 95b06 t0060: skip a few relative_path tests on Windows 22247 test: add test cases for relative_path And commit 95b06 may squash to previous commit 22247. -- Jiang Xin -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] show-branch: use pager
Øystein Walle writes: > This is for consistency with other porcelain commands such as 'log'. I don't think consistency with other porcelain is a sufficient argument. Many commands purposely don't use the pager by default because they will normally have a short output. Users can already set "pager.show-branch" to get the behavior you introduce in the patch. The question is more: will users prefer having the pager by default for this particular command? I don't use show-branch enough to answer by myself, but probably the answer is yes. (This is not an objection, just to make sure you have all the elements) -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
slow process of post-receive script on a remote (samba) share
Dear list, I am using Git bash from version 1.8.3.msysgit.0, on a Windows 7x64 PC. I have an issue with executing git push if I have a post-receive script configured. The content of the script is not really important, as if I have a script that contains only commented out lines (around 70 lines), my git push command is delayed with around 5 seconds. I`ve tested the script on another PC and it is working fine. No delay at all. So there are some issues on my PC regarding how git processes remote scripts. I took a wireshark trace with 2 scenarios on my PC: 1. just execute `cat \post-receive` command in the git bash 2. did a `real` git push Results of the wireshark traces shows: 1. Read AndX Request, FID: 0x228f, 1024 bytes at offset 0 (1024 bytes at time, always) 2. Read AndX Request, FID: 0x21c9, 1 byte at offset 0 (1 byte, always) Conclusion: git push command reads the post-receive script in 1 byte chunks, which dramatically slows down the execution process. If more information is required about my setup or configuration, I will provided it happily, but I think this 1 byte read is the main reason for the issue. Has anyone seen something similar? Or have any clues what is going on? Thanks, tamas ps: this is my 3rd (or 4th) attempt to post on the mailing list, using a different email account this time. If any of the previous attempt also arrived, then sorry for the double posts. -- 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 2/4] push: make upstream, simple work with pushdefault
Junio C Hamano wrote:
> If you recall the earlier discussion on "@{publish} which is
> different from @{upstream}", one idea to allow mapping on the push
> end was to introduce "push.default = single" that would act as
> "upstream" when in "branch I fetch and integrate with is the same
> branch at the same repository the one I want to update with my
> result" workflow, and in a triangular workflow maps the branch being
> pushed using remote.$name.push refspecs (if exists).
I'm still resisting this idea, because I don't like these special-case
push.default modes. If possible, I want to avoid introducing another
one to the existing mess.
> [...]
Okay, so what you're saying makes sense. I'm cooking the following idea:
- current: push "$(HEAD)". No restrictions on destination. The most
generic, sensible, and extensible one, in my opinion.
- matching: push ":" to the destination specified by the current
branch. [since I cannot know what I'm pushing in advance, I think this
is generally ugly]
- upstream: In the special case when fetch source is equal to push
destination, push "$(HEAD):$(branch.$(HEAD).merge)". Otherwise,
fallback to current. Useful in central workflows.
- simple: [still haven't thought about what to do with this; I'm
generally not in favor of artificially crippling functionality by
erroring out]
Just like upstream respects branch..merge, current respects
branch..push, making branch-level ref mapping in triangular
workflows possible. Finally, remote..push is entirely
orthogonal to all this, and is respected no matter what.
Am I making any sense?
--
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 jx/clean-interactive] t0060: skip a few relative_path tests on Windows
From: Johannes Sixt The bash on Windows rewrites paths that look like absolute POSIX paths when they are a command-line argument of a regular Windows program, such as git and the test helpers. As a consequence, the actual tests performed are not what the tests scripts expect. The tests that need *not* be skipped are those where the two paths passed to 'test-path-utils relative_path' have the same prefix and the result is expected to be a relative path. This is because the rewriting changes "/a/b" to "D:/Src/MSysGit/a/b", and when both inputs are extended the same way, this just cancels out in the relative path computation. Signed-off-by: Johannes Sixt --- t/t0060-path-utils.sh | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh index dfe4747..4deec52 100755 --- a/t/t0060-path-utils.sh +++ b/t/t0060-path-utils.sh @@ -190,15 +190,15 @@ test_expect_success SYMLINKS 'real path works on symlinks' ' relative_path /a/b/c/ /a/b/ c/ relative_path /a/b/c/ /a/bc/ -relative_path /a//b//c///a/b// c/ +relative_path /a//b//c///a/b// c/ POSIX relative_path /a/b /a/b./ relative_path /a/b//a/b./ relative_path /a /a/b../ relative_path //a/b/ ../../ relative_path /a/c /a/b/ ../c relative_path /a/c /a/b../c -relative_path /a/b "" /a/b -relative_path /a/b ""/a/b +relative_path /a/b "" /a/bPOSIX +relative_path /a/b ""/a/bPOSIX relative_path ""/a/b./ relative_path """" ./ relative_path """"./ -- 1.8.3.1.1670.g1dbc49e -- 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 4/4] resolve_ref_unsafe(): close race condition reading loose refs
Michael Haggerty writes:
> One race is still possible and undetected: another process could
> change the file from a regular file into a symlink between the call to
> lstat and the call to open(). The open() call would silently follow
> the symlink and not know that something is wrong. I don't see a way
> to detect this situation without the use of the O_NOFOLLOW option,
> which is not portable and is not used elsewhere in our code base.
>
> However, we don't use symlinks anymore, so this situation is unlikely.
> And it doesn't appear that treating a symlink as a regular file would
> have grave consequences; after all, this is exactly how the code
> handles non-relative symlinks.
You could fstat() the fd you got from open(), and verify that it is
still the same inode/device. That's wasting one syscall per ref for
pretty much everyone, but perhaps if we really cared about this (and I
gather from the above that we don't), we could conditionally use
O_NOFOLLOW if available, otherwise do that fstat().
--
Thomas Rast
trast@{inf,student}.ethz.ch
--
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] show-branch: use pager
This is for consistency with other porcelain commands such as 'log'.
Signed-off-by: Øystein Walle
---
Hi, Jeff
Thanks for the (fast!) feedback and good to hear it won't cause any trouble.
I hadn't actually noticed this mechanism of setting up the pager before now but
I fully agree. Here's an updated version.
Øsse
git.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/git.c b/git.c
index 4359086..6d1f6ca 100644
--- a/git.c
+++ b/git.c
@@ -406,7 +406,7 @@ static void handle_internal_command(int argc, const char
**argv)
{ "send-pack", cmd_send_pack, RUN_SETUP },
{ "shortlog", cmd_shortlog, RUN_SETUP_GENTLY | USE_PAGER },
{ "show", cmd_show, RUN_SETUP },
- { "show-branch", cmd_show_branch, RUN_SETUP },
+ { "show-branch", cmd_show_branch, RUN_SETUP | USE_PAGER },
{ "show-ref", cmd_show_ref, RUN_SETUP },
{ "stage", cmd_add, RUN_SETUP | NEED_WORK_TREE },
{ "status", cmd_status, RUN_SETUP | NEED_WORK_TREE },
--
1.8.2.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
Re: Re: Is Git multithreaded ?
Yes it does help ! Thanks a lot Peff for your very complete answer. Cheers, Laurent. Le 12/06/2013 21:38, Jeff King a écrit : I hope that helps. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] t/checkout-last: checkout - doesn't work after rebase -i
Junio C Hamano wrote:
> These four are all valid ways to spell the "rebase -i master" step.
>
> and I think it is sensible to expect
>
> (1) they all behave the same way; or
Yes. My reasoning is very simple: a rebase is a rebase; it should not
write "checkout: " messages to the reflog. Therefore, the @{-}
will ignore it; for the purposes of checkout -, the rebase event is
inconsequential.
--
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 RFC] show-branch: use pager
On Thu, Jun 13, 2013 at 08:43:31AM +0200, Øystein Walle wrote: > This is for consistency with other porcelain commands such as 'log'. I do not use show-branch myself, but being consistent with the other porcelain commands makes sense to me. > I marked this as an RFC because of Jeff King's comments in > daa0c3d97 where I got the impression this this might not be a good idea. > However I haven't found any bugs and all the tests pass. It is more a huble > suggestion than anything but I thought I might as well send it as a patch. I don't think the problems described in daa0c3d97 should be an issue for us, as the purpose of that commit was to delay the color decision until the last minute. That helps commands which load color config before having decided on whether to use a pager. In other words, it covers the exact situation you introduce here: > diff --git a/builtin/show-branch.c b/builtin/show-branch.c > index 90fc6b1..bd3e10c 100644 > --- a/builtin/show-branch.c > +++ b/builtin/show-branch.c > @@ -683,6 +683,7 @@ int cmd_show_branch(int ac, const char **av, const char > *prefix) > }; > > git_config(git_show_branch_config, NULL); > + setup_pager(); So I think your patch is fine with respect to those problems. However, I do not see any need for show_branch to delay its pager setup at all. Commands like "git diff" and "git log" must do so, because they do not know whether they want a pager or not until after parsing command-line arguments. But in this case we are always starting the pager. Would it make more sense to just set the USE_PAGER flag in the "show-branch" entry in git.c (see the "shortlog" entry for an example)? -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
