Skip to content
This repository has been archived by the owner on Mar 5, 2021. It is now read-only.

Replace configuration with RPM FileFlags. #115

Merged

Conversation

talios
Copy link

@talios talios commented Nov 14, 2017

A while ago we had a need/desire to configure our config files with %noreplace, but this wasn't supported or easily exposed via the current PackageDrone library.

This patch adds the full set of RPMFILE_* file flags defined by the current RPM project, and exposes setting those FileFlags via the FileInformation class. The existing CONFIGURATION and setConfiguration / isConfiguration code has been left in, but deprecated for now.

Initially I was going to base this PR on the master branch, but it seems that doesn't match the 0.14.1 release that's available in Maven Central, so this PR is based off the 0.14.x-release branch.

CLA has been signed.

@talios talios force-pushed the expose-rpm-file-flags branch 2 times, most recently from 6fad633 to 22110bb Compare November 14, 2017 04:31
@ctron
Copy link
Contributor

ctron commented Nov 14, 2017

Thank you for this PR. This really is awesome.

I would like to amend a few things in the same PR, if that is fine with you. I would like to understand if there is any particular reason why you deprecated the "CONFIGURATION" literal? As the enum is already named "FileFlags" it seems a bit redundant to me to prefix them with "RPMFILE_". Also is the "NONE" literal not really needed as the same can be achieved with an empty EnumSet<FileFlags>. But maybe I missed something here.

I any case, thank you very much. I definitely want to get this merged soon.

@ctron ctron self-assigned this Nov 14, 2017
@talios
Copy link
Author

talios commented Nov 14, 2017

@ctron Cheers - took awhile to get some time away from normal work to look at this again.

A quick reply as I'm just heading to bed shortly. The only reason I used the RPMFILE_ prefix is that's the constant used in the RPM sources themselves, so mostly keeping a measure of familiarity/consistency but happy to drop the prefix. If we do that, I could drop the @Deprecated and also rename MISSINGOK/NOREPLACE to also mention CONFIGURATION_.

Dropping NONE probably seems wise given the use of EnumSet, initially I left it in for completeness ( likewise the SPECFILE, which based on upstream source sounds like it may be removed at some point.

I'll update the PR in the morning with those changes if they sound good to you.

Ideally I'd love to rename FileFlags to FileFlag as usually enums/classes are named in the singular ( more so now with an EnumSet<FileFlag> but renaming is a change beyond the scope of this having downstream changes.

@ctron
Copy link
Contributor

ctron commented Nov 14, 2017

Cool. Thank you that you will take care of that. I do appreciate it.

I agree with the FileFlag(s) ... But that would break the current API and I don't think this is worth it. Some things are just a bit odd 😉

This change replaces the existing means of setting a file as being
a configuration file, by exposing FileFlags modification to
SimpleFileInformationCustomizer. As part of this we also define the
full set of RPM file flags, using the naming from the upstream
RPM sources at https://github.com/rpm-software-management/rpm/blob/master/lib/rpmfiles.h

Existing behaviour is (currently) deprecated and adapted to use the
newer code.

Fixes eclipse-archived#113

Signed-off-by: Mark Derricutt <mark.derricutt@smxemail.com>
@talios talios force-pushed the expose-rpm-file-flags branch from 22110bb to 4bd1929 Compare November 14, 2017 21:54
@talios
Copy link
Author

talios commented Nov 14, 2017

Updated the PR with:

  • Removal of @Deprecated
  • Dropped RPMFILE_ prefix from FileFlags entries and also removed the NONE and SPECFILE values.
  • Added static decode method to take an int to an EnumSet<FileFlags>

Copy link
Contributor

@ctron ctron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me!

@ctron ctron merged commit 73a6095 into eclipse-archived:0.14.x-release Nov 15, 2017
@ctron
Copy link
Contributor

ctron commented Nov 15, 2017

Thank you very much for the PR. I will try to get out a release as soon as possible.

@ctron
Copy link
Contributor

ctron commented Nov 16, 2017

The release is done. As this only affects the RPM library, I only did a release on Maven Central.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants