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

Specify %attr directive on a per-file basis #719

Merged
merged 1 commit into from Jul 19, 2014

Conversation

Projects
None yet
4 participants
@leslieianson

leslieianson commented Jun 16, 2014

Please consider this pull request as a naive proof of concept for a new proposed command line argument --rpm-attr.

We really like fpm and are using it for building rpm packages but have hit a problem where we'd like to be able to specify file specific attributes on a per-file basis.

We note that the rpm %defattr(<filemode>, <user>, <group>, <dirmode>) directive is managed in fpm via the --rpm-user user, --rpm-group group, --rpm-defattrfile mode and --rpm-defattrdir mode command line arguments.

So we'd like to also be able to specify the %attr(<mode>, <user>, <group>) file directive overriding %defattr on a per-file basis but cannot find fpm command line arguments to handle this.

One use case for this is where the majority of files and directories within an rpm are owned by an application user but the init.d script in /etc/rc.d/init.d/appname should be owned by root and not by the application user.

We note that this could be done with the fpm command line argument --rpm-use-file-permissions but we are not keen on this approach as it would require our build system to run as root in order to call chown/chgrp before invoking fpm.

We are currently working around this by calling chown and chmod from the --after-install script but we consider this a hack given rpm should manage this itself.

Proposed feature request

Add support for new command line argument which can be called multiple times, e.g.

--rpm-attr '0755,root,root:/etc/rc.d/init.d/appname

@leslieianson leslieianson changed the title from naive implementation of rpm %attr directive to Specify %attr directive on a per-file basis Jun 16, 2014

@jordansissel

This comment has been minimized.

Owner

jordansissel commented Jun 16, 2014

I wonder if we could expand this to generally allow setting file permissions for any package?

Something like:

--permissions 'path=user,group,mode'

Maybe? I'm open to discussion on the flag name and format of the arguments.

@leslieianson

This comment has been minimized.

leslieianson commented Jun 17, 2014

Hello Jordan,

Thank you for your reply.

We really like your proposal however, due to time constraints we can only really concentrate on rpms.

We've already got some tidier code (including a test) in progress for;

--rpm-permissions 'path=user,group,mode'

so expect to see something soon.

@jordansissel

This comment has been minimized.

Owner

jordansissel commented Jun 17, 2014

Happy to meet you in the middle! Totally understanding limited time and energy available.

I'm happy to accept and merge the --rpm-permissions code as you propose and we (the fpm community) can work on making a more general --permissions flag work for all package types!

Thanks for your continued work on this feature. Let me know when you want me to take another look at this. Very good feature :)

@dhajoshi

This comment has been minimized.

dhajoshi commented Jun 19, 2014

this is exactly i was also looking for, thanks for writing it.. :)

jordansissel added a commit that referenced this pull request Jul 19, 2014

Merge pull request #719 from leslieianson/master
Specify %attr directive on a per-file basis

@jordansissel jordansissel merged commit e868c34 into jordansissel:master Jul 19, 2014

prof-milki pushed a commit to prof-milki/xpm that referenced this pull request Dec 18, 2014

jls
Merge pull request jordansissel#719 from leslieianson/master
Specify %attr directive on a per-file basis

prof-milki pushed a commit to prof-milki/xpm that referenced this pull request Dec 27, 2014

Merge pull request jordansissel#719 from leslieianson/master
Specify %attr directive on a per-file basis
@clayne

This comment has been minimized.

clayne commented on lib/fpm/package/rpm.rb in 0063eb6 Apr 22, 2015

This section of code was confusing and repetitive when reading it at first to try and figure out what was going on. I vote this to be a cleaner approach:

# If attributes provided, use those, otherwise if rpm-use-file-permissions is false, use rpm defaults.
if !attrs[file].nil?
  return sprintf("%%attr(%s) %s\n", attrs[file], file)
elsif !attributes[:rpm_use_file_permissions?]
  return file
end

Then kill line 148 and let it fall through to the rpm-use-file-permissions true case. Since the attribute check is winning in all cases if it's non-nil, there's no reason to be checking it twice. It's precedence is higher in the overall logic regardless of rpm-use-file-permissions being true or not.

jordansissel added a commit that referenced this pull request Apr 24, 2015

Merge pull request #719 from leslieianson/master
Specify %attr directive on a per-file basis

jordansissel added a commit that referenced this pull request Jun 20, 2016

Merge pull request #719 from leslieianson/master
Specify %attr directive on a per-file basis
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment