Bug 798051 - bugs in per repo config generation when $GL_BIG_CONFIG=1
Summary: bugs in per repo config generation when $GL_BIG_CONFIG=1
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: gitolite
Version: 16
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Lubomir Rintel
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2012-02-27 22:31 UTC by Fabrice Bellet
Modified: 2012-04-12 05:57 UTC (History)
3 users (show)

Fixed In Version: gitolite-2.3-2.el6
Clone Of:
Environment:
Last Closed: 2012-04-01 00:28:49 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)
fix for the hook update propagation pb (466 bytes, patch)
2012-03-21 21:56 UTC, Fabrice Bellet
no flags Details | Diff
dont overwrite single repo configs with @repogroups identical configs (1007 bytes, patch)
2012-03-21 22:00 UTC, Fabrice Bellet
no flags Details | Diff

Description Fabrice Bellet 2012-02-27 22:31:56 UTC
There are two problems in the way config files are generated in each repository when $GL_BIG_CONFIG is enabled (both cases also apply to newer version in rawhide):

. the first one is related to the propagation of config hooks. Here is a quick way to reproduce the problem, on a fresh F16 :

yum install gitolite
su - gitolite
gl-setup /tmp/bellet.pub

Then I tweak the config file in the following way:

$GL_BIG_CONFIG = 1;
$GL_GITCONFIG_KEYS = "hooks\.mailinglist";

git clone gitolite.122.76:gitolite-admin

I edit conf/gitolite.conf:
-----------------------
@repogroup = testing

repo    gitolite-admin
        RW+     =   bellet

repo    @repogroup
        config hooks.mailinglist = root@localhost

repo    testing
        RW+     =   @all

testing "Testing owner" = "Testing software"
------------------------

git commit -a && git push

I modify the the hooks.mailinglist value in conf/gitolite.conf

git commit -a && git push

-> the hook modification is not propagated in
   /var/lib/gitolite/repositories/testing.git/config

I remove the testing description line from conf/gitolite.conf

git commit -a && git push

-> the hook modification is propagated

The suggested fix is to force can_read() to be called in setup_gitweb_access() with this patch :
--- sitaramc-gitolite-4c1e4b2/src/gitolite.pm.orig      2012-02-27 21:59:09.699580170 +0100
+++ sitaramc-gitolite-4c1e4b2/src/gitolite.pm   2012-02-27 21:59:28.802476445 +0100
@@ -482,7 +482,8 @@
         system("git config --remove-section gitweb 2>/dev/null");
     }

-    return ($desc or can_read($repo, 'gitweb'));
+    my $cr = can_read($repo, 'gitweb');
+    return ($desc or $cr);
         # this return value is used by the caller to write to projects.list
 }


The second problem is that config order is not respected when $GL_BIG_CONFIG=1. consider this case :

-------------------------
@repogroup = testing

repo    gitolite-admin
        RW+     =   bellet

repo    @repogroup
        config hooks.mailinglist = bellet2@localhost

repo    testing
        config hooks.mailinglist = bellet3@localhost

repo    testing
        RW+     =   @all
-------------------------

I would expect that the hook specific to the 'testing' repo would have priority on the same hook for the @repogroup containing the testing repo just above. this is not the case.

This other patch fixes that (partly adapted from upstream) :

@@ -650,7 +651,10 @@
         $repos{$dr}{DELETE_IS_D} = 1 if $repos{$r}{DELETE_IS_D};
         $repos{$dr}{CREATE_IS_C} = 1 if $repos{$r}{CREATE_IS_C};
         $repos{$dr}{NAME_LIMITS} = 1 if $repos{$r}{NAME_LIMITS};
-        $git_configs{$dr} = $git_configs{$r} if $git_configs{$r};
+        # this needs to copy the key-value pairs from RHS to LHS, not just
+        # assign RHS to LHS!  However, we want to roll in '@all' configs also
+        # into the actual $repo; there's no need to preserve the distinction
+        map { $git_configs{$repo}{$_} = $git_configs{$r}{$_} if not $git_configs{$repo}{$_} } keys %{$git_configs{$r}} if $git_configs{$r};

         for my $u ('@all', "$gl_user - wild", @user_plus, keys %perm_cats) {
             my $du = $gl_user; $du = '@all' if $u eq '@all' or ($perm_cats{$u} || '') eq '@all';

the idea is to collect the config key-values from the 'repo @repogroup' section when they don't overwrite existing config key-value for the same key in the 'repo testing' section. It doesn't resolve the ordering problem when the repo is part of several groups, but at least it gives a priority to the configuration of a single repo vs. the configuration of a group of repos.

Comment 1 Gwyn Ciesla 2012-02-28 15:49:50 UTC
Excellent, thank you.  Can you attach the patches to the BZ to make sure I get the formatting right?

Comment 2 Fabrice Bellet 2012-03-21 21:56:57 UTC
Created attachment 571876 [details]
fix for the hook update propagation pb

Comment 3 Fabrice Bellet 2012-03-21 22:00:01 UTC
Created attachment 571877 [details]
dont overwrite single repo configs with @repogroups identical configs

Comment 4 Gwyn Ciesla 2012-03-22 13:55:46 UTC
I see these are present in 2.3.  I think I'll just update f17, f16, and el6 to that.

Comment 5 Fedora Update System 2012-03-22 14:09:52 UTC
gitolite-2.3-1.fc17 has been submitted as an update for Fedora 17.
https://admin.fedoraproject.org/updates/gitolite-2.3-1.fc17

Comment 6 Fedora Update System 2012-03-22 14:09:59 UTC
gitolite-2.3-1.fc16 has been submitted as an update for Fedora 16.
https://admin.fedoraproject.org/updates/gitolite-2.3-1.fc16

Comment 7 Fedora Update System 2012-03-22 14:10:07 UTC
gitolite-2.3-1.el6 has been submitted as an update for Fedora EPEL 6.
https://admin.fedoraproject.org/updates/gitolite-2.3-1.el6

Comment 8 Fabrice Bellet 2012-03-22 21:02:03 UTC
I tested gitolite-2.3-1.fc16. The first issue is still there, and the patch from comment #c2 still works. The second issue has disappeared with this new upstream version : now the config hooks overrides are solely based on their order in the config file.

Comment 9 Gwyn Ciesla 2012-03-23 13:26:04 UTC
Whoops, good catch.  I'll apply that patch and re-update.

Comment 10 Fedora Update System 2012-03-23 13:40:27 UTC
gitolite-2.3-2.fc16 has been submitted as an update for Fedora 16.
https://admin.fedoraproject.org/updates/gitolite-2.3-2.fc16

Comment 11 Fedora Update System 2012-03-23 13:40:38 UTC
gitolite-2.3-2.fc17 has been submitted as an update for Fedora 17.
https://admin.fedoraproject.org/updates/gitolite-2.3-2.fc17

Comment 12 Fedora Update System 2012-03-23 13:40:48 UTC
gitolite-2.3-2.el6 has been submitted as an update for Fedora EPEL 6.
https://admin.fedoraproject.org/updates/gitolite-2.3-2.el6

Comment 13 Fedora Update System 2012-03-23 17:11:48 UTC
Package gitolite-2.3-2.fc17:
* should fix your issue,
* was pushed to the Fedora 17 testing repository,
* should be available at your local mirror within two days.
Update it with:
# su -c 'yum update --enablerepo=updates-testing gitolite-2.3-2.fc17'
as soon as you are able to.
Please go to the following url:
https://admin.fedoraproject.org/updates/FEDORA-2012-4564/gitolite-2.3-2.fc17
then log in and leave karma (feedback).

Comment 14 Fabrice Bellet 2012-03-26 14:07:25 UTC
I tested on Fedora 16, and it works for me. Thanks!

Comment 15 Fedora Update System 2012-04-01 00:28:49 UTC
gitolite-2.3-2.fc16 has been pushed to the Fedora 16 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 16 Fedora Update System 2012-04-12 03:10:55 UTC
gitolite-2.3-2.fc17 has been pushed to the Fedora 17 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 17 Fedora Update System 2012-04-12 05:57:48 UTC
gitolite-2.3-2.el6 has been pushed to the Fedora EPEL 6 stable repository.  If problems still persist, please make note of it in this bug report.


Note You need to log in before you can comment on or make changes to this bug.