#13044 closed defect (fixed)
ecl chokes when CC or CXX contains space
Reported by: | ohanar | Owned by: | GeorgSWeber |
---|---|---|---|
Priority: | major | Milestone: | sage-7.2 |
Component: | packages: standard | Keywords: | sd40.5 |
Cc: | Merged in: | ||
Authors: | R. Andrew Ohana, Erik Bray, Jeroen Demeyer | Reviewers: | Jeroen Demeyer, Erik Bray |
Report Upstream: | Fixed upstream, but not in a stable release. | Work issues: | |
Branch: | 9049b15 (Commits, GitHub, GitLab) | Commit: | |
Dependencies: | Stopgaps: |
Description (last modified by )
Currently ecl completely fails to build when CC or CXX contains a space. This adds a patch accepted by upstream to fix this.
Upstream (patched): http://sage.ugent.be/www/jdemeyer/sage/ecl-15.3.7.tar.bz2
Change History (33)
comment:1 Changed 10 years ago by
- Description modified (diff)
- Status changed from new to needs_review
comment:2 Changed 10 years ago by
- Description modified (diff)
- Summary changed from ccache chokes on ecl to ecl chokes when CC or CXX contains space
comment:3 Changed 10 years ago by
comment:4 Changed 10 years ago by
- Component changed from build to packages
comment:5 Changed 9 years ago by
- Milestone changed from sage-5.11 to sage-5.12
comment:6 Changed 8 years ago by
- Status changed from needs_review to needs_info
Is this still needed?
comment:7 Changed 8 years ago by
- Status changed from needs_info to needs_review
Yes, it is still needed.
comment:8 Changed 8 years ago by
- Milestone changed from sage-6.1 to sage-6.2
comment:9 Changed 8 years ago by
- Milestone changed from sage-6.2 to sage-6.3
comment:10 Changed 8 years ago by
- Status changed from needs_review to needs_work
This needs to be converted to a new-style package, for the ECL version we currently use.
comment:11 Changed 8 years ago by
- Milestone changed from sage-6.3 to sage-6.4
comment:12 Changed 6 years ago by
I can confirm this is still an issue--sage build breaks on ecl when CC=ccache gcc
--obviously this can be worked around easily enough, but setting CC
like that is not uncommon either, and only seems to be a problem with ecl.
Would this be worth patching in ecl (both upstream and in sage)?
comment:13 Changed 6 years ago by
The updated spkg mentioned in this ticket is a 404 now.
comment:14 Changed 6 years ago by
Current upstream is https://common-lisp.net/project/ecl/
Also ecl-16.0.0
has been released and it would be nice to upgrade. It would be worth checking if it is still an issue in the latest release.
comment:15 Changed 6 years ago by
There is a similar bug that I've just run into, but in Python, when compiling the Cython sources. I've never run into this before strangely, but maybe have just worked on fewer projects with cpp generated from Cython: https://bugs.python.org/issue8027
I will check if the ecl issue is fixed upstream.
comment:16 Changed 6 years ago by
FYI this bug still exists upstream, or at least in ecl-16.0.0. I will see if I can come up with a patch.
comment:17 Changed 6 years ago by
- Report Upstream changed from N/A to Reported upstream. No feedback yet.
Submitted a patch to upstream: https://gitlab.com/embeddable-common-lisp/ecl/merge_requests/20
comment:18 follow-up: ↓ 20 Changed 6 years ago by
- Branch set to u/embray/issue-13044
- Commit set to c9f77c6dc61f6d3684716df620706f05a0604b64
- Status changed from needs_work to needs_review
Added the patch to the spkg. It applies to the current version of the spkg with a bit of fuzz.
If I understand correctly I should also update the patch version, and upload a new source package? Not sure exactly how to go about that though.
New commits:
c9f77c6 | Add patch to ecl for building when CC=ccache gcc (or the like) from https://gitlab.com/embeddable-common-lisp/ecl/merge_requests/20
|
comment:19 Changed 6 years ago by
- Description modified (diff)
- Milestone changed from sage-6.4 to sage-7.2
comment:20 in reply to: ↑ 18 Changed 6 years ago by
Replying to embray:
Added the patch to the spkg. It applies to the current version of the spkg with a bit of fuzz.
If I understand correctly I should also update the patch version
In this exceptional case I think it's not needed because there is no reason to rebuild everybody's ECL with this patch. And in the cases where ECL didn't install before, there is no rebuild needed since there couldn't have been a build in the first place.
and upload a new source package?
No, because you are not changing the source tarball, just adding a patch.
comment:21 Changed 6 years ago by
If I understand correctly, for the package the patches are being applied by spkg-src
which only seems to apply when building the source tarball.
comment:22 Changed 6 years ago by
Okay, this is unusual...
comment:23 Changed 6 years ago by
Bad separation (i.e. none) between the patches that needs to be applied in spkg-src
because autoconf has to be run after them and "normal" patches. I guess since you are patching the tarball anyway, you may as well put *all* the patches in it.
comment:24 Changed 6 years ago by
FWIW the patch has been accepted as-is upstream.
comment:25 Changed 6 years ago by
- Report Upstream changed from Reported upstream. No feedback yet. to Fixed upstream, but not in a stable release.
- Status changed from needs_review to needs_work
comment:26 Changed 6 years ago by
- Reviewers set to Jeroen Demeyer
comment:27 Changed 6 years ago by
- Description modified (diff)
comment:28 Changed 6 years ago by
- Branch changed from u/embray/issue-13044 to u/jdemeyer/issue-13044
comment:29 Changed 6 years ago by
- Commit changed from c9f77c6dc61f6d3684716df620706f05a0604b64 to 9049b155a9317d421de5ae9c22fad48eca5af824
- Status changed from needs_work to needs_review
Building with CC="ccache cc" CXX="ccache c++"
now works.
I cleaned up the packaging and build of ECL. In particular, I separated the patches which need to be applied by spkg-src
from the ones which are applied at build-time. This part still needs to be reviewed.
New commits:
9049b15 | Improve ECL packaging and build
|
comment:31 Changed 6 years ago by
- Reviewers changed from Jeroen Demeyer to Jeroen Demeyer, Erik Bray
comment:32 Changed 6 years ago by
- Branch changed from u/jdemeyer/issue-13044 to 9049b155a9317d421de5ae9c22fad48eca5af824
- Resolution set to fixed
- Status changed from positive_review to closed
comment:33 Changed 6 years ago by
- Commit 9049b155a9317d421de5ae9c22fad48eca5af824 deleted
- Description modified (diff)
I can verify that our current ECL chokes with "
env CC='nice gcc' ./spkg -i ecl...
" and that your spkg fixes it. Perhaps it would be nice to put something into the generatedecl_cc
so we know whence it came: maybe a comment like# ECL chokes when CC contains spaces; this script works around that. See #13044.
Then it would be easier to remove this workaround when ECL is updated.Also, I tried
env CXX='nice g++' ./sage -f ecl...
with the old version and it worked fine. Perhaps we shouldn't also createecl_cxx
since ECL is not using a C++ compiler.Also
^2
, we should send this upstream.