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

All files related to my RPM spec file project in one commit #7609

Merged
merged 2 commits into from Mar 24, 2016

Conversation

Projects
None yet
5 participants
@AliceWonderMiscreations
Contributor

AliceWonderMiscreations commented Feb 26, 2016

This is an attempt at the former pull request - #7588 (comment) - as a single commit.

The RPM package manager is used by several Linux distributions.

This provides an RPM spec file that is clean and builds properly on CentOS 7 (RHEL 7 clone) and should hopefully easily facilitate building RPM packages on other platforms as well.

It defaults to building against OpenSSL and the Qt5 GUI toolkit, but it supports switches that allow easily building against LibreSSL and/or Qt4 and/no no GUI.

See the README.md file in the contrib/rpm directory for more details.

@AliceWonderMiscreations

This comment has been minimized.

Show comment
Hide comment
@AliceWonderMiscreations

AliceWonderMiscreations Feb 26, 2016

Contributor

Okay in two commits, forgot to commit the modification to the contrib/README.md file

Contributor

AliceWonderMiscreations commented Feb 26, 2016

Okay in two commits, forgot to commit the modification to the contrib/README.md file

@jonasschnelli jonasschnelli added the Docs label Feb 26, 2016

@laanwj

This comment has been minimized.

Show comment
Hide comment
@laanwj

laanwj Feb 29, 2016

Member

utACK

Member

laanwj commented Feb 29, 2016

utACK

@AliceWonderMiscreations

This comment has been minimized.

Show comment
Hide comment
@AliceWonderMiscreations

AliceWonderMiscreations Mar 1, 2016

Contributor

I just wanted to clarify a note about the spec file and the generation of PNG / XPM images in the spec file.

The reason for that, the Source0 specifies the release tarball rather than github tagged release. The PNG / XPM files in github are not in the release tarball, so it was either list 10 additional github sources for them or list one additional vector source for the graphic and generate the icons from it.

I chose the latter.

Contributor

AliceWonderMiscreations commented Mar 1, 2016

I just wanted to clarify a note about the spec file and the generation of PNG / XPM images in the spec file.

The reason for that, the Source0 specifies the release tarball rather than github tagged release. The PNG / XPM files in github are not in the release tarball, so it was either list 10 additional github sources for them or list one additional vector source for the graphic and generate the icons from it.

I chose the latter.

OpenDebugLog();
-#if (OPENSSL_VERSION_NUMBER < 0x10100000L)
+#if defined(LIBRESSL_VERSION_NUMBER) || (OPENSSL_VERSION_NUMBER < 0x10100000L)

This comment has been minimized.

@MarcoFalke

MarcoFalke Mar 3, 2016

Member

Looks like this patch would be no longer needed.

@MarcoFalke

MarcoFalke Mar 3, 2016

Member

Looks like this patch would be no longer needed.

This comment has been minimized.

@jonasschnelli

jonasschnelli Mar 3, 2016

Member

Nice catch!
Agree.

@jonasschnelli

jonasschnelli Mar 3, 2016

Member

Nice catch!
Agree.

This comment has been minimized.

@AliceWonderMiscreations

AliceWonderMiscreations Mar 3, 2016

Contributor

Correct but the patch is needed for 0.12.0 source tarball as distributed by bitcoin.org. The README.md file states that the patch will likely not be needed when 0.12.1 is released.

@AliceWonderMiscreations

AliceWonderMiscreations Mar 3, 2016

Contributor

Correct but the patch is needed for 0.12.0 source tarball as distributed by bitcoin.org. The README.md file states that the patch will likely not be needed when 0.12.1 is released.

@laanwj

This comment has been minimized.

Show comment
Hide comment
@laanwj

laanwj Mar 3, 2016

Member

You're merging this to master, and I've added a Needs Backport so it will end up in 0.12.1. There's no need to include patches for other problems that are solved in 0.12.1.

Member

laanwj commented Mar 3, 2016

You're merging this to master, and I've added a Needs Backport so it will end up in 0.12.1. There's no need to include patches for other problems that are solved in 0.12.1.

RPM Spec File Notes
-------------------
The RPM spec file provided here is for Bitcoin-Core 0.12.0 and builds on CentOS

This comment has been minimized.

@MarcoFalke

MarcoFalke Mar 3, 2016

Member

Is is useful to distribute those RPM spec files via git, considering that they need update/bump prior to each release? (Including them in master for 0.12.0 makes them somewhat less accessible, compared to having them in the tag v0.12.0 or a separate place)

@MarcoFalke

MarcoFalke Mar 3, 2016

Member

Is is useful to distribute those RPM spec files via git, considering that they need update/bump prior to each release? (Including them in master for 0.12.0 makes them somewhat less accessible, compared to having them in the tag v0.12.0 or a separate place)

This comment has been minimized.

@laanwj

laanwj Mar 3, 2016

Member

Well we do the same for the debian files... I think it can make sense to include them, at least if someone is going to maintain them.

@laanwj

laanwj Mar 3, 2016

Member

Well we do the same for the debian files... I think it can make sense to include them, at least if someone is going to maintain them.

This comment has been minimized.

@AliceWonderMiscreations

AliceWonderMiscreations Mar 3, 2016

Contributor

Well my primary motivation was to provide something RPM packagers could use as a starting point that puts things in the proper places, does the proper testing on build of the binaries, etc.

RPM spec files distributed in upstream source rarely make into any package repository without some tweaking by the packager. For example, Fedora packaging guidelines discourage packaging of the .la libtool files or the .a static libraries. So someone building for Fedora packaging would likely change %install to delete those files, and then remove them from the %files bitcoin-devel.

If preferred a bitcoin.specfile.in file could be created so that autoconf / automake system could fill in the version, that means autoconf has to be run before the spec file is useful, and it makes the %changelog less clear.

@AliceWonderMiscreations

AliceWonderMiscreations Mar 3, 2016

Contributor

Well my primary motivation was to provide something RPM packagers could use as a starting point that puts things in the proper places, does the proper testing on build of the binaries, etc.

RPM spec files distributed in upstream source rarely make into any package repository without some tweaking by the packager. For example, Fedora packaging guidelines discourage packaging of the .la libtool files or the .a static libraries. So someone building for Fedora packaging would likely change %install to delete those files, and then remove them from the %files bitcoin-devel.

If preferred a bitcoin.specfile.in file could be created so that autoconf / automake system could fill in the version, that means autoconf has to be run before the spec file is useful, and it makes the %changelog less clear.

@MarcoFalke

This comment has been minimized.

Show comment
Hide comment
@MarcoFalke

MarcoFalke Mar 3, 2016

Member

Concept ACK 0e4b50a

Member

MarcoFalke commented Mar 3, 2016

Concept ACK 0e4b50a

@laanwj laanwj merged commit 0e4b50a into bitcoin:master Mar 24, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

laanwj added a commit that referenced this pull request Mar 24, 2016

Merge #7609: All files related to my RPM spec file project in one commit
0e4b50a Description of RPM directory (Alice Wonder)
146746b All files related to my RPM spec file project in one commit (Alice Wonder)
@wtogami

This comment has been minimized.

Show comment
Hide comment
@wtogami

wtogami Apr 30, 2016

Contributor

I wish I saw this earlier. This RPM spec is far from clean, the quality of the .spec is quite poor and it would not pass any package review standards (like Fedora). I also question why it encourages and offers to patch out the build-time prohibition of libressl which was intentionally disabled in Bitcoin.

I do not advise going further with better automating this with a .in file until we've had the opportunity to discuss how we should proceed together as a community. I generally find it inadvisable for upstream projects to ship any .spec file at all because they tend to be specific to a particular Linux distribution and version, for example the BuildRequires can differ substantially.

I would suggest that we separate out the SELinux policy into a different directory and discuss what that should do. The SELinux policy shipped in an upstream source repo should have documentation of what it is meant to be allowed or disallowed, in addition to a security review like any other code would be reviewed to confirm that it actually meets the stated goals.

Contributor

wtogami commented Apr 30, 2016

I wish I saw this earlier. This RPM spec is far from clean, the quality of the .spec is quite poor and it would not pass any package review standards (like Fedora). I also question why it encourages and offers to patch out the build-time prohibition of libressl which was intentionally disabled in Bitcoin.

I do not advise going further with better automating this with a .in file until we've had the opportunity to discuss how we should proceed together as a community. I generally find it inadvisable for upstream projects to ship any .spec file at all because they tend to be specific to a particular Linux distribution and version, for example the BuildRequires can differ substantially.

I would suggest that we separate out the SELinux policy into a different directory and discuss what that should do. The SELinux policy shipped in an upstream source repo should have documentation of what it is meant to be allowed or disallowed, in addition to a security review like any other code would be reviewed to confirm that it actually meets the stated goals.

@MarcoFalke

This comment has been minimized.

Show comment
Hide comment
@MarcoFalke
Member

MarcoFalke commented Apr 30, 2016

@laanwj

This comment has been minimized.

Show comment
Hide comment
@laanwj

laanwj May 4, 2016

Member

@wtogami feel free to improve it

Member

laanwj commented May 4, 2016

@wtogami feel free to improve it

@MarcoFalke

This comment has been minimized.

Show comment
Hide comment
@MarcoFalke

MarcoFalke May 4, 2016

Member

If this is still WIP, it should probably not be backported right now?

Member

MarcoFalke commented May 4, 2016

If this is still WIP, it should probably not be backported right now?

@wtogami

This comment has been minimized.

Show comment
Hide comment
@wtogami

wtogami May 4, 2016

Contributor

If this is WIP, why was it committed to the core repo? In any case it's fine to exist here, I will just suggest that people ignore it because it is of poor quality. I don't think "fixing" it in the core repo is a good idea because you can't make a RPM spec that is valid on all distros.

Contributor

wtogami commented May 4, 2016

If this is WIP, why was it committed to the core repo? In any case it's fine to exist here, I will just suggest that people ignore it because it is of poor quality. I don't think "fixing" it in the core repo is a good idea because you can't make a RPM spec that is valid on all distros.

@laanwj

This comment has been minimized.

Show comment
Hide comment
@laanwj

laanwj May 5, 2016

Member

If this is WIP, why was it committed to the core repo?

Because it may be a useful start for some people.
I think all of our documents can be considered WIP at this stage.

Member

laanwj commented May 5, 2016

If this is WIP, why was it committed to the core repo?

Because it may be a useful start for some people.
I think all of our documents can be considered WIP at this stage.

codablock added a commit to codablock/dash that referenced this pull request Sep 16, 2017

Merge #7609: All files related to my RPM spec file project in one commit
0e4b50a Description of RPM directory (Alice Wonder)
146746b All files related to my RPM spec file project in one commit (Alice Wonder)

codablock added a commit to codablock/dash that referenced this pull request Sep 19, 2017

Merge #7609: All files related to my RPM spec file project in one commit
0e4b50a Description of RPM directory (Alice Wonder)
146746b All files related to my RPM spec file project in one commit (Alice Wonder)

codablock added a commit to codablock/dash that referenced this pull request Dec 9, 2017

Merge #7609: All files related to my RPM spec file project in one commit
0e4b50a Description of RPM directory (Alice Wonder)
146746b All files related to my RPM spec file project in one commit (Alice Wonder)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment