* Re: [PATCH v7 00/17] port branch.c to use ref-filter's printing options
From: Junio C Hamano @ 2016-11-15 20:43 UTC (permalink / raw)
To: Karthik Nayak; +Cc: git, jacob.keller
In-Reply-To: <20161108201211.25213-1-Karthik.188@gmail.com>
Karthik Nayak <karthik.188@gmail.com> writes:
> This is part of unification of the commands 'git tag -l, git branch -l
> and git for-each-ref'. This ports over branch.c to use ref-filter's
> printing options.
>
> Karthik Nayak (17):
> ref-filter: implement %(if), %(then), and %(else) atoms
> ref-filter: include reference to 'used_atom' within 'atom_value'
> ref-filter: implement %(if:equals=<string>) and
> %(if:notequals=<string>)
> ref-filter: modify "%(objectname:short)" to take length
> ref-filter: move get_head_description() from branch.c
> ref-filter: introduce format_ref_array_item()
> ref-filter: make %(upstream:track) prints "[gone]" for invalid
> upstreams
> ref-filter: add support for %(upstream:track,nobracket)
> ref-filter: make "%(symref)" atom work with the ':short' modifier
> ref-filter: introduce refname_atom_parser_internal()
> ref-filter: introduce symref_atom_parser() and refname_atom_parser()
> ref-filter: make remote_ref_atom_parser() use
> refname_atom_parser_internal()
> ref-filter: add `:dir` and `:base` options for ref printing atoms
> ref-filter: allow porcelain to translate messages in the output
> branch, tag: use porcelain output
> branch: use ref-filter printing APIs
> branch: implement '--format' option
This is not a new issue, but --format='%(HEAD)' you stole from
for-each-ref is broken when you are on an unborn branch, and the
second patch from the tip makes "git branch" (no other args) on
an unborn branch to segfault, when there are real branches that
have commits.
Something like this needs to go before that step.
ref-filter.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/ref-filter.c b/ref-filter.c
index 944671af5a..c71d7360d2 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1318,7 +1318,7 @@ static void populate_value(struct ref_array_item *ref)
head = resolve_ref_unsafe("HEAD", RESOLVE_REF_READING,
sha1, NULL);
- if (!strcmp(ref->refname, head))
+ if (head && !strcmp(ref->refname, head))
v->s = "*";
else
v->s = " ";
^ permalink raw reply related
* [PATCH] worktree: fix a sparse 'Using plain integer as NULL pointer' warning
From: Ramsay Jones @ 2016-11-15 20:28 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy; +Cc: Junio C Hamano, GIT Mailing-list
Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
---
Hi Duy,
If you need to re-roll your 'nd/worktree-move' branch, could you
please squash this into the relevant patch [commit c49e92f5c
("worktree move: refuse to move worktrees with submodules", 12-11-2016)].
Also, one of the new tests introduced by commit 31a8f3066 ("worktree move:
new command", 12-11-2016), fails for me, thus:
$ ./t2028-worktree-move.sh -i -v
...
--- expected 2016-11-15 20:22:50.647241458 +0000
+++ actual 2016-11-15 20:22:50.647241458 +0000
@@ -1,3 +1,3 @@
worktree /home/ramsay/git/t/trash directory.t2028-worktree-move
-worktree /home/ramsay/git/t/trash directory.t2028-worktree-move/destination
worktree /home/ramsay/git/t/trash directory.t2028-worktree-move/elsewhere
+worktree /home/ramsay/git/t/trash directory.t2028-worktree-move/destination
not ok 12 - move worktree
#
# git worktree move source destination &&
# test_path_is_missing source &&
# git worktree list --porcelain | grep "^worktree" >actual &&
# cat <<-EOF >expected &&
# worktree $TRASH_DIRECTORY
# worktree $TRASH_DIRECTORY/destination
# worktree $TRASH_DIRECTORY/elsewhere
# EOF
# test_cmp expected actual &&
# git -C destination log --format=%s >actual2 &&
# echo init >expected2 &&
# test_cmp expected2 actual2
#
$
Is there an expectation that the submodules will be listed in
any particular order by 'git worktree list --porcelain' ?
Thanks!
ATB,
Ramsay Jones
builtin/worktree.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/builtin/worktree.c b/builtin/worktree.c
index e738142..abdf462 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -526,7 +526,7 @@ static int unlock_worktree(int ac, const char **av, const char *prefix)
static void validate_no_submodules(const struct worktree *wt)
{
- struct index_state istate = {0};
+ struct index_state istate = { NULL };
int i, found_submodules = 0;
if (read_index_from(&istate, worktree_git_path(wt, "index")) > 0) {
--
2.10.0
^ permalink raw reply related
* Re: Protecting old temporary objects being reused from concurrent "git gc"?
From: Junio C Hamano @ 2016-11-15 20:01 UTC (permalink / raw)
To: Jeff King; +Cc: Matt McCutchen, git
In-Reply-To: <20161115174028.zvohfcw4jse3jrmm@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> I suspect the issue is that read-tree populates the cache-tree index
> extension, and then write-tree omits the object write before it even
> gets to write_sha1_file(). The solution is that it should probably be
> calling one of the freshen() functions (possibly just replacing
> has_sha1_file() with check_and_freshen(), but I haven't looked).
I think the final writing always happens via write_sha1_file(), but
an earlier cache-tree update that says "if we have a tree object
already, then use it, otherwise even though we know the object name
for this subtree, do not record it in the cache-tree" codepath may
decide to record the subtree's sha1 without refreshing the referent.
A fix may look like this.
cache-tree.c | 2 +-
cache.h | 1 +
sha1_file.c | 9 +++++++--
3 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/cache-tree.c b/cache-tree.c
index 345ea35963..3ae6d056b4 100644
--- a/cache-tree.c
+++ b/cache-tree.c
@@ -401,7 +401,7 @@ static int update_one(struct cache_tree *it,
if (repair) {
unsigned char sha1[20];
hash_sha1_file(buffer.buf, buffer.len, tree_type, sha1);
- if (has_sha1_file(sha1))
+ if (freshen_object(sha1))
hashcpy(it->sha1, sha1);
else
to_invalidate = 1;
diff --git a/cache.h b/cache.h
index 5cdea6833e..1f5694f308 100644
--- a/cache.h
+++ b/cache.h
@@ -1126,6 +1126,7 @@ extern int sha1_object_info(const unsigned char *, unsigned long *);
extern int hash_sha1_file(const void *buf, unsigned long len, const char *type, unsigned char *sha1);
extern int write_sha1_file(const void *buf, unsigned long len, const char *type, unsigned char *return_sha1);
extern int hash_sha1_file_literally(const void *buf, unsigned long len, const char *type, unsigned char *sha1, unsigned flags);
+extern int freshen_object(const unsigned char *);
extern int pretend_sha1_file(void *, unsigned long, enum object_type, unsigned char *);
extern int force_object_loose(const unsigned char *sha1, time_t mtime);
extern int git_open_cloexec(const char *name, int flags);
diff --git a/sha1_file.c b/sha1_file.c
index e030805497..1daeb05dcd 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -3275,6 +3275,11 @@ static int freshen_packed_object(const unsigned char *sha1)
return 1;
}
+int freshen_object(const unsigned char *sha1)
+{
+ return freshen_packed_object(sha1) || freshen_loose_object(sha1);
+}
+
int write_sha1_file(const void *buf, unsigned long len, const char *type, unsigned char *sha1)
{
char hdr[32];
@@ -3284,7 +3289,7 @@ int write_sha1_file(const void *buf, unsigned long len, const char *type, unsign
* it out into .git/objects/??/?{38} file.
*/
write_sha1_file_prepare(buf, len, type, sha1, hdr, &hdrlen);
- if (freshen_packed_object(sha1) || freshen_loose_object(sha1))
+ if (freshen_object(sha1))
return 0;
return write_loose_object(sha1, hdr, hdrlen, buf, len, 0);
}
@@ -3302,7 +3307,7 @@ int hash_sha1_file_literally(const void *buf, unsigned long len, const char *typ
if (!(flags & HASH_WRITE_OBJECT))
goto cleanup;
- if (freshen_packed_object(sha1) || freshen_loose_object(sha1))
+ if (freshen_object(sha1))
goto cleanup;
status = write_loose_object(sha1, header, hdrlen, buf, len, 0);
^ permalink raw reply related
* Re: [PATCH 1/2] push: --dry-run updates submodules when --recurse-submodules=on-demand
From: Johannes Sixt @ 2016-11-15 19:46 UTC (permalink / raw)
To: Brandon Williams; +Cc: git
In-Reply-To: <20161115172909.GA66382@google.com>
Am 15.11.2016 um 18:29 schrieb Brandon Williams:
> I'm assuming the reason we want to avoid sub-shells is
> for performance reasons right?
Yes, every fork() saved is a win on Windows. (No pun intended ;)
-- Hannes
^ permalink raw reply
* Re: ignore blank line removals
From: John Rood @ 2016-11-15 19:32 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqr36ctw47.fsf@gitster.mtv.corp.google.com>
Yes that makes sense.
I was not aware of custom merge drivers, but indeed that may address
my situation. I'll look into it.
Thanks!
On Tue, Nov 15, 2016 at 12:51 PM, Junio C Hamano <gitster@pobox.com> wrote:
> John Rood <mr.john.rood@gmail.com> writes:
>
>> On Thu, Nov 3, 2016 at 10:57 AM, John Rood <mr.john.rood@gmail.com> wrote:
>>> If the contents of a file initially are:
>>> one
>>>
>>> three
>>> and on branch A there is a commit, removing the blank line:
>>> one
>>> three
>>> and on branch B there is a commit, adding 'two':
>>> one
>>> two
>>> three
>>> Normally, if you try to merge A into B (or B into A), git recognizes a
>>> decision needs to be made between removing the second line or add
>>> "two" to the second line. It would be convenient to have a merge
>>> strategy that defaults to the latter in cases where the removed line
>>> was a blank line (or a line containing only whitespace) ...something
>>> like -Xignore-blank-line-removals.
>>
>> Is there any push-back on this, or is there a backlog that we can add
>> this feature to?
>
> If you mean by "push-back" objections that say "this feature is evil
> and should not be added to Git, ever", I do not think we saw any on
> the list. The lack of response is most likely that everybody
> thought "Meh." aka "It is not useful/interesting/valuable enough
> feature to bother discussing."
>
> One thing I wondered was what you would want if the contents were
> one/three without blank, A added blank between the two and B
> replaced blank with two. As your example shows, in the filetype you
> are dealing with, a blank line has no significant meaning (otherwise
> you won't be ignoring the change A made to remove the blank in your
> original example). The outcome desired by you may be one/two/three
> without any blank in that case because of that. Which would lead to
> the suspicion that ignore-blank-line-removals is not a good general
> feature (i.e. in this other example, you want to ignore blank line
> addition). Which further leads to the suspicion that the desire you
> expressed in the original post is not well thought through to be a
> useful specifification to build anything out of (yet), but is merely
> a potentially interesting discussion starter. And nobody so far
> found it interesting enough to spend time discussing it further to
> make the wish detailed enough to be called a "design" of a new
> feature.
>
> Having said all that.
>
> I suspect that you may not have to make any change to Git to do what
> you want; can't you just use the 'merge' attribute with a custom
> 3-way merge driver that removes an empty line?
>
^ permalink raw reply
* Re: Protecting old temporary objects being reused from concurrent "git gc"?
From: Matt McCutchen @ 2016-11-15 19:12 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20161115174028.zvohfcw4jse3jrmm@sigill.intra.peff.net>
On Tue, 2016-11-15 at 12:40 -0500, Jeff King wrote:
> On Tue, Nov 15, 2016 at 12:33:04PM -0500, Matt McCutchen wrote:
>
> >
> > On Tue, 2016-11-15 at 12:06 -0500, Jeff King wrote:
> > >
> > > - when an object write is optimized out because we already have the
> > > object, git will update the mtime on the file (loose object or
> > > packfile) to freshen it
> >
> > FWIW, I am not seeing this happen when I do "git read-tree --prefix"
> > followed by "git write-tree" using the current master (3ab2281). See
> > the attached test script.
>
> The optimization I'm thinking about is the one from write_sha1_file(),
> which learned to freshen in 33d4221c7 (write_sha1_file: freshen existing
> objects, 2014-10-15).
>
> I suspect the issue is that read-tree populates the cache-tree index
> extension, and then write-tree omits the object write before it even
> gets to write_sha1_file(). The solution is that it should probably be
> calling one of the freshen() functions (possibly just replacing
> has_sha1_file() with check_and_freshen(), but I haven't looked).
>
> I'd definitely welcome patches in this area.
Cool, it's nice to have an idea of what's going on. I don't think I'm
going to try to fix it myself though.
By the way, thanks for the fast response to my original question!
Matt
^ permalink raw reply
* [PATCH] git-gc.txt: expand discussion of races with other processes
From: Matt McCutchen @ 2016-11-15 19:08 UTC (permalink / raw)
To: git; +Cc: Jeff King
In-Reply-To: <20161115174028.zvohfcw4jse3jrmm@sigill.intra.peff.net>
In general, "git gc" may delete objects that another concurrent process
is using but hasn't created a reference to. Git has some mitigations,
but they fall short of a complete solution. Document this in the
git-gc(1) man page and add a reference from the documentation of the
gc.pruneExpire config variable.
Based on a write-up by Jeff King:
http://marc.info/?l=git&m=147922960131779&w=2
Signed-off-by: Matt McCutchen <matt@mattmccutchen.net>
---
Documentation/config.txt | 4 +++-
Documentation/git-gc.txt | 34 ++++++++++++++++++++++++++--------
2 files changed, 29 insertions(+), 9 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 21fdddf..3f1d931 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1409,7 +1409,9 @@ gc.pruneExpire::
Override the grace period with this config variable. The value
"now" may be used to disable this grace period and always prune
unreachable objects immediately, or "never" may be used to
- suppress pruning.
+ suppress pruning. This feature helps prevent corruption when
+ 'git gc' runs concurrently with another process writing to the
+ repository; see the "NOTES" section of linkgit:git-gc[1].
gc.worktreePruneExpire::
When 'git gc' is run, it calls
diff --git a/Documentation/git-gc.txt b/Documentation/git-gc.txt
index bed60f4..852b72c 100644
--- a/Documentation/git-gc.txt
+++ b/Documentation/git-gc.txt
@@ -63,11 +63,10 @@ automatic consolidation of packs.
--prune=<date>::
Prune loose objects older than date (default is 2 weeks ago,
overridable by the config variable `gc.pruneExpire`).
- --prune=all prunes loose objects regardless of their age (do
- not use --prune=all unless you know exactly what you are doing.
- Unless the repository is quiescent, you will lose newly created
- objects that haven't been anchored with the refs and end up
- corrupting your repository). --prune is on by default.
+ --prune=all prunes loose objects regardless of their age and
+ increases the risk of corruption if another process is writing to
+ the repository concurrently; see "NOTES" below. --prune is on by
+ default.
--no-prune::
Do not prune any loose objects.
@@ -138,17 +137,36 @@ default is "2 weeks ago".
Notes
-----
-'git gc' tries very hard to be safe about the garbage it collects. In
+'git gc' tries very hard not to delete objects that are referenced
+anywhere in your repository. In
particular, it will keep not only objects referenced by your current set
of branches and tags, but also objects referenced by the index,
remote-tracking branches, refs saved by 'git filter-branch' in
refs/original/, or reflogs (which may reference commits in branches
that were later amended or rewound).
-
-If you are expecting some objects to be collected and they aren't, check
+If you are expecting some objects to be deleted and they aren't, check
all of those locations and decide whether it makes sense in your case to
remove those references.
+On the other hand, when 'git gc' runs concurrently with another process,
+there is a risk of it deleting an object that the other process is using
+but hasn't created a reference to. This may just cause the other process
+to fail or may corrupt the repository if the other process later adds a
+reference to the deleted object. Git has two features that significantly
+mitigate this problem:
+
+. Any object with modification time newer than the `--prune` date is kept,
+ along with everything reachable from it.
+
+. Most operations that add an object to the database update the
+ modification time of the object if it is already present so that #1
+ applies.
+
+However, these features fall short of a complete solution, so users who
+run commands concurrently have to live with some risk of corruption (which
+seems to be low in practice) unless they turn off automatic garbage
+collection with 'git config gc.auto 0'.
+
HOOKS
-----
--
2.7.4
^ permalink raw reply related
* Re: ignore blank line removals
From: Junio C Hamano @ 2016-11-15 18:51 UTC (permalink / raw)
To: John Rood; +Cc: git
In-Reply-To: <CALj-rGdRDa0EU6AeMHWYketr5QBOnV7-3RKraiuPCk=Aepz=pQ@mail.gmail.com>
John Rood <mr.john.rood@gmail.com> writes:
> On Thu, Nov 3, 2016 at 10:57 AM, John Rood <mr.john.rood@gmail.com> wrote:
>> If the contents of a file initially are:
>> one
>>
>> three
>> and on branch A there is a commit, removing the blank line:
>> one
>> three
>> and on branch B there is a commit, adding 'two':
>> one
>> two
>> three
>> Normally, if you try to merge A into B (or B into A), git recognizes a
>> decision needs to be made between removing the second line or add
>> "two" to the second line. It would be convenient to have a merge
>> strategy that defaults to the latter in cases where the removed line
>> was a blank line (or a line containing only whitespace) ...something
>> like -Xignore-blank-line-removals.
>
> Is there any push-back on this, or is there a backlog that we can add
> this feature to?
If you mean by "push-back" objections that say "this feature is evil
and should not be added to Git, ever", I do not think we saw any on
the list. The lack of response is most likely that everybody
thought "Meh." aka "It is not useful/interesting/valuable enough
feature to bother discussing."
One thing I wondered was what you would want if the contents were
one/three without blank, A added blank between the two and B
replaced blank with two. As your example shows, in the filetype you
are dealing with, a blank line has no significant meaning (otherwise
you won't be ignoring the change A made to remove the blank in your
original example). The outcome desired by you may be one/two/three
without any blank in that case because of that. Which would lead to
the suspicion that ignore-blank-line-removals is not a good general
feature (i.e. in this other example, you want to ignore blank line
addition). Which further leads to the suspicion that the desire you
expressed in the original post is not well thought through to be a
useful specifification to build anything out of (yet), but is merely
a potentially interesting discussion starter. And nobody so far
found it interesting enough to spend time discussing it further to
make the wish detailed enough to be called a "design" of a new
feature.
Having said all that.
I suspect that you may not have to make any change to Git to do what
you want; can't you just use the 'merge' attribute with a custom
3-way merge driver that removes an empty line?
^ permalink raw reply
* Re: RFC: Enable delayed responses to Git clean/smudge filter requests
From: Eric Wong @ 2016-11-15 18:27 UTC (permalink / raw)
To: Lars Schneider; +Cc: git
In-Reply-To: <5BC69AC1-5499-4F73-816A-D8786106D796@gmail.com>
Lars Schneider <larsxschneider@gmail.com> wrote:
> > On 15 Nov 2016, at 02:03, Eric Wong <e@80x24.org> wrote:
>
> > Anyways, I'll plan on doing something similar (in Perl) with the
> > synchronous parts of public-inbox which relies on "cat-file --batch"
> > at some point... (my rotational disks are sloooooooow :<)
>
> That sounds interesting! What changes to you have in mind for
> "cat-file --batch"? We are thinking about performance improvements
> in that area, too. I would be happy to help reviewing your patches!
I'm not touching "cat-file --batch" itself, just the Perl code
that uses it. (still trying my best to avoid working on C or
any compiled languages);
There'll probably be a Perl queue mechanism so there's still
only one cat-file process running per-inbox to avoid overloading
on seeks; but the Perl daemons should be able to handle network
duties and other in-memory operations while it waits for
cat-file.
^ permalink raw reply
* Re: gitweb html validation
From: Ralf Thielow @ 2016-11-15 18:26 UTC (permalink / raw)
To: Raphaël Gertz; +Cc: git
In-Reply-To: <13c9b4a74d82a1f0ed3f626406a43e92@rapsys.eu>
Raphaël Gertz <mageia@rapsys.eu> wrote:
> Hi,
>
> There a small bug in gitweb html validation, you need the following patch to
> pass w3c check with searchbox enabled.
>
> The problem lies in the input directly embed inside a form without a wrapper
> which is not valid.
>
I agree this is a small bug. Only block level elements are
allowed to be inside form tags, according to
https://www.w3.org/2010/04/xhtml10-strict.html#elem_form
> Best regards
>
> The following patch fix the issue for git-2.10.2 :
> --- /usr/share/gitweb/gitweb.cgi.orig 2016-11-15 15:37:21.149805026 +0100
> +++ /usr/share/gitweb/gitweb.cgi 2016-11-15 15:37:48.579240429 +0100
> @@ -5518,6 +5518,7 @@ sub git_project_search_form {
>
> print "<div class=\"projsearch\">\n";
> print $cgi->start_form(-method => 'get', -action => $my_uri) .
> + '<div>'.
> $cgi->hidden(-name => 'a', -value => 'project_list') . "\n";
> print $cgi->hidden(-name => 'pf', -value => $project_filter). "\n"
> if (defined $project_filter);
> @@ -5529,6 +5530,7 @@ sub git_project_search_form {
> -checked => $search_use_regexp) .
> "</span>\n" .
> $cgi->submit(-name => 'btnS', -value => 'Search') .
> + '</div>'.
> $cgi->end_form() . "\n" .
> $cgi->a({-href => href(project => undef, searchtext => undef,
> project_filter => $project_filter)},
I think it's better to just move the <form>-Tag outside of the
surrounding div?
Something like this perhaps, I didn't test it myself yet.
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 7cf68f07b..33d7c154f 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -5531,8 +5531,8 @@ sub git_project_search_form {
$limit = " in '$project_filter/'";
}
- print "<div class=\"projsearch\">\n";
print $cgi->start_form(-method => 'get', -action => $my_uri) .
+ "<div class=\"projsearch\">\n" .
$cgi->hidden(-name => 'a', -value => 'project_list') . "\n";
print $cgi->hidden(-name => 'pf', -value => $project_filter). "\n"
if (defined $project_filter);
@@ -5544,11 +5544,11 @@ sub git_project_search_form {
-checked => $search_use_regexp) .
"</span>\n" .
$cgi->submit(-name => 'btnS', -value => 'Search') .
- $cgi->end_form() . "\n" .
$cgi->a({-href => href(project => undef, searchtext => undef,
project_filter => $project_filter)},
esc_html("List all projects$limit")) . "<br />\n";
- print "</div>\n";
+ print "</div>\n" .
+ $cgi->end_form() . "\n";
}
# entry for given @keys needs filling if at least one of keys in list
diff --git a/gitweb/static/gitweb.css b/gitweb/static/gitweb.css
index 321260103..507740b6a 100644
--- a/gitweb/static/gitweb.css
+++ b/gitweb/static/gitweb.css
@@ -539,7 +539,7 @@ div.projsearch {
margin: 20px 0px;
}
-div.projsearch form {
+form div.projsearch {
margin-bottom: 2px;
}
^ permalink raw reply related
* Re: RFC: Enable delayed responses to Git clean/smudge filter requests
From: Junio C Hamano @ 2016-11-15 18:03 UTC (permalink / raw)
To: Lars Schneider; +Cc: Eric Wong, git
In-Reply-To: <5BC69AC1-5499-4F73-816A-D8786106D796@gmail.com>
Lars Schneider <larsxschneider@gmail.com> writes:
>> The filter itself would need to be aware of parallelism
>> if it lives for multiple objects, right?
>
> Correct. This way Git doesn't need to deal with threading...
I think you need to be careful about three things (at least; there
may be more):
* Codepaths that check out multiple cache entries do rely on the
order of checkout. We checkout removals first to make room so
that creation of a path X can succeed if an existing path X/Y
that used to want to see X as a directory can succeed (see the
use of checkout_entry() by "git checkout", which does have two
separate loops to explicitly guarantee this), for example. I
think "remove all and then create" you do not specifically have
to worry about with the proposed change, but you may need to
inspect and verify there aren't other kind of order dependency.
* Done naively, it will lead to unmaintainable code, like this:
+ struct list_of_cache_entries *list = ...;
for (i = 0; i < active_nr; i++)
- checkout_entry(active_cache[i], state, NULL);
+ if (checkout_entry(active_cache[i], state, NULL) == DELAYED)
+ add_cache_to_queue(&list, active_cache[i]);
+ while (list) {
+ wait_for_checkout_to_finish(*list);
+ list = list->next;
+ }
I do not think we want to see such a rewrite all over the
codepaths. It might be OK to add such a "these entries are known
to be delayed" list in struct checkout so that the above becomes
more like this:
for (i = 0; i < active_nr; i++)
checkout_entry(active_cache[i], state, NULL);
+ checkout_entry_finish(state);
That is, addition of a single "some of the checkout_entry() calls
done so far might have been lazy, and I'll give them a chance to
clean up" might be palatable. Anything more than that on the
caller side is not.
* You'd need to rein in the maximum parallelism somehow, as you do
not want to see hundreds of competing filter processes starting
only to tell the main loop over an index with hundreds of entries
that they are delayed checkouts.
^ permalink raw reply
* Re: ignore blank line removals
From: John Rood @ 2016-11-15 17:56 UTC (permalink / raw)
To: git
In-Reply-To: <CALj-rGcSrbAiyxPOYD1KCacK_gqi3T5_R=e0H_UfBTqLb5VRmg@mail.gmail.com>
Is there any push-back on this, or is there a backlog that we can add
this feature to?
On Thu, Nov 3, 2016 at 10:57 AM, John Rood <mr.john.rood@gmail.com> wrote:
> If the contents of a file initially are:
> one
>
> three
> and on branch A there is a commit, removing the blank line:
> one
> three
> and on branch B there is a commit, adding 'two':
> one
> two
> three
> Normally, if you try to merge A into B (or B into A), git recognizes a
> decision needs to be made between removing the second line or add
> "two" to the second line. It would be convenient to have a merge
> strategy that defaults to the latter in cases where the removed line
> was a blank line (or a line containing only whitespace) ...something
> like -Xignore-blank-line-removals.
^ permalink raw reply
* Re: [PATCH v3 0/4] Speedup finding of unpushed submodules
From: Brandon Williams @ 2016-11-15 17:52 UTC (permalink / raw)
To: Stefan Beller
Cc: Heiko Voigt, Junio C Hamano, git@vger.kernel.org, Jeff King,
Jens Lehmann, Fredrik Gustafsson, Leandro Lucarella
In-Reply-To: <CAGZ79kawQOO98GFYZBnUhN_A8jKDVopTDHMjsM=rrG_5ekyidQ@mail.gmail.com>
On 11/15, Stefan Beller wrote:
> On Tue, Nov 15, 2016 at 6:56 AM, Heiko Voigt <hvoigt@hvoigt.net> wrote:
> > You can find the second iteration of this series here:
> >
> > http://public-inbox.org/git/cover.1475851621.git.hvoigt@hvoigt.net/
> >
> > All mentioned issues should be fixed. I put the NEEDSWORK comment in a
> > seperate patch since it seemed to me as if we did not fully agree on
> > that. So in case we decide against it we can just drop that patch.
> >
> > Cheers Heiko
> >
>
> +cc Brandon who started building a series on top of yours.
I don't think there should be too much I'll have to change with my
series, I'll just rebase against these changes once Junio updates his
branch.
If you want to take a look at my series its here:
https://public-inbox.org/git/1479172735-698-1-git-send-email-bmwill@google.com/
Thanks for the heads up Stefan.
--
Brandon Williams
^ permalink raw reply
* Re: [PATCH v3 0/4] Speedup finding of unpushed submodules
From: Stefan Beller @ 2016-11-15 17:43 UTC (permalink / raw)
To: Heiko Voigt, Brandon Williams
Cc: Junio C Hamano, git@vger.kernel.org, Jeff King, Jens Lehmann,
Fredrik Gustafsson, Leandro Lucarella
In-Reply-To: <cover.1479221071.git.hvoigt@hvoigt.net>
On Tue, Nov 15, 2016 at 6:56 AM, Heiko Voigt <hvoigt@hvoigt.net> wrote:
> You can find the second iteration of this series here:
>
> http://public-inbox.org/git/cover.1475851621.git.hvoigt@hvoigt.net/
>
> All mentioned issues should be fixed. I put the NEEDSWORK comment in a
> seperate patch since it seemed to me as if we did not fully agree on
> that. So in case we decide against it we can just drop that patch.
>
> Cheers Heiko
>
+cc Brandon who started building a series on top of yours.
^ permalink raw reply
* Re: [PATCH] remote-curl: don't hang when a server dies before any output
From: Junio C Hamano @ 2016-11-15 17:42 UTC (permalink / raw)
To: Jeff King; +Cc: David Turner, git, spearce
In-Reply-To: <20161115035844.e6ehuy7uigqinbnv@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Mon, Nov 14, 2016 at 05:02:27PM -0800, Junio C Hamano wrote:
>
>> Jeff King <peff@peff.net> writes:
>>
>> > Actually, I take it back. I think it works for a single round of ref
>> > negotiation, but not for multiple. Enabling GIT_TEST_LONG=1 causes it to
>> > fail t5551.
>> >
>> > I think I've probably made a mis-assumption on exactly when in the HTTP
>> > protocol we will see a flush packet (and perhaps that is a sign that
>> > this protocol-snooping approach is not a good one).
>>
>> Hmph. I think I tried David's original under GIT_TEST_LONG and saw
>> it got stuck; could be the same issue, I guess.
>
> It works OK here. I think it is just that the test is really slow (by
> design).
Yeah, I think what I recalled was my old attempt to run the
follow-up "any SHA-1" patch without this one.
^ permalink raw reply
* Re: [PATCH v3 0/6] recursively grep across submodules
From: Stefan Beller @ 2016-11-15 17:42 UTC (permalink / raw)
To: Brandon Williams; +Cc: git@vger.kernel.org, Jonathan Tan, Junio C Hamano
In-Reply-To: <1478908273-190166-1-git-send-email-bmwill@google.com>
coverity seems to dislike this part:
*** CID 1394367: Null pointer dereferences (NULL_RETURNS)
/builtin/grep.c: 625 in grep_submodule()
619 is_submodule_populated(path))) {
620 /*
621 * If searching history, check for the presense of the
622 * submodule's gitdir before skipping the submodule.
623 */
624 if (sha1) {
>>> CID 1394367: Null pointer dereferences (NULL_RETURNS)
>>> Dereferencing a null pointer "submodule_from_path(null_sha1, path)".
625 path = git_path("modules/%s",
626
submodule_from_path(null_sha1, path)->name);
627
628 if (!(is_directory(path) &&
is_git_directory(path)))
629 return 0;
630 } else {
^ permalink raw reply
* Re: [PATCH v7 13/17] ref-filter: add `:dir` and `:base` options for ref printing atoms
From: Junio C Hamano @ 2016-11-15 17:42 UTC (permalink / raw)
To: Jacob Keller; +Cc: Karthik Nayak, Git mailing list
In-Reply-To: <CA+P7+xo6OqcpLZ7v_m1EPm85eK2xCPD_LCw1Ly2RSPeSC0Ei7g@mail.gmail.com>
Jacob Keller <jacob.keller@gmail.com> writes:
> dirname makes sense. What about implementing a reverse variant of
> strip, which you could perform stripping of right-most components and
> instead of stripping by a number, strip "to" a number, ie: keep the
> left N most components, and then you could use something like
> ...
> I think that would be more general purpose than basename, and less confusing?
I think you are going in the right direction. I had a similar
thought but built around a different axis. I.e. if strip=1 strips
one from the left, perhaps we want to have rstrip=1 that strips one
from the right, and also strip=-1 to mean strip everything except
one from the left and so on?. I think this and your keep (and
perhaps you'll have rkeep for completeness) have the same expressive
power. I do not offhand have a preference one over the other.
Somehow it sounds a bit strange to me to treat 'remotes' as the same
class of token as 'heads' and 'tags' (I'd expect 'heads' and
'remotes/origin' would be at the same level in end-user's mind), but
that is probably an unrelated tangent. The reason this series wants
to introduce :base must be to emulate an existing feature, so that
existing feature is a concrete counter-example that argues against
my "it sounds a bit strange" reaction.
^ permalink raw reply
* Re: Protecting old temporary objects being reused from concurrent "git gc"?
From: Jeff King @ 2016-11-15 17:40 UTC (permalink / raw)
To: Matt McCutchen; +Cc: git
In-Reply-To: <1479231184.2406.88.camel@mattmccutchen.net>
On Tue, Nov 15, 2016 at 12:33:04PM -0500, Matt McCutchen wrote:
> On Tue, 2016-11-15 at 12:06 -0500, Jeff King wrote:
> > - when an object write is optimized out because we already have the
> > object, git will update the mtime on the file (loose object or
> > packfile) to freshen it
>
> FWIW, I am not seeing this happen when I do "git read-tree --prefix"
> followed by "git write-tree" using the current master (3ab2281). See
> the attached test script.
The optimization I'm thinking about is the one from write_sha1_file(),
which learned to freshen in 33d4221c7 (write_sha1_file: freshen existing
objects, 2014-10-15).
I suspect the issue is that read-tree populates the cache-tree index
extension, and then write-tree omits the object write before it even
gets to write_sha1_file(). The solution is that it should probably be
calling one of the freshen() functions (possibly just replacing
has_sha1_file() with check_and_freshen(), but I haven't looked).
I'd definitely welcome patches in this area.
> OK. I'll write a patch to add a summary of this information to the
> git-gc man page.
Sounds like a good idea. Thanks.
-Peff
^ permalink raw reply
* Re: Protecting old temporary objects being reused from concurrent "git gc"?
From: Matt McCutchen @ 2016-11-15 17:33 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20161115170634.ichqrqbhmpv2dsiw@sigill.intra.peff.net>
[-- Attachment #1: Type: text/plain, Size: 1349 bytes --]
On Tue, 2016-11-15 at 12:06 -0500, Jeff King wrote:
> - when an object write is optimized out because we already have the
> object, git will update the mtime on the file (loose object or
> packfile) to freshen it
FWIW, I am not seeing this happen when I do "git read-tree --prefix"
followed by "git write-tree" using the current master (3ab2281). See
the attached test script.
> If you have long-running data (like, a temporary index file that might
> literally sit around for days or weeks) I think that is a potential
> problem. And the solution is probably to use refs in some way to point
> to your objects.
Agreed. This is not my current scenario.
> If you're worried about a short-term operation where
> somebody happens to run git-gc concurrently, I agree it's a possible
> problem, but I suspect something you can ignore in practice.
>
> For the most part, a lot of the client-side git tools assume that one
> operation is happening at a time in the repository. And I think that
> largely holds for a developer working on a single clone, and things just
> work in practice.
>
> Auto-gc makes that a little sketchier, but historically does not seem to
> have really caused problems in practice.
OK. I'll write a patch to add a summary of this information to the
git-gc man page.
Matt
[-- Attachment #2: test-git-read-tree-write-tree-touch-object.sh --]
[-- Type: application/x-shellscript, Size: 502 bytes --]
^ permalink raw reply
* Re: [PATCH 1/2] push: --dry-run updates submodules when --recurse-submodules=on-demand
From: Brandon Williams @ 2016-11-15 17:29 UTC (permalink / raw)
To: Johannes Sixt; +Cc: git
In-Reply-To: <4a72ad14-0a8f-ede9-9f54-601fcd37740b@kdbg.org>
On 11/15, Johannes Sixt wrote:
> Am 15.11.2016 um 02:18 schrieb Brandon Williams:
> >diff --git a/t/t5531-deep-submodule-push.sh b/t/t5531-deep-submodule-push.sh
> >index 198ce84..e6ccc30 100755
> >--- a/t/t5531-deep-submodule-push.sh
> >+++ b/t/t5531-deep-submodule-push.sh
> >@@ -427,7 +427,31 @@ test_expect_success 'push unpushable submodule recursively fails' '
> > cd submodule.git &&
> > git rev-parse master >../actual
> > ) &&
> >- test_cmp expected actual
> >+ test_cmp expected actual &&
> >+ git -C work reset --hard master^
>
> This line looks like a clean-up to be done after the test case. You
> should wrap it in test_when_finished, but outside of a sub-shell,
> which looks like it's just one line earlier, before the test_cmp.
K will do.
>
> >+'
> >+
> >+test_expect_failure 'push --dry-run does not recursively update submodules' '
> >+ (
> >+ cd work &&
> >+ (
> >+ cd gar/bage &&
> >+ git checkout master &&
> >+ git rev-parse master >../../../expected_submodule &&
> >+ > junk9 &&
> >+ git add junk9 &&
> >+ git commit -m "Ninth junk"
> >+ ) &&
>
> Could you please avoid this nested sub-shell? It is fine to cd
> around when you are in a sub-shell.
Yes I can reorganize it to avoid the nested sub-shells. I was just
trying to follow the organization of the other tests in the same file.
>
> >+ git checkout master &&
> >+ git rev-parse master >../expected_pub
>
> Broken && chain.
>
> >+ git add gar/bage &&
> >+ git commit -m "Ninth commit for gar/bage" &&
> >+ git push --dry-run --recurse-submodules=on-demand ../pub.git master
> >+ ) &&
> >+ git -C submodule.git rev-parse master >actual_submodule &&
> >+ git -C pub.git rev-parse master >actual_pub &&
>
> All of the commands above are 'git something' that could become 'git
> -C work something' and then the sub-shell would be unnecessary. I'm
> not sure I would appreciate the verbosity of the result, though.
> (Perhaps aligning the git subcommands after -C foo would help.)
I'll play around with it and try to make it look pretty while trying to
avoid sub-shells. I'm assuming the reason we want to avoid sub-shells is
for performance reasons right?
--
Brandon Williams
^ permalink raw reply
* Re: Protecting old temporary objects being reused from concurrent "git gc"?
From: Jeff King @ 2016-11-15 17:06 UTC (permalink / raw)
To: Matt McCutchen; +Cc: git
In-Reply-To: <1479219194.2406.73.camel@mattmccutchen.net>
On Tue, Nov 15, 2016 at 09:13:14AM -0500, Matt McCutchen wrote:
> I want to change this to something that won't leave an inconsistent
> state if interrupted. I've written code for this kind of thing before
> that sets GIT_INDEX_FILE and uses a temporary index file and "git
> write-tree". But I realized that if "git gc" runs concurrently, the
> generated tree could be deleted before it is used and the tool would
> fail. If I had a need to run "git commit-tree", it seems like I might
> even end up with a commit object with a broken reference to a tree.
> "git gc" normally doesn't delete objects that were created in the last
> 2 weeks, but if an identical tree was added to the object database more
> than 2 weeks ago by another operation and is unreferenced, it could be
> reused without updating its mtime and it could still get deleted.
Modern versions of git do two things to help with this:
- any object which is referenced by a "recent" object (within the 2
weeks) is also considered recent. So if you create a new commit
object that points to a tree, even before you reference the commit
that tree is protected
- when an object write is optimized out because we already have the
object, git will update the mtime on the file (loose object or
packfile) to freshen it
This isn't perfect, though. You can decide to reference an existing
object just as it is being deleted. And the pruning process itself is
not atomic (and it's tricky to make it so, just because of what we're
promised by the filesystem).
> Is there a recommended way to avoid this kind of problem in add-on
> tools? (I searched the Git documentation and the web for information
> about races with "git gc" and didn't find anything useful.) If not, it
> seems to be a significant design flaw in "git gc", even if the problem
> is extremely rare in practice. I wonder if some of the built-in
> commands may have the same problem, though I haven't tried to test
> them. If this is confirmed to be a known problem affecting built-in
> commands, then at least I won't feel bad about introducing the
> same problem into add-on tools. :/
If you have long-running data (like, a temporary index file that might
literally sit around for days or weeks) I think that is a potential
problem. And the solution is probably to use refs in some way to point
to your objects. If you're worried about a short-term operation where
somebody happens to run git-gc concurrently, I agree it's a possible
problem, but I suspect something you can ignore in practice.
For the most part, a lot of the client-side git tools assume that one
operation is happening at a time in the repository. And I think that
largely holds for a developer working on a single clone, and things just
work in practice.
Auto-gc makes that a little sketchier, but historically does not seem to
have really caused problems in practice.
For a busy multi-user server, I recommend turning off auto-gc entirely,
and repacking manually with "-k" to be on the safe side.
-Peff
^ permalink raw reply
* Re: [PATCH] t0021, t5615: use $PWD instead of $(pwd) in PATH-like shell variables
From: Johannes Schindelin @ 2016-11-15 16:54 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Lars Schneider, Johannes Sixt, git, Jeff King
In-Reply-To: <xmqqshqux9il.fsf@gitster.mtv.corp.google.com>
Hi Junio,
On Mon, 14 Nov 2016, Junio C Hamano wrote:
> Dscho's mention of 'still times out' may be an indiciation that
> something unspecified on 'pu' is not ready to be merged to 'next',
> but blocking all of 'pu' with a blanket statement is not useful,
> and that was where my response comes from.
Until the time when the test suite takes less than the insane three hours
to run, I am afraid that a blanket statement on `pu` is the best I can do.
Ciao,
Johannes
^ permalink raw reply
* gitweb html validation
From: Raphaël Gertz @ 2016-11-15 15:13 UTC (permalink / raw)
To: git
Hi,
There a small bug in gitweb html validation, you need the following
patch to pass w3c check with searchbox enabled.
The problem lies in the input directly embed inside a form without a
wrapper which is not valid.
Best regards
The following patch fix the issue for git-2.10.2 :
--- /usr/share/gitweb/gitweb.cgi.orig 2016-11-15 15:37:21.149805026
+0100
+++ /usr/share/gitweb/gitweb.cgi 2016-11-15 15:37:48.579240429
+0100
@@ -5518,6 +5518,7 @@ sub git_project_search_form {
print "<div class=\"projsearch\">\n";
print $cgi->start_form(-method => 'get', -action => $my_uri) .
+ '<div>'.
$cgi->hidden(-name => 'a', -value => 'project_list') .
"\n";
print $cgi->hidden(-name => 'pf', -value => $project_filter).
"\n"
if (defined $project_filter);
@@ -5529,6 +5530,7 @@ sub git_project_search_form {
-checked => $search_use_regexp) .
"</span>\n" .
$cgi->submit(-name => 'btnS', -value => 'Search') .
+ '</div>'.
$cgi->end_form() . "\n" .
$cgi->a({-href => href(project => undef, searchtext =>
undef,
project_filter =>
$project_filter)},
^ permalink raw reply
* RE: [PATCH] remote-curl: don't hang when a server dies before any output
From: David Turner @ 2016-11-15 15:45 UTC (permalink / raw)
To: 'Jeff King'; +Cc: git@vger.kernel.org, spearce@spearce.org
In-Reply-To: <20161114234847.2nexsgedpg7zvrr5@sigill.intra.peff.net>
> -----Original Message-----
> From: Jeff King [mailto:peff@peff.net]
...
> I'll make that change and then try to wrap this up with a commit message.
> I plan to steal your tests, if that's OK.
Please do!
^ permalink raw reply
* Re: New to git, need help!
From: Pranit Bauva @ 2016-11-15 15:43 UTC (permalink / raw)
To: Mayank Gupta; +Cc: Git List
In-Reply-To: <CAJNRPQRhyrALj0zdaTxKgwo8j8r8_7ixgX21+C=ue+CGKYgaCg@mail.gmail.com>
Hey Mayank,
On Tue, Nov 15, 2016 at 6:00 PM, Mayank Gupta
<mayankgupta18198@gmail.com> wrote:
> Hi All,
>
> I'm new to open source and have recently joined this mailing list.
> Since I'm new at this, I think I can initially contribute to the
> community by fixing some small bugs or errors but as the documentation
> is too large, I don't know where to start.
> So if anybody could guide me on how to go about it, it would be really
> appreciated.
It is really nice that you want to start contributing. We have a user
documentation[1] and a developer documentation[2]. To contribute to a
feature, firstly you need to learn about that feature. So go through
the man pages of the tool which you would want to work on. The things
that you would be using are documented in the technical documentation.
Apart from this, you can start hunting small errors but I think it
would be difficult in the initial stages. So read this[3] even though
it is specific for GSoC, it is helpful for any new developer. Also you
can search the mailing list archives for "low hanging fruit" to get
things which were thought of but not done or something like that.
[1]: https://github.com/git/git/tree/master/Documentation
[2]: https://github.com/git/git/tree/master/Documentation/technical
[3]: https://git.github.io/SoC-2016-Microprojects/
Hope to see a patch from your side soon! :)
Regards,
Pranit Bauva
^ 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