Re: [PATCH v2 1/2] create_default_files(): don't set u+x bit on $GIT_DIR/config
On Sun, Nov 16, 2014 at 2:21 AM, Michael Haggerty wrote:
> Since time immemorial, the test of whether to set "core.filemode" has
> been done by trying to toggle the u+x bit on $GIT_DIR/config and then
> testing whether the change "took". It is somewhat odd to use the
> config file for this test, but whatever.
>
> The test code didn't set the u+x bit back to its original state
> itself, instead relying on the subsequent call to git_config_set() to
> re-write the config file with correct permissions.
>
> But ever since
>
> daa22c6f8d config: preserve config file permissions on edits (2014-05-06)
>
> git_config_set() copies the permissions from the old config file to
> the new one. This is a good change in and of itself, but it interacts
> badly with create_default_files()'s sloppiness, causing "git init" to
> leave the executable bit set on $GIT_DIR/config.
>
> So change create_default_files() to reset the permissions on
> $GIT_DIR/config after its test.
>
> Signed-off-by: Michael Haggerty
> ---
Should this patch include a test in t1300 to ensure that this bug does
not resurface (and to prove that this patch indeed fixes it)?
> builtin/init-db.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/builtin/init-db.c b/builtin/init-db.c
> index 56f85e2..4c8021d 100644
> --- a/builtin/init-db.c
> +++ b/builtin/init-db.c
> @@ -255,6 +255,7 @@ static int create_default_files(const char *template_path)
> filemode = (!chmod(path, st1.st_mode ^ S_IXUSR) &&
> !lstat(path, &st2) &&
> st1.st_mode != st2.st_mode);
> + filemode &= !chmod(path, st1.st_mode);
> }
> git_config_set("core.filemode", filemode ? "true" : "false");
>
> --
> 2.1.1
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] Documentation: change "gitlink" typo in git-push
The git-push manual page used "gitlink" in one place instead of "linkgit". Fix this so the link renders correctly. Noticed-by: Dan Allen Signed-off-by: brian m. carlson --- Documentation/git-push.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt index 21b3f29..b17283a 100644 --- a/Documentation/git-push.txt +++ b/Documentation/git-push.txt @@ -34,7 +34,7 @@ When the command line does not specify what to push with `...` arguments or `--all`, `--mirror`, `--tags` options, the command finds the default `` by consulting `remote.*.push` configuration, and if it is not found, honors `push.default` configuration to decide -what to push (See gitlink:git-config[1] for the meaning of `push.default`). +what to push (See linkgit:git-config[1] for the meaning of `push.default`). OPTIONS[[OPTIONS]] -- 2.2.0.rc0.207.ga3a616c -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] apply: fix typo in an error message
s/submoule/submodule
Signed-off-by: Slavomir Vlcek
---
For the 'master'. Thank you.
builtin/apply.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index 6696ea4..28d24f8 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -3728,7 +3728,7 @@ static void build_fake_ancestor(struct patch *list, const
char *filename)
if (!preimage_sha1_in_gitlink_patch(patch, sha1))
; /* ok, the textual part looks sane */
else
- die("sha1 information is lacking or useless for
submoule %s",
+ die("sha1 information is lacking or useless for
submodule %s",
name);
} else if (!get_sha1_blob(patch->old_sha1_prefix, sha1)) {
; /* ok */
--
2.2.0.rc1.dirty
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Fwd: Add git ignore as builtin
Alberto Fanjul Alonso gmail.com> writes: > git ignore adds to .git/info/exclude This should be "git exclude" not "git ignore". Difference between the two: http://stackoverflow.com/questions/10066749/git- excludes-vs-ignores I'd second the notion of a "git ignore", however it would have to modify the `.gitignore` not `.git/info/exclude`. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC] builtin: move builtin retrieval to get_builtin()
On 11/13/2014 07:19 PM, Junio C Hamano wrote:
>> git.c | 27 +++
>> 1 file changed, 15 insertions(+), 12 deletions(-)
>>
>> diff --git a/git.c b/git.c
>> index 18fbf79..e32c5b8 100644
>> --- a/git.c
>> +++ b/git.c
>> @@ -487,15 +487,20 @@ static struct cmd_struct commands[] = {
>> { "write-tree", cmd_write_tree, RUN_SETUP },
>> };
>>
>> -int is_builtin(const char *s)
>> +struct cmd_struct *get_builtin(const char *s)
>
> I do not think this has to be extern.
>
> static struct cmd_struct *get_builtin(const char *s)
>
> perhaps.
>
>> @@ -519,15 +525,12 @@ static void handle_builtin(int argc, const char **argv)
>> argv[0] = cmd = "help";
>> }
>>
>> -for (i = 0; i < ARRAY_SIZE(commands); i++) {
>> -struct cmd_struct *p = commands+i;
>> -if (strcmp(p->cmd, cmd))
>> -continue;
>> -if (saved_environment && (p->option & NO_SETUP)) {
>> +builtin = get_builtin(cmd);
>
> Nice.
>
>> +if (builtin) {
>> +if (saved_environment && (builtin->option & NO_SETUP))
>> restore_env();
>> -break;
>> -}
>> -exit(run_builtin(p, argc, argv));
>> +else
>> +exit(run_builtin(builtin, argc, argv));
>
> This change does not seem to have anything to do with the topic of
> the change. Why is it necessary?
Does the commit message lack some explanation
or the patch would better be divided into several parts?
I noticed that the patch has been modified (suggested 'static'
scope modification, commit message) and added
to the 'next' branch. So does this mean my task is done
or is there still something I should explain?
Thank you for your corrections.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] Windows: correct detection of EISDIR in mingw_open()
According to the Linux open(2) man page, open() returns EISDIR if a
directory was attempted to be opened for writing. Our emulation in
mingw_open() does not get this right: it checks only for O_CREAT. Fix
it to check for a write request.
This fixes a failure in reflog handling, which opens files with
O_APPEND|O_WRONLY, but without O_CREAT, and expects EISDIR when the
named file happens to be a directory.
Signed-off-by: Johannes Sixt
---
This version of the patch uses O_ACCMODE.
compat/mingw.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/compat/mingw.c b/compat/mingw.c
index 2ee3fe3..c702731 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -312,7 +312,7 @@ int mingw_open (const char *filename, int oflags, ...)
return -1;
fd = _wopen(wfilename, oflags, mode);
- if (fd < 0 && (oflags & O_CREAT) && errno == EACCES) {
+ if (fd < 0 && (oflags & O_ACCMODE) != O_RDONLY && errno == EACCES) {
DWORD attrs = GetFileAttributesW(wfilename);
if (attrs != INVALID_FILE_ATTRIBUTES && (attrs &
FILE_ATTRIBUTE_DIRECTORY))
errno = EISDIR;
--
2.0.0.12.gbcf935e
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/2] create_default_files(): don't set u+x bit on $GIT_DIR/config
Michael Haggerty writes:
> Since time immemorial, the test of whether to set "core.filemode" has
> been done by trying to toggle the u+x bit on $GIT_DIR/config and then
> testing whether the change "took". It is somewhat odd to use the
> config file for this test, but whatever.
The last sentence should read "We could create a test file and use
it for this purpose and then remove it, but config is a file we know
exists at this point in the code (and it is the only file we know
that exists), so it was a very sensible trick".
Or remove it altogether. In other words, do not sound as if you do
not know what you are doing in your log message. That would rob
confidence in the change from the person who is reading "git log"
output later.
> @@ -255,6 +255,7 @@ static int create_default_files(const char *template_path)
> filemode = (!chmod(path, st1.st_mode ^ S_IXUSR) &&
> !lstat(path, &st2) &&
> st1.st_mode != st2.st_mode);
> + filemode &= !chmod(path, st1.st_mode);
Sounds good.
You could also &&-chain this "flip it back" to the above statement.
If filemode is not trustable on a filesytem, doing one extra chmod()
to correct would not help us anyway, no?
> }
> git_config_set("core.filemode", filemode ? "true" : "false");
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] replace: fix replacing object with itself
Christian Couder writes: >> The patch is not wrong per-se, but I wonder how useful this "do not >> replace itself but all other forms of loops are not checked at all" >> would be in practice. If your user did this: >> >> git replace A B ;# pretend as if what is in B is in A >> git replace B C ;# pretend as if what is in C is in B >> git replace C A ;# pretend as if we have loop >> git log C >> >> she would not be helped with this patch at all, no? > > Yeah, but such loops are much less likely mistakes and are more > difficult to detect, so I think this patch is at least a good first > step. More difficult to notice by humans, hence deserves more help from the tool. When these two are both mistakes, which one do you think is easier to notice (thusly unlikely to happen)? git replace A A git replace C A Of course, the former is immediately obvious to the human who is typing that it is a typo. The latter would be harder to spot as a mistake as the invoker has to know that there is an existing "pretend as if what is in A is in C" aka "replace A C" done earlier in the repository. And the cost of detecting such a possible "too deep replace chain" (do not call that a loop---the runtime barfs if you have too deep a replace chain without any loop) wouldn't be too high. You only need to look at existing refs/replace/* refs, their contents, and the two object names that form the proposed new replacement . Even a kludge (read: I am not suggesting that you solve it this way) like "first install the replacement as proposed, then enumerate all the replacement refs and their values and try to see if the runtime check would barf, and if it would, fail the operation and revert the change" would catch a mistake to cause the repository in trouble. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] config: clear the executable bits (if any) on $GIT_DIR/config
Michael Haggerty writes: > There is no reason for $GIT_DIR/config to be executable, plus this > change will help clean up repositories affected by the bug that was > fixed by the previous commit. I do not think we want to do this. It is a welcome bugfix to create $GIT_DIR/config without executable bit when and only when we create it. It is very much in line with "There is no reason for $GIT_DIR/config to be executable"---we do not need to make it executable ourselves, so we shouldn't, but we did which was a bug we want to fix in patch 1/2 you posted. But with the "preserve existing permissions" fix we did earlier, the end users are now allowed to flip the executable bit on for their own purpose, and dropping it without knowing why they are doing so is simply rude. And honestly, Git do *not* even want to know why the users want to flip the bit. So I would suggest not to spend any cycle or any code complexity to "repair" existing repositories. Having that bit on does not hurt anybody. Those who found it curious can flip that bit off and then Git with "preserve existing permissions" fix will keep that bit off from then on. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Introduce a hook to run after formatting patches
Stefan Beller writes: > +post-format-patch > + > + > +This hook is called after format-patch created a patch and it is > +invoked with the filename of the patch as the first parameter. Such an interface would not work well with --stdout mode, would it? And if this only works with output generated into the files, then $ git format-patch $range | xargs -n1 $your_post_processing_script would do the same without any change to Git, I would imagine. So I would have to say that I am fairly negative on this change in the presented form. An alternative design to implement this as a post-processing filter to work for both "to individual files" and "to standard output stream" output filter may be possible, but even in that case I am not sure if it is worth the churn. In general I'd look at post-anything hook that works locally with a great suspicion, so that may partly be where my comment above is coming from. I dunno. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] difftool: honor --trust-exit-code for builtin tools
Mikael Magnusson writes:
>>> diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
>>> index a40d3df..2b66351 100644
>>> --- a/git-mergetool--lib.sh
>>> +++ b/git-mergetool--lib.sh
>>> @@ -221,6 +221,7 @@ run_merge_tool () {
>>> else
>>> run_diff_cmd "$1"
>>> fi
>>> + status=$?
>>> return $status
>>> }
>>
>> Thanks for a quick turn-around. As a hot-fix for what is already in
>> -rc I am fine with this fix but the patch makes me wonder if $status
>> as a global shell variable has any significance.
>
> $status is an alias for $? in zsh, and so cannot be assigned to. But
> other than that I don't think it holds any meaning and should be fine
> in a .sh script.
That is not what I meant by "global ... significance".
The question was if the codepath in the caller depends on this
setting the global variable here, or nobody looks at and depends on
the global variable we are setting here after this function returns.
It does not have any significance that a random shell implementation
is not POSIX compliant. That would merely mean that such a shell
cannot be used to run POSIX shell scripts like our Porcelain. I
would suspect that zsh has more "posixly correct" mode, with which
it _can_ run POSIX shell scripts, and I would imagine that this
"$status is an alias $?" business is disabled in that mode?
My quick glance across the codepaths in the callers of this funciton
indicated that it should be safe not using this global variable, so
my answer to my original question was "no there is no significance".
I think we can safely remove any mention of status from this shell
function, i.e. if we remove initial assignment to 0, remove this new
assignment and then remove the "return $status" at the end, the
caller would still be happy.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] l10n: de.po: translate 2 new messages
Phillip Signed-off-by: Phillip Sz --- po/de.po | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/po/de.po b/po/de.po index c807967..5af3f8f 100644 --- a/po/de.po +++ b/po/de.po @@ -5473,7 +5473,7 @@ msgstr "prüft die Reflogs (Standard)" #: builtin/fsck.c:613 msgid "also consider packs and alternate objects" -msgstr "" +msgstr "ziehen Sie außerdem Pakete und alternative Objekte in Betracht" #: builtin/fsck.c:614 msgid "enable more strict checking" @@ -8253,7 +8253,7 @@ msgstr "Referenz muss sich auf dem angegebenen Wert befinden" #: builtin/push.c:495 msgid "check" -msgstr "" +msgstr "Überprüfung" #: builtin/push.c:496 msgid "control recursive pushing of submodules" -- 2.1.3 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
How safe are signed git tags? Only as safe as SHA-1 or somehow safer?
Hi! How safe are signed git tags? Especially because git uses SHA-1. There is contradictory information around. So if one verifies a git tag (`git tag -v tagname`), then `checksout`s the tag, and checks that `git status` reports no untracked/modified files, without further manually auditing the code, how secure is this actually? Is it only as safe as SHA-1? Let's assume an adversary, that is capable of producing SHA-1 collisions. Linus Torvalds said: [1] > Git uses SHA-1 not for security And goes on. > The security parts are elsewhere Could you please elaborate on this? Where are the security parts? Can you please briefly explain how these work? Where can I read more about this? Wikipedia says. [2] > Nonetheless, without second preimage resistance [3] of SHA-1 signed commits and tags would no longer secure the state of the repository as they only sign the root of a Merkle tree [4]. Which contradicts what Linus Torvalds said. What does that mean for security? Which statement is true? > "The source control management system Git uses SHA-1 not for security but for ensuring that the data has not changed due to accidental corruption. Linus Torvalds has said, "If you have disk corruption, if you have DRAM corruption, if you have any kind of problems at all, Git will notice them. It's not a question of if, it's a guarantee. You can have people who try to be malicious. They won't succeed. [...] Nobody has been able to break SHA-1, but the point is the SHA-1, as far as Git is concerned, isn't even a security feature. It's purely a consistency check. The security parts are elsewhere, so a lot of people assume that since Git uses SHA-1 and SHA-1 is used for cryptographically secure stuff, they think that, OK, it's a huge security feature. It has nothing at all to do with security, it's just the best hash you can get. [...] I guarantee you, if you put your data in Git, you can trust the fact that five years later, after it was converted from your hard disk to DVD to whatever new technology and you copied it along, five years later you can verify that the data you get back out is the exact same data you put in. [...] One of the reasons I care is for the kernel, we had a break in on one of the BitKeeper sites where people tried to corrupt the kernel source code repositories." [6] If (!) I understand Mike Gerwitz ([...] GNU [...]) 's opinion, his opinion is, that for best security each and every commit should be signed for best possible git verification security. See also: - Mike Gerwitz's "A Git Horror Story: Repository Integrity With Signed Commits" [7] - Verbose reply by Mike Gerwitz to my question. [8] - Similar question on security stackexchange. [9] Quote: "Nevertheless, If somebody managed to find a way how to find SHA1 collisions easily, then git would have much bigger problem." Cheers, Patrick [1] https://www.youtube.com/watch?v=4XpnKHJAok8&t=56m20s [2] https://en.wikipedia.org/wiki/SHA-1#Data_integrity [3] https://en.wikipedia.org/wiki/Second_preimage_resistance [4] https://en.wikipedia.org/wiki/Merkle_tree [5] https://www.youtube.com/watch?v=4XpnKHJAok8&t=56m20s [6] https://en.wikipedia.org/wiki/SHA-1#Data_integrity [7] http://mikegerwitz.com/papers/git-horror-story [8] https://www.whonix.org/forum/index.php/topic,538.msg4278.html#msg4278 [9] https://security.stackexchange.com/questions/67920/how-safe-are-signed-git-tags-only-as-safe-as-sha-1-or-somehow-safer -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC v2 1/2] submodule: add ability to shallowly clone any branch in a repo as a submodule
Was this accepted? I am also interested at this behavior or some kind of other solution to git submodule --depth combined with a branch. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] difftool: honor --trust-exit-code for builtin tools
David Aguilar writes:
> run_merge_tool() was not setting $status, which prevented the
> exit code for builtin tools from being forwarded to the caller.
>
> Capture the exit status and add a test to guarantee the behavior.
>
> Reported-by: Adria Farres <14farr...@gmail.com>
> Signed-off-by: David Aguilar
> ---
> git-mergetool--lib.sh | 1 +
> t/t7800-difftool.sh | 5 +
> 2 files changed, 6 insertions(+)
>
> diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
> index a40d3df..2b66351 100644
> --- a/git-mergetool--lib.sh
> +++ b/git-mergetool--lib.sh
> @@ -221,6 +221,7 @@ run_merge_tool () {
> else
> run_diff_cmd "$1"
> fi
> + status=$?
> return $status
If you want to return the last exit status at the end of a function you
don't need any return at all.
Andreas.
--
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] gitk: Enable mouse horizontal scrolling in diff pane
Gabriele Mazzotta wrote: > Currently it's required to hold Shift and scroll up and down to move > horizontally. Listen to Button-6 and Button-7 events too to make > horizontal scrolling handier with touchpads and some mice. Hm, on my Macbook the diff pane already scrolls in all four directions with two-finger scrolling on the trackpad. (Without your patch, I mean.) In my copy of gitk I did the opposite for the commit list though; I can't stand it that I accidentally scroll left or right with the trackpad, so I commented out some code to restrict it to only vertical scrolling. I never posted this patch because I bet many people like the current behaviour. Just so you know that such a change might be controversial. -- Stefan Haller Berlin, Germany http://www.haller-berlin.de/ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/2] config: clear the executable bits (if any) on $GIT_DIR/config
Am 16.11.2014 um 08:21 schrieb Michael Haggerty:
> @@ -559,9 +562,21 @@ int cmd_config(int argc, const char **argv, const char
> *prefix)
> if (given_config_source.blob)
> die("editing blobs is not supported");
> git_config(git_default_config, NULL);
> - launch_editor(given_config_source.file ?
> - given_config_source.file : git_path("config"),
> - NULL, NULL);
> + config_file = xstrdup(given_config_source.file ?
> + given_config_source.file :
> git_path("config"));
> + launch_editor(config_file, NULL, NULL);
> +
> + /*
> + * In git 2.1, there was a bug in "git init" that left
> + * the u+x bit set on the config file. To clean up any
> + * repositories affected by that bug, and just because
> + * it doesn't make sense for a config file to be
> + * executable anyway, clear any executable bits from
> + * the file (on a "best effort" basis):
> + */
> + if (!lstat(config_file, &st) && (st.st_mode & 0111))
At this point we cannot be sure that config_file is a regular file, can
we? It could also be a symbolic link. Wouldn't plain stat() be more
correct then?
> + chmod(config_file, st.st_mode & 07666);
> + free(config_file);
> }
> else if (actions == ACTION_SET) {
> int ret;
-- Hannes
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
