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

RPM build improvements #385

Merged
merged 33 commits into from May 9, 2019
Merged

RPM build improvements #385

merged 33 commits into from May 9, 2019

Conversation

j-ogas
Copy link
Contributor

@j-ogas j-ogas commented Mar 13, 2019

Addresses #384.

@reidpr
Copy link
Collaborator

reidpr commented Mar 14, 2019

Looks good. I added one more commit.

On your approval, do you want me to merge, or should we wait for feedback from Fedora?

Copy link
Contributor Author

@j-ogas j-ogas left a comment

Choose a reason for hiding this comment

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

I forgot to update Source0: to:

https://github.com/hpc/%{name}/releases/download/v%{version}/%{name}-%{version}.tar.gz

Also, we may need to tinker with the python requirement.

@j-ogas
Copy link
Contributor Author

j-ogas commented Mar 15, 2019

Another thought: I think Build should—for release version—download the tarball via the source0 URL instead of doing a git checkout and make export. This makes it such that the source tarball in the generated SRPM will have a hash that matches the source0 URL tarball, which is required by Fedora.

@reidpr
Copy link
Collaborator

reidpr commented Mar 15, 2019

Downloading Source0 is fine with me. I wonder if build should take an option to say whether to download or make export.

@j-ogas
Copy link
Contributor Author

j-ogas commented Mar 15, 2019

I have pushed two commits. The first updates the source URL and the second adds python 2.7 as a temporary build requirement for charliecloud (ch-run).

I propose that we open a new issue to modify the build procedure to skip building the test suite if python is not detected (0.9.9). Once 0.9.9 is released we can modify the spec file and move the python 2.7 dependency to the charliecloud-doc subpackage and add a release note for 0.9.10.

@reidpr
Copy link
Collaborator

reidpr commented Mar 19, 2019

This is the Bugzilla bug, with some feedback: https://bugzilla.redhat.com/show_bug.cgi?id=1690046

@j-ogas j-ogas requested a review from reidpr March 20, 2019 20:54
Copy link
Collaborator

@reidpr reidpr left a comment

Choose a reason for hiding this comment

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

Per discussion offline, looks like we need a few more tweaks before resubmission.

@j-ogas

This comment has been minimized.

@j-ogas
Copy link
Contributor Author

j-ogas commented Mar 25, 2019

Fedora requires that spec files either install documentation to %{prefix}/doc/%{name}-%{version} (%_pkgdocdir), or use %doc, which automatically copies the documentation files into the correct directory, e.g., %doc README.rst will copy the readme file to ${prefix}/charliecloud-0.9.8/README.rst.

c8d94ad addresses the issue by using %doc in conjunction with %exclude ${prefix}/doc/charliecloud to ensure that the documentation files are not duplicated. We are essentially moving the readme installed via make install PREFIX at package build time.

We should consider modifying our top level Makefile to reflect the ${prefix}/doc/charliecloud-${version} requirement as part of 0.9.9. This would allow us to use the %_pkgdocdir macro exclusively (no need to exclude files or worry about duplicates).

@reidpr reidpr changed the title add release version rpm building (closes #384) RPM build improvements Apr 30, 2019
@reidpr reidpr added this to the 0.9.10 milestone Apr 30, 2019
@j-ogas
Copy link
Contributor Author

j-ogas commented May 8, 2019

Removing ready to merge. We have /usr/bin/python3 listed as a charliecloud build dependency, it should only be a test-package dependency. I need to rebuild the package and submit to Fedora.

@j-ogas
Copy link
Contributor Author

j-ogas commented May 8, 2019

Removing ready to merge. We have /usr/bin/python3 listed as a charliecloud build dependency, it should only be a test-package dependency. I need to rebuild the package and submit to Fedora.

Right, I had forgotten we have python listed as a build dependency because our top level Makefile will also make the contents of charliecloud/test, which fails on systems without python installed.

@reidpr
Copy link
Collaborator

reidpr commented May 8, 2019

Right, I had forgotten we have python listed as a build dependency because our top level Makefile will also make the contents of charliecloud/test, which fails on systems without python installed.

We did change it so it no longer fails to build if Python isn't installed — it just doesn't build/run the tests — but a build dependency is still needed IMO so we can build the test package.

@reidpr reidpr merged commit 2ebe587 into master May 9, 2019
@reidpr reidpr deleted the release-rpm branch May 9, 2019 22:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants