* Re: [PATCH v3 2/3] refs: add option core.logAllRefUpdates = always
From: Junio C Hamano @ 2017-01-31 17:11 UTC (permalink / raw)
To: Cornelius Weig; +Cc: peff, git
In-Reply-To: <68b6ac92-459d-849d-9589-e1fa500e2572@tngtech.com>
Cornelius Weig <cornelius.weig@tngtech.com> writes:
> And again, thanks for not yelling. I overlooked that the
> "should_autocreate_reflog" return value should have been negated as
> shown below.
Heh---I AM blind. I didn't spot it even though I was staring at the
code and even tweaking it (for the constness thing).
> Should I resend this patch, or is it easier for you
> to do the change yourself?
I can squash it in, now we have and the list saw all the bits
necessary.
Thanks for working on this.
> Interdiff v2..v3:
> diff --git a/builtin/checkout.c b/builtin/checkout.c
> index 81ea2ed..1e8631a 100644
> --- a/builtin/checkout.c
> +++ b/builtin/checkout.c
> @@ -612,8 +612,10 @@ static void update_refs_for_switch(const struct checkout_opts *opts,
> const char *old_desc, *reflog_msg;
> if (opts->new_branch) {
> if (opts->new_orphan_branch) {
> - const char *refname = mkpathdup("refs/heads/%s", opts->new_orphan_branch);
> - if (opts->new_branch_log && should_autocreate_reflog(refname)) {
> + char *refname;
> +
> + refname = mkpathdup("refs/heads/%s", opts->new_orphan_branch);
> + if (opts->new_branch_log && !should_autocreate_reflog(refname)) {
> int ret;
> struct strbuf err = STRBUF_INIT;
>
> @@ -622,6 +624,7 @@ static void update_refs_for_switch(const struct checkout_opts *opts,
> fprintf(stderr, _("Can not do reflog for '%s': %s\n"),
> opts->new_orphan_branch, err.buf);
> strbuf_release(&err);
> + free(refname);
> return;
> }
> strbuf_release(&err);
^ permalink raw reply
* Re: [PATCH] t0001: don't let a default ACL interfere with the umask test
From: Junio C Hamano @ 2017-01-31 17:13 UTC (permalink / raw)
To: Matt McCutchen; +Cc: git
In-Reply-To: <1485636766.2482.3.camel@mattmccutchen.net>
Matt McCutchen <matt@mattmccutchen.net> writes:
> The "init creates a new deep directory (umask vs. shared)" test expects
> the permissions of newly created files to be based on the umask, which
> fails if a default ACL is inherited from the working tree for git. So
> attempt to remove a default ACL if there is one. Same idea as
> 8ed0a740dd42bd0724aebed6e3b07c4ea2a2d5e8. (I guess I'm the only one who
> ever runs the test suite with a default ACL set.)
Thanks--people with such a configuration who run tests are valuable ;-)
^ permalink raw reply
* Re: [PATCH v3 2/3] refs: add option core.logAllRefUpdates = always
From: Junio C Hamano @ 2017-01-31 17:08 UTC (permalink / raw)
To: Jeff King; +Cc: cornelius.weig, git
In-Reply-To: <20170130233702.o6naszpz32juf5gt@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> So I agree the current behavior is quietly broken, which is not good.
> But I wonder if "--no-create-reflog" is really sane in the first place,
> and whether we might be better off to simply disallow it.
Thanks for a reasoned argument and a reasonable justification. I
agree with all that.
I think it is probably a good idea to document the behaviour
(i.e. "--no-create" single-shot from the command line is ignored).
I am not sure we should error out, though, in order to "disallow"
it---a documented silent no-op may be sufficient.
^ permalink raw reply
* Feature request: flagging “volatile” branches for integration/development
From: Herbert, Marc @ 2017-01-31 17:12 UTC (permalink / raw)
To: git; +Cc: josh
(Thanks to Josh Triplett[*] for contributing to this message)
Hi,
We often work with development/integration branches that regularly
rebase, in addition to stable branches that do not. Git is used to share
two different types of branches:
1. Pull requests and merged code with final SHA1s
2. Work in progress with volatile SHA1s.
We’d like to have a consistent way to distinguish these two types by
advertising a branch as “volatile”. Such a branch supports shared
development on work-in-progress code (not yet ready to merge, or still
being emailed as PATCHv{2,3,4,...}), or an integration/testing branch
for a combination of such development branches. Branch naming
conventions could help a bit here, but a large and varied set of naming
conventions already exist, none of which provide machine-readable
information that git itself can rely on. So the only thing available is
tribal knowledge or out-of-band documentation at best, e.g.:
https://01.org/linuxgraphics/gfx-docs/maintainer-tools/drm-intel.html
Such a “volatile” flag would instantly warn that code is not ready to
re-use:
https://www.chromium.org/chromium-os/chromiumos-design-docs/upstream-first
Another common issue with volatile branches is their commits being
misunderstood as immutable and then their transient, top SHA1
being reported as bogus and unusable version control information in bugs
(on a non-volatile branch, a single SHA1 captures the entire and exact
snapshot and history).
Humans would be the initial consumers of this flag but I can imagine git
itself also using it. For instance, cherry-pick could have a “smart”
setting for -x, that doesn’t bother recording transient commit hashes. A
merge could print an optional warning when pulling in changes from a
volatile branch, and a rebase could similarly print a warning when
rebasing on top of such a branch. A git server could be configured to
treat non-fast forward forced pushes differently depending on the
“volatility” of the target branch. A fancy user interface could
color volatile SHA1s differently to discourage copy/paste. Etc.
Maybe this has already been discussed (or implemented even), and I
couldn’t find the right search keywords; in this case please help me cut
this discussion short. We’d appreciate any feedback you might have,
either on the idea itself, or on other ways to solve the same problem.
[ ] “send patches”
[ ] use some other existing mechanism to solve this
[ ] will never work because of X and Y; don’t even bother
--
Marc
PS: please keep me in Cc:, thanks.
[*] on a related topic: https://github.com/git-series/git-series
^ permalink raw reply
* Re: [PATCH 2/4] git-prompt.sh: rework of submodule indicator
From: SZEDER Gábor @ 2017-01-31 18:06 UTC (permalink / raw)
To: Benjamin Fuchs; +Cc: SZEDER Gábor, sbeller, sandals, ville.skytta, git
In-Reply-To: <1485809065-11978-3-git-send-email-email@benjaminfuchs.de>
> Rework of the first patch. The prompt now will look like this:
> (+name:master). I tried to considere all suggestions.
> Tests still missing.
>
> Signed-off-by: Benjamin Fuchs <email@benjaminfuchs.de>
> ---
> contrib/completion/git-prompt.sh | 49 ++++++++++++++++++++--------------------
> 1 file changed, 24 insertions(+), 25 deletions(-)
>
> diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
> index 4c82e7f..c44b9a2 100644
> --- a/contrib/completion/git-prompt.sh
> +++ b/contrib/completion/git-prompt.sh
> @@ -95,8 +95,10 @@
> # repository level by setting bash.hideIfPwdIgnored to "false".
> #
> # If you would like __git_ps1 to indicate that you are in a submodule,
> -# set GIT_PS1_SHOWSUBMODULE. In this case a "sub:" will be added before
> -# the branch name.
> +# set GIT_PS1_SHOWSUBMODULE to a nonempty value. In this case the name
> +# of the submodule will be prepended to the branch name (e.g. module:master).
> +# The name will be prepended by "+" if the currently checked out submodule
> +# commit does not match the SHA-1 found in the index of the containing repository.
>
> # check whether printf supports -v
> __git_printf_supports_v=
> @@ -288,30 +290,27 @@ __git_eread ()
> test -r "$f" && read "$@" <"$f"
> }
>
> -# __git_is_submodule
> -# Based on:
> -# http://stackoverflow.com/questions/7359204/git-command-line-know-if-in-submodule
> -__git_is_submodule ()
> -{
> - local git_dir parent_git module_name path
> - # Find the root of this git repo, then check if its parent dir is also a repo
> - git_dir="$(git rev-parse --show-toplevel)"
> - module_name="$(basename "$git_dir")"
> - parent_git="$(cd "$git_dir/.." && git rev-parse --show-toplevel 2> /dev/null)"
> - if [[ -n $parent_git ]]; then
> - # List all the submodule paths for the parent repo
> - while read path
> - do
> - if [[ "$path" != "$module_name" ]]; then continue; fi
> - if [[ -d "$git_dir/../$path" ]]; then return 0; fi
> - done < <(cd $parent_git && git submodule --quiet foreach 'echo $path' 2> /dev/null)
> - fi
> - return 1
> -}
> -
> +# __git_ps1_submodule
> +#
> +# $1 - git_dir path
> +#
> +# Returns the name of the submodule if we are currently inside one. The name
> +# will be prepended by "+" if the currently checked out submodule commit does
> +# not match the SHA-1 found in the index of the containing repository.
> +# NOTE: git_dir looks like in a ...
> +# - submodule: "GIT_PARENT/.git/modules/PATH_TO_SUBMODULE"
> +# - parent: "GIT_PARENT/.git" (exception "." in .git)
> __git_ps1_submodule ()
> {
> - __git_is_submodule && printf "sub:"
> + local git_dir="$1"
> + local submodule_name="$(basename "$git_dir")"
> + if [ "$submodule_name" != ".git" ] && [ "$submodule_name" != "." ]; then
> + local parent_top="${git_dir%.git*}"
> + local submodule_top="${git_dir#*modules}"
> + local status=""
> + git diff -C "$parent_top" --no-ext-diff --ignore-submodules=dirty --quiet -- "$submodule_top" 2>/dev/null || status="+"
This 'git diff' has to read the index of the parent repository, right?
That can be potentially very expensive, if the parent repository, and
thus its index, is big.
You might want to provide finer granularity controls and separate the
"$PWD is in a submodule" indicator from the "submodule commit doesn't
match" indicator. Even if someone is in general interested in the
former, he might have some huge repositories where he would prefer to
disable the latter, because executing 'git diff' would make the prompt
lag.
I'm not sure we need brand new control knobs, though. Perhaps we can
get away by checking the env and config variables used for the dirty
index and worktree status indicator, after all they, too, are about
whether to run 'git diff' for the prompt in a repository or not.
> + printf "$status$submodule_name:"
> + fi
> }
>
> # __git_ps1 accepts 0 or 1 arguments (i.e., format string)
> @@ -545,7 +544,7 @@ __git_ps1 ()
>
> local sub=""
> if [ -n "${GIT_PS1_SHOWSUBMODULE}" ]; then
> - sub="$(__git_ps1_submodule)"
> + sub="$(__git_ps1_submodule $g)"
In Bash, and in shells in general, the visibility of variables works
differently than in "regular" programming languages:
- Any variable existing in a caller, even the ones declared as
'local' in the caller, are visible in all callees.
This means you don't have to pass $g as parameter to
__git_ps1_submodule(), because you can access it inside that
function directly. This has the benefit that there is one less
place where you can forget to quote a path variable :)
- If the callee modifies any variable that isn't declared as local
in the callee, then the caller will see the modified value of that
variable, unless the callee was invoked in a subshell.
Here your sole purpose is to set $sub to something like "+sub:",
which is determined in __git_ps1_submodule(). You don't need the
command substitution for that, you can set $sub directly in
__git_ps1_submodule(), sparing the overhead of fork()ing a
subshell. That's important for the sake of git users on Windows.
> fi
>
> local f="$w$i$s$u"
--
2.7.4
^ permalink raw reply
* Re: [PATCH v3 2/3] refs: add option core.logAllRefUpdates = always
From: Jeff King @ 2017-01-31 18:21 UTC (permalink / raw)
To: Cornelius Weig; +Cc: Junio C Hamano, git
In-Reply-To: <1e341485-6fb6-243a-0b27-4035789a6f2a@tngtech.com>
On Tue, Jan 31, 2017 at 03:00:33PM +0100, Cornelius Weig wrote:
> Concerning branches, I fully agree. For git-branch, the
> "--no-create-reflog" option does not make sense at all and should
> produce an error.
>
> On the other hand, for tags it may make sense to override
> logAllRefUpdates=always. As tag updates come exclusively from
> force-creating the same tag on another revision, a reflog will actually
> not be created by accident.
Hmm. I think you could also see tag creation and update via "git fetch",
though only with explicit refspecs, I think, not tag-following.
So I think ultimately you'd need to use "git -c logallrefupdates=false"
if you want to override reflog options for all commands. A saner
interface would probably be put teaching the ref code to respect a
configured list of exceptions ("I do want reflogs for refs/tags/, but
not for refs/foo/"). But I don't think it's sensible for anybody to go
to the work of doing that, given that I haven't heard a single useful
reason for --no-create-reflog in the first place.
Personally, I'd be fine with leaving it in its current state as a known
bug that somebody may fix later, if they actually care. But if it is not
too hard to fix while we are all thinking about it, we can do that.
-Peff
^ permalink raw reply
* Re: [PATCH 4/4] git-prompt.sh: add tests for submodule indicator
From: SZEDER Gábor @ 2017-01-31 18:32 UTC (permalink / raw)
To: Benjamin Fuchs; +Cc: git, Stefan Beller, brian m. carlson, ville.skytta
In-Reply-To: <1485809065-11978-5-git-send-email-email@benjaminfuchs.de>
On Mon, Jan 30, 2017 at 9:44 PM, Benjamin Fuchs <email@benjaminfuchs.de> wrote:
> Signed-off-by: Benjamin Fuchs <email@benjaminfuchs.de>
> ---
> t/t9903-bash-prompt.sh | 43 +++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 43 insertions(+)
>
> diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh
> index 97c9b32..4dce366 100755
> --- a/t/t9903-bash-prompt.sh
> +++ b/t/t9903-bash-prompt.sh
> @@ -37,6 +37,11 @@ test_expect_success 'setup for prompt tests' '
> git commit -m "yet another b2" file &&
> mkdir ignored_dir &&
> echo "ignored_dir/" >>.gitignore &&
> + git checkout -b submodule &&
> + git submodule add ./. sub &&
./. ?
> + git -C sub checkout master &&
> + git add sub &&
> + git commit -m submodule &&
> git checkout master
> '
>
> @@ -755,4 +760,42 @@ test_expect_success 'prompt - hide if pwd ignored - inside gitdir (stderr)' '
> test_cmp expected "$actual"
> '
>
> +test_expect_success 'prompt - submodule indicator' '
> + printf " (sub:master)" >expected &&
> + git checkout submodule &&
> + test_when_finished "git checkout master" &&
> + (
> + cd sub &&
> + GIT_PS1_SHOWSUBMODULE=1 &&
> + __git_ps1 >"$actual"
> + ) &&
> + test_cmp expected "$actual"
> +'
> +
> +test_expect_success 'prompt - submodule indicator - verify false' '
I was puzzled by the "verify false" here. You mean "disabled", right?
> + printf " (master)" >expected &&
> + git checkout submodule &&
> + test_when_finished "git checkout master" &&
> + (
> + cd sub &&
> + GIT_PS1_SHOWSUBMODULE= &&
> + __git_ps1 >"$actual"
> + ) &&
> + test_cmp expected "$actual"
> +'
> +
> +test_expect_success 'prompt - submodule indicator - dirty status indicator' '
> + printf " (+sub:b1)" >expected &&
> + git checkout submodule &&
> + git -C sub checkout b1 &&
> + test_when_finished "git checkout master" &&
> + (
> + cd sub &&
> + GIT_PS1_SHOWSUBMODULE=1 &&
> + __git_ps1 >"$actual"
> + ) &&
> + test_cmp expected "$actual"
> +'
> +
> +
> test_done
> --
> 2.7.4
>
^ permalink raw reply
* [PATCH] .mailmap: update Gábor Szeder's email address
From: SZEDER Gábor @ 2017-01-31 18:42 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, SZEDER Gábor
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
---
.mailmap | 1 +
1 file changed, 1 insertion(+)
diff --git a/.mailmap b/.mailmap
index 9c87a3840..ab59b2fac 100644
--- a/.mailmap
+++ b/.mailmap
@@ -225,6 +225,7 @@ Steven Walter <stevenrwalter@gmail.com> <swalter@lexmark.com>
Steven Walter <stevenrwalter@gmail.com> <swalter@lpdev.prtdev.lexmark.com>
Sven Verdoolaege <skimo@kotnet.org> <Sven.Verdoolaege@cs.kuleuven.ac.be>
Sven Verdoolaege <skimo@kotnet.org> <skimo@liacs.nl>
+SZEDER Gábor <szeder.dev@gmail.com> <szeder@ira.uka.de>
Tay Ray Chuan <rctay89@gmail.com>
Ted Percival <ted@midg3t.net> <ted.percival@quest.com>
Theodore Ts'o <tytso@mit.edu>
--
2.11.0.555.g967c1bcb3
^ permalink raw reply related
* Re: [PATCH v3 14/21] read-cache: touch shared index files when used
From: Junio C Hamano @ 2017-01-31 19:22 UTC (permalink / raw)
To: Christian Couder
Cc: Duy Nguyen, git, Ævar Arnfjörð Bjarmason,
Christian Couder
In-Reply-To: <CAP8UFD3iWg5g-h209VDyg1U03Jmma8nTONyCYB-kN7GwspkL8Q@mail.gmail.com>
Christian Couder <christian.couder@gmail.com> writes:
>> You are listing only the irrelevant cases. The shared one may be
>> used immediately, and the user can keep using it for a while without
>> "touching".
>
> Now you are talking about a case where the shared index file can be
> used immediately and the user can keep using it.
> This is a different case than the case I was talking about (which is
> the case when the shared index file does not exist anymore).
Yes, as I said, you are listing irrelevant and uninteresting one.
If the shared index is already gone, reporting failure to touch it
is of no use---an attempt to read and use the split index that
depends on the shared index that is gone will fail anyway.
> In a previous email you wrote that if the file system is read only, it
> is a bad thing if we warn.
Yes, but you need to realize that "it is better not to bother users
with a report of failure to touch in read-only repository" and "we
ignore all failures".
IIUC, you attempt to touch the shared index even when you are
only reading the index, because you want to mark the fact that the
shared index is still being depended upon. And I tend to agree that
it is OK not to report a failure for that case. It is very similar
to a situation where you are asked to peek into your coworker's
repository, which you do not have write access to, and run "status".
The command first runs the equivalent of "update-index --refresh"
only in-core, and it attempts to write the updated index because
(1) it paid the cost of doing the refreshing already, and (2) if it
can write into the index, it will help future operations in the
repository. But it does not report a failure to write the index
exactly because it is merely doing an opportunistic write.
And in the "we read from the split index, and we attempt to touch
the underlying shared one to update its timestamp" case, it is OK
not to report if we failed to touch.
But you also attempt to touch the shared index when you are actually
writing out a new split index file based on it, no? The "you
created a ticking bomb" situation is where you fail to touch the
shared index for whatever reason, even when you managed to write the
new split index file.
We agreed that read-only operation should not nag, so it won't
trigger when you are peeking somebody else's repository to help him.
As I said, it is an uninteresting and irrelevant case---when your
read-only peeking did not add new information to be preserved, it is
less severe problem that you fail to reset the expiration. On the
other hand, if you added new information, i.e. wrote the split index
based on it, it is a good indication that the <split,shared> index
pair has information that is more valuable. We must warn in that
case.
> Now if you want to talk about the case when the shared index file has
> not been removed, but just chowned to "root", then I wonder how is it
> different from the case when the file system is read only.
The difference is that your code has enough information to notice
the case where you know your touch failed, you know that you wanted
to write (and the write succeeded), and yet you do NOT know why your
touch failed. That is the ticking bomb we need to warn the user
about.
^ permalink raw reply
* Re: [PATCH] blame: draft of line format
From: Jeff King @ 2017-01-31 19:41 UTC (permalink / raw)
To: Edmundo Carmona Antoranz; +Cc: git
In-Reply-To: <20170131022830.8538-1-eantoranz@gmail.com>
On Mon, Jan 30, 2017 at 08:28:30PM -0600, Edmundo Carmona Antoranz wrote:
> +static void pretty_info(char* revid, struct blame_entry *ent, struct strbuf *rev_buffer)
> +{
> + struct pretty_print_context ctx = {0};
> + struct rev_info rev;
> +
> + struct strbuf format = STRBUF_INIT;
> + strbuf_addstr(&format, format_line);
> + ctx.fmt = CMIT_FMT_USERFORMAT;
> + get_commit_format(format.buf, &rev);
> + pretty_print_commit(&ctx, ent->suspect->commit, rev_buffer);
> + strbuf_release(&format);
> +}
I think this may be less awkward if you use format_commit_message() as
the entry point. Then you do not need a rev_info struct at all, it
touches fewer global variables, etc.
I don't know if that would cause the other difficulties you mentioned,
though.
-Peff
^ permalink raw reply
* Re: [PATCH] Documentation: implement linkgit macro for Asciidoctor
From: Junio C Hamano @ 2017-01-31 20:20 UTC (permalink / raw)
To: brian m. carlson
Cc: Eric Wong, Jeff King, git, Johannes Schindelin,
Øyvind A. Holm
In-Reply-To: <20170127004050.23jrq5iqwfxcwmik@genre.crustytoothpaste.net>
"brian m. carlson" <sandals@crustytoothpaste.net> writes:
> On Thu, Jan 26, 2017 at 07:18:41PM +0000, Eric Wong wrote:
>> > Eric Wong <e@80x24.org> writes:
>> Junio C Hamano <gitster@pobox.com> wrote:
>> > + "<citerefentry>\n"
>> > + "<refentrytitle>#{target}</refentrytitle>"
>> > + "<manvolnum>#{attrs[1]}</manvolnum>\n"
>> > + "</citerefentry>\n"
>> > end
>>
>> You need the '\' at the end of those strings, it's not like C
>> since Ruby doesn't require semi-colons to terminate lines.
>> In other words, that should be:
>>
>> "<citerefentry>\n" \
>> "<refentrytitle>#{target}</refentrytitle>" \
>> "<manvolnum>#{attrs[1]}</manvolnum>\n" \
>> "</citerefentry>\n"
>>
>
> This change is fine with me.
OK, I just squashed the final one in. Will merge to 'next' shortly.
diff --git a/Documentation/Makefile b/Documentation/Makefile
index 11f76506b6..b21e5808b1 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -179,7 +179,8 @@ ASCIIDOC = asciidoctor
ASCIIDOC_CONF =
ASCIIDOC_HTML = xhtml5
ASCIIDOC_DOCBOOK = docbook45
-ASCIIDOC_EXTRA += -I. -rasciidoctor-extensions -alitdd='&\#x2d;&\#x2d;'
+ASCIIDOC_EXTRA += -I. -rasciidoctor-extensions
+ASCIIDOC_EXTRA += -alitdd='&\#x2d;&\#x2d;'
DBLATEX_COMMON =
endif
diff --git a/Documentation/asciidoctor-extensions.rb b/Documentation/asciidoctor-extensions.rb
index 09f7088eea..ec83b4959e 100644
--- a/Documentation/asciidoctor-extensions.rb
+++ b/Documentation/asciidoctor-extensions.rb
@@ -13,10 +13,10 @@ module Git
prefix = parent.document.attr('git-relative-html-prefix')
%(<a href="#{prefix}#{target}.html">#{target}(#{attrs[1]})</a>\n)
elsif parent.document.basebackend? 'docbook'
- %(<citerefentry>
-<refentrytitle>#{target}</refentrytitle><manvolnum>#{attrs[1]}</manvolnum>
-</citerefentry>
-)
+ "<citerefentry>\n" \
+ "<refentrytitle>#{target}</refentrytitle>" \
+ "<manvolnum>#{attrs[1]}</manvolnum>\n" \
+ "</citerefentry>\n"
end
end
end
^ permalink raw reply related
* Re: vger not relaying some of Junio's messages today?
From: Junio C Hamano @ 2017-01-31 20:27 UTC (permalink / raw)
To: Jeff King; +Cc: Eric Wong, git
In-Reply-To: <20170127040139.wz7xmizccjigz5ui@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Fri, Jan 27, 2017 at 03:57:53AM +0000, Eric Wong wrote:
>
>> I noticed both of these are are missing from my archives
>> (which rejects messages unless they come from vger):
>>
>> <xmqq1svp7lcs.fsf@gitster.mtv.corp.google.com>
>> <xmqqefzp1d1x.fsf@gitster.mtv.corp.google.com>
>
> I don't have them either, so presumably vger ate them (I usually delete
> my cc copies after reading and keep the list ones, and I now have
> neither).
>
>> Not sure if there's something up with vger or Junio's setup because
>> other messages (even from Junio) are getting through fine.
>
> Sporadic failures, especially on a single topic, imply that something in
> the content may triggered the taboo filter (though often that takes out
> replies, too, which this doesn't seem to have done).
I think I figured this out yesterday.
It was a dot somewhere in the name of a recipient on To:/Cc: line
that is NOT double-quoted. For this thread, brian's human-readable
name "brian m. carlson" was double-quoted in the original I
responded to and it also was double-quoted in my response.
But there was another recipient with a dot after the middle initial,
which was NOT double-quoted in the original.
My response seems to have been eaten because that name was not
double-quoted. I saw that happen yesterday in another thread with
the same recipient, and resent with the only change to double-quote
that name and it went through.
^ permalink raw reply
* Re: [PATCH v3 2/3] refs: add option core.logAllRefUpdates = always
From: Cornelius Weig @ 2017-01-31 20:28 UTC (permalink / raw)
To: Junio C Hamano, Jeff King; +Cc: git
In-Reply-To: <xmqqbmunrwbf.fsf@gitster.mtv.corp.google.com>
On 01/31/2017 06:08 PM, Junio C Hamano wrote:
> I think it is probably a good idea to document the behaviour
> (i.e. "--no-create" single-shot from the command line is ignored).
> I am not sure we should error out, though, in order to "disallow"
> it---a documented silent no-op may be sufficient.
Yes, maybe abort on seeing "--no-create-reflog" is a too drastic
measure. I presume that the best place to have the documentation would
be to print a warning when seeing the ignored argument?
Or did you just have man pages and code comment in mind?
Cheers,
Cornelius
^ permalink raw reply
* Re: [PATCH 1/5] add SWAP macro
From: René Scharfe @ 2017-01-31 21:02 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Git List, Junio C Hamano
In-Reply-To: <alpine.DEB.2.20.1701311305030.3469@virtualbox>
Am 31.01.2017 um 13:13 schrieb Johannes Schindelin:
> Hi René,
>
> On Mon, 30 Jan 2017, René Scharfe wrote:
>
>> Am 30.01.2017 um 21:48 schrieb Johannes Schindelin:
>>
>>> The commit you quoted embarrasses me, and I have no excuse for it. I
>>> would love to see that myswap() ugliness fixed by replacing it with a
>>> construct that is simpler, and generates good code even without any
>>> smart compiler.
>>
>> I don't see a way to do that without adding a type parameter.
>
> Exactly. And you know what? I would be very okay with that type parameter.
>
> Coccinelle [*1*] should be able to cope with that, too, mehtinks.
Yes, a semantic patch can turn the type of the temporary variable into a
macro parameter. Programmers would have to type the type, though,
making the macro only half as good.
> It would be trivially "optimized" out of the box, even when compiling with
> Tiny C or in debug mode.
Such a compiler is already slowed down by memset(3) calls for
initializing objects and lack of other optimizations. I doubt a few
more memcpy(3) calls would make that much of a difference.
NB: git as compiled with TCC fails several tests, alas. Builds wickedly
fast, though.
> And it would even allow things like this:
>
> #define SIMPLE_SWAP(T, a, b) do { T tmp_ = a; a = b; b = tmp_; } while (0)
> ...
> uint32_t large;
> char nybble;
>
> ...
>
> if (!(large & ~0xf)) {
> SIMPLE_SWAP(char, nybble, large);
> ...
> }
>
> i.e. mixing types, when possible.
>
> And while I do not necessarily expect that we need anything like this
> anytime soon, merely the fact that it allows for this flexibility, while
> being very readable at the same time, would make it a pretty good design
> in my book.
Such a skinny macro which only hides repetition is kind of attractive
due to its simplicity; I can't say the same about the mixed type example
above, though.
The fat version isn't that bad either even without inlining, includes a
few safety checks and doesn't require us to tell the compiler something
it already knows very well. I'd rather let the machine do the work.
René
^ permalink raw reply
* Re: [PATCH 3/5] use SWAP macro
From: René Scharfe @ 2017-01-31 21:02 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git List, Johannes Schindelin
In-Reply-To: <xmqqr33ktch9.fsf@gitster.mtv.corp.google.com>
Am 30.01.2017 um 23:22 schrieb Junio C Hamano:
> René Scharfe <l.s.r@web.de> writes:
>
>> if (tree2->flags & UNINTERESTING) {
>> - struct object *tmp = tree2;
>> - tree2 = tree1;
>> - tree1 = tmp;
>> + SWAP(tree2, tree1);
>
> A human would have written this SWAP(tree1, tree2).
>
> Not that I think such a manual fix-up should be made in _this_
> patch, which may end up mixing mechanical conversion (which we may
> want to keep reproducible) and hand tweaks. But this swapped swap
> reads somewhat silly.
Well, a human wrote "tmp = tree2" -- sometimes one likes to count down
instead of up, I guess.
We can make Coccinelle order the parameters alphabetically, but I don't
know how to do so without duplicating most of the semantic patch. And
thats would result in e.g. SWAP(new_tree, old_tree), which might be odd
as well.
I'd just leave them in whatever order they were, but that's probably
because I'm lazy.
René
^ permalink raw reply
* Re: [PATCH 1/5] add SWAP macro
From: René Scharfe @ 2017-01-31 21:03 UTC (permalink / raw)
To: Brandon Williams
Cc: Johannes Schindelin, Johannes Sixt, Git List, Junio C Hamano
In-Reply-To: <20170130222157.GC35626@google.com>
Am 30.01.2017 um 23:21 schrieb Brandon Williams:
> On 01/30, René Scharfe wrote:
>> Am 30.01.2017 um 22:03 schrieb Johannes Schindelin:
>>> It is curious, though, that an
>>> expression like "sizeof(a++)" would not be rejected.
>>
>> Clang normally warns about something like this ("warning: expression
>> with side effects has no effect in an unevaluated context
>> [-Wunevaluated-expression]"), but not if the code is part of a
>> macro. I don't know if that's intended, but it sure is helpful in
>> the case of SWAP.
>>
>>> Further, what would SWAP(a++, b) do? Swap a and b, and *then* increment a?
>>
>> That might be a valid expectation, but GCC says "error: lvalue
>> required as unary '&' operand" and clang puts it "error: cannot take
>> the address of an rvalue of type".
>>
>> René
>
> Perhaps we could disallow a side-effect operator in the macro. By
> disallow I mean place a comment at the definition to the macro and
> hopefully catch something like that in code-review. We have the same
> issue with the `ALLOC_GROW()` macro.
SWAP(a++, ...) is caught by the compiler, SWAP(*a++, ...) works fine.
Technically that should be enough. :) A comment wouldn't hurt, of course.
René
^ permalink raw reply
* Re: [PATCH 1/5] add SWAP macro
From: Jeff King @ 2017-01-31 21:35 UTC (permalink / raw)
To: René Scharfe
Cc: Brandon Williams, Johannes Schindelin, Johannes Sixt, Git List,
Junio C Hamano
In-Reply-To: <8e94756a-c3a5-9b81-268d-d0f36876f710@web.de>
On Tue, Jan 31, 2017 at 10:03:01PM +0100, René Scharfe wrote:
> > Perhaps we could disallow a side-effect operator in the macro. By
> > disallow I mean place a comment at the definition to the macro and
> > hopefully catch something like that in code-review. We have the same
> > issue with the `ALLOC_GROW()` macro.
>
> SWAP(a++, ...) is caught by the compiler, SWAP(*a++, ...) works fine.
> Technically that should be enough. :) A comment wouldn't hurt, of course.
One funny thing is that your macro takes address-of itself, behind the
scenes. I wonder if it would be more natural for it to take
pointers-to-objects, making it look more like a real function (i.e.,
SWAP(&a, &b) instead of SWAP(a, b)". And then these funny corner cases
become quite obvious in the caller, because the caller is the one who
has to type "&".
-Peff
^ permalink raw reply
* Re: [PATCH v3 14/21] read-cache: touch shared index files when used
From: Junio C Hamano @ 2017-01-31 21:09 UTC (permalink / raw)
To: Christian Couder
Cc: Duy Nguyen, git, Ævar Arnfjörð Bjarmason,
Christian Couder
In-Reply-To: <xmqqy3xrqbkr.fsf@gitster.mtv.corp.google.com>
Junio C Hamano <gitster@pobox.com> writes:
> Yes, but you need to realize that "it is better not to bother users
> with a report of failure to touch in read-only repository" and "we
> ignore all failures".
Sorry about an unfinished sentence here. "need to realize that
... and ... are different things."
> ... It is very similar
> to a situation where you ... run "status".
> The command first runs the equivalent of "update-index --refresh"
> only in-core, and it attempts to write the updated index because
> (1) it paid the cost of doing the refreshing already, and (2) if it
> can write into the index, it will help future operations in the
> repository. But it does not report a failure to write the index
> exactly because it is merely doing an opportunistic write.
>
> And in the "we read from the split index, and we attempt to touch
> the underlying shared one to update its timestamp" case, it is OK
> not to report if we failed to touch.
> ...
> ... On the
> other hand, if you added new information, i.e. wrote the split index
> based on it, it is a good indication that the <split,shared> index
> pair has information that is more valuable. We must warn in that
> case.
This reminds us of a third case. What should happen if we are doing
the "opportunistic writing of the index" in "git status", managed to
write the split index, but failed to touch the shared one?
In the ideal world, I think we should do the following sequence:
- "status" tries to write cache to the file.
- we try to write and on any error, we return error to the caller,
who is already prepared to ignore it and stay silent.
- as the first step of writing the index, we first try to touch
the shared one. If it fails, we return an error here without
writing the split one out.
- then we try to write the split one out. If this fails, we
also return an error.
- otherwise, both touching of the shared one and writing of the
split one are successful.
- "status" finishes the opportunistic refreshing of the index, by
either ignoring an error silently (if either touching of shared
one or writing of split one fails) or writing the refreshed index
out successfully.
It is OK to swap the order of touching the shared one and writing of
the split one in the above sequence, as long as an error in either
step signals a failure to the opportunistic caller.
I do not offhand know if the split-index code is structured in such
a way to allow the above sequence easily, or it needs refactoring.
If such a restructuring is required, it might not be within the
scope of the series and I am OK if you just left the NEEDSWORK
comment that describes the above (i.e. what we should be doing) as
an in-code comment so that we can pick it up later. The whole point
of the step 14/21 on the other hand is to make sure that a shared
index that is still in active use will not go stale, and from that
point of view, such a "punting" may not be a good idea---it
deliberately finishes the series knowing that it does not adequately
do what it promises to do.
So, ... I dunno.
^ permalink raw reply
* Re: [PATCH v3 2/3] refs: add option core.logAllRefUpdates = always
From: Junio C Hamano @ 2017-01-31 22:02 UTC (permalink / raw)
To: Cornelius Weig; +Cc: Jeff King, git
In-Reply-To: <ce8f90a6-d719-63c7-95d0-b2538270e263@tngtech.com>
Cornelius Weig <cornelius.weig@tngtech.com> writes:
> On 01/31/2017 06:08 PM, Junio C Hamano wrote:
>> I think it is probably a good idea to document the behaviour
>> (i.e. "--no-create" single-shot from the command line is ignored).
>> I am not sure we should error out, though, in order to "disallow"
>> it---a documented silent no-op may be sufficient.
>
> Yes, maybe abort on seeing "--no-create-reflog" is a too drastic
> measure. I presume that the best place to have the documentation would
> be to print a warning when seeing the ignored argument?
>
> Or did you just have man pages and code comment in mind?
I meant only in the documentation, but "you gave me a no-op option"
warning would not hurt. I do not care too deeply either way.
^ permalink raw reply
* Re: git push failing when push.recurseSubmodules on-demand and git commit --amend was used in submodule.
From: Stefan Beller @ 2017-01-31 22:08 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Carlo Wood, Heiko Voigt, git@vger.kernel.org
In-Reply-To: <xmqqvasxwee1.fsf@gitster.mtv.corp.google.com>
On Sun, Jan 29, 2017 at 5:00 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Carlo Wood <carlo@alinoe.com> writes:
>
>> there seems to be a problem with using 'git commit --amend' in
>> git submodules when using 'git push --recurse-submodules=on-demand'
>> in the parent.
>>
>> The latter fails, saying "The following submodule paths contain changes
>> that can not be found on any remote:" for such submodule, even though
>> the submodule is clean, pushed and reports 'Everything up-to-date'
>> when trying to push it.
>>
>> I believe that the reason has to be that the parent repository thinks
>> that the comment that was amended, but not pushed, must be on the remote
>> too, while the whole point of amend is that this commit is not pushed.
>
> I am not super familiar with the actualy implementation of the
> codepaths involved in this, so CC'ed the folks who can help you
> better.
>
> I suspect the submodule folks would say it is working as intended,
> if \
>
> - you made a commit in the submodule;
> - recorded the resulting commit in the superproject;
> - you amended the commit in the submodule; and then
> - you did "push, while pushing out in the submodule as needed" from
> the superproject.
Yes, for the current state of affairs, this is it.
>
> There are two commits in the submodule that are involved in the
> above scenario, and the first one before amending is needed by the
> other participants of the project in order for them to check out
> what you are trying to push in the superproject, because that is
> what the superproject's tree records. You somehow need to make that
> commit available to them, but after you amended, the original commit
> may no longer be reachable from any branch in your submodule, so
> even if you (or the "on-demand" mechanism) pushed any and all
> branches out, that would not make the needed commit available to
> others. If you push your top-level superproject out in such a
> situation, you would break others.
In the long term future, we may want to reject non-fastforward
submodule updates. (Not sure if that is feasible)
>
> I think you have two options.
>
> 1. If the amend was done to improve things in submodule but is not
> quite ready, then get rid of that amended commit and restore the
> branch in the submodule to the state before you amended, i.e.
> the tip of the branch will become the same commit as the one
> that is recorded in the superproject. Then push the submodule
> and the superproject out. After that, move the submodule branch
> to point at the amended commit (or record the amended commit as
> a child of the commit you pushed out).
>
> 2. If the amend is good and ready to go, "git add" to update the
> superproject to make that amended result the one that is needed
> in the submodule.
yup.
^ permalink raw reply
* Re: gitconfig get out of sync with submodule entries on branch switch
From: Stefan Beller @ 2017-01-31 22:04 UTC (permalink / raw)
To: Benjamin Schindler; +Cc: Brandon Williams, git@vger.kernel.org
In-Reply-To: <b614a44a-fbc6-b5fe-ae40-ccf43dd9fcfb@gmail.com>
On Mon, Jan 30, 2017 at 11:46 PM, Benjamin Schindler
<beschindler@gmail.com> wrote:
> Hi Brandon
>
> I did try your suggestion, so basically:
>
> git checkout branch
> git submodule init
> git submodule update
Eventually this becomes
git submudule update --init
git checkout --recurse-submodules $branch
>
> Unfortunately, I still have two entries in my git config this way. It seems
> that git submodule update only considers submodules listed in .gitmodules.
Did you rename the name in the gitmodules file or rename the path on the FS?
>
> The background of my question is this - we have a jenkins farm which needs
> to switch branches continuously in an automated fashion and this needs to
> work even in when submodules are around. I did however, just now, find a
> reliable way to switch a branch, keeping the gitconfig in sync:
> The basic workflow for switching a branch is:
> git submodule deinit .
This will become 'git submodule deinit --all' eventually
> git checkout branch
> git submodule init
> git submodule update
This ought to update all the submodules, sounds fine to me.
>
> Because the .git folder of the submodules are not within the submodule
> directories, this is, while still quite heavy-handed, reasonably fast and
> robust. At least it is better than deleting the entire repository every time
> a branch switch is issued.
In the next version there will be 'git submodule absorbgitdirs'
which moves the git dirs of submodules inside the superproject; would
a reversion of this also be useful?
>
> Regards
>
> Benjamin Schindler
>
>
> On 30.01.2017 18:51, Brandon Williams wrote:
>>
>> On 01/30, Benjamin Schindler wrote:
>>>
>>> Hi
>>>
>>> Consider the following usecase: I have the master branch where I
>>> have a submodule A. I create a branch where I rename the submodule
>>> to be in the directory B. After doing all of this, everything looks
>>> good.
>>> Now, I switch back to master. The first oddity is, that it fails to
>>> remove the folder B because there are still files in there:
>>>
>>> bschindler@metis ~/Projects/submodule_test (testbranch) $ git
>>> checkout master
>>> warning: unable to rmdir other_submodule: Directory not empty
>>> Switched to branch 'master'
>>>
>>> Git submodule deinit on B fails because the submodule is not known
>>> to git anymore (after all, the folder B exists only in the other
>>> branch). I can easily just remove the folder B from disk and
>>> initialize the submodule A again, so all seems good.
>>>
>>> However, what is not good is that the submodule b is still known in
>>> .git/config. This is in particular a problem for us, because I know
>>> a number of tools which use git config to retrieve the submodule
>>> list. Is it therefore a bug that upon branch switch, the submodule
>>> gets deregistered, but its entry in .git/config remains?
>>>
>>> thanks a lot
>>> Benjamin Schindler
>>>
>>> P.s. I did not subscribe to the mailing list, please add me at least
>>> do CC. Thanks
>>
>> submodules and checkout don't really play nicely with each other at the
>> moment. Stefan (cc'd) is currently working on a patch series to improve
>> the behavior of checkout with submodules. Currently, if you want to
>> ensure you have a good working state after a checkout you should run
>> `git submodule update` to update all of the submoules. As far as your
>> submodule still being listed in the config, that should be expected
>> given the scenario you described.
>>
>> If I'm understanding you correctly, A and B are both the same submodule
>> just renamed on a different branch. The moment you add a submoule to a
>> repository it is given a name which is fixed. Typically this is the
>> path from the root of the repository. The thing is, since you are able
>> to freely move a submodule, its path can change. To account for this
>> there is the .gitmodules file which allows you to do a lookup from
>> submodule name to the path at which it exists (or vice versa). The
>> submodules that are stored in .git/config are those which are
>> 'initialize' or rather the submodules in which you are interested in and
>> will be updated by `git submodule update`. So given your scenario you
>> should only have a single submodule in .git/config and the .gitmodules
>> file should have a single entry with a differing path for each branch.
>>
>> Hopefully this gives you a bit more information to work with. Since
>> Stefan has been working with this more recently than me he may have some
>> more input.
>>
>
^ permalink raw reply
* Re: [PATCH 4/4] git-prompt.sh: add tests for submodule indicator
From: Stefan Beller @ 2017-01-31 22:12 UTC (permalink / raw)
To: Junio C Hamano
Cc: SZEDER Gábor, Benjamin Fuchs, git@vger.kernel.org,
brian m. carlson, ville.skytta
In-Reply-To: <xmqq37fyriji.fsf@gitster.mtv.corp.google.com>
On Tue, Jan 31, 2017 at 2:06 PM, Junio C Hamano <gitster@pobox.com> wrote:
> SZEDER Gábor <szeder.dev@gmail.com> writes:
>
>> On Mon, Jan 30, 2017 at 9:44 PM, Benjamin Fuchs <email@benjaminfuchs.de> wrote:
>>> Signed-off-by: Benjamin Fuchs <email@benjaminfuchs.de>
>>> ---
>>> t/t9903-bash-prompt.sh | 43 +++++++++++++++++++++++++++++++++++++++++++
>>> 1 file changed, 43 insertions(+)
>>>
>>> diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh
>>> index 97c9b32..4dce366 100755
>>> --- a/t/t9903-bash-prompt.sh
>>> +++ b/t/t9903-bash-prompt.sh
>>> @@ -37,6 +37,11 @@ test_expect_success 'setup for prompt tests' '
>>> git commit -m "yet another b2" file &&
>>> mkdir ignored_dir &&
>>> echo "ignored_dir/" >>.gitignore &&
>>> + git checkout -b submodule &&
>>> + git submodule add ./. sub &&
>>
>> ./. ?
>
> Good eyes. This is a pattern we are trying to wean ourselves off
> of. E.g. cf.
>
> https://public-inbox.org/git/20170105192904.1107-2-sbeller@google.com/#t
>
> Hopefully this reminds us to resurrect and finish the test fixes in
> that thread?
I plan to eventually, yes. but that is a refactoring, that has lower prio
than getting checkout working recursing into submodules.
^ permalink raw reply
* Re: [PATCH 4/4] git-prompt.sh: add tests for submodule indicator
From: Junio C Hamano @ 2017-01-31 22:06 UTC (permalink / raw)
To: SZEDER Gábor
Cc: Benjamin Fuchs, git, Stefan Beller, brian m. carlson,
ville.skytta
In-Reply-To: <CAM0VKj=j8Fy8AQvYbbvwPf5kkV1GYYONADNsQO5RDNTUzdYt8w@mail.gmail.com>
SZEDER Gábor <szeder.dev@gmail.com> writes:
> On Mon, Jan 30, 2017 at 9:44 PM, Benjamin Fuchs <email@benjaminfuchs.de> wrote:
>> Signed-off-by: Benjamin Fuchs <email@benjaminfuchs.de>
>> ---
>> t/t9903-bash-prompt.sh | 43 +++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 43 insertions(+)
>>
>> diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh
>> index 97c9b32..4dce366 100755
>> --- a/t/t9903-bash-prompt.sh
>> +++ b/t/t9903-bash-prompt.sh
>> @@ -37,6 +37,11 @@ test_expect_success 'setup for prompt tests' '
>> git commit -m "yet another b2" file &&
>> mkdir ignored_dir &&
>> echo "ignored_dir/" >>.gitignore &&
>> + git checkout -b submodule &&
>> + git submodule add ./. sub &&
>
> ./. ?
Good eyes. This is a pattern we are trying to wean ourselves off
of. E.g. cf.
https://public-inbox.org/git/20170105192904.1107-2-sbeller@google.com/#t
Hopefully this reminds us to resurrect and finish the test fixes in
that thread?
^ permalink raw reply
* Re: [PATCH v2 7/7] completion: recognize more long-options
From: SZEDER Gábor @ 2017-01-31 22:17 UTC (permalink / raw)
To: Cornelius Weig; +Cc: j6t, spearce, git
In-Reply-To: <20170127211703.24910-2-cornelius.weig@tngtech.com>
On Fri, Jan 27, 2017 at 10:17 PM, <cornelius.weig@tngtech.com> wrote:
> From: Cornelius Weig <cornelius.weig@tngtech.com>
>
> Recognize several new long-options for bash completion in the following
> commands:
Adding more long options that git commands learn along the way is
always an improvement. However, seeing "_several_ new long options"
(or "some long options" in one of the other patches in the series)
makes the reader wonder: are these the only new long options missing
or are there more? If there are more, why only these are added? If
there aren't any more missing long options left, then please say so,
e.g. "Add all missing long options, except the potentially
desctructive ones, for the following commands: ...."
> - apply: --recount --directory=
> - archive: --output
> - branch: --column --no-column --sort= --points-at
> - clone: --no-single-branch --shallow-submodules
> - commit: --patch --short --date --allow-empty
> - describe: --first-parent
> - fetch, pull: --unshallow --update-shallow
> - fsck: --name-objects
> - grep: --break --heading --show-function --function-context
> --untracked --no-index
> - mergetool: --prompt --no-prompt
> - reset: --keep
> - revert: --strategy= --strategy-option=
> - rm: --force
'--force' is a potentially destructive option, too.
> - shortlog: --email
> - tag: --merged --no-merged --create-reflog
>
> Signed-off-by: Cornelius Weig <cornelius.weig@tngtech.com>
> Helped-by: Johannes Sixt <j6t@kdbg.org>
> ---
> contrib/completion/git-completion.bash | 31 +++++++++++++++++++++----------
> 1 file changed, 21 insertions(+), 10 deletions(-)
>
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index 0e09519..933bb6e 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -936,6 +936,7 @@ _git_apply ()
> --apply --no-add --exclude=
> --ignore-whitespace --ignore-space-change
> --whitespace= --inaccurate-eof --verbose
> + --recount --directory=
> "
> return
> esac
> @@ -974,7 +975,7 @@ _git_archive ()
> --*)
> __gitcomp "
> --format= --list --verbose
> - --prefix= --remote= --exec=
> + --prefix= --remote= --exec= --output
> "
> return
> ;;
> @@ -1029,6 +1030,7 @@ _git_branch ()
> --track --no-track --contains --merged --no-merged
> --set-upstream-to= --edit-description --list
> --unset-upstream --delete --move --remotes
> + --column --no-column --sort= --points-at
> "
> ;;
> *)
> @@ -1142,6 +1144,8 @@ _git_clone ()
> --single-branch
> --branch
> --recurse-submodules
> + --no-single-branch
> + --shallow-submodules
> "
> return
> ;;
> @@ -1183,6 +1187,7 @@ _git_commit ()
> --reset-author --file= --message= --template=
> --cleanup= --untracked-files --untracked-files=
> --verbose --quiet --fixup= --squash=
> + --patch --short --date --allow-empty
> "
> return
> esac
> @@ -1201,7 +1206,7 @@ _git_describe ()
> --*)
> __gitcomp "
> --all --tags --contains --abbrev= --candidates=
> - --exact-match --debug --long --match --always
> + --exact-match --debug --long --match --always --first-parent
> "
> return
> esac
> @@ -1284,6 +1289,7 @@ __git_fetch_recurse_submodules="yes on-demand no"
> __git_fetch_options="
> --quiet --verbose --append --upload-pack --force --keep --depth=
> --tags --no-tags --all --prune --dry-run --recurse-submodules=
> + --unshallow --update-shallow
> "
>
> _git_fetch ()
> @@ -1333,7 +1339,7 @@ _git_fsck ()
> --*)
> __gitcomp "
> --tags --root --unreachable --cache --no-reflogs --full
> - --strict --verbose --lost-found
> + --strict --verbose --lost-found --name-objects
> "
> return
> ;;
> @@ -1377,6 +1383,8 @@ _git_grep ()
> --max-depth
> --count
> --and --or --not --all-match
> + --break --heading --show-function --function-context
> + --untracked --no-index
> "
> return
> ;;
> @@ -1576,7 +1584,7 @@ _git_mergetool ()
> return
> ;;
> --*)
> - __gitcomp "--tool="
> + __gitcomp "--tool= --prompt --no-prompt"
> return
> ;;
> esac
> @@ -2456,7 +2464,7 @@ _git_reset ()
>
> case "$cur" in
> --*)
> - __gitcomp "--merge --mixed --hard --soft --patch"
> + __gitcomp "--merge --mixed --hard --soft --patch --keep"
> return
> ;;
> esac
> @@ -2472,7 +2480,10 @@ _git_revert ()
> fi
> case "$cur" in
> --*)
> - __gitcomp "--edit --mainline --no-edit --no-commit --signoff"
> + __gitcomp "
> + --edit --mainline --no-edit --no-commit --signoff
> + --strategy= --strategy-option=
> + "
> return
> ;;
> esac
> @@ -2483,7 +2494,7 @@ _git_rm ()
> {
> case "$cur" in
> --*)
> - __gitcomp "--cached --dry-run --ignore-unmatch --quiet"
> + __gitcomp "--cached --dry-run --ignore-unmatch --quiet --force"
> return
> ;;
> esac
> @@ -2500,7 +2511,7 @@ _git_shortlog ()
> __gitcomp "
> $__git_log_common_options
> $__git_log_shortlog_options
> - --numbered --summary
> + --numbered --summary --email
> "
> return
> ;;
> @@ -2778,8 +2789,8 @@ _git_tag ()
> --*)
> __gitcomp "
> --list --delete --verify --annotate --message --file
> - --sign --cleanup --local-user --force --column --sort
> - --contains --points-at
> + --sign --cleanup --local-user --force --column --sort=
> + --contains --points-at --merged --no-merged --create-reflog
> "
> ;;
> esac
> --
> 2.10.2
>
^ permalink raw reply
* Re: [PATCH 1/5] add SWAP macro
From: Junio C Hamano @ 2017-01-31 22:29 UTC (permalink / raw)
To: Jeff King
Cc: René Scharfe, Brandon Williams, Johannes Schindelin,
Johannes Sixt, Git List
In-Reply-To: <20170131213507.uiwmkkcg7umvd3f4@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> ... I wonder if it would be more natural for it to take
> pointers-to-objects, making it look more like a real function (i.e.,
> SWAP(&a, &b) instead of SWAP(a, b)". And then these funny corner cases
> become quite obvious in the caller, because the caller is the one who
> has to type "&".
Hmmmm.
While this looks very attractive in theory by forcing 'a' and 'b' to
be lvalues, it probably invites mistakes go unnoticed during the
review when the code wants to swap two pointer variables.
For example,
apply.c: SWAP(p->new_name, p->old_name);
is now a bug and will swap only the first byte of these names, and
the correct way to spell it would become:
apply.c: SWAP(&p->new_name, &p->old_name);
The latter clearly looks like swapping the new and old names, which
is good, but I do not have any confidence that I will immediately
spot a bug when presented the former under the new world order.
^ 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