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.

Sign up for GitHub

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

Jump to bottom

Fix shebang error on rpmbuild #695

Closed
wants to merge 1 commit into from
Closed

Fix shebang error on rpmbuild #695

wants to merge 1 commit into from

Conversation

tuxmaster5000
Copy link
Contributor

When building the rpm package, many errors/warnings about an wrong shebang is shown.

@arogge
Copy link
Member

arogge commented Jan 14, 2021

There is a problem and I think your patch will fix it for RH/Fedora and probably also other Linuxes.
However, I'm quite sure #!/usr/bin/sh and #!/usr/bin/perl will break on at least one of the "classic" unix systems.

We will have to look into this a bit more.
What warnings does rpmbuild yield in your environment?

@tuxmaster5000
Copy link
Contributor Author

Hi Andreas,
I get many messages about "WARNING: mangling shebang" after the %file section is called.
It think it comes form this change in the RHEL/Fedora world:
https://pagure.io/packaging-committee/issue/738#comment-490366
It will happens on any bash and python scripts

@pstorz pstorz added the nobuild label Jan 19, 2021
arogge
arogge requested changes Jan 25, 2021
View reviewed changes
Copy link
Member

@arogge arogge left a comment

Choose a reason for hiding this comment

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

Your changes make sense for Red Hat / Fedora, but they break portability. We can probably implement your changes in a system-specific way, but it will be a lot harder to do so.

core/platforms/redhat/bareos-dir.in
@@ -1,4 +1,4 @@
#! /bin/sh
#!/usr/bin/sh
Copy link
Member

Choose a reason for hiding this comment

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

As we dropped support for RHEL6, we should probably remove the old initscripts instead of maintaining them further.
Could you remove these files and see if the build still works? They shouldn't be installed when building with systemd anyway.

core/platforms/redhat/bareos-fd.in
@@ -1,4 +1,4 @@
#! /bin/sh
#!/usr/bin/sh
Copy link
Member

Choose a reason for hiding this comment

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

see above

core/platforms/redhat/bareos-sd.in
@@ -1,4 +1,4 @@
#! /bin/sh
#!/usr/bin/sh
Copy link
Member

Choose a reason for hiding this comment

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

see above

core/scripts/bareos-config-lib.sh.in
@@ -1,3 +1,4 @@
#!/usr/bin/sh
Copy link
Member

Choose a reason for hiding this comment

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

the resulting file should not be executable. So we shouldn't add a shebang, but remove exectuable permissions (this can be done somewhere in a CMakeLists.txt)

core/scripts/bareos-config.in
@@ -1,4 +1,4 @@
#!/bin/sh
#!/usr/bin/sh
Copy link
Member

Choose a reason for hiding this comment

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

Sorry, but /usr/bin/sh is Red Hat / Fedora specific. In POSIX the path to the standard shell is /bin/sh. This change would break everything, but Red Hat / Fedora.
You can provide a patch that applies this change for Fedora/Red Hat only or you can change the specfile to ignore silence the warning using __brp_mangle_shebangs_exclude.

core/src/cats/drop_bareos_tables.in
@@ -1,4 +1,4 @@
#!/bin/sh
#!/usr/bin/sh
Copy link
Member

Choose a reason for hiding this comment

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

see above

core/src/cats/grant_bareos_privileges.in
@@ -1,4 +1,4 @@
#!/bin/sh
#!/usr/bin/sh
Copy link
Member

Choose a reason for hiding this comment

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

see above

core/src/cats/make_bareos_tables.in
@@ -1,4 +1,4 @@
#!/bin/sh
#!/usr/bin/sh
Copy link
Member

Choose a reason for hiding this comment

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

see above

core/src/cats/make_catalog_backup.in
@@ -1,4 +1,4 @@
#!/bin/sh
#!/usr/bin/sh
Copy link
Member

Choose a reason for hiding this comment

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

see above

core/src/cats/make_catalog_backup.pl.in
@@ -1,4 +1,4 @@
#!/usr/bin/env perl
#!/usr/bin/perl
Copy link
Member

Choose a reason for hiding this comment

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

you're not wrong, but we will have to support systems that have perl in unusual places (i.e. FreeBSD has it in /usr/local/bin/perl).
So far /usr/bin/env has worked well. We could change this, but we would have to configure the path system specific and detect the path to perl in CMake and set it using a variable.
If you propose a patch, I'll test and implement it.

Having said that, we're trying to remove/replace all perl scripts anyways, so I'm not really sure it is worth the effort anymore.

@joergsteffens joergsteffens mentioned this pull request Jan 25, 2021
Merged
@arogge
Copy link
Member

arogge commented Apr 15, 2021

It's been three months without reply now. I close the PR.
Feel free to reopen if desired.

@arogge arogge closed this Apr 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@arogge arogge arogge requested changes

Assignees

@arogge arogge

Labels
nobuild
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@tuxmaster5000 @arogge @pstorz