openSUSE / obs-service-go_modules

OBS Source Service to download, verify, and vendor Go module dependency sources
GNU General Public License v2.0
19 stars 18 forks source link

Support archives specified with basename param where files are in top level #36

Open jfkw opened 1 year ago

jfkw commented 1 year ago

When the basename option is specified with an empty value it is ignored and the default autodetection mechanism is used to look for a tarball. This issue is created to discuss the proposed change in PR #35.

This case happens when the application archive does not contain a basename at all. I.e., there isn't any folder in its root containing the rest of files:

$ mkdir testfolder
$ tar xz test.tgz -C testfolder
$ ls testfolder
file1
file2

The PR also identifies a couple of typos in related log lines:

INFO:obs-service-go_modules:Detected basename /path/to/archive.tgz form archive name
ERROR:obs-service-go_modules:Invalid path: {ret} not subdir of {basedir}
jfkw commented 1 year ago

Thanks @marcosbc for surfacing this issue. We will address this by proper handling of archives in the layout you describe, or with a better error message for an empty basename argument.

If the user needs to rely on an basename argument specified but empty to control behaviour, that would be a misfeature we should address in some way that is more clear and the user intent explicit.

Can you help me understand the issue better by describing the file name and layout location of a tarball containing Go code that you are trying to solve? Please include the location of go.mod.

marcosbc commented 1 year ago

You can find a couple of examples here with the Ingress Controller and Dashboard components. In the first case, the go.mod is in the archive root and in the second case, in the api subfolder (but there are more files/folders in the archive root).

Please also note that, in both cases, it is discouraged to download the source code from GitHub since they consider it a non-official release site (example), so the only official tarballs are the ones provided in the download site.

SchoolGuy commented 1 year ago

Another example of this would be https://github.com/openSUSE-zh/node-semver. I think it is quite common to have your go.mod in the repository/archive root.

SchoolGuy commented 1 year ago

We could use default=argparse.SUPPRESS for the argument and use hasattr to check if the flag is given or not. That should do the trick in case we want to separate between empty and not given: https://stackoverflow.com/a/30491369

Edit: Writing this because this way we could stay backwards compatible.

marcosbc commented 1 year ago

@SchoolGuy The issue mainly comes when the compressed archive does not include a basename. In the case of the application you mention, even though the go.mod is in the repository root, when you download the archive from GitHub, it will be inside the node-semver-master folder:

$ curl -sL https://github.com/openSUSE-zh/node-semver/archive/refs/heads/master.tar.gz | tar tz | grep go.mod
node-semver-master/go.mod

However, there are other cases where this does not happen:

$ curl -s https://archive.apache.org/dist/apisix/ingress-controller/1.6.0/apache-apisix-ingress-controller-1.6.0-src.tgz | tar tz | grep go.mod
./go.mod

And those cases are the ones that do not work using go_modules.

SchoolGuy commented 1 year ago

@marcosbc Ah it seems after your hint I was able to get my _service file fixed. Sorry for the noise then.