* [PATCH] sequencer: shut up clang warning
From: Johannes Schindelin @ 2016-11-09 13:56 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Torsten Bögershausen, Lars Schneider,
Jeff King
[-- Attachment #1: Type: text/plain, Size: 1731 bytes --]
When comparing a value of type `enum todo_command` with a value that is
outside the defined enum constants, clang greets the developer with this
warning:
comparison of constant 2 with expression of type
'const enum todo_command' is always true
While this is arguably true *iff* the value was never cast from a
free-form int, we should keep the cautious code in place.
To shut up clang, we simply introduce an otherwise pointless enum constant
and compare against that.
Noticed by Torsten Bögershausen.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
Published-As: https://github.com/dscho/git/releases/tag/enum-warning-v1
Fetch-It-Via: git fetch https://github.com/dscho/git enum-warning-v1
Peff, if you would like me to include more of your commit message,
please let me know.
I will adjust the sequencer-i patches to keep the TODO_INVALID
constant.
sequencer.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index 5fd75f3..f80e9c0 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -619,7 +619,8 @@ static int allow_empty(struct replay_opts *opts, struct commit *commit)
enum todo_command {
TODO_PICK = 0,
- TODO_REVERT
+ TODO_REVERT,
+ TODO_INVALID
};
static const char *todo_command_strings[] = {
@@ -629,7 +630,7 @@ static const char *todo_command_strings[] = {
static const char *command_to_string(const enum todo_command command)
{
- if (command < ARRAY_SIZE(todo_command_strings))
+ if (command < TODO_INVALID)
return todo_command_strings[command];
die("Unknown command: %d", command);
}
base-commit: be5a750939c212bc0781ffa04fabcfd2b2bd744e
--
2.10.1.583.g721a9e0
^ permalink raw reply related
* Re: [PATCH] sequencer: silence -Wtautological-constant-out-of-range-compare
From: Johannes Schindelin @ 2016-11-09 13:54 UTC (permalink / raw)
To: Jeff King; +Cc: git, Lars Schneider
In-Reply-To: <20161109035728.v2mqvtj4ep4dj74j@sigill.intra.peff.net>
Hi Peff,
On Tue, 8 Nov 2016, Jeff King wrote:
> diff --git a/sequencer.c b/sequencer.c
> index 5fd75f30d..6f0ff9e41 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -629,7 +629,7 @@ static const char *todo_command_strings[] = {
>
> static const char *command_to_string(const enum todo_command command)
> {
> - if (command < ARRAY_SIZE(todo_command_strings))
> + if ((size_t)command < ARRAY_SIZE(todo_command_strings))
> return todo_command_strings[command];
> die("Unknown command: %d", command);
I have come to prefer a slightly different approach. Will send it out in a
moment.
Ciao,
Dscho
^ permalink raw reply
* Re: Bug: git config does not respect read-only .gitconfig file
From: Jonathan Word @ 2016-11-09 13:51 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, Markus Hitter, git, jword
In-Reply-To: <xmqqk2cdbg5v.fsf@gitster.mtv.corp.google.com>
> It is unreasonable to drop the write-enable bit of
> a file in a writable directory and expect it to stay unmodified. The
> W-bit on the file is not usable as a security measure, and we do not
> use it as such.
The point here is not a matter of security - it is of expectations.
When a user drops write access on the global ~/.gitconfig I think
a reasonable user would expect future `git config --global` calls to
fail by default. The possibility of an override is a different matter,
and my initial proposal included the details of enabling direct
override. I don't think there is any presumption that this is a
security related discussion.
> I do not offhand know how much a new feature "this repository can be
> modified by pushing into and fetching from, but its configuration
> cannot be modified" is a sensible thing to have.
I agree that per-repository files almost never run into an issue with
this. Our problem is strictly with the global ~/.gitconfig which in our
use case is owned by a shared system account and used implicitly
by many developers. Thus any one of those devs can call
`git config` without any signal that they are changing something
that ought not to be changed and should think carefully.
This would be equivalent to dropping write access to a file that
your account owns so that vi / emacs / etc.. will warn that the
file is read-only before modifying it (useful for any number of
sensitive files). Obviously from a security perspective you have
a number of means of potential override, however all require additional
steps that surface the initial intention that the file should not
change - or should only change rarely after additional confirmation.
> perhaps the lock_file()
> function can have access(path, W_OK) check before it returns a
> tempfile that has been successfully opened?
That sounds ideal
On Tue, Nov 8, 2016 at 8:22 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Jeff King <peff@peff.net> writes:
>
>> Probably converting "rename(from, to)" to first check "access(to,
>> W_OK)". That's racy, but it's the best we could do.
>
> Hmph, if these (possibly problematic) callers are all following the
> usual "lock, write to temp, rename" pattern, perhaps the lock_file()
> function can have access(path, W_OK) check before it returns a
> tempfile that has been successfully opened?
>
> Having said that, I share your assessment that this is not a code or
> design problem. It is unreasonable to drop the write-enable bit of
> a file in a writable directory and expect it to stay unmodified. The
> W-bit on the file is not usable as a security measure, and we do not
> use it as such.
>
> I do not offhand know how much a new feature "this repository can be
> modified by pushing into and fetching from, but its configuration
> cannot be modified" is a sensible thing to have. But it is quite
> clear that even if we were to implement such feature, we wouldn't be
> using W-bit on .git/config to signal that.
>
^ permalink raw reply
* [PATCH] t6026: ensure that long-running script really is
From: Johannes Schindelin @ 2016-11-09 13:51 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Johannes Sixt, Jeff King, Andreas Schwab
When making sure that background tasks are cleaned up in 5babb5b
(t6026-merge-attr: clean up background process at end of test case,
2016-09-07), we considered to let the background task sleep longer, just
to be certain that it will still be running when we want to kill it
after the test.
Sadly, the assumption appears not to hold true that the test case passes
quickly enough to kill the background task within a second.
Simply increase it to an hour. No system can be possibly slow enough to
make above-mentioned assumption incorrect.
Reported by Andreas Schwab.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
Published-As: https://github.com/dscho/git/releases/tag/t6026-sleep-v1
Fetch-It-Via: git fetch https://github.com/dscho/git t6026-sleep-v1
t/t6026-merge-attr.sh | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/t/t6026-merge-attr.sh b/t/t6026-merge-attr.sh
index 7a6e33e..348d78b 100755
--- a/t/t6026-merge-attr.sh
+++ b/t/t6026-merge-attr.sh
@@ -183,16 +183,16 @@ test_expect_success 'up-to-date merge without common ancestor' '
test_expect_success 'custom merge does not lock index' '
git reset --hard anchor &&
- write_script sleep-one-second.sh <<-\EOF &&
- sleep 1 &
+ write_script sleep-an-hour.sh <<-\EOF &&
+ sleep 3600 &
echo $! >sleep.pid
EOF
test_when_finished "kill \$(cat sleep.pid)" &&
test_write_lines >.gitattributes \
- "* merge=ours" "text merge=sleep-one-second" &&
+ "* merge=ours" "text merge=sleep-an-hour" &&
test_config merge.ours.driver true &&
- test_config merge.sleep-one-second.driver ./sleep-one-second.sh &&
+ test_config merge.sleep-an-hour.driver ./sleep-an-hour.sh &&
git merge master
'
base-commit: be5a750939c212bc0781ffa04fabcfd2b2bd744e
--
2.10.1.583.g721a9e0
^ permalink raw reply related
* Re: [PATCH] t6026-merge-attr: don't fail if sleep exits early
From: Johannes Schindelin @ 2016-11-09 13:47 UTC (permalink / raw)
To: Jeff King; +Cc: Andreas Schwab, Johannes Sixt, git
In-Reply-To: <20161108200543.7ivo3xoafdl4uw6h@sigill.intra.peff.net>
Hi
On Tue, 8 Nov 2016, Jeff King wrote:
> On Tue, Nov 08, 2016 at 06:03:04PM +0100, Andreas Schwab wrote:
>
> > Commit 5babb5bdb3 ("t6026-merge-attr: clean up background process at end
> > of test case") added a kill command to clean up after the test, but this
> > can fail if the sleep command exits before the cleanup is executed.
> > Ignore the error from the kill command.
> >
> > Signed-off-by: Andreas Schwab <schwab@suse.de>
> > ---
> > The failure can be simulated by adding a sleep after the last command to
> > delay the cleanup.
>
> Thanks for the reproduction hint. I sometimes run the test suite through
> a "stress" script that sees if a test script racily fails under load,
> but I wasn't able to trigger it here (I guess a full second is just too
> long even under high load). But the extra sleep makes it obvious.
>
> Looks like the original is in v2.10.1, so this should probably go to
> maint, as well as the upcoming v2.11-rc1.
The reason why we do not ignore kill errors is that we want to make sure
that the script *actually ran*. Otherwise, the thing we need to test here
does not necessarily get tested.
So I would rather go with the patch Hannes hinted at when he said that he
did not want to change the file name (as it would have made the diff less
obvious): increase the number of seconds.
Will send out a superseding patch in a minute,
Dscho
^ permalink raw reply
* Re: [PATCH 0/3] gitk: memory consumption improvements
From: Markus Hitter @ 2016-11-09 12:39 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git@vger.kernel.org, Paul Mackerras
In-Reply-To: <xmqqtwbhbql9.fsf@gitster.mtv.corp.google.com>
Am 08.11.2016 um 22:37 schrieb Junio C Hamano:
> Are all semi-modern Tcl/Tk in service have this -undo thing so that
> we can pass unconditionally to the text widget like the patch does?
Good point. As far as my research goes, this flag was introduced in Nov. 2001:
http://core.tcl.tk/tk/info/5265df93d207cec0
To defend Gitk developers, the Tk guys apparently change their mind on the default value from time to time. Official documentation says nothing about a default, the proposal from 2001 talks about -undo 0 as default and there are recent commits changing this default:
http://core.tcl.tk/tk/info/549d2f56757408f3
Markus
--
- - - - - - - - - - - - - - - - - - -
Dipl. Ing. (FH) Markus Hitter
http://www.jump-ing.de/
^ permalink raw reply
* Re: [PATCH 5/6] config docs: Provide for config to specify tags not to abbreviate
From: Ian Jackson @ 2016-11-09 10:51 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, Jacob Keller, Git mailing list, Paul Mackerras
In-Reply-To: <xmqqa8d9b3jh.fsf@gitster.mtv.corp.google.com>
Junio C Hamano writes ("Re: [PATCH 5/6] config docs: Provide for config to specify tags not to abbreviate"):
> And I do not think we would want "log" or any core side Porcelain
> command to have too many "information losing" options like this
> "truncate refnames down to a point where it is no longer unique and
> meaningful". GUI tools can get away with doing sos because they can
> arrange these truncated labels to react to end-user input (e.g. the
> truncated Tag in the history display of gitk could be made to react
> to mouse-over and pop-up to show a full name, for example), but the
> output from the core side is pretty much fixed once it is emitted.
>
> So my first preference would be to teach gitk such a "please
> clarify" UI-reaction, if it does not know how to do so yet. There
> is no need for a configuration variable anywhere with this approach.
gitk already has a way for the user to find out what the elided tag
names are. The underlying difficulty is that the situation that the
gitk behaviour is designed for (long tag names, perhaps several to a
commit, not particularly interesting), is not applicable to these
particular tags.
Whether the tag is `particularly interesting' depends, as I say, on
both what tree it is in, and on its name. It might be appropriate for
terminal-based tools to highlight these tags too, or show them when
tags are not normally displayed.
`core.interestingTags' ?
> If you do want to add a configuration to show fuller name in the
> tag, which would make it unnecessary for the user to do "please
> clarify, as I am hovering over what I want to get details of"
> action, that may also be a good way to go.
I think in my use case, which I hope to become common within Debian,
this is going to be essential.
> But I think the right
> place to do so would be Edit -> Preferences menu in Gitk, and the
> settings will be stored in ~/.gitk or ~/.config/git/gitk or whatever
> gitk-specific place.
This is not correct, because as I have explained, this should be a
per-tree configuration:
If it can't be a `git config' option, even `git config gui.something',
then I guess I will have to teach gitk to read a config file in
GIT_DIR too. But I think that is silly given that git already has a
config file reading system which handles per-tree configs.
If we can't get agreement from the git-core developers on a config to
be used, and documented, for any tool which has similar behaviour, I
think the right answer is `git config gitk.<something>', which would
be documented in gitk.
Thanks,
Ian.
--
Ian Jackson <ijackson@chiark.greenend.org.uk> These opinions are my own.
If I emailed you from an address @fyvzl.net or @evade.org.uk, that is
a private address which bypasses my fierce spamfilter.
^ permalink raw reply
* Re: [PATCH v1 1/2] config.mak.in: set NO_OPENSSL and APPLE_COMMON_CRYPTO for macOS >10.11
From: Torsten Bögershausen @ 2016-11-09 10:51 UTC (permalink / raw)
To: Lars Schneider; +Cc: Jeff King, git, gitster
In-Reply-To: <0966CBE3-2F08-4B89-9716-4EEE3CE2526E@gmail.com>
On 09.11.16 10:29, Lars Schneider wrote:
>
>> On 09 Nov 2016, at 09:18, Torsten Bögershausen <tboegi@web.de> wrote:
>>
>> On 07.11.16 18:26, Jeff King wrote:
>>> On Sun, Nov 06, 2016 at 08:35:04PM +0100, Lars Schneider wrote:
>>>
>>>> Good point. I think I found an even easier way to achieve the same.
>>>> What do you think about the patch below?
>>>>
>>>> [...]
>>>>
>>>> diff --git a/Makefile b/Makefile
>>>> index 9d6c245..f53fcc9 100644
>>>> --- a/Makefile
>>>> +++ b/Makefile
>>>> @@ -1047,6 +1047,7 @@ ifeq ($(uname_S),Darwin)
>>>> endif
>>>> endif
>>>> ifndef NO_APPLE_COMMON_CRYPTO
>>>> + NO_OPENSSL = YesPlease
>>>> APPLE_COMMON_CRYPTO = YesPlease
>>>> COMPAT_CFLAGS += -DAPPLE_COMMON_CRYPTO
>>>> endif
>>>
>>> That is much simpler.
>> []
>> I don't know if that is a correct solution.
>>
>> If I have Mac OS 10.12 and Mac Ports installed, I may want to use
>> OPENSSL from Mac Ports.
>
> Can't you define `NO_APPLE_COMMON_CRYPTO` in that case?
> I think if you use OpenSSL then you don't need the Apple crypto lib, right?
After re-reading the Makefile: that makes sense :-)
Do you want to send a new patch ?
Feel free to omit
"Original-patch-by: Torsten Bögershausen <tboegi@web.de>"
^ permalink raw reply
* Re: [PATCHv2 32/36] pathspec: allow querying for attributes
From: Duy Nguyen @ 2016-11-09 9:57 UTC (permalink / raw)
To: Stefan Beller; +Cc: Junio C Hamano, Brandon Williams, Git Mailing List
In-Reply-To: <20161028185502.8789-33-sbeller@google.com>
On Sat, Oct 29, 2016 at 1:54 AM, Stefan Beller <sbeller@google.com> wrote:
> The pathspec mechanism is extended via the new
> ":(attr:eol=input)pattern/to/match" syntax to filter paths so that it
> requires paths to not just match the given pattern but also have the
> specified attrs attached for them to be chosen.
>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
> Documentation/glossary-content.txt | 20 +++++
> dir.c | 35 ++++++++
Pathspec can be processed in a couple more places. The big two are
match_pathspec and tree_entry_interesting, the former traverses a list
while the latter does a tree. You don't have to implement attr
matching in tree_entry_interesting right now because nobody needs it,
probably. But you need to make sure if somebody accidentally calls
tree_entry_interesting with an attr pathspec, then it should
die("BUG"), not silently ignore attr.
The way to do that is GUARD_PATHSPEC macro which makes sure if only
recognized magic is allowed through. This macro guards all pathspec
processing functions. So you can add a new PATHSPEC_ATTR macro (or
some other name) to the "Pathspec magic" group near the beginning of
pathspec.h, set it whenever attr magic is present when you
parse_pathspec(), then lift the GUARD_PATHSPEC restriction in
match_pathspec() only because this function can handle it. Whenever
attr magic is used by any other functions, it will die() the way we
want.
--
Duy
^ permalink raw reply
* Re: [PATCH 32/36] pathspec: allow querying for attributes
From: Duy Nguyen @ 2016-11-09 9:45 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Stefan Beller, Git Mailing List, Brandon Williams
In-Reply-To: <xmqqzilp63yh.fsf@gitster.mtv.corp.google.com>
On Fri, Oct 28, 2016 at 1:29 AM, Junio C Hamano <gitster@pobox.com> wrote:
> The reason why I am bringing this up in this discussion thread on
> this patch is because I wonder if we would benefit by a similar
> "let's not do too involved things and be cheap by erring on the safe
> and lazy side" strategy in the call to ce_path_match() call made in
> this function to avoid making calls to the attr subsystem.
>
> In other words, would it help the system by either simplifying the
> processing done or reducing the cycle spent in preload_thread() if
> we could tell ce_path_match() "A pathspec we are checking may
> require not just the pattern to match but also attributes given to
> the path to satisfy the criteria, but for the purpose of preloading,
> pretend that the attribute satisfies the match criteria" (or
> "pretend that it does not match"), thereby not having to make any
> call into the attribute subsystem at all from this codepath?
>
> The strategy this round takes to make it unnecessary to punt
> preloading (i.e. dropping "pathspec: disable preload-index when
> attribute pathspec magic is in use" patch the old series had) is to
> make the attribute subsystem thread-safe. But that thread-safety in
> the initial round is based on a single Big Attribute Lock, so it may
> turn out that the end result performs better for this codepath if we
> did not to make any call into the attribute subsystem.
It does sound good and I want to say "yes please do this", but is it
making pathspec api a bit more complex (to express "assume all
attr-related criteria match")? I guess we can have an api to simply
filter out attr-related magic (basically set attr_match_nr back to
zero) then pass a safe (but more relaxing) pathspec to the threaded
code. That would not add big maintenance burden.
> I am probably being two step ahead of ourselves by saying the above,
> which is just something to keep in mind as a possible solution if
> performance in this preload codepath becomes an issue when the
> pathspec has attributes match specified.
--
Duy
^ permalink raw reply
* Re: [PATCH v1 1/2] config.mak.in: set NO_OPENSSL and APPLE_COMMON_CRYPTO for macOS >10.11
From: Lars Schneider @ 2016-11-09 9:29 UTC (permalink / raw)
To: Torsten Bögershausen; +Cc: Jeff King, git, gitster
In-Reply-To: <11cc8bbd-2e67-f53a-c8f4-2244409fd6af@web.de>
> On 09 Nov 2016, at 09:18, Torsten Bögershausen <tboegi@web.de> wrote:
>
> On 07.11.16 18:26, Jeff King wrote:
>> On Sun, Nov 06, 2016 at 08:35:04PM +0100, Lars Schneider wrote:
>>
>>> Good point. I think I found an even easier way to achieve the same.
>>> What do you think about the patch below?
>>>
>>> [...]
>>>
>>> diff --git a/Makefile b/Makefile
>>> index 9d6c245..f53fcc9 100644
>>> --- a/Makefile
>>> +++ b/Makefile
>>> @@ -1047,6 +1047,7 @@ ifeq ($(uname_S),Darwin)
>>> endif
>>> endif
>>> ifndef NO_APPLE_COMMON_CRYPTO
>>> + NO_OPENSSL = YesPlease
>>> APPLE_COMMON_CRYPTO = YesPlease
>>> COMPAT_CFLAGS += -DAPPLE_COMMON_CRYPTO
>>> endif
>>
>> That is much simpler.
> []
> I don't know if that is a correct solution.
>
> If I have Mac OS 10.12 and Mac Ports installed, I may want to use
> OPENSSL from Mac Ports.
Can't you define `NO_APPLE_COMMON_CRYPTO` in that case?
I think if you use OpenSSL then you don't need the Apple crypto lib, right?
- Lars
> How about this:
>
>
> diff --git a/Makefile b/Makefile
> index ee89c06..e93511f 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1038,17 +1038,22 @@ ifeq ($(uname_S),Darwin)
> ifeq ($(shell test -d /sw/lib && echo y),y)
> BASIC_CFLAGS += -I/sw/include
> BASIC_LDFLAGS += -L/sw/lib
> + HAS_OPENSSL = Yes
> endif
> endif
> ifndef NO_DARWIN_PORTS
> ifeq ($(shell test -d /opt/local/lib && echo y),y)
> BASIC_CFLAGS += -I/opt/local/include
> BASIC_LDFLAGS += -L/opt/local/lib
> + HAS_OPENSSL = Yes
> endif
> endif
> ifndef NO_APPLE_COMMON_CRYPTO
> APPLE_COMMON_CRYPTO = YesPlease
> COMPAT_CFLAGS += -DAPPLE_COMMON_CRYPTO
> + ifndef HAS_OPENSSL
> + NO_OPENSSL = YesPlease
> + endif
> endif
> NO_REGEX = YesPlease
> PTHREAD_LIBS =
>
^ permalink raw reply
* Re: [PATCH v1 03/19] split-index: add {add,remove}_split_index() functions
From: Duy Nguyen @ 2016-11-09 9:24 UTC (permalink / raw)
To: Christian Couder
Cc: Git Mailing List, Junio C Hamano,
Ævar Arnfjörð Bjarmason, Christian Couder
In-Reply-To: <CACsJy8DjXrOgB-_-t47uSdCQFg9s_o+Oj9NBmAhDFZ3aYvjBgg@mail.gmail.com>
On Mon, Nov 7, 2016 at 5:08 PM, Duy Nguyen <pclouds@gmail.com> wrote:
> On Sun, Oct 30, 2016 at 5:06 AM, Christian Couder
> <christian.couder@gmail.com> wrote:
>> On Tue, Oct 25, 2016 at 11:58 AM, Duy Nguyen <pclouds@gmail.com> wrote:
>>> On Sun, Oct 23, 2016 at 4:26 PM, Christian Couder
>>> <christian.couder@gmail.com> wrote:
>>>> +void remove_split_index(struct index_state *istate)
>>>> +{
>>>> + if (istate->split_index) {
>>>> + /*
>>>> + * can't discard_split_index(&the_index); because that
>>>> + * will destroy split_index->base->cache[], which may
>>>> + * be shared with the_index.cache[]. So yeah we're
>>>> + * leaking a bit here.
>>>
>>> In the context of update-index, this is a one-time thing and leaking
>>> is tolerable. But because it becomes a library function now, this leak
>>> can become more serious, I think.
>>>
>>> The only other (indirect) caller is read_index_from() so probably not
>>> bad most of the time (we read at the beginning of a command only).
>>> sequencer.c may discard and re-read the index many times though,
>>> leaking could be visible there.
>>
>> So is it enough to check if split_index->base->cache[] is shared with
>> the_index.cache[] and then decide if discard_split_index(&the_index)
>> should be called?
>
> It's likely shared though. We could un-share cache[] by duplicating
> index entries in the_index.cache[] if they point back to
> split_index->base
I have a patch for this. So don't have to do anything else in this
area. I'll probably just pile my patch on top of your series, or post
it once the series graduates to master.
--
Duy
^ permalink raw reply
* Re: [PATCH 4/5] attr: do not respect symlinks for in-tree .gitattributes
From: Duy Nguyen @ 2016-11-09 9:22 UTC (permalink / raw)
To: Jeff King; +Cc: Git Mailing List
In-Reply-To: <20161108222127.mejb74maewzhn3qg@sigill.intra.peff.net>
On Wed, Nov 9, 2016 at 5:21 AM, Jeff King <peff@peff.net> wrote:
> On Tue, Nov 08, 2016 at 08:38:55AM +0700, Duy Nguyen wrote:
>
>> > Another approach is to have a config option to disallow symlinks to
>> > destinations outside of the repository tree (I'm not sure if it should
>> > be on or off by default, though).
>>
>> Let's err on the safe side and disable symlinks to outside repo by
>> default (or even all symlinks on .gitattributes and .gitignore as the
>> first step)
>
> Both of those are actually much harder than you might think.
>
> For matching specific names, we have to deal with case-folding. It's
> easy to hit the common ones like ".GITIGNORE" with fspathcmp(). But if
> this is actually protection against malicious repositories, we have to
> match all of the horrible filesystem-specific junk that we did for
> ".git".
We could realpath() it and check if the result path is inside
realpath($GIT_WORK_TREE). The real work would be done by OS. We will
need to check if it points to .git/something, but I think we have that
covered. The approach is a bit heavy for such a sanity check though
> Symlinks are likewise tricky. If we see that a symlink points to
> "foo/../bar", then we don't know if it leaves the repository unless we
> also look at "foo" to see if it is also a symlink. So you really end up
> having to resolve the symlink yourself (and when checking out multiple
> files, there's an ordering dependency).
We do have this dependency problem right now (e.g. files A and
.gitattributes are checked out at the same time and .gitattributes has
some attribute on A). It looks like we resolve it by reading the index
version at checkout time. We probably can do the same for gitattribute
symlinks.
> I think it might be enough to check:
>
> - leading "../" tokens in the symlink's destination can be checked
> against the symlink's path. So "../foo" is OK for path "one/two",
> but not for path "one".
>
> - interior "../" can be disallowed entirely. Technically
> "foo/../bar/../baz" _can_ be a fine symlink destination, but why?
> It's identical to "baz" unless you are following a bunch of interior
> symlinks. And if those are interior symlinks, it's still confusing
> and unnecessarily obfuscated, and a good sign that somebody is
> trying to do something tricky.
Sounds good.
> So one reasonable fix might be to have a config option like
> "core.saneSymlinks" that enforces both of those rules for _all_ symlinks
> that we checkout to the working tree. And it could either refuse to
> check them out, or replace them with a file containing the symlink
> content (as we do on systems that don't support symlinks, IIRC).
I wonder if anyone want core.saneSymlinks on, but they have some links
that do not meet the above checks and still want to follow them
anyway. One way to add such an exception is mark the path with an
attribute "follow". Yeah I have a dependency loop :(
--
Duy
^ permalink raw reply
* [PATCH v2] rebase: add --forget to cleanup rebase, leave everything else untouched
From: Nguyễn Thái Ngọc Duy @ 2016-11-09 9:11 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
In-Reply-To: <20161026094658.20704-1-pclouds@gmail.com>
There are occasions when you decide to abort an in-progress rebase and
move on to do something else but you forget to do "git rebase --abort"
first. Or the rebase has been in progress for so long you forgot about
it. By the time you realize that (e.g. by starting another rebase)
it's already too late to retrace your steps. The solution is normally
rm -r .git/<some rebase dir>
and continue with your life. But there could be two different
directories for <some rebase dir> (and it obviously requires some
knowledge of how rebase works), and the ".git" part could be much
longer if you are not at top-dir, or in a linked worktree. And
"rm -r" is very dangerous to do in .git, a mistake in there could
destroy object database or other important data.
Provide "git rebase --forget" for this exact use case.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
v2 changes just the subject line
Documentation/git-rebase.txt | 5 ++++-
contrib/completion/git-completion.bash | 4 ++--
git-rebase.sh | 6 +++++-
t/t3407-rebase-abort.sh | 24 ++++++++++++++++++++++++
4 files changed, 35 insertions(+), 4 deletions(-)
diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index de222c8..5a58fb3 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -12,7 +12,7 @@ SYNOPSIS
[<upstream> [<branch>]]
'git rebase' [-i | --interactive] [options] [--exec <cmd>] [--onto <newbase>]
--root [<branch>]
-'git rebase' --continue | --skip | --abort | --edit-todo
+'git rebase' --continue | --skip | --abort | --forget | --edit-todo
DESCRIPTION
-----------
@@ -252,6 +252,9 @@ leave out at most one of A and B, in which case it defaults to HEAD.
will be reset to where it was when the rebase operation was
started.
+--forget::
+ Abort the rebase operation but leave HEAD where it is.
+
--keep-empty::
Keep the commits that do not change anything from its
parents in the result.
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 21016bf..3143cb0 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1734,10 +1734,10 @@ _git_rebase ()
{
local dir="$(__gitdir)"
if [ -f "$dir"/rebase-merge/interactive ]; then
- __gitcomp "--continue --skip --abort --edit-todo"
+ __gitcomp "--continue --skip --abort --forget --edit-todo"
return
elif [ -d "$dir"/rebase-apply ] || [ -d "$dir"/rebase-merge ]; then
- __gitcomp "--continue --skip --abort"
+ __gitcomp "--continue --skip --abort --forget"
return
fi
__git_complete_strategy && return
diff --git a/git-rebase.sh b/git-rebase.sh
index 04f6e44..de712b7 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -43,6 +43,7 @@ continue! continue
abort! abort and check out the original branch
skip! skip current patch and continue
edit-todo! edit the todo list during an interactive rebase
+forget! abort but keep HEAD where it is
"
. git-sh-setup
set_reflog_action rebase
@@ -241,7 +242,7 @@ do
--verify)
ok_to_skip_pre_rebase=
;;
- --continue|--skip|--abort|--edit-todo)
+ --continue|--skip|--abort|--forget|--edit-todo)
test $total_argc -eq 2 || usage
action=${1##--}
;;
@@ -399,6 +400,9 @@ abort)
finish_rebase
exit
;;
+forget)
+ exec rm -rf "$state_dir"
+ ;;
edit-todo)
run_specific_rebase
;;
diff --git a/t/t3407-rebase-abort.sh b/t/t3407-rebase-abort.sh
index a6a6c40..6bc5e71 100755
--- a/t/t3407-rebase-abort.sh
+++ b/t/t3407-rebase-abort.sh
@@ -99,4 +99,28 @@ testrebase() {
testrebase "" .git/rebase-apply
testrebase " --merge" .git/rebase-merge
+test_expect_success 'rebase --forget' '
+ cd "$work_dir" &&
+ # Clean up the state from the previous one
+ git reset --hard pre-rebase &&
+ test_must_fail git rebase master &&
+ test_path_is_dir .git/rebase-apply &&
+ head_before=$(git rev-parse HEAD) &&
+ git rebase --forget &&
+ test $(git rev-parse HEAD) = $head_before &&
+ test ! -d .git/rebase-apply
+'
+
+test_expect_success 'rebase --merge --forget' '
+ cd "$work_dir" &&
+ # Clean up the state from the previous one
+ git reset --hard pre-rebase &&
+ test_must_fail git rebase --merge master &&
+ test_path_is_dir .git/rebase-merge &&
+ head_before=$(git rev-parse HEAD) &&
+ git rebase --forget &&
+ test $(git rev-parse HEAD) = $head_before &&
+ test ! -d .git/rebase-merge
+'
+
test_done
--
2.8.2.524.g6ff3d78
^ permalink raw reply related
* Re: Forbid access to /gitweb but authorize the sub projets
From: Dennis Kaarsemaker @ 2016-11-09 8:25 UTC (permalink / raw)
To: Alexandre Duplaix, git
In-Reply-To: <58207CAB.3060105@sagemcom.com>
On Mon, 2016-11-07 at 14:07 +0100, Alexandre Duplaix wrote:
> Hello,
>
> I have several projects under https://myserver/gitweb and I would like
> to forbid the access to the root, so that the users can't list the
> differents projects.
>
> However, I need to let the access to the sub projects (ex:
> https://myserver/gitweb/?p=project1;a=summary
>
> How can I do please ?
My favourite way of doing this is abusing the fact that gitweb.conf is
perl code that's loaded with do $filename.
This makes it easy to override such things. Try this in gitweb.conf for example:
sub no_index {
die_error(403, "No access to the repository list");
}
$actions{project_list} = \&no_index;
$actions{project_index} = \&no_index;
$actions{opml} = \&no_index;
--
Dennis Kaarsemaker
http://www.kaarsemaker.net
^ permalink raw reply
* Re: [PATCH v1 1/2] config.mak.in: set NO_OPENSSL and APPLE_COMMON_CRYPTO for macOS >10.11
From: Torsten Bögershausen @ 2016-11-09 8:18 UTC (permalink / raw)
To: Jeff King, Lars Schneider; +Cc: git, gitster
In-Reply-To: <20161107172617.tlcrpwbjy2w7aoyc@sigill.intra.peff.net>
On 07.11.16 18:26, Jeff King wrote:
> On Sun, Nov 06, 2016 at 08:35:04PM +0100, Lars Schneider wrote:
>
>> Good point. I think I found an even easier way to achieve the same.
>> What do you think about the patch below?
>>
>> [...]
>>
>> diff --git a/Makefile b/Makefile
>> index 9d6c245..f53fcc9 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -1047,6 +1047,7 @@ ifeq ($(uname_S),Darwin)
>> endif
>> endif
>> ifndef NO_APPLE_COMMON_CRYPTO
>> + NO_OPENSSL = YesPlease
>> APPLE_COMMON_CRYPTO = YesPlease
>> COMPAT_CFLAGS += -DAPPLE_COMMON_CRYPTO
>> endif
>
> That is much simpler.
[]
I don't know if that is a correct solution.
If I have Mac OS 10.12 and Mac Ports installed, I may want to use
OPENSSL from Mac Ports.
How about this:
diff --git a/Makefile b/Makefile
index ee89c06..e93511f 100644
--- a/Makefile
+++ b/Makefile
@@ -1038,17 +1038,22 @@ ifeq ($(uname_S),Darwin)
ifeq ($(shell test -d /sw/lib && echo y),y)
BASIC_CFLAGS += -I/sw/include
BASIC_LDFLAGS += -L/sw/lib
+ HAS_OPENSSL = Yes
endif
endif
ifndef NO_DARWIN_PORTS
ifeq ($(shell test -d /opt/local/lib && echo y),y)
BASIC_CFLAGS += -I/opt/local/include
BASIC_LDFLAGS += -L/opt/local/lib
+ HAS_OPENSSL = Yes
endif
endif
ifndef NO_APPLE_COMMON_CRYPTO
APPLE_COMMON_CRYPTO = YesPlease
COMPAT_CFLAGS += -DAPPLE_COMMON_CRYPTO
+ ifndef HAS_OPENSSL
+ NO_OPENSSL = YesPlease
+ endif
endif
NO_REGEX = YesPlease
PTHREAD_LIBS =
^ permalink raw reply related
* Re: 2.11.0-rc1 will not be tagged for a few days
From: Junio C Hamano @ 2016-11-09 6:29 UTC (permalink / raw)
To: Jeff King; +Cc: Johannes Sixt, git, Johannes Schindelin, Lars Schneider
In-Reply-To: <20161108214825.yo37kvoqkeucuqgg@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> I'm collecting v2.11-rc1 topics in the "refs/heads/for-junio/" section
> of git://github.com/peff/git.git.
>
> I've also got proposed merges for "master" there, though note that none
> of the topics has actually cooked at all in next (the fixes are trivial
> enough that it may be OK, though).
I am not quite back up at full steam yet, but I expect I'd be more
or less functioning tomorrow, so I'll fetch them from your tree and
continue.
Thanks.
^ permalink raw reply
* Re: [PATCH v5 01/16] Git.pm: add subroutines for commenting lines
From: Junio C Hamano @ 2016-11-09 1:06 UTC (permalink / raw)
To: Vasco Almeida
Cc: git, Jiang Xin, Ævar Arnfjörð Bjarmason,
Jean-Noël AVILA, Jakub Narębski, David Aguilar
In-Reply-To: <20161108120823.11204-2-vascomalmeida@sapo.pt>
Vasco Almeida <vascomalmeida@sapo.pt> writes:
> Add subroutines prefix_lines and comment_lines.
>
> Signed-off-by: Vasco Almeida <vascomalmeida@sapo.pt>
> ---
> perl/Git.pm | 23 +++++++++++++++++++++++
> 1 file changed, 23 insertions(+)
>
> diff --git a/perl/Git.pm b/perl/Git.pm
> index b2732822a..17be59fb7 100644
> --- a/perl/Git.pm
> +++ b/perl/Git.pm
> @@ -1438,6 +1438,29 @@ sub END {
>
> } # %TEMP_* Lexical Context
>
> +=item prefix_lines ( PREFIX, STRING )
> +
> +Prefixes lines in C<STRING> with C<PREFIX>.
> +
> +=cut
> +
> +sub prefix_lines {
> + my ($prefix, $string) = @_;
> + $string =~ s/^/$prefix/mg;
> + return $string;
> +}
> +
> +=item comment_lines ( STRING )
> +
> +Comments lines following core.commentchar configuration.
> +
> +=cut
> +
> +sub comment_lines {
> + my $comment_line_char = config("core.commentchar") || '#';
> + return prefix_lines("$comment_line_char ", @_);
> +}
> +
This makes it appear as if comment_lines can take arbitrary number
of strings as its arguments (because the outer caller just passes @_
thru), but in fact because prefix_lines ignores anything other than
$_[0] and $_[1], only the first parameter given to comment_lineS sub
is inspected for lines in it and the prefix-char prefixed at the
beginning of each of them.
Which is not a great interface, as it is quite misleading.
Perhaps
prefix_lines("#", join("\n", @_));
or something like that may make it less confusing.
^ permalink raw reply
* Re: git submodule add broken (2.11.0-rc1): Cannot open git-sh-i18n
From: Junio C Hamano @ 2016-11-08 21:31 UTC (permalink / raw)
To: Stefan Beller; +Cc: Anthony Sottile, git@vger.kernel.org
In-Reply-To: <CAGZ79kbyM0ssz3JgeNHsUpvHnBsCbhm-tHvHRp3+6O1QvmkYpw@mail.gmail.com>
Stefan Beller <sbeller@google.com> writes:
> On Mon, Nov 7, 2016 at 10:30 AM, Anthony Sottile <asottile@umich.edu> wrote:
>> This has worked great up until now (and is very convenient for trying things
>> out without blowing away the system installation). What changed?
>>
>
> (Just guessing myself:)
>
> $ git log --grep git-sh-i18n v2.10.0..v2.11.0-rc0
> ...
> commit 1073094f30a8dd5ae49f2146f587085c4fe86410
> Author: Anders Kaseorg <andersk@mit.edu>
> Date: Sat Oct 29 22:10:02 2016 -0400
>
> git-sh-setup: be explicit where to dot-source git-sh-i18n from.
>
> d323c6b641 ("i18n: git-sh-setup.sh: mark strings for translation",
> 2016-06-17) started to dot-source git-sh-i18n shell script library,
> assuming that $PATH is already adjusted for our scripts, namely,
> $GIT_EXEC_PATH is at the beginning of $PATH.
> ...
Before that one since v2.10.0, there ie d323c6b641 that starts to
include git-sh-i18n from git-sh-setup in the first place. If you are
testing or using a newly-out-of-oven uninstalled Git, you would need
to do these to be correctly using it:
* Many subcommand executables and helpers are not installed on any
directory on your $PATH, but are installed in GIT_EXEC_PATH.
Find out where it is by asking the newly-out-of-oven Git "git
--exec-path" without setting GIT_EXEC_PATH environment variable.
That is where your newly built one wants to find things.
* Because you are trying to use the newly-out-of-oven Git without
installing, you do not want to install into the real
GIT_EXEC_PATH location yet. Pick a new empty directory and
arrange the files that would be installed by our Makefile into
"git --exec-path" if you did "make install" to be in this new
location. The set of files include git-sh-setup and git-sh-i18n.
* Set and export GIT_EXEC_PATH to point at this new empty directory
you just populated.
That incidentally is how we make our "make test" work.
If you only set $PATH to the top of git build directory, without
doing the above arrangement with GIT_EXEC_PATH, things may have
appeared to work due to multitude of accidents. Builtin commands
would worked fine because it is just a single "git" binary after
all, i.e. "git log" would not have consulted a "git-log" binary on
anywhere in your filesystem. Other commands may have been run from
the already installed version on the system. The latter is
particularly problematic, because it means that one thought that one
is testing the newly built on before installing to make sure that
the new one works OK, but is actually testing the already installed
one.
^ permalink raw reply
* Re: [PATCH v4 1/2] lib-proto-disable: variable name fix
From: Junio C Hamano @ 2016-11-08 20:52 UTC (permalink / raw)
To: Jacob Keller
Cc: Jeff King, Brandon Williams, Git mailing list, Stefan Beller,
bburky, Jonathan Nieder
In-Reply-To: <CA+P7+xohFeWb1hsOg1_T1kWenc=AKrt1TNcx=TrVYQ+w3+c63Q@mail.gmail.com>
Jacob Keller <jacob.keller@gmail.com> writes:
> On Mon, Nov 7, 2016 at 12:48 PM, Jeff King <peff@peff.net> wrote:
>> It's possible that I'm overly picky about my commit messages, but that
>> does not stop me from trying to train an army of picky-commit-message
>> clones. :)
>>
>> -Peff
>
> You're not the only one ;)
Somebody seems to have trained y'all very well ;-)
^ permalink raw reply
* Re: [PATCH 5/6] config docs: Provide for config to specify tags not to abbreviate
From: Junio C Hamano @ 2016-11-09 5:55 UTC (permalink / raw)
To: Ian Jackson; +Cc: Jeff King, Jacob Keller, Git mailing list, Paul Mackerras
In-Reply-To: <22562.32428.287354.214659@chiark.greenend.org.uk>
Ian Jackson <ijackson@chiark.greenend.org.uk> writes:
>> I think the two things I found weird were:
>>
>> - it's in the "log" section, which makes me think it's an option for
>> git-log. But it's not. I'm not sure what the _right_ section is, but
>> hopefully it would make it clear that this is command-agnostic.
>>
>> Something like "gui.abbrevTags" might be OK (and as you note, has
>> precedence). But of course it's possible that a command like "tig"
>> could learn to support it. I'm not sure if that counts as a GUI or
>> not. :)
>
> I don't really have an opinion about the name. gui.abbrevTags would
> be a possibility. (It's a bit odd that implicitly, the default would
> be `*'.)
I have trouble with both "log" and "abbrev" in the name. Perhaps I
am biased by our recent discussion on a feature in the core that we
use the word "abbrev" to describe, but I fear that most Git users,
when told the word, would imagine the act of shortening 40-hex full
object name down to shorter but still unique prefix, not the "this
refname is too long, so let's show only the first few letters in GUI
label".
And I do not think we would want "log" or any core side Porcelain
command to have too many "information losing" options like this
"truncate refnames down to a point where it is no longer unique and
meaningful". GUI tools can get away with doing sos because they can
arrange these truncated labels to react to end-user input (e.g. the
truncated Tag in the history display of gitk could be made to react
to mouse-over and pop-up to show a full name, for example), but the
output from the core side is pretty much fixed once it is emitted.
So my first preference would be to teach gitk such a "please
clarify" UI-reaction, if it does not know how to do so yet. There
is no need for a configuration variable anywhere with this approach.
If you do want to add a configuration to show fuller name in the
tag, which would make it unnecessary for the user to do "please
clarify, as I am hovering over what I want to get details of"
action, that may also be a good way to go. But I think the right
place to do so would be Edit -> Preferences menu in Gitk, and the
settings will be stored in ~/.gitk or ~/.config/git/gitk or whatever
gitk-specific place.
^ permalink raw reply
* [PATCH] sequencer: silence -Wtautological-constant-out-of-range-compare
From: Jeff King @ 2016-11-09 3:57 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin, Lars Schneider
When clang compiles sequencer.c, it complains:
sequencer.c:632:14: warning: comparison of constant 2 with
expression of type 'const enum todo_command' is always
true [-Wtautological-constant-out-of-range-compare]
if (command < ARRAY_SIZE(todo_command_strings))
This is because "command" is an enum that may only have two
values (0 and 1) and the array in question has two elements.
As it turns out, clang is actually wrong here, at least
according to its own bug tracker:
https://llvm.org/bugs/show_bug.cgi?id=16154
But it's still worth working around this, as the warning is
present with -Wall, meaning we fail compilation with "make
DEVELOPER=1".
Casting the enum to size_t sufficiently unconfuses clang. As
a bonus, it also catches any possible out-of-bounds access
if the enum takes on a negative value (which shouldn't
happen either, but again, this is a defensive check).
Signed-off-by: Jeff King <peff@peff.net>
---
I know that a different fix is coming in a follow-on series, but I think
it's worth doing this to un-break clang on master (and v2.11) in the
meantime.
sequencer.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sequencer.c b/sequencer.c
index 5fd75f30d..6f0ff9e41 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -629,7 +629,7 @@ static const char *todo_command_strings[] = {
static const char *command_to_string(const enum todo_command command)
{
- if (command < ARRAY_SIZE(todo_command_strings))
+ if ((size_t)command < ARRAY_SIZE(todo_command_strings))
return todo_command_strings[command];
die("Unknown command: %d", command);
}
--
2.11.0.rc0.263.g6f44bc3
^ permalink raw reply related
* Re: Bug: git config does not respect read-only .gitconfig file
From: Jeff King @ 2016-11-09 3:34 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jonathan Word, Markus Hitter, git, jword
In-Reply-To: <xmqqk2cdbg5v.fsf@gitster.mtv.corp.google.com>
On Tue, Nov 08, 2016 at 05:22:52PM -0800, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > Probably converting "rename(from, to)" to first check "access(to,
> > W_OK)". That's racy, but it's the best we could do.
>
> Hmph, if these (possibly problematic) callers are all following the
> usual "lock, write to temp, rename" pattern, perhaps the lock_file()
> function can have access(path, W_OK) check before it returns a
> tempfile that has been successfully opened?
Yeah, that is a lot friendlier, as it prevents the caller from doing
work (which may even involve the user typing things!) when it is clear
that we would fail the final step anyway.
-Peff
^ permalink raw reply
* Re: [PATCH 5/6] config docs: Provide for config to specify tags not to abbreviate
From: Ian Jackson @ 2016-11-09 1:41 UTC (permalink / raw)
To: Jeff King; +Cc: Jacob Keller, Git mailing list, Junio C Hamano, Paul Mackerras
In-Reply-To: <20161108215709.rvmsnz4fvhizbocl@sigill.intra.peff.net>
Jeff King writes ("Re: [PATCH 5/6] config docs: Provide for config to specify tags not to abbreviate"):
> Yeah, I think git's config system was always designed to carry options
> for porcelains outside of git-core itself. So your new option fits into
> that.
Good, thanks.
> I think the two things I found weird were:
>
> - it's in the "log" section, which makes me think it's an option for
> git-log. But it's not. I'm not sure what the _right_ section is, but
> hopefully it would make it clear that this is command-agnostic.
>
> Something like "gui.abbrevTags" might be OK (and as you note, has
> precedence). But of course it's possible that a command like "tig"
> could learn to support it. I'm not sure if that counts as a GUI or
> not. :)
I don't really have an opinion about the name. gui.abbrevTags would
be a possibility. (It's a bit odd that implicitly, the default would
be `*'.)
> - The description talks about tag abbreviation, but doesn't really
> define it. Not being a gitk user, it was hard for me to figure out
> whether this was even relevant. Does it mean turning
> "refs/tags/v1.0" into "1.0"? From the rest of the series, it sounds
> like no. That should be more clear from the documentation.
I can do that, sure.
By default, gitk doesn't like to use much screen real estate for tags.
If there are long tag names, or many tags, it shows them all as a
single small indication saying just `<tag...|' or whatever with the
literal `tag...', not with the tag value.
Maybe a better name would be
gui.alwaysShowTags
?
I'm happy to be just told what the name ought to be, if the gitk and
git maintainers can agree. It seems largely a matter of taste.
Thanks,
Ian.
--
Ian Jackson <ijackson@chiark.greenend.org.uk> These opinions are my own.
If I emailed you from an address @fyvzl.net or @evade.org.uk, that is
a private address which bypasses my fierce spamfilter.
^ permalink raw reply
* Re: Bug: git config does not respect read-only .gitconfig file
From: Junio C Hamano @ 2016-11-09 1:22 UTC (permalink / raw)
To: Jeff King; +Cc: Jonathan Word, Markus Hitter, git, jword
In-Reply-To: <20161108200110.zvqdm2nlu5zxfyv5@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> Probably converting "rename(from, to)" to first check "access(to,
> W_OK)". That's racy, but it's the best we could do.
Hmph, if these (possibly problematic) callers are all following the
usual "lock, write to temp, rename" pattern, perhaps the lock_file()
function can have access(path, W_OK) check before it returns a
tempfile that has been successfully opened?
Having said that, I share your assessment that this is not a code or
design problem. It is unreasonable to drop the write-enable bit of
a file in a writable directory and expect it to stay unmodified. The
W-bit on the file is not usable as a security measure, and we do not
use it as such.
I do not offhand know how much a new feature "this repository can be
modified by pushing into and fetching from, but its configuration
cannot be modified" is a sensible thing to have. But it is quite
clear that even if we were to implement such feature, we wouldn't be
using W-bit on .git/config to signal that.
^ 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