* [PATCH] git-svn: document useLogAuthor and addAuthorFrom config keys
From: Eric Wong @ 2016-12-11 0:06 UTC (permalink / raw)
To: Juergen Kosel; +Cc: git
In-Reply-To: <6160a58b-5f86-384f-30b5-2a3826019157@gmx.de>
Juergen Kosel <juergen.kosel@gmx.de> wrote:
> Am 05.12.2016 um 23:54 schrieb Eric Wong:
> > So, can you confirm that svn.addAuthorFrom and svn.useLogAuthor
> > config keys work and can be documented?
>
> yes, I can confirm, that adding this configuration keys works with git
> 2.1.4 work.
> I have added the config keys as follows:
>
> git config --add --bool svn.useLogAuthor true
> git config --add --bool svn.addAuthorFrom true
Thanks for testing, patch below.
> > Even better would be for you to provide a patch to the
> > documentation :) Otherwise, I can write one up sometime this week.
>
> My English is not that well. So I prefer, if you update the
> documentation :-)
No problem, my asciidoc is a bit rusty, but I think
the formatted result will be fine.
(Btw, list convention here is to reply-to-all;
it prevents vger from being a single-point-of-failure)
-------8<-------
Subject: [PATCH] git-svn: document useLogAuthor and addAuthorFrom config keys
We've always supported these config keys in git-svn,
so document them so users won't have to respecify them
on every invocation.
Reported-by: Juergen Kosel <juergen.kosel@gmx.de>
Signed-off-by: Eric Wong <e@80x24.org>
---
Documentation/git-svn.txt | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/Documentation/git-svn.txt b/Documentation/git-svn.txt
index 5f9e65b0c..9bee9b0c4 100644
--- a/Documentation/git-svn.txt
+++ b/Documentation/git-svn.txt
@@ -664,13 +664,19 @@ creating the branch or tag.
When retrieving svn commits into Git (as part of 'fetch', 'rebase', or
'dcommit' operations), look for the first `From:` or `Signed-off-by:` line
in the log message and use that as the author string.
++
+[verse]
+config key: svn.useLogAuthor
+
--add-author-from::
When committing to svn from Git (as part of 'commit-diff', 'set-tree' or 'dcommit'
operations), if the existing log message doesn't already have a
`From:` or `Signed-off-by:` line, append a `From:` line based on the
Git commit's author string. If you use this, then `--use-log-author`
will retrieve a valid author string for all commits.
-
++
+[verse]
+config key: svn.addAuthorFrom
ADVANCED OPTIONS
----------------
--
EW
^ permalink raw reply related
* Re: [RFC PATCH] send-email: allow a custom hook to prevent sending email
From: Junio C Hamano @ 2016-12-10 22:11 UTC (permalink / raw)
To: Stefan Beller; +Cc: Brandon Williams, Jeff King, git@vger.kernel.org
In-Reply-To: <CAGZ79kZKify7VLfNp3FXXvc4UOGjdXfUaspNaeW0t5+hWc+cpw@mail.gmail.com>
Stefan Beller <sbeller@google.com> writes:
> On Fri, Dec 9, 2016 at 3:52 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Stefan Beller <sbeller@google.com> writes:
>>
>>> So you are suggesting to
>>> * have the check later in the game (e.g. just after asking
>>> "Send this email? ([y]es|[n]o|[q]uit|[a]ll): " as then other information
>>> such as additional @to @cc are available.
>>
>> Yeah, probably before the loop starts asking that question for each
>> message. And hook does not necessarily need to cause the program to
>> die. The question can be reworded to "Your hook says no, but do you
>> really want to send it?",
>
> You could, but...
Yes, "does not necessarily need to die" is different from "hence you
must avoid dying". Running the hook before you start the loop to
ask for each message merely makes it possible to avoid dying.
^ permalink raw reply
* Re: [PATCH v6 01/16] Git.pm: add subroutines for commenting lines
From: Junio C Hamano @ 2016-12-10 22:09 UTC (permalink / raw)
To: Vasco Almeida
Cc: Johannes Schindelin, git, Jiang Xin,
Ævar Arnfjörð Bjarmason, Jean-Noël AVILA,
Jakub Narębski, David Aguilar
In-Reply-To: <1481364496.1993.14.camel@sapo.pt>
Vasco Almeida <vascomalmeida@sapo.pt> writes:
> I wonder why this is important when Git errors out when
> core.commentChar is set to more than 1 characters or 0 characters.
I think it should be consistent with the way core.commentchar is
treated in the rest of the system, namely this bit from config.c:
if (!strcmp(var, "core.commentchar")) {
if (!value)
return config_error_nonbool(var);
else if (!strcasecmp(value, "auto"))
auto_comment_line_char = 1;
else if (value[0] && !value[1]) {
comment_line_char = value[0];
auto_comment_line_char = 0;
} else
return error("core.commentChar should only be one character");
return 0;
}
And I think I misread this piece of code.
We only update comment_line_char from the default "#" when the
configured value is a single-byte character and we ignore incorrect
values in the configuration file. So I think the patch you sent is
correct after all.
^ permalink raw reply
* Re: git add -p with new file
From: Junio C Hamano @ 2016-12-10 22:04 UTC (permalink / raw)
To: Jeff King; +Cc: Ariel, git
In-Reply-To: <20161210085556.nwg3pbay367jqin5@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Fri, Dec 09, 2016 at 01:43:24PM -0500, Ariel wrote:
> ...
>> But it doesn't have to be that way. You could make add -p identical to add
>> without options, except the -p prompts to review diffs first.
>
> The question is whether you would annoy people using "-p" if you started
> including untracked files by default. I agree because it's inherently an
> interactive process that we can be looser with backwards compatibility.
It might be interactive, but it will be irritating that we suddenly
have to see hundreds of lines in an untracked file before we are
asked to say "no I do not want to add this file" and have to do so
for all the untracked files that happen to match the given pathspec.
It might make it less irritating if one of the interactive choices
offered in the first prompt were N that tells the command: "No,
ignore all the untracked paths", though. I dunno.
Also, because "git add -p" (no pathspec) is NOT a no-op, the
similarity Ariel sees with "git add" (no other options) does not
hold. As you kept explaining (but perhaps not successfully being
understood yet), "add -p" works like "add -u", and it will make the
command incoherent if we allowed "git add -p <path>" to add new
paths, exactly because "git add -u <path>" does not do so. Of
course that can be fixed by allowing "git add -u" to also add new
paths that match pathspec.
Of course, Ariel can vote for making "add -p" more like "add" (with
no options) than "add -u". I _think_ it is a better way to solve
the incoherence than making "add -u" to add new paths. But what it
means is that "add -p <paths>" works on both tracked and untracked
paths that match the given pathspec <paths>, and also "add -p" (no
pathspec) must become a no-op (because "add" without option and
pathspec is).
^ permalink raw reply
* [PATCHv2] git-p4: support git worktrees
From: Luke Diamand @ 2016-12-10 21:57 UTC (permalink / raw)
To: git; +Cc: viniciusalexandre, Lars Schneider, Junio C Hamano, Luke Diamand
In-Reply-To: <20161210215734.7468-1-luke@diamand.org>
git-p4 would attempt to find the git directory using
its own specific code, which did not know about git
worktrees. This caused git operations to fail needlessly.
Rework it to use "git rev-parse --git-dir" instead, which
knows about worktrees.
Signed-off-by: Luke Diamand <luke@diamand.org>
---
git-p4.py | 47 ++++++++++++++++++++++++++---------------------
t/t9800-git-p4-basic.sh | 20 ++++++++++++++++++++
2 files changed, 46 insertions(+), 21 deletions(-)
diff --git a/git-p4.py b/git-p4.py
index fd5ca52..6aa8957 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -49,6 +49,13 @@ defaultLabelRegexp = r'[a-zA-Z0-9_\-.]+$'
# Grab changes in blocks of this many revisions, unless otherwise requested
defaultBlockSize = 512
+def gitdir():
+ d = read_pipe("git rev-parse --git-dir").strip()
+ if not d or len(d) == 0:
+ return None
+ else:
+ return d
+
def p4_build_cmd(cmd):
"""Build a suitable p4 command line.
@@ -562,12 +569,6 @@ def currentGitBranch():
else:
return read_pipe(["git", "name-rev", "HEAD"]).split(" ")[1].strip()
-def isValidGitDir(path):
- if (os.path.exists(path + "/HEAD")
- and os.path.exists(path + "/refs") and os.path.exists(path + "/objects")):
- return True;
- return False
-
def parseRevision(ref):
return read_pipe("git rev-parse %s" % ref).strip()
@@ -3462,7 +3463,7 @@ class P4Sync(Command, P4UserMap):
if self.tempBranches != []:
for branch in self.tempBranches:
read_pipe("git update-ref -d %s" % branch)
- os.rmdir(os.path.join(os.environ.get("GIT_DIR", ".git"), self.tempBranchLocation))
+ os.rmdir(os.path.join(gitdir(), self.tempBranchLocation))
# Create a symbolic ref p4/HEAD pointing to p4/<branch> to allow
# a convenient shortcut refname "p4".
@@ -3678,23 +3679,27 @@ def main():
(cmd, args) = parser.parse_args(sys.argv[2:], cmd);
global verbose
verbose = cmd.verbose
+
if cmd.needsGit:
- if cmd.gitdir == None:
- cmd.gitdir = os.path.abspath(".git")
- if not isValidGitDir(cmd.gitdir):
- cmd.gitdir = read_pipe("git rev-parse --git-dir").strip()
- if os.path.exists(cmd.gitdir):
- cdup = read_pipe("git rev-parse --show-cdup").strip()
- if len(cdup) > 0:
- chdir(cdup);
-
- if not isValidGitDir(cmd.gitdir):
- if isValidGitDir(cmd.gitdir + "/.git"):
- cmd.gitdir += "/.git"
- else:
+ if cmd.gitdir:
+ os.environ["GIT_DIR"] = cmd.gitdir
+
+ # did we get a valid git dir on the command line or via $GIT_DIR?
+ if not gitdir():
die("fatal: cannot locate git repository at %s" % cmd.gitdir)
- os.environ["GIT_DIR"] = cmd.gitdir
+ else:
+ # already in a git directory?
+ if not gitdir():
+ die("fatal: not in a valid git repository")
+
+ cdup = read_pipe("git rev-parse --show-cdup").strip()
+ if len(cdup) > 0:
+ chdir(cdup);
+
+ # ensure subshells spawned in the p4 repo directory
+ # get the correct GIT_DIR
+ os.environ["GIT_DIR"] = os.path.abspath(gitdir())
if not cmd.run(args):
parser.print_help()
diff --git a/t/t9800-git-p4-basic.sh b/t/t9800-git-p4-basic.sh
index 0730f18..093e9bd 100755
--- a/t/t9800-git-p4-basic.sh
+++ b/t/t9800-git-p4-basic.sh
@@ -257,6 +257,26 @@ test_expect_success 'submit from detached head' '
)
'
+test_expect_success 'submit from worktree' '
+ test_when_finished cleanup_git &&
+ git p4 clone --dest="$git" //depot &&
+ (
+ cd "$git" &&
+ git worktree add ../worktree-test
+ ) &&
+ (
+ cd "$git/../worktree-test" &&
+ test_commit "worktree-commit" &&
+ git config git-p4.skipSubmitEdit true &&
+ git p4 submit
+ ) &&
+ (
+ cd "$cli" &&
+ p4 sync &&
+ test_path_is_file worktree-commit.t
+ )
+'
+
test_expect_success 'kill p4d' '
kill_p4d
'
--
2.8.2.703.g78b384c.dirty
^ permalink raw reply related
* [PATCHv2] git-p4 worktree support
From: Luke Diamand @ 2016-12-10 21:57 UTC (permalink / raw)
To: git; +Cc: viniciusalexandre, Lars Schneider, Junio C Hamano, Luke Diamand
Second attempt at teaching git-p4 about worktrees.
Earlier discussion here:
http://marc.info/?l=git&m=148097985622294
Git-p4 exports GIT_DIR so that when it chdirs into the
P4 client area to apply the change, git commands called
from there will work correctly.
Luke Diamand (1):
git-p4: support git worktrees
git-p4.py | 47 ++++++++++++++++++++++++++---------------------
t/t9800-git-p4-basic.sh | 20 ++++++++++++++++++++
2 files changed, 46 insertions(+), 21 deletions(-)
--
2.8.2.703.g78b384c.dirty
^ permalink raw reply
* [PATCH] branch: mark a file-local symbol with static
From: Ramsay Jones @ 2016-12-10 21:46 UTC (permalink / raw)
To: Karthik Nayak; +Cc: Junio C Hamano, GIT Mailing-list
Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
---
Hi Karthik,
If you need to re-roll your 'kn/ref-filter-branch-list' branch, could
you please squash this into the relevant patch (commit 715a4826ab,
"branch: use ref-filter printing APIs", 07-12-2016).
Thanks!
ATB,
Ramsay Jones
builtin/branch.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/builtin/branch.c b/builtin/branch.c
index 2c9aa2b29..4386273ce 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -306,7 +306,7 @@ static int calc_maxwidth(struct ref_array *refs, int remote_bonus)
return max;
}
-const char *quote_literal_for_format(const char *s)
+static const char *quote_literal_for_format(const char *s)
{
struct strbuf buf = STRBUF_INIT;
--
2.11.0
^ permalink raw reply related
* Re: BUG: "cherry-pick A..B || git reset --hard OTHER"
From: Junio C Hamano @ 2016-12-10 21:23 UTC (permalink / raw)
To: Duy Nguyen
Cc: Stephan Beyer, Git Mailing List, Christian Couder,
SZEDER Gábor
In-Reply-To: <CACsJy8Div=Baenn7c-1wxgvrOh5PG=naeDrEYC8gs+AvJE7wZA@mail.gmail.com>
Duy Nguyen <pclouds@gmail.com> writes:
> rebase and cherry-pick/revert are not exactly in the same situation.
> When cherry-pick/revert in "continue/abort" mode, there's usually some
> conflicted files and it's easy to notice.
>
> But an interactive rebase could stop at some commit with clean
> worktree (the 'edit' command). Then I could even add some more commits
> on top. I don't see how 'rebase --abort' can know my intention in this
> case, whether I tried (with some new commits) and failed, and want to
> revert/abort the whole thing, moving HEAD back to the original; or
> whether I forgot I was in the middle of rebase and started to do
> something else, and --abort needs to keep HEAD where it is.
OK.
^ permalink raw reply
* Re: [PATCH 3/4] doc: make the intent of sentence clearer
From: Philip Oakley @ 2016-12-10 20:52 UTC (permalink / raw)
To: Kristoffer Haugsbakk, git; +Cc: Kristoffer Haugsbakk
In-Reply-To: <20161209155112.2112-4-kristoffer.haugsbakk@gmail.com>
From: "Kristoffer Haugsbakk" <kristoffer.haugsbakk@gmail.com>
Sent: Friday, December 09, 2016 3:51 PM
> By adding the word "just", which might have been accidentally omitted.
>
> Adding the word "just" makes it clear that the point is to *not* do an
> octopus merge simply because you *can* do it. In other words, you
> should have a reason for doing it beyond simply having two (seemingly)
> independent commits that you need to merge into another branch, since
> it's not always the best approach.
>
> The previous sentence made it look more like it was trying to say that
> you shouldn't do an octopus merge *because* you can do an octopus merge.
> Although this interpretation doesn't make sense and the rest of the
> paragraph makes the intended meaning clear, this adjustment should make
> the intent of the sentence more immediately clear to the reader.
>
> Signed-off-by: Kristoffer Haugsbakk <kristoffer.haugsbakk@gmail.com>
> ---
> Documentation/gitcore-tutorial.txt | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Documentation/gitcore-tutorial.txt
> b/Documentation/gitcore-tutorial.txt
> index 72ed90ca3..72ca9c1ef 100644
> --- a/Documentation/gitcore-tutorial.txt
> +++ b/Documentation/gitcore-tutorial.txt
> @@ -1635,7 +1635,7 @@ $ git show-branch
> ++* [master~2] Pretty-print messages.
> ------------
>
> -Note that you should not do Octopus because you can. An octopus
> +Note that you should not do Octopus just because you can. An octopus
> is a valid thing to do and often makes it easier to view the
> commit history if you are merging more than two independent
> changes at the same time. However, if you have merge conflicts
> --
> 2.11.0
>
Looks like a reasonable emphasis to me
--
Philip (UK)
^ permalink raw reply
* Re: [PATCH 2/4] doc: add verb in front of command to run
From: Philip Oakley @ 2016-12-10 20:48 UTC (permalink / raw)
To: Kristoffer Haugsbakk, git; +Cc: Kristoffer Haugsbakk
In-Reply-To: <20161209155112.2112-3-kristoffer.haugsbakk@gmail.com>
From: "Kristoffer Haugsbakk" <kristoffer.haugsbakk@gmail.com>
Sent: Friday, December 09, 2016 3:51 PM
> Instead of using the command 'git clone' as a verb, use "run" as the
> verb indicating the action of executing the command 'git clone'.
I would expect 'cloning' as the commonly in use verb here, with the command
then quoted.
>
> Signed-off-by: Kristoffer Haugsbakk <kristoffer.haugsbakk@gmail.com>
> ---
> Documentation/gitcore-tutorial.txt | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Documentation/gitcore-tutorial.txt
> b/Documentation/gitcore-tutorial.txt
> index 6c434aff3..72ed90ca3 100644
> --- a/Documentation/gitcore-tutorial.txt
> +++ b/Documentation/gitcore-tutorial.txt
> @@ -1478,7 +1478,7 @@ You can repack this private repository whenever you
> feel like.
> A recommended work cycle for a "subsystem maintainer" who works
> on that project and has an own "public repository" goes like this:
>
> -1. Prepare your work repository, by 'git clone' the public
> +1. Prepare your work repository, by running 'git clone' on the public
Perhaps ?
... by cloning ('git clone <URL>') the public
The full command is shown using the same terminology as the following line.
> repository of the "project lead". The URL used for the
> initial cloning is stored in the remote.origin.url
> configuration variable.
> --
> 2.11.0
>
>
--
Philip
^ permalink raw reply
* Re: [PATCH 1/4] doc: add articles (grammar)
From: Philip Oakley @ 2016-12-10 20:28 UTC (permalink / raw)
To: Kristoffer Haugsbakk, git; +Cc: Kristoffer Haugsbakk
In-Reply-To: <20161209155112.2112-2-kristoffer.haugsbakk@gmail.com>
From: "Kristoffer Haugsbakk" <kristoffer.haugsbakk@gmail.com>
Sent: Friday, December 09, 2016 3:51 PM
> Add definite and indefinite articles in three places where they were
> missing.
>
> - Use "the" in front of a directory name
> - Use "the" in front of "style of cooperation"
> - Use an indefinite article in front of "CVS background"
>
> Signed-off-by: Kristoffer Haugsbakk <kristoffer.haugsbakk@gmail.com>
> ---
> Documentation/gitcore-tutorial.txt | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/gitcore-tutorial.txt
> b/Documentation/gitcore-tutorial.txt
> index 4546fa0d7..6c434aff3 100644
> --- a/Documentation/gitcore-tutorial.txt
> +++ b/Documentation/gitcore-tutorial.txt
> @@ -1368,7 +1368,7 @@ $ git repack
> will do it for you. If you followed the tutorial examples, you
> would have accumulated about 17 objects in `.git/objects/??/`
> directories by now. 'git repack' tells you how many objects it
> -packed, and stores the packed file in `.git/objects/pack`
> +packed, and stores the packed file in the `.git/objects/pack`
> directory.
>
> [NOTE]
> @@ -1543,9 +1543,9 @@ like this:
> Working with Others, Shared Repository Style
> --------------------------------------------
>
> -If you are coming from CVS background, the style of cooperation
> +If you are coming from a CVS background, the style of cooperation
> suggested in the previous section may be new to you. You do not
> -have to worry. Git supports "shared public repository" style of
> +have to worry. Git supports the "shared public repository" style of
> cooperation you are probably more familiar with as well.
>
> See linkgit:gitcvs-migration[7] for the details.
> --
> 2.11.0
This looks good to me.
--
Philip (UK)
^ permalink raw reply
* Re: [PATCH v2 4/5] Make sequencer abort safer
From: Stephan Beyer @ 2016-12-10 20:20 UTC (permalink / raw)
To: Jeff King, Christian Couder; +Cc: git, Junio C Hamano, SZEDER Gábor
In-Reply-To: <20161210200437.ijmahia6e6xifhk6@sigill.intra.peff.net>
On 12/10/2016 09:04 PM, Jeff King wrote:
> On Sat, Dec 10, 2016 at 08:56:26PM +0100, Christian Couder wrote:
>
>>> +static int rollback_is_safe(void)
>>> +{
>>> + struct strbuf sb = STRBUF_INIT;
>>> + struct object_id expected_head, actual_head;
>>> +
>>> + if (strbuf_read_file(&sb, git_path_abort_safety_file(), 0) >= 0) {
>>> + strbuf_trim(&sb);
>>> + if (get_oid_hex(sb.buf, &expected_head)) {
>>> + strbuf_release(&sb);
>>> + die(_("could not parse %s"), git_path_abort_safety_file());
>>> + }
>>> + strbuf_release(&sb);
>>> + }
>>
>> Maybe the following is a bit simpler:
>>
>> if (strbuf_read_file(&sb, git_path_abort_safety_file(), 0) >= 0) {
>> int res;
>> strbuf_trim(&sb);
>> res = get_oid_hex(sb.buf, &expected_head);
>> strbuf_release(&sb);
>> if (res)
>> die(_("could not parse %s"), git_path_abort_safety_file());
>> }
>
> Is there any point in calling strbuf_release() if we're about to die
> anyway? I could see it if it were "return error()", but it's normal in
> our code base for die() to be abrupt.
The point is that someone "libifies" the function some day; then "die()"
becomes "return error()" almost automatically. Chances are high that the
resulting memory leak is forgotten. That's one of the reasons why I like
being strict about memory leaks.
However, I cannot tell if mine or Christian's variant is really
"simpler" (with whatever measure) and I also don't care much.
~Stephan
^ permalink raw reply
* Re: [PATCH v2 4/5] Make sequencer abort safer
From: Jeff King @ 2016-12-10 20:04 UTC (permalink / raw)
To: Christian Couder; +Cc: Stephan Beyer, git, Junio C Hamano, SZEDER Gábor
In-Reply-To: <CAP8UFD0hCke_W6C=gOHinpj+G3WCFKf7Cji6zREDer4RUBxKxg@mail.gmail.com>
On Sat, Dec 10, 2016 at 08:56:26PM +0100, Christian Couder wrote:
> > +static int rollback_is_safe(void)
> > +{
> > + struct strbuf sb = STRBUF_INIT;
> > + struct object_id expected_head, actual_head;
> > +
> > + if (strbuf_read_file(&sb, git_path_abort_safety_file(), 0) >= 0) {
> > + strbuf_trim(&sb);
> > + if (get_oid_hex(sb.buf, &expected_head)) {
> > + strbuf_release(&sb);
> > + die(_("could not parse %s"), git_path_abort_safety_file());
> > + }
> > + strbuf_release(&sb);
> > + }
>
> Maybe the following is a bit simpler:
>
> if (strbuf_read_file(&sb, git_path_abort_safety_file(), 0) >= 0) {
> int res;
> strbuf_trim(&sb);
> res = get_oid_hex(sb.buf, &expected_head);
> strbuf_release(&sb);
> if (res)
> die(_("could not parse %s"), git_path_abort_safety_file());
> }
Is there any point in calling strbuf_release() if we're about to die
anyway? I could see it if it were "return error()", but it's normal in
our code base for die() to be abrupt.
-Peff
^ permalink raw reply
* Re: [PATCH v2 4/5] Make sequencer abort safer
From: Christian Couder @ 2016-12-10 19:56 UTC (permalink / raw)
To: Stephan Beyer; +Cc: git, Junio C Hamano, SZEDER Gábor
In-Reply-To: <20161209190111.9571-4-s-beyer@gmx.net>
On Fri, Dec 9, 2016 at 8:01 PM, Stephan Beyer <s-beyer@gmx.net> wrote:
[...]
> +static int rollback_is_safe(void)
> +{
> + struct strbuf sb = STRBUF_INIT;
> + struct object_id expected_head, actual_head;
> +
> + if (strbuf_read_file(&sb, git_path_abort_safety_file(), 0) >= 0) {
> + strbuf_trim(&sb);
> + if (get_oid_hex(sb.buf, &expected_head)) {
> + strbuf_release(&sb);
> + die(_("could not parse %s"), git_path_abort_safety_file());
> + }
> + strbuf_release(&sb);
> + }
Maybe the following is a bit simpler:
if (strbuf_read_file(&sb, git_path_abort_safety_file(), 0) >= 0) {
int res;
strbuf_trim(&sb);
res = get_oid_hex(sb.buf, &expected_head);
strbuf_release(&sb);
if (res)
die(_("could not parse %s"), git_path_abort_safety_file());
}
Thanks,
Christian.
^ permalink raw reply
* Re: Resend: Gitk: memory consumption improvements
From: Markus Hitter @ 2016-12-10 18:23 UTC (permalink / raw)
To: Stefan Beller; +Cc: git@vger.kernel.org, Paul Mackerras
In-Reply-To: <CAGZ79ka4TRXW7-YY8hqvYz_NJ+dZxtwY6KDSOJ+0cZF4i-J+fA@mail.gmail.com>
Am 09.12.2016 um 18:57 schrieb Stefan Beller:
> On Fri, Dec 9, 2016 at 3:51 AM, Markus Hitter <mah@jump-ing.de> wrote:
>>
>> It's a month now since I sent three patches to this list for reducing memory consumption of Gitk considerably:
>>
>> https://public-inbox.org/git/de7cd593-0c10-4e93-1681-7e123504f5d5@jump-ing.de/
>> https://public-inbox.org/git/e09a5309-351d-d246-d272-f527f50ad444@jump-ing.de/
>> https://public-inbox.org/git/8e1c5923-d2a6-bc77-97ab-3f154b41d2ea@jump-ing.de/
>> https://public-inbox.org/git/2cb7f76f-0004-a5b6-79f1-9bb4f979cf14@jump-ing.de/
>>
>> Everybody cheered, but apparently nobody picked these patches up and applied them to either the Git or Gitk repository. They don't appear in the regular "what's cooking" post either.
>
> The "What's cooking" email is done by Junio (the Git maintainer)
> describing the development of Git, whereas gitk is maintained by Paul
> if I understand correctly.
TBH, there might be no Gitk maintenance at all, because the last commit in either repository dates February 2016. It's a bit hard to believe there was not a single contribution in 10 months.
> I'd love to see those patches in use here (via a regular upstream update).
>
> So I guess the way to go for you is to keep bugging Paul for an ack?
Like:
Hey hey Paul!
Come and get the goal!
Fetch the patch and get applied
to our all's delight!
or like
Macke- Macke- Mackerras!
Come here and join with us!
It's just a simple 'git am'
and you are the man!
Another encouraging poem?
Markus
--
- - - - - - - - - - - - - - - - - - -
Dipl. Ing. (FH) Markus Hitter
http://www.jump-ing.de/
^ permalink raw reply
* Re: [BUG] Colon in remote urls
From: Johannes Schindelin @ 2016-12-10 18:18 UTC (permalink / raw)
To: Klaus Ethgen; +Cc: git
In-Reply-To: <20161210093230.26q7fxcrs2cpll6g@ikki.ethgen.ch>
Hi Klaus,
On Sat, 10 Dec 2016, Klaus Ethgen wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA512
>
> Am Fr den 9. Dez 2016 um 22:32 schrieb Johannes Sixt:
> > There are too many systems out there that use a backslash in path names. I
> > don't think it is wise to use it also as the quoting character.
>
> Well, the minority, I believe. And only the minority where the command
> line git is used anywhere.
Please provide evidence for such bold assumptions.
Ciao,
Johannes
^ permalink raw reply
* Re: [PATCH v2 12/16] pathspec: create parse_long_magic function
From: Junio C Hamano @ 2016-12-10 18:18 UTC (permalink / raw)
To: Stefan Beller; +Cc: Brandon Williams, git@vger.kernel.org, Duy Nguyen
In-Reply-To: <CAGZ79ka0P0rKF8QH3V0jC-O19eT0oaE+fJLGifbfmm3jC_SijA@mail.gmail.com>
Stefan Beller <sbeller@google.com> writes:
> Jonathan Nieder mentioned off list that he prefers to see that
> series rerolled without mutexes if possible. That is possible by
> creating the questions "struct attr_check" before preloading the
> index and then using the read only questions in the threaded code,
> to obtain answers fast; also no need for a mutex.
I do not see how it would work without further splitting the
attr_stack. I think you made it per check[], but you would further
split it per <check, thread> before losing the mutex, no?
^ permalink raw reply
* Re: Any interest in 'git merge --continue' as a command
From: Junio C Hamano @ 2016-12-10 18:16 UTC (permalink / raw)
To: Jeff King; +Cc: Chris Packham, GIT
In-Reply-To: <20161210085938.rfbkuwpvyhnhuzhn@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> No, I think your reasoning makes sense. But I also think we've already
> choosen to have "--continue" mean "conclude the current, and continue if
> there is anything left" in other contexts (e.g., a single-item
> cherry-pick). It's more vague, but I think it keeps the user's mental
> model simpler if we provide a standard set of options for multi-step
> commands (e.g., always "--continue/--abort/--skip", though there are
> some like merge that omit "--skip" if it does not make sense).
Yup. I know you know me well enough to know that I didn't mean to
say "oh this one needs to be called differently" ;-) I just felt
that "--continue" in that context did not sit well.
^ permalink raw reply
* Re: [REGRESSION 2.10.2] problematic "empty auth" changes
From: brian m. carlson @ 2016-12-10 15:23 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: David Turner, git
In-Reply-To: <alpine.DEB.2.20.1612101551100.23160@virtualbox>
[-- Attachment #1: Type: text/plain, Size: 1197 bytes --]
On Sat, Dec 10, 2016 at 03:52:39PM +0100, Johannes Schindelin wrote:
> One of my colleagues offered a legitimate concern: it potentially adds
> another round-trip.
>
> Do you happen to know whether regular HTTPS negotiation will have an extra
> round-trip if Kerberos is attempted, but we have to fall back to
> interactively prompt for (or use stored) credentials?
With Kerberos (using tickets), you have 7 request/response pairs, and an
additional round trip for the 100 Continue if your push is larger than
http.postBuffer. You only have 6 for Basic using Kerberos.
However, libcurl is generally going to be able to figure out whether
your Kerberos credentials can be used, so when it falls back to Basic,
it does so because it knows you have nothing to use with Negotiate (e.g.
you have no ticket), and therefore it doesn't even try. I suppose if I
tried to push to a server that offered Negotiate and Basic, but didn't
accept my Kerberos credentials, it might fall back in such a way,
though.
--
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 868 bytes --]
^ permalink raw reply
* Re: [REGRESSION 2.10.2] problematic "empty auth" changes
From: Johannes Schindelin @ 2016-12-10 14:52 UTC (permalink / raw)
To: brian m. carlson; +Cc: David Turner, git
In-Reply-To: <20161209221854.re6qf3e5225wxvge@genre.crustytoothpaste.net>
Hi Brian,
On Fri, 9 Dec 2016, brian m. carlson wrote:
> On Thu, Dec 08, 2016 at 04:12:32PM -0500, David Turner wrote:
> > I know of no reason that shouldn't work. Indeed, it's what we use do
> > internally. So far, nobody has reported problems. That said, we have
> > exactly three sets of git servers that most users talk to (two
> > different internal; and occasionally github.com for external stuff).
> > So our coverage is not very broad.
> >
> > If you're going to do it, tho, don't just do it for Windows users --
> > do it for everyone. Plenty of Unix clients connect to Windows-based
> > auth systems.
>
> Let me echo this. This would make Kerberos (and probably other forms of
> SPNEGO) work out of the box, which would reduce a lot of confusion that
> people have.
>
> I can confirm enabling http.emptyAuth works properly with Kerberos,
> including with fallback to Basic, so I see no reason why we shouldn't do
> it.
One of my colleagues offered a legitimate concern: it potentially adds
another round-trip.
Do you happen to know whether regular HTTPS negotiation will have an extra
round-trip if Kerberos is attempted, but we have to fall back to
interactively prompt for (or use stored) credentials?
Ciao,
Johannes
^ permalink raw reply
* Re: [PATCH] submodule--helper: set alternateLocation for cloned submodules
From: vi0oss @ 2016-12-10 13:41 UTC (permalink / raw)
To: stefanbeller; +Cc: git
In-Reply-To: <20161208013814.4943-1-vi0oss@gmail.com>
On 12/08/2016 04:38 AM, vi0oss@gmail.com wrote:
> Third review: missing && in test fixed.
>
Shall something more be done about this or just wait until the patch
gets reviewed and integrated?
^ permalink raw reply
* Re: Feature request: read git config from parent directory
From: Dominique Dumont @ 2016-12-10 11:16 UTC (permalink / raw)
To: Duy Nguyen; +Cc: Git Mailing List
In-Reply-To: <CACsJy8AgbGXvMC0XWSPuBHEveJfJFEYUgghDC1Yc7Eka1Dyd8Q@mail.gmail.com>
On Friday, 9 December 2016 19:38:05 CET Duy Nguyen wrote:
> >> Sounds like the same problem I have (and the reason I came up with
> >> conditional include [1]). Would that work for you (check out the
> >> example part in that patch)?
> >
> > If I understand correcly, I would need to set up config include in each
> > git
> > repository. This is as much work as setting up user.email in the same
> > place.
>
> Well, no. You set this up in ~/.gitconfig. If you need to add some
> settings in /abc/def/.gitconfig and expect repositories in this path
> to reach it via the parent chain, then you could write something like
>
> [include "gitdir:/abc/def/"]
> file = your-config-file
>
> in ~/.gitconfig and achieve the same effect, because all repos will
> read ~/.gitconfig, and if it finds out the repo's location is inside
> /abc/def, your-config-file will be loaded. It could contain email
> settings or whatever.
>
> So, instead of spreading .gitconfig files around and relying on
> parent-chain to reach them, you write a few filter rules in
> ~/.gitconfig to tell all the repos what to load.
oh... yes, that would solve my problem and have no impact on other user who
don't need this feature.
I do hope that the improvement you proposed will be merged.
Thanks for the explanation.
All the best
--
https://github.com/dod38fr/ -o- http://search.cpan.org/~ddumont/
http://ddumont.wordpress.com/ -o- irc: dod at irc.debian.org
^ permalink raw reply
* Re: [PATCHv7 4/6] worktree: have a function to check if worktrees are in use
From: Duy Nguyen @ 2016-12-10 11:16 UTC (permalink / raw)
To: Stefan Beller; +Cc: Brandon Williams, git@vger.kernel.org, Junio C Hamano
In-Reply-To: <CAGZ79kaV4FYZEbRWQxKBHJg1jVzOkte1QxLZgu75=Jja_BMRGQ@mail.gmail.com>
On Sat, Dec 10, 2016 at 1:49 AM, Stefan Beller <sbeller@google.com> wrote:
> On Fri, Dec 9, 2016 at 4:00 AM, Duy Nguyen <pclouds@gmail.com> wrote:
>
>> int submodule_uses_worktrees(const char *path)
>> {
>> struct strbuf path = STRBUF_INIT;
>> DIR *dir;
>> struct dirent *d;
>> int ret = 0;
>>
>> strbuf_addf(&path, "%s/worktrees", path);
>> dir = opendir(path.buf);
>> strbuf_release(&path);
>>
>> if (!dir)
>> return 0;
>
> The submodule may be one of the linked worktrees, which would be
> caught if we use the code as I sent it out?
I think I simplified it too much, there should still be a
git_common_dir_noenv() to retrieve the correct common dir in the
submodule from gitdir. Then, if the repo in question has any linked
worktrees, you probably can't handle it. It does not matter if the
submodule gitdir in question is a linked or main worktree.
> If this is one of the linked worktrees, we'd rather check if a file
> "commondir" or "gitdir" exists?
Well, in theory yes. But we're apparently not ready for that. If you
check the files exist, but the files are not valid, then it's still
not considered a worktree. Which is not much different from what we do
here (if the directory exists, assuming it's a worktree). It should
catch all the valid worktrees. But yes it could give some false
positive. Since we're just using this function to stop the aborbing
git dir operation, i think this is acceptable.
(It goes even trickier, even get_worktrees() won't detect if the
linked worktree is already dead, .e.g. deleted manually, I believe.
Those have to be cleaned up by 'git worktree prune' which does even
more checks before it declare "this directory is garbage").
> I ask that because I would not know how to relocate such a linked
> worktree gitdir?
--
Duy
^ permalink raw reply
* Re: [PATCH v2 0/4] road to reentrant real_path
From: Duy Nguyen @ 2016-12-10 11:02 UTC (permalink / raw)
To: Brandon Williams
Cc: Git Mailing List, Stefan Beller, Jeff King, Jacob Keller,
Junio C Hamano, Ramsay Jones, Torsten Bögershausen,
Johannes Sixt
In-Reply-To: <20161209194232.GC88637@google.com>
On Sat, Dec 10, 2016 at 2:42 AM, Brandon Williams <bmwill@google.com> wrote:
> On 12/09, Duy Nguyen wrote:
>> On Fri, Dec 9, 2016 at 6:58 AM, Brandon Williams <bmwill@google.com> wrote:
>> > diff --git a/setup.c b/setup.c
>> > index fe572b8..0d9fdd0 100644
>> > --- a/setup.c
>> > +++ b/setup.c
>> > @@ -254,10 +254,12 @@ int get_common_dir_noenv(struct strbuf *sb, const char *gitdir)
>> > if (!is_absolute_path(data.buf))
>> > strbuf_addf(&path, "%s/", gitdir);
>> > strbuf_addbuf(&path, &data);
>> > - strbuf_addstr(sb, real_path(path.buf));
>> > + strbuf_realpath(sb, path.buf, 1);
>>
>> This is not the same because of this hunk in strbuf_realpath()
>
> Then perhaps I shouldn't make this change (and just leave it as is)
> since the way real_path_internal/strbuf_realpath is written requires
> that the strbuf being used for the resolved path only contains the
> resolved path (see the lstat(resolved->buf &st) call). Sidenote it
> looks like strbuf_getcwd() also does a reset, though more subtlety,
> since it just passes its buffer to getcwd().
Yeah that's ok too (I did not see this subtlety when I suggested the change).
--
Duy
^ permalink raw reply
* Re: BUG: "cherry-pick A..B || git reset --hard OTHER"
From: Duy Nguyen @ 2016-12-10 11:00 UTC (permalink / raw)
To: Stephan Beyer
Cc: Junio C Hamano, Git Mailing List, Christian Couder,
SZEDER Gábor
In-Reply-To: <e0780f7c-ccb4-29fe-3d72-80e45a202f65@gmx.net>
On Sat, Dec 10, 2016 at 2:24 AM, Stephan Beyer <s-beyer@gmx.net> wrote:
> Hi Junio,
>
> On 12/09/2016 07:07 PM, Junio C Hamano wrote:
>> Duy Nguyen <pclouds@gmail.com> writes:
>>> Having the same operation with different names only increases git
>>> reputation of bad/inconsistent UI. Either forget is renamed to quit,
>>> or vice versa. I prefer forget, but the decision is yours and the
>>> community's. So I'm sending two patches to rename in either direction.
>>> You can pick one.
>>
>> I actually was advocating to remove both by making --abort saner.
>> With an updated --abort that behaves saner, is "rebase --forget"
>> still necessary?
>
> A quick change in t3407 of the "rebase --forget" test to use "rebase
> --abort" failed. That's because it checks the use-case of
> forgetting/aborting without changing the HEAD. So --abort makes a
> rollback, --forget just keeps the current head. I am not sure if that
> tested use-case is a real use-case though.
It is. I wanted something like this for years but "rm -rf
/path/to/.git/rebase*" was not as bad when there were no linked
worktrees.
rebase and cherry-pick/revert are not exactly in the same situation.
When cherry-pick/revert in "continue/abort" mode, there's usually some
conflicted files and it's easy to notice.
But an interactive rebase could stop at some commit with clean
worktree (the 'edit' command). Then I could even add some more commits
on top. I don't see how 'rebase --abort' can know my intention in this
case, whether I tried (with some new commits) and failed, and want to
revert/abort the whole thing, moving HEAD back to the original; or
whether I forgot I was in the middle of rebase and started to do
something else, and --abort needs to keep HEAD where it is.
--
Duy
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox