rpm-builder icon indicating copy to clipboard operation
rpm-builder copied to clipboard

Rules are not applied to implicit created directories

Open moedan opened this issue 2 years ago • 8 comments

Hello,

I have noticed that rules are not applied to implicitly created directories. For example, when a file is created in a target directory that does not yet exist, no rules are applied to these directories. For explicitly created directories, the rules are applied (see example below).

I am not sure if this is a bug or the expected behavior. I would have expected that the rules are applied to all applicable intermediate directories in a path as well.

Can you please clarify that? Thanks in advance!


Example

To reproduce the described problem, I have attached a simplified MWE (Used rpm-bilder-Plugin 1.11.0): mwe.zip

Ruleset:

<ruleset>
    <id>my-default</id>
    <rules>
        <rule>
            <when>
                <prefix>/opt/mycompany</prefix>
            </when>
            <user>myuser</user>
            <group>mygroup</group>
        </rule>
        <rule>
            <when>
                <prefix>/opt/mycompany</prefix>
                <type>directory</type>
            </when>
            <mode>0777</mode>
        </rule>
    </rules>
</ruleset>

Entries:

<entries>
    <entry>
        <!-- implicitly create directories "mycompany/mwe/a/b/c/" in "/opt" -->
        <name>/opt/mycompany/mwe/a/b/c/someFile.txt</name>
        <file>${project.basedir}/src/main/resources/someFile.txt</file>
        <ruleset>my-default</ruleset>
    </entry>
    <entry>
        <!-- explicitly create directory "mwe/" in "/opt/mycompany" -->
        <name>/opt/mycompany/mwe</name>
        <directory>true</directory>
        <ruleset>my-default</ruleset>
    </entry>
</entries>

Output of created files/directroies with permissions after installation of built rpm from MWE:

[/]$ ls -alpR /opt
/opt:
total 20
drwxr-xr-x. 1 root root 4096 Nov 29 10:28 ./
dr-xr-xr-x. 1 root root 4096 Nov 29 10:28 ../
drwxr-xr-x. 3 root root 4096 Nov 29 10:28 mycompany/

/opt/mycompany:
total 16
drwxr-xr-x. 3 root   root    4096 Nov 29 10:28 ./
drwxr-xr-x. 1 root   root    4096 Nov 29 10:28 ../
drwxrwxrwx. 3 myuser mygroup 4096 Nov 29 10:14 mwe/

/opt/mycompany/mwe:
total 12
drwxrwxrwx. 3 myuser mygroup 4096 Nov 29 10:14 ./
drwxr-xr-x. 3 root   root    4096 Nov 29 10:28 ../
drwxr-xr-x. 3 root   root    4096 Nov 29 10:28 a/

/opt/mycompany/mwe/a:
total 12
drwxr-xr-x. 3 root   root    4096 Nov 29 10:28 ./
drwxrwxrwx. 3 myuser mygroup 4096 Nov 29 10:14 ../
drwxr-xr-x. 3 root   root    4096 Nov 29 10:28 b/

/opt/mycompany/mwe/a/b:
total 12
drwxr-xr-x. 3 root root 4096 Nov 29 10:28 ./
drwxr-xr-x. 3 root root 4096 Nov 29 10:28 ../
drwxr-xr-x. 2 root root 4096 Nov 29 10:28 c/

/opt/mycompany/mwe/a/b/c:
total 12
drwxr-xr-x. 2 root   root    4096 Nov 29 10:28 ./
drwxr-xr-x. 3 root   root    4096 Nov 29 10:28 ../
-rw-r--r--. 1 myuser mygroup   11 Nov 29 10:14 someFile.txt

Expected output of created files/directroies:

[/]$ ls -alpR /opt
/opt:
total 20
drwxr-xr-x. 1 root   root    4096 Nov 29 10:28 ./
dr-xr-xr-x. 1 root   root    4096 Nov 29 10:28 ../
drwxrwxrwx. 3 myuser mygroup 4096 Nov 29 10:28 mycompany/

/opt/mycompany:
total 16
drwxrwxrwx. 3 myuser mygroup 4096 Nov 29 10:28 ./
drwxr-xr-x. 1 root   root    4096 Nov 29 10:28 ../
drwxrwxrwx. 3 myuser mygroup 4096 Nov 29 10:14 mwe/

/opt/mycompany/mwe:
total 12
drwxrwxrwx. 3 myuser mygroup 4096 Nov 29 10:14 ./
drwxrwxrwx. 3 myuser mygroup 4096 Nov 29 10:28 ../
drwxrwxrwx. 3 myuser mygroup 4096 Nov 29 10:28 a/

/opt/mycompany/mwe/a:
total 12
drwxrwxrwx. 3 myuser mygroup 4096 Nov 29 10:28 ./
drwxrwxrwx. 3 myuser mygroup 4096 Nov 29 10:14 ../
drwxrwxrwx. 3 myuser mygroup 4096 Nov 29 10:28 b/

/opt/mycompany/mwe/a/b:
total 12
drwxrwxrwx. 3 myuser mygroup 4096 Nov 29 10:28 ./
drwxrwxrwx. 3 myuser mygroup 4096 Nov 29 10:28 ../
drwxrwxrwx. 3 myuser mygroup 4096 Nov 29 10:28 c/

/opt/mycompany/mwe/a/b/c:
total 12
drwxrwxrwx. 3 myuser mygroup 4096 Nov 29 10:28 ./
drwxrwxrwx. 3 myuser mygroup 4096 Nov 29 10:28 ../
-rw-r--r--. 1 myuser mygroup   11 Nov 29 10:14 someFile.txt

moedan avatar Nov 29 '23 11:11 moedan

I am not sure if this is a bug or the expected behavior. I would have expected that the rules are applied to all applicable intermediate directories in a path as well.

Indeed, that feels like a bug.

ctron avatar Nov 29 '23 12:11 ctron

Thank you for your feedback!

I thought about the problem and played around a bit.

I have no experience in packaging RPMs, but from my point of view, the following points should be considered for fixing the error:

  • Intermediate directories may only be added once all entries have been processed.
  • Only the rules specified in the entry may be applied to the intermediate directories, but not the explicit information of the entry.
  • The order of the entries must be taken into account. E.g.: An intermediate directory from a second entry may not overwrite the intermediate directory from the first entry.
  • Intermediate directories that are part of the defined prefixes should not be added.

Taking these aspects into account, I created a pull request as a first draft. This could be a starting point for a potential fix of the problem. I have not adapted the integration tests. Currently, some of them fail because the implicit intermediate folders are now part of the output and the hash sums are no longer correct.

At the moment, my bugfix would be a breaking change (Prefixes should be configured correctly and not ignored). If a breaking change is not an option, perhaps an extra flag should be added for the feature of generating intermediate directories automatically.

Sample-Output of rpm generated by mwe:

$ rpm -q --dump -p mwe.rpm
/opt/mycompany/mwe 0 1702461307 0000000000000000000000000000000000000000000000000000000000000000 040777 myuser mygroup 0 0 0 X
/opt/mycompany/mwe/a 0 1702461307 0000000000000000000000000000000000000000000000000000000000000000 040777 myuser mygroup 0 0 0 X
/opt/mycompany/mwe/a/b 0 1702461307 0000000000000000000000000000000000000000000000000000000000000000 040777 myuser mygroup 0 0 0 X
/opt/mycompany/mwe/a/b/c 0 1702461307 0000000000000000000000000000000000000000000000000000000000000000 040777 myuser mygroup 0 0 0 X
/opt/mycompany/mwe/a/b/c/someFile.txt 11 1702461307 6f6b40bbcd7bf26ef883167642d8117b58973c145da7d7818173a282eda0903c 0100644 myuser mygroup 0 0 0 X

moedan avatar Dec 13 '23 11:12 moedan

That looks awesome. I think those four assumptions are correct. Especially the last one, directories like /usr should not end up on the RPM. Maybe directories should only be processed starting with point where the scanner starts.

Changes of hashes are expected when the tool changes. And a breaking change would also work for me. But as always when adding some new feature, it might good to have a way to disable it.

ctron avatar Dec 13 '23 12:12 ctron

At the moment I (mis)use the prefixes property to exclude directories like /usr. If this has unwanted side effects, we'll have to think of a new property.

In the current version I've added the new configuration property 'generateIntermediateDirectories' to disable the feature. At default, the property is enabled. I would be happy to receive suggestions for improving the naming and description of this property. The same applies to the source code.

moedan avatar Dec 13 '23 14:12 moedan

Are there any news on this topic? Have you had time to take a closer look at my bugfix?

moedan avatar Jan 10 '24 07:01 moedan

Hello, I also look for the changes to this maven plugin. So I am also interested in this issue and pull request. What is the actual status, what is missing to get this feature merged?

RolandRodenbeck avatar Feb 20 '24 17:02 RolandRodenbeck

Sorry I lost track of the PR. Thanks @RolandRodenbeck for pinging!

ctron avatar Feb 21 '24 07:02 ctron

Many thanks for the quick release!

Unfortunately, I encountered an error when trying to install the generated RPM package using the new feature.

$ dnf install example.rpm
...
error: unpacking of archive failed on file /etc/mycompany/myapp: cpio: mkdir
error: example.noarch: install failed

It looks like the order of unpacking files with cpio depends on the inode index. The inode index is set and incremented when adding entries to the RpmBuilder. Regretfully, I did not take this into account when adding the intermediate folders. In the current implementation I'm adding the missing directories after all entries have been processed. This then leads to the error mentioned above. :-/

The output of the predefined rpm query commands like rpm -q --dump example.rpm was misleading here, as it is sorted automatically by path(?).

The output of the command rpm -q --queryformat "[%2{FILEINODES} %{FILENAMES}\n]" example.rpm | sort -n displays the ordering correctly.

I'll think about the problem again and hope that I can come up with a better solution.

moedan avatar Mar 06 '24 17:03 moedan

I think the ticket can be closed as the main problem has been fixed.

Unfortunately, I have not yet found time for the improvement mentioned in the PR to make the build process fail if an invalid file structure is used (wrong ordering of added files / directories). I haven't forgotten about it, and it's still on my list.

moedan avatar Aug 12 '24 13:08 moedan