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

Add clang to the installed dependencies. #38

Closed
wants to merge 1 commit into from
Closed

Conversation

freakboy3742
Copy link
Member

Fixes beeware/briefcase#1500

Python-build-standalone has always been built with clang, but until the 20231002 release, it didn't use any CFLAGS that were clang specific. That has now changed, so we need to provide clang in the Dockerfile so that extension modules can be compiled.

PR Checklist:

  • All new features have been tested
  • All new features have been documented
  • I have read the CONTRIBUTING.md file
  • I will abide by the code of conduct

@rmartin16
Copy link
Member

Unfortunately, the version of clang for CentOS 7 is 3.4.2 (released in 2014)....so, it's much too old to support -fdebug-default-version which appears to have been added in late 2019 with the next release being 9.0.1 a month or so later.

RHEL released a metapackage called llvm-toolset-7 which contains clang 5....but it's too old as well...

I was able to install (err...hack) clang 11....but it did work to build PyGObject...

# As root, Install system packages required by app
ARG SYSTEM_REQUIRES
RUN yum install -y gcc make pkgconfig ${SYSTEM_REQUIRES}

# create a custom repo for clang 11
RUN printf "%s\n" "[llvmtoolset-build]" > /etc/yum.repos.d/llvmtoolset-build.repo && \
    printf "%s\n" "name=LLVM Toolset 11.0 - Build" >> /etc/yum.repos.d/llvmtoolset-build.repo && \
    printf "%s\n" "baseurl=https://buildlogs.centos.org/c7-llvm-toolset-11.0.x86_64/" >> /etc/yum.repos.d/llvmtoolset-uild.repo && \
    printf "%s\n" "gpgcheck=0" >> /etc/yum.repos.d/llvmtoolset-build.repo && \
    printf "%s\n" "enabled=1" >> /etc/yum.repos.d/llvmtoolset-build.repo

# install clang 11
RUN yum install -y --nogpgcheck llvm-toolset-11.0-clang

# Use the brutus user for operations in the container
USER brutus

# Add clang to PATH
ENV PATH="/opt/rh/llvm-toolset-11.0/root/usr/bin:/opt/rh/llvm-toolset-11.0/root/usr/sbin${PATH:+:${PATH}}"
ENV LD_LIBRARY_PATH="/opt/rh/llvm-toolset-11.0/root/usr/lib64${LD_LIBRARY_PATH:+:${LD_LIBRARY_PATH}}"
 
# Put the Standalone Python on the build path
ENV PATH="/app/Hello World.AppDir/usr/python/bin:${PATH}"

This seems less than ideal for obvious reasons...

@freakboy3742
Copy link
Member Author

Ugh. I'm starting to be very happy that we downgraded Appimage support.

Should we just document that AppImage only works on manylinux 2_28 (and presumably later... as soon as there is a later release)? I'm not sure I see any other way forward, other than taking on the maintenance of our own support packages again.

@rmartin16
Copy link
Member

Should we just document that AppImage only works on manylinux 2_28 (and presumably later... as soon as there is a later release)? I'm not sure I see any other way forward, other than taking on the maintenance of our own support packages again.

I think that'd be fine; although, I think we should also update the Briefcase template to default to manylinux_2_28 instead of manylinux2014 like it does now.....and maybe even pin PyGObject and toga while we're at it (/s). Although, given AppImage support is becoming so wonky....maybe Briefcase should even emit a warning for the user noting this is all quite unstable now.

All that said, llvm/llvm-project#67966 makes me think that greg introduced this clang flag as a workaround. Perhaps when the underlying problem is resolved....none of this will be a problem anymore. Alternatively, I wonder if an older flag could be used instead...and one that's supported by gcc....it seems like -gdwarf-4 may be appropriate....but I don't really know enough about all this and we're trying to avoid spending a lot of time on AppImage at this point...

@freakboy3742
Copy link
Member Author

I think that'd be fine; although, I think we should also update the Briefcase template to default to manylinux_2_28 instead of manylinux2014 like it does now.....and maybe even pin PyGObject and toga while we're at it (/s). Although, given AppImage support is becoming so wonky....maybe Briefcase should even emit a warning for the user noting this is all quite unstable now.

Fair call. I'll get PRs up for those two changes shortly.

All that said, llvm/llvm-project#67966 makes me think that greg introduced this clang flag as a workaround. Perhaps when the underlying problem is resolved....none of this will be a problem anymore. Alternatively, I wonder if an older flag could be used instead...and one that's supported by gcc....it seems like -gdwarf-4 may be appropriate....but I don't really know enough about all this and we're trying to avoid spending a lot of time on AppImage at this point...

We can but hope :-)

@freakboy3742
Copy link
Member Author

Thinking about it - there's another possible solution. If this is an unintended breakage that might see a fix in an upcoming release, we could downgrade the support package to 20230826 for every Python version except 3.12 (since 20231002 is the first release supporting 3.12).

I've logged the issue - that should give some clarity as to whether this is an intended change, or something that might be reverted in the next release.

The banner warning about AppImage is still a good idea regardless - but we might need to bump the manylinux base if this is an unintended side effect.

@rmartin16
Copy link
Member

rmartin16 commented Oct 24, 2023

That makes sense to me.

@freakboy3742
Copy link
Member Author

Based on the comment from @indygreg on indygreg/python-build-standalone#194, it appears this was an unintended side effect, so I'll close this PR in favor of reverting the version bump (except for Py3.12).

@freakboy3742 freakboy3742 deleted the appimage-clang branch October 24, 2023 23:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

pycairo failing appimage build
2 participants