Skip to content

build: update build.ps1, use update-checkout for more dependencies #69555

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

Merged
merged 3 commits into from
Nov 2, 2023

Conversation

compnerd
Copy link
Member

Update the build.ps1 to match the current state which introduces -BuildTo and updates most of the dependencies. Take the opportunity to migrate to update-checkout for the majority of the dependencies. The SQLite amalgamation and installer image are still fetched later, and ICU is still cloned in the CI wrapper.

@compnerd compnerd requested a review from shahmishal as a code owner October 31, 2023 22:06
@compnerd
Copy link
Member Author

compnerd commented Nov 1, 2023

@swift-ci please build toolchain Windows platform

@compnerd
Copy link
Member Author

compnerd commented Nov 1, 2023

@swift-ci please build toolchain Windows platform

@shahmishal
Copy link
Member

shahmishal commented Nov 1, 2023

@parkera @iCharlesHu Are you ok with updating ICU tag to maint/maint-69 as its required for Windows?

@parkera
Copy link
Contributor

parkera commented Nov 1, 2023

@compnerd is there a way to verify that swift-corelibs-foundation's unit tests still pass with the updated ICU? Sometimes updates to formatting styles and such can cause issues.

@iCharlesHu
Copy link
Contributor

iCharlesHu commented Nov 1, 2023

@parkera @iCharlesHu Are you ok with updating ICU tag to maint/maint-69 as its required for Windows?

If Windows has always used 69 I don't see an issue. Although I'd say we should definitely test swift-corelibs-foundation to make sure the new ICU doesn't break things.

In the past we've seen ICU change things (like the separator it uses etc) between releases so this could break some tests.

@compnerd
Copy link
Member Author

compnerd commented Nov 1, 2023

@iCharlesHu, yeah, we have always used 69.1 for Windows. @parkera, I can stack the change to get ICU switched over to 69.1 and do a cross-repository test for Linux. I know that the build on Windows will also test Foundation as well.

Update the build.ps1 to match the current state which introduces
`-BuildTo` and updates most of the dependencies. Take the opportunity to
migrate to update-checkout for the majority of the dependencies. The
SQLite amalgamation and installer image are still fetched later, and ICU
is still cloned in the CI wrapper.
Unify the non-Darwin targets to ICU 69. Windows has been using ICU 69
since 5.3, Linux was at 65.
Update next to match main for dependencies. This includes:
- CURL
- ICU
- libxml2
- zlib
@compnerd
Copy link
Member Author

compnerd commented Nov 1, 2023

@swift-ci please test

@compnerd
Copy link
Member Author

compnerd commented Nov 2, 2023

@parkera https://ci.swift.org/job/swift-corelibs-foundation-PR-Linux/306/ is the test run - seems that it doesn't impact the tests.

@compnerd compnerd requested a review from shahmishal November 2, 2023 00:29
@shahmishal
Copy link
Member

We will need to have these changes cherry-picked into release/5.10.

@compnerd
Copy link
Member Author

compnerd commented Nov 2, 2023

@shahmishal I think that we want to get the changes into main and then cherry-pick. I can prepare a cherry-pick before it is merged of course if you prefer.

@shahmishal
Copy link
Member

@shahmishal I think that we want to get the changes into main and then cherry-pick. I can prepare a cherry-pick before it is merged of course if you prefer.

@compnerd It can be done after, just wanted to make sure we cherry-pick these changes to support the nightly toolchain for release/5.10.

Comment on lines 92 to 100
"curl": {
"remote": { "id": "curl/curl" }
},
"libxml2": {
"remote": { "id": "gnome/libxml2" }
},
"zlib": {
"remote": { "id": "madler/zlib" }
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do we suddenly need these, and icu, on macOS?

@vanvoorden
Copy link
Contributor

@compnerd Could this be related to #70966?

@compnerd
Copy link
Member Author

No, this only adds the repositories to the set, it doesn't write files into it.

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.

6 participants