rpmvenv icon indicating copy to clipboard operation
rpmvenv copied to clipboard

Why some absolute paths have double slashes?

Open bialix opened this issue 2 years ago • 1 comments

I've noticed that some of absolute paths in generated spec and shebangs have doubled slash, e.g. I have files section with config and spec shows:

%config(noreplace) //lib/systemd/system/my-app.service
%config(noreplace) //etc/my-app/.env

Also shebang of my script /usr/bin/python has changed to #!/opt/my-company//my-app/bin/python.

Probably that's not problem but my daemon in the output of top then shows this double slashed path, which I don't like honestly to say.

... /opt/my-company//my-app/bin/python /opt/my-company/my-app/bin/my-server.py

Also, due to this problem I have to set parent directory in rpmvenv.json without first slash, e.g.

    "python_venv": {
        "name": "my-app",
        "path": "opt/my-company/",
        "pip_flags": "--no-warn-script-location"
    },

I'd like to fix this if you provide some hints. I suppose all paths need to be processed via os.path.normpath.

bialix avatar May 06 '22 11:05 bialix

I appreciate that you're interested in fixing this issue. It's a complicated issue, though.

I understand that the double slashes look odd but they are not a bug. The POSIX standard allows multiple slashes. They are equivalent to a single slash. The only exception to this is when two slashes appear at the beginning of a path. A double slash at the beginning has no standard meaning and is left to the implementation. Linux always treats double slashes at the beginning of a path as a single slash. So while the paths may look wrong, they are considered correct and acceptable paths.

The main reason why this happens in rpmvenv is because it is a convenient and safe way to join two path segments when the code does not know the contents of one or both of the paths being joined. The issue is that you can't run os.path.normpath because most of the rpmvenv code that handles paths operates on RPM macros and does not actually have access to the whole path. For example, https://github.com/kevinconway/rpmvenv/blob/master/rpmvenv/extensions/python/venv.py#L107. Here the code must ensure that the virtualenv path is set to a child of the buildroot. At this point, the code knows that there is a %{buildroot} macro because it is built-in to RPM but it does not know the contents of that macro. It may end in a slash already or may not have a trailing slash but there is no way to know until rpm build determines the exact value of the macro. The default RPM value and the default rpmvenv value for buildroot do not end in a slash but a user may enter any path as a buildroot. Additionally, an RPM plugin could inject its own value for that macro and there is no way to know what that is until rpm build. The easy option is to always add another slash because it cannot cause an issue. This practice of always adding a slash when joining paths or when ensuring that a path is absolute means that the system works with any kind of path given by the user for any configuration value and for any externally injected macro value such as those from an RPM plugin.

Admittedly, it is rare that an RPM plugin would do this kind of injection. There is only one case that I know of and it is handled here. If we assume that no plugins are injecting path values then the code could avoid using macros to join paths and instead use Python variables and pathlib to manage everything. This would be a significant refactor of the code but achieve your goal of getting cleaner path strings. So long as we still write all the same macros to the spec file then this refactor should be possible without breaking backwards compatibility.

Personally, I think the "always add a slash" method works as expected and without the need to normalize all paths. The only downside is that it looks strange. I'm hesitant to change the current behavior because it would be a fairly big change to the code just to make the generated paths look better. However, If you can demonstrate a scenario where the double slashes cause a bug when building or installing an RPM then I may be persuaded to consider a change.

kevinconway avatar May 09 '22 02:05 kevinconway