Skip to content

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

Closed
wants to merge 4 commits into from

Conversation

arubdesu
Copy link
Contributor

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

arubdesu added 2 commits June 18, 2025 15:05
--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
Copy link
Member

@natewalck natewalck left a 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
Copy link
Member

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.

# 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)
Copy link
Member

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gotit, restoring

Copy link
Member

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'
@arubdesu arubdesu requested a review from natewalck June 24, 2025 14:00
@arubdesu
Copy link
Contributor Author

As far as pip, can you expand on what was happening there? Was pip unable to update itself after installing the resulting package?

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

@erikng
Copy link
Member

erikng commented Jun 24, 2025

There's no way this will work on the notarizing side. I used that flag to get it to work.

@erikng
Copy link
Member

erikng commented Jun 24, 2025

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.

@arubdesu
Copy link
Contributor Author

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

@@ -96,7 +97,7 @@ if [ -d "${PIPCACHEDIR}" ]; then
/usr/bin/sudo /bin/rm -rf "${PIPCACHEDIR}"
fi

# kill homebrew packages
# # kill homebrew packages
Copy link
Member

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"
Copy link
Member

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.

Copy link
Contributor Author

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

Copy link
Contributor Author

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)

Copy link
Member

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
Comment on lines -144 to -155
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
Copy link
Member

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.

@arubdesu
Copy link
Contributor Author

I hadn't properly anticipated the build environment and made probably inapplicable changes, closing in deference to starting over in a new one

@arubdesu arubdesu closed this Jun 25, 2025
@arubdesu arubdesu deleted the 2025_updates branch June 25, 2025 00:49
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.

3 participants