Skip to content
This repository has been archived by the owner on Nov 18, 2020. It is now read-only.

build requires rst2man even after make dist #25

Closed
ingvarha opened this issue Oct 26, 2015 · 6 comments
Closed

build requires rst2man even after make dist #25

ingvarha opened this issue Oct 26, 2015 · 6 comments
Assignees

Comments

@ingvarha
Copy link

make dist when successfully run, produces a configure that unnecessarily requires rst2man, as the manpage is prebuilt by make dist. This makes the release tarball difficult to build on platforms that misses rst2man, like rhel5 and clones.

Workaround: export RST2MAN=/bin/true before calling configure.

See toreanderson/libvmod-rfc6052#2 . vmod-rfc6052 is based on this example vmod, and the bug originates here.

Ingvar

@Dridi Dridi self-assigned this Dec 15, 2015
@Dridi
Copy link
Member

Dridi commented Dec 15, 2015

I believe I broke this one, assigning to myself.

@Dridi
Copy link
Member

Dridi commented Jan 2, 2016

Hi Ingvar,

I have started a new VMOD this week and took this as an occasion to clean the build system, including this issue, #26 and #27.

The main changes are:

  • the use of autotools's test driver
    • we get the same green PASS and red FAIL as Varnish
    • I also have unit tests in this module (out of scope)
  • simpler dependencies with generated sources
    • explicit dependencies between $(srcdir) and $(builddir) [1]
    • declaration of all the files created by the vmodtool
    • the man page is generated in the src/ directory
  • a cleanup of the RPM spec file
  • a cleanup of .gitignore

I believe I also managed to remove all target/source names duplication in the makefiles, without overdoing it. I barely touched configure.ac, only to search the VTC test suite there. I haven't touched the debian directory.

So it works on my machine, but...

  • could you please try to build this module on different platforms, including el5?
  • @lkarsten could you also please review it?

If it's OK for the both of you, I'll submit a pull request with similar changes.

Happy New Year,
Dridi

[1] $(builddir) no longer appears in the makefiles

@ingvarha
Copy link
Author

ingvarha commented Jan 4, 2016

Hello, Dridi. Nice work!

I have not done any functionality testing, only builds and make check.

Builds fine on fedora. Builds fine on el5, el6, and el7 as well.

The build needs varnish-4.1 to build, but configure does not stop nor warn on varnish-4.0 (default on epel7). It should.

Do you want me to put this up at https://copr.fedoraproject.org/coprs/ingvar/varnish41/ ?

Ingvar

----- On Jan 2, 2016, at 12:43 PM, Dridi Boukelmoune notifications@github.com wrote:

Hi Ingvar,

I have started a new VMOD this week and took this as an occasion to clean the build system, including this issue, #26 and #27 .

The main changes are:

* the use of autotools's test driver 
    * we get the same green PASS and red FAIL as Varnish 
    * I also have unit tests in this module (out of scope) 
* simpler dependencies with generated sources 
    * explicit dependencies between $(srcdir) and $(builddir) [1] 
    * declaration of all the files created by the vmodtool 
    * the man page is generated in the src/ directory 
* a cleanup of the RPM spec file 
* a cleanup of .gitignore 

I believe I also managed to remove all target/source names duplication in the makefiles, without overdoing it. I barely touched configure.ac , only to search the VTC test suite there. I haven't touched the debian directory.

So it works on my machine, but...

* could you please try to build this module on different platforms, including el5? 
* @lkarsten could you also please review it? 

If it's OK for the both of you, I'll submit a pull request with similar changes.

Happy New Year,
Dridi

[1] $(builddir) no longer appears in the makefiles


Reply to this email directly or view it on GitHub .

@Dridi
Copy link
Member

Dridi commented Jan 4, 2016

Hey Ingvar,

I have not done any functionality testing, only builds and make check.

It should work up to make distcheck, and package builds from a make dist tarball. However there is currently no functionality.

The build needs varnish-4.1 to build, but configure does not stop nor warn on varnish-4.0 (default on epel7). It should.

I'm not sure branches of this repo check for the Varnish version in configure, I can fix that too.

As for myself I had only tested the build on a mock cache (fc22) I had on my machine, I was offline and luckily I had done Varnish 4.1 mock builds previously.

The VMOD hasn't been started yet. I'm barely starting with HPACK, so I think it is way too early to consider packaging it in your COPR repo. I'll keep that in mind though :)

Dridi

@Dridi
Copy link
Member

Dridi commented Jun 3, 2016

Going back to the original subject, I'm not sure it's a good idea to include man pages in the distribution. They depend on the vmodtool used at build-time, so I'm not sure it's a good idea to save them at dist-time.

@Dridi
Copy link
Member

Dridi commented Feb 27, 2018

Closing for the reason stated in my previous comment.

@Dridi Dridi closed this as completed Feb 27, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants