Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix wrong ordering of files in rpm when using feature to generate intermediate directories #90

Conversation

moedan
Copy link
Contributor

@moedan moedan commented Mar 27, 2024

Hello!

First of all, I apologize for overlooking the bug in my last merge request (#86).
To fix the bug with the wrong order of the files in the RPM, I have revised the implementation of the feature to generate automatically missing intermediate directories.

As mentioned in Issue #85, the output of the predefined rpm querys are somehow missleading, because they do not reflect the order of installation. Therefore, I have updated the tests with a custom rpm query to get the correct order of installtion.

Unlike in the original implementation, I now add the missing intermediate folders of an entry to the RPM BuilderContext before the corresponding entry. So I get the correct sequence of files and directories for installation.

The new approach has the disadvantage that the order of the entries in the plugin is relevant, when using this feature.
E.g. the following Configuration will fail with Duplicate entry: ./opt/mycompany/myapp because after processing the first entry, the directory /opt/mycompany/myapp is already added to the RPM BuilderContext.
To fix this, you have to change the odering of the entries.

<generateIntermediateDirectories>
    <baseDirectory>/opt/mycompany/myapp</baseDirectory>
</generateIntermediateDirectories>
<entries>
    <!-- wrong order of entries -->
    <entry>
        <name>/opt/mycompany/myapp/myfile</name>
        <file>${project.build.directory}/myfile</file>
    </entry>
    <entry>
        <name>/opt/mycompany/myapp</name>
        <directory>true</directory>
    </entry>
</entries>

I look forward to your feedback.


Edit: As already mentioned in issue #14, the ordering of the entries has always been relevant. So it is not really a new "disadvantage".

@ctron
Copy link
Owner

ctron commented Apr 2, 2024

The new approach has the disadvantage that the order of the entries in the plugin is relevant, when using this feature.
E.g. the following Configuration will fail with Duplicate entry: ./opt/mycompany/myapp because after processing the first entry, the directory /opt/mycompany/myapp is already added to the RPM BuilderContext.
To fix this, you have to change the odering of the entries.

As you're recording the entries and tapping into the creation through the listeners. Wouldn't it be possible to detect this issue during the build time of the RPM? And fail the build? Because the output would not be a working RPM, that feels reasonable.

@moedan
Copy link
Contributor Author

moedan commented Apr 2, 2024

Your idea sounds reasonable. However, I would implement this functionality independently of the feature to generate intermediate directories, because the latter is optional.

Maybe I can outsource the part for tracking added files/directories and add an extra validator to detect incorrect builds. I'll think about it and see if it works just like that.

@ctron ctron merged commit 1331698 into ctron:master Apr 2, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants