afrog
afrog copied to clipboard
Remove requirement "lsb-core-noarch" from the RPM spec file.
Requirements for Contributing a Bug Fix
- Fill out the template below. Any pull request that does not include enough information to be reviewed in a timely manner may be closed at the maintainers' discretion.
- The pull request must only fix an existing bug. To contribute other changes, you must use a different template. You can see all templates at https://github.com/atom/.github/tree/master/.github/PULL_REQUEST_TEMPLATE.
- The pull request must update the test suite to demonstrate the changed functionality. For guidance, please see https://flight-manual.atom.io/hacking-atom/sections/writing-specs/.
- After you create the pull request, all status checks must be pass before a maintainer reviews your contribution. For more details, please see https://github.com/atom/.github/tree/master/CONTRIBUTING.md#pull-requests.
Identify the Bug
See #23560.
The dependency lsb-core-noarch
is not (properly) supported anymore by many distributions and leads to completely unnecessary and deprecated (e.g. Python 2 on openSUSE Tumbleweed) dependencies. Packages should instead define the minimal requirements they actually have.
Description of the Change
The requirement lsb-core-noarch
was removed from the RPM spec file.
Alternate Designs
See #23560 for alternative solutions.
Possible Drawbacks
A dependency of atom may not be required by the RPM anymore.
Verification Process
None, besides an educated guess that the base system requirements are already installed by default. (I could not find documentation on building the package locally, and ./script/build --create-rpm-package
failed on installing the script dependencies.)
Release Notes
Dropped the lsb-core-noarch
requirement from the RPM.
https://github.com/atom/atom/pull/6888
Oops, I was wrong. lsb_release
is called by the notifications package if there is a bug report popup, apparently.
I should have looked more closely and seen this PR they were talking about: https://github.com/atom/notifications/pull/64
It does appear it has error handling and will proceed with slightly less detailed info if you don't have lsb_release
(or if lsb_release
errors out for whatever reason).
There are apparently better-supported options than lsb_release
on RPM-based distros. For now, either moving it to Suggests:
or removing it entirely from the rpm spec seems fair to me. And I think that, long term, the best solution would be to use a different, more-appropriate command on RPM-based distros.
Oops, I was wrong.
lsb_release
is called by the notifications package if there is a bug report popup, apparently.I should have looked more closely and seen this PR they were talking about: atom/notifications#64
It does appear it has error handling and will proceed with slightly less detailed info if you don't have
lsb_release
(or iflsb_release
errors out for whatever reason).There are apparently better-supported options than
lsb_release
on RPM-based distros. For now, either moving it toSuggests:
or removing it entirely from the rpm spec seems fair to me. And I think that, long term, the best solution would be to use a different, more-appropriate command on RPM-based distros.
There is another (short-term) solution to that. As far as I know, an RPM spec can specify not only required packages, but also required files like this:
Requires: /usr/bin/lsb_release
That would remove the requirement of lsb-core-noarch
and at the same time do what should have probably been done in the first place: specify a minimal set of requirements.
openSUSE Tumbleweed has a package lsb_release
that packages thkukuk/lsb-release_os-release and has /bin/sh
as its only requirement while providing only /usr/bin/lsb_release
. I wouldn’t be surprised if other RPM-based distros have such a package as well.
If that idea is accepted, I would update the pull request accordingly.
Sounds logical and like a good idea. We've done similar for various shared object (.so
) files.
I don't have any permissions to merge things at this repo, but I'll at least comment that I support it. If it's never merged here, I would be interested in doing it at the atom-community/atom fork.
It's neat that this was merged. Thank you for that @darangi.
Looking closer, I think the author forgot to actually add /usr/bin/lsb_release
to the dependencies. Maybe it can be added now? (Or under Recommends:
instead,so it won't cause a problem if that program is no-longer available some time in the future?)
I can open a PR to do that if desired.
- DeeDeeG
@DeeDeeG yes, you can open a PR to address it. I will merge it