Re: [RFC] make --set-upstream work for local branches not in refs/heads
On Tue, 2014-03-04 at 11:44 -0800, Junio C Hamano wrote:
Krzesimir Nowak krzesi...@endocode.com writes:
It might be possible (in Gerrited setups) to have local branches
outside refs/heads/, like for example in following fetch config:
[remote origin]
url = ssh://u...@example.com/my-project
fetch = +refs/heads/*:refs/remotes/origin/*
fetch = +refs/wip/*:refs/remotes/origin-wip/*
Let's say that 'test' branch already exist in origin/refs/wip/. If I
call:
git checkout test
then it will create a new branch and add an entry to .git/config:
[branch test]
remote = origin
merge = refs/wip/test
But if I create a branch 'test2' and call:
git push --set-upstream origin test2:refs/wip/test2
then branch is pushed, but no entry in .git config is created.
By definition anything otuside refs/heads/ is not a branch, so do
not call things in refs/wip branches. Retitle it to work for
local refs outside refs/heads or something.
I always have problems with proper use of git's terminology. Sorry.
Having said that, I do not see a major problem if we allowed
[branch test2]
remote = origin
merge = refs/wip/test2
to be created when push --setupstream is requested, making
test2@{upstream} to point at refs/remotes/origin-wip/test2.
I do not know what the correct implementation of such a feature
should be, though.
Hm, have some idea about it, though I will leave its sanity to judge to
you.
Given the following config snippet:
[remote origin]
url = ssh://u...@example.com/my-project
fetch = +refs/heads/*:refs/remotes/origin/*
fetch = +refs/wip/*:refs/remotes/origin-wip/*
We could have get_local_ref_hierarchies function defined somewhat as
follows (memory management details are left out):
char **get_local_ref_hierarchies(char *remote)
{
char **refspecs = get_fetch_refspecs_for_remote (remote);
char **iter;
char **local = NULL;
for (iter = refspecs; iter *iter; ++iter) {
char *src = get_src_refspec_part (*iter);
push (local, src);
}
/* maybe filter dups and make refs/heads/ first */
return local;
}
I'm sure that there are some corner-cases this code does not handle.
Also, maybe it would be good to add some information when --set-upstream
does nothing. Something like after doing git push --set-upstream origin
test:refs/wip/test:
Could not set temp to track refs/wip/test. Either call:
git config branch.test.remote origin
git config branch.test.merge refs/wip/test
or (this part would appear if this solution in patch is accepted)
git config --add remote.origin.fetch \
+refs/wip/*:refs/remotes/origin-wip/*
Cheers,
--
Krzesimir Nowak
Software Developer
Endocode AG
krzesi...@endocode.com
--
Endocode AG, Johannisstraße 20, 10117 Berlin
i...@endocode.com | www.endocode.com
Vorstandsvorsitzender: Mirko Boehm
Vorstände: Dr. Karl Beecher, Chris Kühl, Sebastian Sucker
Aufsichtsratsvorsitzende: Jennifer Beecher
Registergericht: Amtsgericht Charlottenburg - HRB 150748 B
--
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] make --set-upstream work for local branches not in refs/heads
It might be possible (in Gerrited setups) to have local branches outside refs/heads/, like for example in following fetch config: [remote origin] url = ssh://u...@example.com/my-project fetch = +refs/heads/*:refs/remotes/origin/* fetch = +refs/wip/*:refs/remotes/origin-wip/* Let's say that 'test' branch already exist in origin/refs/wip/. If I call: git checkout test then it will create a new branch and add an entry to .git/config: [branch test] remote = origin merge = refs/wip/test But if I create a branch 'test2' and call: git push --set-upstream origin test2:refs/wip/test2 then branch is pushed, but no entry in .git config is created. I have to do it manually. I attached a hack-patch to have it working just to check with you if anything like that would be accepted. Obviously the get_local_refs() would need to compute the actual list of local hierarchies (if it is possible at all). And it probably should get a better name. And probably something else. What do you think? Krzesimir Nowak (1): RFC: make --set-upstream work for branches not in refs/heads/ transport.c | 41 ++--- 1 file changed, 34 insertions(+), 7 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] RFC: make --set-upstream work for branches not in refs/heads/
---
transport.c | 41 ++---
1 file changed, 34 insertions(+), 7 deletions(-)
diff --git a/transport.c b/transport.c
index ca7bb44..ac933ee 100644
--- a/transport.c
+++ b/transport.c
@@ -143,6 +143,25 @@ static void insert_packed_refs(const char *packed_refs,
struct ref **list)
}
}
+/* That of course should not be hardcoded. */
+static const char* list_of_local_refs[] = {refs/heads/, refs/wip/, NULL};
+static const char** get_local_refs(void)
+{
+ return list_of_local_refs;
+}
+
+static int is_local_ref(const char *ref)
+{
+ const char **local_refs = get_local_refs();
+ const char **iter;
+
+ for (iter = local_refs; iter != NULL; ++iter)
+ if (starts_with(ref, *iter))
+ return strlen(*iter);
+
+ return 0;
+}
+
static void set_upstreams(struct transport *transport, struct ref *refs,
int pretend)
{
@@ -153,6 +172,8 @@ static void set_upstreams(struct transport *transport,
struct ref *refs,
const char *remotename;
unsigned char sha[20];
int flag = 0;
+ int localadd = 0;
+ int remoteadd = 0;
/*
* Check suitability for tracking. Must be successful /
* already up-to-date ref create/modify (not delete).
@@ -169,23 +190,29 @@ static void set_upstreams(struct transport *transport,
struct ref *refs,
localname = ref-peer_ref-name;
remotename = ref-name;
tmp = resolve_ref_unsafe(localname, sha, 1, flag);
- if (tmp flag REF_ISSYMREF
- starts_with(tmp, refs/heads/))
- localname = tmp;
+
+ if (tmp flag REF_ISSYMREF) {
+ localadd = is_local_ref (tmp);
+ if (localadd 0)
+ localname = tmp;
+ }
+ if (localadd == 0)
+ localadd = is_local_ref(localname);
+ remoteadd = is_local_ref(remotename);
/* Both source and destination must be local branches. */
- if (!localname || !starts_with(localname, refs/heads/))
+ if (!localname || localadd == 0)
continue;
- if (!remotename || !starts_with(remotename, refs/heads/))
+ if (!remotename || remoteadd == 0)
continue;
if (!pretend)
install_branch_config(BRANCH_CONFIG_VERBOSE,
- localname + 11, transport-remote-name,
+ localname + localadd, transport-remote-name,
remotename);
else
printf(Would set upstream of '%s' to '%s' of '%s'\n,
- localname + 11, remotename + 11,
+ localname + localadd, remotename + remoteadd,
transport-remote-name);
}
}
--
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 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 krzesi...@endocode.com
---
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 krzesi...@endocode.com
---
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
[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-short hash.tgz'.
Signed-off-by: Krzesimir Nowak krzesi...@endocode.com
---
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 krzesi...@endocode.com
---
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 =~ m!\0
Re: [PATCH 4/5] gitweb: Add a feature for adding more branch refs
On Thu, 2013-12-05 at 11:00 +0100, Krzesimir Nowak wrote: On Wed, 2013-12-04 at 19:06 +0100, Jakub Narębski wrote: On Wed, Dec 4, 2013 at 2:43 PM, Krzesimir Nowak krzesi...@endocode.com wrote: ++ +It is an error to specify a ref that does not pass git check-ref-format +scrutiny. Duplicated values are filtered. + Hmmm... 'snapshot' feature ignores invalid values, but in this case formerly valid compression schemes might get invalid via tightening %known_snapshot_formats, and we don't want existing config getting suddenly invalid. So, should I just ignore the invalid refs or treat them as errors? Ping, I'd like to know the answer for this question before I roll out another set of patches - there seem to be no agreement over it. Thanks! Cheers, -- Krzesimir Nowak Software Developer Endocode AG krzesi...@endocode.com -- Endocode AG, Johannisstraße 20, 10117 Berlin i...@endocode.com | www.endocode.com Vorstandsvorsitzender: Mirko Boehm Vorstände: Dr. Karl Beecher, Chris Kühl, Sebastian Sucker Aufsichtsratsvorsitzende: Jennifer Beecher Registergericht: Amtsgericht Charlottenburg - HRB 150748 B -- 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/5] gitweb: Add a comment explaining the meaning of $/
On Wed, 2013-12-04 at 12:28 -0800, Junio C Hamano wrote: Martin Langhoff martin.langh...@gmail.com writes: On Wed, Dec 4, 2013 at 10:46 AM, Krzesimir Nowak krzesi...@endocode.com wrote: On Wed, 2013-12-04 at 16:11 +0100, Jakub Narębski wrote: On Wed, Dec 4, 2013 at 2:42 PM, Krzesimir Nowak krzesi...@endocode.com wrote: So future reader will know what does it mean without running perldoc perlvar. Hmmm... shouldn't future reader know it anyway? It is not that cryptic. I'd say it is idiomatic Perl. It's plainly obscure. And I think it is not that often used - It's classic Perl. Perhaps you'd want to use English; and call it $INPUT_RECORD_SEPARATOR in a patch titled Make things readable to non-Perl natives. Hmm, but do we want to see use English there in the first place? Nevermind, I'm going to drop that patch. -- Krzesimir Nowak Software Developer Endocode AG krzesi...@endocode.com -- Endocode AG, Johannisstraße 20, 10117 Berlin i...@endocode.com | www.endocode.com Vorstandsvorsitzender: Mirko Boehm Vorstände: Dr. Karl Beecher, Chris Kühl, Sebastian Sucker Aufsichtsratsvorsitzende: Jennifer Beecher Registergericht: Amtsgericht Charlottenburg - HRB 150748 B -- 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/5] gitweb: Move check-ref-format code into separate function
On Wed, 2013-12-04 at 12:31 -0800, Junio C Hamano wrote:
Krzesimir Nowak krzesi...@endocode.com writes:
This check will be used in more than one place later.
Signed-off-by: Krzesimir Nowak krzesi...@endocode.com
Reviewed-by: Junio C Hamano gits...@pobox.com
Again, I do not think I reviewed this exact version. Nor did I say
that use of the ... or return undef is a good idea.
Ok, I'll drop them. Too much fuss over those lines.
Reviewed-by: Jakub Narębski jna...@gmail.com
---
gitweb/gitweb.perl | 17 +
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index ee61f9e..67415b9 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1452,6 +1452,16 @@ sub validate_pathname {
return $input;
}
+sub check_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
+ check_ref_format($input)
+ or return undef;
return $input;
}
--
Krzesimir Nowak
Software Developer
Endocode AG
krzesi...@endocode.com
--
Endocode AG, Johannisstraße 20, 10117 Berlin
i...@endocode.com | www.endocode.com
Vorstandsvorsitzender: Mirko Boehm
Vorstände: Dr. Karl Beecher, Chris Kühl, Sebastian Sucker
Aufsichtsratsvorsitzende: Jennifer Beecher
Registergericht: Amtsgericht Charlottenburg - HRB 150748 B
--
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/5] gitweb: Move check-ref-format code into separate function
On Wed, 2013-12-04 at 16:56 +0100, Jakub Narębski wrote:
On Wed, Dec 4, 2013 at 2:43 PM, Krzesimir Nowak krzesi...@endocode.com
wrote:
This check will be used in more than one place later.
Signed-off-by: Krzesimir Nowak krzesi...@endocode.com
Reviewed-by: Junio C Hamano gits...@pobox.com
Reviewed-by: Jakub Narębski jna...@gmail.com
All right, that is nice refactoring.
---
gitweb/gitweb.perl | 17 +
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index ee61f9e..67415b9 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1452,6 +1452,16 @@ sub validate_pathname {
return $input;
}
+sub check_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
+ check_ref_format($input)
+ or return undef;
return $input;
}
Right, check_ref_format() has name after git-check-ref-format...
though... check_ref_format() or die doesn't read completely
naturally...
Ok, I'll rename it to is_valid_ref_format.
--
Krzesimir Nowak
Software Developer
Endocode AG
krzesi...@endocode.com
--
Endocode AG, Johannisstraße 20, 10117 Berlin
i...@endocode.com | www.endocode.com
Vorstandsvorsitzender: Mirko Boehm
Vorstände: Dr. Karl Beecher, Chris Kühl, Sebastian Sucker
Aufsichtsratsvorsitzende: Jennifer Beecher
Registergericht: Amtsgericht Charlottenburg - HRB 150748 B
--
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 3/5] gitweb: Return plain booleans in validation methods
On Wed, 2013-12-04 at 17:07 +0100, Jakub Narębski wrote:
On Wed, Dec 4, 2013 at 2:43 PM, Krzesimir Nowak krzesi...@endocode.com
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));
I would say that the problem was that validate_sth() subroutines returned
value of parameter if it was valid, which could be a problem if said value is
false-ish (e.g. validate_refname(0), or validate_pathname(0)).
That's what I meant.
Returning undef on invalid data newer was a problem, using 'return $input;'
on valid input was, especially that validate_sth() functions were ever used
in a conditional:
if (!validate_sth($param)) {
die_error(...)
}
While at it validate_sth() is not a best name for boolean predicate:
is_valid_sth() would be better, I think.
Ok, I'll rename those.
Signed-off-by: Krzesimir Nowak krzesi...@endocode.com
---
gitweb/gitweb.perl | 45 +
1 file changed, 25 insertions(+), 20 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 67415b9..3434602 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1419,63 +1419,68 @@ sub href {
## validation, quoting/unquoting and escaping
sub validate_action {
- my $input = shift || return undef;
- return undef unless exists $actions{$input};
- return $input;
+ my $input = shift;
+
+ return 0 unless defined $input;
+ return 0 unless exists $actions{$input};
+ return 1;
}
The only change that needs to be doe is replacing
return $input;
with
return 1;
I prefer to use zeros instead of undefs - one might wonder if that undef
is somewhat special that we can't use 0.
--
Krzesimir Nowak
Software Developer
Endocode AG
krzesi...@endocode.com
--
Endocode AG, Johannisstraße 20, 10117 Berlin
i...@endocode.com | www.endocode.com
Vorstandsvorsitzender: Mirko Boehm
Vorstände: Dr. Karl Beecher, Chris Kühl, Sebastian Sucker
Aufsichtsratsvorsitzende: Jennifer Beecher
Registergericht: Amtsgericht Charlottenburg - HRB 150748 B
--
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 4/5] gitweb: Add a feature for adding more branch refs
On Wed, 2013-12-04 at 19:06 +0100, Jakub Narębski wrote:
On Wed, Dec 4, 2013 at 2:43 PM, Krzesimir Nowak krzesi...@endocode.com
wrote:
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 krzesi...@endocode.com
Reviewed-by: Junio C Hamano gits...@pobox.com
Reviewed-by: Jakub Narębski jna...@gmail.com
This version is Helped-by (maybe), but not (yet!) Reviewed-by.
---
Documentation/gitweb.conf.txt | 37 +++
gitweb/gitweb.perl| 85
+--
2 files changed, 110 insertions(+), 12 deletions(-)
diff --git a/Documentation/gitweb.conf.txt b/Documentation/gitweb.conf.txt
index e2113d9..5a77452 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'];
+
++
+If overriding was enabled then this feature can be configured on a
s/was/is/;
Perhaps it would better read as
This feature can be configured on per-repository basis after setting
$feature{'extra-branch-refs'}{'override'} to true, via repository's
`gitweb.extraBranchRefs` ...
I see that you also insist on using camelCase for config variables. I
will rephrase it.
+per-repository basis via repository's `gitweb.extrabranchrefs`
+configuration variable, which contains a space separated list of
+refs. An example:
++
+
+[gitweb]
+ extrabranchrefs = sandbox wip other
+
O.K.
++
+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
+
I think this part should be better left for a separate patch. There is
important difference between single-valued and multi-valued configuration
variable: with single-valued later occurrences override earlier ones,
which includes settings in more specific config file (e.g. per-repository)
overriding setting in more general one (e.g. per-user or system-wide).
With multi-valued we won't be able to override earlier / more generic
settings... well, unless we add support for no-value, or empty-value
as clearer, i.e.
[gitweb]
extrabranchrefs = sandbox
extrabranchrefs
# orextrabranchrefs =
extrabranchrefs = wip other
resulting in ('wip', 'other').
That point in this example is a bit moot IMO - if you don't want sandbox
ref to appear in list of branches view then just delete the offending
line.
I also made a small experiment. In per-instance config I have
$feature{'extra-branch-refs'}{'default'} = ['wip'];
$feature{'extra-branch-refs'}{'override'} = 1;
In per-repository config I have:
[gitweb]
extrabranchrefs = sandbox
extrabranchrefs = other
List of branches view shows only branches from sandbox and other. So
clearly per-repository config overrides per-instance config.
++
+It is an error to specify a ref that does not pass git check-ref-format
+scrutiny. Duplicated values are filtered.
+
Hmmm... 'snapshot' feature ignores invalid values, but in this case
formerly valid compression schemes might get invalid via tightening
%known_snapshot_formats, and we don't want existing config getting
suddenly invalid.
So, should I just ignore the invalid refs or treat them as errors?
[cut]
Nice!
--
Krzesimir Nowak
Software Developer
Endocode AG
krzesi...@endocode.com
--
Endocode AG, Johannisstraße 20, 10117 Berlin
i...@endocode.com | www.endocode.com
Vorstandsvorsitzender: Mirko Boehm
Vorstände: Dr. Karl Beecher, Chris Kühl, Sebastian Sucker
Aufsichtsratsvorsitzende: Jennifer Beecher
[PATCH 1/5] gitweb: Add a comment explaining the meaning of $/
So future reader will know what does it mean without running perldoc
perlvar.
Signed-off-by: Krzesimir Nowak krzesi...@endocode.com
---
gitweb/gitweb.perl | 2 ++
1 file changed, 2 insertions(+)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 68c77f6..ee61f9e 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -2629,6 +2629,8 @@ sub git_parse_project_config {
my $section_regexp = shift;
my %config;
+ # input record separator, so getline does end on null, not
+ # newline
local $/ = \0;
open my $fh, -|, git_cmd(), config, '-z', '-l',
--
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 4/5] 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 krzesi...@endocode.com
Reviewed-by: Junio C Hamano gits...@pobox.com
Reviewed-by: Jakub Narębski jna...@gmail.com
---
Documentation/gitweb.conf.txt | 37 +++
gitweb/gitweb.perl| 85 +--
2 files changed, 110 insertions(+), 12 deletions(-)
diff --git a/Documentation/gitweb.conf.txt b/Documentation/gitweb.conf.txt
index e2113d9..5a77452 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'];
+
++
+If overriding was enabled then this feature can be configured on a
+per-repository basis 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 3434602..6d3d52d 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,26 @@ sub feature_avatar {
return @val ? @val : @_;
}
+sub feature_extra_branch_refs {
+ my (@branch_refs) = @_;
+ my $values = git_get_project_config('extrabranchrefs');
+
+ if ($values) {
+ unless (ref $values) {
+ $values = [$values];
+ }
+ unless (ref $values eq 'ARRAY') {
+ return @branch_refs;
+ }
+ @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 +690,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 (check_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
[PATCH 3/5] gitweb: Return plain booleans in validation methods
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));
Signed-off-by: Krzesimir Nowak krzesi...@endocode.com
---
gitweb/gitweb.perl | 45 +
1 file changed, 25 insertions(+), 20 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 67415b9..3434602 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1419,63 +1419,68 @@ sub href {
## validation, quoting/unquoting and escaping
sub validate_action {
- my $input = shift || return undef;
- return undef unless exists $actions{$input};
- return $input;
+ my $input = shift;
+
+ return 0 unless defined $input;
+ return 0 unless exists $actions{$input};
+ return 1;
}
sub validate_project {
- my $input = shift || return undef;
+ my $input = shift;
+
+ return 0 unless defined $input;
if (!validate_pathname($input) ||
!(-d $projectroot/$input) ||
!check_export_ok($projectroot/$input) ||
($strict_export !project_in_list($input))) {
- return undef;
+ return 0;
} else {
- return $input;
+ return 1;
}
}
sub validate_pathname {
- my $input = shift || return undef;
+ my $input = shift;
+ return 0 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
if ($input =~ m!(^|/)(|\.|\.\.)(/|$)!) {
- return undef;
+ return 0;
}
# no null characters
if ($input =~ m!\0!) {
- return undef;
+ return 0;
}
- return $input;
+ return 1;
}
sub check_ref_format {
- my $input = shift || return undef;
+ my $input = shift;
+ return 0 unless defined $input;
# restrictions on ref name according to git-check-ref-format
if ($input =~ m!(/\.|\.\.|[\000-\040\177 ~^:?*\[]|/$)!) {
- return undef;
+ return 0;
}
- return $input;
+ return 1;
}
sub validate_refname {
- my $input = shift || return undef;
+ my $input = shift;
+ return undef unless defined $input;
# textual hashes are O.K.
if ($input =~ m/^[0-9a-fA-F]{40}$/) {
- return $input;
+ return 1;
}
# it must be correct pathname
- $input = validate_pathname($input)
- or return undef;
+ validate_pathname($input) or return 0;
# check git-check-ref-format restrictions
- check_ref_format($input)
- or return undef;
- return $input;
+ check_ref_format($input) or return 0;
+ return 1;
}
# decode sequences of octets in utf8 into Perl's internal form,
--
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
Re: [PATCH 0/3] Show extra branch refs in gitweb
On Tue, 2013-12-03 at 15:56 +0100, Krzesimir Nowak wrote: First patch just splits some code to a function, second patch adds the extra-branch-refs feature and third one adds some visual differentation of branches from non-standard ref directories. Krzesimir Nowak (3): gitweb: Move check-ref-format code into separate function gitweb: Add a feature for adding more branch refs gitweb: Denote non-heads, non-remotes branches. Documentation/gitweb.conf.txt | 27 ++ gitweb/gitweb.perl| 120 +++--- 2 files changed, 129 insertions(+), 18 deletions(-) New version of patches are in Show extra branch refs in gitweb v6 thread. Cheers, -- Krzesimir Nowak Software Developer Endocode AG krzesi...@endocode.com -- Endocode AG, Johannisstraße 20, 10117 Berlin i...@endocode.com | www.endocode.com Vorstandsvorsitzender: Mirko Boehm Vorstände: Dr. Karl Beecher, Chris Kühl, Sebastian Sucker Aufsichtsratsvorsitzende: Jennifer Beecher Registergericht: Amtsgericht Charlottenburg - HRB 150748 B -- 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/5] gitweb: Add a comment explaining the meaning of $/
On Wed, 2013-12-04 at 16:11 +0100, Jakub Narębski wrote:
On Wed, Dec 4, 2013 at 2:42 PM, Krzesimir Nowak krzesi...@endocode.com
wrote:
So future reader will know what does it mean without running perldoc
perlvar.
Hmmm... shouldn't future reader know it anyway? It is not that cryptic.
I'd say it is idiomatic Perl.
It's plainly obscure. And I think it is not that often used - I keep
forgetting what that pair of punctuation is actually meaning. In this
case I guess it would be more readable to use the following code
instead:
$fh-input_record_separator (\0);
Besides, it is not the only place where we set input record separator to NUL,
to match corresponding option to git command to separate records with NUL
(the '-z' option in this case). Others are git_get_path_by_hash(),
parse_commit(),
and parse_commits(), git_tree(), not including places where we set $/ to undef
to slurp whole file: git_get_link_target(), git_blobdiff() for $format
== 'plain', etc.
Yes, but I stumbled upon that one when trying to understand how config
parsing works. So I added a comment.
Signed-off-by: Krzesimir Nowak krzesi...@endocode.com
---
gitweb/gitweb.perl | 2 ++
1 file changed, 2 insertions(+)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 68c77f6..ee61f9e 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -2629,6 +2629,8 @@ sub git_parse_project_config {
my $section_regexp = shift;
my %config;
+ # input record separator, so getline does end on null, not
+ # newline
local $/ = \0;
It is here because of '-z' option below (to account for values with
embedded newlines).
open my $fh, -|, git_cmd(), config, '-z', '-l',
--
1.8.3.1
--
Krzesimir Nowak
Software Developer
Endocode AG
krzesi...@endocode.com
--
Endocode AG, Johannisstraße 20, 10117 Berlin
i...@endocode.com | www.endocode.com
Vorstandsvorsitzender: Mirko Boehm
Vorstände: Dr. Karl Beecher, Chris Kühl, Sebastian Sucker
Aufsichtsratsvorsitzende: Jennifer Beecher
Registergericht: Amtsgericht Charlottenburg - HRB 150748 B
--
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 v3] gitweb: Add an option for adding more branch refs
On Mon, 2013-12-02 at 18:34 +0100, Jakub Narębski wrote:
W dniu 2013-12-02 13:06, Krzesimir Nowak pisze:
On Mon, 2013-12-02 at 01:21 +0100, Jakub Narębski wrote:
On Thu, Nov 28, 2013 at 12:44 PM, Krzesimir Nowak
krzesi...@endocode.com wrote:
Allow @additional_branch_refs configuration variable to tell gitweb to
show refs from additional hierarchies in addition to branches in the
list-of-branches view.
Signed-off-by: Krzesimir Nowakkrzesi...@endocode.com
Why not use %feature hash instead of adding new configuration variable?
I think that this option is similar enough to 'remote_heads' feature
(which BTW should be 'remote-heads'), and could conceivably enabled
on a per-repository basis, i.e. with repository configuration override,
isn't it?
I'd like to see some consensus on it before I start changing the patch
again.
%feature hash is mainly (but not only) about options that can be
configured on per-repository basis. Configuration variables are
about options that are per-instance (per gitweb).
Well, I am mostly interested in per-instance configuration in this case,
but if that is also possible with %feature hash, then ok, I'll try to
make it work.
From what I've seen (correct me please if I got it wrong) feature
settings is taken from per-repository config file from [gitweb] section.
If there's nothing then some default value is taken. That default value
can be overriden with per-instance perl config file.
So it is easy to override it from per-instance perl config by typing:
$feature{'additional-branch-refs'}{'default'} = ['wip', 'no|tfun,ny'];
$feature{'additional-branch-refs'}{'override'} = 1;
(Note the edge case of refs/no|tfun,ny, which passes the git
check-ref-format scrutiny.)
But for now, most of features are quite simple - either booleans,
integers or list of simple strings (in snapshot feature). What I need
here is a list of strings, like CSV in following example:
[gitweb]
additional_branch_refs = wip,no|tfun,ny
Is dependency on external module like Text::CSV or Text::CSV_XS ok? If
not, I can hack some CSV reading code.
Usually %feature hash is preferred over adding new configuration variable
but this is not some hard rule. Note however that patches adding new config
are met with more scrutiny, as it is harder to fix mistakes because of
requirement of backwards compatibility of configuration files.
I don't know what kind of backwards compatibility you mention. Whether
you want gitweb to survive reading old config file or to honor
deprecated/old config variables.
I meant here honoring deprecated/old variables, i.e. honoring existing
configuration files. See for example backward compatibility for old
$stylesheet variable vs new @stylesheets in print_header_links().
Though in this case it shouldn't be much of a problem; it would be
easy to honor @additional_branch_refs by setting 'default' for
'extra-branch-refs' feature to it.
extra-branch-refs is nicer than additional-branch-refs, I'll use it.
BTW. there really should be gitweb/CodingGuidelines...
Yes, would be useful. As in every other project. :)
Well, Git itself *has* Documentation/CodingGuidelines, but perhaps
gitweb subsystem should have it's own...
[...]
@@ -3662,7 +3701,8 @@ sub git_get_heads_list {
my ($committer, $epoch, $tz) =
($committerinfo =~ /^(.*) ([0-9]+) (.*)$/);
$ref_item{'fullname'} = $name;
- $name =~ s!^refs/(?:head|remote)s/!!;
+ my $strip_refs = join '|', map { quotemeta }
get_branch_refs();
+ $name =~ s!^refs/(?:$strip_refs|remotes)/!!;
$ref_item{'name'} = $name;
$ref_item{'id'}= $hash;
@@ -7179,7 +7219,8 @@ sub snapshot_name {
$ver = $1;
} else {
# branches and other need shortened SHA-1 hash
- if ($hash =~ m!^refs/(?:heads|remotes)/(.*)$!) {
+ my $strip_refs = join '|', map { quotemeta }
get_branch_refs();
+ if ($hash =~ m!^refs/(?:$strip_refs|remotes)/(.*)$!) {
$ver = $1;
}
$ver .= '-' . git_get_short_hash($project, $hash);
One one hand, it is about threating extra branch refs the same way as
'head'.
On the other hand we loose distinction between 'refs/heads/foo' and e.g.
'refs/wip/foo'. But maybe that's all right...
In git_get_heads_list sub I could append a ($ref_dir) to refs which
are in neither 'heads' nor 'remotes', so heads view would look like:
master
old-stable
some-work-in-progress (wip)
some-other-branch (other)
where both master and old-stable are in refs/heads/,
some-work-in-progress in refs/wip/ and some-other-branch in refs/other/.
In case of branch snapshot names (snapshot_name sub) I could change it,
so names for branches mentioned above
[PATCH 1/3] 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 krzesi...@endocode.com
Reviewed-by: Junio C Hamano gits...@pobox.com
Reviewed-by: Jakub Narębski jna...@gmail.com
Reviewed-by: Eric Sunshine sunsh...@sunshineco.com
---
gitweb/gitweb.perl | 17 +
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 68c77f6..f7730d7 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1452,6 +1452,16 @@ sub validate_pathname {
return $input;
}
+sub check_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
+ $input = check_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 3/3] 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-short hash.tgz'.
Signed-off-by: Krzesimir Nowak krzesi...@endocode.com
Reviewed-by: Junio C Hamano gits...@pobox.com
Reviewed-by: Jakub Narębski jna...@gmail.com
Reviewed-by: Eric Sunshine sunsh...@sunshineco.com
---
gitweb/gitweb.perl | 27 +--
1 file changed, 25 insertions(+), 2 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 6326075..eb8d962 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -3723,8 +3723,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 different 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;
@@ -7241,7 +7247,24 @@ 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 = $1;
+ $ver = $2;
+
+ if (defined $ref_dir) {
+ # this is going to be a part of
+ # filename, so lets stick to
+ # alphanumerics, dashes and underlines
+ # only - some filesystems do not like
+ # some punctuation symbols for
+ # example.
+ $ref_dir =~ s/[^[:alnum:]_-]//g;
+ }
+
+ # for refs not 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);
}
--
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 2/3] 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 krzesi...@endocode.com
Reviewed-by: Junio C Hamano gits...@pobox.com
Reviewed-by: Jakub Narębski jna...@gmail.com
Reviewed-by: Eric Sunshine sunsh...@sunshineco.com
---
Documentation/gitweb.conf.txt | 27 +++
gitweb/gitweb.perl| 76 ---
2 files changed, 91 insertions(+), 12 deletions(-)
diff --git a/Documentation/gitweb.conf.txt b/Documentation/gitweb.conf.txt
index e2113d9..3de8e14 100644
--- a/Documentation/gitweb.conf.txt
+++ b/Documentation/gitweb.conf.txt
@@ -849,6 +849,33 @@ 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'];
+
++
+If overriding was enabled then this feature can be configured on a
+per-repository basis via repository's `gitweb.extrabranchrefs`
+configuration variable, which contains a space separated list of
+refs. An example:
++
+
+[gitweb]
+ extrabranchrefs = sandbox 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 f7730d7..6326075 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,17 @@ sub feature_avatar {
return @val ? @val : @_;
}
+sub feature_extra_branch_refs {
+ my (@branch_refs) = @_;
+ my $values = git_get_project_config('extra_branch_refs');
+
+ if ($values) {
+ @branch_refs = split /\s+/, $values;
+ }
+
+ 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 +681,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 (check_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 +1150,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_features {
# list of supported snapshot formats
our @snapshot_fmts = gitweb_get_feature('snapshot');
@@ -1131,6 +1168,13 @@ sub configure_gitweb_features {
} else {
$git_avatar = '';
}
+
+ our @extra_branch_refs = gitweb_get_feature('extra-branch-refs');
+ @extra_branch_refs = filter_and_validate_refs (@extra_branch_refs);
+}
+
+sub get_branch_refs {
+ return ('heads', @extra_branch_refs);
}
# custom error
[PATCH 0/3] Show extra branch refs in gitweb
First patch just splits some code to a function, second patch adds the extra-branch-refs feature and third one adds some visual differentation of branches from non-standard ref directories. Krzesimir Nowak (3): gitweb: Move check-ref-format code into separate function gitweb: Add a feature for adding more branch refs gitweb: Denote non-heads, non-remotes branches. Documentation/gitweb.conf.txt | 27 ++ gitweb/gitweb.perl| 120 +++--- 2 files changed, 129 insertions(+), 18 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
Re: [PATCH v3] gitweb: Add an option for adding more branch refs
On Tue, 2013-12-03 at 14:02 +0100, Jakub Narębski wrote:
On Tue, Dec 3, 2013 at 11:53 AM, Krzesimir Nowak krzesi...@endocode.com
wrote:
On Mon, 2013-12-02 at 18:34 +0100, Jakub Narębski wrote:
W dniu 2013-12-02 13:06, Krzesimir Nowak pisze:
On Mon, 2013-12-02 at 01:21 +0100, Jakub Narębski wrote:
On Thu, Nov 28, 2013 at 12:44 PM, Krzesimir Nowak
krzesi...@endocode.com wrote:
Allow @additional_branch_refs configuration variable to tell gitweb to
show refs from additional hierarchies in addition to branches in the
list-of-branches view.
Signed-off-by: Krzesimir Nowakkrzesi...@endocode.com
Why not use %feature hash instead of adding new configuration variable?
I think that this option is similar enough to 'remote_heads' feature
(which BTW should be 'remote-heads'), and could conceivably enabled
on a per-repository basis, i.e. with repository configuration override,
isn't it?
I'd like to see some consensus on it before I start changing the patch
again.
%feature hash is mainly (but not only) about options that can be
configured on per-repository basis. Configuration variables are
about options that are per-instance (per gitweb).
Well, I am mostly interested in per-instance configuration in this case,
but if that is also possible with %feature hash, then ok, I'll try to
make it work.
Yes, it is possible to have per-instance configuration (you can even
forbid per-repository configuration).
From what I've seen (correct me please if I got it wrong) feature
settings is taken from per-repository config file from [gitweb] section.
If there's nothing then some default value is taken. That default value
can be overriden with per-instance perl config file.
%feature settings are taken from gitweb configuration (the 'default'
key), and if given feature is overrideable and per-repository configuration
in the form of appropriate key in [gitweb] section of repository config
file exists, it is used instead.
So it is easy to override it from per-instance perl config by typing:
$feature{'additional-branch-refs'}{'default'} = ['wip', 'no|tfun,ny'];
$feature{'additional-branch-refs'}{'override'} = 1;
Yes. The 'override' is about checking (which imposes a bit of
performance penalty) and respecting per-repository configuration.
(Note the edge case of refs/no|tfun,ny, which passes the git
check-ref-format scrutiny.)
But for now, most of features are quite simple - either booleans,
integers or list of simple strings (in snapshot feature). What I need
here is a list of strings, like CSV in following example:
[gitweb]
additional_branch_refs = wip,no|tfun,ny
Is dependency on external module like Text::CSV or Text::CSV_XS ok? If
not, I can hack some CSV reading code.
Why not use space, which is forbidden in refnames, to separate
entries? Similar to feature_snapshot(), which is about comma separated
list, without escaping.
Thanks for explanations and a hint. Following patches are in Show extra
branch refs in gitweb.
Cheers,
--
Krzesimir Nowak
Software Developer
Endocode AG
krzesi...@endocode.com
--
Endocode AG, Johannisstraße 20, 10117 Berlin
i...@endocode.com | www.endocode.com
Vorstandsvorsitzender: Mirko Boehm
Vorstände: Dr. Karl Beecher, Chris Kühl, Sebastian Sucker
Aufsichtsratsvorsitzende: Jennifer Beecher
Registergericht: Amtsgericht Charlottenburg - HRB 150748 B
--
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 v3] gitweb: Add an option for adding more branch refs
On Thu, 2013-11-28 at 20:13 -0500, Eric Sunshine wrote:
On Thu, Nov 28, 2013 at 6:44 AM, Krzesimir Nowak krzesi...@endocode.com
wrote:
Allow @additional_branch_refs configuration variable to tell gitweb to
show refs from additional hierarchies in addition to branches in the
list-of-branches view.
Signed-off-by: Krzesimir Nowak krzesi...@endocode.com
---
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 68c77f6..25e1d37 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -680,6 +688,19 @@ sub read_config_file {
return;
}
+# performs sanity checks on parts of configuration.
+sub config_sanity_check {
+ # check additional refs validity
+ my %unique_branch_refs = ();
+ for my $ref (@additional_branch_refs) {
+ die_error(500, Invalid ref '$ref' in
\@additional_branch_refs) unless (validate_ref($ref));
+ # 'heads' are added implicitly in get_branch_refs().
+ $unique_branch_refs{$ref} = 1 if ($ref ne 'heads');
+ }
+ @additional_branch_refs = sort keys %unique_branch_refs;
+ %unique_branch_refs = undef;
+}
%unique_branch_refs is going out of scope here, so clearing it seems
unnecessary.
I am cleaning it in case when more sanity checking code gets added. So
there is no need to keep the data further.
Moreover, with warnings enabled, perl should be complaining about an
Odd number of elements in hash assignment. (Normally, you would
clear a hash with '%foo=()' or 'undef %foo'.)
Gah, ok. I'll fix it. Thanks.
+
our ($GITWEB_CONFIG, $GITWEB_CONFIG_SYSTEM, $GITWEB_CONFIG_COMMON);
sub evaluate_gitweb_config {
our $GITWEB_CONFIG = $ENV{'GITWEB_CONFIG'} || ++GITWEB_CONFIG++;
--
Krzesimir Nowak
Software Developer
Endocode AG
krzesi...@endocode.com
--
Endocode AG, Johannisstraße 20, 10117 Berlin
i...@endocode.com | www.endocode.com
Vorstandsvorsitzender: Mirko Boehm
Vorstände: Dr. Karl Beecher, Chris Kühl, Sebastian Sucker
Aufsichtsratsvorsitzende: Jennifer Beecher
Registergericht: Amtsgericht Charlottenburg - HRB 150748 B
--
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 v3] gitweb: Add an option for adding more branch refs
On Mon, 2013-12-02 at 01:21 +0100, Jakub Narębski wrote:
On Thu, Nov 28, 2013 at 12:44 PM, Krzesimir Nowak
krzesi...@endocode.com wrote:
Allow @additional_branch_refs configuration variable to tell gitweb to
show refs from additional hierarchies in addition to branches in the
list-of-branches view.
Signed-off-by: Krzesimir Nowak krzesi...@endocode.com
Why not use %feature hash instead of adding new configuration variable?
I think that this option is similar enough to 'remote_heads' feature
(which BTW should be 'remote-heads'), and could conceveilably enabled
on a per-repository basis, i.e. with repository configuration override,
isn't it?
I'd like to see some consensus on it before I start changing the patch
again.
Usually %feature hash is preferred over adding new configuration variable
but this is not some hard rule. Note however that patches adding new config
are met with more scrutiny, as it is harder to fix mistakes because of
requirement of backwards compatibility of configuration files.
I don't know what kind of backwards compatibility you mention. Whether
you want gitweb to survive reading old config file or to honor
deprecated/old config variables. If the former than I have already read
somewhere that you always should use config vars like:
our $config = 'value';
Note the 'our' which avoids gitweb failures in case of config variable
removal.
BTW. there really should be gitweb/CodingGuidelines...
Yes, would be useful. As in every other project. :)
---
Documentation/gitweb.conf.txt | 13
gitweb/gitweb.perl| 75
+--
2 files changed, 71 insertions(+), 17 deletions(-)
diff --git a/Documentation/gitweb.conf.txt b/Documentation/gitweb.conf.txt
index e2113d9..cd1a945 100644
--- a/Documentation/gitweb.conf.txt
+++ b/Documentation/gitweb.conf.txt
@@ -549,6 +549,19 @@ This variable matters only when using persistent web
environments that
serve multiple requests using single gitweb instance, like mod_perl,
FastCGI or Plackup.
+@additional_branch_refs::
+ List of additional directories under refs which are going to be
used
+ as branch refs. You might want to set this variable 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, for
example
++
+
+our @additional_branch_refs = ('sandbox', 'wip', 'other');
+
I think the last (long) sentence would better read if it began with For
example
if you have... then you could set this variable to ..., IMVHO.
Right, thanks. Will rephrase it.
BTW. if we decide on using %feature hash instead, it would be in the
CONFIGURING GITWEB FEATURES section.
Yes, but I'll wait for some consensus with it.
++
+It is an error to specify a ref that does not pass git check-ref-format
+scrutiny.
Hmmm... One one hand erroring out on invalid refs means that we can
find error in config earlier and easier, on the other hand ignoring invalid
refs would make it resilent to errors in gitweb config (and repository config,
if we use %feature with per-repository override).
We could ignore bad values, but that would make it harder to find out
what exactly is wrong when something we configured to be shown is not
shown at all.
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
[...]
@@ -626,6 +630,10 @@ sub feature_avatar {
return @val ? @val : @_;
}
+sub get_branch_refs {
+return ('heads', @additional_branch_refs);
+}
Nice way of ensuring that list of all branches starts with heads.
# 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.
@@ -680,6 +688,19 @@ sub read_config_file {
return;
}
+# performs sanity checks on parts of configuration.
+sub config_sanity_check {
+ # check additional refs validity
+ my %unique_branch_refs = ();
+ for my $ref (@additional_branch_refs) {
+ die_error(500, Invalid ref '$ref' in
\@additional_branch_refs) unless (validate_ref($ref));
+ # 'heads' are added implicitly in get_branch_refs().
+ $unique_branch_refs{$ref} = 1 if ($ref ne 'heads');
+ }
+ @additional_branch_refs = sort keys %unique_branch_refs;
+ %unique_branch_refs = undef;
+}
This subroutine is quite similar to filter_snapshot_fmts for 'snapshot'
feature, perhaps the name should be patterned after it, i.e.
filter_branch_refs() or something...
If there were generic config_sanity_check(), it would call
Re: [PATCH v2] gitweb: Add an option for adding more branch refs
On Wed, 2013-11-27 at 12:55 -0800, Junio C Hamano wrote:
Eric Sunshine sunsh...@sunshineco.com writes:
On Wed, Nov 27, 2013 at 3:34 PM, Eric Sunshine sunsh...@sunshineco.com
wrote:
On Wed, Nov 27, 2013 at 10:30 AM, Krzesimir Nowak
krzesi...@endocode.com wrote:
Overriding an @additional_branch_refs configuration variable with
value ('wip') will make gitweb to show branches that appear in
refs/heads and refs/wip (refs/heads is hardcoded).
branches are by definition what are in refs/heads/ hierarchy, so
Allow @additional_branch_refs configuration variable to tell
gitweb to show refs from additional hierarchies in addition to
branches in the list-of-branches view.
would be more appropriate and sufficient.
Thanks.
Mentioning $ref in the error message would help the user resolve the
problem more quickly.
+ die_error(500, 'heads specified in
@additional_branch_refs') if ($ref eq 'heads');
Rephrasing this as
heads disallowed in @additional_branch_refs
would better explain the problem to a user who has only made a cursory
read of the documentation.
The program could easily filter out the redundant 'heads', so does
this really deserve a diagnostic?
True.
Ok, I'm deduping both heads and other refs as well. Now we send 500 only
if the ref is simply invalid.
I was primarily worried about metacharacters in the specified
strings getting in the way of regexp matches the new code allows on
them, but that has been resolved with the use of \Q..\E; if that
automatic deduping is done, I do not immediately see any remaining
issues in the latest round of the patch.
New patch in [PATCH v3] gitweb: Add an option for adding more branch
refs. Thanks for reviews!
Thanks.
--
Krzesimir Nowak
Software Developer
Endocode AG
krzesi...@endocode.com
--
Endocode AG, Johannisstraße 20, 10117 Berlin
i...@endocode.com | www.endocode.com
Vorstandsvorsitzender: Mirko Boehm
Vorstände: Dr. Karl Beecher, Chris Kühl, Sebastian Sucker
Aufsichtsratsvorsitzende: Jennifer Beecher
Registergericht: Amtsgericht Charlottenburg - HRB 150748 B
--
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 v3] gitweb: Add an option for adding more branch refs
Allow @additional_branch_refs configuration variable to tell gitweb to
show refs from additional hierarchies in addition to branches in the
list-of-branches view.
Signed-off-by: Krzesimir Nowak krzesi...@endocode.com
---
Documentation/gitweb.conf.txt | 13
gitweb/gitweb.perl| 75 +--
2 files changed, 71 insertions(+), 17 deletions(-)
diff --git a/Documentation/gitweb.conf.txt b/Documentation/gitweb.conf.txt
index e2113d9..cd1a945 100644
--- a/Documentation/gitweb.conf.txt
+++ b/Documentation/gitweb.conf.txt
@@ -549,6 +549,19 @@ This variable matters only when using persistent web
environments that
serve multiple requests using single gitweb instance, like mod_perl,
FastCGI or Plackup.
+@additional_branch_refs::
+ List of additional directories under refs which are going to be used
+ as branch refs. You might want to set this variable 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, for example
++
+
+our @additional_branch_refs = ('sandbox', 'wip', 'other');
+
++
+It is an error to specify a ref that does not pass git check-ref-format
+scrutiny.
Other variables
~~~
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 68c77f6..25e1d37 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -122,6 +122,10 @@ our $logo_label = git homepage;
# source of projects list
our $projects_list = ++GITWEB_LIST++;
+# list of additional directories under refs/ we want to display as
+# branches
+our @additional_branch_refs = ();
+
# the width (in characters) of the projects list Description column
our $projects_list_description_width = 25;
@@ -626,6 +630,10 @@ sub feature_avatar {
return @val ? @val : @_;
}
+sub get_branch_refs {
+return ('heads', @additional_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.
@@ -680,6 +688,19 @@ sub read_config_file {
return;
}
+# performs sanity checks on parts of configuration.
+sub config_sanity_check {
+ # check additional refs validity
+ my %unique_branch_refs = ();
+ for my $ref (@additional_branch_refs) {
+ die_error(500, Invalid ref '$ref' in
\@additional_branch_refs) unless (validate_ref($ref));
+ # 'heads' are added implicitly in get_branch_refs().
+ $unique_branch_refs{$ref} = 1 if ($ref ne 'heads');
+ }
+ @additional_branch_refs = sort keys %unique_branch_refs;
+ %unique_branch_refs = undef;
+}
+
our ($GITWEB_CONFIG, $GITWEB_CONFIG_SYSTEM, $GITWEB_CONFIG_COMMON);
sub evaluate_gitweb_config {
our $GITWEB_CONFIG = $ENV{'GITWEB_CONFIG'} || ++GITWEB_CONFIG++;
@@ -698,8 +719,11 @@ sub evaluate_gitweb_config {
# Use first config file that exists. This means use the per-instance
# GITWEB_CONFIG if exists, otherwise use GITWEB_SYSTEM_CONFIG.
- read_config_file($GITWEB_CONFIG) and return;
- read_config_file($GITWEB_CONFIG_SYSTEM);
+ if (!read_config_file($GITWEB_CONFIG)) {
+ read_config_file($GITWEB_CONFIG_SYSTEM);
+ }
+
+ config_sanity_check();
}
# Get loadavg of system, to compare against $maxload.
@@ -1452,6 +1476,16 @@ sub validate_pathname {
return $input;
}
+sub validate_ref {
+ 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 +1496,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
+ $input = validate_ref($input)
+ or return undef;
return $input;
}
@@ -2515,6 +2548,7 @@ sub format_snapshot_links {
sub get_feed_info {
my $format = shift || 'Atom';
my %res = (action = lc($format));
+ my $matched_ref = 0;
# feed links are possible only for project views
return unless (defined $project);
@@ -2522,12 +2556,17 @@ sub get_feed_info {
# or don't have specific feed yet (so they should use generic)
return if (!$action || $action =~ /^(?:tags|heads|forks|tag|search
Re: [PATCH] gitweb: Add an option for adding more branch refs
On Tue, 2013-11-26 at 13:48 -0800, Junio C Hamano wrote:
Krzesimir Nowak krzesi...@endocode.com writes:
Overriding an @additional_branch_refs configuration variable with
value ('wip') will make gitweb to show branches that appear in
refs/heads and refs/wip (refs/heads is hardcoded). Might be useful for
gerrit setups where user branches are not stored under refs/heads/.
Signed-off-by: Krzesimir Nowak krzesi...@endocode.com
---
gitweb/gitweb.perl | 99
--
1 file changed, 74 insertions(+), 25 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 68c77f6..9bfd38b 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -17,6 +17,7 @@ use Encode;
use Fcntl ':mode';
use File::Find qw();
use File::Basename qw(basename);
+use List::Util qw(min);
use Time::HiRes qw(gettimeofday tv_interval);
binmode STDOUT, ':utf8';
@@ -122,6 +123,10 @@ our $logo_label = git homepage;
# source of projects list
our $projects_list = ++GITWEB_LIST++;
+# list of additional directories under refs/ we want to display as
+# branches
+our @additional_branch_refs = ();
+
# the width (in characters) of the projects list Description column
our $projects_list_description_width = 25;
@@ -626,14 +631,29 @@ sub feature_avatar {
return @val ? @val : @_;
}
+sub get_branch_refs {
+return ('heads', @additional_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.
sub check_head_link {
my ($dir) = @_;
my $headfile = $dir/HEAD;
- return ((-e $headfile) ||
- (-l $headfile readlink($headfile) =~ /^refs\/heads\//));
+
+ if (-e $headfile) {
+ return 1;
+ }
+ if (-l $headfile) {
+ my $rl = readlink($headfile);
+
+ for my $ref (get_branch_refs()) {
+ return 1 if $rl =~ /^refs\/$ref\//;
+ }
+ }
+ return 0;
The change to this function is wrong. A non-detached HEAD that
points at anything other than refs/heads/ should be considered a
repository corruption and should not be encouraged.
Ok, I'll get rid of it.
@@ -2515,6 +2535,7 @@ sub format_snapshot_links {
sub get_feed_info {
my $format = shift || 'Atom';
my %res = (action = lc($format));
+ my $matched_ref = 0;
# feed links are possible only for project views
return unless (defined $project);
@@ -2522,12 +2543,17 @@ sub get_feed_info {
# or don't have specific feed yet (so they should use generic)
return if (!$action || $action =~ /^(?:tags|heads|forks|tag|search)$/x);
- my $branch;
- # branches refs uses 'refs/heads/' prefix (fullname) to differentiate
- # from tag links; this also makes possible to detect branch links
- if ((defined $hash_base $hash_base =~ m!^refs/heads/(.*)$!) ||
- (defined $hash $hash =~ m!^refs/heads/(.*)$!)) {
- $branch = $1;
+ my $branch = undef;
+ # branches refs uses 'refs/' + $get_branch_refs()[x] + '/' prefix
+ # (fullname) to differentiate from tag links; this also makes
+ # possible to detect branch links
+ for my $ref (get_branch_refs()) {
+ if ((defined $hash_base $hash_base =~ m!^refs/$ref/(.*)$!) ||
+ (defined $hash $hash =~ m!^refs/$ref/(.*)$!)) {
+ $branch = $1;
+ $matched_ref = $ref;
+ last;
+ }
}
# find log type for feed description (title)
my $type = 'log';
@@ -2540,7 +2566,7 @@ sub get_feed_info {
}
$res{-title} = $type;
- $res{'hash'} = (defined $branch ? refs/heads/$branch : undef);
+ $res{'hash'} = (defined $branch ? refs/$matched_ref/$branch : undef);
$res{'file_name'} = $file_name;
OK.
@@ -3184,24 +3210,43 @@ sub git_get_project_owner {
return $owner;
}
-sub git_get_last_activity {
- my ($path) = @_;
- my $fd;
+sub git_get_last_activity_age {
+ my ($refs) = @_;
+ my $fd = -1;
- $git_dir = $projectroot/$path;
open($fd, -|, git_cmd(), 'for-each-ref',
'--format=%(committer)',
'--sort=-committerdate',
'--count=1',
-'refs/heads') or return;
+$refs) or return undef;
+
my $most_recent = $fd;
- close $fd or return;
+ close $fd or return undef;
if (defined $most_recent
$most_recent =~ / (\d+) [-+][01]\d\d\d$/) {
my $timestamp = $1;
- my $age = time - $timestamp;
- return ($age, age_string($age));
+ return time - $timestamp;
}
+
+ return undef;
+}
+
+sub git_get_last_activity {
+ my ($path) = @_;
+ my @ages = ();
+
+ $git_dir = $projectroot
[PATCH v2] gitweb: Add an option for adding more branch refs
Overriding an @additional_branch_refs configuration variable with
value ('wip') will make gitweb to show branches that appear in
refs/heads and refs/wip (refs/heads is hardcoded). Might be useful for
gerrit setups where user branches are not stored under refs/heads/.
Signed-off-by: Krzesimir Nowak krzesi...@endocode.com
---
Documentation/gitweb.conf.txt | 14
gitweb/gitweb.perl| 75 +--
2 files changed, 72 insertions(+), 17 deletions(-)
diff --git a/Documentation/gitweb.conf.txt b/Documentation/gitweb.conf.txt
index e2113d9..74de87a 100644
--- a/Documentation/gitweb.conf.txt
+++ b/Documentation/gitweb.conf.txt
@@ -549,6 +549,20 @@ This variable matters only when using persistent web
environments that
serve multiple requests using single gitweb instance, like mod_perl,
FastCGI or Plackup.
+@additional_branch_refs::
+ List of additional directories under refs which are going to be used
+ as branch refs. You might want to set this variable 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, for example
++
+
+our @additional_branch_refs = ('sandbox', 'wip', 'other');
+
++
+It is an error to specify 'heads' in the list - it is implicitly added. It is
an
+error to specify a ref that does not pass git check-ref-format scrutiny. It
is
+an error to specify a ref twice in the list.
Other variables
~~~
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 68c77f6..499281b 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -122,6 +122,10 @@ our $logo_label = git homepage;
# source of projects list
our $projects_list = ++GITWEB_LIST++;
+# list of additional directories under refs/ we want to display as
+# branches
+our @additional_branch_refs = ();
+
# the width (in characters) of the projects list Description column
our $projects_list_description_width = 25;
@@ -626,6 +630,10 @@ sub feature_avatar {
return @val ? @val : @_;
}
+sub get_branch_refs {
+return ('heads', @additional_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.
@@ -680,6 +688,19 @@ sub read_config_file {
return;
}
+# performs sanity checks on parts of configuration.
+sub config_sanity_check {
+ # check additional refs validity
+ my %unique_branch_refs = ();
+ for my $ref (@additional_branch_refs) {
+ die_error(500, 'Invalid ref in @additional_branch_refs') unless
(validate_ref($ref));
+ die_error(500, 'heads specified in @additional_branch_refs')
if ($ref eq 'heads');
+ die_error(500, Ref '$ref' repeated in
\@additional_branch_refs) if (exists $unique_branch_refs{$ref});
+ $unique_branch_refs{$ref} = 1;
+ }
+ %unique_branch_refs = undef;
+}
+
our ($GITWEB_CONFIG, $GITWEB_CONFIG_SYSTEM, $GITWEB_CONFIG_COMMON);
sub evaluate_gitweb_config {
our $GITWEB_CONFIG = $ENV{'GITWEB_CONFIG'} || ++GITWEB_CONFIG++;
@@ -698,8 +719,11 @@ sub evaluate_gitweb_config {
# Use first config file that exists. This means use the per-instance
# GITWEB_CONFIG if exists, otherwise use GITWEB_SYSTEM_CONFIG.
- read_config_file($GITWEB_CONFIG) and return;
- read_config_file($GITWEB_CONFIG_SYSTEM);
+ if (!read_config_file($GITWEB_CONFIG)) {
+ read_config_file($GITWEB_CONFIG_SYSTEM);
+ }
+
+ config_sanity_check();
}
# Get loadavg of system, to compare against $maxload.
@@ -1452,6 +1476,16 @@ sub validate_pathname {
return $input;
}
+sub validate_ref {
+ 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 +1496,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
+ $input = validate_ref($input)
+ or return undef;
return $input;
}
@@ -2515,6 +2548,7 @@ sub format_snapshot_links {
sub get_feed_info {
my $format = shift || 'Atom';
my %res = (action = lc($format));
+ my $matched_ref = 0
Re: [PATCH] gitweb: Add an option for adding more branch refs
On Wed, 2013-11-27 at 16:56 +0100, Jakub Narębski wrote:
Krzesimir Nowak wrote:
Overriding an @additional_branch_refs configuration variable with
value ('wip') will make gitweb to show branches that appear in
refs/heads and refs/wip (refs/heads is hardcoded). Might be useful for
gerrit setups where user branches are not stored under refs/heads/.
The description of this change starts with technical details,
instead of starting with intent of this change.
Perhaps (this is only a proposal)
Introduce @additional_branch_refs configuration variable, holding
names of references to be considered branches; by default empty.
For example setting it to ('wip') will make gitweb ...
I have already posted second version of the patch. But I didn't change
the commit message though. But thanks for proposal - it sounds better.
I'll try to make it better next time I post a patch.
BTW. I have thought at first that is something similar to 'remote_heads'
feature, which among others adds 'remotes' section to 'summary' view
displaying refs/remotes/* refs... but no, gitweb still doesn't treat
refs/remotes as branches, even with this feature set.
Nb. why new configuration variable, and not new %feature?
I dunno. Hard to tell where it fits. Junio told me about using normal
gitweb configuration mechanism, so that's the first thing that got my
attention.
http://www.mail-archive.com/git@vger.kernel.org/msg39859.html
Signed-off-by: Krzesimir Nowak krzesi...@endocode.com
---
gitweb/gitweb.perl | 99
--
1 file changed, 74 insertions(+), 25 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 68c77f6..9bfd38b 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -17,6 +17,7 @@ use Encode;
use Fcntl ':mode';
use File::Find qw();
use File::Basename qw(basename);
+use List::Util qw(min);
use Time::HiRes qw(gettimeofday tv_interval);
binmode STDOUT, ':utf8';
[...]
@@ -3184,24 +3210,43 @@ sub git_get_project_owner {
return $owner;
}
-sub git_get_last_activity {
- my ($path) = @_;
- my $fd;
+sub git_get_last_activity_age {
+ my ($refs) = @_;
+ my $fd = -1;
- $git_dir = $projectroot/$path;
open($fd, -|, git_cmd(), 'for-each-ref',
'--format=%(committer)',
'--sort=-committerdate',
'--count=1',
- 'refs/heads') or return;
+ $refs) or return undef;
git-for-each-ref accepts more than one pattern. Why not simply
open($fd, -|, git_cmd(), 'for-each-ref',
'--format=%(committer)',
'--sort=-committerdate',
'--count=1',
- 'refs/heads') or return;
+ get_branch_refs()) or return;
Then we won't need List::Util::min.
Yes, Junio pointed that out to me - fixed in second version of the
patch.
[...]
+sub git_get_last_activity {
+ my ($path) = @_;
+ my @ages = ();
+
+ $git_dir = $projectroot/$path;
+ for my $ref (get_branch_refs()) {
+ my $age = git_get_last_activity_age('refs/' . $_);
+
+ push @ages, $age if defined $age;
+ }
+ if (@ages) {
+ my $min_age = min(@ages);
+
+ return ($min_age, age_string($min_age));
+ }
+
return (undef, undef);
}
[...]
--
Krzesimir Nowak
Software Developer
Endocode AG
krzesi...@endocode.com
--
Endocode AG, Johannisstraße 20, 10117 Berlin
i...@endocode.com | www.endocode.com
Vorstandsvorsitzender: Mirko Boehm
Vorstände: Dr. Karl Beecher, Chris Kühl, Sebastian Sucker
Aufsichtsratsvorsitzende: Jennifer Beecher
Registergericht: Amtsgericht Charlottenburg - HRB 150748 B
--
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] gitweb: Add an option for adding more branch refs
Overriding an @additional_branch_refs configuration variable with
value ('wip') will make gitweb to show branches that appear in
refs/heads and refs/wip (refs/heads is hardcoded). Might be useful for
gerrit setups where user branches are not stored under refs/heads/.
Signed-off-by: Krzesimir Nowak krzesi...@endocode.com
---
gitweb/gitweb.perl | 99 --
1 file changed, 74 insertions(+), 25 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 68c77f6..9bfd38b 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -17,6 +17,7 @@ use Encode;
use Fcntl ':mode';
use File::Find qw();
use File::Basename qw(basename);
+use List::Util qw(min);
use Time::HiRes qw(gettimeofday tv_interval);
binmode STDOUT, ':utf8';
@@ -122,6 +123,10 @@ our $logo_label = git homepage;
# source of projects list
our $projects_list = ++GITWEB_LIST++;
+# list of additional directories under refs/ we want to display as
+# branches
+our @additional_branch_refs = ();
+
# the width (in characters) of the projects list Description column
our $projects_list_description_width = 25;
@@ -626,14 +631,29 @@ sub feature_avatar {
return @val ? @val : @_;
}
+sub get_branch_refs {
+return ('heads', @additional_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.
sub check_head_link {
my ($dir) = @_;
my $headfile = $dir/HEAD;
- return ((-e $headfile) ||
- (-l $headfile readlink($headfile) =~ /^refs\/heads\//));
+
+ if (-e $headfile) {
+ return 1;
+ }
+ if (-l $headfile) {
+ my $rl = readlink($headfile);
+
+ for my $ref (get_branch_refs()) {
+ return 1 if $rl =~ /^refs\/$ref\//;
+ }
+ }
+
+ return 0;
}
sub check_export_ok {
@@ -2515,6 +2535,7 @@ sub format_snapshot_links {
sub get_feed_info {
my $format = shift || 'Atom';
my %res = (action = lc($format));
+ my $matched_ref = 0;
# feed links are possible only for project views
return unless (defined $project);
@@ -2522,12 +2543,17 @@ sub get_feed_info {
# or don't have specific feed yet (so they should use generic)
return if (!$action || $action =~ /^(?:tags|heads|forks|tag|search)$/x);
- my $branch;
- # branches refs uses 'refs/heads/' prefix (fullname) to differentiate
- # from tag links; this also makes possible to detect branch links
- if ((defined $hash_base $hash_base =~ m!^refs/heads/(.*)$!) ||
- (defined $hash $hash =~ m!^refs/heads/(.*)$!)) {
- $branch = $1;
+ my $branch = undef;
+ # branches refs uses 'refs/' + $get_branch_refs()[x] + '/' prefix
+ # (fullname) to differentiate from tag links; this also makes
+ # possible to detect branch links
+ for my $ref (get_branch_refs()) {
+ if ((defined $hash_base $hash_base =~ m!^refs/$ref/(.*)$!) ||
+ (defined $hash $hash =~ m!^refs/$ref/(.*)$!)) {
+ $branch = $1;
+ $matched_ref = $ref;
+ last;
+ }
}
# find log type for feed description (title)
my $type = 'log';
@@ -2540,7 +2566,7 @@ sub get_feed_info {
}
$res{-title} = $type;
- $res{'hash'} = (defined $branch ? refs/heads/$branch : undef);
+ $res{'hash'} = (defined $branch ? refs/$matched_ref/$branch : undef);
$res{'file_name'} = $file_name;
return %res;
@@ -3184,24 +3210,43 @@ sub git_get_project_owner {
return $owner;
}
-sub git_get_last_activity {
- my ($path) = @_;
- my $fd;
+sub git_get_last_activity_age {
+ my ($refs) = @_;
+ my $fd = -1;
- $git_dir = $projectroot/$path;
open($fd, -|, git_cmd(), 'for-each-ref',
'--format=%(committer)',
'--sort=-committerdate',
'--count=1',
-'refs/heads') or return;
+$refs) or return undef;
+
my $most_recent = $fd;
- close $fd or return;
+ close $fd or return undef;
if (defined $most_recent
$most_recent =~ / (\d+) [-+][01]\d\d\d$/) {
my $timestamp = $1;
- my $age = time - $timestamp;
- return ($age, age_string($age));
+ return time - $timestamp;
}
+
+ return undef;
+}
+
+sub git_get_last_activity {
+ my ($path) = @_;
+ my @ages = ();
+
+ $git_dir = $projectroot/$path;
+ for my $ref (get_branch_refs()) {
+ my $age = git_get_last_activity_age('refs/' . $_);
+
+ push @ages, $age if defined $age;
+ }
+ if (@ages) {
+ my $min_age = min(@ages
Re: [PATCH] gitweb: Make showing branches configurable
On Mon, 2013-11-25 at 11:32 -0800, Junio C Hamano wrote:
Krzesimir Nowak krzesi...@endocode.com writes:
On Fri, 2013-11-22 at 09:34 -0800, Junio C Hamano wrote:
Krzesimir Nowak krzesi...@endocode.com writes:
Running 'make GITWEB_WANTED_REFS=heads wip gitweb.cgi' will create a
gitweb CGI script showing branches that appear in refs/heads/ and in
refs/wip/. Might be useful for gerrit setups where user branches are
not stored under refs/heads/.
Signed-off-by: Krzesimir Nowak krzesi...@endocode.com
---
Notes:
I'm actually not sure if all those changes are really necessary as I
was mostly targeting it for Gerrit use. Especially I mean the changes
in git_get_remotes_list, fill_remote_heads and print_page_nav. I
tried
to make it as general as it gets, so there's nothing Gerrit specific.
Thanks.
Two knee-jerk reactions after a quick scan.
- You include heads for normal builds by hardcoded
GITWEB_WANTED_REFS = heads but include tags unconditionally
by having @ref_views = (tags, @wanted_refs) in the code. Why?
Earlier both tags and heads were hardcoded there. So now instead of
heads we have @wanted_refs.
I suppose I should have given it a better name, like @branch_refs.
Right?
My original question was why the change was not done like this:
In gitweb/Makefile, to give the default that is the same as
before:
GITWEB_WANTED_REFS = heads tags
In format_refs_views
my @ref_views = @wanted_refs;
But looking at the existing code again, you are only interested in
renaming 'heads' to some other possibly multiple hierarchies, so
that would not fly well. Indeed, as you said, wanted refs is a
misnomer that led to the above confusion. If it is only about what
are the branches, then the configuration should be named after the
branch ness of that thing.
But that leads to another question. Is there _any_ use case where
showing 'heads/' hierarchy is undesirable? It would be utterly
confusing if something that claims to show branches does not include
the 'heads/', even though it might be acceptable if it showed other
things.
Agreed, although certainly such setup is possible, albeit inpractical
and, as you said, confusing. Also, remembering about adding 'heads' to
overriden config variable is bothersome. Gitweb rather does not need to
support such unusual setup then.
- Does this have be a compile-time decision? It looks like this is
something that can and should be made controllable with the
normal gitweb configuration mechanism.
Maybe. I was just looking at Makefile and saw a bunch of configuration
options there, so I just added another one. Haven't noticed the gitweb
config thing. Sorry.
So, we should just hardcode the @wanted_refs (or @branch_refs after the
rename) to simply ('heads'), let it be overriden by perl gitweb config
file and get rid of a new substitution from Makefile?
Something along that line, but perhaps use @additional_branch_refs
to allow users to specify additional hierarchies to be shown, and
always include 'heads' to where we currently show 'heads' hierarchy,
just like your version of format_refs_views always included 'tags'
hierarchy regardless of the setting of @wanted_refs?
Right. New patch in topic [PATCH] gitweb: Add an option for adding more
branch refs will follow.
Thanks.
gitweb/Makefile| 4 ++-
gitweb/gitweb.perl | 94
+++---
2 files changed, 72 insertions(+), 26 deletions(-)
diff --git a/gitweb/Makefile b/gitweb/Makefile
index cd194d0..361dce9 100644
--- a/gitweb/Makefile
+++ b/gitweb/Makefile
@@ -38,6 +38,7 @@ GITWEB_SITE_HTML_HEAD_STRING =
GITWEB_SITE_HEADER =
GITWEB_SITE_FOOTER =
HIGHLIGHT_BIN = highlight
+GITWEB_WANTED_REFS = heads
# include user config
-include ../config.mak.autogen
@@ -148,7 +149,8 @@ GITWEB_REPLACE = \
-e
's|++GITWEB_SITE_HTML_HEAD_STRING++|$(GITWEB_SITE_HTML_HEAD_STRING)|g' \
-e 's|++GITWEB_SITE_HEADER++|$(GITWEB_SITE_HEADER)|g' \
-e 's|++GITWEB_SITE_FOOTER++|$(GITWEB_SITE_FOOTER)|g' \
--e 's|++HIGHLIGHT_BIN++|$(HIGHLIGHT_BIN)|g'
+-e 's|++HIGHLIGHT_BIN++|$(HIGHLIGHT_BIN)|g' \
+-e 's|++GITWEB_WANTED_REFS++|$(GITWEB_WANTED_REFS)|g'
GITWEB-BUILD-OPTIONS: FORCE
@rm -f $@+
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 68c77f6..8bc9e9a 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -17,6 +17,7 @@ use Encode;
use Fcntl ':mode';
use File::Find qw();
use File::Basename qw(basename);
+use List::Util qw(min);
use Time::HiRes qw(gettimeofday tv_interval);
binmode STDOUT, ':utf8';
@@ -122,6 +123,9 @@ our $logo_label = git homepage;
# source of projects list
our $projects_list
Re: [PATCH] gitweb: Make showing branches configurable
On Fri, 2013-11-22 at 09:34 -0800, Junio C Hamano wrote:
Krzesimir Nowak krzesi...@endocode.com writes:
Running 'make GITWEB_WANTED_REFS=heads wip gitweb.cgi' will create a
gitweb CGI script showing branches that appear in refs/heads/ and in
refs/wip/. Might be useful for gerrit setups where user branches are
not stored under refs/heads/.
Signed-off-by: Krzesimir Nowak krzesi...@endocode.com
---
Notes:
I'm actually not sure if all those changes are really necessary as I
was mostly targeting it for Gerrit use. Especially I mean the changes
in git_get_remotes_list, fill_remote_heads and print_page_nav. I tried
to make it as general as it gets, so there's nothing Gerrit specific.
Thanks.
Two knee-jerk reactions after a quick scan.
- You include heads for normal builds by hardcoded
GITWEB_WANTED_REFS = heads but include tags unconditionally
by having @ref_views = (tags, @wanted_refs) in the code. Why?
Earlier both tags and heads were hardcoded there. So now instead of
heads we have @wanted_refs.
I suppose I should have given it a better name, like @branch_refs.
Right?
- Does this have be a compile-time decision? It looks like this is
something that can and should be made controllable with the
normal gitweb configuration mechanism.
Maybe. I was just looking at Makefile and saw a bunch of configuration
options there, so I just added another one. Haven't noticed the gitweb
config thing. Sorry.
So, we should just hardcode the @wanted_refs (or @branch_refs after the
rename) to simply ('heads'), let it be overriden by perl gitweb config
file and get rid of a new substitution from Makefile?
gitweb/Makefile| 4 ++-
gitweb/gitweb.perl | 94
+++---
2 files changed, 72 insertions(+), 26 deletions(-)
diff --git a/gitweb/Makefile b/gitweb/Makefile
index cd194d0..361dce9 100644
--- a/gitweb/Makefile
+++ b/gitweb/Makefile
@@ -38,6 +38,7 @@ GITWEB_SITE_HTML_HEAD_STRING =
GITWEB_SITE_HEADER =
GITWEB_SITE_FOOTER =
HIGHLIGHT_BIN = highlight
+GITWEB_WANTED_REFS = heads
# include user config
-include ../config.mak.autogen
@@ -148,7 +149,8 @@ GITWEB_REPLACE = \
-e
's|++GITWEB_SITE_HTML_HEAD_STRING++|$(GITWEB_SITE_HTML_HEAD_STRING)|g' \
-e 's|++GITWEB_SITE_HEADER++|$(GITWEB_SITE_HEADER)|g' \
-e 's|++GITWEB_SITE_FOOTER++|$(GITWEB_SITE_FOOTER)|g' \
- -e 's|++HIGHLIGHT_BIN++|$(HIGHLIGHT_BIN)|g'
+ -e 's|++HIGHLIGHT_BIN++|$(HIGHLIGHT_BIN)|g' \
+ -e 's|++GITWEB_WANTED_REFS++|$(GITWEB_WANTED_REFS)|g'
GITWEB-BUILD-OPTIONS: FORCE
@rm -f $@+
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 68c77f6..8bc9e9a 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -17,6 +17,7 @@ use Encode;
use Fcntl ':mode';
use File::Find qw();
use File::Basename qw(basename);
+use List::Util qw(min);
use Time::HiRes qw(gettimeofday tv_interval);
binmode STDOUT, ':utf8';
@@ -122,6 +123,9 @@ our $logo_label = git homepage;
# source of projects list
our $projects_list = ++GITWEB_LIST++;
+# list of directories under refs/ we want to display as branches
+our @wanted_refs = qw{++GITWEB_WANTED_REFS++};
+
# the width (in characters) of the projects list Description column
our $projects_list_description_width = 25;
@@ -632,8 +636,19 @@ sub feature_avatar {
sub check_head_link {
my ($dir) = @_;
my $headfile = $dir/HEAD;
- return ((-e $headfile) ||
- (-l $headfile readlink($headfile) =~ /^refs\/heads\//));
+
+ if (-e $headfile) {
+ return 1;
+ }
+ if (-l $headfile) {
+ my $rl = readlink($headfile);
+
+ for my $ref (@wanted_refs) {
+ return 1 if $rl =~ /^refs\/$ref\//;
+ }
+ }
+
+ return 0;
}
sub check_export_ok {
@@ -2515,6 +2530,7 @@ sub format_snapshot_links {
sub get_feed_info {
my $format = shift || 'Atom';
my %res = (action = lc($format));
+ my $matched_ref = 0;
# feed links are possible only for project views
return unless (defined $project);
@@ -2522,12 +2538,17 @@ sub get_feed_info {
# or don't have specific feed yet (so they should use generic)
return if (!$action || $action =~ /^(?:tags|heads|forks|tag|search)$/x);
- my $branch;
- # branches refs uses 'refs/heads/' prefix (fullname) to differentiate
- # from tag links; this also makes possible to detect branch links
- if ((defined $hash_base $hash_base =~ m!^refs/heads/(.*)$!) ||
- (defined $hash $hash =~ m!^refs/heads/(.*)$!)) {
- $branch = $1;
+ my $branch = undef;
+ # branches refs uses 'refs/' + $wanted_refs[x] + '/' prefix
+ # (fullname) to differentiate from tag links; this also makes
+ # possible to detect branch links
+ for my $ref (@wanted_refs
[PATCH] gitweb: Make showing branches configurable
Running 'make GITWEB_WANTED_REFS=heads wip gitweb.cgi' will create a
gitweb CGI script showing branches that appear in refs/heads/ and in
refs/wip/. Might be useful for gerrit setups where user branches are
not stored under refs/heads/.
Signed-off-by: Krzesimir Nowak krzesi...@endocode.com
---
Notes:
I'm actually not sure if all those changes are really necessary as I
was mostly targeting it for Gerrit use. Especially I mean the changes
in git_get_remotes_list, fill_remote_heads and print_page_nav. I tried
to make it as general as it gets, so there's nothing Gerrit specific.
gitweb/Makefile| 4 ++-
gitweb/gitweb.perl | 94 +++---
2 files changed, 72 insertions(+), 26 deletions(-)
diff --git a/gitweb/Makefile b/gitweb/Makefile
index cd194d0..361dce9 100644
--- a/gitweb/Makefile
+++ b/gitweb/Makefile
@@ -38,6 +38,7 @@ GITWEB_SITE_HTML_HEAD_STRING =
GITWEB_SITE_HEADER =
GITWEB_SITE_FOOTER =
HIGHLIGHT_BIN = highlight
+GITWEB_WANTED_REFS = heads
# include user config
-include ../config.mak.autogen
@@ -148,7 +149,8 @@ GITWEB_REPLACE = \
-e
's|++GITWEB_SITE_HTML_HEAD_STRING++|$(GITWEB_SITE_HTML_HEAD_STRING)|g' \
-e 's|++GITWEB_SITE_HEADER++|$(GITWEB_SITE_HEADER)|g' \
-e 's|++GITWEB_SITE_FOOTER++|$(GITWEB_SITE_FOOTER)|g' \
- -e 's|++HIGHLIGHT_BIN++|$(HIGHLIGHT_BIN)|g'
+ -e 's|++HIGHLIGHT_BIN++|$(HIGHLIGHT_BIN)|g' \
+ -e 's|++GITWEB_WANTED_REFS++|$(GITWEB_WANTED_REFS)|g'
GITWEB-BUILD-OPTIONS: FORCE
@rm -f $@+
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 68c77f6..8bc9e9a 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -17,6 +17,7 @@ use Encode;
use Fcntl ':mode';
use File::Find qw();
use File::Basename qw(basename);
+use List::Util qw(min);
use Time::HiRes qw(gettimeofday tv_interval);
binmode STDOUT, ':utf8';
@@ -122,6 +123,9 @@ our $logo_label = git homepage;
# source of projects list
our $projects_list = ++GITWEB_LIST++;
+# list of directories under refs/ we want to display as branches
+our @wanted_refs = qw{++GITWEB_WANTED_REFS++};
+
# the width (in characters) of the projects list Description column
our $projects_list_description_width = 25;
@@ -632,8 +636,19 @@ sub feature_avatar {
sub check_head_link {
my ($dir) = @_;
my $headfile = $dir/HEAD;
- return ((-e $headfile) ||
- (-l $headfile readlink($headfile) =~ /^refs\/heads\//));
+
+ if (-e $headfile) {
+ return 1;
+ }
+ if (-l $headfile) {
+ my $rl = readlink($headfile);
+
+ for my $ref (@wanted_refs) {
+ return 1 if $rl =~ /^refs\/$ref\//;
+ }
+ }
+
+ return 0;
}
sub check_export_ok {
@@ -2515,6 +2530,7 @@ sub format_snapshot_links {
sub get_feed_info {
my $format = shift || 'Atom';
my %res = (action = lc($format));
+ my $matched_ref = 0;
# feed links are possible only for project views
return unless (defined $project);
@@ -2522,12 +2538,17 @@ sub get_feed_info {
# or don't have specific feed yet (so they should use generic)
return if (!$action || $action =~ /^(?:tags|heads|forks|tag|search)$/x);
- my $branch;
- # branches refs uses 'refs/heads/' prefix (fullname) to differentiate
- # from tag links; this also makes possible to detect branch links
- if ((defined $hash_base $hash_base =~ m!^refs/heads/(.*)$!) ||
- (defined $hash $hash =~ m!^refs/heads/(.*)$!)) {
- $branch = $1;
+ my $branch = undef;
+ # branches refs uses 'refs/' + $wanted_refs[x] + '/' prefix
+ # (fullname) to differentiate from tag links; this also makes
+ # possible to detect branch links
+ for my $ref (@wanted_refs) {
+ if ((defined $hash_base $hash_base =~ m!^refs/$ref/(.*)$!) ||
+ (defined $hash $hash =~ m!^refs/$ref/(.*)$!)) {
+ $branch = $1;
+ $matched_ref = $ref;
+ last;
+ }
}
# find log type for feed description (title)
my $type = 'log';
@@ -2540,7 +2561,7 @@ sub get_feed_info {
}
$res{-title} = $type;
- $res{'hash'} = (defined $branch ? refs/heads/$branch : undef);
+ $res{'hash'} = (defined $branch ? refs/$matched_ref/$branch : undef);
$res{'file_name'} = $file_name;
return %res;
@@ -3184,24 +3205,43 @@ sub git_get_project_owner {
return $owner;
}
-sub git_get_last_activity {
- my ($path) = @_;
- my $fd;
+sub git_get_last_activity_age {
+ my ($refs) = @_;
+ my $fd = -1;
- $git_dir = $projectroot/$path;
open($fd, -|, git_cmd(), 'for-each-ref',
'--format=%(committer)',
'--sort=-committerdate',
'--count=1',
-'refs/heads
