Wip make dist rpm 3 #4911

osynge · 2015-06-09T15:38:33Z

replaces #4905

ghost · 2015-06-09T15:47:46Z

@osynge would you mind rebasing instead of creating new pull requests ? Unless there is a specific reason to do so, it saves a little time with pull request triage :-)

osynge · 2015-06-09T15:57:20Z

@dachary sure, I will do in future, some thing funny happened when I tried to reorder so no commit would break anything (for bisecting)

ktdreyer · 2015-06-09T16:26:22Z

The general concept is good here.

One detail regarding implementation: I'd really like this RPM generation process to be outside of autoconf entirely. The fact that we have to do a full ./configure run just to get a version number for an SRPM adds a lot of time onto the build process, since ./configure takes so long.

Sage wrote a ./make_dist.sh script (currently in master) that we're going use in place of "make dist" soon. This will allow us to get a tarball very quickly, without having to run ./configure.

Can we do the same thing with a "./make_rpm.sh" ? (it could take an --srpm flag if you want to have it build both SRPMs and RPMs, as this PR currently does with make rpm and make srpm.)

ktdreyer · 2015-06-09T16:27:47Z

Re: my comment above, there's some background information about the "make dist" transition in this ceph-devel thread (http://www.spinics.net/lists/ceph-devel/msg23958.html)

osynge · 2015-06-09T16:46:29Z

@ktdreyer do you have any reason apart from performance ? This is a very bad idea in my opinion.

By not using configure to set the paths in the spec or debian files.

You have to set the paths in the code base of ceph for debian.
You have to set the paths in the code base of ceph for redhat.
You have to set the paths in the code base of ceph for SUSE.
You have to make conditionals for all the paths for each OS.

A good example would be the file :

src/ceph-osd-prestart.sh

By using configure we in a wrapper script such as ./make_rpm.sh we can set the paths to distribution in one place :)

ktdreyer · 2015-06-09T17:07:15Z

The RPMs and DEBs will still run ./configure during the actual builds. For example, the RPMs will run ./configure during %prep. The point is that we shouldn't have to run ./configure just to get a tarball or SRPM.

The problem is that right now we have to run ./configure twice: once to simply build a tarball with the proper version number, and once within rpmbuild itself.

osynge · 2015-06-09T17:14:39Z

@ktdreyer this seems a very small issue compared to the potentially lost features.

I answered on the mailing list in more depth why I feel this is wrong.

loic-bot · 2015-06-09T21:15:26Z

SUCCESS: http://jenkins.ceph.dachary.org/job/ceph/5772/

osynge · 2015-06-09T22:12:53Z

@ktdreyer

First execution.
- Defaulting all release values as specified into all templated files such as ceph.spec.
Second execution.
- The subprocess of executing a rpm processed build that will result in an rpm.
- In rpm based testing we can check is "make srpm" makes recursively building srpms.

osynge · 2015-06-09T22:53:23Z

Note "./make_rpm.sh" and "make rpm" applications provide different features.

making tar balls for publishing on the web.
makes rpm, srpm, and version tar ball.

They are two different use cases addressed.

ktdreyer · 2015-06-09T22:58:54Z

@ktdreyer this seems a very small issue compared to the potentially lost features.

It's not clear to me what features we'd be losing by moving this out of autoconf and into a standalone script.

osynge · 2015-06-09T23:03:00Z

You will loose the ability to apply templates to things like "ceph.spec"

You loose sane default values and replace them with hard coded values.
Have to put a lot of conditionals in ceph.sepc.in
You can support configure time calculation of default values
- eg "git hash"

osynge · 2015-06-09T23:10:28Z

Maybe I should have merged this patch and the #4871 and suggested they be used together.

ktdreyer · 2015-06-09T23:13:44Z

You loose sane default values and replace them with hard coded values.

can you give some further examples?

Have to put a lot of conditionals in ceph.sepc.in

This is annoying. Ceph's .spec is a lot messier than anything else I've worked on. But os-specific conditionals have to go somewhere, and this follows the conventions that a lot of other projects and packages follow.

You can support configure time calculation of default values eg "git hash"

Right, that's what "make_dist.sh" does, entirely independent of autoconf, and a lot faster than autoconf.

osynge · 2015-06-09T23:17:25Z

So we are agreed the arguments about this duplicating functionality of "make_dist.sh" is irrelevant?

You agree now that I only need to provide examples of why variables are useful well I have to make a union of a few patches to show you :)

osynge · 2015-06-09T23:37:16Z

Being able to make nearly everything a variable in ceph.spec.in will prevent he turbulence caused by different conditionals being merged, ie where does this go?

%if (0%{?rhel} && 0%{?rhel} < 7)
install -m 0644 -D udev/95-ceph-osd-alt.rules $RPM_BUILD_ROOT/lib/udev/rules.d/95-ceph-osd.rules
%else
install -m 0644 -D udev/95-ceph-osd.rules $RPM_BUILD_ROOT/lib/udev/rules.d/95-ceph-osd.rules
%endif

clearly in the spec.in file it would be nice to be:

install -m 0644 -D udev/95-ceph-osd-alt.rules $RPM_BUILD_ROOT/@udev-dir@/95-ceph-osd-sysv.rules

Clearly the second line is better. We could even have configure find the directory, on every distribution if not set in the options :)

Its just a few lines in configure.ac and its done making the job easier :)

osynge · 2015-06-09T23:56:35Z

In summary

I think using autotools to provide autodetection of defaults and allow configure override with parameters is useful and simple.
Is a good way to insulate a project from the operating system configuration.
os-specific conditionals have to go somewhere

tip: If you explicitly set autotools options it can some times run faster, as it does not have to find out a default. Explicitly turning off java may help in configure run time.

ktdreyer · 2015-06-09T23:58:15Z

Using @udev-dir@ in ceph.spec.in is not the way to go. Adding more autoconf variables into ceph.spec.in means building more layers of complexity on a system that's already horribly complex.

The particular example with the 95-ceph-osd.rules file vs 95-ceph-osd-alt.rules is a good example of a bad convention that was done as an expedient hack years ago to fix http://tracker.ceph.com/issues/4632. The two files have diverged since then, and the proper solution is to make ./configure detect the system's udev version and conditionally install the correct .rules file during make install.

osynge · 2015-06-10T00:29:36Z

Adding more autoconf variables into ceph.spec.in means building more layers of complexity to allow the variables, to be variable and stored else wear.

In the end it comes down to isolating the changed values from the debian defaults to SUSE and redhat and making them variable, and handling the conditionals in a language with nicer conditionals the an rpm spec file makes a lot of sence.

Aded options --with-rpm-dir=/yourrpmdir Default is `pwd`/RPM rpm topdir. Signed-off-by: Owen Synge <osynge@suse.com>

osynge · 2015-06-16T13:27:36Z

when tested on fedora it did not work reasons where:

1 not adding patch file to sources for fedora builds.

+cp rpm/init-ceph.in-fedora.patch  $(rpmtopdir)/SOURCES

2 fedora already defines _topodir so typo in _topdir definition caused failures.

-       rpmbuild --define "_rpmtopdir $(rpmtopdir)" -ba @PACKAGE@.spec
+       rpmbuild --define "_topdir $(rpmtopdir)" -ba @PACKAGE@.spec

These are now fixed in latest commit.

loic-bot · 2015-06-16T13:30:07Z

FAILURE: http://jenkins.ceph.dachary.org/job/ceph/5903/

FAILURE http://jenkins.ceph.dachary.org/job/ceph/LABELS=centos-7&&x86_64/5903/

Add "make rpm" to Makefile.am Use RPM_DIR created in configure.ac to define rpmtopdir. Signed-off-by: Owen Synge <osynge@suse.com>

Add "make srpm" to Makefile.am as a build dependency to "make rpm" Signed-off-by: Owen Synge <osynge@suse.com>

osynge · 2015-06-16T14:27:32Z

Added a comment between testing and making patch that caused build to fail. This comment is now in style of previous comments and no longer produces interesting errors.

loic-bot · 2015-06-16T16:39:23Z

SUCCESS: http://jenkins.ceph.dachary.org/job/ceph/5906/

osynge · 2015-06-18T18:06:08Z

@ktdreyer "using @udev-dir@ in ceph.spec.in is not the way to go."

(1) Please note this patch does not contain @udev-dir@ type changes.

On that subject, the only strong contender not to just put more logic into autotools defaults as in #4871

ghost · 2015-09-01T22:18:43Z

@osynge this needs rebasing. And maybe a gentle ping ;-)

ghost · 2015-11-25T14:28:36Z

@osynge feel free to re-open when you have time to rebase

osynge mentioned this pull request Jun 9, 2015

Wip make dist rpm 2 #4905

Closed

ghost added the build/ops label Jun 9, 2015

ghost assigned smithfarm Jun 9, 2015

smithfarm assigned ktdreyer and unassigned smithfarm Jun 9, 2015

osynge mentioned this pull request Jun 11, 2015

Wip backport make dist rpm SUSE/ceph#4

Closed

define topdir for to allow "make rpm" target

912c231

Aded options --with-rpm-dir=/yourrpmdir Default is `pwd`/RPM rpm topdir. Signed-off-by: Owen Synge <osynge@suse.com>

osynge force-pushed the wip-make-dist-rpm_3 branch from 54bbde8 to fd2a08a Compare June 16, 2015 13:21

Add "make rpm" target

b20cbce

Add "make rpm" to Makefile.am Use RPM_DIR created in configure.ac to define rpmtopdir. Signed-off-by: Owen Synge <osynge@suse.com>

Add "make srpm" target

6e48fd8

Add "make srpm" to Makefile.am as a build dependency to "make rpm" Signed-off-by: Owen Synge <osynge@suse.com>

osynge force-pushed the wip-make-dist-rpm_3 branch from fd2a08a to 6e48fd8 Compare June 16, 2015 14:23

ghost closed this Nov 25, 2015

jan--f deleted the wip-make-dist-rpm_3 branch February 8, 2017 20:33

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Wip make dist rpm 3 #4911

Wip make dist rpm 3 #4911

osynge commented Jun 9, 2015

ghost commented Jun 9, 2015

osynge commented Jun 9, 2015

ktdreyer commented Jun 9, 2015

ktdreyer commented Jun 9, 2015

osynge commented Jun 9, 2015

ktdreyer commented Jun 9, 2015

osynge commented Jun 9, 2015

loic-bot commented Jun 9, 2015

osynge commented Jun 9, 2015

osynge commented Jun 9, 2015

ktdreyer commented Jun 9, 2015

osynge commented Jun 9, 2015

osynge commented Jun 9, 2015

ktdreyer commented Jun 9, 2015

osynge commented Jun 9, 2015

osynge commented Jun 9, 2015

osynge commented Jun 9, 2015

ktdreyer commented Jun 9, 2015

osynge commented Jun 10, 2015

osynge commented Jun 16, 2015

loic-bot commented Jun 16, 2015

osynge commented Jun 16, 2015

loic-bot commented Jun 16, 2015

osynge commented Jun 18, 2015

ghost commented Sep 1, 2015

ghost commented Nov 25, 2015

Wip make dist rpm 3 #4911

Wip make dist rpm 3 #4911

Conversation

osynge commented Jun 9, 2015

ghost commented Jun 9, 2015

osynge commented Jun 9, 2015

ktdreyer commented Jun 9, 2015

ktdreyer commented Jun 9, 2015

osynge commented Jun 9, 2015

ktdreyer commented Jun 9, 2015

osynge commented Jun 9, 2015

loic-bot commented Jun 9, 2015

osynge commented Jun 9, 2015

osynge commented Jun 9, 2015

ktdreyer commented Jun 9, 2015

osynge commented Jun 9, 2015

osynge commented Jun 9, 2015

ktdreyer commented Jun 9, 2015

osynge commented Jun 9, 2015

osynge commented Jun 9, 2015

osynge commented Jun 9, 2015

ktdreyer commented Jun 9, 2015

osynge commented Jun 10, 2015

osynge commented Jun 16, 2015

loic-bot commented Jun 16, 2015

osynge commented Jun 16, 2015

loic-bot commented Jun 16, 2015

osynge commented Jun 18, 2015

ghost commented Sep 1, 2015

ghost commented Nov 25, 2015