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

add rpm_attributes[] to be able to specify per file attributes #120

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

glensc
Copy link
Contributor

@glensc glensc commented Aug 26, 2015

this is to add impelementation for #119

to use:

    rpm_attributes['/etc/myapp'] = '751,root,http'

as the .attrs seems to be rpm-only, prefixed it with rpm_:

$ fpm --help|grep attr
    --rpm-defattrfile ATTR        (rpm only) Set the default file mode (%defattr). (default: "-")
    --rpm-defattrdir ATTR         (rpm only) Set the default dir mode (%defattr). (default: "-")
    --rpm-attr ATTRFILE           (rpm only) Set the attribute for a file (%attr).

glensc added a commit to pld-linux/fpm-cookery that referenced this pull request Aug 26, 2015
@beddari
Copy link
Contributor

beddari commented Aug 26, 2015

Hm, not tested myself, but I guess there is a reason you can't simply use something like this?

fpm_attributes[:rpm_attr] = [ ... array ... ]

@glensc
Copy link
Contributor Author

glensc commented Aug 26, 2015

fpm_attributes[:rpm_attr] gets proxied to fpm.attributes, but need to proxy to fpm.attrs different objects.

@beddari
Copy link
Contributor

beddari commented Aug 26, 2015

Thanks, I see now, attrs is defined at https://github.com/jordansissel/fpm/blob/master/lib/fpm/package.rb#L112-L114 ... and was added with this commit jordansissel/fpm@0063eb6

Seems a bit strange and perhaps redundant ... (to me, at this point hehe). I'd very much like the functionality you are proposing, though.

@glensc
Copy link
Contributor Author

glensc commented Aug 27, 2015

as for fpm cookery side, i would extend cookery dsl to support adding dirs and files in one run:

instead of:

    directories << '/etc/app'
    rpm_attributes['/etc/app'] = '751,root,http'

do:

    directories << '/etc/app' => '751,root,http', 
                   '/srv/app' => '751,root,http', 

yet my Ruby skills aren't that good to do it one run :)

@beddari
Copy link
Contributor

beddari commented Aug 27, 2015

Well, same here, limited Ruby skills, but from what I can tell the code that introduced the feature in fpm wasn't clean and that is the sole reason fpm.attrs exist at all? ;-)

If that was cleaned up perhaps we could still use fpm_attributes and not add a rpm specific feature to cookery

@bernd
Copy link
Owner

bernd commented Aug 30, 2015

Thank you for the PR and the conversation. I will think about a way to abstract this a bit more and if we can make this work on DEB platforms as well. (i.e. by generating post-script content to set the permissions or something like that)

@boc-tothefuture
Copy link

I am struggling with the same issue with perms/owner/group. I agree with @bernd would be nice to keep this generic.

@beddari
Copy link
Contributor

beddari commented Sep 4, 2015

Would any of you be able to verify that this could be 'fixed' in fpm? I think if we get that cleaned up (check the code I referred above) we'll be able to use fpm_attributes as-is?

@boc-tothefuture
Copy link

@bernd If we can provide some syntactic sugar to set permissions/file attributes before fpm is called would that solve the problem generically? Would post script be needed for deb?

@bernd
Copy link
Owner

bernd commented Sep 10, 2015

@bernd If we can provide some syntactic sugar to set permissions/file attributes before fpm is called would that solve the problem generically? Would post script be needed for deb?

@Brian-OConnell The problem is that you cannot really set permissions before running fpm because the user might not exist on the build system yet. That's why lots of deb pre/post install scripts contain that kind of logic.

@beddari
Copy link
Contributor

beddari commented Sep 10, 2015

Yes, scripting is the only way to do this for deb. Permissions always come back as bothersome when dealing with multiple package formats. I'm voting for a new abstraction :) Not quite sure how it would look like, though.

@bernd bernd changed the base branch from master to main July 13, 2022 18:06
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.

4 participants