Opened 5 years ago

Closed 5 years ago

#17622 closed defect (fixed)

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

Reported by: jpflori Owned by:
Priority: major Milestone: sage-6.5
Component: porting: Cygwin Keywords:
Cc: gouezel, tscrim Merged in:
Authors: Jean-Pierre Flori Reviewers: Sebastien Gouezel
Report Upstream: N/A Work issues:
Branch: 740da8b (Commits) Commit: 740da8b4a26e5806aa401f815fa4998b4c0cd766
Dependencies: Stopgaps:

Description

The fix at #17333 was misplaced...

Change History (10)

comment:1 Changed 5 years ago by jpflori

  • Authors set to Jean-Pierre Flori
  • Branch set to u/jpflori/ticket/17622
  • Commit set to 740da8b4a26e5806aa401f815fa4998b4c0cd766
  • Status changed from new to needs_review

@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.

comment:2 Changed 5 years ago by gouezel

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...

comment:3 Changed 5 years ago by gouezel

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?

comment:4 Changed 5 years ago by gouezel

  • Status changed from needs_review to needs_info

comment:5 Changed 5 years ago by jpflori

  • Status changed from needs_info to needs_review

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)

comment:6 Changed 5 years ago by gouezel

OK, thanks a lot for the detailed and convincing explanation

comment:7 Changed 5 years ago by gouezel

  • Status changed from needs_review to positive_review

comment:8 Changed 5 years ago by vbraun

  • Status changed from positive_review to needs_work

Reviewer name is missing

comment:9 Changed 5 years ago by tscrim

  • Reviewers set to Sebastien Gouezel
  • Status changed from needs_work to positive_review

comment:10 Changed 5 years ago by vbraun

  • Branch changed from u/jpflori/ticket/17622 to 740da8b4a26e5806aa401f815fa4998b4c0cd766
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.