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

net-im/slack: version bump, wrt 713094 #15543

Closed
wants to merge 2 commits into from

Conversation

SpiderX
Copy link
Contributor

@SpiderX SpiderX commented Apr 27, 2020

No description provided.

Package-Manager: Portage-2.3.89, Repoman-2.3.20
Signed-off-by: Vladimir Pavljuchenkov <spiderx@spiderx.dp.ua>
Closes: https://bugs.gentoo.org/716638
Closes: https://bugs.gentoo.org/713094
Package-Manager: Portage-2.3.89, Repoman-2.3.20
Signed-off-by: Vladimir Pavljuchenkov <spiderx@spiderx.dp.ua>
@gentoo-bot
Copy link

Pull Request assignment

Submitter: @SpiderX
Areas affected: ebuilds
Packages affected: net-im/slack

net-im/slack: @SpiderX, @gentoo/proxy-maint

Linked bugs

Bugs linked: 713094, 716638


In order to force reassignment and/or bug reference scan, please append [please reassign] to the pull request title.

Docs: Code of ConductCopyright policy (expl.) ● DevmanualGitHub PRsProxy-maint guide

@gentoo-bot gentoo-bot added self-maintained The PR changes only packages that are maintained by the submitter (i.e. no need to ask anybody else) assigned PR successfully assigned to the package maintainer(s). bug linked Bug/Closes found in footer, and cross-linked with the PR. labels Apr 27, 2020
@gentoo-repo-qa-bot
Copy link
Collaborator

Pull request CI report

Report generated at: 2020-04-27 10:01 UTC
Newest commit scanned: a5f0f2b
Status: ✅ good

There are existing issues already. Please look into the report to make sure none of them affect the packages in question:
https://qa-reports.gentoo.org/output/gentoo-ci/6440d74/output.html

@@ -82,7 +84,8 @@ src_install() {

insinto /opt/slack
doins -r usr/lib/slack/.
fperms +x /opt/slack/slack
for i in $(echo -n "${QA_PREBUILT}") ; do fperms +x "$i" ; done
use suid && fperms u+s /opt/slack/chrome-sandbox # wrt 713094
Copy link
Member

Choose a reason for hiding this comment

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

Would it do any harm if you always unconditionally just applied this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't like to have suid binaries on my system, but upstream provides them.
Personally for myself the only reason to have suid USE flag is having a possibility to unset it. On my system slack works fine without suid bit.
And the only reason why I am introducing it now is 713094. It looks like it is needed on some systems.
Another point to have it — do not make "silent bad surprise" for other users who may not like suid binaries too, especially without need.

We can remove use flag and just warn users via einfo/elog about that change for two-three ebuild bumps and remove einfo/elog after all.

Copy link
Member

@juippis juippis left a comment

Choose a reason for hiding this comment

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

I'd prefer having it done unconditionally and then maybe elog message as you said.

But no strong preference so let's go with this for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
assigned PR successfully assigned to the package maintainer(s). bug linked Bug/Closes found in footer, and cross-linked with the PR. self-maintained The PR changes only packages that are maintained by the submitter (i.e. no need to ask anybody else)
Projects
None yet
4 participants