diff mbox series

fetch2/gomod: Fix mirroring problem

Message ID 20250320065011.1560156-1-christli@axis.com
State Accepted, archived
Commit 7762cea087597019460d66b04268757bd46befdf
Headers show
Series fetch2/gomod: Fix mirroring problem | expand

Commit Message

Christian Lindeberg March 20, 2025, 6:50 a.m. UTC 
From: Christian Lindeberg <christian.lindeberg@axis.com>

Build the 'downloadfilename' parameter by replacing path separators in
the module path like the git fetcher builds the mirror tar ball name.
Copy the downloaded file in the fetcher's unpack method like the crate
fetcher instead of calling the base fetcher's unpack method.

Signed-off-by: Christian Lindeberg <christian.lindeberg@axis.com>
---
 lib/bb/fetch2/gomod.py | 35 ++++++++++++++++++++++-------------
 lib/bb/tests/fetch.py  |  9 +++++++++
 2 files changed, 31 insertions(+), 13 deletions(-)

Comments

Stefan Herbrechtsmeier March 20, 2025, 9:17 a.m. UTC | #1 
Am 20.03.2025 um 07:50 schrieb Christian Lindeberg via 
lists.openembedded.org:
> From: Christian Lindeberg <christian.lindeberg@axis.com>
>
> Build the 'downloadfilename' parameter by replacing path separators in
> the module path like the git fetcher builds the mirror tar ball name.
> Copy the downloaded file in the fetcher's unpack method like the crate
> fetcher instead of calling the base fetcher's unpack method.

What is the reason for the change?

Do we really prefer to duplicate the code from the unpack function?

Regards
   Stefan
Christian Lindeberg March 20, 2025, 9:55 a.m. UTC | #2 
On 20/03/2025 10:17, Stefan Herbrechtsmeier via lists.openembedded.org 
wrote:
> Am 20.03.2025 um 07:50 schrieb Christian Lindeberg via 
> lists.openembedded.org:
>> From: Christian Lindeberg <christian.lindeberg@axis.com>
>>
>> Build the 'downloadfilename' parameter by replacing path separators in
>> the module path like the git fetcher builds the mirror tar ball name.
>> Copy the downloaded file in the fetcher's unpack method like the crate
>> fetcher instead of calling the base fetcher's unpack method.
>
> What is the reason for the change?
>
> Do we really prefer to duplicate the code from the unpack function?
>
One could argue that it is not true duplication as it is a special case 
not supported by the base fetcher. The attempts at fixing this in the 
base fetcher seemed to have broken other things from what I read in the 
threads related to this and in the git history. So I am suggesting this 
patch as a step to get the fetcher working with respect to mirroring. 
But longer term, some refactoring might be preferred.

Regards,
Christian

> Regards
>   Stefan
>
diff mbox series

Patch

diff --git a/lib/bb/fetch2/gomod.py b/lib/bb/fetch2/gomod.py
index 6c999e8ba..933a897c7 100644
--- a/lib/bb/fetch2/gomod.py
+++ b/lib/bb/fetch2/gomod.py
@@ -107,23 +107,23 @@  class GoMod(Wget):
         if ud.path != '/':
             module += ud.path
         ud.parm['module'] = module
+        version = ud.parm['version']
 
         # Set URL and filename for wget download
-        path = escape(module + '/@v/' + ud.parm['version'])
         if ud.parm.get('mod', '0') == '1':
-            path += '.mod'
+            ext = '.mod'
         else:
-            path += '.zip'
-            ud.parm['unpack'] = '0'
+            ext = '.zip'
+        path = escape(f"{module}/@v/{version}{ext}")
         ud.url = bb.fetch2.encodeurl(
             ('https', proxy, '/' + path, None, None, None))
-        ud.parm['downloadfilename'] = path
+        ud.parm['downloadfilename'] =  f"{module.replace('/', '.')}@{version}{ext}"
 
-        ud.parm['name'] = f"{module}@{ud.parm['version']}"
+        # Set name for checksum verification
+        ud.parm['name'] = f"{module}@{version}"
 
-        # Set subdir for unpack
-        ud.parm['subdir'] = os.path.join(moddir, 'cache/download',
-                                         os.path.dirname(path))
+        # Set path for unpack
+        ud.parm['unpackpath'] = os.path.join(moddir, 'cache/download', path)
 
         super().urldata_init(ud, d)
 
@@ -131,13 +131,22 @@  class GoMod(Wget):
         """Unpack the module in the module cache."""
 
         # Unpack the module zip file or go.mod file
-        super().unpack(ud, rootdir, d)
+        unpackpath = os.path.join(rootdir, ud.parm['unpackpath'])
+        unpackdir = os.path.dirname(unpackpath)
+        bb.utils.mkdirhier(unpackdir)
+        name = os.path.basename(unpackpath)
+        cmd = f"cp {ud.localpath} {name}"
+        path = d.getVar('PATH')
+        if path:
+            cmd = f"PATH={path} {cmd}"
+        bb.note(f"Unpacking {name} to {unpackdir}/")
+        subprocess.check_call(cmd, shell=True, cwd=unpackdir,
+                              preexec_fn=subprocess_setup)
 
-        if ud.localpath.endswith('.zip'):
+        if name.endswith('.zip'):
             # Unpack the go.mod file from the zip file
             module = ud.parm['module']
-            unpackdir = os.path.join(rootdir, ud.parm['subdir'])
-            name = os.path.basename(ud.localpath).rsplit('.', 1)[0] + '.mod'
+            name = name.rsplit('.', 1)[0] + '.mod'
             bb.note(f"Unpacking {name} to {unpackdir}/")
             with zipfile.ZipFile(ud.localpath) as zf:
                 with open(os.path.join(unpackdir, name), mode='wb') as mf:
diff --git a/lib/bb/tests/fetch.py b/lib/bb/tests/fetch.py
index 0c87730c5..40c8448d2 100644
--- a/lib/bb/tests/fetch.py
+++ b/lib/bb/tests/fetch.py
@@ -3431,6 +3431,8 @@  class GoModTest(FetcherTest):
         fetcher = bb.fetch2.Fetch(urls, self.d)
         ud = fetcher.ud[urls[0]]
         self.assertEqual(ud.url, 'https://proxy.golang.org/github.com/%21azure/azure-sdk-for-go/sdk/storage/azblob/%40v/v1.0.0.zip')
+        self.assertEqual(ud.parm['downloadfilename'], 'github.com.Azure.azure-sdk-for-go.sdk.storage.azblob@v1.0.0.zip')
+        self.assertEqual(ud.parm['name'], 'github.com/Azure/azure-sdk-for-go/sdk/storage/azblob@v1.0.0')
 
         fetcher.download()
         fetcher.unpack(self.unpackdir)
@@ -3448,6 +3450,8 @@  class GoModTest(FetcherTest):
         fetcher = bb.fetch2.Fetch(urls, self.d)
         ud = fetcher.ud[urls[0]]
         self.assertEqual(ud.url, 'https://proxy.golang.org/github.com/%21azure/azure-sdk-for-go/sdk/storage/azblob/%40v/v1.0.0.mod')
+        self.assertEqual(ud.parm['downloadfilename'], 'github.com.Azure.azure-sdk-for-go.sdk.storage.azblob@v1.0.0.mod')
+        self.assertEqual(ud.parm['name'], 'github.com/Azure/azure-sdk-for-go/sdk/storage/azblob@v1.0.0')
 
         fetcher.download()
         fetcher.unpack(self.unpackdir)
@@ -3462,6 +3466,7 @@  class GoModTest(FetcherTest):
         fetcher = bb.fetch2.Fetch(urls, self.d)
         ud = fetcher.ud[urls[0]]
         self.assertEqual(ud.url, 'https://proxy.golang.org/gopkg.in/ini.v1/%40v/v1.67.0.zip')
+        self.assertEqual(ud.parm['downloadfilename'], 'gopkg.in.ini.v1@v1.67.0.zip')
         self.assertEqual(ud.parm['name'], 'gopkg.in/ini.v1@v1.67.0')
 
         fetcher.download()
@@ -3480,6 +3485,8 @@  class GoModTest(FetcherTest):
         fetcher = bb.fetch2.Fetch(urls, self.d)
         ud = fetcher.ud[urls[0]]
         self.assertEqual(ud.url, 'https://proxy.golang.org/gopkg.in/ini.v1/%40v/v1.67.0.zip')
+        self.assertEqual(ud.parm['downloadfilename'], 'gopkg.in.ini.v1@v1.67.0.zip')
+        self.assertEqual(ud.parm['name'], 'gopkg.in/ini.v1@v1.67.0')
 
         fetcher.download()
         fetcher.unpack(self.unpackdir)
@@ -3497,6 +3504,8 @@  class GoModTest(FetcherTest):
         fetcher = bb.fetch2.Fetch(urls, self.d)
         ud = fetcher.ud[urls[0]]
         self.assertEqual(ud.url, 'https://proxy.golang.org/go.opencensus.io/%40v/v0.24.0.zip')
+        self.assertEqual(ud.parm['downloadfilename'], 'go.opencensus.io@v0.24.0.zip')
+        self.assertEqual(ud.parm['name'], 'go.opencensus.io@v0.24.0')
 
         fetcher.download()
         fetcher.unpack(self.unpackdir)