[PATCH] rebase -i: test "Nothing to do" case with autostash
Signed-off-by: Matthieu Moy --- Ram's patch lacks a test. Here it is. Fails without Ram's patch, and passes with it. Can be squashed into Ram's patch. t/t3420-rebase-autostash.sh | 17 + 1 file changed, 17 insertions(+) diff --git a/t/t3420-rebase-autostash.sh b/t/t3420-rebase-autostash.sh index 90eb264..c2e9a4c 100755 --- a/t/t3420-rebase-autostash.sh +++ b/t/t3420-rebase-autostash.sh @@ -167,4 +167,21 @@ testrebase "" .git/rebase-apply testrebase " --merge" .git/rebase-merge testrebase " --interactive" .git/rebase-merge +test_expect_success 'Abort rebase with --autostash' ' + git log && + echo new-content >file0 && + ( + write_script abort-editor.sh <<-\EOF && + echo > "$1" + EOF + GIT_EDITOR=\"$(pwd)/abort-editor.sh\" && + export GIT_EDITOR && + test_must_fail git rebase -i --autostash HEAD^ && + rm -f abort-editor.sh + ) && + git status && + echo new-content >expected && + test_cmp expected file0 +' + test_done -- 2.0.0.rc3.499.gd6dc9ad -- 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: format-patch crashes with a huge patchset
On Mon, May 19, 2014 at 10:35:56PM +0300, Michael S. Tsirkin wrote:
> I tried to fump the whole history of qemu with format-patch.
> It crashes both with v2.0.0-rc2-21-g6087111
> and with git 1.8.3.1:
>
> ~/opt/libexec/git-core/git-format-patch --follow -o patches/all
> e63c3dc74bfb90e4522d075d0d5a7600c5145745..
You can't use "--follow" without specifying a single pathspec. Running
"git log --follow" notices and blocks this, but format-patch doesn't
(and segfaults later when the follow code assumes there is a pathspec).
I think we need:
-- >8 --
Subject: move "--follow needs one pathspec" rule to diff_setup_done
Because of the way "--follow" is implemented, we must have
exactly one pathspec. "git log" enforces this restriction,
but other users of the revision traversal code do not. For
example, "git format-patch --follow" will segfault during
try_to_follow_renames, as we have no pathspecs at all.
We can push this check down into diff_setup_done, which is
probably a better place anyway. It is the diff code that
introduces this restriction, so other parts of the code
should not need to care themselves.
Reported-by: "Michael S. Tsirkin"
Signed-off-by: Jeff King
---
I didn't include a test, as I don't think the current behavior is what
we want in the long run. That is, it would be nice if eventually
--follow learned to be more flexible. Any test for this would
necessarily be looking for the opposite of that behavior. But maybe it's
worth it to just have in the meantime, and anyone who works on --follow
can rip out the test.
builtin/log.c | 8 ++--
diff.c| 3 +++
2 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/builtin/log.c b/builtin/log.c
index 39e8836..3b6a6bb 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -158,13 +158,9 @@ static void cmd_log_init_finish(int argc, const char
**argv, const char *prefix,
if (rev->show_notes)
init_display_notes(&rev->notes_opt);
- if (rev->diffopt.pickaxe || rev->diffopt.filter)
+ if (rev->diffopt.pickaxe || rev->diffopt.filter ||
+ DIFF_OPT_TST(&rev->diffopt, FOLLOW_RENAMES))
rev->always_show_header = 0;
- if (DIFF_OPT_TST(&rev->diffopt, FOLLOW_RENAMES)) {
- rev->always_show_header = 0;
- if (rev->diffopt.pathspec.nr != 1)
- usage("git logs can only follow renames on one pathname
at a time");
- }
if (source)
rev->show_source = 1;
diff --git a/diff.c b/diff.c
index f72769a..68bb8c5 100644
--- a/diff.c
+++ b/diff.c
@@ -3325,6 +3325,9 @@ void diff_setup_done(struct diff_options *options)
}
options->diff_path_counter = 0;
+
+ if (DIFF_OPT_TST(options, FOLLOW_RENAMES) && options->pathspec.nr != 1)
+ die(_("--follow requires exactly one pathspec"));
}
static int opt_arg(const char *arg, int arg_short, const char *arg_long, int
*val)
--
2.0.0.rc1.436.g03cb729
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 2/5] t4041, t4205, t6006, t7102: Don't hardcode tested encoding value
On Tue, May 20, 2014 at 01:49:31AM +, brian m. carlson wrote: > On Mon, May 19, 2014 at 07:28:17PM +0400, Alexey Shumkin wrote: > > The tested encoding is always available in a variable. Use it instead of > > hardcoding. Also, to be in line with other tests use ISO8859-1 > > (uppercase) rather then iso8895-1. > > You wrote "iso8895" when I think you meant "iso8859". Oops! Yes, you're right, I've meant iso8859. > > -- > brian m. carlson / brian with sandals: Houston, Texas, US > +1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only > OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187 -- Alexey Shumkin E-mail: alex.crez...@gmail.com ICQ: 118001447 Jabber (GoogleTalk): alex.crez...@gmail.com Skype: crezoff -- 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] format-patch --signature-file
On Mon, May 19, 2014 at 10:46:21PM -0700, Jeremiah Mahler wrote: > > Avoiding that is easy with an indirection, no? Something like this > > at the top: > > > > static const char *the_default_signature = git_version_string; > > static const char *signature = the_default_signature; > > > > and comparing to see if signature points at the same address as > > the_default_signature would give you what you want, I think. > > I like your suggestion for a default signature variable. > Unfortunately, C doesn't like it as much. > > static const char *the_default_signature = git_version_string; > static const char *signature = the_default_signature; // ERROR > // initializer element is not constant You could do: #define DEFAULT_SIGNATURE git_version_string static const char *signature = DEFAULT_SIGNATURE; ... if (signature == DEFAULT_SIGNATURE) ... but maybe that is getting a little unwieldy, considering the scope of the original problem. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] format-patch --signature-file
On Mon, May 19, 2014 at 09:54:33AM -0700, Junio C Hamano wrote: > Jeremiah Mahler writes: > > > On Sat, May 17, 2014 at 06:00:14AM -0400, Jeff King wrote: > >> > > Avoiding that is easy with an indirection, no? Something like this > at the top: > > static const char *the_default_signature = git_version_string; > static const char *signature = the_default_signature; > > and comparing to see if signature points at the same address as > the_default_signature would give you what you want, I think. I like your suggestion for a default signature variable. Unfortunately, C doesn't like it as much. static const char *the_default_signature = git_version_string; static const char *signature = the_default_signature; // ERROR // initializer element is not constant -- Jeremiah Mahler jmmah...@gmail.com http://github.com/jmahler -- 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] remote-helpers: point at their upstream repositories
Felipe Contreras writes: > Junio C Hamano wrote: >> Junio C Hamano writes: >> >> > Felipe Contreras writes: >> > >> >> We could. Personally I don't see the point of making the warning any >> >> more annoying >> >> If we were giving the users a choice of "no thanks, I'll keep using >> the obsolete one", then trying to be a low key and giving them a way >> to squelch with an advice.* config might make sense, but if we plan >> to remove/stub at as early as v2.1, I think annoyance is very much >> what we want, actually, because it clearly is the case that we do >> prefer users switching instead of waiting for v2.1. >> >> How does this sound? > > The patch below assumes the user has ~/bin in his PATH, which might not > be the case. Personally I don't see the point of creating extra > annoyance with instructions that might not work. Yeah, I would be lying if I said that "that might not work" did not bother me, but I decided it would be on the good side of the borderline (it is better to be concise and slightly wrong than ultra-verbose and precise). As you may have guessed, they were stolen from your earlier message that shows what the site says after all ;-) I will probably tag -rc4 with the patch applied sometime tomorrow. -- 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] RelNotes/2.0.0.txt: Fix several grammar issues, notably a lack of hyphens, double quotes, or articles
Richard Hansen writes: > On 2014-05-19 19:46, Junio C Hamano wrote: >> "Jason St. John" writes: >>> @@ -53,7 +53,7 @@ Updates since v1.9 series >>> UI, Workflows & Features >>> >>> * The "multi-mail" post-receive hook (in contrib/) has been updated >>> - to a more recent version from the upstream. >>> + to a more recent version from upstream. >> >> Hmph, I have only one multi-mail upstream; shouldn't I call it "the" >> upstream from my point of view? > > Plain "upstream" (without "the") is correct because it's an adverb, not > a noun. (Alternatively, this could be written "from the upstream > repository" or "from the upstream project".) OK; I was trying to use "upstream" as a noun. > >>> @@ -217,7 +218,7 @@ notes for details). >>> * "git diff --no-index -Mq a b" fell into an infinite loop. >>> (merge ad1c3fb jc/fix-diff-no-index-diff-opt-parse later to maint). >>> >>> - * "git fetch --prune", when the right-hand-side of multiple fetch >>> + * "git fetch --prune", when the right-hand side of multiple fetch >>> refspecs overlap (e.g. storing "refs/heads/*" to >> >> Hmph, I read this as a "right-hand", a multi-word adjective, is used >> to describe one "side" (the other side being the "left-hand side"). >> Otherwise, you would be writing command-line-option, no? > > Are you reading the diff backwards? (The second hyphen is being > removed, not added.) Yes. 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] RelNotes/2.0.0.txt: Fix several grammar issues, notably a lack of hyphens, double quotes, or articles
On 2014-05-19 19:46, Junio C Hamano wrote: > "Jason St. John" writes: >> @@ -53,7 +53,7 @@ Updates since v1.9 series >> UI, Workflows & Features >> >> * The "multi-mail" post-receive hook (in contrib/) has been updated >> - to a more recent version from the upstream. >> + to a more recent version from upstream. > > Hmph, I have only one multi-mail upstream; shouldn't I call it "the" > upstream from my point of view? Plain "upstream" (without "the") is correct because it's an adverb, not a noun. (Alternatively, this could be written "from the upstream repository" or "from the upstream project".) >> @@ -217,7 +218,7 @@ notes for details). >> * "git diff --no-index -Mq a b" fell into an infinite loop. >> (merge ad1c3fb jc/fix-diff-no-index-diff-opt-parse later to maint). >> >> - * "git fetch --prune", when the right-hand-side of multiple fetch >> + * "git fetch --prune", when the right-hand side of multiple fetch >> refspecs overlap (e.g. storing "refs/heads/*" to > > Hmph, I read this as a "right-hand", a multi-word adjective, is used > to describe one "side" (the other side being the "left-hand side"). > Otherwise, you would be writing command-line-option, no? Are you reading the diff backwards? (The second hyphen is being removed, not added.) -Richard -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] remote-helpers: point at their upstream repositories
Junio C Hamano wrote: > Junio C Hamano writes: > > > Felipe Contreras writes: > > > >> We could. Personally I don't see the point of making the warning any > >> more annoying > > If we were giving the users a choice of "no thanks, I'll keep using > the obsolete one", then trying to be a low key and giving them a way > to squelch with an advice.* config might make sense, but if we plan > to remove/stub at as early as v2.1, I think annoyance is very much > what we want, actually, because it clearly is the case that we do > prefer users switching instead of waiting for v2.1. > > How does this sound? The patch below assumes the user has ~/bin in his PATH, which might not be the case. Personally I don't see the point of creating extra annoyance with instructions that might not work. -- 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 v3 2/5] t4041, t4205, t6006, t7102: Don't hardcode tested encoding value
On Mon, May 19, 2014 at 07:28:17PM +0400, Alexey Shumkin wrote: > The tested encoding is always available in a variable. Use it instead of > hardcoding. Also, to be in line with other tests use ISO8859-1 > (uppercase) rather then iso8895-1. You wrote "iso8895" when I think you meant "iso8859". -- brian m. carlson / brian with sandals: Houston, Texas, US +1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187 signature.asc Description: Digital signature
Re: [PATCH] RelNotes/2.0.0.txt: Fix several grammar issues, notably a lack of hyphens, double quotes, or articles
"Jason St. John" writes: > Signed-off-by: Jason St. John > --- Please accept "Thanks" for all the hunks I won't comment below; they looked correct to me. > @@ -53,7 +53,7 @@ Updates since v1.9 series > UI, Workflows & Features > > * The "multi-mail" post-receive hook (in contrib/) has been updated > - to a more recent version from the upstream. > + to a more recent version from upstream. Hmph, I have only one multi-mail upstream; shouldn't I call it "the" upstream from my point of view? > @@ -63,12 +63,13 @@ UI, Workflows & Features > single strand-of-pearls is broken in its output. > > * The "rev-parse --parseopt" mechanism used by scripted Porcelains to > - parse command line options and to give help text learned to take > + parse command-line options and to give help text learned to take We seem to have 30 "command line option" in Documentation/ vs 16 "command-line option", but this is two words used as an adjective, and it may read better with the dash in between. Thanks. > @@ -190,20 +191,20 @@ notes for details). > > * The remote-helper interface to fast-import/fast-export via the > transport-helper has been tightened to avoid leaving the import > - marks file from a failed/crashed run, as such a file that is out of > - sync with the reality confuses a later invocation of itself. > + marks file from a failed/crashed run, as such a file that is out-of- > + sync with reality confuses a later invocation of itself. Likewise, but I somehow think this is borderline; the spelling without hyphen is used frequently enough. > @@ -217,7 +218,7 @@ notes for details). > * "git diff --no-index -Mq a b" fell into an infinite loop. > (merge ad1c3fb jc/fix-diff-no-index-diff-opt-parse later to maint). > > - * "git fetch --prune", when the right-hand-side of multiple fetch > + * "git fetch --prune", when the right-hand side of multiple fetch > refspecs overlap (e.g. storing "refs/heads/*" to Hmph, I read this as a "right-hand", a multi-word adjective, is used to describe one "side" (the other side being the "left-hand side"). Otherwise, you would be writing command-line-option, no? > - * Codepaths that parse timestamps in commit objects have been > + * Code paths that parse timestamps in commit objects have been > tightened. > (merge f80d1f9 jk/commit-dates-parsing-fix later to maint). We seem to spell this as one-word in the documentation too often, but "Code path" is the right form, I agree. -- 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/technical/api-hashmap: Remove source highlighting
On Mon, 19 May 2014, Junio C Hamano wrote: > If Ubuntu does not want to use highlight, it can apply a change like the > patch in question as part of their fork to make the end result > consistent and they are failing to do so. Sure, Ubuntu can apply that patch, but the larger problem remains: if the Ubuntu packaging is even slightly different from Debian’s, it is no longer eligible for automatic synchronization from Debian. When that has happened in the past, the result was that the Ubuntu version languished far behind the Debian version, because Canonical doesn’t have the manpower to manually merge every Debian update. Ubuntu 9.04 shipped with Git 1.6.0.4 instead of 1.6.2.4 because they had failed to update their packaging after patching out a dependency on cvsps. If this Ubuntu nonsense were obstructing something important upstream, then of course I would be yelling at Ubuntu to get their act together; in that case, I filed a main inclusion report to get Canonical to officially support cvsps (https://bugs.launchpad.net/bugs/369111) so Ubuntu could drop their patch. But here we’re talking about syntax highlighting in one page of internal documentation that basically nobody is going to read, and that argument wouldn’t carry any weight. We could put the patch into Debian instead of Ubuntu to eliminate the Ubuntu delta; Jonathan has been friendly enough to Ubuntu that I think he would grudgingly agree. But I’m submitting it upstream because other distros will eventually run into the same problem: there’s no obvious cue that source-highlight needs to be added as a new dependency besides a warning message buried in the middle of the build log. > How bad does the documentation look with the patch applied (I know how > bad it looks without source-highlight installed)? If it is not too bad, > then it sounds like a sensible solution to drop the highlight markup > unconditionally like the patch that started this thread does, taking the > "common denominator" approach. You seem to agree, and I do not object, > either. Original version with syntax-highlight installed (pretty): http://web.mit.edu/andersk/Public/api-hashmap/old-highlight.html Original version with syntax-highlight missing (corrupted): http://web.mit.edu/andersk/Public/api-hashmap/old-no-highlight.html Patched version (boring but readable): http://web.mit.edu/andersk/Public/api-hashmap/patched.html Anders -- 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 07/31] refs.c: add a flag to allow reflog updates to truncate the log
On Fri, May 16, 2014 at 2:20 PM, Junio C Hamano wrote:
> Ronnie Sahlberg writes:
>
>> Add a flag that allows us to truncate the reflog before we write the update.
>>
>> Signed-off-by: Ronnie Sahlberg
>> ---
>
> Until we read the callers it is hard to see how such a feature is
> useful, though.
Shortly later in the series comes the update to builtin/reflog.c which
uses this feature.
Even later in the series I also use this in builtin/reflog.c to both
do the ref update as well as the corresponding reflog updates all in
one single transaction.
>
> (style) The two multi-line comments are formatted differently.
Fixed. Thanks.
>
>> refs.c | 12 ++--
>> refs.h | 4 +++-
>> 2 files changed, 13 insertions(+), 3 deletions(-)
>>
>> diff --git a/refs.c b/refs.c
>> index d8a1568..a8b583a 100644
>> --- a/refs.c
>> +++ b/refs.c
>> @@ -3608,7 +3608,11 @@ int transaction_commit(struct ref_transaction
>> *transaction,
>> }
>> }
>>
>> - /* Update all reflog files */
>> + /* Update all reflog files
>> +We have already done all ref updates and deletes.
>> +There is not much we can do here if there are any reflog
>> +update errors, other than complain.
>> + */
>> for (i = 0; i < n; i++) {
>> struct ref_update *update = updates[i];
>>
>> @@ -3616,7 +3620,11 @@ int transaction_commit(struct ref_transaction
>> *transaction,
>> continue;
>> if (update->reflog_fd == -1)
>> continue;
>> -
>> + if (update->flags & REFLOG_TRUNCATE)
>> + if (lseek(update->reflog_fd, 0, SEEK_SET) < 0 ||
>> + ftruncate(update->reflog_fd, 0)) {
>> + error("Could not truncate reflog");
>> + }
>
> The {} looks somewhat curious here. If you seeked to the beginning,
> and failed to truncate, do you still want to go on without "return"
> in front of the error()? Would that overwrite existing entries?
I have changed it to do this :
+ if (update->flags & REFLOG_TRUNCATE)
+ if (lseek(update->reflog_fd, 0, SEEK_SET) < 0 ||
+ ftruncate(update->reflog_fd, 0)) {
+ error("Could not truncate reflog: %s",
+ update->refname);
+ rollback_lock_file(&update->reflog_lock);
+ update->reflog_fd = -1;
+ }
At this point we have already committed all the changes to refs and
also performed all unlinks for refs we are deleting. So if things
start to go wrong here during the reflog updates it is too late to do
much more than complain.
>
>> if (log_ref_write_fd(update->reflog_fd, update->old_sha1,
>>update->new_sha1,
>>update->committer, update->msg)) {
>> diff --git a/refs.h b/refs.h
>> index 2e22a26..b1b97e7 100644
>> --- a/refs.h
>> +++ b/refs.h
>> @@ -269,8 +269,10 @@ int transaction_delete_sha1(struct ref_transaction
>> *transaction,
>> const unsigned char *old_sha1,
>> int flags, int have_old, const char *msg);
>>
>> +#define REFLOG_TRUNCATE 0x01
>> /*
>> - * Append a reflog entry for refname.
>> + * Append a reflog entry for refname. If the REFLOG_TRUNCATE flag is set
>> + * this update will first truncate the reflog before writing the entry.
>> */
>> int transaction_update_reflog(struct ref_transaction *transaction,
>> const char *refname,
--
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 03/31] refs.c: rename the transaction functions
Ronnie Sahlberg writes:
> I am not sure if we need transactions for other types of data, such as
> sha1 objects, but if it turns out we do in the future we can rename
> these functions again.
I was wrong (and I think you read it in the later patch review).
If we need transaction for other types of data, and we will
eventually need to coordinate the transaction semantics over refs
and those other types of data. It would be far cleaner to express
that coordination within the same transaction framework.
In other words, we do not want to be in a situation like this:
other_transaction_begin();
ref_transaction_begin();
ref_transaction_update();
other_transaction_update();
ref_transaction_commit();
if (other_transaction_commit() != SUCCESS) {
... oops it is too late to roll back the ref_transaction ...
other_transaction_rollback();
}
and force us doing 3-phase commit inside ourselves to work it
around.
--
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] remote-helpers: point at their upstream repositories
Junio C Hamano writes:
> Felipe Contreras writes:
>
>> We could. Personally I don't see the point of making the warning any
>> more annoying
If we were giving the users a choice of "no thanks, I'll keep using
the obsolete one", then trying to be a low key and giving them a way
to squelch with an advice.* config might make sense, but if we plan
to remove/stub at as early as v2.1, I think annoyance is very much
what we want, actually, because it clearly is the case that we do
prefer users switching instead of waiting for v2.1.
How does this sound?
-- >8 --
From: Junio C Hamano
Date: Mon, 19 May 2014 16:05:34 -0700
Subject: [PATCH] remote-helpers: give short instructions to download the
latest
Signed-off-by: Junio C Hamano
---
contrib/remote-helpers/git-remote-bzr | 6 ++
contrib/remote-helpers/git-remote-hg | 6 ++
2 files changed, 12 insertions(+)
diff --git a/contrib/remote-helpers/git-remote-bzr
b/contrib/remote-helpers/git-remote-bzr
index be4b9a3..c0cb652 100755
--- a/contrib/remote-helpers/git-remote-bzr
+++ b/contrib/remote-helpers/git-remote-bzr
@@ -46,6 +46,12 @@ import atexit, shutil, hashlib, urlparse, subprocess
sys.stderr.write('WARNING: git-remote-bzr is now maintained independently.\n')
sys.stderr.write('WARNING: For more information visit
https://github.com/felipec/git-remote-bzr\n')
+sys.stderr.write('''WARNING: You can pick a directory on your $PATH and
download it, e.g.:
+WARNING: $ wget -O $HOME/bin/git-remote-bzr \\
+WARNING:
https://raw.github.com/felipec/git-remote-bzr/master/git-remote-bzr
+WARNING: $ chmod +x $HOME/bin/git-remote-bzr
+''')
+
NAME_RE = re.compile('^([^<>]+)')
AUTHOR_RE = re.compile('^([^<>]+?)? ?[<>]([^<>]*)(?:$|>)')
EMAIL_RE = re.compile(r'([^ \t<>]+@[^ \t<>]+)')
diff --git a/contrib/remote-helpers/git-remote-hg
b/contrib/remote-helpers/git-remote-hg
index 989df66..c07d1a5 100755
--- a/contrib/remote-helpers/git-remote-hg
+++ b/contrib/remote-helpers/git-remote-hg
@@ -28,6 +28,12 @@ import time as ptime
sys.stderr.write('WARNING: git-remote-hg is now maintained independently.\n')
sys.stderr.write('WARNING: For more information visit
https://github.com/felipec/git-remote-hg\n')
+sys.stderr.write('''WARNING: You can pick a directory on your $PATH and
download it, e.g.:
+WARNING: $ wget -O $HOME/bin/git-remote-hg \\
+WARNING: https://raw.github.com/felipec/git-remote-hg/master/git-remote-hg
+WARNING: $ chmod +x $HOME/bin/git-remote-hg
+''')
+
#
# If you want to see Mercurial revisions as Git commit notes:
# git config core.notesRef refs/notes/hg
--
2.0.0-rc3-442-ga28c44b
--
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 03/31] refs.c: rename the transaction functions
On Fri, May 16, 2014 at 2:15 PM, Junio C Hamano wrote: > Ronnie Sahlberg writes: > >> Rename the transaction functions. Remove the leading ref_ from the names >> and append _sha1 to the names for functions that create/delete/update sha1 >> refs. >> ... >> - transaction = ref_transaction_begin(); >> + transaction = transaction_begin(); > > Why? Do we forsee that there will never be other kinds of > transaction, and whenever we say a "transaction" that will be always > about updating the refs and nothing else? ref_transaction_... is a bit of a mouthful. I think initially we will only need a transaction framework for normal sha1 refs, for symrefs and for reflog updates and these should all be handled by the same transaction system so a single _begin/_update*/_commit should be able to atomically commit changes that involve updates to all three types. I am not sure if we need transactions for other types of data, such as sha1 objects, but if it turns out we do in the future we can rename these functions again. -- 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 09/31] refs.c: allow multiple reflog updates during a single transaction
On Fri, May 16, 2014 at 2:35 PM, Junio C Hamano wrote:
> Ronnie Sahlberg writes:
>
>> Allow to make multiple reflog updates to the same ref during a transaction.
>> This means we only need to lock the reflog once, during the first update that
>> touches the reflog, and that all further updates can just write the reflog
>> entry since the reflog is already locked.
>>
>> This allows us to write code such as:
>>
>> t = transaction_begin()
>> transaction_reflog_update(t, "foo", REFLOG_TRUNCATE, NULL);
>> loop-over-somehting...
>>transaction_reflog_update(t, "foo", 0, );
>> transaction_commit(t)
>
> OK, so you are now doing not just "refs" but also "reflogs", you
> felt that "ref_transaction()" does not cover the latter. Is that
> the reason for the rename in the earlier step?
>
> I am sort-of on the fence.
>
> Calling the begin "ref_transaction_begin" and then calling the new
> function "ref_transaction_log_update" would still allow us to
> differentiate transactions on refs and reflogs, while allowing other
> kinds of transactions that are not related to refs at all to employ
> a mechanism that is different from the one that is used to implement
> the transactions on refs and reflogs you are building here.
>
> But I think I am OK with the generic "transaction-begin" now.
> Having one mechanism for refs and reflogs, and then having another
> completely different mechanism for other things, will not let us
> coordinate between the two easily, so "allow transactions that are
> not related to refs at all to be built on a different mechanism" may
> not be a worthwhile goal to pursue in the first place. Please
> consider the question on the naming in the earlier one dropped.
>
>>
>> where we first truncate the reflog and then build the new content one line
>> at a
>> time.
>>
>> Signed-off-by: Ronnie Sahlberg
>> ---
>> refs.c | 58 +-
>> 1 file changed, 49 insertions(+), 9 deletions(-)
>>
>> diff --git a/refs.c b/refs.c
>> index a3f60ad..e7ede03 100644
>> --- a/refs.c
>> +++ b/refs.c
>> @@ -37,6 +37,10 @@ static inline int bad_ref_char(int ch)
>> * need to lock the loose ref during the transaction.
>> */
>> #define REF_ISPACKONLY 0x0200
>> +/** Only the first reflog update needs to lock the reflog file. Further
>> updates
>> + * just use the lock taken by the first update.
>> + */
>
> Style.
Done.
>
>> @@ -3349,8 +3355,23 @@ int transaction_update_reflog(struct ref_transaction
>> *transaction,
>> int flags)
>> {
>> struct ref_update *update;
>> + int i;
>>
>> update = add_update(transaction, refname, UPDATE_LOG);
>> + update->flags = flags;
>> + for (i = 0; i < transaction->nr - 1; i++) {
>> + if (transaction->updates[i]->update_type != UPDATE_LOG)
>> + continue;
>> + if (!strcmp(transaction->updates[i]->refname,
>> + update->refname)) {
>> + update->flags |= UPDATE_REFLOG_NOLOCK;
>> + update->orig_update = transaction->updates[i];
>> + break;
>> + }
>> + }
>> + if (!(update->flags & UPDATE_REFLOG_NOLOCK))
>> + update->reflog_lock = xcalloc(1, sizeof(struct lock_file));
>
> So with two calls to transaction-update-reflog, we make two calls to
> add-update, and each holds a separate lock? If we write two entries
> to record two updates of the same ref, would we still want to do so?
If we call transaction_update_reflog twice we will call add_update
twice since we are doing two updates.
But if the two calls are for the same ref then we only lock the reflog
for the first update and then in the second update we re-use the lock
structure from the first.
Later updates to reflog.c will show how to loop over the old reflog,
using the existing iterator, and then create a new expired reflog one
entry at a time.
>
>> + /* Rollback any reflog files that are still open */
>> + for (i = 0; i < n; i++) {
>> + struct ref_update *update = updates[i];
>> +
>> + if (update->update_type != UPDATE_LOG)
>> + continue;
>> + if (update->flags & UPDATE_REFLOG_NOLOCK)
>> + continue;
>> + if (update->reflog_fd == -1)
>> + continue;
>> + rollback_lock_file(update->reflog_lock);
>> + }
>> transaction->status = ret ? REF_TRANSACTION_ERROR
>> : REF_TRANSACTION_CLOSED;
--
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 09/31] refs.c: allow multiple reflog updates during a single transaction
On Fri, May 16, 2014 at 3:01 PM, Eric Sunshine wrote:
> On Fri, May 16, 2014 at 5:35 PM, Junio C Hamano wrote:
>> Ronnie Sahlberg writes:
>>
>>> Allow to make multiple reflog updates to the same ref during a transaction.
>>> This means we only need to lock the reflog once, during the first update
>>> that
>>> touches the reflog, and that all further updates can just write the reflog
>>> entry since the reflog is already locked.
>>>
>>> This allows us to write code such as:
>>>
>>> t = transaction_begin()
>>> transaction_reflog_update(t, "foo", REFLOG_TRUNCATE, NULL);
>>> loop-over-somehting...
>>>transaction_reflog_update(t, "foo", 0, );
>>> transaction_commit(t)
>>
>> OK, so you are now doing not just "refs" but also "reflogs", you
>> felt that "ref_transaction()" does not cover the latter. Is that
>> the reason for the rename in the earlier step?
>>
>> I am sort-of on the fence.
>>
>> Calling the begin "ref_transaction_begin" and then calling the new
>> function "ref_transaction_log_update" would still allow us to
>> differentiate transactions on refs and reflogs, while allowing other
>> kinds of transactions that are not related to refs at all to employ
>> a mechanism that is different from the one that is used to implement
>> the transactions on refs and reflogs you are building here.
>>
>> But I think I am OK with the generic "transaction-begin" now.
>> Having one mechanism for refs and reflogs, and then having another
>> completely different mechanism for other things, will not let us
>> coordinate between the two easily, so "allow transactions that are
>> not related to refs at all to be built on a different mechanism" may
>> not be a worthwhile goal to pursue in the first place. Please
>> consider the question on the naming in the earlier one dropped.
>>
>>>
>>> where we first truncate the reflog and then build the new content one line
>>> at a
>>> time.
>>>
>>> Signed-off-by: Ronnie Sahlberg
>>> ---
>>> refs.c | 58 +-
>>> 1 file changed, 49 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/refs.c b/refs.c
>>> index a3f60ad..e7ede03 100644
>>> --- a/refs.c
>>> +++ b/refs.c
>>> @@ -37,6 +37,10 @@ static inline int bad_ref_char(int ch)
>>> * need to lock the loose ref during the transaction.
>>> */
>>> #define REF_ISPACKONLY 0x0200
>>> +/** Only the first reflog update needs to lock the reflog file. Further
>>> updates
>>> + * just use the lock taken by the first update.
>>> + */
>>
>> Style.
>>
>>> @@ -3349,8 +3355,23 @@ int transaction_update_reflog(struct ref_transaction
>>> *transaction,
>>> int flags)
>>> {
>>> struct ref_update *update;
>>> + int i;
>>>
>>> update = add_update(transaction, refname, UPDATE_LOG);
>>> + update->flags = flags;
>>> + for (i = 0; i < transaction->nr - 1; i++) {
>>> + if (transaction->updates[i]->update_type != UPDATE_LOG)
>>> + continue;
>>> + if (!strcmp(transaction->updates[i]->refname,
>>> + update->refname)) {
>>> + update->flags |= UPDATE_REFLOG_NOLOCK;
>>> + update->orig_update = transaction->updates[i];
>>> + break;
>>> + }
>>> + }
>>> + if (!(update->flags & UPDATE_REFLOG_NOLOCK))
>>> + update->reflog_lock = xcalloc(1, sizeof(struct lock_file));
>>
>> So with two calls to transaction-update-reflog, we make two calls to
>> add-update, and each holds a separate lock? If we write two entries
>> to record two updates of the same ref, would we still want to do so?
>
> Also, indent with tabs rather than spaces (the line following the 'if').
Done.
>
>>> + /* Rollback any reflog files that are still open */
>>> + for (i = 0; i < n; i++) {
>>> + struct ref_update *update = updates[i];
>>> +
>>> + if (update->update_type != UPDATE_LOG)
>>> + continue;
>>> + if (update->flags & UPDATE_REFLOG_NOLOCK)
>>> + continue;
>>> + if (update->reflog_fd == -1)
>>> + continue;
>>> + rollback_lock_file(update->reflog_lock);
>>> + }
>>> transaction->status = ret ? REF_TRANSACTION_ERROR
>>> : REF_TRANSACTION_CLOSED;
>
> Indent with tabs.
Done. 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
[PATCH] git-prompt.sh: don't assume the shell expands the value of PS1
Not all shells subject the prompt string to parameter expansion. Test
whether the shell will expand the value of PS1, and use the result to
control whether raw ref names are included directly in PS1.
This fixes a regression introduced in commit 8976500 ("git-prompt.sh:
don't put unsanitized branch names in $PS1"): zsh does not expand PS1
by default, but that commit assumed it did. The bug resulted in
prompts containing the literal string '${__git_ps1_branch_name}'
instead of the actual branch name.
Reported-by: Caleb Thompson
Signed-off-by: Richard Hansen
---
To prevent a regression like this from happening again, I plan on
adding new zsh test cases and expanding the bash test cases (to test
the behavior with 'shopt -u promptvars'). I'd like the zsh tests to
cover the same stuff as the bash tests. These are the steps I am
considering:
1. delete the last test case in t9903 ("prompt - zsh color pc mode")
2. add two new functions to t/lib-bash.sh:
ps1_expansion_enable () { shopt -s promptvars; }
ps1_expansion_disable () { shopt -u promptvars; }
3. loop over the relevant test cases twice: once after calling
ps1_expansion_enable and once after calling ps1_expansion_disable
(with appropriate adjustments to the expected output)
4. move the test cases in t9903 to a separate library file and
source it from t9903-bash-prompt.sh
5. create two new files:
* t/lib-zsh.sh (same as t/lib-bash.sh but tweaked for zsh)
* t/t9904-zsh-prompt.sh (same as t/t9903-bash-prompt.sh but
tweaked for zsh)
Does this approach sound reasonable?
contrib/completion/git-prompt.sh | 56
t/t9903-bash-prompt.sh | 6 ++---
2 files changed, 42 insertions(+), 20 deletions(-)
diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index 853425d..9d684b1 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -209,9 +209,7 @@ __git_ps1_show_upstream ()
if [[ -n "$count" && -n "$name" ]]; then
__git_ps1_upstream_name=$(git rev-parse \
--abbrev-ref "$upstream" 2>/dev/null)
- if [ $pcmode = yes ]; then
- # see the comments around the
- # __git_ps1_branch_name variable below
+ if [ $pcmode = yes ] && [ $ps1_expanded = yes ]; then
p="$p \${__git_ps1_upstream_name}"
else
p="$p ${__git_ps1_upstream_name}"
@@ -308,6 +306,43 @@ __git_ps1 ()
;;
esac
+ # ps1_expanded: This variable is set to 'yes' if the shell
+ # subjects the value of PS1 to parameter expansion:
+ #
+ # * bash does unless the promptvars option is disabled
+ # * zsh does not unless the PROMPT_SUBST option is set
+ # * POSIX shells always do
+ #
+ # If the shell would expand the contents of PS1 when drawing
+ # the prompt, a raw ref name must not be included in PS1.
+ # This protects the user from arbitrary code execution via
+ # specially crafted ref names. For example, a ref named
+ # 'refs/heads/$(IFS=_;cmd=sudo_rm_-rf_/;$cmd)' might cause the
+ # shell to execute 'sudo rm -rf /' when the prompt is drawn.
+ #
+ # Instead, the ref name should be placed in a separate global
+ # variable (in the __git_ps1_* namespace to avoid colliding
+ # with the user's environment) and that variable should be
+ # referenced from PS1. For example:
+ #
+ # __git_ps1_foo=$(do_something_to_get_ref_name)
+ # PS1="...stuff...\${__git_ps1_foo}...stuff..."
+ #
+ # If the shell does not expand the contents of PS1, the raw
+ # ref name must be included in PS1.
+ #
+ # The value of this variable is only relevant when in pcmode.
+ #
+ # Assume that the shell follows the POSIX specification and
+ # expands PS1 unless determined otherwise. (This is more
+ # likely to be correct if the user has a non-bash, non-zsh
+ # shell and safer than the alternative if the assumption is
+ # incorrect.)
+ #
+ local ps1_expanded=yes
+ [ -z "$ZSH_VERSION" ] || [[ -o PROMPT_SUBST ]] || ps1_expanded=no
+ [ -z "$BASH_VERSION" ] || shopt -q promptvars || ps1_expanded=no
+
local repo_info rev_parse_exit_code
repo_info="$(git rev-parse --git-dir --is-inside-git-dir \
--is-bare-repository --is-inside-work-tree \
@@ -457,21 +492,8 @@ __git_ps1 ()
fi
b=${b##refs/heads/}
- if [ $pcmode = yes ]; then
- # In pcmode (and only pcmode) the contents of
- # $gitstring are subject to expansion by the shell.
- # Avoid putting the raw ref name in the prompt to
Re: [PATCH 08/31] refs.c: only write reflog update if msg is non-NULL
On Fri, May 16, 2014 at 2:24 PM, Junio C Hamano wrote:
> Ronnie Sahlberg writes:
>
>> When performing a reflog transaction update, only write to the reflog iff
>> msg is non-NULL. This can then be combined with REFLOG_TRUNCATE to perform
>> an update that only truncates but does not write.
>>
>> Signed-off-by: Ronnie Sahlberg
>> ---
>> refs.c | 8 +---
>> refs.h | 1 +
>> 2 files changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/refs.c b/refs.c
>> index a8b583a..a3f60ad 100644
>> --- a/refs.c
>> +++ b/refs.c
>> @@ -3625,9 +3625,11 @@ int transaction_commit(struct ref_transaction
>> *transaction,
>> ftruncate(update->reflog_fd, 0)) {
>> error("Could not truncate reflog");
>> }
>> - if (log_ref_write_fd(update->reflog_fd, update->old_sha1,
>> - update->new_sha1,
>> - update->committer, update->msg)) {
>> + if (update->msg && log_ref_write_fd(update->reflog_fd,
>> + update->old_sha1,
>> + update->new_sha1,
>> + update->committer,
>> + update->msg)) {
>
> Wouldn't it make it easier to read if you chopped immediately after
> the &&, i.e. chopping at a gap at a higher-level in the parse tree?
Yepp it does.
Changed. Thanks.
>
> if (update->msg &&
> log_ref_write_fd(update->reflog_fd,
> update->old_sha1, update->old_sha1,
> update->committer, update->msg)) {
--
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/4] rebase: test ack
On Mon, May 19, 2014 at 02:34:26PM -0700, Junio C Hamano wrote: > "Michael S. Tsirkin" writes: > > > test ack! handling > > > > Signed-off-by: Michael S. Tsirkin > > Will queue with this squashed in. Thanks! And sorry about the style issues. > 4/4 seems to have some style issues as well, but I didn't look very > closely. I'll try to clean it for the next submission. I'll be glad to hear about them as well. Thanks! > Thanks. > > t/t3415-rebase-autosquash.sh | 12 ++-- > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/t/t3415-rebase-autosquash.sh b/t/t3415-rebase-autosquash.sh > index 9d7db13..dcdba6f 100755 > --- a/t/t3415-rebase-autosquash.sh > +++ b/t/t3415-rebase-autosquash.sh > @@ -75,18 +75,18 @@ test_expect_success 'auto squash (option)' ' > ' > > test_expect_success 'auto ack' ' > - ack="Acked-by: xyz" > - msg=$(test_write_lines "ack! first commit" "" "$ack") > + ack="Acked-by: xyz" && > + msg=$(test_write_lines "ack! first commit" "" "$ack") && > git reset --hard base && > git commit --allow-empty -m "$msg" -- && > git tag ack && > test_tick && > git rebase --autosquash -i HEAD^^^ && > git log --oneline >actual && > - git show -s first-commit | grep -v ^commit > expected-msg && > - echo "$ack" >> expected-msg && > - git show -s HEAD^ | grep -v ^commit > actual-msg && > - diff actual-msg expected-msg > + git show -s first-commit | grep -v ^commit >expected-msg && > + echo "$ack" >>expected-msg && > + git show -s HEAD^ | grep -v ^commit >actual-msg && > + test_cmp actual-msg expected-msg > ' > > test_expect_success 'auto squash (config)' ' -- 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] remote-helpers: point at their upstream repositories
Felipe Contreras writes: >> We could add these two to the warning, then, to discourage people >> who see "please visit this URL" and say "Yuck, I have no time for >> that" without actually visiting. > > We could. Personally I don't see the point of making the warning any > more annoying. The instructiosn are just one click away, and if they > have no time for that, they can just ignore the warning. Yes, if they know a short-and-sweet instruction is at the top of the page, "just one click away" is a good justification, but the reason I suggested to add the instruction to the warning is because the URL alone does not tell them that there is a short and easy-to-follow instruction is behind it, not giving them enough clue to judge if they have time for that or not. > To me the endgame is that the code is removed, and only stubs remain. > ... > I meant I want 3. eventually, hopefully for v2.1. > ... > No, I don't want them crippled for v2.0. A warning should suffice. OK, I think I understand what you want, and I am fine with that timeline. I can start preparing -rc4 now, but it may slip into tomorrow. 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
[PATCH] rebase -i: handle "Nothing to do" case with autostash
When a user invokes
$ git rebase -i @~3
with dirty files and rebase.autostash turned on, and exits the $EDITOR
with an empty buffer, the autostash fails to apply. Although the primary
focus of rr/rebase-autostash was to get the git-rebase--backend.sh
scripts to return control to git-rebase.sh, it missed this case in
git-rebase--interactive.sh. Since this case is unlike the other cases
which return control for housekeeping, assign it a special return status
and handle that return value explicitly in git-rebase.sh.
Reported-by: Karen Etheridge
Signed-off-by: Ramkumar Ramachandra
---
Thanks to Karen for reporting this.
I chose 2 arbitrarily. Let me know if you have a rationale for other
return values.
git-rebase--interactive.sh | 4 ++--
git-rebase.sh | 11 ++-
2 files changed, 12 insertions(+), 3 deletions(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 6ec9d3c..f267d8b 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -1049,14 +1049,14 @@ fi
has_action "$todo" ||
- die_abort "Nothing to do"
+ return 2
cp "$todo" "$todo".backup
git_sequence_editor "$todo" ||
die_abort "Could not execute editor"
has_action "$todo" ||
- die_abort "Nothing to do"
+ return 2
expand_todo_ids
diff --git a/git-rebase.sh b/git-rebase.sh
index 4543815..47ca3b9 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -155,7 +155,7 @@ move_to_original_branch () {
esac
}
-finish_rebase () {
+apply_autostash () {
if test -f "$state_dir/autostash"
then
stash_sha1=$(cat "$state_dir/autostash")
@@ -171,6 +171,10 @@ You can run "git stash pop" or "git stash drop" at any
time.
'
fi
fi
+}
+
+finish_rebase () {
+ apply_autostash &&
git gc --auto &&
rm -rf "$state_dir"
}
@@ -186,6 +190,11 @@ run_specific_rebase () {
if test $ret -eq 0
then
finish_rebase
+ elif test $ret -eq 2 # special exit status for rebase -i
+ then
+ apply_autostash &&
+ rm -rf "$state_dir" &&
+ die "Nothing to do"
fi
exit $ret
}
--
2.0.0.rc2.20.gfc2568d.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: [ANNOUNCE] git related v0.3
Ævar Arnfjörð Bjarmason wrote: > On Mon, May 19, 2014 at 2:36 AM, Felipe Contreras > wrote: > > This tool finds people that might be interested in a patch, by going > > back through the history for each single hunk modified, and finding > > people that reviewed, acknowledged, signed, or authored the code the > > patch is modifying. > > > > It does this by running `git blame` incrementally on each hunk, and > > finding the relevant commit message. After gathering all the relevant > > people, it groups them to show what exactly was their role when the > > participated in the development of the relevant commit, and on how many > > relevant commits they participated. They are only displayed if they pass > > a minimum threshold of participation. > > > > It is similar the the `git contacts` tool in the contrib area, which is a > > rewrite of this tool, except that `git contacts` does the absolute minimum; > > `git related` is way superior in every way. > > The general heuristic I use, which I've found to be much better than > git-blame is: > > 1. Find substrings of code I'm directly removing/altering, and > functions I'm removing/altering > 2. Do git log --reverse -p -S'' (maybe with -- file) for a > list of substrings Yes, that is true, but it cannot be automated. When I'm lazy I just do `git related -cfull a..b`, which will show me the full patches so I can decide if they are relevant or not. One possibility would be to add an additional --keywords option to `git related`. Another would be to add an --interactive where each supposedly relevant patch is shown for the user to decide if it truly is. -- 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 3/4] rebase: test ack
"Michael S. Tsirkin" writes: > test ack! handling > > Signed-off-by: Michael S. Tsirkin Will queue with this squashed in. 4/4 seems to have some style issues as well, but I didn't look very closely. Thanks. t/t3415-rebase-autosquash.sh | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/t/t3415-rebase-autosquash.sh b/t/t3415-rebase-autosquash.sh index 9d7db13..dcdba6f 100755 --- a/t/t3415-rebase-autosquash.sh +++ b/t/t3415-rebase-autosquash.sh @@ -75,18 +75,18 @@ test_expect_success 'auto squash (option)' ' ' test_expect_success 'auto ack' ' - ack="Acked-by: xyz" - msg=$(test_write_lines "ack! first commit" "" "$ack") + ack="Acked-by: xyz" && + msg=$(test_write_lines "ack! first commit" "" "$ack") && git reset --hard base && git commit --allow-empty -m "$msg" -- && git tag ack && test_tick && git rebase --autosquash -i HEAD^^^ && git log --oneline >actual && - git show -s first-commit | grep -v ^commit > expected-msg && - echo "$ack" >> expected-msg && - git show -s HEAD^ | grep -v ^commit > actual-msg && - diff actual-msg expected-msg + git show -s first-commit | grep -v ^commit >expected-msg && + echo "$ack" >>expected-msg && + git show -s HEAD^ | grep -v ^commit >actual-msg && + test_cmp actual-msg expected-msg ' test_expect_success 'auto squash (config)' ' -- 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] remote-helpers: point at their upstream repositories
Junio C Hamano wrote: > Felipe Contreras writes: > > > Junio C Hamano wrote: > > > >> - The "always warn" does not force update at the point of use, but > >>it still does not help them to notice well before they try to use > >>it for the first time after update; > > > > I don't understand this sentence. They will see a big fat warning every > > time they run the tool, of course they'll notice. > > Let me ask one question first, in order to avoid miscommunication, > as I really want to get the first step for v2.0-rc4 in a concrete > shape tomorrow. Do you think gradual transition worth pursuing or > do you think it is a waste of time? If by "gradual transition" you mean place the contrib/remote-helpers in another directory before removing the code, I do think it's a waste of time, but I don't really care, as long as eventually stubs are put in place. > I do not think it matters that much, but since you said you do not > understand... > > What I meant was that when they update their Git (perhaps at the > beginning of the week), the won't know they will now be running > stale code. The warning comes only when they first run it (perhaps > at the end of the week) to do some real work with a remote hg > repository, which may not be a convenient time to do their sysadmin > task. And the next time when they update their Git, they may have > already forgotten about the warning. The ideal transition would be > to somehow let them notice when they are in sysadmin mode. You can add such change in the release notes. Other than that I don't see what we could do. > >> - "Break the build" attempts to help them notice when they try to > >>update, not when they need to use the updated one right at this > >>moment. > > > > This cannot be done. > > Renaming the directory will not "break at the build time" for those > who have already did "ln -s" these scripts, of course, but it will > "break at the build time" for others (i.e. those who "cp"), no? How many people copy the scripts every time they update Git? Not many I'd gather. > > They click that URL, and the are immediately greated with this: > > > > To enable this, simply add the git-remote-hg script anywhere in your > > $PATH: > > > > wget https://raw.github.com/felipec/git-remote-hg/master/git-remote-hg > > -O ~/bin/git-remote-hg > > chmod +x ~/bin/git-remote-hg > > Perfect. I did check the page when double-checking the URL while > writing the README thing, and I did skim the page, but I must have > missed it. > > We could add these two to the warning, then, to discourage people > who see "please visit this URL" and say "Yuck, I have no time for > that" without actually visiting. We could. Personally I don't see the point of making the warning any more annoying. The instructiosn are just one click away, and if they have no time for that, they can just ignore the warning. > >> So to summarize, the following timeline is a full possibility: > >> ... > >> 2. add warning that is given every time the scripts are run and > >> give the same instruction as in README. > >> > >> 3. (optional) cripple the script to make them always fail after > >> showing the same warning as above. > > > > This is what I want, and I already sent the patches for; the scripts > > will be stubs. At this point you would have effectively removed the > > code, which what I want. > > I think explained why the step 3 would not help very much compared > to the "there is no script, only README remains" endgame (and that > is why it is marked as "optional"). Actually, you reminded me that > a very short and easy-to-follow instruction is on the page referred > to from README and the warnings, which means that this step would > make even less difference compared to the endgame. > > I don't think I saw you explain why that is not the case and why we > do want this step (and I cannot quite tell if you are aiming for > more gradual transition that wants this step, or an expedited one > that does not). I am fine with either way. To me the endgame is that the code is removed, and only stubs remain. What you do after that is up to you. > In any case, I'd ask another question to avoid wasting time on > miscommunication. By "This is what I want", do you mean you want > this step 3 also in v2.0, or do you mean you want 2 alone in v2.0 > then step 3 some time later? I meant I want 3. eventually, hopefully for v2.1. > You said your "wish" wasn't "respected" in another message, when I > explained that I thought you did not want to disrupt v2.0 by > insisting on removing these scripts and that was why I listed > options that did not involve removal of the scripts. Are you saying > that you wish you want to see them removed or crippled at v2.0? > Changing your mind after discussion is perfectly fine, by the way. You did not specify those were only for v2.0. No, I don't want them crippled for v2.0. A warning should suffice. -- Felipe Contreras -- To
Re: [PATCH] remote-helpers: point at their upstream repositories
Felipe Contreras writes:
> Junio C Hamano wrote:
>>
>> After looking at the reverse-depends list of packages, my faith is
>> strengthened in that the Git ecosystem is truly maturing and useful
>> third-party plug-ins will be picked up by distro packagers.
>
> Where is git-imerge packaged?
I didn't see it on the archive the said Ubuntu box slurps from, but
I did not check all the other distros.
Michael, do you know what distro folks are doing with imerge? For
the purpose of this thread, "I do not follow distros, and I do not
know" is a perfectly acceptable answer, but it would be very
relevant if your answer is "I suggested these distros to include it,
but so far they have been uncooperative and I haven't had much
success".
> Do you want to bet? Nah, you don't *ever* want to accept you were wrong,
> even you clearly where.
> ...
> This is what's going to happen: there won't be an official git-hg
> package for *years*, if there is ever one. That is my prediction based
> on all the available evidence, I am willing to stand by it and accept I
> was wrong if it proves otherwise.
>
> Are you willing to stand by your own decisions?
If I understand correctly, you have made and you do maintain some
packages and as an insider, you do not have to wait for "an
outsider" to step up to make remote-{hg,bzr} packages yourself. You
may already have done so for your own use and told other people
about them, and others may have chosen to wait for you to push them
to distros instead of championing these tools by packaging them
themselves.
When you have such an influence on the outcome either way of your
choice, I do not see much value in such a bet.
I do know enough to agree with you that there may be no committee,
packagers may scratch their own itches, and a program that is not
very useful for the packagers, especially the ones useful only for
non-technical niche audiences, may fall through the cracks.
But I actually think that "we package what we want to use" is a good
thing for programs whose primary audience is the software developer
types. The packagers are part of their audiences [*1*]. Because of
that, even if remote-{hg,bzr} do not get packaged for a long time, I
doubt that it tells us what you are stipulating. The only thing we
can infer would be that these programs did not interest the software
developer types to motivate them enough, and we wouldn't know why
they found the programs uninteresting. It may be because those who
have history in Hg prefer to interact with remote Git repositories
by pushing into and fetching from them using Hg tools than using Git
tools. It would not indicate "useful tools fall through the cracks"
if it were the case, would it?
Indeed I saw bzr-git that came from the Bazaar land packaged on the
box I mentioned, and its description sounded like it is meant to
work in such a way that allows Bazaar commits to be pushed to Git
repositories using a bzr tool.
By the way, I also saw git-mediawiki packaged from contrib/ in our
tree. I found it not very credible to say "contrib/ is treated as a
single ball of wax without much value by packagers, and we need to
move the helpers up to core in order for them to be used more
widely" after seeing that.
[Footnotes]
*1* I saw you called them "wolves" at least twice recently---where
does such a distrust come from?
--
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] send-pack.c: Allow to disable side-band-64k
On Mon, May 19, 2014 at 11:15 PM, Thomas Braun wrote: > Am 19.05.2014 21:33, schrieb Jonathan Nieder: > >> Hi, >> >> Thomas Braun wrote: >> >>> pushing over the dump git protocol with a windows git client. >> >> >> I've never heard of the dump git protocol. Do you mean the git >> protocol that's used with git:// URLs? > > > You are right I mean the protocol involving git:// URLs. But unfortunately I > got it wrong as according to [1] the git:// is one of the so-called smart > protocols. That was also the source where I read that there are smart and > dump protocols. > > [1]: http://git-scm.com/book/en/Git-Internals-Transfer-Protocols > > >> [...] >>> >>> Alternative approaches considered but deemed too invasive: >>> - Rewrite read/write wrappers in mingw.c in order to distinguish between >>>a file descriptor which has a socket behind and a file descriptor >>>which has a file behind. >> >> >> I assume here "too invasive" means "too much engineering effort"? >> >> It sounds like a clean fix, not too invasive at all. But I can >> understand wanting a stopgap in the meantime. > > > No actually I meant too invasive in the sense of "requiring large rewrites > which only benefit git on windows and hurt all others". > > The two fixes I can think of either involve: > - In a read *and* write wrapper the need to check if the fd is a socket, if > yes use send/recv if no use read/write. According to Erik's comments this > should be possible. But I would deem the expected performance penalty quite > large as that will be done in every call. You clearly haven't stepped through MSVCRT's read and write implementations :P I wouldn't worry too much about this, at least not until the numbers are in. -- 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] send-pack.c: Allow to disable side-band-64k
Am 19.05.2014 21:33, schrieb Jonathan Nieder: Hi, Thomas Braun wrote: pushing over the dump git protocol with a windows git client. I've never heard of the dump git protocol. Do you mean the git protocol that's used with git:// URLs? You are right I mean the protocol involving git:// URLs. But unfortunately I got it wrong as according to [1] the git:// is one of the so-called smart protocols. That was also the source where I read that there are smart and dump protocols. [1]: http://git-scm.com/book/en/Git-Internals-Transfer-Protocols [...] Alternative approaches considered but deemed too invasive: - Rewrite read/write wrappers in mingw.c in order to distinguish between a file descriptor which has a socket behind and a file descriptor which has a file behind. I assume here "too invasive" means "too much engineering effort"? It sounds like a clean fix, not too invasive at all. But I can understand wanting a stopgap in the meantime. No actually I meant too invasive in the sense of "requiring large rewrites which only benefit git on windows and hurt all others". The two fixes I can think of either involve: - In a read *and* write wrapper the need to check if the fd is a socket, if yes use send/recv if no use read/write. According to Erik's comments this should be possible. But I would deem the expected performance penalty quite large as that will be done in every call. - Rewriting read/write to accept windows handles instead of file descriptors. Only a theoretical option IMHO. For me the goal is also to minimise the diff between git and msysgit/git. - Turning the capability side-band-64k off completely. This would remove a useful feature for users of non-affected transport protocols. Would it make sense to turn off sideband unconditionally on Windows when using the relevant protocols? Yes, if this would be also acceptable for git.git. I can check at the call site of send_pack in transport.c what protocol is in use, and then pass a new parameter use_sideband to it. Or maybe "adapt" server_capabilities in connect.c to not include side-band-64k if using 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: [ANNOUNCE] git reintegrate v0.3; manager of integration branches
Felipe Contreras writes: > ... > Which will generate the integration instructions for you: > > % git reintegrate --cat > base master > merge jl/submodule-mv > > Moving a regular file in a repository with a .gitmodules file was > producing a warning 'Could not find section in .gitmodules where > path='. > > merge ap/remote-hg-unquote-cquote > > It also has support for "evil merges", so it should be perfectly > usable for git.git maintenance. Yeah, it sounds like it is almost there. I think the infrastructure to maintain "What's cooking" could be updated to use these comments after "merge" instructions if I wanted to. I build two branches on top of 'master', one is called 'jch' and has a marker line somewhere that says '### match next' that is turned into an empty commit, and 'pu' that is built on top of the tip of 'jch'. The marker line is used to apply only an earlier part of the instruction stream to build 'jch' on top of 'master' on top of 'next' (i.e. "base master" in the above example will not be applied to hard-reset 'next' to match master) and stop there, and is meant to be a way to sanity check 'next' (which is made by repeated incremental merges on top of 'master' without rewinding) by comparing the "### match next" commit between 'master' and 'jch' (which is made afresh from 'master' by taking only the necessary topics). They must match or I caught a possible mismerge on 'next'. I presume that the workflow can be mimicked by having another branch 'match-next' and building it on top of 'master', and then building 'jch' on top of it, and then building 'pu' on top of it. Then you should be able to play 'match-next' instruction on top of 'next' (provided that there is a way to tell it to replay on HEAD and ignore "base" and have "merge" instruction become a no-op when the branch has already been merged). Fun. -- 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 v3 00/10] replace: add option to edit a Git object
Christian Couder writes: > This patch series comes from what Peff sent in the following thread: > > http://thread.gmane.org/gmane.comp.version-control.git/243361/focus=243528 > > The only changes compared to v2 are in the test (8/10) and documentation > patches (10/10). Thanks to Peff. > > Christian Couder (6): > replace: make sure --edit results in a different object > replace: refactor checking ref validity > replace: die early if replace ref already exists > replace: add tests for --edit > replace: add --edit to usage string > Documentation: replace: describe new --edit option > > Jeff King (4): > replace: refactor command-mode determination > replace: use OPT_CMDMODE to handle modes > replace: factor object resolution out of replace_object > replace: add --edit option Part of this is already in 'next'; I'd assume that the first four can be discarded and it is safe to queue only the remaining six on top of what we already have. 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: [msysGit] Re: [PATCH/RFC] send-pack.c: Allow to disable side-band-64k
On Mon, May 19, 2014 at 10:00 PM, Erik Faye-Lund wrote:
> On Mon, May 19, 2014 at 9:33 PM, Jonathan Nieder wrote:
>> Hi,
>>
>> Thomas Braun wrote:
>>
>>> pushing over the dump git protocol with a windows git client.
>>
>> I've never heard of the dump git protocol. Do you mean the git
>> protocol that's used with git:// URLs?
>>
>> [...]
>>> Alternative approaches considered but deemed too invasive:
>>> - Rewrite read/write wrappers in mingw.c in order to distinguish between
>>> a file descriptor which has a socket behind and a file descriptor
>>> which has a file behind.
>>
>> I assume here "too invasive" means "too much engineering effort"?
>>
>> It sounds like a clean fix, not too invasive at all. But I can
>> understand wanting a stopgap in the meantime.
>>
>
> Yeah, now that the problem seems to be understood, I don't think that
> would be too bad. I recently killed off our previous write()-wrapper
> in c9df6f4, but I see no reason why we can't add a new one.
>
> Would we need to wrap both ends, shouldn't wrapping only reading be
> good enough to prevent deadlocking?
>
> compat/poll/poll.c already contains a function called IsSocketHandle
> that is able to tell if a HANDLE points to a socket or not.
This very quick attempt did not work out :(
diff --git a/compat/mingw.c b/compat/mingw.c
index 0335958..ec1d81f 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -370,6 +370,65 @@ int mingw_open (const char *filename, int oflags, ...)
return fd;
}
+#define is_console_handle(h) (((long) (h) & 3) == 3)
+
+static int is_socket_handle(HANDLE h)
+{
+WSANETWORKEVENTS ev;
+
+if (is_console_handle(h))
+return 0;
+
+/*
+ * Under Wine, it seems that getsockopt returns 0 for pipes too.
+ * WSAEnumNetworkEvents instead distinguishes the two correctly.
+ */
+ev.lNetworkEvents = 0xDEADBEEF;
+WSAEnumNetworkEvents((SOCKET) h, NULL, &ev);
+return ev.lNetworkEvents != 0xDEADBEEF;
+}
+
+#undef read
+ssize_t mingw_read(int fd, void *buf, size_t count)
+{
+int ret;
+HANDLE fh = (HANDLE)_get_osfhandle(fd);
+
+if (fh == INVALID_HANDLE_VALUE) {
+errno = EBADF;
+return -1;
+}
+
+if (!is_socket_handle(fh))
+return read(fd, buf, count);
+
+ret = recv((SOCKET)fh, buf, count, 0);
+if (ret < 0)
+errno = WSAGetLastError();
+return ret;
+}
+
+#undef write
+ssize_t mingw_write(int fd, const void *buf, size_t count)
+{
+int ret;
+HANDLE fh = (HANDLE)_get_osfhandle(fd);
+
+if (fh == INVALID_HANDLE_VALUE) {
+errno = EBADF;
+return -1;
+}
+
+if (!is_socket_handle(fh))
+return write(fd, buf, count);
+
+return send((SOCKET)fh, buf, count, 0);
+if (ret < 0)
+errno = WSAGetLastError();
+return ret;
+}
+
+
static BOOL WINAPI ctrl_ignore(DWORD type)
{
return TRUE;
diff --git a/compat/mingw.h b/compat/mingw.h
index 08b83fe..1690098 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -177,6 +177,12 @@ int mingw_rmdir(const char *path);
int mingw_open (const char *filename, int oflags, ...);
#define open mingw_open
+ssize_t mingw_read(int fd, void *buf, size_t count);
+#define read mingw_read
+
+ssize_t mingw_write(int fd, const void *buf, size_t count);
+#define write mingw_write
+
int mingw_fgetc(FILE *stream);
#define fgetc mingw_fgetc
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [msysGit] Re: [PATCH/RFC] send-pack.c: Allow to disable side-band-64k
On Mon, May 19, 2014 at 9:33 PM, Jonathan Nieder wrote: > Hi, > > Thomas Braun wrote: > >> pushing over the dump git protocol with a windows git client. > > I've never heard of the dump git protocol. Do you mean the git > protocol that's used with git:// URLs? > > [...] >> Alternative approaches considered but deemed too invasive: >> - Rewrite read/write wrappers in mingw.c in order to distinguish between >> a file descriptor which has a socket behind and a file descriptor >> which has a file behind. > > I assume here "too invasive" means "too much engineering effort"? > > It sounds like a clean fix, not too invasive at all. But I can > understand wanting a stopgap in the meantime. > Yeah, now that the problem seems to be understood, I don't think that would be too bad. I recently killed off our previous write()-wrapper in c9df6f4, but I see no reason why we can't add a new one. Would we need to wrap both ends, shouldn't wrapping only reading be good enough to prevent deadlocking? compat/poll/poll.c already contains a function called IsSocketHandle that is able to tell if a HANDLE points to a socket or not. -- 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
format-patch crashes with a huge patchset
I tried to fump the whole history of qemu with format-patch. It crashes both with v2.0.0-rc2-21-g6087111 and with git 1.8.3.1: ~/opt/libexec/git-core/git-format-patch --follow -o patches/all e63c3dc74bfb90e4522d075d0d5a7600c5145745.. Backtrace: Program received signal SIGSEGV, Segmentation fault. 0x0814d9d5 in try_to_follow_renames (opt=0xc8e4, base=base@entry=0x816e4fe "", t2=0xbdf0, t1=0xbddc) at tree-diff.c:227 227 diff_opts.single_follow = opt->pathspec.items[0].match; Missing separate debuginfos, use: debuginfo-install openssl-libs-1.0.1e-37.fc19.1.i686 (gdb) p opt $1 = (struct diff_options *) 0xc8e4 (gdb) where #0 0x0814d9d5 in try_to_follow_renames (opt=0xc8e4, base=base@entry=0x816e4fe "", t2=0xbdf0, t1=0xbddc) at tree-diff.c:227 #1 diff_tree_sha1 (old=0x97469b4 "\372\022\366\336k\345\236\362\062K\021\236\300\227\036\302\217\251\202f", new=new@entry=0x9746994 "$\305H\250)\237\203\266ya\311W\n\274 \n\027^*\221", base=base@entry=0x816e4fe "", opt=opt@entry=0xc8e4) at tree-diff.c:305 #2 0x080fb83d in log_tree_diff (log=0xbf28, commit=0x9734730, opt=0xc618) at log-tree.c:780 #3 log_tree_commit (opt=opt@entry=0xc618, commit=commit@entry=0x9734730) at log-tree.c:810 #4 0x08088406 in cmd_format_patch (argc=, argv=0xccc4, prefix=0x0) at builtin/log.c:1510 #5 0x0804c666 in run_builtin (argv=0xccc4, argc=5, p=0x81cb524 ) at git.c:314 #6 handle_builtin (argc=5, argv=0xccc4) at git.c:487 #7 0x0804bc22 in main (argc=5, av=0xccc4) at git.c:584 (gdb) p opt->pathspec.items $2 = (struct pathspec_item *) 0x0 Did not debug further: could be related to the fact swap is disabled on my box, so attempts to allocate huge amounts of RAM might fail. Still should not segv I think ... -- MST -- 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] send-pack.c: Allow to disable side-band-64k
Hi, Thomas Braun wrote: > pushing over the dump git protocol with a windows git client. I've never heard of the dump git protocol. Do you mean the git protocol that's used with git:// URLs? [...] > Alternative approaches considered but deemed too invasive: > - Rewrite read/write wrappers in mingw.c in order to distinguish between > a file descriptor which has a socket behind and a file descriptor > which has a file behind. I assume here "too invasive" means "too much engineering effort"? It sounds like a clean fix, not too invasive at all. But I can understand wanting a stopgap in the meantime. > - Turning the capability side-band-64k off completely. This would remove a > useful > feature for users of non-affected transport protocols. Would it make sense to turn off sideband unconditionally on Windows when using the relevant protocols? I assume someone on the list wouldn't mind writing such a patch, so I don't think the engineering effort would be a problem for that. Thanks, Jonathan -- 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] send-pack.c: Allow to disable side-band-64k
Since commit 0c499ea60f the send-pack builtin uses the side-band-64k
capability if advertised by the server. Unfortunately this breaks
pushing over the dump git protocol with a windows git client.
The detailed reasons for this breakage are (by courtesy of Jeff
Preshing, quoted from
https://groups.google.com/d/msg/msysgit/at8D7J-h7mw/eaLujILGUWoJ):
MinGW wraps Windows sockets in CRT file descriptors in order to mimic
the functionality of POSIX sockets. This causes msvcrt.dll to treat
sockets as Installable File System (IFS) handles, calling ReadFile,
WriteFile, DuplicateHandle and CloseHandle on them. This approach works
well in simple cases on recent versions of Windows, but does not support
all usage patterns. In particular, using this approach, any attempt to
read & write concurrently on the same socket (from one or more
processes) will deadlock in a scenario where the read waits for a
response from the server which is only invoked after the write. This is
what send_pack currently attempts to do in the use_sideband codepath.
The new config option "sendpack.sideband" allows to override the
side-band-64k capability of the server.
Other transportation methods like ssh and http/https still benefit from
the sideband channel, therefore the default value of "sendpack.sideband"
is still true.
Alternative approaches considered but deemed too invasive:
- Rewrite read/write wrappers in mingw.c in order to distinguish between
a file descriptor which has a socket behind and a file descriptor
which has a file behind.
- Turning the capability side-band-64k off completely. This would remove a
useful
feature for users of non-affected transport protocols.
Signed-off-by: Thomas Braun
---
This patch, with a slightly less polished commit message, is already part of
msysgit/git see b68e386.
A lengthy discussion can be found here [1].
What do you think, is this also for you as upstream interesting?
[1]: https://github.com/msysgit/git/issues/101
Documentation/config.txt | 6 ++
send-pack.c | 14 +-
2 files changed, 19 insertions(+), 1 deletion(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 1932e9b..13ff657 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2435,3 +2435,9 @@ web.browser::
Specify a web browser that may be used by some commands.
Currently only linkgit:git-instaweb[1] and linkgit:git-help[1]
may use it.
+
+sendpack.sideband::
+ Allows to disable the side-band-64k capability for send-pack even
+ when it is advertised by the server. Makes it possible to work
+ around a limitation in the git for windows implementation together
+ with the dump git protocol. Defaults to true.
diff --git a/send-pack.c b/send-pack.c
index 6129b0f..aace1fc 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -12,6 +12,16 @@
#include "version.h"
#include "sha1-array.h"
+static int config_use_sideband = 1;
+
+static int send_pack_config(const char *var, const char *value, void *unused)
+{
+ if (!strcmp("sendpack.sideband", var))
+ config_use_sideband = git_config_bool(var, value);
+
+ return 0;
+}
+
static int feed_object(const unsigned char *sha1, int fd, int negative)
{
char buf[42];
@@ -209,6 +219,8 @@ int send_pack(struct send_pack_args *args,
int ret;
struct async demux;
+ git_config(send_pack_config, NULL);
+
/* Does the other end support the reporting? */
if (server_supports("report-status"))
status_report = 1;
@@ -216,7 +228,7 @@ int send_pack(struct send_pack_args *args,
allow_deleting_refs = 1;
if (server_supports("ofs-delta"))
args->use_ofs_delta = 1;
- if (server_supports("side-band-64k"))
+ if (config_use_sideband && server_supports("side-band-64k"))
use_sideband = 1;
if (server_supports("quiet"))
quiet_supported = 1;
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [msysGit] Re: [PATCH v2] config: preserve config file permissions on edits
On Mon, May 19, 2014 at 9:17 PM, Thomas Braun
wrote:
> Am Montag, den 19.05.2014, 11:59 +0200 schrieb Erik Faye-Lund:
>> On Mon, May 19, 2014 at 9:44 AM, Erik Faye-Lund wrote:
>> > On Mon, May 19, 2014 at 9:13 AM, Johannes Sixt
>> > wrote:
>> >> Am 5/6/2014 2:17, schrieb Eric Wong:
>> >>> Users may already store sensitive data such as imap.pass in
>> >>> ..git/config; making the file world-readable when "git config"
>> >>> is called to edit means their password would be compromised
>> >>> on a shared system.
>> >>>
>> >>> [v2: updated for section renames, as noted by Junio]
>> >>>
>> >>> Signed-off-by: Eric Wong
>> >>> ---
>> >>> config.c | 16
>> >>> t/t1300-repo-config.sh | 10 ++
>> >>> 2 files changed, 26 insertions(+)
>> >>>
>> >>> diff --git a/config.c b/config.c
>> >>> index a30cb5c..c227aa8 100644
>> >>> --- a/config.c
>> >>> +++ b/config.c
>> >>> @@ -1636,6 +1636,13 @@ int git_config_set_multivar_in_file(const char
>> >>> *config_filename,
>> >>> MAP_PRIVATE, in_fd, 0);
>> >>> close(in_fd);
>> >>>
>> >>> + if (fchmod(fd, st.st_mode & 0) < 0) {
>> >>> + error("fchmod on %s failed: %s",
>> >>> + lock->filename, strerror(errno));
>> >>> + ret = CONFIG_NO_WRITE;
>> >>> + goto out_free;
>> >>> + }
>> >>
>> >> This introduces a severe failure in the Windows port because we do not
>> >> implement fchmod. Existing fchmod invocations do not check the return
>> >> value, and they are only interested in removing the write bits, and we
>> >> generally don't care a lot if files remain writable.
>> >>
>> >> I'm not proficient enough to add any ACL fiddling to fchmod that would be
>> >> required by the above change, whose purpose is to be strict about
>> >> permissions. Nor am I interested (who the heck is sharing a Windows box
>> >> anyway? ;-)
>> >>
>> >> Therefore, here's just a work-around patch to keep things going on
>> >> Windows. Any opinions from the Windows corner?
>> >>
>> >
>> > Since we use MSVCRT's chmod implementation directly which only checks
>> > for S_IWRITE,and Get/SetFileAttributes to simply set or clear the
>> > FILE_ATTRIBUTE_READONLY-flag, perhaps we could do the same except
>> > using Get/SetFileInformationByHandle instead? That takes us in a
>> > better direction, IMO. Adding full ACL support seems like a bigger
>> > project.
>> >
>> > If there's no objection, I'll sketch up a patch for that...
>>
>> OK, this turned out a bit more yucky than I assumed, because
>> SetFileInformationByHandle is only available on Windows Vista and up.
>> There's a static library (FileExtd.lib) that ships with MSVC that
>> emulates it for legacy support, I guess I should have a look at what
>> that does.
>
> Do we still want to fully support Windows XP?
> Adding a work around here for a EOL operating system sounds like a lot
> of effort.
I personally don't care about Windows XP, but some other influential
people (J6T IIRC) disagreed the last time this was discussed. I don't
want to step on anyone's toes.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [msysGit] Re: [PATCH v2] config: preserve config file permissions on edits
Am Montag, den 19.05.2014, 11:59 +0200 schrieb Erik Faye-Lund:
> On Mon, May 19, 2014 at 9:44 AM, Erik Faye-Lund wrote:
> > On Mon, May 19, 2014 at 9:13 AM, Johannes Sixt wrote:
> >> Am 5/6/2014 2:17, schrieb Eric Wong:
> >>> Users may already store sensitive data such as imap.pass in
> >>> ..git/config; making the file world-readable when "git config"
> >>> is called to edit means their password would be compromised
> >>> on a shared system.
> >>>
> >>> [v2: updated for section renames, as noted by Junio]
> >>>
> >>> Signed-off-by: Eric Wong
> >>> ---
> >>> config.c | 16
> >>> t/t1300-repo-config.sh | 10 ++
> >>> 2 files changed, 26 insertions(+)
> >>>
> >>> diff --git a/config.c b/config.c
> >>> index a30cb5c..c227aa8 100644
> >>> --- a/config.c
> >>> +++ b/config.c
> >>> @@ -1636,6 +1636,13 @@ int git_config_set_multivar_in_file(const char
> >>> *config_filename,
> >>> MAP_PRIVATE, in_fd, 0);
> >>> close(in_fd);
> >>>
> >>> + if (fchmod(fd, st.st_mode & 0) < 0) {
> >>> + error("fchmod on %s failed: %s",
> >>> + lock->filename, strerror(errno));
> >>> + ret = CONFIG_NO_WRITE;
> >>> + goto out_free;
> >>> + }
> >>
> >> This introduces a severe failure in the Windows port because we do not
> >> implement fchmod. Existing fchmod invocations do not check the return
> >> value, and they are only interested in removing the write bits, and we
> >> generally don't care a lot if files remain writable.
> >>
> >> I'm not proficient enough to add any ACL fiddling to fchmod that would be
> >> required by the above change, whose purpose is to be strict about
> >> permissions. Nor am I interested (who the heck is sharing a Windows box
> >> anyway? ;-)
> >>
> >> Therefore, here's just a work-around patch to keep things going on
> >> Windows. Any opinions from the Windows corner?
> >>
> >
> > Since we use MSVCRT's chmod implementation directly which only checks
> > for S_IWRITE,and Get/SetFileAttributes to simply set or clear the
> > FILE_ATTRIBUTE_READONLY-flag, perhaps we could do the same except
> > using Get/SetFileInformationByHandle instead? That takes us in a
> > better direction, IMO. Adding full ACL support seems like a bigger
> > project.
> >
> > If there's no objection, I'll sketch up a patch for that...
>
> OK, this turned out a bit more yucky than I assumed, because
> SetFileInformationByHandle is only available on Windows Vista and up.
> There's a static library (FileExtd.lib) that ships with MSVC that
> emulates it for legacy support, I guess I should have a look at what
> that does.
Do we still want to fully support Windows XP?
Adding a work around here for a EOL operating system sounds like a lot
of effort.
--
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 v10 25/44] receive-pack.c: use a reference transaction for updating the refs
On Sat, May 17, 2014 at 8:35 AM, Michael Haggerty wrote:
> On 05/16/2014 07:37 PM, Ronnie Sahlberg wrote:
>> Wrap all the ref updates inside a transaction to make the update atomic.
>>
>> Signed-off-by: Ronnie Sahlberg
>> ---
>> builtin/receive-pack.c | 20 ++--
>> 1 file changed, 10 insertions(+), 10 deletions(-)
>>
>> diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
>> index c323081..5534138 100644
>> --- a/builtin/receive-pack.c
>> +++ b/builtin/receive-pack.c
>> @@ -46,6 +46,8 @@ static void *head_name_to_free;
>> static int sent_capabilities;
>> static int shallow_update;
>> static const char *alt_shallow_file;
>> +static struct strbuf err = STRBUF_INIT;
>> +static struct ref_transaction *transaction;
>>
>> static enum deny_action parse_deny_action(const char *var, const char
>> *value)
>> {
>> @@ -475,7 +477,6 @@ static const char *update(struct command *cmd, struct
>> shallow_info *si)
>> const char *namespaced_name;
>> unsigned char *old_sha1 = cmd->old_sha1;
>> unsigned char *new_sha1 = cmd->new_sha1;
>> - struct ref_lock *lock;
>>
>> /* only refs/... are allowed */
>> if (!starts_with(name, "refs/") || check_refname_format(name + 5, 0)) {
>> @@ -580,15 +581,9 @@ static const char *update(struct command *cmd, struct
>> shallow_info *si)
>> update_shallow_ref(cmd, si))
>> return "shallow error";
>>
>> - lock = lock_any_ref_for_update(namespaced_name, old_sha1,
>> -0, NULL);
>> - if (!lock) {
>> - rp_error("failed to lock %s", name);
>> - return "failed to lock";
>> - }
>> - if (write_ref_sha1(lock, new_sha1, "push")) {
>> - return "failed to write"; /* error() already called */
>> - }
>> + if (ref_transaction_update(transaction, namespaced_name,
>> +new_sha1, old_sha1, 0, 1, &err))
>> + return "failed to update";
>> return NULL; /* good */
>> }
>> }
>> @@ -812,6 +807,7 @@ static void execute_commands(struct command *commands,
>> head_name = head_name_to_free = resolve_refdup("HEAD", sha1, 0, NULL);
>>
>> checked_connectivity = 1;
>> + transaction = ref_transaction_begin();
>> for (cmd = commands; cmd; cmd = cmd->next) {
>> if (cmd->error_string)
>> continue;
>> @@ -827,6 +823,10 @@ static void execute_commands(struct command *commands,
>> checked_connectivity = 0;
>> }
>> }
>> + if (ref_transaction_commit(transaction, "push", &err))
>> + error("%s", err.buf);
>> + ref_transaction_free(transaction);
>> + strbuf_release(&err);
>>
>> if (shallow_update && !checked_connectivity)
>> error("BUG: run 'git fsck' for safety.\n"
>>
>
> This patch is strange, because even if one ref_transaction_update() call
> fails, subsequent updates are nevertheless also attempted, and the
> ref_transaction_commit() is also attempted. Is this an officially
> sanctioned use of the ref_transactions API? Should it be?
I think it should be supported. Because otherwise, unless you have the
entire transaction localized in a single block you would end up having
to check and recheck the return value everywhere.
It makes the API much easier to use if you can continue calling
transaction functions even after the transaction has failed. If the
transaction has already failed then _update/_create/_delete will do
nothing except return an error.
If _commit is called on a failed transaction then the commit will fail
with an error
and do nothing.
I think it is convenient, and it allows things like :
struct ref_transaction *transaction;
void foo()
{
...
ref_transaction_update(transaction, ... , &err);
...
}
transaction = ref_transaction_begin(&err);
... doing stuff and call things that eventually ends up calling foo,
possible multiple times ...
ret = ref_transaction_commit(transaction, &err);
In foo() we ignore checking the return value so we will not see/care
if it failed. IF it fails however it will mark the transaction as
failed and update &err. (Note that this can not yet happen since
_update can not really fail, ever, but the next series will introduce
_update failures when we move locking there.)
Instead we can depend on that IF _update failed, then the call to
_commit will fail too and &err is already updated so we can defer any
checking for errors until _commit time.
This will make the API much more convenient for use cases where you
begin/commit the transaction in one function but the calls to
_update/_delete/_create are somewhere else, possible many function
calls away.
It does not mean that a caller must ignore the return value from
ref_transaction_update, just that the caller can do so and defer
checki
Re: [PATCH] remote-helpers: point at their upstream repositories
Felipe Contreras writes:
> Junio C Hamano wrote:
>
>> 2. add warning that is given every time the scripts are run and
>> give the same instruction as in README.
>>
>> 3. (optional) cripple the script to make them always fail after
>> showing the same warning as above.
>
> This is what I want, and I already sent the patches for; the scripts
> will be stubs. At this point you would have effectively removed the
> code, which what I want.
>
>> 4. Keep README and retire everything else.
>
> After you've removed the code, I don't care what you do, but I'd say you
> should remove the stubs after a long period of time.
Let's try this in a different way, as I sense there is a
misunderstanding somewhere about your "wish".
>> "that" does not refer to "remove them at v2.0 (unconditional)". It
>> refers to "If Felipe really wants for the removal for v2.0, I would
>> respect that". And I saw you said you did not want to disrupt v2.0.
>>
>> If the options I listed all meant removal at v2.0, then I would
>> understand your complaints, but that is not the case, so I am not
>> sure what to make of that.
>
> It is a weird choice of semantics then. You said you would "respect" my
> wish, but your proposals did not "follow" my wish.
I understand you do not want to disrupt v2.0. My assumption of that
"not disrupting v2.0" has been "there still are git-remote-{hg,bzr}
that work just like what they had in v1.9.x, perhaps with some
enhancements and regressions you added in the meantime", and I
understood Peff's comment "If Felipe wants the removal" to mean that
kind of "disruption", i.e. "there is no git-remote-{hg,bzr} that
work.", which would be either step 3 or 4.
But your "After you've removed the code" comment above makes me
wonder that perhaps your definition of "not disrupting" was
different from ours (which is not good or bad, just different) and
you consider that step 3. is "removal but not distupting v2.0"?
If that is what you want in v2.0, then please say so, and I already
said I am fine with that.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] replace: add --graft option
Jeff King writes: > On Mon, May 19, 2014 at 10:25:10AM -0700, Junio C Hamano wrote: > >> The headers up to committer are cast in stone in their ordering, and >> I do not immediately see how loosening it would be beneficial. >> >> Unless you are trying to give users a new way to record exactly the >> same commit in twenty-four (or more) ways with their own object >> names, that is ;-) > > Sorry, I didn't mean to imply that people can do what they want with > that ordering. Implementations that reorder the headers are stupid and > wrong, and should be fixed. Yeah, that was the only thing I meant to say, and what you said in the rest of message makes sense to me. I very much like the approach to parse line-by-line, noticing products from stupid and wrong implementations and warning or erroring out against them, while allowing an option to be lenient to help users who want to fix their repositories contaminated with such objects. -- 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 v10 14/44] replace.c: use the ref transaction functions for updates
I have moved the patch to add an err argument to this branch and
update all patches that adds
calls to ref_Transaction_begin to the new signature.
Please see
https://github.com/rsahlberg/git/tree/ref-transactions
Thanks!
On Sat, May 17, 2014 at 6:14 AM, Michael Haggerty wrote:
> On 05/16/2014 07:37 PM, Ronnie Sahlberg wrote:
>> Update replace.c to use ref transactions for updates.
>>
>> Signed-off-by: Ronnie Sahlberg
>> ---
>> builtin/replace.c | 14 --
>> 1 file changed, 8 insertions(+), 6 deletions(-)
>>
>> diff --git a/builtin/replace.c b/builtin/replace.c
>> index 3da1bae..e8932cd 100644
>> --- a/builtin/replace.c
>> +++ b/builtin/replace.c
>> @@ -133,7 +133,8 @@ static int replace_object_sha1(const char *object_ref,
>> unsigned char prev[20];
>> enum object_type obj_type, repl_type;
>> char ref[PATH_MAX];
>> - struct ref_lock *lock;
>> + struct ref_transaction *transaction;
>> + struct strbuf err = STRBUF_INIT;
>>
>> if (snprintf(ref, sizeof(ref),
>>"refs/replace/%s",
>> @@ -156,11 +157,12 @@ static int replace_object_sha1(const char *object_ref,
>> else if (!force)
>> die("replace ref '%s' already exists", ref);
>>
>> - lock = lock_any_ref_for_update(ref, prev, 0, NULL);
>> - if (!lock)
>> - die("%s: cannot lock the ref", ref);
>> - if (write_ref_sha1(lock, repl, NULL) < 0)
>> - die("%s: cannot update the ref", ref);
>> + transaction = ref_transaction_begin();
>> + if (!transaction ||
>> + ref_transaction_update(transaction, ref, repl, prev,
>> +0, !is_null_sha1(prev), &err) ||
>> + ref_transaction_commit(transaction, NULL, &err))
>> + die("%s", err.buf);
>
> Same here: err can be empty if ref_transaction_begin() fails. Please
> check later patches for the same error.
>
>>
>> return 0;
>> }
>>
>
>
> --
> Michael Haggerty
> mhag...@alum.mit.edu
> http://softwareswirl.blogspot.com/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v10 13/44] tag.c: use ref transactions when doing updates
I have moved the patch to add &err to ref_transaction_begin to the
current patch series.
Please see
https://github.com/rsahlberg/git/tree/ref-transactions
On Mon, May 19, 2014 at 10:16 AM, Ronnie Sahlberg wrote:
> On Sat, May 17, 2014 at 6:09 AM, Michael Haggerty
> wrote:
>> On 05/16/2014 07:37 PM, Ronnie Sahlberg wrote:
>>> Change tag.c to use ref transactions for all ref updates.
>>>
>>> Signed-off-by: Ronnie Sahlberg
>>> ---
>>> builtin/tag.c | 14 --
>>> 1 file changed, 8 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/builtin/tag.c b/builtin/tag.c
>>> index c6e8a71..b05f9a5 100644
>>> --- a/builtin/tag.c
>>> +++ b/builtin/tag.c
>>> @@ -548,7 +548,6 @@ int cmd_tag(int argc, const char **argv, const char
>>> *prefix)
>>> struct strbuf ref = STRBUF_INIT;
>>> unsigned char object[20], prev[20];
>>> const char *object_ref, *tag;
>>> - struct ref_lock *lock;
>>> struct create_tag_options opt;
>>> char *cleanup_arg = NULL;
>>> int annotate = 0, force = 0, lines = -1;
>>> @@ -556,6 +555,8 @@ int cmd_tag(int argc, const char **argv, const char
>>> *prefix)
>>> const char *msgfile = NULL, *keyid = NULL;
>>> struct msg_arg msg = { 0, STRBUF_INIT };
>>> struct commit_list *with_commit = NULL;
>>> + struct ref_transaction *transaction;
>>> + struct strbuf err = STRBUF_INIT;
>>> struct option options[] = {
>>> OPT_CMDMODE('l', "list", &cmdmode, N_("list tag names"), 'l'),
>>> { OPTION_INTEGER, 'n', NULL, &lines, N_("n"),
>>> @@ -701,11 +702,12 @@ int cmd_tag(int argc, const char **argv, const char
>>> *prefix)
>>> if (annotate)
>>> create_tag(object, tag, &buf, &opt, prev, object);
>>>
>>> - lock = lock_any_ref_for_update(ref.buf, prev, 0, NULL);
>>> - if (!lock)
>>> - die(_("%s: cannot lock the ref"), ref.buf);
>>> - if (write_ref_sha1(lock, object, NULL) < 0)
>>> - die(_("%s: cannot update the ref"), ref.buf);
>>> + transaction = ref_transaction_begin();
>>> + if (!transaction ||
>>> + ref_transaction_update(transaction, ref.buf, object, prev,
>>> +0, !is_null_sha1(prev), &err) ||
>>> + ref_transaction_commit(transaction, NULL, &err))
>>> + die("%s", err.buf);
>>
>> If ref_transaction_begin() fails, then won't err still be empty? (I
>> know it can't happen, and you know it can't happen, but should the
>> caller have to know that?) It almost seems like ref_transaction_begin()
>> should have an err parameter, too.
>
> I add an err argument in the next series. I would prefer we let that
> patch remain in the next series to
> avoid unbounded growth of the current one.
>
> Ok ?
>
>
>
>>
>> It's kindof late for this question to pop into my head, but: have you
>> thought about embedding the err strbuf in the transaction object? I
>> admit it would make the problem with ref_transaction_begin() even worse,
>> but maybe it would be a net win in terms of boilerplate?
>
> I think it is more flexible to allow the caller to manage the lifetime
> of the error buffer independently of the transaction.
> It would allow a caller to free the transaction early but delay
> printing the error message until later.
>
> Or it could be used for a multi transaction caller to use a single err
> buffer for all transactions and finally print
> all errors in a single error() call at the end.
>
> struct strbuf err = STRBUF_INIT;
> ... first transaction (... &err)...
> ... second transaction (... &err)...
> ... third transaction (... &err)...
> error("%s", err.buf);
>
>
>
> Similar to how rsync handles errors:
> sahlberg@sahlberg1:~$ mkdir foo
> sahlberg@sahlberg1:~$ touch foo/foo.1
> sahlberg@sahlberg1:~$ touch foo/foo.2
> sahlberg@sahlberg1:~$ mkdir bar
> sahlberg@sahlberg1:~$ chmod 0500 bar
> sahlberg@sahlberg1:~$ rsync -Pav foo/* bar
> sending incremental file list
> foo.1
>0 100%0.00kB/s0:00:00 (xfer#1, to-check=1/2)
> foo.2
>0 100%0.00kB/s0:00:00 (xfer#2, to-check=0/2)
> rsync: mkstemp "/usr/local/google/home/sahlberg/bar/.foo.1.K7dFIP"
> failed: Permission denied (13)
> rsync: mkstemp "/usr/local/google/home/sahlberg/bar/.foo.2.4WdRsW"
> failed: Permission denied (13)
>
> sent 136 bytes received 50 bytes 372.00 bytes/sec
> total size is 0 speedup is 0.00
> rsync error: some files/attrs were not transferred (see previous
> errors) (code 23) at main.c(1070) [sender=3.0.9]
>
>
>
>>
>>> if (force && !is_null_sha1(prev) && hashcmp(prev, object))
>>> printf(_("Updated tag '%s' (was %s)\n"), tag,
>>> find_unique_abbrev(prev, DEFAULT_ABBREV));
>>>
>>>
>>
>>
>> --
>> Michael Haggerty
>> mhag...@alum.mit.edu
>> http://softwareswirl.blogspot.com/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] replace: add --graft option
On Mon, May 19, 2014 at 10:25:10AM -0700, Junio C Hamano wrote:
> Jeff King writes:
>
> > It might make sense to just teach parse_commit_header to be a little
> > more thorough; it has to read past those lines anyway to find the author
> > and committer lines, so it would not be much more expensive to note
> > them. And then of course the code needs to be pulled out of the
> > pretty-printer and made generally accessible.
>
> I notice that you said "might" above.
Yeah. I think having a reusable parser is definitely a good thing. I'm
not sure if it's worth the massive amount of refactoring that would be
required for this particular use case.
> > That's more or less what Christian's function is doing, though it
> > assumes things like that the parents come immediately after the tree,
> > and that they are all in a group. Those are all true for objects created
> > by git, but I think we can be a little flexible.
>
> The headers up to committer are cast in stone in their ordering, and
> I do not immediately see how loosening it would be beneficial.
>
> Unless you are trying to give users a new way to record exactly the
> same commit in twenty-four (or more) ways with their own object
> names, that is ;-)
Sorry, I didn't mean to imply that people can do what they want with
that ordering. Implementations that reorder the headers are stupid and
wrong, and should be fixed.
BUT. I really don't like making these assumptions or doing ad-hoc
parsing all through the code. We _do_ see quirky, wrong objects, and
want to handle them sanely and consistently. That's hard to do when
there are parsers sprinkled throughout the code which handle each case
slightly differently. I don't recall seeing this with header ordering,
but I know that broken ident lines have caused us headaches in the past,
and I'm happy that we've (mostly) settled on the code in
split_ident_line to handle this.
Things like:
+ /* find existing parents */
+ parent_start = buf.buf;
+ parent_start += 46; /* "tree " + "hex sha1" + "\n" */
+ parent_end = parent_start;
scare me. Is buf actually 46 bytes long? If not, we read past the end of
the buffer. What if it contains something besides "tree \n" at the
beginning? We don't even notice!
The version I posted elsewhere in the thread at least operates on a
line-by-line basis, and tries to verify its assumptions (and barfs if
not). It's still doing its own parsing, but at least it's keeping its
assumptions on the object format to a minimum ("drop old parent lines",
"add new ones after tree, and barf if there's not exactly one tree
line").
-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: [RFC/PATCH] replace: add --graft option
Jeff King writes: > It might make sense to just teach parse_commit_header to be a little > more thorough; it has to read past those lines anyway to find the author > and committer lines, so it would not be much more expensive to note > them. And then of course the code needs to be pulled out of the > pretty-printer and made generally accessible. I notice that you said "might" above. > That's more or less what Christian's function is doing, though it > assumes things like that the parents come immediately after the tree, > and that they are all in a group. Those are all true for objects created > by git, but I think we can be a little flexible. The headers up to committer are cast in stone in their ordering, and I do not immediately see how loosening it would be beneficial. Unless you are trying to give users a new way to record exactly the same commit in twenty-four (or more) ways with their own object names, that is ;-) -- 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 v10 18/44] branch.c: use ref transaction for all ref updates
On Sat, May 17, 2014 at 6:33 AM, Michael Haggerty wrote:
> On 05/16/2014 07:37 PM, Ronnie Sahlberg wrote:
>> Change create_branch to use a ref transaction when creating the new branch.
>> ref_transaction_create will check that the ref does not already exist and
>> fail
>> otherwise meaning that we no longer need to keep a lock on the ref during the
>> setup_tracking. This simplifies the code since we can now do the transaction
>> in one single step.
>>
>> If the forcing flag is false then use ref_transaction_create since this will
>> fail if the ref already exist. Otherwise use ref_transaction_update.
>
> s/exist/exists/
>
> And the references to ref_transaction_create() in the commit message are
> obsolete.
>
Thanks. Fixed typo and deleted obsolete text.
>>
>> This also fixes a race condition in the old code where two concurrent
>> create_branch could race since the lock_any_ref_for_update/write_ref_sha1
>> did not protect against the ref already existing. I.e. one thread could end
>> up
>> overwriting a branch even if the forcing flag is false.
>>
>> Reviewed-by: Jonathan Nieder
>> Signed-off-by: Ronnie Sahlberg
>> ---
>> branch.c | 29 +++--
>> 1 file changed, 15 insertions(+), 14 deletions(-)
>>
>> diff --git a/branch.c b/branch.c
>> index 660097b..8bf7588 100644
>> --- a/branch.c
>> +++ b/branch.c
>> @@ -226,7 +226,6 @@ void create_branch(const char *head,
>> int force, int reflog, int clobber_head,
>> int quiet, enum branch_track track)
>> {
>> - struct ref_lock *lock = NULL;
>> struct commit *commit;
>> unsigned char sha1[20];
>> char *real_ref, msg[PATH_MAX + 20];
>> @@ -285,15 +284,6 @@ void create_branch(const char *head,
>> die(_("Not a valid branch point: '%s'."), start_name);
>> hashcpy(sha1, commit->object.sha1);
>>
>> - if (!dont_change_ref) {
>> - lock = lock_any_ref_for_update(ref.buf, NULL, 0, NULL);
>> - if (!lock)
>> - die_errno(_("Failed to lock ref for update"));
>> - }
>> -
>> - if (reflog)
>> - log_all_ref_updates = 1;
>> -
>> if (forcing)
>> snprintf(msg, sizeof msg, "branch: Reset to %s",
>>start_name);
>> @@ -301,13 +291,24 @@ void create_branch(const char *head,
>> snprintf(msg, sizeof msg, "branch: Created from %s",
>>start_name);
>>
>> + if (reflog)
>> + log_all_ref_updates = 1;
>> +
>> + if (!dont_change_ref) {
>> + struct ref_transaction *transaction;
>> + struct strbuf err = STRBUF_INIT;
>> +
>> + transaction = ref_transaction_begin();
>> + if (!transaction ||
>> + ref_transaction_update(transaction, ref.buf, sha1,
>> +null_sha1, 0, !forcing, &err) ||
>> + ref_transaction_commit(transaction, msg, &err))
>> + die("%s", err.buf);
>> + }
>> +
>> if (real_ref && track)
>> setup_tracking(ref.buf + 11, real_ref, track, quiet);
>>
>> - if (!dont_change_ref)
>> - if (write_ref_sha1(lock, sha1, msg) < 0)
>> - die_errno(_("Failed to write ref"));
>> -
>> strbuf_release(&ref);
>> free(real_ref);
>> }
>>
>
>
> --
> Michael Haggerty
> mhag...@alum.mit.edu
> http://softwareswirl.blogspot.com/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [GIT GUI PATCH v2] git-gui: tolerate major version changes when comparing the git version
Jens Lehmann writes: > Am 17.05.2014 14:23, schrieb Pat Thoyts: >> >> The analysis about the major version number being significant is >> correct. ... >> >> package vsatisfies $::_git_version 1.7.0- >> >> will suffice. > > Junio, please replace my old version with this. Thanks, both. Will replace. -- 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 v10 13/44] tag.c: use ref transactions when doing updates
On Sat, May 17, 2014 at 6:09 AM, Michael Haggerty wrote:
> On 05/16/2014 07:37 PM, Ronnie Sahlberg wrote:
>> Change tag.c to use ref transactions for all ref updates.
>>
>> Signed-off-by: Ronnie Sahlberg
>> ---
>> builtin/tag.c | 14 --
>> 1 file changed, 8 insertions(+), 6 deletions(-)
>>
>> diff --git a/builtin/tag.c b/builtin/tag.c
>> index c6e8a71..b05f9a5 100644
>> --- a/builtin/tag.c
>> +++ b/builtin/tag.c
>> @@ -548,7 +548,6 @@ int cmd_tag(int argc, const char **argv, const char
>> *prefix)
>> struct strbuf ref = STRBUF_INIT;
>> unsigned char object[20], prev[20];
>> const char *object_ref, *tag;
>> - struct ref_lock *lock;
>> struct create_tag_options opt;
>> char *cleanup_arg = NULL;
>> int annotate = 0, force = 0, lines = -1;
>> @@ -556,6 +555,8 @@ int cmd_tag(int argc, const char **argv, const char
>> *prefix)
>> const char *msgfile = NULL, *keyid = NULL;
>> struct msg_arg msg = { 0, STRBUF_INIT };
>> struct commit_list *with_commit = NULL;
>> + struct ref_transaction *transaction;
>> + struct strbuf err = STRBUF_INIT;
>> struct option options[] = {
>> OPT_CMDMODE('l', "list", &cmdmode, N_("list tag names"), 'l'),
>> { OPTION_INTEGER, 'n', NULL, &lines, N_("n"),
>> @@ -701,11 +702,12 @@ int cmd_tag(int argc, const char **argv, const char
>> *prefix)
>> if (annotate)
>> create_tag(object, tag, &buf, &opt, prev, object);
>>
>> - lock = lock_any_ref_for_update(ref.buf, prev, 0, NULL);
>> - if (!lock)
>> - die(_("%s: cannot lock the ref"), ref.buf);
>> - if (write_ref_sha1(lock, object, NULL) < 0)
>> - die(_("%s: cannot update the ref"), ref.buf);
>> + transaction = ref_transaction_begin();
>> + if (!transaction ||
>> + ref_transaction_update(transaction, ref.buf, object, prev,
>> +0, !is_null_sha1(prev), &err) ||
>> + ref_transaction_commit(transaction, NULL, &err))
>> + die("%s", err.buf);
>
> If ref_transaction_begin() fails, then won't err still be empty? (I
> know it can't happen, and you know it can't happen, but should the
> caller have to know that?) It almost seems like ref_transaction_begin()
> should have an err parameter, too.
I add an err argument in the next series. I would prefer we let that
patch remain in the next series to
avoid unbounded growth of the current one.
Ok ?
>
> It's kindof late for this question to pop into my head, but: have you
> thought about embedding the err strbuf in the transaction object? I
> admit it would make the problem with ref_transaction_begin() even worse,
> but maybe it would be a net win in terms of boilerplate?
I think it is more flexible to allow the caller to manage the lifetime
of the error buffer independently of the transaction.
It would allow a caller to free the transaction early but delay
printing the error message until later.
Or it could be used for a multi transaction caller to use a single err
buffer for all transactions and finally print
all errors in a single error() call at the end.
struct strbuf err = STRBUF_INIT;
... first transaction (... &err)...
... second transaction (... &err)...
... third transaction (... &err)...
error("%s", err.buf);
Similar to how rsync handles errors:
sahlberg@sahlberg1:~$ mkdir foo
sahlberg@sahlberg1:~$ touch foo/foo.1
sahlberg@sahlberg1:~$ touch foo/foo.2
sahlberg@sahlberg1:~$ mkdir bar
sahlberg@sahlberg1:~$ chmod 0500 bar
sahlberg@sahlberg1:~$ rsync -Pav foo/* bar
sending incremental file list
foo.1
0 100%0.00kB/s0:00:00 (xfer#1, to-check=1/2)
foo.2
0 100%0.00kB/s0:00:00 (xfer#2, to-check=0/2)
rsync: mkstemp "/usr/local/google/home/sahlberg/bar/.foo.1.K7dFIP"
failed: Permission denied (13)
rsync: mkstemp "/usr/local/google/home/sahlberg/bar/.foo.2.4WdRsW"
failed: Permission denied (13)
sent 136 bytes received 50 bytes 372.00 bytes/sec
total size is 0 speedup is 0.00
rsync error: some files/attrs were not transferred (see previous
errors) (code 23) at main.c(1070) [sender=3.0.9]
>
>> if (force && !is_null_sha1(prev) && hashcmp(prev, object))
>> printf(_("Updated tag '%s' (was %s)\n"), tag,
>> find_unique_abbrev(prev, DEFAULT_ABBREV));
>>
>>
>
>
> --
> Michael Haggerty
> mhag...@alum.mit.edu
> http://softwareswirl.blogspot.com/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Any difference to unstage files using "git checkout" and "git rm"
On Monday, May 19, 2014 12:01:07 PM you wrote: > On Mon, May 19, 2014 at 09:12:47PM +0630, Arup Rakshit wrote: > > Is there any difference between the below 2 commands ? I didn't see > > anything. > > > > Does that help? For me who is in Git just 6-7 days, It is huge. On your way, I was walking to test those out. But I got some messages from Git, which made me confused to think, about the *philosophy of those*. arup@linux-wzza:~> mkdir Git_tutorial arup@linux-wzza:~> cd Git_tutorial/ arup@linux-wzza:~/Git_tutorial> LS If 'LS' is not a typo you can use command-not-found to lookup the package that contains it, like this: cnf LS arup@linux-wzza:~/Git_tutorial> ls arup@linux-wzza:~/Git_tutorial> echo "welcome to git" > test.txt arup@linux-wzza:~/Git_tutorial> ls test.txt arup@linux-wzza:~/Git_tutorial> git init Initialized empty Git repository in /home/arup/Git_tutorial/.git/ arup@linux-wzza:~/Git_tutorial> git status # On branch master # # Initial commit # # Untracked files: # (use "git add ..." to include in what will be committed) # # test.txt nothing added to commit but untracked files present (use "git add" to track) arup@linux-wzza:~/Git_tutorial> git add test.txt arup@linux-wzza:~/Git_tutorial> git status # On branch master # # Initial commit # # Changes to be committed: # (use "git rm --cached ..." to unstage) # # new file: test.txt # NOTE :- While, I have a new file in my repository, and I staged it, it is telling me to unstage it using *git rm -- cached *. But once I committed, and the file became a tracked file in my repository, *Git* internal message got changed, *for unstaging*, Which is why, I asked this question. Look below - arup@linux-wzza:~/Git_tutorial> git commit -m "commit1" [master (root-commit) 20bf27f] commit1 1 file changed, 1 insertion(+) create mode 100644 test.txt arup@linux-wzza:~/Git_tutorial> echo "How are you enjoying?" > test.txt arup@linux-wzza:~/Git_tutorial> git status # On branch master # Changes not staged for commit: # (use "git add ..." to update what will be committed) # (use "git checkout -- ..." to discard changes in working directory) # # modified: test.txt # no changes added to commit (use "git add" and/or "git commit -a") arup@linux-wzza:~/Git_tutorial> git add test.txt arup@linux-wzza:~/Git_tutorial> git status # On branch master # Changes to be committed: # (use "git reset HEAD ..." to unstage) # # modified: test.txt # arup@linux-wzza:~/Git_tutorial> It is now telling to USE "git reset HEAD ..." to unstage, NOT "git rm --cached ..." to unstage. Please let me know, If I am making you guys more confused. -- === Regards, Arup Rakshit -- 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/technical/api-hashmap: Remove source highlighting
Anders Kaseorg writes: > Yes; when I noticed this failure, I asked Jonathan to add source-highlight > as a build dependency in Debian (https://bugs.debian.org/745591). But > then Ubuntu forked the packaging to revert this change > (https://bugs.launchpad.net/bugs/1316810), because source-highlight in the > community-supported universe repository is not allowed to be a build > dependency ... The reasoning and solution Ubuntu has sounds sensible *but* it also soudns like it is incomplete. If Ubuntu does not want to use highlight, it can apply a change like the patch in question as part of their fork to make the end result consistent and they are failing to do so. If the tooling do not use highlight, the source should not require highlight, either. It is ultimately their bug. It however *is* our business, as their upstream, to make it easier for distros that want to use and distros that do not want to depend on highlight, and aiming for a solution that relieves Ubuntu or any other distros from needing to carry one more patch is a good thing. How bad does the documentation look with the patch applied (I know how bad it looks without source-highlight installed)? If it is not too bad, then it sounds like a sensible solution to drop the highlight markup unconditionally like the patch that started this thread does, taking the "common denominator" approach. You seem to agree, and I do not object, either. > But I don’t that would be worth it just to make one page of the API > documentation a little more colorful (and it sounds like you agree). -- 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 v8 23/44] fetch.c: change s_update_ref to use a ref transaction
On Fri, May 16, 2014 at 3:54 PM, Jonathan Nieder wrote:
> Ronnie Sahlberg wrote:
>
>> --- a/builtin/fetch.c
>> +++ b/builtin/fetch.c
> [...]
>> @@ -384,15 +384,16 @@ static int s_update_ref(const char *action,
>> snprintf(msg, sizeof(msg), "%s: %s", rla, action);
>>
>> errno = 0;
>> - lock = lock_any_ref_for_update(ref->name,
>> -check_old ? ref->old_sha1 : NULL,
>> -0, NULL);
>> - if (!lock)
>> - return errno == ENOTDIR ? STORE_REF_ERROR_DF_CONFLICT :
>> - STORE_REF_ERROR_OTHER;
>> - if (write_ref_sha1(lock, ref->new_sha1, msg) < 0)
>> + transaction = ref_transaction_begin();
>> + if (!transaction ||
>> + ref_transaction_update(transaction, ref->name, ref->new_sha1,
>> +ref->old_sha1, 0, check_old) ||
>> + ref_transaction_commit(transaction, msg, NULL)) {
>
> Since 'err' is NULL, does that mean there's no message shown to the
> user on error?
Yes. Updated in the ref-transactions branch.
--
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 v8 24/44] fetch.c: use a single ref transaction for all ref updates
On Fri, May 16, 2014 at 3:52 PM, Jonathan Nieder wrote:
> Ronnie Sahlberg wrote:
>
>> Change store_updated_refs to use a single ref transaction for all refs that
>> are updated during the fetch. This makes the fetch more atomic when update
>> failures occur.
>
> Fun.
>
> [...]
>> --- a/builtin/fetch.c
>> +++ b/builtin/fetch.c
> [...]
>> @@ -373,27 +374,13 @@ static int s_update_ref(const char *action,
>> struct ref *ref,
>> int check_old)
>> {
>> - char msg[1024];
>> - char *rla = getenv("GIT_REFLOG_ACTION");
>> - struct ref_transaction *transaction;
>
> Previously fetch respected GIT_REFLOG_ACTION, and this is dropping
> that support. Intended?
I think it was added back later in the series when
ref_transaction_update started taking a "msg" argument.
I have reordered the patches in the ref-transactions so that the fetch
updates come after we add msg support to _update.
Please see current builtin/fetch.c in my ref-transactions branch.
>
> [...]
>> + if (ref_transaction_update(transaction, ref->name, ref->new_sha1,
>> +ref->old_sha1, 0, check_old))
>> + return STORE_REF_ERROR_OTHER;
>
> Should pass a strbuf in to get a message back.
I added err strbuf to both update and commit and now also error("%s",
err.buf) on failure.
>
> [...]
>> +
>> return 0;
>> }
>>
>> @@ -565,6 +552,13 @@ static int store_updated_refs(const char *raw_url,
>> const char *remote_name,
>> goto abort;
>> }
>>
>> + errno = 0;
>> + transaction = ref_transaction_begin();
>> + if (!transaction) {
>> + rc = error(_("cannot start ref transaction\n"));
>> + goto abort;
>
> error() appends a newline on its own, so the message shouldn't
> end with newline.
>
> More importantly, this message isn't helpful without more detail about
> why _transaction_begin() failed. The user doesn't know what
>
> error: cannot start ref transaction
>
> means.
>
> error: cannot connect to mysqld for ref storage: [etc]
>
> would tell what they need to know. That is:
>
> transaction = ref_transaction_begin(err);
> if (!transaction) {
> rc = error("%s", err.buf);
> goto abort;
> }
>
I add this in the next patch series. Right now ref_transaction_begin
can never return failure back to the caller.
> errno is not used here, so no need to set errno = 0.
removed.
>
> [...]
>> @@ -676,6 +670,10 @@ static int store_updated_refs(const char *raw_url,
>> const char *remote_name,
>> }
>> }
>> }
>> + if ((ret = ref_transaction_commit(transaction, NULL)))
>> + rc |= ret == -2 ? STORE_REF_ERROR_DF_CONFLICT :
>> + STORE_REF_ERROR_OTHER;
>
> (I cheated and stole the newer code from your repo.)
>
> Yay! Style nit: git avoids assignments in "if" conditionals.
>
> ret = ref_tr...
> if (ret == -2)
> rc |= ...
> else if (ret)
> rc |= ...
>
> Does _update need the too as futureproofing for backends that can
> detect the name conflict sooner?
Yes it will. I will update the next series to do this. If _update
fails with name conflict it will return -2.
Also it will store the status in the transaction so that a later call
to _commit will also return -2.
>
> Thanks,
> Jonathan
--
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] format-patch --signature-file
Jeremiah Mahler writes:
> On Sat, May 17, 2014 at 06:00:14AM -0400, Jeff King wrote:
>>
>> If you wanted to know whether it was set, I guess you'd have to compare
>> it to the default, like:
>>
>> if (signature_file) {
>> if (signature && signature != git_version_string)
>> die("you cannot specify both a signature and a signature-file");
>> ... read signature file ...
>> }
>>
>
> That works until someone changes the default value.
> But if they did that then some tests should fail.
>
> I like the address comparision which avoids a string comparision.
Well, "avoids" is not quite a correct phrasing, because !strcmp()
would be wrong there. You cannot tell "the user did not set
anything and the variable stayed as the default" and "the user
explicitly gave us a string but it happened to be the same as the
default" apart with !strcmp(). Address comparison is not just
"avoids" but is the right thing to do in this case.
>> though it's a bit ugly that this code has to know what the default is.
Avoiding that is easy with an indirection, no? Something like this
at the top:
static const char *the_default_signature = git_version_string;
static const char *signature = the_default_signature;
and comparing to see if signature points at the same address as
the_default_signature would give you what you want, I think.
--
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: Any difference to unstage files using "git checkout" and "git rm"
On Mon, May 19, 2014 at 09:27:47PM +0630, Arup Rakshit wrote: > > Is there any difference between the below 2 commands ? I didn't see > > anything. > > > > git rm --cached -- .. > > git checkout -- .. > > Please Ignore the previous. Too late. :) > I apologize to rewriting and increase the load of this mailing list. I > actually wanted to ask the below > > git rm -- cached -- .. > git reset HEAD .. OK, this is a more sensible comparison. The first command will remove the entries from the index entirely, and the second one will return them to their state in HEAD. So _if_ the existing commit in HEAD did not have the files at all, the two are identical. But if the files were committed previously, then the second command will return them to that state, not remove them entirely. Does that make sense? -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: [BUG] git push writes to stderr instead of stdout on success
On Mon, May 19, 2014 at 11:49:09AM -0400, Jeff King wrote: > On Mon, May 19, 2014 at 07:03:58PM +0400, Marat Radchenko wrote: > > > `git push` writes to stderr instead of stdout > > That's by design. > > Which one is correct is largely a matter of philosophy / mental model. > This case has been discussed before: > > http://thread.gmane.org/gmane.comp.version-control.git/180673 > > Keep in mind also that "git push --porcelain" does go to stdout and is > machine-parsed, so no other messages can go to stdout when that option > is enabled. Oh, missed --porcelain switch. 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] request-pull: resurrect for-linus -> tags/for-linus DWIM
"Michael S. Tsirkin" writes: > Well RHEL6 apparently comes with git 1.7.1, there are > probably others. > > The problem isn't theorectical actually, Thanks. Let's do the fix for 2.0 then. -- 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: Any difference to unstage files using "git checkout" and "git rm"
On Monday, May 19, 2014 09:12:47 PM you wrote: > Hi, > > Is there any difference between the below 2 commands ? I didn't see > anything. > > git rm --cached -- .. > git checkout -- .. Please Ignore the previous. I apologize to rewriting and increase the load of this mailing list. I actually wanted to ask the below git rm -- cached -- .. git reset HEAD .. -- === Regards, Arup Rakshit -- 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: Any difference to unstage files using "git checkout" and "git rm"
On Mon, May 19, 2014 at 09:12:47PM +0630, Arup Rakshit wrote: > Is there any difference between the below 2 commands ? I didn't see anything. > > git rm --cached -- .. This one removes the index entries for those files. > git checkout -- .. This one checks out the content from the index into the working tree (for just those files). Try: # setup... git init echo content >file git add file git commit -m one # now rm git rm --cached -- file git status which yields: rm 'file' On branch master Changes to be committed: deleted:file Untracked files: file but it we restore our state and try checkout: git reset git checkout -- file nothing happens: On branch master nothing to commit, working directory clean but if you had changes in the working tree: echo changes >file git status | sed 's/^/before> /' git checkout -- file git status | sed 's/^/ after> /' you get: before> On branch master before> Changes not staged for commit: before> modified: file before> before> no changes added to commit after> On branch master after> nothing to commit, working directory clean Does that help? -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: [msysGit] Re: [PATCH v2] config: preserve config file permissions on edits
On 5/19/2014 2:44 AM, Erik Faye-Lund wrote: On Mon, May 19, 2014 at 9:13 AM, Johannes Sixt wrote: I'm not proficient enough to add any ACL fiddling to fchmod that would be required by the above change, whose purpose is to be strict about permissions. Nor am I interested (who the heck is sharing a Windows box anyway? ;-) Since we use MSVCRT's chmod implementation directly which only checks for S_IWRITE,and Get/SetFileAttributes to simply set or clear the FILE_ATTRIBUTE_READONLY-flag, perhaps we could do the same except using Get/SetFileInformationByHandle instead? That takes us in a better direction, IMO. Adding full ACL support seems like a bigger project. If there's no objection, I'll sketch up a patch for that... IMO, try to stay away from full ACL support, as ACL is slow as hell. If it's only triggered in edge cases, that's fine, but be sure not to add any ACL in the main call-paths as it will surely kill performance. The major speedup which was done by circumventing MSYS/Cygwin standard posix implementations was bypassing much of the ACL slowness. -- .marius -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BUG] git push writes to stderr instead of stdout on success
On Mon, May 19, 2014 at 07:03:58PM +0400, Marat Radchenko wrote: > `git push` writes to stderr instead of stdout That's by design. Which one is correct is largely a matter of philosophy / mental model. This case has been discussed before: http://thread.gmane.org/gmane.comp.version-control.git/180673 Keep in mind also that "git push --porcelain" does go to stdout and is machine-parsed, so no other messages can go to stdout when that option is enabled. -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
Any difference to unstage files using "git checkout" and "git rm"
Hi, Is there any difference between the below 2 commands ? I didn't see anything. git rm --cached -- .. git checkout -- .. -- === Regards, Arup Rakshit -- 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 v3 3/5] t4205 (log-pretty-format): Use `tformat` rather than `format`
Use `tformat` to avoid using of `echo` to complete end of line. Signed-off-by: Alexey Shumkin --- t/t4205-log-pretty-formats.sh | 52 +++ 1 file changed, 13 insertions(+), 39 deletions(-) diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh index f5ea3f8..c03a65e 100755 --- a/t/t4205-log-pretty-formats.sh +++ b/t/t4205-log-pretty-formats.sh @@ -144,9 +144,7 @@ test_expect_success 'setup more commits' ' ' test_expect_success 'left alignment formatting' ' - git log --pretty="format:%<(40)%s" >actual && - # complete the incomplete line at the end - echo >>actual && + git log --pretty="tformat:%<(40)%s" >actual && qz_to_tab_spaceactual && - # complete the incomplete line at the end - echo >>actual && + git log --pretty="tformat:%h %<|(40)%s" >actual && qz_to_tab_space actual && - # complete the incomplete line at the end - echo >>actual && + git log --pretty="tformat:%<(1)%s" >actual && cat actual && - # complete the incomplete line at the end - echo >>actual && + git log --pretty="tformat:%<(10,trunc)%s" >actual && qz_to_tab_space actual && - # complete the incomplete line at the end - echo >>actual && + git log --pretty="tformat:%<(10,ltrunc)%s" >actual && qz_to_tab_space actual && - # complete the incomplete line at the end - echo >>actual && + git log --pretty="tformat:%<(10,mtrunc)%s" >actual && qz_to_tab_space actual && - # complete the incomplete line at the end - echo >>actual && + git log --pretty="tformat:%>(40)%s" >actual && qz_to_tab_space actual && - # complete the incomplete line at the end - echo >>actual && + git log --pretty="tformat:%h %>|(40)%s" >actual && qz_to_tab_space actual && - # complete the incomplete line at the end - echo >>actual && + git log --pretty="tformat:%>(1)%s" >actual && cat actual && - # complete the incomplete line at the end - echo >>actual && + git log --pretty="tformat:%><(40)%s" >actual && qz_to_tab_space actual && - # complete the incomplete line at the end - echo >>actual && + git log --pretty="tformat:%h %><|(40)%s" >actual && qz_to_tab_space actual && - # complete the incomplete line at the end - echo >>actual && + git log --pretty="tformat:%><(1)%s" >actual && cat
[PATCH v3 5/5] pretty.c: format string with truncate respects logOutputEncoding
Pretty format string %<(N,[ml]trunc)>%s truncates subject to a given
length with an appropriate padding. This works for non-ASCII texts when
i18n.logOutputEncoding is UTF-8 only (independently of a printed commit
message encoding) but does not work when i18n.logOutputEncoding is NOT
UTF-8.
In 7e77df3 (pretty: two phase conversion for non utf-8 commits, 2013-04-19)
'format_commit_item' function assumes commit message to be in UTF-8.
And that was so until ecaee80 (pretty: --format output should honor
logOutputEncoding, 2013-06-26) where conversion to logOutputEncoding was
added before calling 'format_commit_message'.
Correct this by converting a commit message to UTF-8 first (as it
assumed in 7e77df3 (pretty: two phase conversion for non utf-8 commits,
2013-04-19)). Only after that convert a commit message to an actual
logOutputEncoding.
Signed-off-by: Alexey Shumkin
Reviewed-by: Nguyễn Thái Ngọc Duy
---
pretty.c | 7 ++-
t/t4205-log-pretty-formats.sh | 8
t/t6006-rev-list-format.sh| 6 +++---
3 files changed, 13 insertions(+), 8 deletions(-)
diff --git a/pretty.c b/pretty.c
index 6e266dd..25e8825 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1507,13 +1507,18 @@ void format_commit_message(const struct commit *commit,
context.commit = commit;
context.pretty_ctx = pretty_ctx;
context.wrap_start = sb->len;
+ /*
+* convert a commit message to UTF-8 first
+* as far as 'format_commit_item' assumes it in UTF-8
+*/
context.message = logmsg_reencode(commit,
&context.commit_encoding,
- output_enc);
+ utf8);
strbuf_expand(sb, format, format_commit_item, &context);
rewrap_message_tail(sb, &context, 0, 0, 0);
+ /* then convert a commit message to an actual output encoding */
if (output_enc) {
if (same_encoding(utf8, output_enc))
output_enc = NULL;
diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh
index 74babce..c84ec9a 100755
--- a/t/t4205-log-pretty-formats.sh
+++ b/t/t4205-log-pretty-formats.sh
@@ -220,7 +220,7 @@ EOF
test_cmp expected actual
'
-test_expect_failure 'left alignment formatting with trunc.
i18n.logOutputEncoding' '
+test_expect_success 'left alignment formatting with trunc.
i18n.logOutputEncoding' '
git -c i18n.logOutputEncoding=$test_encoding log
--pretty="tformat:%<(10,trunc)%s" >actual &&
qz_to_tab_space actual &&
qz_to_tab_space actual &&
qz_to_tab_space actual &&
cat
[PATCH v3 4/5] t4205, t6006: Add failing tests for the case when i18n.logOutputEncoding is set
Pretty format string %<(N,[ml]trunc)>%s truncates subject to a given length with an appropriate padding. This works for non-ASCII texts when i18n.logOutputEncoding is UTF-8 only (independently of a printed commit message encoding) but does not work when i18n.logOutputEncoding is NOT UTF-8. There were no breakages as far as were no tests for the case when both a commit message and logOutputEncoding are not UTF-8. Add failing tests for that which will be fixed in the next patch. Signed-off-by: Alexey Shumkin Reviewed-by: Nguyễn Thái Ngọc Duy --- t/t4205-log-pretty-formats.sh | 140 ++ t/t6006-rev-list-format.sh| 75 +- 2 files changed, 213 insertions(+), 2 deletions(-) diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh index c03a65e..74babce 100755 --- a/t/t4205-log-pretty-formats.sh +++ b/t/t4205-log-pretty-formats.sh @@ -154,6 +154,17 @@ EOF test_cmp expected actual ' +test_expect_success 'left alignment formatting. i18n.logOutputEncoding' ' + git -c i18n.logOutputEncoding=$test_encoding log --pretty="tformat:%<(40)%s" >actual && + qz_to_tab_spaceactual && qz_to_tab_space actual && + qz_to_tab_space actual && cat actual && + cat actual && qz_to_tab_space actual && + qz_to_tab_space actual && qz_to_tab_space actual && + qz_to_tab_space actual && qz_to_tab_space actual && + qz_to_tab_space actual && qz_to_tab_space actual && + qz_to_tab_space actual && qz_to_tab_space
[PATCH v3 2/5] t4041, t4205, t6006, t7102: Don't hardcode tested encoding value
The tested encoding is always available in a variable. Use it instead of
hardcoding. Also, to be in line with other tests use ISO8859-1
(uppercase) rather then iso8895-1.
Signed-off-by: Alexey Shumkin
---
t/t4041-diff-submodule-option.sh | 7 +--
t/t4205-log-pretty-formats.sh| 11 +++
t/t6006-rev-list-format.sh | 35 +++
t/t7102-reset.sh | 13 -
4 files changed, 39 insertions(+), 27 deletions(-)
diff --git a/t/t4041-diff-submodule-option.sh b/t/t4041-diff-submodule-option.sh
index 1751c83..463d63b 100755
--- a/t/t4041-diff-submodule-option.sh
+++ b/t/t4041-diff-submodule-option.sh
@@ -11,6 +11,9 @@ This test tries to verify the sanity of the --submodule
option of git diff.
. ./test-lib.sh
+# Tested non-UTF-8 encoding
+test_encoding="ISO8859-1"
+
# String "added" in German (translated with Google Translate), encoded in
UTF-8,
# used in sample commit log messages in add_file() function below.
added=$(printf "hinzugef\303\274gt")
@@ -23,8 +26,8 @@ add_file () {
echo "$name" >"$name" &&
git add "$name" &&
test_tick &&
- msg_added_iso88591=$(echo "Add $name ($added $name)" |
iconv -f utf-8 -t iso8859-1) &&
- git -c 'i18n.commitEncoding=iso8859-1' commit -m
"$msg_added_iso88591"
+ msg_added_iso88591=$(echo "Add $name ($added $name)" |
iconv -f utf-8 -t $test_encoding) &&
+ git -c "i18n.commitEncoding=$test_encoding" commit -m
"$msg_added_iso88591"
done >/dev/null &&
git rev-parse --short --verify HEAD
)
diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh
index f9f33ae..f5ea3f8 100755
--- a/t/t4205-log-pretty-formats.sh
+++ b/t/t4205-log-pretty-formats.sh
@@ -7,6 +7,9 @@
test_description='Test pretty formats'
. ./test-lib.sh
+# Tested non-UTF-8 encoding
+test_encoding="ISO8859-1"
+
sample_utf8_part=$(printf "f\303\244ng")
commit_msg () {
@@ -27,8 +30,8 @@ test_expect_success 'set up basic repos' '
>bar &&
git add foo &&
test_tick &&
- git config i18n.commitEncoding iso8859-1 &&
- git commit -m "$(commit_msg iso8859-1)" &&
+ git config i18n.commitEncoding $test_encoding &&
+ git commit -m "$(commit_msg $test_encoding)" &&
git add bar &&
test_tick &&
git commit -m "add bar" &&
@@ -56,8 +59,8 @@ test_expect_success 'alias user-defined format' '
test_cmp expected actual
'
-test_expect_success 'alias user-defined tformat with %s (iso8859-1 encoding)' '
- git config i18n.logOutputEncoding iso8859-1 &&
+test_expect_success 'alias user-defined tformat with %s (ISO8859-1 encoding)' '
+ git config i18n.logOutputEncoding $test_encoding &&
git log --oneline >expected-s &&
git log --pretty="tformat:%h %s" >actual-s &&
git config --unset i18n.logOutputEncoding &&
diff --git a/t/t6006-rev-list-format.sh b/t/t6006-rev-list-format.sh
index 9874403..9e4ba62 100755
--- a/t/t6006-rev-list-format.sh
+++ b/t/t6006-rev-list-format.sh
@@ -9,19 +9,22 @@ test_description='git rev-list --pretty=format test'
. "$TEST_DIRECTORY"/lib-terminal.sh
test_tick
+# Tested non-UTF-8 encoding
+test_encoding="ISO8859-1"
+
# String "added" in German
# (translated with Google Translate),
# encoded in UTF-8, used as a commit log message below.
added=$(printf "added (hinzugef\303\274gt) foo")
-added_iso88591=$(echo "$added" | iconv -f utf-8 -t iso8859-1)
+added_iso88591=$(echo "$added" | iconv -f utf-8 -t $test_encoding)
# same but "changed"
changed=$(printf "changed (ge\303\244ndert) foo")
-changed_iso88591=$(echo "$changed" | iconv -f utf-8 -t iso8859-1)
+changed_iso88591=$(echo "$changed" | iconv -f utf-8 -t $test_encoding)
test_expect_success 'setup' '
: >foo &&
git add foo &&
- git config i18n.commitEncoding iso8859-1 &&
+ git config i18n.commitEncoding $test_encoding &&
git commit -m "$added_iso88591" &&
head1=$(git rev-parse --verify HEAD) &&
head1_short=$(git rev-parse --verify --short $head1) &&
@@ -124,9 +127,9 @@ EOF
test_format encoding %e < commit-msg < commit-msg expect
[PATCH v3 1/5] t4205 (log-pretty-formats): don't hardcode SHA-1 in expected outputs
The expected SHA-1 digests are always available in variables. Use them instead of hardcoding. That was introduced in a742f2a (t4205 (log-pretty-formats): don't hardcode SHA-1 in expected outputs, 2013-06-26) but unfortunately was not followed in 5e1361c (log: properly handle decorations with chained tags, 2013-12-17) Signed-off-by: Alexey Shumkin --- t/t4205-log-pretty-formats.sh | 14 +++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh index 2a6278b..f9f33ae 100755 --- a/t/t4205-log-pretty-formats.sh +++ b/t/t4205-log-pretty-formats.sh @@ -296,6 +296,10 @@ EOF test_cmp expected actual ' +# save HEAD's SHA-1 digest (with no abbreviations) to use it below +# as far as the next test amends HEAD +old_head1=$(git rev-parse --verify HEAD~0) + test_expect_success 'left/right alignment formatting with stealing' ' git commit --amend -m short --author "long long long " && git log --pretty="format:%<(10,trunc)%s%>>(10,ltrunc)% an" >actual && @@ -310,6 +314,10 @@ EOF test_cmp expected actual ' +# get new digests (with no abbreviations) +head1=$(git rev-parse --verify HEAD~0) && +head2=$(git rev-parse --verify HEAD~1) && + test_expect_success 'log decoration properly follows tag chain' ' git tag -a tag1 -m tag1 && git tag -a tag2 -m tag2 tag1 && @@ -317,9 +325,9 @@ test_expect_success 'log decoration properly follows tag chain' ' git commit --amend -m shorter && git log --no-walk --tags --pretty="%H %d" --decorate=full >actual && catactual1 && test_cmp expected actual1 -- 1.9.2-15 -- 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 v3 0/5] Reroll patches. Pretty print truncate does not work
This patch series differs from the previous (v2) version in the following: t4205 refactored: hardcoded SHA-1 digests replaced with variabled Tested encoding `iso8859-1` replaced with variable (and uppercased to be in line in other tests) Usage of `format` + `echo` replaced with `tformat` brian m. carlson CCed as he committed 5e1361c (log: properly handle decorations with chained tags, 2013-12-17) Alexey Shumkin (5): t4205 (log-pretty-formats): don't hardcode SHA-1 in expected outputs t4041, t4205, t6006, t7102: Don't hardcode tested encoding value t4205 (log-pretty-format): Use `tformat` rather than `format` t4205, t6006: Add failing tests for the case when i18n.logOutputEncoding is set pretty.c: format string with truncate respects logOutputEncoding pretty.c | 7 +- t/t4041-diff-submodule-option.sh | 7 +- t/t4205-log-pretty-formats.sh| 217 ++- t/t6006-rev-list-format.sh | 110 t/t7102-reset.sh | 13 ++- 5 files changed, 282 insertions(+), 72 deletions(-) -- 1.9.2-15 -- 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: Returning error message from custom smart http server
On Mon, May 19, 2014 at 06:12:10PM +1000, Bryan Turner wrote: > For starters, to return an error message, your status must be 200 OK. > You can't return any other status code or Git will interpret your > error as some form of _HTTP_ error rather than a _git_ error. As of git v1.8.3, git will show text/plain content sent along with a a non-200 HTTP code. However, it does this _only_ for the initial refs fetch (along with several other error-reporting niceties, including specifically handling HTTP 401s). The thinking was that the interesting smart-http errors happen on that initial contact (e.g., failure to login, access denied, etc). Errors at the HTTP level that happen later during POST requests mean that the server is misconfigured or broken somehow, and should be rare. That's the theory anyway. In the original poster's example, it looks like the server is rejecting the push with an HTTP 401 during the POST call, after the initial ref advertisement. This is non-ideal, because it means the client may have gone to significant work to generate the packfile. It should instead reject it as soon as it sees a request for ".../info/refs?service=git-receive-pack". Current git clients will prompt for errors, and will also show the text/plain content. > - Set the content type to "application/x--advertisement" > (e.g. "application/x-git-receive-pack-advertisement") (Not all command > line Git versions require this, but JGit does) A side note, but command-line Git cares about the content-type since v1.8.1.5. > [...how git's ERR lines work...] Your description seemed accurate from my brief read. Sending ERR lines goes back much further. However, for a 401, I think they really want to send the HTTP code (and at the right time), so that the client can recognize this, gather credentials from the user, and try again. -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
[BUG] git push writes to stderr instead of stdout on success
`git push` writes to stderr instead of stdout Steps to reproduce: mkdir foo cd foo git init echo foo > fo git add -A git commit -am "foo" git push . HEAD:refs/heads/newbranch 2>out Expected: `out` file is empty Actual: `out` file contents: To . * [new branch] HEAD -> newbranch -- 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 showing tags in logs when it shouldn't
On Mon, May 19, 2014 at 08:17:07AM -0500, Robert Dailey wrote: > I have run the following command: > > log --graph --abbrev-commit --decorate --date=relative > --format=format:'%C(bold blue)%h%C(reset)%x09%C(bold > green)(%ar)%C(reset)%C(bold yellow)%d%C(reset) %C(dim > white)%an%C(reset) - %C(white)%s%C(reset)' > > After running this, I am seeing tag labels in the log graph. I believe > these shouldn't be showing because the docs for --decorate state that > refs/tags/ should not show. I'm also obviously not specifying --tags > > Is this a bug? If not, what changes do I need to make? I only want to > see branch labels, nothing else. I assume the docs you are referring to are (from "git help log"): --no-decorate, --decorate[=short|full|no] Print out the ref names of any commits that are shown. If short is specified, the ref name prefixes refs/heads/, refs/tags/ and refs/remotes/ will not be printed. If full is specified, the full ref name (including prefix) will be printed. The default option is short. The intent of that is to say that if we are in "short" mode, then for any ref names we show, we will show them without their "refs/heads", "refs/tags", or "refs/remotes" prefixes. Not that we will omit those refs entirely (if you omitted those three, you would generally have nothing left). So no, it is not a bug, the code is working as intended. Patches welcome for rewriting the documentation to make this more clear (I thought it was pretty clear, but I believe you are the second person in the last few weeks to misread it in exactly the same way, so it is not just you). I don't think there is currently a way to specify exactly what you want. Off the top of my head, it would be something like a "--decorate-refs=" option, which lets you specify which refs are interesting (right now, we always decorate _all_ refs). But that does not yet exist, and somebody would have to volunteer to implement it. -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: Misspelled directory
On Mon, May 19, 2014 at 03:05:42PM +0200, Alexandre Badez wrote: > The '.' directory is not the same as the root directory; you can see > that the /README.md and /./README.md are different. Right. Inside git trees, "." as an entry name does not have any special meaning. However, because it does have meaning in filesystems, git itself will never create such an entry, and "git fsck" will warn about it. > I've reported the bug to stackedit ( > https://github.com/benweet/stackedit/issues/405 > ) who ignore it (not their fault). GitHub normally blocks objects that do not pass "fsck" from being pushed, but there are some loopholes when going through the API, which stackedit does. I'll bring this one to the attention of the GitHub API developers. However, the end result will probably be GitHub rejecting the API request from stackedit to create the bogus tree. So stackedit may end up wanting to adjust their code to handle the situation more gracefully anyway. -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
[PATCH] Windows: Allow using UNC path for git repository
From: Cezary Zawadka
Date: Tue, 13 Jul 2010 16:17:43 +0200
[efl: moved MinGW-specific part to compat/]
[jes: fixed compilation on non-Windows]
Eric Sunshine fixed mingw_offset_1st_component() to return consistently "foo"
for UNC "//machine/share/foo", cf
http://groups.google.com/group/msysgit/browse_thread/thread/c0af578549b5dda0
Author: Eric Sunshine
Signed-off-by: Cezary Zawadka
Signed-off-by: Eric Sunshine
Signed-off-by: Erik Faye-Lund
Signed-off-by: Johannes Schindelin
Signed-off-by: Stepan Kasal
---
Hello,
this is another patch that lived in msysGit for years, at least from
Jul 13, 2010. It was there in two parts, first sketch by Cezary and
a fix from Eric Sunshine, but I decided to submit the combined
version.
Let me note that this patch should not affect any non-Windows
platform. The chnage of offset_1st_component() to a simple macro is
ok, because has_dos_drive_prefix() is 0 there.
Regards,
Stepan
cache.h | 1 -
compat/mingw.c| 24
compat/mingw.h| 2 ++
git-compat-util.h | 4
path.c| 7 ---
5 files changed, 30 insertions(+), 8 deletions(-)
diff --git a/cache.h b/cache.h
index ebe9a40..0961fb5 100644
--- a/cache.h
+++ b/cache.h
@@ -781,7 +781,6 @@ int normalize_path_copy(char *dst, const char *src);
int longest_ancestor_length(const char *path, struct string_list *prefixes);
char *strip_path_suffix(const char *path, const char *suffix);
int daemon_avoid_alias(const char *path);
-int offset_1st_component(const char *path);
/* object replacement */
#define LOOKUP_REPLACE_OBJECT 1
diff --git a/compat/mingw.c b/compat/mingw.c
index e9892f8..a0e13bc 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -1823,3 +1823,27 @@ pid_t waitpid(pid_t pid, int *status, int options)
errno = EINVAL;
return -1;
}
+
+int mingw_offset_1st_component(const char *path)
+{
+ int offset = 0;
+ if (has_dos_drive_prefix(path))
+ offset = 2;
+
+ /* unc paths */
+ else if (is_dir_sep(path[0]) && is_dir_sep(path[1])) {
+
+ /* skip server name */
+ char *pos = strpbrk(path + 2, "\\/");
+ if (!pos)
+ return 0; /* Error: malformed unc path */
+
+ do {
+ pos++;
+ } while (*pos && !is_dir_sep(*pos));
+
+ offset = pos - path;
+ }
+
+ return offset + is_dir_sep(path[offset]);
+}
diff --git a/compat/mingw.h b/compat/mingw.h
index e033e72..3eaf822 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -339,6 +339,8 @@ static inline char *mingw_find_last_dir_sep(const char
*path)
return ret;
}
#define find_last_dir_sep mingw_find_last_dir_sep
+int mingw_offset_1st_component(const char *path);
+#define offset_1st_component mingw_offset_1st_component
#define PATH_SEP ';'
#define PRIuMAX "I64u"
#define PRId64 "I64d"
diff --git a/git-compat-util.h b/git-compat-util.h
index d493a8c..ec41cfb 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -270,6 +270,10 @@ extern char *gitbasename(char *);
#define has_dos_drive_prefix(path) 0
#endif
+#ifndef offset_1st_component
+#define offset_1st_component(path) (is_dir_sep((path)[0]))
+#endif
+
#ifndef is_dir_sep
#define is_dir_sep(c) ((c) == '/')
#endif
diff --git a/path.c b/path.c
index f9c5062..bc804a3 100644
--- a/path.c
+++ b/path.c
@@ -823,10 +823,3 @@ int daemon_avoid_alias(const char *p)
}
}
}
-
-int offset_1st_component(const char *path)
-{
- if (has_dos_drive_prefix(path))
- return 2 + is_dir_sep(path[2]);
- return is_dir_sep(path[0]);
-}
--
1.9.2.msysgit.0.490.ga07b726
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Git showing tags in logs when it shouldn't
I have run the following command: log --graph --abbrev-commit --decorate --date=relative --format=format:'%C(bold blue)%h%C(reset)%x09%C(bold green)(%ar)%C(reset)%C(bold yellow)%d%C(reset) %C(dim white)%an%C(reset) - %C(white)%s%C(reset)' After running this, I am seeing tag labels in the log graph. I believe these shouldn't be showing because the docs for --decorate state that refs/tags/ should not show. I'm also obviously not specifying --tags Is this a bug? If not, what changes do I need to make? I only want to see branch labels, nothing else. -- 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
Misspelled directory
Hi, Today I've issue a problem with git. I've start a little project on github ( https://github.com/Djabx/mgd ). And I used https://stackedit.io/ for editing my README.md file. When I publish it on my repo (stackedit), I've set the destination file path to "./README.md". The problem is, that Git commit and create a '.' directory. Visible here: https://github.com/Djabx/mgd/tree/15a0ec6aee0ae08764623a304e3fc5ce96cef821 The '.' directory is not the same as the root directory; you can see that the /README.md and /./README.md are different. I've reported the bug to stackedit ( https://github.com/benweet/stackedit/issues/405 ) who ignore it (not their fault). Asked about it on stackoverflow: https://stackoverflow.com/questions/23734960/how-to-delete-a-misspelled-directory And finally found a work around. My question is, is it really "normal" that a '.' directory is accepted by git ? Shouldn't it be a "special" case ? Thanks for your time. -- Alex -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] replace: add --graft option
On Sun, May 18, 2014 at 08:29:38PM +0200, Christian Couder wrote:
> +static int create_graft(int argc, const char **argv, int force)
> +{
> + unsigned char old[20], new[20];
> + const char *old_ref = argv[0];
> + struct strbuf buf = STRBUF_INIT;
> + struct strbuf new_parents = STRBUF_INIT;
> + const char *parent_start, *parent_end;
> + int i;
> +
> + if (get_sha1(old_ref, old) < 0)
> + die(_("Not a valid object name: '%s'"), old_ref);
> + lookup_commit_or_die(old, old_ref);
> + if (read_sha1_commit(old, &buf))
> + die(_("Invalid commit: '%s'"), old_ref);
Do we want to peel to commits here? That is, should:
git replace --graft v1.5.0 v1.4.0
work? On the one hand, I see it as friendly. On the other, it may be a
bit surprising when working with something as potentially dangerous a
replace refs. If we don't do it automatically, the user can still say
"v1.5.0^{commit}" to be explicit. I dunno; maybe I am being overly
paranoid.
> + /* prepare new parents */
> + for (i = 1; i < argc; i++) {
> + unsigned char sha1[20];
> + if (get_sha1(argv[i], sha1) < 0)
> + die(_("Not a valid object name: '%s'"), argv[i]);
> + lookup_commit_or_die(sha1, argv[i]);
> + strbuf_addf(&new_parents, "parent %s\n", sha1_to_hex(sha1));
> + }
Either way, I think _this_ peeling is a sane thing to do.
-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: [RFC/PATCH] replace: add --graft option
On Mon, May 19, 2014 at 11:42:07AM +0200, Michael Haggerty wrote:
> On 05/18/2014 08:29 PM, Christian Couder wrote:
> > The usage string for this option is:
> >
> > git replace [-f] --graft [...]
> >
> > First we create a new commit that is the same as
> > except that its parents are [...]
> >
> > Then we create a replace ref that replace with
> > the commit we just created.
> >
> > With this new option, it should be straightforward to
> > convert grafts to replace refs, with something like:
> >
> > cat .git/info/grafts | while read line
> > do git replace --graft $line; done
>
> I love the functionality; I think it's a great step towards making
> grafts obsolete.
Me too.
> I haven't worked with Git's object reading/writing code much, but it
> surprised me that you are editing the commit object basically as a
> string, using hard-coded length constants and stuff. It seems
> error-prone, and we already have a commit parser.
I think we have at least two commit parsers already. :)
The one in commit.c:parse_commit_buffer tries hard to be fast and only
get out enough information for a traversal (so parents, commit
timestamp, and tree pointer).
The ones in pretty.c that back format_commit_message
(parse_commit_header, parse_commit_message) are a bit more flexible, as
they record the offsets and lengths of certain key lines. But "parents"
is not one of those lines (we rely on the already-parsed copy in the
"struct commit" for parents and tree information).
It might make sense to just teach parse_commit_header to be a little
more thorough; it has to read past those lines anyway to find the author
and committer lines, so it would not be much more expensive to note
them. And then of course the code needs to be pulled out of the
pretty-printer and made generally accessible.
While I do like the thought of having a decent reusable commit parser,
this particular case is really about trying to tweak one header, without
touching anything else. IOW, it is basically:
sed '/^parent /d; /^tree /a\
parent ...\
parent ...
'
That's more or less what Christian's function is doing, though it
assumes things like that the parents come immediately after the tree,
and that they are all in a group. Those are all true for objects created
by git, but I think we can be a little flexible.
It turns out I wrote a proof-of-concept of this in March when we had the
original discussion, but forgot to ever send it to the list. My parsing
function looked like:
/*
* Rewrite the commit object found in "buf", removing any existing
* parents and adding lines for any parents in the NULL-terminated
* array "parents" (whose strings should be 40-char hex sha1s).
*
* The output is written to the strbuf "out".
*/
static void rewrite_parents(struct strbuf *out, const char *buf, const
char **parents)
{
int added = 0;
while (buf && *buf) {
const char *eol = strchrnul(buf, '\n');
const char *next = *eol ? eol + 1 : eol;
if (eol == buf + 1)
break;
if (!starts_with(buf, "parent "))
strbuf_add(out, buf, next - buf);
/*
* We always put our parent lines right after the tree
line,
* which matches how git creates commit objects.
*/
if (starts_with(buf, "tree ")) {
const char **p;
if (added++)
die("commit has two tree lines?");
for (p = parents; *p; p++)
strbuf_addf(out, "parent %s\n", *p);
}
buf = next;
}
if (!added)
die("commit has no tree line?");
strbuf_addstr(out, buf);
}
Rather than a splice, I did it as I copied into the strbuf, and I
treated each line independently (dropping parent lines, no matter where,
and then finding the tree line as the "proper" place to add new parent
lines).
-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 v4] format-patch --signature-file
On Sun, May 18, 2014 at 01:22:02PM -0700, Jeremiah Mahler wrote:
> @@ -742,6 +743,8 @@ static int git_format_config(const char *var, const char
> *value, void *cb)
> }
> if (!strcmp(var, "format.signature"))
> return git_config_string(&signature, var, value);
> + if (!strcmp(var, "format.signaturefile"))
> + return git_config_string(&signature_file, var, value);
Should this be git_config_pathname? That would handle tilde-expansion.
> +test_expect_success 'format-patch --signature-file=file' '
> + git format-patch --stdout --signature-file=expect -1 >output &&
> + check_patch output &&
> + sed -n "/^-- $/,\$p" >output2 &&
> + test_cmp expect output2
> +'
I don't think --lines is portable (it's not even POSIX), nor is a
negative value for head (but a "+" form for tail is). You can replace
the "head" call with a "$d" sed command, and tail should be fine with
"-n +2". Like:
sed -n '$d; /^-- $/,$p' | tail -n +2
You can also do it in one sed like:
sed -n '$d; /^-- $/,${//!p}'
but that is starting to look a bit like line-noise. ;)
> +test_expect_success 'format-patch with format.signaturefile config' '
> + git config format.signaturefile expect &&
> + git format-patch --stdout -1 >output &&
> + check_patch output &&
> + sed -n "/^-- $/,\$p" >output2 &&
> + test_cmp expect output2 &&
> + git config --unset-all format.signaturefile
> +'
You might want to use "test_config format.signaturefile expect" here,
which handles unsetting after the test. Besides being one line shorter,
it also cleans up when the middle part of the test fails.
You could also use "git -c", but I think setting the actual config in a
file is a more realistic test.
-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: [ANNOUNCE] git related v0.3
On Mon, May 19, 2014 at 2:36 AM, Felipe Contreras wrote: > This tool finds people that might be interested in a patch, by going > back through the history for each single hunk modified, and finding > people that reviewed, acknowledged, signed, or authored the code the > patch is modifying. > > It does this by running `git blame` incrementally on each hunk, and > finding the relevant commit message. After gathering all the relevant > people, it groups them to show what exactly was their role when the > participated in the development of the relevant commit, and on how many > relevant commits they participated. They are only displayed if they pass > a minimum threshold of participation. > > It is similar the the `git contacts` tool in the contrib area, which is a > rewrite of this tool, except that `git contacts` does the absolute minimum; > `git related` is way superior in every way. The general heuristic I use, which I've found to be much better than git-blame is: 1. Find substrings of code I'm directly removing/altering, and functions I'm removing/altering 2. Do git log --reverse -p -S'' (maybe with -- file) for a list of substrings I've generally found that to be a better heuristic to start with in both git.git and non-git.git code, blame tends to bias the view towards giving you people who've just moved the code around or made minor changes (are you at least using blame -w?). We recently discussed having a tool like this at work to aid in our review process, but I pointed out there that you had to be careful with how it was written, e.g. if you rank importance as a function of the number of commits you're now going to bother people more with review requests if they make granular commits, whereas what you actually want is to contact the "significant" authors, which generally speaking can be defined as the original authors of the code you're altering or replacing. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Watchman support for git
On Fri, May 16, 2014 at 2:42 AM, David Turner wrote: >> I assume you won't change your mind about this. Which is fine to me. I >> will still try out my approach with your libwatchman though. Just >> curious about its performance and complexity, compared to your >> approach. > > I am open-minded here. This code is really the first time I have looked > at git's internals, and I accept that your way might be better. If > you're going to try the watchman version of your approach, then we do a > direct comparison. Let me know if there is something I can do to help > out on that. You already helped by publishing your patches (and letting me know about libwatchman) so I can steal bits here and there ;-) >> A bit off topic, but msys fork has another "fscache" in compat/win32. >> If you could come up with a different name, maybe it'll be less >> confusing for them after merging. But this is not a big deal, as this >> fscache won't work on windows anyway. > > Does wtcache sounds like a better name? > Heh i'm bad at naming. But that sounds a bit cryptic. Maybe watchman-cache.c (with funtions starting with prefix wmc_)? Should not worry too much about this though, unless some msysgit guys yell up. -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Problem: staging of an alternative repository
On Sat, May 17, 2014 at 11:31 PM, Pasha Bolokhov wrote: > Now if you guys don't see anything against this, I would shoot out a > patch? > If you have written the patch already, I see no harm in sending it here. I'm concerned about the perfomance impact on this code, which is already slow when the repo is large. But we can benchmark it later. -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [msysGit] Re: [PATCH v2] config: preserve config file permissions on edits
On Mon, May 19, 2014 at 9:44 AM, Erik Faye-Lund wrote:
> On Mon, May 19, 2014 at 9:13 AM, Johannes Sixt wrote:
>> Am 5/6/2014 2:17, schrieb Eric Wong:
>>> Users may already store sensitive data such as imap.pass in
>>> ..git/config; making the file world-readable when "git config"
>>> is called to edit means their password would be compromised
>>> on a shared system.
>>>
>>> [v2: updated for section renames, as noted by Junio]
>>>
>>> Signed-off-by: Eric Wong
>>> ---
>>> config.c | 16
>>> t/t1300-repo-config.sh | 10 ++
>>> 2 files changed, 26 insertions(+)
>>>
>>> diff --git a/config.c b/config.c
>>> index a30cb5c..c227aa8 100644
>>> --- a/config.c
>>> +++ b/config.c
>>> @@ -1636,6 +1636,13 @@ int git_config_set_multivar_in_file(const char
>>> *config_filename,
>>> MAP_PRIVATE, in_fd, 0);
>>> close(in_fd);
>>>
>>> + if (fchmod(fd, st.st_mode & 0) < 0) {
>>> + error("fchmod on %s failed: %s",
>>> + lock->filename, strerror(errno));
>>> + ret = CONFIG_NO_WRITE;
>>> + goto out_free;
>>> + }
>>
>> This introduces a severe failure in the Windows port because we do not
>> implement fchmod. Existing fchmod invocations do not check the return
>> value, and they are only interested in removing the write bits, and we
>> generally don't care a lot if files remain writable.
>>
>> I'm not proficient enough to add any ACL fiddling to fchmod that would be
>> required by the above change, whose purpose is to be strict about
>> permissions. Nor am I interested (who the heck is sharing a Windows box
>> anyway? ;-)
>>
>> Therefore, here's just a work-around patch to keep things going on
>> Windows. Any opinions from the Windows corner?
>>
>
> Since we use MSVCRT's chmod implementation directly which only checks
> for S_IWRITE,and Get/SetFileAttributes to simply set or clear the
> FILE_ATTRIBUTE_READONLY-flag, perhaps we could do the same except
> using Get/SetFileInformationByHandle instead? That takes us in a
> better direction, IMO. Adding full ACL support seems like a bigger
> project.
>
> If there's no objection, I'll sketch up a patch for that...
OK, this turned out a bit more yucky than I assumed, because
SetFileInformationByHandle is only available on Windows Vista and up.
There's a static library (FileExtd.lib) that ships with MSVC that
emulates it for legacy support, I guess I should have a look at what
that does.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] replace: add --graft option
On 05/18/2014 08:29 PM, Christian Couder wrote: > The usage string for this option is: > > git replace [-f] --graft [...] > > First we create a new commit that is the same as > except that its parents are [...] > > Then we create a replace ref that replace with > the commit we just created. > > With this new option, it should be straightforward to > convert grafts to replace refs, with something like: > > cat .git/info/grafts | while read line > do git replace --graft $line; done I love the functionality; I think it's a great step towards making grafts obsolete. I haven't worked with Git's object reading/writing code much, but it surprised me that you are editing the commit object basically as a string, using hard-coded length constants and stuff. It seems error-prone, and we already have a commit parser. Would it be possible to program this at a higher layer of abstraction based on the commit object produced by the existing commit parser? E.g., edit the object it produces, and write the result? Or create a new commit object out of the parsed commit object and write that? It's great that you're working on this! Michael -- Michael Haggerty mhag...@alum.mit.edu http://softwareswirl.blogspot.com/ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Returning error message from custom smart http server
On Mon, 2014-05-19 at 18:12 +1000, Bryan Turner wrote: > On Sat, May 17, 2014 at 9:01 AM, brian m. carlson > wrote: > > On Tue, May 13, 2014 at 09:39:59AM +0200, "Ákos, Tajti" wrote: > >> Dear List, > >> > >> we implemented our own git smart http server to be able to check > >> permissions > >> and other thing before pushes. It works fine, however, the error messages > >> we > >> generate on the server side are not displayed by the command line client. > >> On > >> the server we generate error messages like this: > >> > >> response.setStatus(HttpServletResponse.SC_UNAUTHORIZED); > >> response.getWriter().write(msg); > >> > >> On the command line we get this: > >> > >> Total 0 (delta 0), reused 0 (delta 0) > >> POST git-receive-pack (290 bytes) > >> efrror: RPC failed; result=22, HTTP code = 401 > >> atal: The remote end hung up unexpectedly > >> fatal: The remote end hung up unexpectedly > >> > >> The server message is completely missing. Is there a solution for this? > > You should not need a patched git; the wire protocol itself has a > mechanism for sending "smart" error messages. It's not particularly > _obvious_, but it's there. > > For starters, to return an error message, your status must be 200 OK. > You can't return any other status code or Git will interpret your > error as some form of _HTTP_ error rather than a _git_ error. > > In the smart protocol the client sends a service to the server as a > query parameter, like "?service=git-receive-pack". For such a request, > you need to: > - Set the content type to "application/x--advertisement" > (e.g. "application/x-git-receive-pack-advertisement") (Not all command > line Git versions require this, but JGit does) > - Set the status code as 200 OK > - Write back a payload where the first 4 bytes are the hex-encoded > length of the text (where "" is max length for a single packet). > Note that the 4 bytes for the size are _part_ of that length, so if > you're writing "Test" the length is 8, not 4 > - After the size, you write "# service=" (e.g. "# > service=git-receive-pack"; note the space after the #) This is the > metadata. For an error, you don't really have much to say. > - After that, an empty packet, which is "" (four zeros) This > separates the metadata from the ref advertisement > - After that you can write your message, beginning with "ERR " (note > the trailing space there). The "ERR " tells Git what you're writing > isn't a ref, it's an error. I'd recommend appending a newline (and add > 1 more to your length for it), because when Git echoes your error > message it doesn't seem to do that > > I'm not sure whether there's a document that describes all of this; I > found it by digging into the Git source code (you can find the "ERR" > handling in connect.c, get_remote_heads). This may be exploiting the > protocol, I'll leave that to someone more knowledgeable on how they > _intended_ this all to be used, but it works for us. > > A full example looks something like this: "0036# > service=git-receive-packERR This is a test\n" This is indeed documented, namely in Documentation/technical/pack-protocol.txt I guess it could do with an example, but your usage seems correct. There are two different places where things could go wrong, either in HTTP, such as authentication, or in the Git part of the request. If you return an HTTP 404, then all you're telling the client is that you couldn't find what it asked for, but that could mean either the receice-pack/upload-pack program or the repository itself. If something went wrong at the Git level, whether it's a resource problem in the server or simply that the repo doesn't exist, then ERR is the right thing to use. Particularly, we can't rely on the HTTP 404 response being anything meaningful, as it could simply be the host's default 404 page, and you don't want html flying through your terminal. Cheers, cmn -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: bug: autostash is lost after aborted rebase
[ Cc-ing Ramkumar ] Karen Etheridge writes: > scenario: > - edit some tracked files; do not add them to the index > - "git config rebase.autostash true" > - "git rebase -i HEAD~3" (an autostash will be created) > - delete the entire buffer and save/exit the editor - this will abort the > rebase > > poof, the autostash is gone (it is not reapplied) -- it must be explicitly > applied again via the SHA that was printed earlier. Indeed. Confirmed, even with pu (I thought I remembered seeing a fix on the list, but I must have mixed up with something else). -- 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
Bash completion: merge --abort
Hi all! The Git Bash completion script does not complete the `--abort` option for the `merge ` command. It correctly completes the other flags. It correctly completes the `--abort` flag for `rebase` and others. `git merge --abort` is considered another form of the merge command instead of a simple option. This could be the cause of the completion to be missed from the script. Cheers! -- 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: Returning error message from custom smart http server
On Sat, May 17, 2014 at 9:01 AM, brian m. carlson
wrote:
> On Tue, May 13, 2014 at 09:39:59AM +0200, "Ákos, Tajti" wrote:
>> Dear List,
>>
>> we implemented our own git smart http server to be able to check permissions
>> and other thing before pushes. It works fine, however, the error messages we
>> generate on the server side are not displayed by the command line client. On
>> the server we generate error messages like this:
>>
>> response.setStatus(HttpServletResponse.SC_UNAUTHORIZED);
>> response.getWriter().write(msg);
>>
>> On the command line we get this:
>>
>> Total 0 (delta 0), reused 0 (delta 0)
>> POST git-receive-pack (290 bytes)
>> efrror: RPC failed; result=22, HTTP code = 401
>> atal: The remote end hung up unexpectedly
>> fatal: The remote end hung up unexpectedly
>>
>> The server message is completely missing. Is there a solution for this?
You should not need a patched git; the wire protocol itself has a
mechanism for sending "smart" error messages. It's not particularly
_obvious_, but it's there.
For starters, to return an error message, your status must be 200 OK.
You can't return any other status code or Git will interpret your
error as some form of _HTTP_ error rather than a _git_ error.
In the smart protocol the client sends a service to the server as a
query parameter, like "?service=git-receive-pack". For such a request,
you need to:
- Set the content type to "application/x--advertisement"
(e.g. "application/x-git-receive-pack-advertisement") (Not all command
line Git versions require this, but JGit does)
- Set the status code as 200 OK
- Write back a payload where the first 4 bytes are the hex-encoded
length of the text (where "" is max length for a single packet).
Note that the 4 bytes for the size are _part_ of that length, so if
you're writing "Test" the length is 8, not 4
- After the size, you write "# service=" (e.g. "#
service=git-receive-pack"; note the space after the #) This is the
metadata. For an error, you don't really have much to say.
- After that, an empty packet, which is "" (four zeros) This
separates the metadata from the ref advertisement
- After that you can write your message, beginning with "ERR " (note
the trailing space there). The "ERR " tells Git what you're writing
isn't a ref, it's an error. I'd recommend appending a newline (and add
1 more to your length for it), because when Git echoes your error
message it doesn't seem to do that
I'm not sure whether there's a document that describes all of this; I
found it by digging into the Git source code (you can find the "ERR"
handling in connect.c, get_remote_heads). This may be exploiting the
protocol, I'll leave that to someone more knowledgeable on how they
_intended_ this all to be used, but it works for us.
A full example looks something like this: "0036#
service=git-receive-packERR This is a test\n"
Hope this helps,
Bryan Turner
>
> It does look that way. Does the following patch work for you?
>
> -- >8 --
> Subject: [PATCH] http: provide server's error message on RPC failure
>
> The server might provide a custom error message that is useful to the user.
> Provide this message to the user if HTTP RPC fails.
>
> Signed-off-by: brian m. carlson
> ---
> remote-curl.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/remote-curl.c b/remote-curl.c
> index 52c2d96..5984d35 100644
> --- a/remote-curl.c
> +++ b/remote-curl.c
> @@ -426,8 +426,8 @@ static int run_slot(struct active_request_slot *slot,
> err = run_one_slot(slot, results);
>
> if (err != HTTP_OK && err != HTTP_REAUTH) {
> - error("RPC failed; result=%d, HTTP code = %ld",
> - results->curl_result, results->http_code);
> + error("RPC failed; result=%d, HTTP code = %ld (%s)",
> + results->curl_result, results->http_code,
> curl_errorstr);
> }
>
> return err;
> -- >8 --
>
> --
> brian m. carlson / brian with sandals: Houston, Texas, US
> +1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
> OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [msysGit] Re: [PATCH v2] config: preserve config file permissions on edits
On Mon, May 19, 2014 at 9:13 AM, Johannes Sixt wrote:
> Am 5/6/2014 2:17, schrieb Eric Wong:
>> Users may already store sensitive data such as imap.pass in
>> ..git/config; making the file world-readable when "git config"
>> is called to edit means their password would be compromised
>> on a shared system.
>>
>> [v2: updated for section renames, as noted by Junio]
>>
>> Signed-off-by: Eric Wong
>> ---
>> config.c | 16
>> t/t1300-repo-config.sh | 10 ++
>> 2 files changed, 26 insertions(+)
>>
>> diff --git a/config.c b/config.c
>> index a30cb5c..c227aa8 100644
>> --- a/config.c
>> +++ b/config.c
>> @@ -1636,6 +1636,13 @@ int git_config_set_multivar_in_file(const char
>> *config_filename,
>> MAP_PRIVATE, in_fd, 0);
>> close(in_fd);
>>
>> + if (fchmod(fd, st.st_mode & 0) < 0) {
>> + error("fchmod on %s failed: %s",
>> + lock->filename, strerror(errno));
>> + ret = CONFIG_NO_WRITE;
>> + goto out_free;
>> + }
>
> This introduces a severe failure in the Windows port because we do not
> implement fchmod. Existing fchmod invocations do not check the return
> value, and they are only interested in removing the write bits, and we
> generally don't care a lot if files remain writable.
>
> I'm not proficient enough to add any ACL fiddling to fchmod that would be
> required by the above change, whose purpose is to be strict about
> permissions. Nor am I interested (who the heck is sharing a Windows box
> anyway? ;-)
>
> Therefore, here's just a work-around patch to keep things going on
> Windows. Any opinions from the Windows corner?
>
Since we use MSVCRT's chmod implementation directly which only checks
for S_IWRITE,and Get/SetFileAttributes to simply set or clear the
FILE_ATTRIBUTE_READONLY-flag, perhaps we could do the same except
using Get/SetFileInformationByHandle instead? That takes us in a
better direction, IMO. Adding full ACL support seems like a bigger
project.
If there's no objection, I'll sketch up a patch for that...
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: bug: autostash is lost after aborted rebase
> scenario: > - edit some tracked files; do not add them to the index > - "git config rebase.autostash true" > - "git rebase -i HEAD~3" (an autostash will be created) > - delete the entire buffer and save/exit the editor - this will abort the > rebase > > poof, the autostash is gone (it is not reapplied) -- it must be explicitly > applied again via the SHA that was printed earlier. Yes, I hit this often and it's annoying in "sausage making" workflows. Thanks for reporting this issue, I don't know why I didn't think of reporting it myself :) It likely affects a large portion of the users who like to set `rebase.autostash` and rebase.autosquash` in their config, but for some reason they didn't think of reporting it either. Philippe -- 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] config: preserve config file permissions on edits
Am 5/6/2014 2:17, schrieb Eric Wong:
> Users may already store sensitive data such as imap.pass in
> ..git/config; making the file world-readable when "git config"
> is called to edit means their password would be compromised
> on a shared system.
>
> [v2: updated for section renames, as noted by Junio]
>
> Signed-off-by: Eric Wong
> ---
> config.c | 16
> t/t1300-repo-config.sh | 10 ++
> 2 files changed, 26 insertions(+)
>
> diff --git a/config.c b/config.c
> index a30cb5c..c227aa8 100644
> --- a/config.c
> +++ b/config.c
> @@ -1636,6 +1636,13 @@ int git_config_set_multivar_in_file(const char
> *config_filename,
> MAP_PRIVATE, in_fd, 0);
> close(in_fd);
>
> + if (fchmod(fd, st.st_mode & 0) < 0) {
> + error("fchmod on %s failed: %s",
> + lock->filename, strerror(errno));
> + ret = CONFIG_NO_WRITE;
> + goto out_free;
> + }
This introduces a severe failure in the Windows port because we do not
implement fchmod. Existing fchmod invocations do not check the return
value, and they are only interested in removing the write bits, and we
generally don't care a lot if files remain writable.
I'm not proficient enough to add any ACL fiddling to fchmod that would be
required by the above change, whose purpose is to be strict about
permissions. Nor am I interested (who the heck is sharing a Windows box
anyway? ;-)
Therefore, here's just a work-around patch to keep things going on
Windows. Any opinions from the Windows corner?
--- >8 ---
From: Johannes Sixt
Subject: [PATCH] mingw: turn the always-failing fchmod stub into
always-succeeding
A recent change introduced new call sites of fchmod, but these new call
sites check the return value. The test suite can't get past t0001
without a dozen or so failures.
Just fake that the call was successful even though it did nothing.
Signed-off-by: Johannes Sixt
---
compat/mingw.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/compat/mingw.h b/compat/mingw.h
index c3859cc..7b2455c 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -89,7 +89,7 @@ static inline int readlink(const char *path, char *buf,
size_t bufsiz)
static inline int symlink(const char *oldpath, const char *newpath)
{ errno = ENOSYS; return -1; }
static inline int fchmod(int fildes, mode_t mode)
-{ errno = ENOSYS; return -1; }
+{ /* do nothing */ return 0; }
static inline pid_t fork(void)
{ errno = ENOSYS; return -1; }
static inline unsigned int alarm(unsigned int seconds)
--
2.0.0.rc3.1741.g0520b9e
--
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
