-
Notifications
You must be signed in to change notification settings - Fork 2.7k
[ARM/CI] Add manual triggering CI for arm and armel #9853
Conversation
@dotnet-bot test ci please |
@dotnet-bot test ci please |
0512a53
to
1bd2c8a
Compare
@dotnet-bot test ci please |
@dotnet-bot test ci please |
1 similar comment
@dotnet-bot test ci please |
@dotnet-bot test ci please |
@dotnet-bot test ci please |
@dotnet-bot test ci please |
@dotnet-bot test ci please |
1 similar comment
@dotnet-bot test ci please |
I've ran Among the generated jobs, I think following jobs are added or changed by this PR.
@gkhanna79 I'm looking for someone who can review and test this changes to prevent a accidental change which might cause break existing CI. Can you invite someone who can look into this? |
@mmitche Can you PTAL? |
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.
Looks good except for the couple questions
Utilities.getFolderName(branch) | ||
|
||
// Copy the Windows test binaries and the Corefx build binaries | ||
copyArtifacts(WindowTestsName) { |
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 doesn't look quite right. Is this trying to copy corefx binaries from latest?
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'm trying to copy (1) test binary and (2) corefx binary from the latest to prepare coreclr test.
Because I don't know well about relation between coreclr CI and corefx CI, I just followed a way of current CI for arm and other architectures.
https://github.com/dotnet/coreclr/blob/master/netci.groovy#L2505-L2516 (for arm)
https://github.com/dotnet/coreclr/blob/master/netci.groovy#L2884-L2900 (for other)
If this is not a right way, please let me know how to fetch correct banries of corefx and test to prepare testing.
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.
If comments is misleading I will update the comments.
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.
Okay I think that's right
netci.groovy
Outdated
arm_abi='armel' | ||
corefx_os='tizen' | ||
} | ||
copyArtifacts("${corefxFolder}/${corefx_os}_${arm_abi}_cross_${lowerConfiguration}") { |
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.
Or is this the intended corefx copy.
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.
Addressed above :)
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 think you have it all correct except there is no checked configuration of corefx (only debug and reease). So you might want to copy from the release version in the checked and release coreclr cases, and from the debug version in the debug coreclr case
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.
Oops, I almost missed it.
Thank you for the feedback and I've updated this as you suggested :)
Is this ready for merge? |
I'm ready and waiing for @mmitche, since he has some questions which I've answered. |
@dotnet-bot test ci please (one final check, but otherwise looks good) |
@dotnet-bot test ci please |
@dotnet-bot test ci please |
Add follwoing manual triggering CI for arm and armel. This will be used by new arm32 CI script which is already applied to Core-Setup and CoreFX. Ubuntu arm Cross Release Build Ubuntu arm Cross Debug Build Ubuntu16.04 arm Cross Release Build Ubuntu16.04 arm Cross Debug Build Tizen arm Cross Release Build Tizen arm Cross Debug Build Currently there are two default CI for ARM cross using ARM emulator which will be disabled after enabling new arm32 CI. Linux ARM Emulator Cross Release Build Linux ARM Emulator Cross Debug Build Signed-off-by: Hyung-Kyu Choi <hk0110.choi@samsung.com>
@dotnet-bot test ci please |
@mmitche I've update the PR by applying your feedback and I think it's ready for merge. |
@gkhanna79 @mmitche Can you please take a look again ? It's ready for merge and I can test new arm CI #9445 after this PR is merged. Thanks. |
@gkhanna79 @mmitche I don't want to be impatient, but I'm curious when this will be merged. |
Add following manual triggering CI for arm and armel.
This will be used with a new arm32 CI script (#9445) which is already applied to CoreFx(dotnet/corefx#16378) and Core-Setup (dotnet/core-setup#1512).
Currently there are two default CI for ARM cross using ARM emulator which will be disabled after enabling new arm32 CI.
After merging this PR, new arm32 CI script in #9445 will be tested with new commands.
Related issues:
https://github.com/dotnet/core-setup/issues/790
https://github.com/dotnet/core-setup/issues/725