-
Notifications
You must be signed in to change notification settings - Fork 30
mostly dep updates, remove bad option to RP #65
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
Conversation
--no-unsign was the culprit stopping pip from updating and several of the old pip versions weren't installing cleanly, bumped those versions and sha's of RP/MP among other minor tweaks.
as flagged after previous commit
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As far as pip, can you expand on what was happening there? Was pip unable to
update itself after installing the resulting package?
With signing, we may need to add that back.
@@ -84,7 +84,7 @@ jobs: | |||
files: ${{github.workspace}}/outputs/*.pkg | |||
|
|||
- name: Upload packages | |||
uses: actions/upload-artifact@0b7f8abb1508181956e8e162db84b466c27e18ce # v3.1.2 | |||
uses: actions/upload-artifact@v4.6.2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Grab the commit hash of this version and use that instead if you would. The tag can change the underlying commit and we never want this to change out from under us.
build_python_framework_pkgs.zsh
Outdated
# kill homebrew packages | ||
/usr/local/bin/brew remove --force $(/usr/local/bin/brew list) | ||
# # kill homebrew packages | ||
# /usr/local/bin/brew remove --force $(/usr/local/bin/brew list) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We had issues where homebrew stuff would creep into the build and cause issues. Not sure what the impact of doing this will be.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
gotit, restoring
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah it will break building the packages on the GitHub runner images.
instead of tag use commit, in case build node needs homebrew purgin'
ensurepip/the python.org interpreter RP was downloading was unable to run (on Apple Silicon vs. how the old runners were intel-only) because we explicitly weren't resigning and AMFId was SIGKILL'ing, then the versions pinned in the reqs.txt weren't cutting the mustard... |
There's no way this will work on the notarizing side. I used that flag to get it to work. |
I'm happy to try it and be proven wrong but why all of the other script changes? Those are working well and I'm not sure we need to make those other changes at this time. |
So I should at least plug in certs I have access to locally to confirm? There's essentially one flag omitted when it comes to material changes, I've rolled the others back/forward as requested... if you'd rather not give it a shot until I confirm notarization works I'm happy to do so, but if it ends up still bork3d on GitHubs runners it would take me a bit longer |
build_python_framework_pkgs.zsh
Outdated
@@ -96,7 +97,7 @@ if [ -d "${PIPCACHEDIR}" ]; then | |||
/usr/bin/sudo /bin/rm -rf "${PIPCACHEDIR}" | |||
fi | |||
|
|||
# kill homebrew packages | |||
# # kill homebrew packages |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit, but get rid of the extra #
so it doesn't show as a change.
@@ -235,7 +224,7 @@ else | |||
/usr/bin/find "$TOOLSDIR/$TYPE/payload${FRAMEWORKDIR}/Python3.framework/Versions/${PYTHON_BIN_VERSION}/lib" -type f -name "*dylib" -exec /usr/bin/codesign -s - --preserve-metadata=identifier,entitlements,flags,runtime -f {} \; | |||
/usr/bin/codesign -s - --deep --force --preserve-metadata=identifier,entitlements,flags,runtime "$TOOLSDIR/$TYPE/payload${FRAMEWORKDIR}/Python3.framework/Versions/${PYTHON_BIN_VERSION}/Resources/Python.app" | |||
/usr/bin/codesign -s - --force --preserve-metadata=identifier,entitlements,flags,runtime "$TOOLSDIR/$TYPE/payload${FRAMEWORKDIR}/Python3.framework/Versions/${PYTHON_BIN_VERSION}/Python" | |||
/usr/bin/codesign -s - --force --preserve-metadata=identifier,entitlements,flags,runtime "$TOOLSDIR/$TYPE/payload${FRAMEWORKDIR}Python3.framework/Versions/Current/Python" | |||
/usr/bin/codesign -s - --force --preserve-metadata=identifier,entitlements,flags,runtime "$TOOLSDIR/$TYPE/payload${FRAMEWORKDIR}/Python3.framework/Versions/Current/Python" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is this doing? I think the /
are in the FRAMEWORKDIR
var, which, admittedly, isn't my favorite.
This seems like an out of scope change, so let's keep this as tidy as we can, let the CI/CD cut a build and then adjust as needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it will almost certainly fail as-is, as per the lines directly preceding where there isn't the trailing forwardslash in the var... your call
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(I should be honest and say I don't know if it will have any effect on the signing of that exact path in the payload/as installed)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This worked the last time we ran it, so I have to assume its something silly in the github action :D
Speaking of which, you'll want to copy this: https://github.com/macadmins/python/blob/main/.github/workflows/build_python_3.12.yml
And update it for 3.13 as well.
it's just typing
if [[ "${PYTHON_MAJOR_VERSION}" == "3.9" ]]; then | ||
/bin/ln -s "$PYTHON_BIN_NEW" "$TOOLSDIR/$TYPE/payload/usr/local/bin/managed_python3" | ||
fi | ||
if [[ "${PYTHON_MAJOR_VERSION}" == "3.10" ]]; then | ||
/bin/ln -s "$PYTHON_BIN_NEW" "$TOOLSDIR/$TYPE/payload/usr/local/bin/managed_python3" | ||
fi | ||
if [[ "${PYTHON_MAJOR_VERSION}" == "3.11" ]]; then | ||
/bin/ln -s "$PYTHON_BIN_NEW" "$TOOLSDIR/$TYPE/payload/usr/local/bin/managed_python3" | ||
fi | ||
if [[ "${PYTHON_MAJOR_VERSION}" == "3.12" ]]; then | ||
/bin/ln -s "$PYTHON_BIN_NEW" "$TOOLSDIR/$TYPE/payload/usr/local/bin/managed_python3" | ||
fi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, I knew there had to be a reason for this: https://github.com/macadmins/python/blob/main/.github/workflows/build_python_3.12.yml
Looks liker there is a workflow for each major Python 3 version, so removing this will break the other build files. I'd add 'em back for now.
If you have the energy to refactor later, I'd entertain that.
I hadn't properly anticipated the build environment and made probably inapplicable changes, closing in deference to starting over in a new one |
--no-unsign was the culprit stopping pip from updating (and several of the pip pkg versions in minimal weren't installing cleanly), dropped that flag and bumped sha's of RP/MP among other minor whitespace/comment tweaks.
Haven't tested with signing, more than happy to look at build stuff even though I have ~negative GitHub CI experience 😅