[PATCH v4 2/2] diff: don't read index when --no-index is given
git diff --no-index ... currently reads the index, during setup, when calling gitmodules_config(). This results in worse performance when the index is not actually needed. This patch avoids calling gitmodules_config() when the --no-index option is given. The times for executing "git diff --no-index" in the WebKit repository are improved as follows: Test HEAD~3HEAD -- 4001.1: diff --no-index 0.24(0.15+0.09) 0.01(0.00+0.00) -95.8% An additional improvement of this patch is that "git diff --no-index" no longer breaks when the index file is corrupt, which makes it possible to use it for investigating the broken repository. To improve the possible usage as investigation tool for broken repositories, setup_git_directory_gently() is also not called when the --no-index option is given. Also add a test to guard against future breakages, and a performance test to show the improvements. Signed-off-by: Thomas Gummerer --- builtin/diff.c| 7 +-- t/perf/p4001-diff-no-index.sh | 22 ++ t/t4053-diff-no-index.sh | 15 +++ 3 files changed, 42 insertions(+), 2 deletions(-) create mode 100755 t/perf/p4001-diff-no-index.sh diff --git a/builtin/diff.c b/builtin/diff.c index da69e4a..ea1dd65 100644 --- a/builtin/diff.c +++ b/builtin/diff.c @@ -298,7 +298,9 @@ int cmd_diff(int argc, const char **argv, const char *prefix) break; } - prefix = setup_git_directory_gently(&nongit); + if (!no_index) + prefix = setup_git_directory_gently(&nongit); + /* * Treat git diff with at least one path outside of the * repo the same as if the command would have been executed @@ -311,7 +313,8 @@ int cmd_diff(int argc, const char **argv, const char *prefix) !path_inside_repo(prefix, argv[i + 1] no_index = DIFF_NO_INDEX_IMPLICIT; - gitmodules_config(); + if (!no_index) + gitmodules_config(); git_config(git_diff_ui_config, NULL); init_revisions(&rev, prefix); diff --git a/t/perf/p4001-diff-no-index.sh b/t/perf/p4001-diff-no-index.sh new file mode 100755 index 000..683be69 --- /dev/null +++ b/t/perf/p4001-diff-no-index.sh @@ -0,0 +1,22 @@ +#!/bin/sh + +test_description="Test diff --no-index performance" + +. ./perf-lib.sh + +test_perf_large_repo +test_checkout_worktree + +file1=$(git ls-files | tail -n 2 | head -1) +file2=$(git ls-files | tail -n 1 | head -1) + +test_expect_success "empty files, so they take no time to diff" " + echo >$file1 && + echo >$file2 +" + +test_perf "diff --no-index" " + git diff --no-index $file1 $file2 >/dev/null +" + +test_done diff --git a/t/t4053-diff-no-index.sh b/t/t4053-diff-no-index.sh index 979e983..077c775 100755 --- a/t/t4053-diff-no-index.sh +++ b/t/t4053-diff-no-index.sh @@ -29,4 +29,19 @@ test_expect_success 'git diff --no-index relative path outside repo' ' ) ' +test_expect_success 'git diff --no-index with broken index' ' + ( + cd repo && + echo broken >.git/index && + git diff --no-index a ../non/git/a + ) +' + +test_expect_success 'git diff outside repo with broken index' ' + ( + cd repo && + git diff ../non/git/a ../non/git/b + ) +' + test_done -- 1.8.5.4.g8639e57 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 1/2] diff: move no-index detection to builtin/diff.c
Currently the --no-index option is parsed in diff_no_index(). Move the
detection if a no-index diff should be executed to builtin/diff.c, where
we can use it for executing diff_no_index() conditionally. This will
also allow us to execute other operations conditionally, which will be
done in the next patch.
There are no functional changes.
Helped-by: Jonathan Nieder
Signed-off-by: Thomas Gummerer
---
Thanks to Jonathan for the suggestions in the previous round.
Since the last roud I've made the no_index detection easier to read,
and use two different values for no_index to indicate whether the
no_index option is given explicitly, or if a no_index diff should be
executed implicitly.
builtin/diff.c | 52 +---
diff-no-index.c | 44 +---
diff.h | 2 +-
3 files changed, 51 insertions(+), 47 deletions(-)
diff --git a/builtin/diff.c b/builtin/diff.c
index adb93a9..da69e4a 100644
--- a/builtin/diff.c
+++ b/builtin/diff.c
@@ -16,6 +16,9 @@
#include "submodule.h"
#include "sha1-array.h"
+#define DIFF_NO_INDEX_EXPLICIT 1
+#define DIFF_NO_INDEX_IMPLICIT 2
+
struct blobinfo {
unsigned char sha1[20];
const char *name;
@@ -257,7 +260,7 @@ int cmd_diff(int argc, const char **argv, const char
*prefix)
int blobs = 0, paths = 0;
const char *path = NULL;
struct blobinfo blob[2];
- int nongit;
+ int nongit = 0, no_index = 0;
int result = 0;
/*
@@ -283,14 +286,57 @@ int cmd_diff(int argc, const char **argv, const char
*prefix)
* Other cases are errors.
*/
+ /* Were we asked to do --no-index explicitly? */
+ for (i = 1; i < argc; i++) {
+ if (!strcmp(argv[i], "--")) {
+ i++;
+ break;
+ }
+ if (!strcmp(argv[i], "--no-index"))
+ no_index = DIFF_NO_INDEX_EXPLICIT;
+ if (argv[i][0] != '-')
+ break;
+ }
+
prefix = setup_git_directory_gently(&nongit);
+ /*
+* Treat git diff with at least one path outside of the
+* repo the same as if the command would have been executed
+* outside of a git repository. In this case it behaves
+* the same way as "git diff --no-index ", which acts
+* as a colourful "diff" replacement.
+*/
+ if (nongit || ((argc == i + 2) &&
+ (!path_inside_repo(prefix, argv[i]) ||
+ !path_inside_repo(prefix, argv[i + 1]
+ no_index = DIFF_NO_INDEX_IMPLICIT;
+
gitmodules_config();
git_config(git_diff_ui_config, NULL);
init_revisions(&rev, prefix);
- /* If this is a no-index diff, just run it and exit there. */
- diff_no_index(&rev, argc, argv, nongit, prefix);
+ if (no_index) {
+ if (argc != i + 2) {
+ if (no_index == DIFF_NO_INDEX_IMPLICIT) {
+ /*
+* There was no --no-index and there were not
two
+* paths. It is possible that the user intended
+* to do an inside-repository operation.
+*/
+ fprintf(stderr, "Not a git repository\n");
+ fprintf(stderr,
+ "To compare two paths outside a working
tree:\n");
+ }
+ /* Give the usage message for non-repository usage and
exit. */
+ usagef("git diff %s ",
+ no_index == DIFF_NO_INDEX_EXPLICIT ?
+ "--no-index" : "[--no-index]");
+
+ }
+ /* If this is a no-index diff, just run it and exit there. */
+ diff_no_index(&rev, argc, argv, prefix);
+ }
/* Otherwise, we are doing the usual "git" diff */
rev.diffopt.skip_stat_unmatch = !!diff_auto_refresh_index;
diff --git a/diff-no-index.c b/diff-no-index.c
index 00a8eef..33e5982 100644
--- a/diff-no-index.c
+++ b/diff-no-index.c
@@ -183,54 +183,12 @@ static int queue_diff(struct diff_options *o,
void diff_no_index(struct rev_info *revs,
int argc, const char **argv,
- int nongit, const char *prefix)
+ const char *prefix)
{
int i, prefixlen;
- int no_index = 0;
unsigned deprecated_show_diff_q_option_used = 0;
const char *paths[2];
- /* Were we asked to do --no-index explicitly? */
- for (i = 1; i < argc; i++) {
- if (!strcmp(argv[i], "--")) {
- i++;
- break;
- }
- if (!strcmp(argv[i], "--no-index"))
- no_index = 1;
-
Re: Unexpected cherry-pick behaviour
On 10/12/2013 19:34, Junio C Hamano wrote: Perhaps immediately after "cherry-pick" stopped and asked your help to resolve the conflicts, running $ git checkout --conflicts=diff3 gcc/tree-ssa-threadedge.c and looking at the file again may show you what is going on better. I don't know how to interpret the fact that the line you sent (with the obvious --conflicts being --conflict) outputs nothing... Any suggestions? -- Paulo Matos -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Unexpected cherry-pick behaviour
On Wed, Dec 11, 2013 at 11:04 AM, Paulo Matos wrote: > On 10/12/2013 19:34, Junio C Hamano wrote: >> >> Perhaps immediately after "cherry-pick" stopped and asked your help >> to resolve the conflicts, running >> >> $ git checkout --conflicts=diff3 gcc/tree-ssa-threadedge.c >> >> and looking at the file again may show you what is going on better. > > > I don't know how to interpret the fact that the line you sent (with the > obvious --conflicts being --conflict) outputs nothing... That is expected. git-checkout with this option [1] will reset the conflict on gcc/tree-ssa-threadedge.c file to the initial conflict state, and use the diff3 markers. You should have a new look at that file as you will now be able to see the "ancestor" in the conflict. [1] You can have a look either at git-checkout manpage or here: http://git-scm.com/docs/git-checkout, especially --merge and --conflict options. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Unexpected cherry-pick behaviour
On 11/12/2013 11:09, Antoine Pelisse wrote:
I don't know how to interpret the fact that the line you sent (with
the
obvious --conflicts being --conflict) outputs nothing...
That is expected. git-checkout with this option [1] will reset the
conflict on gcc/tree-ssa-threadedge.c file to the initial conflict
state, and use the diff3 markers. You should have a new look at that
file as you will now be able to see the "ancestor" in the conflict.
[1] You can have a look either at git-checkout manpage or here:
http://git-scm.com/docs/git-checkout, especially --merge and
--conflict options.
--
Got it, but still not helpful as git is still modifying code out of the
conflicting zone.
$ git checkout --conflict=diff3 tree-ssa-threadedge.c
$ git diff tree-ssa-threadedge.c
diff --cc gcc/tree-ssa-threadedge.c
index cb6accf,f022eed..000
--- a/gcc/tree-ssa-threadedge.c
+++ b/gcc/tree-ssa-threadedge.c
@@@ -936,34 -854,33 +936,57 @@@ thread_around_empty_blocks (edge taken_
STACK is used to undo temporary equivalences created during the
walk of
E->dest.
- SIMPLIFY is a pass-specific function used to simplify statements.
*/
-
-void
-thread_across_edge (gimple dummy_cond,
- edge e,
- bool handle_dominating_asserts,
- vec *stack,
- tree (*simplify) (gimple, gimple))
-{
- gimple stmt;
+ SIMPLIFY is a pass-specific function used to simplify statements.
++<<< ours
+ Our caller is responsible for restoring the state of the expression
+ and const_and_copies stacks. */
++||| base
++ /* If E is a backedge, then we want to verify that the COND_EXPR,
++ SWITCH_EXPR or GOTO_EXPR at the end of e->dest is not affected
++ by any statements in e->dest. If it is affected, then it is not
++ safe to thread this edge. */
++ if (e->flags & EDGE_DFS_BACK)
++{
++ if (cond_arg_set_in_bb (e, e->dest))
++ goto fail;
++}
++===
+ /* If E is a backedge, then we want to verify that the COND_EXPR,
+ SWITCH_EXPR or GOTO_EXPR at the end of e->dest is not affected
+ by any statements in e->dest. If it is affected, then it is not
+ safe to thread this edge. */
+ if (e->flags & EDGE_DFS_BACK)
+ {
+ if (cond_arg_set_in_bb (e, e->dest))
+ goto fail;
+ }
+ myport_hook ()
++>>> theirs
- stmt_count = 0;
+static bool
+thread_through_normal_block (edge e,
+ gimple dummy_cond,
+ bool handle_dominating_asserts,
+ vec *stack,
+ tree (*simplify) (gimple, gimple),
+ vec *path,
+ bitmap visited,
+ bool *backedge_seen_p,
+ bitmap src_map,
+ bitmap dst_map)
+{
+ /* If we have traversed a backedge, then we do not want to look
+ at certain expressions in the table that can not be relied upon.
+ Luckily the only code that looked at those expressions is the
+ SIMPLIFY callback, which we replace if we can no longer use it.
*/
+ if (*backedge_seen_p)
+simplify = dummy_simplify;
/* PHIs create temporary equivalences. */
- if (!record_temporary_equivalences_from_phis (e, stack))
-goto fail;
+ if (!record_temporary_equivalences_from_phis (e, stack,
*backedge_seen_p,
+ src_map, dst_map))
+return false;
/* Now walk each statement recording any context sensitive
temporary equivalences we can detect. */
--
Paulo Matos
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 0/4] Show extra branch refs in gitweb
First patch splits some code to a function. Second patch fixes validation functions to return either 0 or 1, instead of undef or passed $input. Third patch adds the extra-branch-feature and some documentation. Fourth patch adds some visual differentation of branches from non-standard ref directories. Krzesimir Nowak (4): gitweb: Move check-ref-format code into separate function gitweb: Return 1 on validation success instead of passed input gitweb: Add a feature for adding more branch refs gitweb: Denote non-heads, non-remotes branches Documentation/gitweb.conf.txt | 37 + gitweb/gitweb.perl| 184 +++--- 2 files changed, 175 insertions(+), 46 deletions(-) -- 1.8.3.1 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 1/4] gitweb: Move check-ref-format code into separate function
This check will be used in more than one place later.
Signed-off-by: Krzesimir Nowak
---
gitweb/gitweb.perl | 17 +
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 68c77f6..46bd6ac 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1452,6 +1452,16 @@ sub validate_pathname {
return $input;
}
+sub is_valid_ref_format {
+ my $input = shift || return undef;
+
+ # restrictions on ref name according to git-check-ref-format
+ if ($input =~ m!(/\.|\.\.|[\000-\040\177 ~^:?*\[]|/$)!) {
+ return undef;
+ }
+ return $input;
+}
+
sub validate_refname {
my $input = shift || return undef;
@@ -1462,10 +1472,9 @@ sub validate_refname {
# it must be correct pathname
$input = validate_pathname($input)
or return undef;
- # restrictions on ref name according to git-check-ref-format
- if ($input =~ m!(/\.|\.\.|[\000-\040\177 ~^:?*\[]|/$)!) {
- return undef;
- }
+ # check git-check-ref-format restrictions
+ is_valid_ref_format($input)
+ or return undef;
return $input;
}
--
1.8.3.1
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 3/4] gitweb: Add a feature for adding more branch refs
Allow extra-branch-refs feature to tell gitweb to show refs from
additional hierarchies in addition to branches in the list-of-branches
view.
Signed-off-by: Krzesimir Nowak
---
Documentation/gitweb.conf.txt | 37
gitweb/gitweb.perl| 80 ---
2 files changed, 105 insertions(+), 12 deletions(-)
diff --git a/Documentation/gitweb.conf.txt b/Documentation/gitweb.conf.txt
index e2113d9..db4154f 100644
--- a/Documentation/gitweb.conf.txt
+++ b/Documentation/gitweb.conf.txt
@@ -849,6 +849,43 @@ time zones in the form of "+/-HHMM", such as "+0200".
+
Project specific override is not supported.
+extra-branch-refs::
+ List of additional directories under "refs" which are going to
+ be used as branch refs. For example if you have a gerrit setup
+ where all branches under refs/heads/ are official,
+ push-after-review ones and branches under refs/sandbox/,
+ refs/wip and refs/other are user ones where permissions are
+ much wider, then you might want to set this variable as
+ follows:
++
+
+$feature{'extra-branch-refs'}{'default'} =
+ ['sandbox', 'wip', 'other'];
+
++
+This feature can be configured on per-repository basis after setting
+$feature{'extra-branch-refs'}{'override'} to true, via repository's
+`gitweb.extraBranchRefs` configuration variable, which contains a
+space separated list of refs. An example:
++
+
+[gitweb]
+ extraBranchRefs = sandbox wip other
+
++
+The gitweb.extraBranchRefs is actually a multi-valued configuration
+variable, so following example is also correct and the result is the
+same as of the snippet above:
++
+
+[gitweb]
+ extraBranchRefs = sandbox
+ extraBranchRefs = wip other
+
++
+It is an error to specify a ref that does not pass "git check-ref-format"
+scrutiny. Duplicated values are filtered.
+
EXAMPLES
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index b5a8a36..222699a 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -548,6 +548,20 @@ our %feature = (
'sub' => sub { feature_bool('remote_heads', @_) },
'override' => 0,
'default' => [0]},
+
+ # Enable showing branches under other refs in addition to heads
+
+ # To set system wide extra branch refs have in $GITWEB_CONFIG
+ # $feature{'extra-branch-refs'}{'default'} = ['dirs', 'of', 'choice'];
+ # To have project specific config enable override in $GITWEB_CONFIG
+ # $feature{'extra-branch-refs'}{'override'} = 1;
+ # and in project config gitweb.extrabranchrefs = dirs of choice
+ # Every directory is separated with whitespace.
+
+ 'extra-branch-refs' => {
+ 'sub' => \&feature_extra_branch_refs,
+ 'override' => 0,
+ 'default' => []},
);
sub gitweb_get_feature {
@@ -626,6 +640,21 @@ sub feature_avatar {
return @val ? @val : @_;
}
+sub feature_extra_branch_refs {
+ my (@branch_refs) = @_;
+ my $values = git_get_project_config('extrabranchrefs');
+
+ if ($values) {
+ $values = config_to_multi ($values);
+ @branch_refs = ();
+ foreach my $value (@{$values}) {
+ push @branch_refs, split /\s+/, $value;
+ }
+ }
+
+ return @branch_refs;
+}
+
# checking HEAD file with -e is fragile if the repository was
# initialized long time ago (i.e. symlink HEAD) and was pack-ref'ed
# and then pruned.
@@ -656,6 +685,18 @@ sub filter_snapshot_fmts {
!$known_snapshot_formats{$_}{'disabled'}} @fmts;
}
+sub filter_and_validate_refs {
+ my @refs = @_;
+ my %unique_refs = ();
+
+ foreach my $ref (@refs) {
+ die_error(500, "Invalid ref '$ref' in 'extra-branch-refs'
feature") unless (is_valid_ref_format($ref));
+ # 'heads' are added implicitly in get_branch_refs().
+ $unique_refs{$ref} = 1 if ($ref ne 'heads');
+ }
+ return sort keys %unique_refs;
+}
+
# If it is set to code reference, it is code that it is to be run once per
# request, allowing updating configurations that change with each request,
# while running other code in config file only once.
@@ -1113,7 +1154,7 @@ sub evaluate_git_dir {
our $git_dir = "$projectroot/$project" if $project;
}
-our (@snapshot_fmts, $git_avatar);
+our (@snapshot_fmts, $git_avatar, @extra_branch_refs);
sub configure_gitweb_
Re: [BUG] "echo HEAD | git cat-file --batch=''" fails catastrophically
On Tue, Dec 10, 2013 at 11:37:14PM -0500, Samuel Bronson wrote: > % echo HEAD | git cat-file --batch= > > fatal: object fde075cb72fc0773d8e8ca93d55a35d77bb6688b changed type!? > > Without the =, it works fine; with a string that has both > "%(objecttype)" and "%(objectsize)", it's fine; but when you don't > include both, it complains about one of the values that you did not > mention having changed. > > jrnieder fingered v1.8.4-rc0~7^2~15 as the (likely?) culprit here. It's not actually that commit itself, but rather that commit in conjunction with further optimizations in that patch series. The rest of the series tries hard to avoid looking up items that we aren't going to print, for --batch-check. But I didn't think about the fact that "--batch" got the same custom-header feature, but was relying on the values from the default header to do its consistency checks. The following patches should fix it. [1/2]: cat-file: pass expand_data to print_object_or_die [2/2]: cat-file: handle --batch format with missing type/size Doing "--batch=" is somewhat pointless. If you do not get the size, you cannot know when the object content ends, so it only makes sense with a single object. At which point using --batch is pointless. Doing "--batch=%(objectsize)" is reasonable, though, and that is broken, too. v1.8.4 has the breakage, though it's not a regression (doing "--batch=anything" did not exist before that). This can probably just go to the regular "maint" track for v1.8.5). -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 4/4] gitweb: Denote non-heads, non-remotes branches
Given two branches residing in refs/heads/master and refs/wip/feature
the list-of-branches view will present them in following way:
master
feature (wip)
When getting a snapshot of a 'feature' branch, the tarball is going to
have name like 'project-wip-feature-.tgz'.
Signed-off-by: Krzesimir Nowak
---
gitweb/gitweb.perl | 34 +-
1 file changed, 29 insertions(+), 5 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 222699a..3bc0f0b 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -3730,8 +3730,14 @@ sub git_get_heads_list {
$ref_item{'fullname'} = $name;
my $strip_refs = join '|', map { quotemeta } get_branch_refs();
$name =~ s!^refs/($strip_refs|remotes)/!!;
+ $ref_item{'name'} = $name;
+ # for refs neither in 'heads' nor 'remotes' we want to
+ # show their ref dir
+ my $ref_dir = (defined $1) ? $1 : '';
+ if ($ref_dir ne '' and $ref_dir ne 'heads' and $ref_dir ne
'remotes') {
+ $ref_item{'name'} .= ' (' . $ref_dir . ')';
+ }
- $ref_item{'name'} = $name;
$ref_item{'id'}= $hash;
$ref_item{'title'} = $title || '(no commit message)';
$ref_item{'epoch'} = $epoch;
@@ -7223,6 +7229,15 @@ sub git_tree {
git_footer_html();
}
+sub sanitize_for_filename {
+my $name = shift;
+
+$name =~ s!/!-!g;
+$name =~ s/[^[:alnum:]_.-]//g;
+
+return $name;
+}
+
sub snapshot_name {
my ($project, $hash) = @_;
@@ -7230,9 +7245,7 @@ sub snapshot_name {
# path/to/project/.git -> project
my $name = to_utf8($project);
$name =~ s,([^/])/*\.git$,$1,;
- $name = basename($name);
- # sanitize name
- $name =~ s/[[:cntrl:]]/?/g;
+ $name = sanitize_for_filename(basename($name));
my $ver = $hash;
if ($hash =~ /^[0-9a-fA-F]+$/) {
@@ -7248,12 +7261,23 @@ sub snapshot_name {
# branches and other need shortened SHA-1 hash
my $strip_refs = join '|', map { quotemeta } get_branch_refs();
if ($hash =~ m!^refs/($strip_refs|remotes)/(.*)$!) {
- $ver = $1;
+ my $ref_dir = (defined $1) ? $1 : '';
+ $ver = $2;
+
+ $ref_dir = sanitize_for_filename($ref_dir);
+ # for refs neither in heads nor remotes we want to
+ # add a ref dir to archive name
+ if ($ref_dir ne '' and $ref_dir ne 'heads' and $ref_dir
ne 'remotes') {
+ $ver = $ref_dir . '-' . $ver;
+ }
}
$ver .= '-' . git_get_short_hash($project, $hash);
}
+ # special case of sanitization for filename - we change
+ # slashes to dots instead of dashes
# in case of hierarchical branch names
$ver =~ s!/!.!g;
+ $ver =~ s/[^[:alnum:]_.-]//g;
# name = project-version_string
$name = "$name-$ver";
--
1.8.3.1
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 2/4] gitweb: Return 1 on validation success instead of passed input
Users of validate_* passing "0" might get failures on correct name
because of coercion of "0" to false in code like:
die_error(500, "invalid ref") unless (check_ref_format ("0"));
Also, the validate_foo subs are renamed to is_valid_foo.
Signed-off-by: Krzesimir Nowak
---
gitweb/gitweb.perl | 61 --
1 file changed, 32 insertions(+), 29 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 46bd6ac..b5a8a36 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -994,7 +994,7 @@ our ($action, $project, $file_name, $file_parent, $hash,
$hash_parent, $hash_bas
sub evaluate_and_validate_params {
our $action = $input_params{'action'};
if (defined $action) {
- if (!validate_action($action)) {
+ if (!is_valid_action($action)) {
die_error(400, "Invalid action parameter");
}
}
@@ -1002,7 +1002,7 @@ sub evaluate_and_validate_params {
# parameters which are pathnames
our $project = $input_params{'project'};
if (defined $project) {
- if (!validate_project($project)) {
+ if (!is_valid_project($project)) {
undef $project;
die_error(404, "No such project");
}
@@ -1010,21 +1010,21 @@ sub evaluate_and_validate_params {
our $project_filter = $input_params{'project_filter'};
if (defined $project_filter) {
- if (!validate_pathname($project_filter)) {
+ if (!is_valid_pathname($project_filter)) {
die_error(404, "Invalid project_filter parameter");
}
}
our $file_name = $input_params{'file_name'};
if (defined $file_name) {
- if (!validate_pathname($file_name)) {
+ if (!is_valid_pathname($file_name)) {
die_error(400, "Invalid file parameter");
}
}
our $file_parent = $input_params{'file_parent'};
if (defined $file_parent) {
- if (!validate_pathname($file_parent)) {
+ if (!is_valid_pathname($file_parent)) {
die_error(400, "Invalid file parent parameter");
}
}
@@ -1032,21 +1032,21 @@ sub evaluate_and_validate_params {
# parameters which are refnames
our $hash = $input_params{'hash'};
if (defined $hash) {
- if (!validate_refname($hash)) {
+ if (!is_valid_refname($hash)) {
die_error(400, "Invalid hash parameter");
}
}
our $hash_parent = $input_params{'hash_parent'};
if (defined $hash_parent) {
- if (!validate_refname($hash_parent)) {
+ if (!is_valid_refname($hash_parent)) {
die_error(400, "Invalid hash parent parameter");
}
}
our $hash_base = $input_params{'hash_base'};
if (defined $hash_base) {
- if (!validate_refname($hash_base)) {
+ if (!is_valid_refname($hash_base)) {
die_error(400, "Invalid hash base parameter");
}
}
@@ -1066,7 +1066,7 @@ sub evaluate_and_validate_params {
our $hash_parent_base = $input_params{'hash_parent_base'};
if (defined $hash_parent_base) {
- if (!validate_refname($hash_parent_base)) {
+ if (!is_valid_refname($hash_parent_base)) {
die_error(400, "Invalid hash parent base parameter");
}
}
@@ -1418,27 +1418,30 @@ sub href {
## ==
## validation, quoting/unquoting and escaping
-sub validate_action {
- my $input = shift || return undef;
+sub is_valid_action {
+ my $input = shift;
return undef unless exists $actions{$input};
- return $input;
+ return 1;
}
-sub validate_project {
- my $input = shift || return undef;
- if (!validate_pathname($input) ||
+sub is_valid_project {
+ my $input = shift;
+
+ return unless defined $input;
+ if (!is_valid_pathname($input) ||
!(-d "$projectroot/$input") ||
!check_export_ok("$projectroot/$input") ||
($strict_export && !project_in_list($input))) {
return undef;
} else {
- return $input;
+ return 1;
}
}
-sub validate_pathname {
- my $input = shift || return undef;
+sub is_valid_pathname {
+ my $input = shift;
+ return undef unless defined $input;
# no '.' or '..' as elements of path, i.e. no '.' nor '..'
# at the beginning, at the end, and between slashes.
# also this catches doubled slashes
@@ -1449,33 +1452,33 @@ sub validate_pathname {
if ($input =~
[PATCH 1/2] cat-file: pass expand_data to print_object_or_die
We currently individually pass the sha1, type, and size
fields calculated by sha1_object_info. However, if we pass
the whole struct, the called function can make more
intelligent decisions about which fields were actualled
filled by sha1_object_info.
As a side effect, we can rename the local variables in the
function to "type" and "size", since the names are no longer
taken.
There should be no functional change to this patch.
Signed-off-by: Jeff King
---
I split this out mostly to keep the noise out of the follow-on diff.
builtin/cat-file.c | 21 +++--
1 file changed, 11 insertions(+), 10 deletions(-)
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index b2ca775..1434afb 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -193,25 +193,26 @@ static size_t expand_format(struct strbuf *sb, const char
*start, void *data)
return end - start + 1;
}
-static void print_object_or_die(int fd, const unsigned char *sha1,
- enum object_type type, unsigned long size)
+static void print_object_or_die(int fd, struct expand_data *data)
{
- if (type == OBJ_BLOB) {
+ const unsigned char *sha1 = data->sha1;
+
+ if (data->type == OBJ_BLOB) {
if (stream_blob_to_fd(fd, sha1, NULL, 0) < 0)
die("unable to stream %s to stdout", sha1_to_hex(sha1));
}
else {
- enum object_type rtype;
- unsigned long rsize;
+ enum object_type type;
+ unsigned long size;
void *contents;
- contents = read_sha1_file(sha1, &rtype, &rsize);
+ contents = read_sha1_file(sha1, &type, &size);
if (!contents)
die("object %s disappeared", sha1_to_hex(sha1));
- if (rtype != type)
+ if (type != data->type)
die("object %s changed type!?", sha1_to_hex(sha1));
- if (rsize != size)
- die("object %s change size!?", sha1_to_hex(sha1));
+ if (size != data->size)
+ die("object %s changed size!?", sha1_to_hex(sha1));
write_or_die(fd, contents, size);
free(contents);
@@ -250,7 +251,7 @@ static int batch_one_object(const char *obj_name, struct
batch_options *opt,
strbuf_release(&buf);
if (opt->print_contents) {
- print_object_or_die(1, data->sha1, data->type, data->size);
+ print_object_or_die(1, data);
write_or_die(1, "\n", 1);
}
return 0;
--
1.8.5.524.g6743da6
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] cat-file: handle --batch format with missing type/size
Commit 98e2092 taught cat-file to stream blobs with --batch,
which requires that we look up the object type before
loading it into memory. As a result, we now print the
object header from information in sha1_object_info, and the
actual contents from the read_sha1_file. We double-check
that the information we printed in the header matches the
content we are about to show.
Later, commit 93d2a60 allowed custom header lines for
--batch, and commit 5b08640 made type lookups optional. As a
result, specifying a header line without the type or size
means that we will not look up those items at all.
This causes our double-checking to erroneously die with an
error; we think the type or size has changed, when in fact
it was simply left at "0".
For the size, we can fix this by only doing the consistency
double-check when we have retrieved the size via
sha1_object_info. In the case that we have not retrieved the
value, that means we also did not print it, so there is
nothing for us to check that we are consistent with.
We could do the same for the type. However, besides our
consistency check, we also care about the type in deciding
whether to stream or not. We therefore make sure to always
trigger a type lookup when we are printing, so that even a
format without the type will stream as we would in the
normal case.
Signed-off-by: Jeff King
---
builtin/cat-file.c | 9 -
t/t1006-cat-file.sh | 22 ++
2 files changed, 30 insertions(+), 1 deletion(-)
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 1434afb..4af67fd 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -211,7 +211,7 @@ static void print_object_or_die(int fd, struct expand_data
*data)
die("object %s disappeared", sha1_to_hex(sha1));
if (type != data->type)
die("object %s changed type!?", sha1_to_hex(sha1));
- if (size != data->size)
+ if (data->info.sizep && size != data->size)
die("object %s changed size!?", sha1_to_hex(sha1));
write_or_die(fd, contents, size);
@@ -276,6 +276,13 @@ static int batch_objects(struct batch_options *opt)
data.mark_query = 0;
/*
+* If we are printing out the object, then always fill in the type,
+* since we will want to decide whether or not to stream.
+*/
+ if (opt->print_contents)
+ data.info.typep = &data.type;
+
+ /*
* We are going to call get_sha1 on a potentially very large number of
* objects. In most large cases, these will be actual object sha1s. The
* cost to double-check that each one is not also a ref (just so we can
diff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh
index 8a1bc5c..1687098 100755
--- a/t/t1006-cat-file.sh
+++ b/t/t1006-cat-file.sh
@@ -85,6 +85,28 @@ $content"
git cat-file --batch-check="%(objecttype) %(rest)" >actual &&
test_cmp expect actual
'
+
+test -z "$content" ||
+test_expect_success "--batch without type ($type)" '
+ {
+ echo "$size" &&
+ maybe_remove_timestamp "$content" $no_ts
+ } >expect &&
+ echo $sha1 | git cat-file --batch="%(objectsize)" >actual.full &&
+ maybe_remove_timestamp "$(cat actual.full)" $no_ts >actual &&
+ test_cmp expect actual
+'
+
+test -z "$content" ||
+test_expect_success "--batch without size ($type)" '
+ {
+ echo "$type" &&
+ maybe_remove_timestamp "$content" $no_ts
+ } >expect &&
+ echo $sha1 | git cat-file --batch="%(objecttype)" >actual.full &&
+ maybe_remove_timestamp "$(cat actual.full)" $no_ts >actual &&
+ test_cmp expect actual
+'
}
hello_content="Hello World"
--
1.8.5.524.g6743da6
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/POC 0/7] Support multiple worktrees
This is what I imagine multi worktree support (aka git-new-workdir) looks like. Basically two variables will control access to the repo: $GIT_SUPER_DIR for worktree specific stuff and $GIT_DIR for the rest. I like the idea of using git_path() to hide path relocation caused by GIT_SUPER_DIR, but I may have made some design mistakes here.. setup_git_directory() changes are hairy. It's not surprise if I broke something in there. Not important for a PoC though. Final series may be a few patches longer as I only lay the foundation in this series. Nguyễn Thái Ngọc Duy (7): Make git_path() beware of file relocation in $GIT_DIR Add new environment variable $GIT_SUPER_DIR setup.c: add split-repo support to .git files setup.c: add split-repo support to is_git_directory() setup.c: reduce cleanup sites in setup_explicit_git_dir() setup.c: add split-repo support to setup_git_directory* init: add --split-repo with the same functionality as git-new-workdir builtin/init-db.c | 42 +++ cache.h | 5 ++ environment.c | 37 -- path.c| 45 ++-- setup.c | 139 ++ t/t0060-path-utils.sh | 115 + test-path-utils.c | 7 +++ trace.c | 1 + 8 files changed, 339 insertions(+), 52 deletions(-) -- 1.8.5.1.77.g42c48fa -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/POC 1/7] Make git_path() beware of file relocation in $GIT_DIR
We allow the user to relocate certain paths out of $GIT_DIR via
environment variables, e.g. GIT_OBJECT_DIRECTORY, GIT_INDEX_FILE and
GIT_GRAFT_FILE. All callers are not supposed to use git_path() or
git_pathdup() to get those paths. Instead they must use
get_object_directory(), get_index_file() and get_graft_file()
respectively. This is inconvenient and could be missed in review
(there's git_path("objects/info/alternates") somewhere in
sha1_file.c).
This patch makes git_path() and git_pathdup() understand those
environment variables. So if you set GIT_OBJECT_DIRECTORY to /foo/bar,
git_path("objects/abc") should return /tmp/bar/abc. The same is done
for the two remaining env variables.
XXX trim consecutive slashes
Signed-off-by: Nguyễn Thái Ngọc Duy
---
cache.h | 1 +
environment.c | 9 +++--
path.c| 35 ---
t/t0060-path-utils.sh | 48
test-path-utils.c | 7 +++
5 files changed, 95 insertions(+), 5 deletions(-)
diff --git a/cache.h b/cache.h
index ce377e1..cdafbd7 100644
--- a/cache.h
+++ b/cache.h
@@ -584,6 +584,7 @@ extern int fsync_object_files;
extern int core_preload_index;
extern int core_apply_sparse_checkout;
extern int precomposed_unicode;
+extern int git_db_env, git_index_env, git_graft_env;
/*
* The character that begins a commented line in user-editable file
diff --git a/environment.c b/environment.c
index 0a15349..1d74dde 100644
--- a/environment.c
+++ b/environment.c
@@ -81,6 +81,7 @@ static size_t namespace_len;
static const char *git_dir;
static char *git_object_dir, *git_index_file, *git_graft_file;
+int git_db_env, git_index_env, git_graft_env;
/*
* Repository-local GIT_* environment variables; see cache.h for details.
@@ -134,15 +135,19 @@ static void setup_git_env(void)
if (!git_object_dir) {
git_object_dir = xmalloc(strlen(git_dir) + 9);
sprintf(git_object_dir, "%s/objects", git_dir);
- }
+ } else
+ git_db_env = 1;
git_index_file = getenv(INDEX_ENVIRONMENT);
if (!git_index_file) {
git_index_file = xmalloc(strlen(git_dir) + 7);
sprintf(git_index_file, "%s/index", git_dir);
- }
+ } else
+ git_index_env = 1;
git_graft_file = getenv(GRAFT_ENVIRONMENT);
if (!git_graft_file)
git_graft_file = git_pathdup("info/grafts");
+ else
+ git_graft_env = 1;
if (getenv(NO_REPLACE_OBJECTS_ENVIRONMENT))
read_replace_refs = 0;
namespace = expand_namespace(getenv(GIT_NAMESPACE_ENVIRONMENT));
diff --git a/path.c b/path.c
index 24594c4..eda9176 100644
--- a/path.c
+++ b/path.c
@@ -49,10 +49,38 @@ char *mksnpath(char *buf, size_t n, const char *fmt, ...)
return cleanup_path(buf);
}
+static int dir_prefix(const char *buf, const char *dir)
+{
+ int len = strlen(dir);
+ return !strncmp(buf, dir, len) &&
+ (is_dir_sep(buf[len]) || buf[len] == '\0');
+}
+
+static void replace_dir(char *buf, int len, const char *newdir)
+{
+ int newlen = strlen(newdir);
+ int buflen = strlen(buf);
+ memmove(buf + newlen + 1, buf + len, buflen - len + 1);
+ memcpy(buf, newdir, newlen);
+ buf[newlen] = '/';
+}
+
+static void adjust_git_path(char *buf, int git_dir_len)
+{
+ /* XXX buffer overflow */
+ char *base = buf + git_dir_len;
+ if (git_graft_env && !strcmp(base, "info/grafts"))
+ strcpy(buf, get_graft_file());
+ else if (git_index_env && !strcmp(base, "index"))
+ strcpy(buf, get_index_file());
+ else if (git_db_env && dir_prefix(base, "objects"))
+ replace_dir(buf, git_dir_len + 7, get_object_directory());
+}
+
static char *vsnpath(char *buf, size_t n, const char *fmt, va_list args)
{
const char *git_dir = get_git_dir();
- size_t len;
+ size_t len, total_len;
len = strlen(git_dir);
if (n < len + 1)
@@ -60,9 +88,10 @@ static char *vsnpath(char *buf, size_t n, const char *fmt,
va_list args)
memcpy(buf, git_dir, len);
if (len && !is_dir_sep(git_dir[len-1]))
buf[len++] = '/';
- len += vsnprintf(buf + len, n - len, fmt, args);
- if (len >= n)
+ total_len = len + vsnprintf(buf + len, n - len, fmt, args);
+ if (total_len >= n)
goto bad;
+ adjust_git_path(buf, len);
return cleanup_path(buf);
bad:
strlcpy(buf, bad_path, n);
diff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh
index 07c10c8..7ad2730 100755
--- a/t/t0060-path-utils.sh
+++ b/t/t0060-path-utils.sh
@@ -223,4 +223,52 @@ relative_path "" "" ./
relative_path "" ""./
relative_path "" /foo/a/b./
+test_expect_success 'git_path info/grafts without GIT_GRAFT_FILE' '
+ test-pat
[PATCH/POC 3/7] setup.c: add split-repo support to .git files
If a .git file contains
gitsuper:
gitdir:
then we set GIT_SUPER_DIR to and GIT_DIR to
$GIT_SUPER_DIR/repos/.
Signed-off-by: Nguyễn Thái Ngọc Duy
---
cache.h | 1 +
setup.c | 40 +---
2 files changed, 34 insertions(+), 7 deletions(-)
diff --git a/cache.h b/cache.h
index 823582f..f85ee70 100644
--- a/cache.h
+++ b/cache.h
@@ -410,6 +410,7 @@ extern const char *get_git_namespace(void);
extern const char *strip_namespace(const char *namespaced_ref);
extern const char *get_git_work_tree(void);
extern const char *read_gitfile(const char *path);
+extern const char *read_gitfile_super(const char *path, char **super);
extern const char *resolve_gitdir(const char *suspect);
extern void set_git_work_tree(const char *tree);
diff --git a/setup.c b/setup.c
index 5432a31..84362a6 100644
--- a/setup.c
+++ b/setup.c
@@ -281,16 +281,23 @@ static int check_repository_format_gently(const char
*gitdir, int *nongit_ok)
/*
* Try to read the location of the git directory from the .git file,
* return path to git directory if found.
+ *
+ * If "gitsuper: " line is found and super is not NULL, *super points
+ * to the absolute path of the given path. The next line contains the
+ * repo id.
*/
-const char *read_gitfile(const char *path)
+const char *read_gitfile_super(const char *path, char **super)
{
- char *buf;
+ struct strbuf sb = STRBUF_INIT;
+ char *buf, *to_free;
char *dir;
const char *slash;
struct stat st;
int fd;
ssize_t len;
+ if (super)
+ *super = NULL;
if (stat(path, &st))
return NULL;
if (!S_ISREG(st.st_mode))
@@ -298,12 +305,19 @@ const char *read_gitfile(const char *path)
fd = open(path, O_RDONLY);
if (fd < 0)
die_errno("Error opening '%s'", path);
- buf = xmalloc(st.st_size + 1);
+ to_free = buf = xmalloc(st.st_size + 1);
len = read_in_full(fd, buf, st.st_size);
close(fd);
if (len != st.st_size)
die("Error reading %s", path);
buf[len] = '\0';
+ if (super &&!prefixcmp(buf, "gitsuper: ")) {
+ char *p = strchr(buf, '\n');
+ *super = buf + strlen("gitsuper: ");
+ *p = '\0';
+ len -= (p + 1) - buf;
+ buf = p + 1;
+ }
if (prefixcmp(buf, "gitdir: "))
die("Invalid gitfile format: %s", path);
while (buf[len - 1] == '\n' || buf[len - 1] == '\r')
@@ -312,6 +326,11 @@ const char *read_gitfile(const char *path)
die("No path in gitfile: %s", path);
buf[len] = '\0';
dir = buf + 8;
+ if (super && *super) {
+ strbuf_addf(&sb, "%s/repos/%s", *super, dir);
+ dir = sb.buf;
+ *super = xstrdup(real_path(*super));
+ }
if (!is_absolute_path(dir) && (slash = strrchr(path, '/'))) {
size_t pathlen = slash+1 - path;
@@ -320,18 +339,25 @@ const char *read_gitfile(const char *path)
strncpy(dir, path, pathlen);
strncpy(dir + pathlen, buf + 8, len - 8);
dir[dirlen] = '\0';
- free(buf);
- buf = dir;
+ free(to_free);
+ to_free = buf = dir;
}
- if (!is_git_directory(dir))
+ if (!is_git_directory_super(dir, super ? *super : dir))
die("Not a git repository: %s", dir);
path = real_path(dir);
- free(buf);
+ free(to_free);
+ strbuf_release(&sb);
return path;
}
+const char *read_gitfile(const char *path)
+{
+ return read_gitfile_super(path, NULL);
+}
+
+
static const char *setup_explicit_git_dir(const char *gitdirenv,
char *cwd, int len,
int *nongit_ok)
--
1.8.5.1.77.g42c48fa
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/POC 2/7] Add new environment variable $GIT_SUPER_DIR
This is the base for git-new-workdir integration. The git-new-workdir
script creates a separate worktree that shares everything except
worktree-related stuff. The sharing is eanbled by this new env
variable.
In the new worktree, both variables are set at the same time, GIT_DIR
and GIT_SUPER_DIR. Shared paths are checked at adjust_git_path() and
rewritten to use $GIT_SUPER_DIR instead of $GIT_DIR. Let's call this
"split-repo" setup to distinguish from $GIT_DIR-only one.
The "ln -s" is avoided because Windows probably does not have the
support, and symlinks don't survive operations that delete the old
file, then create a new one.
Signed-off-by: Nguyễn Thái Ngọc Duy
---
cache.h | 2 ++
environment.c | 11 +++--
path.c| 10
t/t0060-path-utils.sh | 67 +++
4 files changed, 88 insertions(+), 2 deletions(-)
diff --git a/cache.h b/cache.h
index cdafbd7..823582f 100644
--- a/cache.h
+++ b/cache.h
@@ -347,6 +347,7 @@ static inline enum object_type object_type(unsigned int
mode)
/* Double-check local_repo_env below if you add to this list. */
#define GIT_DIR_ENVIRONMENT "GIT_DIR"
+#define GIT_SUPER_DIR_ENVIRONMENT "GIT_SUPER_DIR"
#define GIT_NAMESPACE_ENVIRONMENT "GIT_NAMESPACE"
#define GIT_WORK_TREE_ENVIRONMENT "GIT_WORK_TREE"
#define GIT_PREFIX_ENVIRONMENT "GIT_PREFIX"
@@ -399,6 +400,7 @@ extern int is_inside_git_dir(void);
extern char *git_work_tree_cfg;
extern int is_inside_work_tree(void);
extern const char *get_git_dir(void);
+extern const char *get_git_super_dir(void);
extern int is_git_directory(const char *path);
extern char *get_object_directory(void);
extern char *get_index_file(void);
diff --git a/environment.c b/environment.c
index 1d74dde..d5ae7a3 100644
--- a/environment.c
+++ b/environment.c
@@ -79,7 +79,7 @@ static char *work_tree;
static const char *namespace;
static size_t namespace_len;
-static const char *git_dir;
+static const char *git_dir, *git_super_dir;
static char *git_object_dir, *git_index_file, *git_graft_file;
int git_db_env, git_index_env, git_graft_env;
@@ -131,10 +131,12 @@ static void setup_git_env(void)
git_dir = DEFAULT_GIT_DIR_ENVIRONMENT;
gitfile = read_gitfile(git_dir);
git_dir = xstrdup(gitfile ? gitfile : git_dir);
+ git_super_dir = getenv(GIT_SUPER_DIR_ENVIRONMENT);
git_object_dir = getenv(DB_ENVIRONMENT);
if (!git_object_dir) {
git_object_dir = xmalloc(strlen(git_dir) + 9);
- sprintf(git_object_dir, "%s/objects", git_dir);
+ sprintf(git_object_dir, "%s/objects",
+ git_super_dir ? git_super_dir : git_dir);
} else
git_db_env = 1;
git_index_file = getenv(INDEX_ENVIRONMENT);
@@ -167,6 +169,11 @@ const char *get_git_dir(void)
return git_dir;
}
+const char *get_git_super_dir(void)
+{
+ return git_super_dir;
+}
+
const char *get_git_namespace(void)
{
if (!namespace)
diff --git a/path.c b/path.c
index eda9176..86a7c15 100644
--- a/path.c
+++ b/path.c
@@ -75,6 +75,16 @@ static void adjust_git_path(char *buf, int git_dir_len)
strcpy(buf, get_index_file());
else if (git_db_env && dir_prefix(base, "objects"))
replace_dir(buf, git_dir_len + 7, get_object_directory());
+ else if (get_git_super_dir()) {
+ if (dir_prefix(base, "objects") || dir_prefix(base, "info") ||
+ dir_prefix(base, "refs") ||
+ (dir_prefix(base, "logs") && strcmp(base, "logs/HEAD")) ||
+ dir_prefix(base, "rr-cache") || dir_prefix(base, "hooks") ||
+ dir_prefix(base, "svn") ||
+ !strcmp(base, "config") || !strcmp(base, "packed-refs") ||
+ !strcmp(base, "shallow"))
+ replace_dir(buf, git_dir_len, get_git_super_dir());
+ }
}
static char *vsnpath(char *buf, size_t n, const char *fmt, va_list args)
diff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh
index 7ad2730..45f114c 100755
--- a/t/t0060-path-utils.sh
+++ b/t/t0060-path-utils.sh
@@ -271,4 +271,71 @@ test_expect_success 'git_path objects2' '
test_cmp expect actual
'
+test_expect_success 'git_path super index' '
+ GIT_SUPER_DIR=foo test-path-utils git_path index >actual &&
+ echo .git/index >expect &&
+ test_cmp expect actual
+'
+
+test_expect_success 'git_path super HEAD' '
+ GIT_SUPER_DIR=foo test-path-utils git_path HEAD >actual &&
+ echo .git/HEAD >expect &&
+ test_cmp expect actual
+'
+
+test_expect_success 'git_path super objects/*' '
+ GIT_SUPER_DIR=foo test-path-utils git_path objects/foo >actual &&
+ echo foo/objects/foo >expect &&
+ test_cmp expect actual
+'
+
+test_expect_success 'git_path super info/*' '
+ GIT_SUPER_DIR=foo test-path-utils git_path info/exclude >actual &&
+ echo
[PATCH/POC 5/7] setup.c: reduce cleanup sites in setup_explicit_git_dir()
Signed-off-by: Nguyễn Thái Ngọc Duy
---
setup.c | 20
1 file changed, 8 insertions(+), 12 deletions(-)
diff --git a/setup.c b/setup.c
index 43f56fb..dfe9d08 100644
--- a/setup.c
+++ b/setup.c
@@ -388,16 +388,13 @@ static const char *setup_explicit_git_dir(const char
*gitdirenv,
if (!is_git_directory(gitdirenv)) {
if (nongit_ok) {
*nongit_ok = 1;
- free(gitfile);
- return NULL;
+ goto done_null;
}
die("Not a git repository: '%s'", gitdirenv);
}
- if (check_repository_format_gently(gitdirenv, nongit_ok)) {
- free(gitfile);
- return NULL;
- }
+ if (check_repository_format_gently(gitdirenv, nongit_ok))
+ goto done_null;
/* #3, #7, #11, #15, #19, #23, #27, #31 (see t1510) */
if (work_tree_env)
@@ -408,8 +405,7 @@ static const char *setup_explicit_git_dir(const char
*gitdirenv,
/* #18, #26 */
set_git_dir(gitdirenv);
- free(gitfile);
- return NULL;
+ goto done_null;
}
else if (git_work_tree_cfg) { /* #6, #14 */
if (is_absolute_path(git_work_tree_cfg))
@@ -430,8 +426,7 @@ static const char *setup_explicit_git_dir(const char
*gitdirenv,
else if (!git_env_bool(GIT_IMPLICIT_WORK_TREE_ENVIRONMENT, 1)) {
/* #16d */
set_git_dir(gitdirenv);
- free(gitfile);
- return NULL;
+ goto done_null;
}
else /* #2, #10 */
set_git_work_tree(".");
@@ -442,8 +437,7 @@ static const char *setup_explicit_git_dir(const char
*gitdirenv,
/* both get_git_work_tree() and cwd are already normalized */
if (!strcmp(cwd, worktree)) { /* cwd == worktree */
set_git_dir(gitdirenv);
- free(gitfile);
- return NULL;
+ goto done_null;
}
offset = dir_inside_of(cwd, worktree);
@@ -459,6 +453,8 @@ static const char *setup_explicit_git_dir(const char
*gitdirenv,
/* cwd outside worktree */
set_git_dir(gitdirenv);
+
+done_null:
free(gitfile);
return NULL;
}
--
1.8.5.1.77.g42c48fa
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/POC 4/7] setup.c: add split-repo support to is_git_directory()
Signed-off-by: Nguyễn Thái Ngọc Duy
---
setup.c | 19 ++-
1 file changed, 14 insertions(+), 5 deletions(-)
diff --git a/setup.c b/setup.c
index 84362a6..43f56fb 100644
--- a/setup.c
+++ b/setup.c
@@ -182,28 +182,31 @@ void verify_non_filename(const char *prefix, const char
*arg)
*a proper "ref:", or a regular file HEAD that has a properly
*formatted sha1 object name.
*/
-int is_git_directory(const char *suspect)
+static int is_git_directory_super(const char *suspect, const char *super)
{
char path[PATH_MAX];
size_t len = strlen(suspect);
+ size_t super_len = strlen(super);
- if (PATH_MAX <= len + strlen("/objects"))
+ if (PATH_MAX <= super_len + strlen("/objects"))
die("Too long path: %.*s", 60, suspect);
- strcpy(path, suspect);
+ strcpy(path, super);
if (getenv(DB_ENVIRONMENT)) {
if (access(getenv(DB_ENVIRONMENT), X_OK))
return 0;
}
else {
- strcpy(path + len, "/objects");
+ strcpy(path + super_len, "/objects");
if (access(path, X_OK))
return 0;
}
- strcpy(path + len, "/refs");
+ strcpy(path + super_len, "/refs");
if (access(path, X_OK))
return 0;
+ if (super != suspect)
+ strcpy(path, suspect);
strcpy(path + len, "/HEAD");
if (validate_headref(path))
return 0;
@@ -211,6 +214,12 @@ int is_git_directory(const char *suspect)
return 1;
}
+int is_git_directory(const char *suspect)
+{
+ return is_git_directory_super(suspect, suspect);
+}
+
+
int is_inside_git_dir(void)
{
if (inside_git_dir < 0)
--
1.8.5.1.77.g42c48fa
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/POC 7/7] init: add --split-repo with the same functionality as git-new-workdir
"git init --split-repo abc" will create abc/.git file with the content
gitsuper: `git rev-parse --git-dir`
gitdir: abc
and a new directory in current .git/repos/abc with a valid HEAD.
This is enough to start checking out and do stuff. We should probably
take a branch name and check that branch out though. If that's the
case, this feature may better be parked in "git checkout" instead of
here..
So far it's not any better than git-new-workdir, except that info from
all worktrees created this way is centralized in the super repo's
.git/repos, which makes it possible to ensure what worktree does not
step on one another. In particular:
- If a worktree updates a ref, we could check if any other worktrees
are also checking out that ref. Detach those worktrees.
- prune/gc should be taught about the extra HEADs in .git/repos. fsck
on the super repo should be taught about extra indexes in
.git/repos
- ...
Signed-off-by: Nguyễn Thái Ngọc Duy
---
builtin/init-db.c | 42 ++
1 file changed, 42 insertions(+)
diff --git a/builtin/init-db.c b/builtin/init-db.c
index 78aa387..9c7139a 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -426,6 +426,38 @@ int init_db(const char *template_dir, unsigned int flags)
return 0;
}
+static int create_new_worktree(const char *path)
+{
+ struct strbuf sb = STRBUF_INIT;
+ const char *name;
+ unsigned char sha1[20];
+ FILE *fp;
+
+ for (name = path + strlen(path) - 1; name > path; name--)
+ if (is_dir_sep(*name)) {
+ name++;
+ break;
+ }
+
+ strbuf_addf(&sb, "%s/.git", path);
+ safe_create_leading_directories_const(sb.buf);
+ fp = fopen(sb.buf, "w");
+ fprintf(fp, "gitsuper: %s\ngitdir: %s\n",
+ real_path(get_git_dir()), name);
+ fclose(fp);
+ safe_create_leading_directories_const(git_path("repos/%s/HEAD",
+ name));
+ fp = fopen(git_path("repos/%s/HEAD", name), "w");
+ get_sha1("HEAD", sha1);
+ fprintf(fp, "%s\n", sha1_to_hex(sha1));
+ fclose(fp);
+#if 0
+ chdir(path);
+ system("git reset --hard");
+#endif
+ return 0;
+}
+
static int guess_repository_type(const char *git_dir)
{
char cwd[PATH_MAX];
@@ -481,6 +513,7 @@ int cmd_init_db(int argc, const char **argv, const char
*prefix)
const char *work_tree;
const char *template_dir = NULL;
unsigned int flags = 0;
+ int split_repo = 0;
const struct option init_db_options[] = {
OPT_STRING(0, "template", &template_dir,
N_("template-directory"),
N_("directory from which templates will be
used")),
@@ -491,6 +524,7 @@ int cmd_init_db(int argc, const char **argv, const char
*prefix)
N_("specify that the git repository is to be shared
amongst several users"),
PARSE_OPT_OPTARG | PARSE_OPT_NONEG, shared_callback, 0},
OPT_BIT('q', "quiet", &flags, N_("be quiet"), INIT_DB_QUIET),
+ OPT_BOOL(0, "split-repo", &split_repo, N_("git-new-workdir")),
OPT_STRING(0, "separate-git-dir", &real_git_dir, N_("gitdir"),
N_("separate git dir from working tree")),
OPT_END()
@@ -498,6 +532,14 @@ int cmd_init_db(int argc, const char **argv, const char
*prefix)
argc = parse_options(argc, argv, prefix, init_db_options,
init_db_usage, 0);
+ if (split_repo) {
+ if (real_git_dir)
+ die(_("--split-repo and --separate-git-dir are
incompatible"));
+ if (!argv[0])
+ die(_("--split-repo requires a path"));
+ return create_new_worktree(argv[0]);
+ }
+
if (real_git_dir && !is_absolute_path(real_git_dir))
real_git_dir = xstrdup(real_path(real_git_dir));
--
1.8.5.1.77.g42c48fa
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/POC 6/7] setup.c: add split-repo support to setup_git_directory*
XXX bare repos probably not worth supporting..
Signed-off-by: Nguyễn Thái Ngọc Duy
---
cache.h | 1 +
environment.c | 19 --
setup.c | 62 +++
trace.c | 1 +
4 files changed, 60 insertions(+), 23 deletions(-)
diff --git a/cache.h b/cache.h
index f85ee70..4c09223 100644
--- a/cache.h
+++ b/cache.h
@@ -406,6 +406,7 @@ extern char *get_object_directory(void);
extern char *get_index_file(void);
extern char *get_graft_file(void);
extern int set_git_dir(const char *path);
+extern int set_git_dir_super(const char *path, const char *super);
extern const char *get_git_namespace(void);
extern const char *strip_namespace(const char *namespaced_ref);
extern const char *get_git_work_tree(void);
diff --git a/environment.c b/environment.c
index d5ae7a3..8152c7e 100644
--- a/environment.c
+++ b/environment.c
@@ -125,13 +125,17 @@ static char *expand_namespace(const char *raw_namespace)
static void setup_git_env(void)
{
const char *gitfile;
+ char *super;
git_dir = getenv(GIT_DIR_ENVIRONMENT);
if (!git_dir)
git_dir = DEFAULT_GIT_DIR_ENVIRONMENT;
- gitfile = read_gitfile(git_dir);
+ gitfile = read_gitfile_super(git_dir, &super);
git_dir = xstrdup(gitfile ? gitfile : git_dir);
- git_super_dir = getenv(GIT_SUPER_DIR_ENVIRONMENT);
+ if (super)
+ git_super_dir = xstrdup(super);
+ else
+ git_super_dir = getenv(GIT_SUPER_DIR_ENVIRONMENT);
git_object_dir = getenv(DB_ENVIRONMENT);
if (!git_object_dir) {
git_object_dir = xmalloc(strlen(git_dir) + 9);
@@ -280,6 +284,17 @@ int set_git_dir(const char *path)
return 0;
}
+int set_git_dir_super(const char *path, const char *super)
+{
+ if (super != path &&
+ setenv(GIT_SUPER_DIR_ENVIRONMENT, super, 1))
+ return error("Could not set GIT_SUPER_DIR to '%s'", super);
+ if (setenv(GIT_DIR_ENVIRONMENT, path, 1))
+ return error("Could not set GIT_DIR to '%s'", path);
+ setup_git_env();
+ return 0;
+}
+
const char *get_log_output_encoding(void)
{
return git_log_output_encoding ? git_log_output_encoding
diff --git a/setup.c b/setup.c
index dfe9d08..f22381e 100644
--- a/setup.c
+++ b/setup.c
@@ -255,7 +255,8 @@ void setup_work_tree(void)
if (getenv(GIT_WORK_TREE_ENVIRONMENT))
setenv(GIT_WORK_TREE_ENVIRONMENT, ".", 1);
- set_git_dir(remove_leading_path(git_dir, work_tree));
+ set_git_dir_super(remove_leading_path(git_dir, work_tree),
+ get_git_super_dir());
initialized = 1;
}
@@ -368,24 +369,28 @@ const char *read_gitfile(const char *path)
static const char *setup_explicit_git_dir(const char *gitdirenv,
+ const char *super_,
char *cwd, int len,
int *nongit_ok)
{
const char *work_tree_env = getenv(GIT_WORK_TREE_ENVIRONMENT);
const char *worktree;
- char *gitfile;
+ char *gitfile, *super = (char *)super_;
int offset;
if (PATH_MAX - 40 < strlen(gitdirenv))
die("'$%s' too big", GIT_DIR_ENVIRONMENT);
- gitfile = (char*)read_gitfile(gitdirenv);
+ gitfile = (char*)read_gitfile_super(gitdirenv, !super ? &super : NULL);
if (gitfile) {
gitfile = xstrdup(gitfile);
gitdirenv = gitfile;
- }
+ if (!super)
+ super = gitfile;
+ } else if (!super)
+ super = (char *)gitdirenv;
- if (!is_git_directory(gitdirenv)) {
+ if (!is_git_directory_super(gitdirenv, super)) {
if (nongit_ok) {
*nongit_ok = 1;
goto done_null;
@@ -393,7 +398,7 @@ static const char *setup_explicit_git_dir(const char
*gitdirenv,
die("Not a git repository: '%s'", gitdirenv);
}
- if (check_repository_format_gently(gitdirenv, nongit_ok))
+ if (check_repository_format_gently(super, nongit_ok))
goto done_null;
/* #3, #7, #11, #15, #19, #23, #27, #31 (see t1510) */
@@ -404,7 +409,7 @@ static const char *setup_explicit_git_dir(const char
*gitdirenv,
die("core.bare and core.worktree do not make sense");
/* #18, #26 */
- set_git_dir(gitdirenv);
+ set_git_dir_super(gitdirenv, super);
goto done_null;
}
else if (git_work_tree_cfg) { /* #6, #14 */
@@ -425,7 +430,7 @@ static const char *setup_explicit_git_dir(const char
*gitdirenv,
}
else if (!git_env_bool(GIT_IMPLICIT_WORK_TREE_ENVIRONMENT, 1)) {
/* #16d */
- set_git_dir(gitdirenv);
+ set_git_dir_super(gitdirenv
[Question] Git recovery with HEAD commit broken
I am not a developer for git, but i am a regular user with git, i came the following problem: A power off cause my top commit broken, and then git branch/log/reflog..etc won't work. I do a hack that i change the HEAD commit to the one that i can make sure is right, and then i do: # git reset --hard HEAD In fact, i hope git fsck can fix up such problems(maybe can backup top commit for example)... Someone has faced such problems or have some suggestion for this? Thanks, Wang -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Question] Git recovery with HEAD commit broken
Shilong Wang writes: > A power off cause my top commit broken, and then git > branch/log/reflog..etc won't work. With a bit of luck, the reflog actually contain useful information. Look at .git/logs/HEAD (or refs/heads/* instead of HEAD for branches' reflog). It's a human-readable text format. You should be able to walk up the history looking for a good commit. > I do a hack that Before anything else: do a backup of your full repository while it's still time. > i change the HEAD commit to the one that i can make sure is right, (don't forget to run "git fsck" to make sure that not only the commit but also its ancestry is right). > In fact, i hope git fsck can fix up such problems(maybe can backup top > commit for example)... Not as far as I know. But "git fsck" has a --lost-found option that can help recovering unreachable (dangling) commits. You may have a look at http://hackage.haskell.org/package/git-repair but I do not think it would solve your particular case. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] t5541: Improve push test
On 2013-12-09 23.10, Junio C Hamano wrote: > Torsten Bögershausen writes: > >> The old log-line looked like this: >> + 9d498b0...8598732 master -> master (forced update) >> And the new one like this: >>9d498b0..8598732 master -> master >> >> - Loosen the grep pattern by not demanding "(forced update)" > > Hmm, what is the reason for the change the output? The output this > piece is testing is the result of this: > > git push origin master:retsam > > echo "change changed" > path2 && > git commit -a -m path2 --amend && > > # push master too; this ensures there is at least one '"'push'"' > command to > # the remote helper and triggers interaction with the helper. > test_must_fail git push -v origin +master master:retsam >output 2>&1' > > This is run inside test_repo_clone, which has /smart/test_repo.git > as its origin, which in turn has 'master' branch (and nothing else). > > It > > - pushes master to another branch retsam; > > - amends its 'master'; > > - attempts to push the updated master to force-update master, and >also retsam without forcing. The latter needs to be forced to >succeed, and that is why we expect it to fail. > > If the output from the push process says > > + 9d498b0...8598732 master -> master (forced update) > ! [rejected]master -> retsam (non-fast-forward) > error: failed to push some refs to '../test_repo_copy/' > > I think that is a good thing to do, no? After all, that is what we > show with Git native transports. > > Is this patch merely matching a test to a broken behaviour of some > sort? Puzzled... Thanks for the analysis: I thing the patch isn't the way to go. The regression in t5541 was introduced in f9e3c6bebb89de12. Which was a cleanup to previous commits: "transport-helper: add 'force' to 'export' helpers" So reverting f9e3c6bebb89de fixes t5541, but breaks contrib/remote-helpers. Felipe, could you have a look, please ? /Torsten force -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 2/4] gitweb: Return 1 on validation success instead of passed input
On Wed, Dec 11, 2013 at 12:54 PM, Krzesimir Nowak
wrote:
> Users of validate_* passing "0" might get failures on correct name
> because of coercion of "0" to false in code like:
> die_error(500, "invalid ref") unless (check_ref_format ("0"));
Very minor issue: there is no check_ref_format() after v7; perhaps
validate_pathname / is_valid_pathname
> Also, the validate_foo subs are renamed to is_valid_foo.
is_valid_foo is better than validate_foo IMVHO, though still not perfect
(it does some validation, but complete validation is done by git command).
>
> Signed-off-by: Krzesimir Nowak
--
Jakub Narebski
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Question] Git recovery with HEAD commit broken
Matthieu Moy wrote: > Not as far as I know. But "git fsck" has a --lost-found option that can > help recovering unreachable (dangling) commits. > > You may have a look at http://hackage.haskell.org/package/git-repair but > I do not think it would solve your particular case. Well, let's find out.. I corrupted .git/refs/heads/master to refer to a commit that does not exist. The history has a few prior commits. joey@darkstar:/tmp/yy>git fsck Checking object directories: 100% (256/256), done. error: HEAD: invalid sha1 pointer 10814e97cc8bf5f6f8ce0c0d5302f778c09cac88 error: refs/heads/master does not point to a valid object! notice: No default references joey@darkstar:/tmp/yy>~/src/git-repair/git-repair Running git fsck ... Initialized empty Git repository in /home/joey/tmp/tmprepo.0/.git/ 1 missing objects could not be recovered! To force a recovery to a usable state, retry with the --force parameter. - exit 1 If there had been a remote that had the missing 10814e97cc8bf5f6f8ce0c0d5302f778c09cac88 commit, it would have cloned it from there, and this would have succeeded. But with a fully missing commit, --force is needed to enable more destructive repairs. joey@darkstar:/tmp/yy>~/src/git-repair/git-repair --force Running git fsck ... Initialized empty Git repository in /home/joey/tmp/tmprepo.0/.git/ fatal: bad object refs/heads/master fatal: bad object refs/heads/master fatal: bad object refs/heads/master Deleted these local branches, which could not be recovered due to missing objects: refs/heads/master You currently have refs/heads/master checked out. You may have staged changes in the index that can be committed to recover the lost state of this branch! Successfully recovered repository! Please carefully check that the changes mentioned above are ok.. Hmm, that could have gone better. While it successfully detected the broken HEAD, and removed that ref, which is enough to make git fsck pass[1], it failed to find the old ref in the reflog, despite containing code that walks up it to find a usable commit. joey@darkstar:/tmp/yy>git reflog fatal: bad default revision 'HEAD' And that's why.. git-reflog requires a valid HEAD to work. Bit of a catch-22. I could work around this by manually parsing the reflog. It would not be the first thing git-repair has to re-implement because the git command isn't robust enough[2]. I have made a TODO about this. OTOH, if a kind git developer would like to make git-reflog work when HEAD is missing, that seems like a generally useful improvement.. -- see shy jo [1] It will make fsck pass 100% of the time -- its test suite randomly corrupts repositories and checks that it can force some repair good enough to make git fsck pass. [2] A particularly annoying one is that git branch -d cannot be used to remove a branch that is directly pointing to a corrupted commit! signature.asc Description: Digital signature
gitk "Parent" is wrong
Hello. I noticed a bug in gitk (as delivered with Git for Windows 1.8.4.msysgit.0). The description of the current commit (left down quarter) does contain items "Parent:" and "Child:". This is not true if gitk is called with a parameter. These should actually be labeled "Previous" and "Next", as they point to the neighbours is the displayed list of commits. Have a nice day, Stepan Kasal -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
git gui clone bug
Hello, I tried to use git gui in an empty dir (via Git Cheetah). I selected to clone from a file:///Y:/subdir/foo.git The result differed from the usual "git clone URL", as the newly created "master" branch was not set to track origin/master. "git pull" failed. When I start by "git clone", I can immediately proceed with "git pull" The difference can be checked in .git/config, in section branch.master. Have a nice day, Stepan Kasal -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Question] Git recovery with HEAD commit broken
Joey Hess wrote: > [2] A particularly annoying one is that git branch -d cannot be used > to remove a branch that is directly pointing to a corrupted commit! It's generally considered okay for everyday commands like "git branch -d" not to cope well with corrupted repositories, but we try to keep plumbing like "git update-ref -d" working to give people a way out. Is update-ref -d broken in this situation, too? Curious, Jonathan -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] cat-file: pass expand_data to print_object_or_die
Hi, Jeff King wrote: >However, if we pass > the whole struct, the called function can make more > intelligent decisions about which fields were actualled > filled by sha1_object_info. Thanks. s/actualled/actually/, I think. At first I thought this patch was going to be about making those intelligent decisions. Maybe s/the called function can/a future patch can teach the called function/ or something? [...] > There should be no functional change to this patch. The patch itself looks straightforward, yep. :) With the typofix mentioned above, Reviewed-by: Jonathan Nieder -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] cat-file: handle --batch format with missing type/size
Jeff King wrote:
> We could do the same for the type. However, besides our
> consistency check, we also care about the type in deciding
> whether to stream or not. We therefore make sure to always
> trigger a type lookup when we are printing, so that
This "We make sure" is the behavior after this patch, not before,
right?
[...]
> --- a/builtin/cat-file.c
> +++ b/builtin/cat-file.c
> @@ -211,7 +211,7 @@ static void print_object_or_die(int fd, struct
> expand_data *data)
> die("object %s disappeared", sha1_to_hex(sha1));
> if (type != data->type)
> die("object %s changed type!?", sha1_to_hex(sha1));
Maybe an assert(data.info.typep) or similar would make this more
locally readable.
[...]
> @@ -276,6 +276,13 @@ static int batch_objects(struct batch_options *opt)
> data.mark_query = 0;
>
> + /*
> + * If we are printing out the object, then always fill in the type,
> + * since we will want to decide whether or not to stream.
> + */
> + if (opt->print_contents)
> + data.info.typep = &data.type;
Oof. I guess this means that the optimization from 98e2092b wasn't being
applied by 'git cat-file --batch' with format specifiers that don't
include %(objecttype), but no one would have noticed because of the
"changed type" thing. :)
> --- a/t/t1006-cat-file.sh
> +++ b/t/t1006-cat-file.sh
> @@ -85,6 +85,28 @@ $content"
> git cat-file --batch-check="%(objecttype) %(rest)" >actual &&
> test_cmp expect actual
> '
> +
> +test -z "$content" ||
> +test_expect_success "--batch without type ($type)" '
> + {
> + echo "$size" &&
> + maybe_remove_timestamp "$content" $no_ts
> + } >expect &&
> + echo $sha1 | git cat-file --batch="%(objectsize)" >actual.full &&
> + maybe_remove_timestamp "$(cat actual.full)" $no_ts >actual &&
> + test_cmp expect actual
> +'
> +
> +test -z "$content" ||
> +test_expect_success "--batch without size ($type)" '
> + {
> + echo "$type" &&
> + maybe_remove_timestamp "$content" $no_ts
> + } >expect &&
> + echo $sha1 | git cat-file --batch="%(objecttype)" >actual.full &&
> + maybe_remove_timestamp "$(cat actual.full)" $no_ts >actual &&
> + test_cmp expect actual
> +'
> }
Looks good.
(not about this patch) I suspect a test_cmp_ignore_timestamp helper
could simplify these tests somewhat. :)
For what it's worth, with or without commit message changes or the
check that data->type is initialized,
Reviewed-by: Jonathan Nieder
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Publishing "filtered branch repositories" - workflow / recommendations?
Am 10.12.2013 00:56, schrieb Junio C Hamano: > Heiko Voigt writes: > >> On Fri, Dec 06, 2013 at 02:40:15PM -0500, Martin Langhoff wrote: >>> On Fri, Dec 6, 2013 at 3:48 AM, Jens Lehmann wrote: Right you are, we need tutorials for the most prominent use cases. >>> >>> In the meantime, are there any hints? Emails on this list showing a >>> current "smart" workflow? Blog posts? Notes on a wiki? >> >> None that I know of mainly because we have not yet reached the goal we >> are aiming at. Maybe we should write something, A few points from >> $dayjob that come to my mind: >> >> * A submodule commit is only allowed to be merged into master in a >> superproject commit if it is merged into master (or a stable branch) >> in the submodule. That way you ensure that any submodules commits >> that are tracked in a superproject are contained in each other and >> can be cleanly merged. (no rewinds, one commit contains the other) > > I think this is closely related to Martin's list of wishes we > earlier saw in the thread: remind the user to push necessary > submodule tip before the top-level commit that needs that commit in > the submodule is pushed out. Giving projects a way to implement > such a policy decision would be good, and having a default policy, > if we can find one that would be reasonable for any submodule users, > would be even better. Would adding a generic pre-push hook at the > top-level sufficient for that kind of thing, I have to wonder. That could call "git push --dry-run --recurse-submodules=check" to deny the push if the submodule commit isn't on a remote branch. But that would only work for a single hardcoded remote, as the remote itself does not seem to be passed to the pre-push hook. So me thinks adding a configuration option for the --recurse-submodule option of push is the best way to achieve that. This could be set to "check" for those who want to push the submodule manually and to "on-demand" for those who would like to push automatically. And with that option the user could configure push just like he already can do for fetch and pull. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git-submodule.sh respects submodule.$name.update in .git/config but not .gitmodules
Am 10.12.2013 00:40, schrieb Junio C Hamano: > Heiko Voigt writes: > >> This notion has changed in a way that only the url (by that the name) >> should be copied on init. The default for everything else should come >> from .gitmodules or gits own default. > > I think you need to be a bit careful here. I do not think > everything should blindly default to .gitmodules (otherwise there > are obvious security implications as well as usability ones). I believe everything except the URL should default to .gitmodules, for the same reasons we already do that for fetch and ignore [1]. But it should not do so blindly and take precautions that this only happens for safe values. The only current exception is the update setting, but I'd like to change that in two steps: 1) Teach git-submodule.sh to fall back to the update setting from .gitmodules if none is found in .git/config (more details below) 2) Wait some time and remove the copying on init later (to make life easier for people who are working on the same checkout with different versions of git). >> The update configuration option was implemented before we realized that. >> So currently it is still copied when submodule init is called. The main >> reason is that we have not found the time to change that. > > And 'update', as it allows any custom way to run it, is unlikely to > be allowed to be used blindly from .gitmodules, no? Definitely not. And while thinking about it it might make sense to pass a list of allowed values for all configurations to the get_submodule_config() function, making sure that fallbacks are only used when they are safe. [1] http://thread.gmane.org/gmane.comp.version-control.git/161193/focus=161357 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [git] Re: git-submodule.sh respects submodule.$name.update in .git/config but not .gitmodules
On Wed, Dec 11, 2013 at 11:26:17PM +0100, Jens Lehmann wrote: > Am 10.12.2013 00:40, schrieb Junio C Hamano: > > Heiko Voigt writes: > >> This notion has changed in a way that only the url (by that the > >> name) should be copied on init. The default for everything else > >> should come from .gitmodules or gits own default. > > > > I think you need to be a bit careful here. I do not think > > everything should blindly default to .gitmodules (otherwise there > > are obvious security implications as well as usability ones). > > I believe everything except the URL should default to .gitmodules, > for the same reasons we already do that for fetch and ignore [1]. > But it should not do so blindly and take precautions that this only > happens for safe values. I think the "safety of .git/config vs. convenience of .gitmodules" balance is going to break down differently for different folks. That's why I proposed get_submodule_config() [1] and submodule..active as an activation marker [2,3]. Then users can move as much or as little of the submodule config from .gitmodules into .git/config to strike the balance they feel is appropriate. For safety, maybe the default `init` should copy *everything* into .git/config, after which users can remove stuff they'd like to delegate to .gitmodules. Cheers, Trevor [1]: http://article.gmane.org/gmane.comp.version-control.git/210930 [2]: http://article.gmane.org/gmane.comp.version-control.git/211014 [3]: http://article.gmane.org/gmane.comp.version-control.git/211042 -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy signature.asc Description: OpenPGP digital signature
Re: [PATCH 1/2] cat-file: pass expand_data to print_object_or_die
On Wed, Dec 11, 2013 at 12:11:12PM -0800, Jonathan Nieder wrote:
> >However, if we pass
> > the whole struct, the called function can make more
> > intelligent decisions about which fields were actualled
> > filled by sha1_object_info.
>
> Thanks.
>
> s/actualled/actually/, I think.
Yes. Not sure how I managed that typo.
> At first I thought this patch was going to be about making those
> intelligent decisions. Maybe s/the called function can/a future patch
> can teach the called function/ or something?
I clarified it in the commit message below.
> > There should be no functional change to this patch.
>
> The patch itself looks straightforward, yep. :)
It technically does typo-fix the error message, which I guess is a
functional change. But I didn't count that. :)
Here it is with the commit message fixes and your reviewed-by.
-- >8 --
Subject: cat-file: pass expand_data to print_object_or_die
We currently individually pass the sha1, type, and size
fields calculated by sha1_object_info. However, if we pass
the whole struct, the called function can make more
intelligent decisions about which fields were actually
filled by sha1_object_info.
This patch takes that first refactoring step, passing the
whole struct, so further patches can make those decisions
with less noise in their diffs. There should be no
functional change to this patch (aside from a minor typo fix
in the error message).
As a side effect, we can rename the local variables in the
function to "type" and "size", since the names are no longer
taken.
Reviewed-by: Jonathan Nieder
Signed-off-by: Jeff King
---
builtin/cat-file.c | 21 +++--
1 file changed, 11 insertions(+), 10 deletions(-)
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index b2ca775..1434afb 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -193,25 +193,26 @@ static size_t expand_format(struct strbuf *sb, const char
*start, void *data)
return end - start + 1;
}
-static void print_object_or_die(int fd, const unsigned char *sha1,
- enum object_type type, unsigned long size)
+static void print_object_or_die(int fd, struct expand_data *data)
{
- if (type == OBJ_BLOB) {
+ const unsigned char *sha1 = data->sha1;
+
+ if (data->type == OBJ_BLOB) {
if (stream_blob_to_fd(fd, sha1, NULL, 0) < 0)
die("unable to stream %s to stdout", sha1_to_hex(sha1));
}
else {
- enum object_type rtype;
- unsigned long rsize;
+ enum object_type type;
+ unsigned long size;
void *contents;
- contents = read_sha1_file(sha1, &rtype, &rsize);
+ contents = read_sha1_file(sha1, &type, &size);
if (!contents)
die("object %s disappeared", sha1_to_hex(sha1));
- if (rtype != type)
+ if (type != data->type)
die("object %s changed type!?", sha1_to_hex(sha1));
- if (rsize != size)
- die("object %s change size!?", sha1_to_hex(sha1));
+ if (size != data->size)
+ die("object %s changed size!?", sha1_to_hex(sha1));
write_or_die(fd, contents, size);
free(contents);
@@ -250,7 +251,7 @@ static int batch_one_object(const char *obj_name, struct
batch_options *opt,
strbuf_release(&buf);
if (opt->print_contents) {
- print_object_or_die(1, data->sha1, data->type, data->size);
+ print_object_or_die(1, data);
write_or_die(1, "\n", 1);
}
return 0;
--
1.8.5.524.g6743da6
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] cat-file: handle --batch format with missing type/size
On Wed, Dec 11, 2013 at 12:42:00PM -0800, Jonathan Nieder wrote:
> > We could do the same for the type. However, besides our
> > consistency check, we also care about the type in deciding
> > whether to stream or not. We therefore make sure to always
> > trigger a type lookup when we are printing, so that
>
> This "We make sure" is the behavior after this patch, not before,
> right?
Correct. I'll clarify that.
> > --- a/builtin/cat-file.c
> > +++ b/builtin/cat-file.c
> > @@ -211,7 +211,7 @@ static void print_object_or_die(int fd, struct
> > expand_data *data)
> > die("object %s disappeared", sha1_to_hex(sha1));
> > if (type != data->type)
> > die("object %s changed type!?", sha1_to_hex(sha1));
>
> Maybe an assert(data.info.typep) or similar would make this more
> locally readable.
I'm not sure it makes it more readable, but it would protect against
violating our assumptions later (and ease people's minds who wonder why
we can touch data->type without a similar check).
> > + /*
> > +* If we are printing out the object, then always fill in the type,
> > +* since we will want to decide whether or not to stream.
> > +*/
> > + if (opt->print_contents)
> > + data.info.typep = &data.type;
>
> Oof. I guess this means that the optimization from 98e2092b wasn't being
> applied by 'git cat-file --batch' with format specifiers that don't
> include %(objecttype), but no one would have noticed because of the
> "changed type" thing. :)
Yes. The loss of the optimization was a small thing compared to being
totally broken. :)
> > + {
> > + echo "$type" &&
> > + maybe_remove_timestamp "$content" $no_ts
> > + } >expect &&
> > + echo $sha1 | git cat-file --batch="%(objecttype)" >actual.full &&
> > + maybe_remove_timestamp "$(cat actual.full)" $no_ts >actual &&
> > + test_cmp expect actual
> [...]
> (not about this patch) I suspect a test_cmp_ignore_timestamp helper
> could simplify these tests somewhat. :)
Yeah, the maybe_remove_timestamp is ugly on so many levels. I used it
because it's deeply embedded in the existing tests, and I didn't want to
tackle refactoring the whole thing. Be my guest if you want to do it on
top. :)
> For what it's worth, with or without commit message changes or the
> check that data->type is initialized,
>
> Reviewed-by: Jonathan Nieder
Thanks. Updated patch is below.
-- >8 --
Subject: cat-file: handle --batch format with missing type/size
Commit 98e2092 taught cat-file to stream blobs with --batch,
which requires that we look up the object type before
loading it into memory. As a result, we now print the
object header from information in sha1_object_info, and the
actual contents from the read_sha1_file. We double-check
that the information we printed in the header matches the
content we are about to show.
Later, commit 93d2a60 allowed custom header lines for
--batch, and commit 5b08640 made type lookups optional. As a
result, specifying a header line without the type or size
means that we will not look up those items at all.
This causes our double-checking to erroneously die with an
error; we think the type or size has changed, when in fact
it was simply left at "0".
For the size, we can fix this by only doing the consistency
double-check when we have retrieved the size via
sha1_object_info. In the case that we have not retrieved the
value, that means we also did not print it, so there is
nothing for us to check that we are consistent with.
We could do the same for the type. However, besides our
consistency check, we also care about the type in deciding
whether to stream or not. So instead of handling the case
where we do not know the type, this patch instead makes sure
that we always trigger a type lookup when we are printing,
so that even a format without the type will stream as we
would in the normal case.
Reviewed-by: Jonathan Nieder
Signed-off-by: Jeff King
---
builtin/cat-file.c | 11 ++-
t/t1006-cat-file.sh | 22 ++
2 files changed, 32 insertions(+), 1 deletion(-)
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 1434afb..f8288c8 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -197,6 +197,8 @@ static void print_object_or_die(int fd, struct expand_data
*data)
{
const unsigned char *sha1 = data->sha1;
+ assert(data->info.typep);
+
if (data->type == OBJ_BLOB) {
if (stream_blob_to_fd(fd, sha1, NULL, 0) < 0)
die("unable to stream %s to stdout", sha1_to_hex(sha1));
@@ -211,7 +213,7 @@ static void print_object_or_die(int fd, struct expand_data
*data)
die("object %s disappeared", sha1_to_hex(sha1));
if (type != data->type)
die("object %s changed type!?", sha1_to_hex(sha1));
- if (size != data->size)
+ if (data->info.sizep && size != data->size)
Re: Publishing "filtered branch repositories" - workflow / recommendations?
Jens Lehmann writes: >> I think this is closely related to Martin's list of wishes we >> earlier saw in the thread: remind the user to push necessary >> submodule tip before the top-level commit that needs that commit in >> the submodule is pushed out. Giving projects a way to implement >> such a policy decision would be good, and having a default policy, >> if we can find one that would be reasonable for any submodule users, >> would be even better. Would adding a generic pre-push hook at the >> top-level sufficient for that kind of thing, I have to wonder. > > That could call "git push --dry-run --recurse-submodules=check" to > deny the push if the submodule commit isn't on a remote branch. > that would only work for a single hardcoded remote, as the remote > itself does not seem to be passed to the pre-push hook. > > So me thinks adding a configuration option for the --recurse-submodule > option of push is the best way to achieve that. This could be set to > "check" ... Yes, that uses only a single hard-coded decision, and making the branch name or remote name customizable is not enough, as you are still hardcoding "if ... isn't on" part. It is not far-fetched to imagine a project wants to add more restrictions to what commit in the submodule history can be bound to a tree of a published commit in the top-level project (e.g. "must be a tagged release point", "must be older at least by more than two weeks", "must be signed by one of these developers' keys", etc.). So I am not yet convinced that a simple "option" that supplies a few parameters to a single hard-coded policy is sufficient in the long run. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] cat-file: handle --batch format with missing type/size
Jeff King wrote: > Updated patch is below. Thanks! v2 of both patches looks good. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC] HACK: use anchor mark instead of sel tag
---
I hacked somehow around this. It seems that just usilg the
anchor mark should be enough to implement almost the same
behavior. The hard part is that I don't know which feature
is intentional and which is just random consequence of using
sel for search highlight.
One thing which seems to me important and not possible to
implement without using another mask or tag is to return
currentsearchhit back when user removes characters from the
end of search string, and it starts to match previous
places.
This is not intended as patch for merging, just proof of
concept.
gitk-git/gitk | 32 ++--
1 file changed, 18 insertions(+), 14 deletions(-)
diff --git a/gitk-git/gitk b/gitk-git/gitk
index cf25472..c8b223f 100755
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -8320,24 +8320,29 @@ proc settabs {{firstab {}}} {
proc incrsearch {name ix op} {
global ctext searchstring searchdirn
-if {[catch {$ctext index anchor}]} {
+if {[catch {set prev_anchor [$ctext index anchor]}]} {
# no anchor set, use start of selection, or of visible area
set sel [$ctext tag ranges sel]
if {$sel ne {}} {
- $ctext mark set anchor [lindex $sel 0]
+ set start [lindex $sel 0]
} elseif {$searchdirn eq "-forwards"} {
- $ctext mark set anchor @0,0
+ set start @0,0
+ } else {
+ set start @0,[winfo height $ctext]
+ }
+} else {
+ if {$searchdirn eq "-forwards"} {
+ set start $prev_anchor
} else {
- $ctext mark set anchor @0,[winfo height $ctext]
+ set start "$prev_anchor + [string length $searchstring] c"
}
}
if {$searchstring ne {}} {
- set here [$ctext search -count mlen $searchdirn -- $searchstring anchor]
+ set here [$ctext search -count mlen $searchdirn -- $searchstring $start]
if {$here ne {}} {
$ctext see $here
set mend "$here + $mlen c"
- $ctext tag remove sel 1.0 end
- $ctext tag add sel $here $mend
+ $ctext mark set anchor $here
suppress_highlighting_file_for_current_scrollpos
highlightfile_for_scrollpos $here
}
@@ -8355,7 +8360,7 @@ proc dosearch {} {
set sel [$ctext tag ranges sel]
if {$sel ne {}} {
set start "[lindex $sel 0] + 1c"
- } elseif {[catch {set start [$ctext index anchor]}]} {
+ } elseif {[catch {set start "[$ctext index anchor] + 1c"}]} {
set start "@0,0"
}
set match [$ctext search -count mlen -- $searchstring $start]
@@ -8368,8 +8373,7 @@ proc dosearch {} {
suppress_highlighting_file_for_current_scrollpos
highlightfile_for_scrollpos $match
set mend "$match + $mlen c"
- $ctext tag add sel $match $mend
- $ctext mark unset anchor
+ $ctext mark set anchor $match
rehighlight_search_results
}
}
@@ -8397,8 +8401,7 @@ proc dosearchback {} {
suppress_highlighting_file_for_current_scrollpos
highlightfile_for_scrollpos $match
set mend "$match + $ml c"
- $ctext tag add sel $match $mend
- $ctext mark unset anchor
+$ctext mark set anchor $match
rehighlight_search_results
}
}
@@ -8418,14 +8421,15 @@ proc searchmark {first last} {
global ctext searchstring
puts [list $first $last]
-set sel [$ctext tag ranges sel]
+#TODO: catch no anchor
+set anchor [$ctext index anchor]
set mend $first.0
while {1} {
set match [$ctext search -count mlen -- $searchstring $mend $last.end]
if {$match eq {}} break
set mend "$match + $mlen c"
- if {$sel ne {} && [$ctext compare $match == [lindex $sel 0]]} {
+ if {$anchor eq $match} {
$ctext tag add currentsearchhit $match $mend
} else {
$ctext tag add found $match $mend
--
1.8.4.2.1566.g3c1a064
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
I have end-of-lifed cvsps
On the git tools wiki, the first paragraph of the entry for cvsps now reads: Warning: this code has been end-of-lifed by its maintainer in favor of cvs-fast-export. Several attempts over the space of a year to repair its deficient branch analysis and tag assignment have failed. Do not use it unless you are converting a strictly linear repository and cannot get rsync/ssh read access to the repo masters. If you must use it, be prepared to inspect and manually correct the history using reposurgeon. I tried very hard to salvage this program - the ability to remote-fetch CVS repos without rsync access was appealing - but I reached my limit earlier today when I actually found time to assemble a test set of CVS repos and run head-to-head tests comparing cvsps output to cvs-fast-export output. I've long believed that that cvs-fast-export has a better analyzer than cvsps just from having read the code for both of them, and having had to fix some serious bugs in cvsps that have no analogs in cvs-fast-export. Direct comparison of the stream outputs revealed that the difference in quality was larger than I had prevously grasped. Alas, I'm afraid the cvsps repo analysis code turns out to be crap all the way down on anything but the simplest linear and near-linear cases, and it doesn't do so hot on even those (all this *after* I fixed the most obvious bugs in the 2.x version). In retrospect, trying to repair it was misdirected effort. I recommend that git sever its dependency on this tool as soon as possible. I have shipped a 3.13 release with deprecation warnings fot archival purposes, after which I will cease maintainance and redirect anyone inquiring about cvsps to cvs-fast-export. (I also maintain cvs-fast-export, but credit for the excellent analysis code goes to Keith Packard. All I did was write the output stage, document it, and fix a few minor bugs.) -- http://www.catb.org/~esr/";>Eric S. Raymond You [should] not examine legislation in the light of the benefits it will convey if properly administered, but in the light of the wrongs it would do and the harm it would cause if improperly administered -- Lyndon Johnson, former President of the U.S. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git-submodule.sh respects submodule.$name.update in .git/config but not .gitmodules
"W. Trevor King" writes: > For > safety, maybe the default `init` should copy *everything* into > .git/config, after which users can remove stuff they'd like to > delegate to .gitmodules. Copying everything into config is "be unsafe and inconvenient by default for everybody", isn't it? Folks who want safety are forced to inspect the resulting entries in their config file (which is more inconvenent if you compare with the design where nothing is copied and nothing dynamically defaults to what then-current .gitmodules happens to contain). Folks who trust those who update .gitmodules for them are forced to update their config every time upstream decides to use different settings in .gitmodules, because they have stale values in their config that mask what are in .gitmodules. I think the solution we want is to copy only minimum to the config (and that "minimum" may turn out to be "nothing"), and to default keys that are only absolutely safe to .gitmodules file. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] cat-file: pass expand_data to print_object_or_die
Jeff King writes:
> It technically does typo-fix the error message, which I guess is a
> functional change. But I didn't count that. :)
>
> Here it is with the commit message fixes and your reviewed-by.
Thanks, both.
Will queue, to eventually merge to 'maint'.
>
> -- >8 --
> Subject: cat-file: pass expand_data to print_object_or_die
>
> We currently individually pass the sha1, type, and size
> fields calculated by sha1_object_info. However, if we pass
> the whole struct, the called function can make more
> intelligent decisions about which fields were actually
> filled by sha1_object_info.
>
> This patch takes that first refactoring step, passing the
> whole struct, so further patches can make those decisions
> with less noise in their diffs. There should be no
> functional change to this patch (aside from a minor typo fix
> in the error message).
>
> As a side effect, we can rename the local variables in the
> function to "type" and "size", since the names are no longer
> taken.
>
> Reviewed-by: Jonathan Nieder
> Signed-off-by: Jeff King
> ---
> builtin/cat-file.c | 21 +++--
> 1 file changed, 11 insertions(+), 10 deletions(-)
>
> diff --git a/builtin/cat-file.c b/builtin/cat-file.c
> index b2ca775..1434afb 100644
> --- a/builtin/cat-file.c
> +++ b/builtin/cat-file.c
> @@ -193,25 +193,26 @@ static size_t expand_format(struct strbuf *sb, const
> char *start, void *data)
> return end - start + 1;
> }
>
> -static void print_object_or_die(int fd, const unsigned char *sha1,
> - enum object_type type, unsigned long size)
> +static void print_object_or_die(int fd, struct expand_data *data)
> {
> - if (type == OBJ_BLOB) {
> + const unsigned char *sha1 = data->sha1;
> +
> + if (data->type == OBJ_BLOB) {
> if (stream_blob_to_fd(fd, sha1, NULL, 0) < 0)
> die("unable to stream %s to stdout", sha1_to_hex(sha1));
> }
> else {
> - enum object_type rtype;
> - unsigned long rsize;
> + enum object_type type;
> + unsigned long size;
> void *contents;
>
> - contents = read_sha1_file(sha1, &rtype, &rsize);
> + contents = read_sha1_file(sha1, &type, &size);
> if (!contents)
> die("object %s disappeared", sha1_to_hex(sha1));
> - if (rtype != type)
> + if (type != data->type)
> die("object %s changed type!?", sha1_to_hex(sha1));
> - if (rsize != size)
> - die("object %s change size!?", sha1_to_hex(sha1));
> + if (size != data->size)
> + die("object %s changed size!?", sha1_to_hex(sha1));
>
> write_or_die(fd, contents, size);
> free(contents);
> @@ -250,7 +251,7 @@ static int batch_one_object(const char *obj_name, struct
> batch_options *opt,
> strbuf_release(&buf);
>
> if (opt->print_contents) {
> - print_object_or_die(1, data->sha1, data->type, data->size);
> + print_object_or_die(1, data);
> write_or_die(1, "\n", 1);
> }
> return 0;
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] cat-file: handle --batch format with missing type/size
Jeff King writes: > Yes. The loss of the optimization was a small thing compared to being > totally broken. :) > ... >> Reviewed-by: Jonathan Nieder > > Thanks. Updated patch is below. ;-) I like it when I see patches are polished between the submitter and reviewer(s) fully, before the maintainer has a chance to pick an intermediate version (only to later replace and requeue). Thanks, both. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: I have end-of-lifed cvsps
On Wed, Dec 11, 2013 at 7:17 PM, Eric S. Raymond wrote: > I tried very hard to salvage this program - the ability to > remote-fetch CVS repos without rsync access was appealing Is that the only thing we lose, if we abandon cusps? More to the point, is there today an incremental import option, outside of git-cvsimport+cvsps? [ I am a bit out of touch with the current codebase but I coded and maintained a good part of it back in the day. However naive/limited the cvsps parser was, it did help a lot of projects make the leap to git... ] regards, m -- martin.langh...@gmail.com - ask interesting questions - don't get distracted with shiny stuff - working code first ~ http://docs.moodle.org/en/User:Martin_Langhoff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: I have end-of-lifed cvsps
Martin Langhoff : > On Wed, Dec 11, 2013 at 7:17 PM, Eric S. Raymond wrote: > > I tried very hard to salvage this program - the ability to > > remote-fetch CVS repos without rsync access was appealing > > Is that the only thing we lose, if we abandon cusps? More to the > point, is there today an incremental import option, outside of > git-cvsimport+cvsps? You'll have to remind me what you mean by "incremental" here. Possibly it's something cvs-fast-export could support. But what I'm trying to tell you is that, even after I've done a dozen releases and fixed the worst problems I could find, cvsps is far too likely to mangle anything that passes through it. The idea that you are preserving *anything* valuable by sticking with it is a mirage. "That bear trap! It's mangling your leg!" "But it's so *shiny*..." > [ I am a bit out of touch with the current codebase but I coded and > maintained a good part of it back in the day. However naive/limited > the cvsps parser was, it did help a lot of projects make the leap to > git... ] I fear those "lots of projects" have subtly damaged repository histories, then. I warned about this problem a year ago; today I found out it is much worse than I knew then, in fact so bad that I cannot responsibly do anything but try to get cvsps turfed out of use *as soon as possible*. And no, that should *not* wait on cvs-fast-export getting better support for "incremental" or any other legacy feature. Every week that cvsps remains the git project's choice is another week in which somebody's project history is likely to get trashed. This feels very strange and unpleasant. I've never had to shoot one of my own projects through the head before. I blogged about it: http://esr.ibiblio.org/?p=5167 Ignore the malware warning. It's triggered by something else on ibiblio.org; they're fixing it. -- http://www.catb.org/~esr/";>Eric S. Raymond -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
