-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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 llvm ios #55222
Add llvm ios #55222
Conversation
I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label. |
- script: rm -r -f $(Build.SourcesDirectory)/src/mono/sample/iOS/bin | ||
workingDirectory: $(Build.SourcesDirectory)/src/mono/sample/iOS | ||
displayName: Clean bindir |
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 assume this got added because we were seeing some build flakiness?
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.
good catch - it's left over from my previous scheme of running the two tests back to back instead of in different runs. Will remove.
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.
Oh, no. I was thinking about the wrong place. This is just making sure we are clean to rebuild with LLVM. It stays.
- template: /eng/pipelines/common/download-artifact-step.yml | ||
parameters: | ||
unpackFolder: $(Build.SourcesDirectory)/iosHelloWorld/llvm | ||
cleanUnpackFolder: false | ||
artifactFileName: 'iOSSampleAppLLVM.tar.gz' | ||
artifactName: 'iOSSampleAppLLVM' | ||
displayName: 'iOS Sample App LLVM' |
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 should be conditioned on LLVM status we pass in, right? As it stands right now we will always download both artifacts.
if($iOSLlvmBuild) { | ||
Copy-Item -path "$SourceDirectory\iosHelloWorld\nollvm" $PayloadDirectory\iosHelloWorld\llvm -Recurse | ||
} else { | ||
Copy-Item -path "$SourceDirectory\iosHelloWorld\llvm" $PayloadDirectory\iosHelloWorld\nollvm -Recurse | ||
} | ||
|
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 guess this prevents it from going to down to Helix machines, but we still download the artifacts for both as part of the AzDO job.
Most comments are nit, really. If this has gone through private AzDO and worked LGTM. |
@@ -152,7 +152,12 @@ if ($iOSMono) { | |||
{ | |||
mkdir $WorkItemDirectory | |||
} | |||
Copy-Item -path "$SourceDirectory\iosHelloWorld\nollvm" $PayloadDirectory\iosHelloWorld\nollvm -Recurse | |||
if($iOSLlvmBuild) { |
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.
Do we mean to copy "$SourceDirectory\iosHelloWorld\nollvm" to $PayloadDirectory\iosHelloWorld\nollvm and "$SourceDirectory\iosHelloWorld\llvm" to $PayloadDirectory\iosHelloWorld\llvm instead?
No description provided.