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

Fix Cygwin's "-no-undefined" patches for zeromq #17622

Closed
jpflori opened this issue Jan 12, 2015 · 11 comments
Closed

Fix Cygwin's "-no-undefined" patches for zeromq #17622

jpflori opened this issue Jan 12, 2015 · 11 comments

Comments

@jpflori
Copy link

jpflori commented Jan 12, 2015

The fix at #17333 was misplaced...

CC: @sagetrac-gouezel @tscrim

Component: porting: Cygwin

Author: Jean-Pierre Flori

Branch/Commit: 740da8b

Reviewer: Sebastien Gouezel

Issue created by migration from https://trac.sagemath.org/ticket/17622

@jpflori jpflori added this to the sage-6.5 milestone Jan 12, 2015
@jpflori
Copy link
Author

jpflori commented Jan 12, 2015

Branch: u/jpflori/ticket/17622

@jpflori
Copy link
Author

jpflori commented Jan 12, 2015

comment:1

@sébastien: can you test the fixed patches (just pull the branch and force zeromq install with ./sage -f)?
I don't have my VMs at hand so it's not trivial for me to test.
I should have time to reboot under Windows at some point though.


New commits:

740da8bFix Cygwin's "-no-undefined" patch for zeromq.

@jpflori
Copy link
Author

jpflori commented Jan 12, 2015

Author: Jean-Pierre Flori

@jpflori
Copy link
Author

jpflori commented Jan 12, 2015

Commit: 740da8b

@sagetrac-gouezel
Copy link
Mannequin

sagetrac-gouezel mannequin commented Jan 12, 2015

comment:2

I will test it later today. One question though: what is the role of the directory patches/build_system? As far as I can tell, it contains modified copies of the patches, but these copies are never applied...

@sagetrac-gouezel
Copy link
Mannequin

sagetrac-gouezel mannequin commented Jan 12, 2015

comment:3

The new version is much more sensible than the previous one, and works fine. It also works fine if one removes the subdirectory patches/build_system, so is there a good reason to keep it?

@jpflori
Copy link
Author

jpflori commented Jan 12, 2015

comment:5

zeromq uses autotools.
The patches in build_system directory are those you need to apply to the zeromq build system before autotools automatically generate the full build system.
Once these patches are applied, one can invoke autotools to update the build system (e.g. running autoreconf -fiv && rm -rf autom4te.cache) and deduce the full patches in the patches directory which don't need autotools invokation anymore and can be applied at build time.

To summarize it's good to ship two set of patches because:

  • we can't rely on invoking autotools at build time because it is not a prerequisite and would not be manageable anyway, it's kind of impossible to make sure that the build system will be correctly regenarated with one of the random versions of autotools programs in the wild, therefore the build system has to be regenerated on a dev machine and full patches have to be distributed,
  • the patches only touching the input to autotools are still of interest as these are the ones to be updated manually if we want to introduce further changes in the build system, and the ones to submit upstream for inclusion (and when upstream makes new releases it will regenerate the full build system)

@sagetrac-gouezel
Copy link
Mannequin

sagetrac-gouezel mannequin commented Jan 12, 2015

comment:6

OK, thanks a lot for the detailed and convincing explanation

@vbraun
Copy link
Member

vbraun commented Jan 13, 2015

comment:8

Reviewer name is missing

@tscrim
Copy link
Collaborator

tscrim commented Jan 13, 2015

Reviewer: Sebastien Gouezel

@vbraun
Copy link
Member

vbraun commented Jan 15, 2015

Changed branch from u/jpflori/ticket/17622 to 740da8b

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants