* Re: Server-side hooks on non-bare repository
From: Mike Lewis @ 2017-03-06 22:33 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqq1suankus.fsf@junio-linux.mtv.corp.google.com>
Hi Junio,
Thanks for taking the time to reply. I apologize if there was a misunderstanding in my previous email; I'm relatively new to dealing with some of the more advanced features of git, and did not describe my situation as clearly as possible.
I am using the pre-receive hook to determine whether to allow the push or not. Essentially, I'm rejecting the entire push if a branch not checked out is pushed, and then ensuring that a backup has been made of some critical data to prevent any issues with deployment (and if that backup fails for whatever reason, I also reject the push). The post-receive hook is essentially used for notification of a couple services that the push had been completed.
My point #1 was supposed to be something along the lines of "I found this behavior unintuitive and poorly documented, but it's fine since I can work around it", but it didn't come across that way. I understand that breaking backwards compatibility by changing this behavior would be a huge deal, and I was not intending to suggest it.
Point #2, however is what I was attempting to discuss in more detail. I'm curious as to why git doesn't recognize that it's currently inside the working tree in that situation, and if that's intended or not. There's no update currently happening in either hook (since they are called immediately before and after any changes), which means that I can't see a reason to have behavior that differs (having to specify a GIT_DIR, which may or may not be a complete workaround) between when the hooks are running and when they're not. That doesn't mean there isn't a reason; it's more likely I'm just not familiar enough with how the hooks are called internally to see it, and am looking for a more thorough explanation as to the motivations behind this behavior. If it's intended and not going to change, I think the behavior should be documented more thoroughly so that nobody has to repeat the process I did to find out why my hooks were failing. I don't mind contributing to this, but I have no idea where to start.
Anyways, thanks for responding, and I apologize again if my first email was poorly written and rushed.
Mike Lewis
> On Mar 6, 2017, at 16:42, Junio C Hamano <gitster@pobox.com> wrote:
>
> Mike Lewis <mike@mplew.is> writes:
>
>> I’m having some issues with using server-side hooks when pushing
>> to a non-bare repository. In my git config, I have
>> `receive.denyCurrentBranch` set to `updateInstead`, which behaves
>> as expected, and updates the current working tree when the current
>> branch is pushed to. However, attempting to process those changes
>> with pre-receive and post-receive hooks results in some unexpected
>> behavior regarding the current working directory of the scripts
>> and using git commands.
>
> The pre-receive hook is to inspect the objects and ref updates and
> say "yes, allow it" or "no, refuse it"---you are not supposed to do
> anything else, so even though what processing you are interested in
> doing in your "attempting to process" is unclear, this hook is not
> what you want to use anyway.
>
> The post-receive hook is a more interesting case. It is called only
> after everything finishes, so it is like running a custom script
> after "git push" is processed.
>
>> I’ve tested these issues using both git
>> 2.11 and 2.12 on various systems (macOS and CentOS), and get the
>> same behavior each time.
>
> That is a very good news, as I do not think at least in the past few
> years we planned to change the established behaviour of the hook.
>
>> 1. When using a non-bare repository, I would expect the the
>> working directory of the hook to be the root directory of the
>> working tree,...
>> 2. While running the hooks, git treats the repository as being
>> bare, regardless of whether it actually is.
>
> Yes, and it is unlikely that the behaviour wrt to where $cwd is
> during the hook's execution will ever change; otherwise existing
> scripts that know what the rule is (i.e. the rule you figured out in
> 2.) will be broken. denycurrent=updateinstead *is* an odd-man out,
> and its processing is purely internal---its addition does not mean
> hook authors are suddenly required to do things differently
> depending on bare/non-bare-ness of the repository.
>
>
^ permalink raw reply
* Re: [Request for Documentation] Differentiate signed (commits/tags/pushes)
From: Stefan Beller @ 2017-03-06 22:52 UTC (permalink / raw)
To: Junio C Hamano; +Cc: tom, Matthieu Moy, git@vger.kernel.org
In-Reply-To: <xmqqshmqm4ur.fsf@junio-linux.mtv.corp.google.com>
On Mon, Mar 6, 2017 at 2:13 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Stefan Beller <sbeller@google.com> writes:
>
>> What is the difference between signed commits and tags?
>> (Not from a technical perspective, but for the end user)
>
> When you "commit -s", you are signing the bytes in the commit
> object, which means that you are attesting the fact that the tree
> you wanted to record is one of the 47 other colliding tree objects
> that happen to share that 40-hex hash value, and also the fact that
> the commits you wanted to record as its parents have certain SHA-1
> hash values. As you are relying on the resistance to preimage
> attack against SHA-1 at least locally around that signed commit,
> there wouldn't be meaningful difference between a 50-commit series
> each of which is individually signed with "commit -s", such a
> 50-commit series, only the top of which is signed with "commit -s",
> and the same 50-commit series, on the top of which is signed with
> "tag -s".
>
> "tag -s" also has the benefit of being retroactive. You can create
> commit, think about it for a week and then later tag it. And ask
> others to also tag the same one. You cannot do so with "commit -s".
ok, so there is *no* advantage of signing a commit over tags?
I'll see if I can write a patch that enhances Documentation/git-commit.txt
pointing to git-tag instead.
>> A signed push can certify that a given payload (consisting
>> of multiple commits on possibly multiple branches) was transmitted
>> to a remote, which can be recorded by the remote as e.g. a proof
>> of work.
>
> A signed push is _NOT_ about certifying the objects in the history
Yes that is my understanding, though I was unclear in writing it.
> I'd agree with that "checkbox" description, [...]
> "commit -s" was a remedy for that.
Out of curiosity: Does (did) such a project exist? Can I read up on that
and their best practices?
Thanks,
Stefan
^ permalink raw reply
* [RFC PATCH] grep: fix bug when recursing with relative pathspec
From: Brandon Williams @ 2017-03-06 23:07 UTC (permalink / raw)
To: git; +Cc: Brandon Williams, sbeller, gitster, peff
In-Reply-To: <20170224235100.52627-1-bmwill@google.com>
When using the --recurse-submodules flag with a relative pathspec which
includes "..", an error is produced inside the child process spawned for
a submodule. When creating the pathspec struct in the child, the ".."
is interpreted to mean "go up a directory" which causes an error stating
that the path ".." is outside of the repository.
While it is true that ".." is outside the scope of the submodule, it is
confusing to a user who originally invoked the command where ".." was
indeed still inside the scope of the superproject. Since the child
process launched for the submodule has some context that it is operating
underneath a superproject, this error could be avoided.
This patch fixes the bug by passing the 'prefix' to the child process.
Now each child process that works on a submodule has two points of
reference to the superproject: (1) the 'super_prefix' which is the path
from the root of the superproject down to root of the submodule and (2)
the 'prefix' which is the path from the root of the superproject down to
the directory where the user invoked the git command.
With these two pieces of information a child process can correctly
interpret the pathspecs provided by the user as well as being able to
properly format the its output relative to the directory the user
invoked a git command from.
Signed-off-by: Brandon Williams <bmwill@google.com>
---
After taking a closer look at this bug I determined that I did a poor job at
thinking of all the corner cases when implementing a recursive grep. As it
turns out I think we really need to pass on the prefix information to the child
process so that it has enough context to do path matching and to format its
output. Unfortunately I don't know the best way to pass on this information
without breaking other commands that rely on the GIT_PREFIX being overridden
during discovery. Maybe we'll need to add in another env var to account for
that? For the purposes of showing how to fix this bug while still getting the
test suite to pass, I have git respect the GIT_PREFIX env var only if the
super_prefix is set (which it should only be used in submodule operations at
this point in time). Its a pretty hacky fix, so I'm up for other suggestions
on how to properly pass on this information to the child process.
builtin/grep.c | 39 ++++++++++++--------
git.c | 2 --
setup.c | 5 +++
t/t7814-grep-recurse-submodules.sh | 73 ++++++++++++++++++++++++++++++++++++++
4 files changed, 102 insertions(+), 17 deletions(-)
diff --git a/builtin/grep.c b/builtin/grep.c
index 2c727ef49..acecad0e0 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -310,10 +310,7 @@ static int grep_sha1(struct grep_opt *opt, const unsigned char *sha1,
{
struct strbuf pathbuf = STRBUF_INIT;
- if (opt->relative && opt->prefix_length) {
- quote_path_relative(filename + tree_name_len, opt->prefix, &pathbuf);
- strbuf_insert(&pathbuf, 0, filename, tree_name_len);
- } else if (super_prefix) {
+ if (super_prefix) {
strbuf_add(&pathbuf, filename, tree_name_len);
strbuf_addstr(&pathbuf, super_prefix);
strbuf_addstr(&pathbuf, filename + tree_name_len);
@@ -321,6 +318,13 @@ static int grep_sha1(struct grep_opt *opt, const unsigned char *sha1,
strbuf_addstr(&pathbuf, filename);
}
+ if (opt->relative && opt->prefix_length) {
+ char *name = strbuf_detach(&pathbuf, NULL);
+ quote_path_relative(name + tree_name_len, opt->prefix, &pathbuf);
+ strbuf_insert(&pathbuf, 0, name, tree_name_len);
+ free(name);
+ }
+
#ifndef NO_PTHREADS
if (num_threads) {
add_work(opt, GREP_SOURCE_SHA1, pathbuf.buf, path, sha1);
@@ -345,12 +349,14 @@ static int grep_file(struct grep_opt *opt, const char *filename)
{
struct strbuf buf = STRBUF_INIT;
+ if (super_prefix)
+ strbuf_addstr(&buf, super_prefix);
+ strbuf_addstr(&buf, filename);
+
if (opt->relative && opt->prefix_length) {
- quote_path_relative(filename, opt->prefix, &buf);
- } else {
- if (super_prefix)
- strbuf_addstr(&buf, super_prefix);
- strbuf_addstr(&buf, filename);
+ char *name = strbuf_detach(&buf, NULL);
+ quote_path_relative(name, opt->prefix, &buf);
+ free(name);
}
#ifndef NO_PTHREADS
@@ -399,13 +405,12 @@ static void run_pager(struct grep_opt *opt, const char *prefix)
}
static void compile_submodule_options(const struct grep_opt *opt,
- const struct pathspec *pathspec,
+ const char **argv,
int cached, int untracked,
int opt_exclude, int use_index,
int pattern_type_arg)
{
struct grep_pat *pattern;
- int i;
if (recurse_submodules)
argv_array_push(&submodule_options, "--recurse-submodules");
@@ -523,9 +528,8 @@ static void compile_submodule_options(const struct grep_opt *opt,
/* Add Pathspecs */
argv_array_push(&submodule_options, "--");
- for (i = 0; i < pathspec->nr; i++)
- argv_array_push(&submodule_options,
- pathspec->items[i].original);
+ for (; *argv; argv++)
+ argv_array_push(&submodule_options, *argv);
}
/*
@@ -549,6 +553,11 @@ static int grep_submodule_launch(struct grep_opt *opt,
prepare_submodule_repo_env(&cp.env_array);
argv_array_push(&cp.env_array, GIT_DIR_ENVIRONMENT);
+ if (opt->relative && opt->prefix_length)
+ argv_array_pushf(&cp.env_array, "%s=%s",
+ GIT_PREFIX_ENVIRONMENT,
+ opt->prefix);
+
/* Add super prefix */
argv_array_pushf(&cp.args, "--super-prefix=%s%s/",
super_prefix ? super_prefix : "",
@@ -1206,7 +1215,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
if (recurse_submodules) {
gitmodules_config();
- compile_submodule_options(&opt, &pathspec, cached, untracked,
+ compile_submodule_options(&opt, argv + i, cached, untracked,
opt_exclude, use_index,
pattern_type_arg);
}
diff --git a/git.c b/git.c
index b367cf668..b05108afd 100644
--- a/git.c
+++ b/git.c
@@ -361,8 +361,6 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv)
if (!help && get_super_prefix()) {
if (!(p->option & SUPPORT_SUPER_PREFIX))
die("%s doesn't support --super-prefix", p->cmd);
- if (prefix)
- die("can't use --super-prefix from a subdirectory");
}
if (!help && p->option & NEED_WORK_TREE)
diff --git a/setup.c b/setup.c
index 1b534a750..0de379319 100644
--- a/setup.c
+++ b/setup.c
@@ -936,10 +936,15 @@ static const char *setup_git_directory_gently_1(int *nongit_ok)
const char *setup_git_directory_gently(int *nongit_ok)
{
const char *prefix;
+ const char *env_prefix;
prefix = setup_git_directory_gently_1(nongit_ok);
+ env_prefix = getenv(GIT_PREFIX_ENVIRONMENT);
+
if (prefix)
setenv(GIT_PREFIX_ENVIRONMENT, prefix, 1);
+ else if (env_prefix && get_super_prefix())
+ prefix = env_prefix;
else
setenv(GIT_PREFIX_ENVIRONMENT, "", 1);
diff --git a/t/t7814-grep-recurse-submodules.sh b/t/t7814-grep-recurse-submodules.sh
index 67247a01d..d8ed3d64f 100755
--- a/t/t7814-grep-recurse-submodules.sh
+++ b/t/t7814-grep-recurse-submodules.sh
@@ -227,6 +227,79 @@ test_expect_success 'grep history with moved submoules' '
test_cmp expect actual
'
+test_expect_success 'grep using relative path' '
+ test_when_finished "rm -rf parent sub" &&
+ git init sub &&
+ echo "foobar" >sub/file &&
+ git -C sub add file &&
+ git -C sub commit -m "add file" &&
+
+ git init parent &&
+ echo "foobar" >parent/file &&
+ git -C parent add file &&
+ mkdir parent/src &&
+ echo "foobar" >parent/src/file2 &&
+ git -C parent add src/file2 &&
+ git -C parent submodule add ../sub &&
+ git -C parent commit -m "add files and submodule" &&
+
+ # From top works
+ cat >expect <<-\EOF &&
+ file:foobar
+ src/file2:foobar
+ sub/file:foobar
+ EOF
+ git -C parent grep --recurse-submodules -e "foobar" >actual &&
+ test_cmp expect actual &&
+
+ # Relative path to top errors out
+ cat >expect <<-\EOF &&
+ ../file:foobar
+ file2:foobar
+ ../sub/file:foobar
+ EOF
+ git -C parent/src grep --recurse-submodules -e "foobar" -- .. >actual &&
+ test_cmp expect actual &&
+
+ # Relative path to submodule errors out
+ cat >expect <<-\EOF &&
+ ../sub/file:foobar
+ EOF
+ git -C parent/src grep --recurse-submodules -e "foobar" -- ../sub >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'grep from a subdir' '
+ test_when_finished "rm -rf parent sub" &&
+ git init sub &&
+ echo "foobar" >sub/file &&
+ git -C sub add file &&
+ git -C sub commit -m "add file" &&
+
+ git init parent &&
+ mkdir parent/src &&
+ echo "foobar" >parent/src/file &&
+ git -C parent add src/file &&
+ git -C parent submodule add ../sub src/sub &&
+ git -C parent submodule add ../sub sub &&
+ git -C parent commit -m "add files and submodules" &&
+
+ cat >expect <<-\EOF &&
+ src/file:foobar
+ src/sub/file:foobar
+ sub/file:foobar
+ EOF
+ git -C parent grep --recurse-submodules -e "foobar" >actual &&
+ test_cmp expect actual &&
+
+ cat >expect <<-\EOF &&
+ file:foobar
+ sub/file:foobar
+ EOF
+ git -C parent/src grep --recurse-submodules -e "foobar" >actual &&
+ test_cmp expect actual
+'
+
test_incompatible_with_recurse_submodules ()
{
test_expect_success "--recurse-submodules and $1 are incompatible" "
--
2.12.0.rc1.440.g5b76565f74-goog
^ permalink raw reply related
* Re: RFC: Another proposed hash function transition plan
From: Jonathan Nieder @ 2017-03-06 23:40 UTC (permalink / raw)
To: David Lang; +Cc: git, sbeller, bmwill, jonathantanmy, peff, Linus Torvalds
In-Reply-To: <nycvar.QRO.7.75.62.1703050258200.6590@qynat-yncgbc>
David Lang wrote:
>> Translation table
>> ~~~~~~~~~~~~~~~~~
>> A fast bidirectional mapping between sha1-names and sha256-names of
>> all local objects in the repository is kept on disk. The exact format
>> of that mapping is to be determined.
>>
>> All operations that make new objects (e.g., "git commit") add the new
>> objects to the translation table.
>
> This seems like a rather nontrival thing to design. It will need to
> hold millions of mappings, and be quickly searchable from either
> direction (sha1->new and new->sha1) while still be fairly fast to
> insert new records into.
I am currently thinking of using LevelDB, since it has the advantages of
being simple, already existing, and having already been ported to Java
(allowing JGit can read and write the same format).
If that doesn't work, we'd try some other key-value store like Samba's
tdb or Kyoto Cabinet.
Jonathan
^ permalink raw reply
* Re: [Request for Documentation] Differentiate signed (commits/tags/pushes)
From: Jakub Narębski @ 2017-03-06 23:59 UTC (permalink / raw)
To: Junio C Hamano, Stefan Beller; +Cc: tom, Matthieu Moy, git@vger.kernel.org
In-Reply-To: <xmqqshmqm4ur.fsf@junio-linux.mtv.corp.google.com>
W dniu 06.03.2017 o 23:13, Junio C Hamano pisze:
> Stefan Beller <sbeller@google.com> writes:
>
>> What is the difference between signed commits and tags?
>> (Not from a technical perspective, but for the end user)
[...]
>> Off list I was told gpg-signed commits are a "checkbox feature",
>> i.e. no real world workflow would actually use it. (That's a bold
>> statement, someone has to use it as there was enough interest
>> to implement it, no?)
>
> I'd agree with that "checkbox" description, except that you need to
> remember that a project can enforce _any_ workflow to its developer,
> even if it does not make much sense, and at that point, the workflow
> would become a real-world workflow. The word "real world workflow"
> does not make any assurance if that workflow is sensible.
>
> Historically, "tag -s" came a lot earlier. When a project for
> whatever reason wants signature for each and every commit so that
> they somehow can feel good, without "commit -s", it would have made
> us unnecessary work to scale tag namespace only because there will
> be tons of pointless tags. "commit -s" was a remedy for that.
Also from what I remember signed commits came before mergetags, that
is the result of merging a signed tag (storing the signature of
one of parents of the merge commit to not pollute tag namespace).
And this workflow, from what I know, is quite useful.
--
Jakub Narębski
^ permalink raw reply
* Re: [Request for Documentation] Differentiate signed (commits/tags/pushes)
From: Junio C Hamano @ 2017-03-07 0:08 UTC (permalink / raw)
To: Stefan Beller; +Cc: tom, Matthieu Moy, git@vger.kernel.org
In-Reply-To: <CAGZ79kZU+-5D0bHSA1duRLnvjb+P67AzGhESS6J1z5qtO8SXsQ@mail.gmail.com>
Stefan Beller <sbeller@google.com> writes:
>> "tag -s" also has the benefit of being retroactive. You can create
>> commit, think about it for a week and then later tag it. And ask
>> others to also tag the same one. You cannot do so with "commit -s".
>
> ok, so there is *no* advantage of signing a commit over tags?
Did I say anything that remotely resembles that? Puzzled.
If the reason you want to have GPG signature on a commit is not
because you want to mark some meaningful place in the history, but
you are signing each and every ones out of some random reason, there
is no reason why you would want "tag -s" them, so you can see it as
an advantage of "commit -s" over "tag -s", because to such a
project, all commits that are not tagged look the same and there is
no "landmark" value to use "tag -s" for each and every one of them.
^ permalink raw reply
* RFC v3: Another proposed hash function transition plan
From: Jonathan Nieder @ 2017-03-07 0:17 UTC (permalink / raw)
To: Linus Torvalds
Cc: Git Mailing List, Stefan Beller, bmwill, jonathantanmy, Jeff King,
David Lang, brian m. carlson
In-Reply-To: <CA+55aFz+gkAsDZ24zmePQuEs1XPS9BP_s8O7Q4wQ7LV7X5-oDA@mail.gmail.com>
Linus Torvalds wrote:
> On Fri, Mar 3, 2017 at 5:12 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
>> This document is still in flux but I thought it best to send it out
>> early to start getting feedback.
>
> This actually looks very reasonable if you can implement it cleanly
> enough.
Thanks for the kind words on what had quite a few flaws still. Here's
a new draft. I think the next version will be a patch against
Documentation/technical/.
As before, comments welcome, both here and inline at
https://goo.gl/gh2Mzc
Changes since v2:
Use SHA3-256 instead of SHA2 (thanks, Linus and brian m.
carlson).[1][2]
Make sha3-based signatures a separate field, avoiding the need for
"hash" and "nohash" fields (thanks to peff[3]).
Add a sorting phase to fetch (thanks to Junio for noticing the need
for this).
Omit blobs from the topological sort during fetch (thanks to peff).
Discuss alternates, git notes, and git servers in the caveats section
(thanks to Junio Hamano, brian m. carlson[4], and Shawn Pearce).
Clarify language throughout (thanks to various commenters, especially
Junio).
Sincerely,
Jonathan
Git hash function transition
============================
Status: Draft
Last Updated: 2017-03-06
Objective
---------
Migrate Git from SHA-1 to a stronger hash function.
Background
----------
At its core, the Git version control system is a content addressable
filesystem. It uses the SHA-1 hash function to name content. For
example, files, directories, and revisions are referred to by hash
values unlike in other traditional version control systems where files
or versions are referred to via sequential numbers. The use of a hash
function to address its content delivers a few advantages:
* Integrity checking is easy. Bit flips, for example, are easily
detected, as the hash of corrupted content does not match its name.
* Lookup of objects is fast.
Using a cryptographically secure hash function brings additional
advantages:
* Object names can be signed and third parties can trust the hash to
address the signed object and all objects it references.
* Communication using Git protocol and out of band communication
methods have a short reliable string that can be used to reliably
address stored content.
Over time some flaws in SHA-1 have been discovered by security
researchers. https://shattered.io demonstrated a practical SHA-1 hash
collision. As a result, SHA-1 cannot be considered cryptographically
secure any more. This impacts the communication of hash values because
we cannot trust that a given hash value represents the known good
version of content that the speaker intended.
SHA-1 still possesses the other properties such as fast object lookup
and safe error checking, but other hash functions are equally suitable
that are believed to be cryptographically secure.
Goals
-----
1. The transition to SHA3-256 can be done one local repository at a time.
a. Requiring no action by any other party.
b. A SHA3-256 repository can communicate with SHA-1 Git servers
(push/fetch).
c. Users can use SHA-1 and SHA3-256 identifiers for objects
interchangeably.
d. New signed objects make use of a stronger hash function than
SHA-1 for their security guarantees.
2. Allow a complete transition away from SHA-1.
a. Local metadata for SHA-1 compatibility can be removed from a
repository if compatibility with SHA-1 is no longer needed.
3. Maintainability throughout the process.
a. The object format is kept simple and consistent.
b. Creation of a generalized repository conversion tool.
Non-Goals
---------
1. Add SHA3-256 support to Git protocol. This is valuable and the
logical next step but it is out of scope for this initial design.
2. Transparently improving the security of existing SHA-1 signed
objects.
3. Intermixing objects using multiple hash functions in a single
repository.
4. Taking the opportunity to fix other bugs in git's formats and
protocols.
5. Shallow clones and fetches into a SHA3-256 repository. (This will
change when we add SHA3-256 support to Git protocol.)
6. Skip fetching some submodules of a project into a SHA3-256
repository. (This also depends on SHA3-256 support in Git
protocol.)
Overview
--------
We introduce a new repository format extension `sha3`. Repositories
with this extension enabled use SHA3-256 instead of SHA-1 to name
their objects. This affects both object names and object content ---
both the names of objects and all references to other objects within
an object are switched to the new hash function.
sha3 repositories cannot be read by older versions of Git.
Alongside the packfile, a sha3 repository stores a bidirectional
mapping between sha3 and sha1 object names. The mapping is generated
locally and can be verified using "git fsck". Object lookups use this
mapping to allow naming objects using either their sha1 and sha3 names
interchangeably.
"git cat-file" and "git hash-object" gain options to display an object
in its sha1 form and write an object given its sha1 form. This
requires all objects referenced by that object to be present in the
object database so that they can be named using the appropriate name
(using the bidirectional hash mapping).
Fetches from a SHA-1 based server convert the fetched objects into
sha3 form and record the mapping in the bidirectional mapping table
(see below for details). Pushes to a SHA-1 based server convert the
objects being pushed into sha1 form so the server does not have to be
aware of the hash function the client is using.
Detailed Design
---------------
Object names
~~~~~~~~~~~~
Objects can be named by their 40 hexadecimal digit sha1-name or 64
hexadecimal digit sha3-name, plus names derived from those (see
gitrevisions(7)).
The sha1-name of an object is the SHA-1 of the concatenation of its
type, length, a nul byte, and the object's sha1-content. This is the
traditional <sha1> used in Git to name objects.
The sha3-name of an object is the SHA3-256 of the concatenation of its
type, length, a nul byte, and the object's sha3-content.
Object format
~~~~~~~~~~~~~
The content as a byte sequence of a tag, commit, or tree object named
by sha1 and sha3 differ because an object named by sha3-name refers to
other objects by their sha3-names and an object named by sha1-name
refers to other objects by their sha1-names.
The sha3-content of an object is the same as its sha1-content, except
that objects referenced by the object are named using their sha3-names
instead of sha1-names. Because a blob object does not refer to any
other object, its sha1-content and sha3-content are the same.
The format allows round-trip conversion between sha3-content and
sha1-content.
Object storage
~~~~~~~~~~~~~~
Loose objects use zlib compression and packed objects use the packed
format described in Documentation/technical/pack-format.txt, just like
today. The content that is compressed and stored uses sha3-content
instead of sha1-content.
Translation table
~~~~~~~~~~~~~~~~~
A fast bidirectional mapping between sha1-names and sha3-names of all
local objects in the repository is kept on disk. The exact format of
that mapping is to be determined.
All operations that make new objects (e.g., "git commit") add the new
objects to the translation table.
(This work could have been deferred to push time, but that would
significantly complicate and slow down pushes. Calculating the
sha1-name at object creation time at the same time it is being
streamed to disk and having its sha3-name calculated should be an
acceptable cost.)
Reading an object's sha1-content
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
The sha1-content of an object can be read by converting all sha3-names
its sha3-content references to sha1-names using the translation table.
Fetch
~~~~~
Fetching from a SHA-1 based server requires translating between SHA-1
and SHA3-256 based representations on the fly.
SHA-1s named in the ref advertisement that are present on the client
can be translated to SHA3-256 and looked up as local objects using the
translation table.
Negotiation proceeds as today. Any "have"s generated locally are
converted to SHA-1 before being sent to the server, and SHA-1s
mentioned by the server are converted to SHA3-256 when looking them up
locally.
After negotiation, the server sends a packfile containing the
requested objects. We convert the packfile to SHA3-256 format using
the following steps:
1. index-pack: inflate each object in the packfile and compute its
SHA-1. Objects can contain deltas in OBJ_REF_DELTA format against
objects the client has locally. These objects can be looked up
using the translation table and their sha1-content read as
described above to resolve the deltas.
2. topological sort: starting at the "want"s from the negotiation
phase, walk through objects in the pack and emit a list of them,
excluding blobs, in reverse topologically sorted order, with each
object coming later in the list than all objects it references.
(This list only contains objects reachable from the "wants". If the
pack from the server contained additional extraneous objects, then
they will be discarded.)
3. convert to sha3: open a new (sha3) packfile. Read the topologically
sorted list just generated. For each object, inflate its
sha1-content, convert to sha3-content, and write it to the sha3
pack. Include the new sha1<->sha3 mapping entry in the translation
table.
4. sort: reorder entries in the new pack to match the order of objects
in the pack the server generated and include blobs. Write a sha3 idx
file.
5. clean up: remove the SHA-1 based pack file, index, and
topologically sorted list obtained from the server and steps 1
and 2.
Step 3 requires every object referenced by the new object to be in the
translation table. This is why the topological sort step is necessary.
As an optimization, step 1 could write a file describing what non-blob
objects each object it has inflated from the packfile references. This
makes the topological sort in step 2 possible without inflating the
objects in the packfile for a second time. The objects need to be
inflated again in step 3, for a total of two inflations.
Step 4 is probably necessary for good read-time performance. "git
pack-objects" on the server optimizes the pack file for good data
locality (see Documentation/technical/pack-heuristics.txt).
Details of this process are likely to change. It will take some
experimenting to get this to perform well.
Push
~~~~
Push is simpler than fetch because the objects referenced by the
pushed objects are already in the translation table. The sha1-content
of each object being pushed can be read as described in the "Reading
an object's sha1-content" section to generate the pack written by git
send-pack.
Signed Commits
~~~~~~~~~~~~~~
We add a new field "gpgsig-sha3" to the commit object format to allow
signing commits without relying on SHA-1. It is similar to the
existing "gpgsig" field. Its signed payload is the sha3-content of the
commit object with any "gpgsig" and "gpgsig-sha3" fields removed.
This means commits can be signed
1. using SHA-1 only, as in existing signed commit objects
2. using both SHA-1 and SHA3-256, by using both gpgsig-sha3 and gpgsig
fields.
3. using only SHA3-256, by only using the gpgsig-sha3 field.
Old versions of "git verify-commit" can verify the gpgsig signature in
cases (1) and (2) without modifications and view case (3) as an
ordinary unsigned commit.
Signed Tags
~~~~~~~~~~~
We add a new field "gpgsig-sha3" to the tag object format to allow
signing tags without relying on SHA-1. Its signed payload is the
sha3-content of the tag with its gpgsig-sha3 field and "-----BEGIN PGP
SIGNATURE-----" delimited in-body signature removed.
This means tags can be signed
1. using SHA-1 only, as in existing signed tag objects
2. using both SHA-1 and SHA3-256, by using gpgsig-sha3 and an in-body
signature.
3. using only SHA3-256, by only using the gpgsig-sha3 field.
Mergetag embedding
~~~~~~~~~~~~~~~~~~
The mergetag field in the sha1-content of a commit contains the
sha1-content of a tag that was merged by that commit.
The mergetag field in the sha3-content of the same commit contains the
sha3-content of the same tag.
Submodules
~~~~~~~~~~
To convert recorded submodule pointers, you need to have the converted
submodule repository in place. The translation table of the submodule
can be used to look up the new hash.
Caveats
-------
Invalid objects
~~~~~~~~~~~~~~~
The conversion from sha1-content to sha3-content retains any
brokenness in the original object (e.g., tree entry modes encoded with
leading 0, tree objects whose paths are not sorted correctly, and
commit objects without an author or committer). This is a deliberate
feature of the design to allow the conversion to round-trip.
More profoundly broken objects (e.g., a commit with a truncated "tree"
header line) cannot be converted but were not usable by current Git
anyway.
Shallow clone and submodules
~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Because it requires all referenced objects to be available in the
locally generated translation table, this design does not support
shallow clone or unfetched submodules. Protocol improvements might
allow lifting this restriction.
Alternates
~~~~~~~~~~
For the same reason, a sha3 repository cannot borrow objects from a
sha1 repository using objects/info/alternates or
$GIT_ALTERNATE_OBJECT_REPOSITORIES.
git notes
~~~~~~~~~
The "git notes" tool annotates objects using their sha1-name as key.
This design does not describe a way to migrate notes trees to use
sha3-names. That migration is expected to happen separately (for
example using a file at the root of the notes tree to describe which
hash it uses).
Server-side cost
~~~~~~~~~~~~~~~~
Until Git protocol gains SHA3-256 support, using sha3 based storage on
public-facing Git servers is strongly discouraged. Once Git protocol
gains SHA3-256 support, sha3 based servers are likely not to support
sha1 compatibility, to avoid what may be a very expensive hash
reencode during clone and to encourage peers to modernize.
The design described here allows fetches by SHA-1 clients of a
personal SHA256 repository because it's not much more difficult than
allowing pushes from that repository. This support needs to be guarded
by a configuration option --- servers like git.kernel.org that serve a
large number of clients would not be expected to bear that cost.
Meaning of signatures
~~~~~~~~~~~~~~~~~~~~~
The signed payload for signed commits and tags does not explicitly
name the hash used to identify objects. If some day Git adopts a new
hash function with the same length as the current SHA-1 (40
hexadecimal digit) or SHA2-256 (64 hexadecimal digit) objects then the
intent behind the PGP signed payload in an object signature is
unclear:
object e7e07d5a4fcc2a203d9873968ad3e6bd4d7419d7
type commit
tag v2.12.0
tagger Junio C Hamano <gitster@pobox.com> 1487962205 -0800
Git 2.12
Does this mean Git v2.12.0 is the commit with sha1-name
e7e07d5a4fcc2a203d9873968ad3e6bd4d7419d7 or the commit with
new-40-digit-hash-name e7e07d5a4fcc2a203d9873968ad3e6bd4d7419d7?
Fortunately SHA3-256 and SHA-1 have different lengths. If Git starts
using another hash with the same length to name objects, then it will
need to change the format of signed payloads using that hash to
address this issue.
Alternatives considered
-----------------------
Upgrading everyone working on a particular project on a flag day
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Projects like the Linux kernel are large and complex enough that
flipping the switch for all projects based on the repository at once
is infeasible.
Not only would all developers and server operators supporting
developers have to switch on the same flag day, but supporting tooling
(continuous integration, code review, bug trackers, etc) would have to
be adapted as well. This also makes it difficult to get early feedback
from some project participants testing before it is time for mass
adoption.
Using hash functions in parallel
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
(e.g. https://public-inbox.org/git/22708.8913.864049.452252@chiark.greenend.org.uk/ )
Objects newly created would be addressed by the new hash, but inside
such an object (e.g. commit) it is still possible to address objects
using the old hash function.
* You cannot trust its history (needed for bisectability) in the
future without further work
* Maintenance burden as the number of supported hash functions grows
(they will never go away, so they accumulate). In this proposal, by
comparison, converted objects lose all references to SHA-1.
Signed objects with multiple hashes
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Instead of introducing the gpgsig-sha3 field in commit and tag objects
for sha3-content based signatures, an earlier version of this design
added "hash sha3 <sha3-name>" fields to strengthen the existing
sha1-content based signatures.
In other words, a single signature was used to attest to the object
content using both hash functions. This had some advantages:
* Using one signature instead of two speeds up the signing process.
* Having one signed payload with both hashes allows the signer to
attest to the sha1-name and sha3-name referring to the same object.
* All users consume the same signature. Broken signatures are likely
to be detected quickly using current versions of git.
However, it also came with disadvantages:
* Verifying a signed object requires access to the sha1-names of all
objects it references, even after the transition is complete and
translation table is no longer needed for anything else. To support
this, the design added fields such as "hash sha1 tree <sha1-name>"
and "hash sha1 parent <sha1-name>" to the sha3-content of a signed
commit, complicating the conversion process.
* Allowing signed objects without a sha1 (for after the transition is
complete) complicated the design further, requiring a "nohash sha1"
field to suppress including "hash sha1" fields in the sha3-content
and signed payload.
Document History
----------------
2017-03-03
bmwill@google.com, jonathantanmy@google.com, jrnieder@gmail.com,
sbeller@google.com
Initial version sent to
http://public-inbox.org/git/20170304011251.GA26789@aiede.mtv.corp.google.com
2017-03-03 jrnieder@gmail.com
Incorporated suggestions from jonathantanmy and sbeller:
* describe purpose of signed objects with each hash type
* redefine signed object verification using object content under the
first hash function
2017-03-06 jrnieder@gmail.com
* Use SHA3-256 instead of SHA2 (thanks, Linus and brian m. carlson).[1][2]
* Make sha3-based signatures a separate field, avoiding the need for
"hash" and "nohash" fields (thanks to peff[3]).
* Add a sorting phase to fetch (thanks to Junio for noticing the need
for this).
* Omit blobs from the topological sort during fetch (thanks to peff).
* Discuss alternates, git notes, and git servers in the caveats
section (thanks to Junio Hamano, brian m. carlson[4], and Shawn
Pearce).
* Clarify language throughout (thanks to various commenters,
especially Junio).
[1] http://public-inbox.org/git/CA+55aFzJtejiCjV0e43+9oR3QuJK2PiFiLQemytoLpyJWe6P9w@mail.gmail.com/
[2] http://public-inbox.org/git/CA+55aFz+gkAsDZ24zmePQuEs1XPS9BP_s8O7Q4wQ7LV7X5-oDA@mail.gmail.com/
[3] http://public-inbox.org/git/20170306084353.nrns455dvkdsfgo5@sigill.intra.peff.net/
[4] http://public-inbox.org/git/20170304224936.rqqtkdvfjgyezsht@genre.crustytoothpaste.net
^ permalink raw reply
* Re: RFC: Another proposed hash function transition plan
From: Mike Hommey @ 2017-03-07 0:03 UTC (permalink / raw)
To: Jonathan Nieder
Cc: David Lang, git, sbeller, bmwill, jonathantanmy, peff,
Linus Torvalds
In-Reply-To: <20170306234030.GB26789@aiede.mtv.corp.google.com>
On Mon, Mar 06, 2017 at 03:40:30PM -0800, Jonathan Nieder wrote:
> David Lang wrote:
>
> >> Translation table
> >> ~~~~~~~~~~~~~~~~~
> >> A fast bidirectional mapping between sha1-names and sha256-names of
> >> all local objects in the repository is kept on disk. The exact format
> >> of that mapping is to be determined.
> >>
> >> All operations that make new objects (e.g., "git commit") add the new
> >> objects to the translation table.
> >
> > This seems like a rather nontrival thing to design. It will need to
> > hold millions of mappings, and be quickly searchable from either
> > direction (sha1->new and new->sha1) while still be fairly fast to
> > insert new records into.
>
> I am currently thinking of using LevelDB, since it has the advantages of
> being simple, already existing, and having already been ported to Java
> (allowing JGit can read and write the same format).
>
> If that doesn't work, we'd try some other key-value store like Samba's
> tdb or Kyoto Cabinet.
FWIW, I'm using notes-like data to store mercurial->git mappings in
git-cinnabar, (ab)using the commit type in tree items. It's fast enough.
Mike
^ permalink raw reply
* Re: [Request for Documentation] Differentiate signed (commits/tags/pushes)
From: Junio C Hamano @ 2017-03-07 0:16 UTC (permalink / raw)
To: Jakub Narębski; +Cc: Stefan Beller, tom, Matthieu Moy, git@vger.kernel.org
In-Reply-To: <47ad8b6d-0a65-2f8c-dcc5-49a8a8d5ab2a@gmail.com>
Jakub Narębski <jnareb@gmail.com> writes:
> Also from what I remember signed commits came before mergetags, that
> is the result of merging a signed tag (storing the signature of
> one of parents of the merge commit to not pollute tag namespace).
>
> And this workflow, from what I know, is quite useful.
The "commit -s" on a merge commit lets you as the integrator to
attest that you made that merge. The "mergetag" records the
signature by the contributor that says the tip that was merged was
what the contributor wanted to get merged.
It is entirely reasonable to sign a merge commit that merges a
signed tag. They serve two different and unrelated purposes.
^ permalink raw reply
* Re: [PATCH v5 1/1] config: add conditional include
From: Stefan Beller @ 2017-03-06 22:44 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy
Cc: git@vger.kernel.org, Junio C Hamano, Jeff King,
Sebastian Schuberth, Matthieu Moy
In-Reply-To: <20170223122346.12222-2-pclouds@gmail.com>
Being late to the review party here.
> +static int include_condition_is_true(const char *cond, size_t cond_len)
> +{
...
> +
> + error(_("unrecognized include condition: %.*s"), (int)cond_len, cond);
> + /* unknown conditionals are always false */
> + return 0;
> +}
Thanks for putting an error message here. I was looking at what
is currently queued as origin/nd/conditional-config-include,
which doesn't have this error() (yet / not any more?)
I'd strongly suggest to keep the error message here as that way
a user can diagnose e.g. a typo in the condition easily.
If we plan to extend this list of conditions in the future, and a user
switches between versions of git, then they may see this message
on a regular basis (whenever they use the 'old' version).
In that case it may be only a warning() instead of an error(),
but I have no strong opinion on that.
---
Reason why I am reviewing this series now:
I thought about extending the config system for submodule
usage (see debate at [1]).
My gut reaction was to have a condition for "if a superproject
exists" and then include a special config (e.g. "config.super"
in the superprojects $GIT_DIR).
However while reviewing these patches I realized
I am not interested in conditional includes, but when setting
up the submodules we know for a fact that we have a superproject,
so no conditional needed. Instead we need a special markup
of paths, i.e. we want to have an easy way to say
"$GIT_DIR of superproject". Ideas how to do that?
Thanks,
Stefan
[1] https://public-inbox.org/git/84fcb0bd-85dc-0142-dd58-47a04eaa7c2b@durchholz.org/
^ permalink raw reply
* [PATCH 06/18] make is_submodule_populated gently
From: Stefan Beller @ 2017-03-06 20:59 UTC (permalink / raw)
To: sbeller, gitster; +Cc: git, bmwill, novalis, sandals, hvoigt, jrnieder, ramsay
In-Reply-To: <20170306205919.9713-1-sbeller@google.com>
We need the gentle version in a later patch. As we have just one caller,
migrate the caller.
Signed-off-by: Stefan Beller <sbeller@google.com>
---
builtin/grep.c | 2 +-
submodule.c | 7 ++-----
submodule.h | 8 +++++++-
3 files changed, 10 insertions(+), 7 deletions(-)
diff --git a/builtin/grep.c b/builtin/grep.c
index 2c727ef499..b17835aed6 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -616,7 +616,7 @@ static int grep_submodule(struct grep_opt *opt, const unsigned char *sha1,
{
if (!is_submodule_initialized(path))
return 0;
- if (!is_submodule_populated(path)) {
+ if (!is_submodule_populated_gently(path, NULL)) {
/*
* If searching history, check for the presense of the
* submodule's gitdir before skipping the submodule.
diff --git a/submodule.c b/submodule.c
index 3b98766a6b..0e55372f37 100644
--- a/submodule.c
+++ b/submodule.c
@@ -234,15 +234,12 @@ int is_submodule_initialized(const char *path)
return ret;
}
-/*
- * Determine if a submodule has been populated at a given 'path'
- */
-int is_submodule_populated(const char *path)
+int is_submodule_populated_gently(const char *path, int *return_error_code)
{
int ret = 0;
char *gitdir = xstrfmt("%s/.git", path);
- if (resolve_gitdir(gitdir))
+ if (resolve_gitdir_gently(gitdir, return_error_code))
ret = 1;
free(gitdir);
diff --git a/submodule.h b/submodule.h
index 05ab674f06..0b915bd3ac 100644
--- a/submodule.h
+++ b/submodule.h
@@ -41,7 +41,13 @@ extern int submodule_config(const char *var, const char *value, void *cb);
extern void gitmodules_config(void);
extern void gitmodules_config_sha1(const unsigned char *commit_sha1);
extern int is_submodule_initialized(const char *path);
-extern int is_submodule_populated(const char *path);
+/*
+ * Determine if a submodule has been populated at a given 'path' by checking if
+ * the <path>/.git resolves to a valid git repository.
+ * If return_error_code is NULL, die on error.
+ * Otherwise the return error code is the same as of resolve_gitdir_gently.
+ */
+extern int is_submodule_populated_gently(const char *path, int *return_error_code);
extern int parse_submodule_update_strategy(const char *value,
struct submodule_update_strategy *dst);
extern const char *submodule_strategy_to_string(const struct submodule_update_strategy *s);
--
2.12.0.rc1.52.ge239d7e709.dirty
^ permalink raw reply related
* Re: [Request for Documentation] Differentiate signed (commits/tags/pushes)
From: Junio C Hamano @ 2017-03-06 23:03 UTC (permalink / raw)
To: Stefan Beller; +Cc: tom, Matthieu Moy, git@vger.kernel.org
In-Reply-To: <xmqqshmqm4ur.fsf@junio-linux.mtv.corp.google.com>
Junio C Hamano <gitster@pobox.com> writes:
> Stefan Beller <sbeller@google.com> writes:
>
>> What is the difference between signed commits and tags?
>> (Not from a technical perspective, but for the end user)
> ...
>> A signed push can certify that a given payload (consisting
>> of multiple commits on possibly multiple branches) was transmitted
>> to a remote, which can be recorded by the remote as e.g. a proof
>> of work.
> ...
> A signed push is _NOT_ about certifying the objects in the history
> DAG. It is about certifying the _intent_ of pointing _REFS_ into
> points in the object graph.
> ...
> Historically, "tag -s" came a lot earlier. When a project for
> whatever reason wants signature for each and every commit so that
> they somehow can feel good, without "commit -s", it would have made
> us unnecessary work to scale tag namespace only because there will
> be tons of pointless tags. "commit -s" was a remedy for that.
While we are enumerating them, it is worth mentioning the mergetag
header of a commit object.
This is added to a (merge) commit object when you merged a signed
tag that points at a commit, and the intent is to eliminate the need
to _keep_ the ref around that is created only for the purpose of
"please pull from me, I tagged and signed the tip of the history I
want you to pull" request. From that point of view, you could say
it is also reducing the load on refs/tags/ namespace, but more
importantly by not requiring the ref around, it allows you to verify
that the merge commit merged the correct tag with _only_ the commit
object by reproducing the payload of the signed tag that was merged
in the commit object in full.
^ permalink raw reply
* Re: [Request for Documentation] Differentiate signed (commits/tags/pushes)
From: Stefan Beller @ 2017-03-07 0:58 UTC (permalink / raw)
To: Junio C Hamano
Cc: tom, Matthieu Moy, git@vger.kernel.org, Jakub Narębski
In-Reply-To: <xmqq4lz6ymlt.fsf@junio-linux.mtv.corp.google.com>
On Mon, Mar 6, 2017 at 4:08 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Stefan Beller <sbeller@google.com> writes:
>
>>> "tag -s" also has the benefit of being retroactive. You can create
>>> commit, think about it for a week and then later tag it. And ask
>>> others to also tag the same one. You cannot do so with "commit -s".
>>
>> ok, so there is *no* advantage of signing a commit over tags?
>
> Did I say anything that remotely resembles that? Puzzled.
Well that was brain having a short circuit.
>
> If the reason you want to have GPG signature on a commit is not
> because you want to mark some meaningful place in the history, but
> you are signing each and every ones out of some random reason,
and I am looking for these "some random reason"s.
If it is e.g. a ISO9001 requirement, I'll happily accept that as such.
By signing things, you certify your intent, i.e. by signing a commit,
you certify that you intent to create the commit as-is in some repository
on some branch (unlike the push certificate that specifies the repo and
branch).
> there
> is no reason why you would want "tag -s" them, so you can see it as
> an advantage of "commit -s" over "tag -s", because to such a
> project, all commits that are not tagged look the same and there is
> no "landmark" value to use "tag -s" for each and every one of them.
Okay. They are two different things, but to me they seem to archive
the same thing, with a tag having more niceties provided.
e.g. when you make a new release, you could just bump the version
in the versions file and sign the commit. As the commit is part of the
master branch it would not get lost.
The formerly mentioned "not polluting the refs/tags namespace"
is applicable to mergetags, that are a side tangent to signing
the commit vs creating a tag?
Now as Jakub mentions that signed commits came before the
mergetags were introduced, the existence of signed commits
sort of makes sense, as they were there first, but now are
superseded by more powerful tools.
> It is entirely reasonable to sign a merge commit that merges a
> signed tag. They serve two different and unrelated purposes.
The signed tag that gets merged certifies the intent of the lieutenant
to ask for this specific content to be pulled and integrated, whereas
the signing of the commit certifies that the integrator intends to create
the merge commit as-is and e.g. resolve the merge conflicts as recorded.
Thanks,
Stefan
^ permalink raw reply
* Re: [PATCH] blame: draft of line format
From: Edmundo Carmona Antoranz @ 2017-03-06 23:29 UTC (permalink / raw)
To: Jeff King; +Cc: Git List
In-Reply-To: <20170131194138.fbcbdnkfjgizxnoa@sigill.intra.peff.net>
On Tue, Jan 31, 2017 at 1:41 PM, Jeff King <peff@peff.net> wrote:
> 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
Thanks for the tip, Peff. It made the code to get rev info much
shorter. I'll work on some other improvements and then I'll send
another patch.
Best regards!
^ permalink raw reply
* [PATCH 09/18] update submodules: add a config option to determine if submodules are updated
From: Stefan Beller @ 2017-03-06 20:59 UTC (permalink / raw)
To: sbeller, gitster; +Cc: git, bmwill, novalis, sandals, hvoigt, jrnieder, ramsay
In-Reply-To: <20170306205919.9713-1-sbeller@google.com>
In later patches we introduce the options and flag for commands
that modify the working directory, e.g. git-checkout.
Have a central place to store such settings whether we want to update
a submodule.
Signed-off-by: Stefan Beller <sbeller@google.com>
---
submodule.c | 6 ++++++
submodule.h | 1 +
2 files changed, 7 insertions(+)
diff --git a/submodule.c b/submodule.c
index 04d185738f..591f4a694e 100644
--- a/submodule.c
+++ b/submodule.c
@@ -17,6 +17,7 @@
#include "worktree.h"
static int config_fetch_recurse_submodules = RECURSE_SUBMODULES_ON_DEMAND;
+static int config_update_recurse_submodules = RECURSE_SUBMODULES_DEFAULT;
static int parallel_jobs = 1;
static struct string_list changed_submodule_paths = STRING_LIST_INIT_NODUP;
static int initialized_fetch_ref_tips;
@@ -542,6 +543,11 @@ void set_config_fetch_recurse_submodules(int value)
config_fetch_recurse_submodules = value;
}
+void set_config_update_recurse_submodules(int value)
+{
+ config_update_recurse_submodules = value;
+}
+
static int has_remote(const char *refname, const struct object_id *oid,
int flags, void *cb_data)
{
diff --git a/submodule.h b/submodule.h
index 0b915bd3ac..b4e60c08d2 100644
--- a/submodule.h
+++ b/submodule.h
@@ -64,6 +64,7 @@ extern void show_submodule_inline_diff(FILE *f, const char *path,
const char *del, const char *add, const char *reset,
const struct diff_options *opt);
extern void set_config_fetch_recurse_submodules(int value);
+extern void set_config_update_recurse_submodules(int value);
extern void check_for_new_submodule_commits(unsigned char new_sha1[20]);
extern int fetch_populated_submodules(const struct argv_array *options,
const char *prefix, int command_line_option,
--
2.12.0.rc1.52.ge239d7e709.dirty
^ permalink raw reply related
* [RFCv7 PATCH 00/18] Checkout aware of Submodules!
From: Stefan Beller @ 2017-03-06 20:59 UTC (permalink / raw)
To: sbeller, gitster; +Cc: git, bmwill, novalis, sandals, hvoigt, jrnieder, ramsay
In-Reply-To: <20170302004759.27852-1-sbeller@google.com>
previous work:
https://public-inbox.org/git/20170302004759.27852-1-sbeller@google.com
https://public-inbox.org/git/20161203003022.29797-1-sbeller@google.com/
v7:
* addressed Erics comment by fixing the bashism in t/lib-submodule-update.sh
diff to v6:
diff --git a/t/lib-submodule-update.sh b/t/lib-submodule-update.sh
index 5caae06bc5..949ebd546c 100755
--- a/t/lib-submodule-update.sh
+++ b/t/lib-submodule-update.sh
@@ -255,7 +255,7 @@ test_superproject_content () {
# Test that the given submodule at path "$1" contains the content according
# to the submodule commit recorded in the superproject's commit "$2"
test_submodule_content () {
- if test "$1" == "-C"
+ if test x"$1" = "x-C"
then
cd "$2"
shift; shift;
* interdiff to v5 below
v6:
* added support for read-tree (see last patch) to see how generic the
code of the previous patches is. I am pretty pleased with that patch.
* marked two functions static. Thanks Ramsay!
* fixed the recursive test; it still fails but it is the code that fails,
not the test. For this I had to change the setup code slightly.
* 2 new patches adding tiny refactoring to the submodule test lib.
* interdiff (to origin/sb/checkout-recurse-submodules, which is v5) below.
v5:
* as v4 was the first version queued by Junio, we do have an interdiff below!
* renamed functions
* changed the API, now the caller has to take care of the submodule strategy
themselves. (Note this can be different for different situations, e.g.
when a submodule is deleted, we can do that for any strategy except none and
!command. But for moving to a new state of the submodule we currently
only implement "checkout" [unspecified defaults to checkout]. warning about
the others, doing nothing there.)
v4:
* addressed all comments of Brian, Junio and Brandon.
Thanks!
* one major point of change is the introduction of another patch
"lib-submodule-update.sh: do not use ./. as submodule remote",
as that took some time to track down the existing bug.
v3:
* moved tests from t2013 to the generic submodule library.
* factored out the refactoring patches to be up front
* As I redid the complete implementation, I have the impression this time
it is cleaner than previous versions.
I think we still have to fix the corner cases of directory/file/submodule
conflicts before merging, but this serves as a status update on my current
way of thinking how to implement the worktree commands being aware of
submodules.
Thanks,
Stefan
v2:
* based on top of the series sent out an hour ago
"[PATCHv4 0/5] submodule embedgitdirs"
* Try to embed a submodule if we need to remove it.
* Strictly do not change behavior if not giving the new flag.
* I think I missed some review comments from v1, but I'd like to get
the current state out over the weekend, as a lot has changed so far.
On Monday I'll go through the previous discussion with a comb to see
if I missed something.
v1:
When working with submodules, nearly anytime after checking out
a different state of the projects, that has submodules changed
you'd run "git submodule update" with a current version of Git.
There are two problems with this approach:
* The "submodule update" command is dangerous as it
doesn't check for work that may be lost in the submodule
(e.g. a dangling commit).
* you may forget to run the command as checkout is supposed
to do all the work for you.
Integrate updating the submodules into git checkout, with the same
safety promises that git-checkout has, i.e. not throw away data unless
asked to. This is done by first checking if the submodule is at the same
sha1 as it is recorded in the superproject. If there are changes we stop
proceeding the checkout just like it is when checking out a file that
has local changes.
The integration happens in the code that is also used in other commands
such that it will be easier in the future to make other commands aware
of submodule.
This also solves d/f conflicts in case you replace a file/directory
with a submodule or vice versa.
The patches are still a bit rough, but the overall series seems
promising enough to me that I want to put it out here.
Any review, specifically on the design level welcome!
Thanks,
Stefan
Stefan Beller (14):
lib-submodule-update.sh: reorder create_lib_submodule_repo
lib-submodule-update.sh: define tests for recursing into submodules
make is_submodule_populated gently
connect_work_tree_and_git_dir: safely create leading directories
update submodules: add submodule config parsing
update submodules: add a config option to determine if submodules are
updated
update submodules: introduce is_interesting_submodule
update submodules: move up prepare_submodule_repo_env
update submodules: add submodule_go_from_to
unpack-trees: pass old oid to verify_clean_submodule
unpack-trees: check if we can perform the operation for submodules
read-cache: remove_marked_cache_entries to wipe selected submodules.
entry.c: update submodules when interesting
builtin/checkout: add --recurse-submodules switch
diff --git a/Documentation/git-read-tree.txt b/Documentation/git-read-tree.txt
index fa1d557e5b..ed9d63ef4a 100644
--- a/Documentation/git-read-tree.txt
+++ b/Documentation/git-read-tree.txt
@@ -115,6 +115,12 @@ OPTIONS
directories the index file and index output file are
located in.
+--[no-]recurse-submodules::
+ Using --recurse-submodules will update the content of all initialized
+ submodules according to the commit recorded in the superproject by
+ calling read-tree recursively, also setting the submodules HEAD to be
+ detached at that commit.
+
--no-sparse-checkout::
Disable sparse checkout support even if `core.sparseCheckout`
is true.
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 207ce09771..e9c5fcfaf8 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -29,8 +29,8 @@ static const char * const checkout_usage[] = {
NULL,
};
-int option_parse_recurse_submodules(const struct option *opt,
- const char *arg, int unset)
+static int option_parse_recurse_submodules(const struct option *opt,
+ const char *arg, int unset)
{
if (unset) {
recurse_submodules = RECURSE_SUBMODULES_OFF;
diff --git a/builtin/read-tree.c b/builtin/read-tree.c
index 8ba64bc785..2dc5cc06da 100644
--- a/builtin/read-tree.c
+++ b/builtin/read-tree.c
@@ -15,10 +15,13 @@
#include "builtin.h"
#include "parse-options.h"
#include "resolve-undo.h"
+#include "submodule.h"
+#include "submodule-config.h"
static int nr_trees;
static int read_empty;
static struct tree *trees[MAX_UNPACK_TREES];
+int recurse_submodules = RECURSE_SUBMODULES_DEFAULT;
static int list_tree(unsigned char *sha1)
{
@@ -96,6 +99,23 @@ static int debug_merge(const struct cache_entry * const *stages,
return 0;
}
+static int option_parse_recurse_submodules(const struct option *opt,
+ const char *arg, int unset)
+{
+ if (unset) {
+ recurse_submodules = RECURSE_SUBMODULES_OFF;
+ return 0;
+ }
+ if (arg)
+ recurse_submodules =
+ parse_update_recurse_submodules_arg(opt->long_name,
+ arg);
+ else
+ recurse_submodules = RECURSE_SUBMODULES_ON;
+
+ return 0;
+}
+
static struct lock_file lock_file;
int cmd_read_tree(int argc, const char **argv, const char *unused_prefix)
@@ -137,6 +157,9 @@ int cmd_read_tree(int argc, const char **argv, const char *unused_prefix)
N_("skip applying sparse checkout filter")),
OPT_BOOL(0, "debug-unpack", &opts.debug_unpack,
N_("debug unpack-trees")),
+ { OPTION_CALLBACK, 0, "recurse-submodules", &recurse_submodules,
+ "checkout", "control recursive updating of submodules",
+ PARSE_OPT_OPTARG, option_parse_recurse_submodules },
OPT_END()
};
@@ -152,6 +175,12 @@ int cmd_read_tree(int argc, const char **argv, const char *unused_prefix)
hold_locked_index(&lock_file, LOCK_DIE_ON_ERROR);
+ if (recurse_submodules != RECURSE_SUBMODULES_DEFAULT) {
+ gitmodules_config();
+ git_config(submodule_config, NULL);
+ set_config_update_recurse_submodules(RECURSE_SUBMODULES_ON);
+ }
+
prefix_set = opts.prefix ? 1 : 0;
if (1 < opts.merge + opts.reset + prefix_set)
die("Which one? -m, --reset, or --prefix?");
diff --git a/submodule.c b/submodule.c
index a2cf8c9376..bc5fecf8c5 100644
--- a/submodule.c
+++ b/submodule.c
@@ -1257,7 +1257,7 @@ static int submodule_has_dirty_index(const struct submodule *sub)
return finish_command(&cp);
}
-void submodule_reset_index(const char *path)
+static void submodule_reset_index(const char *path)
{
struct child_process cp = CHILD_PROCESS_INIT;
prepare_submodule_repo_env_no_git_dir(&cp.env_array);
diff --git a/t/lib-submodule-update.sh b/t/lib-submodule-update.sh
index 54cd8a6366..429cd1041d 100755
--- a/t/lib-submodule-update.sh
+++ b/t/lib-submodule-update.sh
@@ -4,7 +4,7 @@
# - New submodule (no_submodule => add_sub1)
# - Removed submodule (add_sub1 => remove_sub1)
# - Updated submodule (add_sub1 => modify_sub1)
-# - Updated submodule recursively (modify_sub1 => modify_sub1_recursively)
+# - Updated submodule recursively (add_nested_sub => modify_sub1_recursively)
# - Submodule updated to invalid commit (add_sub1 => invalid_sub1)
# - Submodule updated from invalid commit (invalid_sub1 => valid_sub1)
# - Submodule replaced by tracked files in directory (add_sub1 =>
@@ -20,8 +20,8 @@
# / ^
# / remove_sub1
# /
-# add_sub1 /-------O---------O
-# | / ^ modify_sub1_recursive
+# add_sub1 /-------O---------O--------O modify_sub1_recursively
+# | / ^ add_nested_sub
# | / modify_sub1
# v/
# O------O-----------O---------O
@@ -96,12 +96,24 @@ create_lib_submodule_repo () {
git add sub1 &&
git commit -m "Modify sub1" &&
- git checkout -b modify_sub1_recursively modify_sub1 &&
+ git checkout -b add_nested_sub modify_sub1 &&
git -C sub1 checkout -b "add_nested_sub" &&
git -C sub1 submodule add --branch no_submodule ../submodule_update_sub2 sub2 &&
git -C sub1 commit -a -m "add a nested submodule" &&
git add sub1 &&
git commit -a -m "update submodule, that updates a nested submodule" &&
+ git checkout -b modify_sub1_recursively &&
+ git -C sub1 checkout -b modify_sub1_recursively &&
+ git -C sub1/sub2 checkout -b modify_sub1_recursively &&
+ echo change >sub1/sub2/file3 &&
+ git -C sub1/sub2 add file3 &&
+ git -C sub1/sub2 commit -m "make a change in nested sub" &&
+ git -C sub1 add sub2 &&
+ git -C sub1 commit -m "update nested sub" &&
+ git add sub1 &&
+ git commit -m "update sub1, that updates nested sub" &&
+ git -C sub1 push origin modify_sub1_recursively &&
+ git -C sub1/sub2 push origin modify_sub1_recursively &&
git -C sub1 submodule deinit -f --all &&
git checkout -b replace_sub1_with_directory add_sub1 &&
@@ -200,9 +212,9 @@ reset_work_tree_to () {
git checkout -f "$1" &&
git status -u -s >actual &&
test_must_be_empty actual &&
- sha1=$(git rev-parse --revs-only HEAD:sub1) &&
- if test -n "$sha1" &&
- test $(cd "../submodule_update_sub1" && git rev-parse --verify "$sha1^{commit}")
+ hash=$(git rev-parse --revs-only HEAD:sub1) &&
+ if test -n "$hash" &&
+ test $(cd "../submodule_update_sub1" && git rev-parse --verify "$hash^{commit}")
then
git submodule update --init --recursive "sub1"
fi
@@ -211,14 +223,23 @@ reset_work_tree_to () {
reset_work_tree_to_interested () {
reset_work_tree_to $1 &&
- # indicate we are interested in the submodule:
- git -C submodule_update config submodule.sub1.url "bogus" &&
- # also have it available:
+ # make the submodule git dirs available
if ! test -d submodule_update/.git/modules/sub1
then
mkdir -p submodule_update/.git/modules &&
cp -r submodule_update_repo/.git/modules/sub1 submodule_update/.git/modules/sub1
- fi
+ GIT_WORK_TREE=. git -C submodule_update/.git/modules/sub1 config --unset core.worktree
+ fi &&
+ if ! test -d submodule_update/.git/modules/sub1/modules/sub2
+ then
+ mkdir -p submodule_update/.git/modules/sub1/modules &&
+ cp -r submodule_update_repo/.git/modules/sub1/modules/sub2 submodule_update/.git/modules/sub1/modules/sub2
+ GIT_WORK_TREE=. git -C submodule_update/.git/modules/sub1/modules/sub2 config --unset core.worktree
+ fi &&
+ # indicate we are interested in the submodule:
+ git -C submodule_update config submodule.sub1.url "bogus" &&
+ # sub1 might not be checked out, so use the git dir
+ git -C submodule_update/.git/modules/sub1 config submodule.sub2.url "bogus"
}
# Test that the superproject contains the content according to commit "$1"
@@ -234,6 +255,11 @@ test_superproject_content () {
# Test that the given submodule at path "$1" contains the content according
# to the submodule commit recorded in the superproject's commit "$2"
test_submodule_content () {
+ if test x"$1" == "x-C"
+ then
+ cd "$2"
+ shift; shift;
+ fi
if test $# != 2
then
echo "test_submodule_content needs two arguments"
@@ -761,6 +787,11 @@ test_submodule_switch_recursing () {
then
RESULT=failure
fi
+
+ if test "$KNOWN_FAILURE_SUBMODULE_OVERWRITE_IGNORED_UNTRACKED" = 1
+ then
+ RESULT1=failure
+ fi
######################### Appearing submodule #########################
# Switching to a commit letting a submodule appear checks it out ...
test_expect_success "$command: added submodule is checked out" '
@@ -801,7 +832,7 @@ test_submodule_switch_recursing () {
)
'
# ... but an ignored file is fine.
- test_expect_success "$command: added submodule removes an untracked ignored file" '
+ test_expect_$RESULT1 "$command: added submodule removes an untracked ignored file" '
test_when_finished "rm submodule_update/.git/info/exclude" &&
prolog &&
reset_work_tree_to_interested no_submodule &&
@@ -966,21 +997,17 @@ test_submodule_switch_recursing () {
)
'
- # This test fails, due to missing setup, we do not clone sub2 into
- # submodule_update, because it doesn't exist in the 'add_sub1' version
- #
- test_expect_success "$command: modified submodule updates submodule recursively" '
+ # recursing deeper than one level doesn't work yet.
+ test_expect_failure "$command: modified submodule updates submodule recursively" '
prolog &&
- reset_work_tree_to_interested add_sub1 &&
+ reset_work_tree_to_interested add_nested_sub &&
(
cd submodule_update &&
git branch -t modify_sub1_recursively origin/modify_sub1_recursively &&
- test_must_fail $command modify_sub1_recursively &&
- test_superproject_content origin/add_sub1 &&
- test_submodule_content sub1 origin/add_sub1
- # test_superproject_content origin/modify_sub1_recursively &&
- # test_submodule_content sub1 origin/modify_sub1_recursively &&
- # test_submodule_content sub1/sub2 no_submodule
+ $command modify_sub1_recursively &&
+ test_superproject_content origin/modify_sub1_recursively &&
+ test_submodule_content sub1 origin/modify_sub1_recursively &&
+ test_submodule_content -C sub1 sub2 origin/modify_sub1_recursively
)
'
}
diff --git a/t/t1013-read-tree-submodule.sh b/t/t1013-read-tree-submodule.sh
index 20526aed34..7019d0a04f 100755
--- a/t/t1013-read-tree-submodule.sh
+++ b/t/t1013-read-tree-submodule.sh
@@ -5,6 +5,13 @@ test_description='read-tree can handle submodules'
. ./test-lib.sh
. "$TEST_DIRECTORY"/lib-submodule-update.sh
+KNOWN_FAILURE_DIRECTORY_SUBMODULE_CONFLICTS=1
+KNOWN_FAILURE_SUBMODULE_OVERWRITE_IGNORED_UNTRACKED=1
+
+test_submodule_switch_recursing "git read-tree --recurse-submodules -u -m"
+
+test_submodule_forced_switch_recursing "git read-tree --recurse-submodules -u --reset"
+
test_submodule_switch "git read-tree -u -m"
test_submodule_forced_switch "git read-tree -u --reset"
^ permalink raw reply related
* [RFC PATCH] rev-parse: add --show-superproject-working-tree
From: Stefan Beller @ 2017-03-07 3:45 UTC (permalink / raw)
To: szeder.dev, email, git, sandals, ville.skytta; +Cc: Stefan Beller
In-Reply-To: <CAGZ79kZv=LoP+nG0GVtU_Zi+_SX8_98AXFqzXNh=xM6ASy+=Gw@mail.gmail.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
Marking this as RFC as documentation and tests are missing.
builtin/rev-parse.c | 7 +++++
submodule.c | 87 +++++++++++++++++++++++++++++++++++++++++++++++++++++
submodule.h | 8 +++++
3 files changed, 102 insertions(+)
diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index e08677e559..2549643267 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -12,6 +12,7 @@
#include "diff.h"
#include "revision.h"
#include "split-index.h"
+#include "submodule.h"
#define DO_REVS 1
#define DO_NOREV 2
@@ -779,6 +780,12 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
puts(work_tree);
continue;
}
+ if (!strcmp(arg, "--show-superproject-working-tree")) {
+ const char *superproject = get_superproject_working_tree();
+ if (superproject)
+ puts(superproject);
+ continue;
+ }
if (!strcmp(arg, "--show-prefix")) {
if (prefix)
puts(prefix);
diff --git a/submodule.c b/submodule.c
index 3b98766a6b..a63aef2c6b 100644
--- a/submodule.c
+++ b/submodule.c
@@ -1514,3 +1514,90 @@ void absorb_git_dir_into_superproject(const char *prefix,
strbuf_release(&sb);
}
}
+
+static int superproject_exists(void)
+{
+ struct child_process cp = CHILD_PROCESS_INIT;
+ struct strbuf sb = STRBUF_INIT;
+ struct strbuf buf = STRBUF_INIT;
+ const char *one_up = real_path_if_valid("../");
+ const char *dirname;
+ int code, has_superproject = 0;
+
+ if (!one_up)
+ /* At the root of the file system. */
+ return 0;
+
+ dirname = relative_path(xgetcwd(), one_up, &sb);
+ prepare_submodule_repo_env(&cp.env_array);
+ argv_array_pop(&cp.env_array);
+ argv_array_pushl(&cp.args, "--literal-pathspecs", "-C", "..",
+ "ls-tree", "HEAD", "--", dirname, NULL);
+
+ cp.no_stdin = 1;
+ cp.no_stderr = 1;
+ cp.out = -1;
+ cp.git_cmd = 1;
+
+ if (start_command(&cp))
+ die(_("could not start ls-tree in .."));
+
+ strbuf_read(&buf, cp.out, 7);
+ close(cp.out);
+ if (starts_with(buf.buf, "160000"))
+ /* there is a superproject having this as a submodule */
+ has_superproject = 1;
+
+ code = finish_command(&cp);
+
+ if (code == 128)
+ /* not a git repository */
+ goto out;
+ if (code == 0 && !has_superproject)
+ /* there is an unrelated git repository */
+ goto out;
+
+ if (code)
+ die(_("ls-tree returned unexpected return code"));
+
+ return 1;
+
+out:
+ strbuf_release(&sb);
+
+ return 0;
+}
+
+const char *get_superproject_working_tree()
+{
+ struct child_process cp = CHILD_PROCESS_INIT;
+ struct strbuf sb = STRBUF_INIT;
+
+ if (!superproject_exists())
+ return NULL;
+
+ prepare_submodule_repo_env(&cp.env_array);
+ argv_array_pop(&cp.env_array);
+
+ argv_array_pushl(&cp.args, "-C", "..",
+ "rev-parse", "--show-toplevel", NULL);
+
+ cp.no_stdin = 1;
+ cp.no_stderr = 1;
+ cp.out = -1;
+ cp.git_cmd = 1;
+
+ if (start_command(&cp))
+ die(_("could not start rev-parse in .."));
+
+ strbuf_reset(&sb);
+ strbuf_read(&sb, cp.out, PATH_MAX);
+
+ /* remove trailing new line */
+ strbuf_rtrim(&sb);
+
+ if (finish_command(&cp))
+ die(_("rev-parse died unexpectedly"));
+
+ return strbuf_detach(&sb, NULL);
+}
diff --git a/submodule.h b/submodule.h
index 05ab674f06..f207bb8d5f 100644
--- a/submodule.h
+++ b/submodule.h
@@ -93,4 +93,12 @@ extern void prepare_submodule_repo_env(struct argv_array *out);
extern void absorb_git_dir_into_superproject(const char *prefix,
const char *path,
unsigned flags);
+
+/*
+ * Return the absolute path of the working tree of the superproject, which this
+ * project is a submodule of. If this repository is not a submodule of
+ * another repository, return NULL.
+ */
+extern const char *get_superproject_working_tree();
+
#endif
--
2.12.0.189.g3bc53220cb.dirty
^ permalink raw reply related
* Re: [RFC PATCH] rev-parse: add --show-superproject-working-tree
From: Junio C Hamano @ 2017-03-07 5:13 UTC (permalink / raw)
To: Stefan Beller; +Cc: szeder.dev, email, git, sandals, ville.skytta
In-Reply-To: <20170307034553.10770-1-sbeller@google.com>
Stefan Beller <sbeller@google.com> writes:
> diff --git a/submodule.c b/submodule.c
> index 3b98766a6b..a63aef2c6b 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -1514,3 +1514,90 @@ void absorb_git_dir_into_superproject(const char *prefix,
> strbuf_release(&sb);
> }
> }
Please have a comment here what this function expects (e.g. it is
called after we ran repository discovery and $cwd is at the root of
the working tree of the current project, we _know_ we have a working
tree, etc.).
> +
> +static int superproject_exists(void)
> +{
> + struct child_process cp = CHILD_PROCESS_INIT;
> + struct strbuf sb = STRBUF_INIT;
> + struct strbuf buf = STRBUF_INIT;
> + const char *one_up = real_path_if_valid("../");
> + const char *dirname;
> + int code, has_superproject = 0;
> +
> + if (!one_up)
> + /* At the root of the file system. */
> + return 0;
Hmph. I would have expected that "/../" would be the same as "/".
> + dirname = relative_path(xgetcwd(), one_up, &sb);
So, the idea is we start at the root level of the current project's
working tree, and we go up one level, then we know the last component
of the path our submodule is bound at in the superproject.
> + prepare_submodule_repo_env(&cp.env_array);
> + argv_array_pop(&cp.env_array);
> + argv_array_pushl(&cp.args, "--literal-pathspecs", "-C", "..",
> + "ls-tree", "HEAD", "--", dirname, NULL);
This would ask our superproject what is at the "dirname" in its
HEAD. Two possible issues:
- Shouldn't that be looking at its index instead? It would be more
correct for unborn branch case, and new or moved submodule case.
- If our submodule is bound at path sub/dir in the superproject,
the relative-path thing above would get "dir" and this ls-tree
ends up asking what is at "dir", but the question you really want
to ask is what is at "sub/dir", isn't it?
> +const char *get_superproject_working_tree()
const char *get_superproject_working_tree(void)
^ permalink raw reply
* Re: [RFC PATCH] rev-parse: add --show-superproject-working-tree
From: Junio C Hamano @ 2017-03-07 7:16 UTC (permalink / raw)
To: Stefan Beller; +Cc: szeder.dev, email, git, sandals, ville.skytta
In-Reply-To: <xmqqvarly8hn.fsf@junio-linux.mtv.corp.google.com>
Junio C Hamano <gitster@pobox.com> writes:
>> + dirname = relative_path(xgetcwd(), one_up, &sb);
>
> So, the idea is we start at the root level of the current project's
> working tree, and we go up one level, then we know the last component
> of the path our submodule is bound at in the superproject.
>
>> + prepare_submodule_repo_env(&cp.env_array);
>> + argv_array_pop(&cp.env_array);
>> + argv_array_pushl(&cp.args, "--literal-pathspecs", "-C", "..",
>> + "ls-tree", "HEAD", "--", dirname, NULL);
>
> This would ask our superproject what is at the "dirname" in its
> HEAD. Two possible issues:
>
> - Shouldn't that be looking at its index instead? It would be more
> correct for unborn branch case, and new or moved submodule case.
>
> - If our submodule is bound at path sub/dir in the superproject,
> the relative-path thing above would get "dir" and this ls-tree
> ends up asking what is at "dir", but the question you really want
> to ask is what is at "sub/dir", isn't it?
IOW, the basic ontline of the idea may be OK, but I think you would
want to do something along this line:
- chdir to .. from the root of your submodule working tree;
- in that .. directory, ask what prefix it is (you'd get
"sub/dir", or "not a git" if you are not a submodule);
- in that .. directory, ask ls-files what sub/dir is;
- if it is 160000, you're happy.
^ permalink raw reply
* Re: [RFC PATCH] rev-parse: add --show-superproject-working-tree
From: Junio C Hamano @ 2017-03-07 7:23 UTC (permalink / raw)
To: Stefan Beller; +Cc: szeder.dev, email, git, sandals, ville.skytta
In-Reply-To: <xmqqr329y2sm.fsf@junio-linux.mtv.corp.google.com>
Junio C Hamano <gitster@pobox.com> writes:
>> - If our submodule is bound at path sub/dir in the superproject,
>> the relative-path thing above would get "dir" and this ls-tree
>> ends up asking what is at "dir", but the question you really want
>> to ask is what is at "sub/dir", isn't it?
>
> IOW, the basic ontline of the idea may be OK, but I think you would
> want to do something along this line:
>
> - chdir to .. from the root of your submodule working tree;
> - in that .. directory, ask what prefix it is (you'd get
> "sub/dir", or "not a git" if you are not a submodule);
> - in that .. directory, ask ls-files what sub/dir is;
> - if it is 160000, you're happy.
Nah, that wouldn't be necessary and would not work. --prefix would
be "sub" in that case, and you'd need to concatenate the "dir" that
is the basename of the path to the submodule to get "sub/dir".
Besides, output from "ls-files" by default is relative to cwd, so if
you did ls-files or ls-tree HEAD in "sub/", you'll find where you
came from as "dir", not "sub/dir", so the original code happens to
work even from a subdirectory of a superproject, but the reason why
it works is a bit subtle. Perhaps it deserves in-code comment to
explain it.
^ permalink raw reply
* Re: RFC: Another proposed hash function transition plan
From: Jeff King @ 2017-03-07 8:59 UTC (permalink / raw)
To: Jonathan Tan; +Cc: Jonathan Nieder, git, sbeller, bmwill, Linus Torvalds
In-Reply-To: <cdd7779a-acdb-99fd-a685-89b36df65393@google.com>
On Mon, Mar 06, 2017 at 10:39:49AM -0800, Jonathan Tan wrote:
> The "nohash" thing was in the hope of requiring only one signature to sign
> all the hashes (in all the functions) that the user wants, while preserving
> round-tripping ability.
Thanks, this explained it very well.
I understand the tradeoff now, though I am still of the opinion that
simplicity is probably a more important goal.
In practice I'd imagine that anybody doing commit-signing would just
sign the more-secure hash, and people doing tag releases would probably
do a dual-sign to be verifiable by both old and new clients. Those are
infrequent enough that the extra computation probably doesn't matter.
But that's just my gut feeling.
-Peff
^ permalink raw reply
* Re: Delta compression not so effective
From: Thomas Braun @ 2017-03-07 9:07 UTC (permalink / raw)
To: Marius Storm-Olsen, Linus Torvalds; +Cc: Git Mailing List
In-Reply-To: <9961a973-0d5d-5ff9-ab78-eea07bdb5dbf@gmail.com>
> Marius Storm-Olsen <mstormo@gmail.com> hat am 4. März 2017 um 09:27
> geschrieben:
[...]
> I really don't want the files on the mailinglist, so I'll send you a
> link directly. However, small snippets for public discussions about
> potential issues would be fine, obviously.
git fast-export can anonymize a repository [1]. Maybe an anonymized repository
still shows the issue you are seeing.
[1]: https://www.git-scm.com/docs/git-fast-export#_anonymizing
^ permalink raw reply
* Re: [Request for Documentation] Differentiate signed (commits/tags/pushes)
From: Jeff King @ 2017-03-07 9:23 UTC (permalink / raw)
To: Stefan Beller; +Cc: tom, Matthieu Moy, git@vger.kernel.org, Junio C Hamano
In-Reply-To: <CAGZ79kYxD9B_+3vBgO+Z-wh2GMg_REazA-xpTSAqe3_64VMV3w@mail.gmail.com>
On Mon, Mar 06, 2017 at 11:59:24AM -0800, Stefan Beller wrote:
> What is the difference between signed commits and tags?
> (Not from a technical perspective, but for the end user)
I think git has never really tried to assign any _meaning_ to the
signatures. It implements the technical bits and leaves it up to the
user and their workflows to decide what a given signature means.
People generally seem to take tag signatures to mean one (or both) of:
1. Certifying that the tree contents at that tag are the official,
signed release contents (i.e., this is version 1.0, and I'm the
maintainer, so believe me).
2. Certifying that all the history leading up to that tag is
"official" in some sense (i.e., I'm the maintainer, and this was
the tip of my git tree at the time I tagged the release).
Signing individual commits _could_ convey meaning (2), but "all history
leading up" part is unlikely to be something that the signer needs to
enforce on every commit.
In my opinion, the most useful meaning for commit-signing is simply to
cryptographically certify the identity of the committer. We don't
compare the GPG key ident to the "committer" field, but IMHO that would
be a reasonable optional feature for verify-commit (I say optional
because we're starting to assign semantics now).
I think one of the reasons kernel (and git) developers aren't that
interested in signed commits is that they're not really that interesting
in a patch workflow. You're verifying the committer, who in this project
is invariably Junio, and we just take his word that whatever is in the
"author" field is reasonable.
But for a project whose workflow is based around pushing and pulling
commits, I think it does make sense. The author field may not always
match the committer (e.g., in a cherry-pick), but it still lets you
trace that attestation of the author back to the committer. And it's up
to UI to make that distinction clear (e.g., if you push a signed
cherry-pick to GitHub, the commit is labeled with something like "A U
Thor committed with C O Mitter", and then you get a little "Verified"
tag for C O Mitter that gives you more information about the signature).
So I don't think it's just a checkbox feature. It's a useful thing for
certain workflows that really want to be able to attribute individual
commits with cryptographic strength.
-Peff
^ permalink raw reply
* Reg : GSoC 2017 Microproject
From: Vedant Bassi @ 2017-03-07 10:22 UTC (permalink / raw)
To: git
Hi,
I would like to participate in GSoC 2017 and I have chosen the Use
unsigned integral type for collection of bits , idea from the Micro
projects list.
I request the help of the community for clarifying a few questions that I have.
1. Is this Microproject already taken ?
2. If it is free , I would like to point out one place where a signed
int is used .
In bisect.h , the structure struct rev_list_info uses flags of
type signed int but , the value of MSB is not checked as a test case
for any error checking. Hence it can be of type unsigned int.
It is only used in rev-list.c for checking cases (BISECT_SHOW_ALL and
REV_LIST_QUIET ).
Is this a valid case.
Thanks.
^ permalink raw reply
* Re: [Request for Documentation] Differentiate signed (commits/tags/pushes)
From: Matthieu Moy @ 2017-03-07 7:16 UTC (permalink / raw)
To: Stefan Beller; +Cc: tom, git@vger.kernel.org, Junio C Hamano
In-Reply-To: <CAGZ79kYxD9B_+3vBgO+Z-wh2GMg_REazA-xpTSAqe3_64VMV3w@mail.gmail.com>
Stefan Beller <sbeller@google.com> writes:
> What is the difference between signed commits and tags?
> (Not from a technical perspective, but for the end user)
In addition to what the others already said:
If you use GitHub, then in the web interface you get a "Verified" stamp
for each signed commits:
https://help.github.com/articles/signing-commits-using-gpg/
It's not a Git feature but a GitHub one, but given the popularity of
GitHub, this probably led some users to believe that signed commits are
more convenient than signed tags.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ 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