Skip to content

Conversation

marinaaisa
Copy link
Member

@marinaaisa marinaaisa commented Jul 28, 2025

Bug/issue #156708326, if applicable:

Summary

By aiming to update the Node version to 22.17.0 on Swift Docker for Swift DocC Render [1], we need to specify version 3 of python, otherwise it will try to use python 2 which it's not available in Node 22.

[1] swiftlang/swift-docker#487

Dependencies

This change unblocks this PR in Swift Docker: swiftlang/swift-docker#487

Testing

Steps:

  1. Make sure that SwiftCI works.

Checklist

Make sure you check off the following items. If they cannot be completed, provide a reason.

  • Added tests
  • Ran npm test, and it succeeded
  • Updated documentation if necessary

@marinaaisa marinaaisa requested a review from mportiz08 July 28, 2025 17:51
@marinaaisa
Copy link
Member Author

@swift-ci test

@marinaaisa
Copy link
Member Author

@swift-ci test

@marinaaisa
Copy link
Member Author

@swift-ci test

1 similar comment
@marinaaisa
Copy link
Member Author

@swift-ci test

@marinaaisa marinaaisa requested a review from mportiz08 July 31, 2025 13:01
Copy link
Contributor

@mportiz08 mportiz08 left a comment

Choose a reason for hiding this comment

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

Would it make sense to pull these changes into the update-node branch/PR?

When I try building the new image from the associated swift-docker PR, I'm getting an error from running npm ci (not sure why but most likely since this branch is still using/checking the older node version while that PR updates the node version in the Dockerfile)

@marinaaisa
Copy link
Member Author

Would it make sense to pull these changes into the update-node branch/PR?

My idea was:

  1. Merge this PR Make build-script-helper.py use python3 #956
    which unblocks Update Node version to 22.17.0 swift-docker#487
  2. Merge Update Node version to 22.17.0 swift-docker#487 which unblocks CI for Update Node.js & NPM #952
  3. Run @swift-ci test in Update Node.js & NPM #952 and see all the test passings
  4. Merge Update Node.js & NPM #952

does this make sense?

When I try building the new image from the associated swift-docker PR, I'm getting an error from running npm ci (not sure why but most likely since this branch is still using/checking the older node version while that PR updates the node version in the Dockerfile)

I have cherry-picked 73b3197 into #952 to unblock your testing. We can close this PR if you prefer! The other 2 commits are very minor things.

By aiming to update the Node version to 22.17.0 on Swift Docker for
Swift DocC Render [1], we need to specify the version 3 of python, otherwise
it will try to use python 2 which it's not available in Node 22.

[1] swiftlang/swift-docker#487
Removed from __future__ import print_function on line 16, which is not
needed in Python 3 since print() is already a function by default.
Updated the node version check to properly handle the fact that
subprocess.check_output() returns bytes in Python 3, not strings. Added
code to decode bytes to UTF-8 string and strip whitespace before
checking the version.
@marinaaisa
Copy link
Member Author

@swift-ci test

@marinaaisa marinaaisa mentioned this pull request Sep 16, 2025
3 tasks
@mportiz08
Copy link
Contributor

I think that makes sense, and I see now why you did it that way. Sorry for the confusion

@mportiz08 mportiz08 merged commit 595cae7 into swiftlang:main Sep 16, 2025
1 check passed
@mportiz08 mportiz08 deleted the python3 branch September 16, 2025 18:10
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.

2 participants