Issue #326: Namespace must be stripped during format git config items - rpkg

rpkg

#326 Namespace must be stripped during format git config items

Closed: Fixed 6 years ago Opened 6 years ago by cqi.

This fixes rpkg issue #230 - Cloning from "rpms/foo" produces different git config than when cloning from "foo".

@cqi, are you thinking as simple as just passing base_module to _clone_config(), like:

diff --git i/pyrpkg/__init__.py w/pyrpkg/__init__.py
index 178e6df..01f8871 100644
--- i/pyrpkg/__init__.py
+++ w/pyrpkg/__init__.py
@@ -1270,7 +1270,7 @@ class Commands(object):
             base_module = self.get_base_module(module)
             git_dir = target if target else bare_dir if bare_dir else base_module
             conf_git = git.Git(os.path.join(path, git_dir))
-            self._clone_config(conf_git, module)
+            self._clone_config(conf_git, base_module)

         return

That would work for the current config, but if there is ever a part of clone_config which wants to use the full module with namespace it would need further adjustment, perhaps by passing both base_module and module to the dict used in _clone_config():

diff --git i/pyrpkg/__init__.py w/pyrpkg/__init__.py
index 178e6df..024561a 100644
--- i/pyrpkg/__init__.py
+++ w/pyrpkg/__init__.py
@@ -1348,7 +1348,9 @@ class Commands(object):
         shutil.rmtree(repo_path, ignore_errors=True)

     def _clone_config(self, conf_git, module):
-        clone_config = self.clone_config.strip() % {'module': module}
+        base_module = self.get_base_module(module)
+        clone_config = self.clone_config.strip() % {
+            'base_module': base_module, 'module': module}
         for confline in clone_config.splitlines():
             if confline:
                 conf_git.config(*confline.split())

and then adjusting the clone_config setting in fedpkg.conf.

Perhaps a better step toward that which leaves the door open to extend clone_config to allowing both base_module and module would be to call get_base_module() in _clone_config() to override module, if it seems unlikely that we'd want to have the full module in clone_config. As in:

diff --git i/pyrpkg/__init__.py w/pyrpkg/__init__.py
index 178e6df..f49916e 100644
--- i/pyrpkg/__init__.py
+++ w/pyrpkg/__init__.py
@@ -1348,6 +1348,7 @@ class Commands(object):
         shutil.rmtree(repo_path, ignore_errors=True)

     def _clone_config(self, conf_git, module):
+        module = self.get_base_module(module)
         clone_config = self.clone_config.strip() % {'module': module}
         for confline in clone_config.splitlines():
             if confline:

The first and last diffs should be functionally equivalent, I believe. The latter diff adjusts more clearly to the second diff, if/when the need arises.

To be clear, I haven't tested any of these yet. They're just my first thoughts after a quick look at the code this afternoon when @tibbs pointed out the issue.

(I also noticed that a few changes have been pushed since I fetched earlier today, so the diff's don't apply as-is. That's trivial to correct, of course. If any of these routes look like what you have in mind, I can work on submitting them properly.)

Maybe there's a much simpler and/or elegant fix you have in mind. :)