Skip to content

Add tizen armel build CI for pull request #56281

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 9 commits into from
Oct 13, 2021
Merged

Add tizen armel build CI for pull request #56281

merged 9 commits into from
Oct 13, 2021

Conversation

@clamp03
Copy link
Member

@clamp03 clamp03 commented Jul 26, 2021

With ubuntu-18.04-cross-armel-tizen docker image,
check Tizen_armel build for each pull reqest.

@ghost ghost added the community-contribution Indicates that the PR has been added by a community member label Jul 26, 2021
@ghost
Copy link

ghost commented Jul 26, 2021

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.

@clamp03 clamp03 mentioned this pull request Jul 26, 2021
Closed
@clamp03 clamp03 force-pushed the tizen-armel-build-ci branch from f2b2066 to 4156526 Compare July 26, 2021 00:17
@clamp03
Copy link
Member Author

clamp03 commented Jul 26, 2021

Currently, it fails before #55789 is merged.

am11
am11 reviewed Jul 26, 2021
View reviewed changes
@karelz karelz mentioned this pull request Jul 26, 2021
Closed
@clamp03 clamp03 force-pushed the tizen-armel-build-ci branch from 4156526 to 99a316b Compare July 27, 2021 04:41
@karelz karelz mentioned this pull request Jul 28, 2021
Closed
@ghost
Copy link

ghost commented Jul 28, 2021

Tagging subscribers to this area: @hoyosjs
See info in area-owners.md if you want to be subscribed.

Issue Details

With ubuntu-18.04-cross-armel-tizen docker image,
check Tizen_armel build for each pull reqest.

Author: clamp03
Assignees: -
Labels:

area-Infrastructure-coreclr, community-contribution
Milestone: -

@clamp03 clamp03 mentioned this pull request Jul 30, 2021
Merged
@clamp03 clamp03 force-pushed the tizen-armel-build-ci branch from 99a316b to 72f1373 Compare July 31, 2021 03:53
@am11 am11 mentioned this pull request Jul 31, 2021
Closed
clamp03
clamp03 commented Aug 2, 2021
View reviewed changes
eng/pipelines/coreclr/templates/build-job.yml Outdated
Copy link
Member Author

@clamp03 clamp03 Aug 2, 2021

Choose a reason for hiding this comment

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

Right. It needs a portable option and some others.
However, as I know, there is no ARMEL environment for test in the CI.
To prevent build failure, I just want to add Tizen ARMEL build this time.
Thank you.

@clamp03
Copy link
Member Author

clamp03 commented Aug 2, 2021

@safern Could you review this PR? I added Tizen ARMEL Build and it works. However, I think I missed some details. Thank you.

@clamp03 clamp03 force-pushed the tizen-armel-build-ci branch 2 times, most recently from 2d50683 to 1a03b2a Compare August 3, 2021 06:28
@clamp03
Copy link
Member Author

clamp03 commented Aug 4, 2021

@safern I think errors are not related to this patch. Could you please review it?

@safern
Copy link
Member

safern commented Aug 4, 2021

@safern I think errors are not related to this patch. Could you please review it?

Sorry, @clamp03, I was OOF. Will take a look tomorrow.

@clamp03
Copy link
Member Author

clamp03 commented Aug 4, 2021

@safern Thank you! :)

@clamp03 clamp03 force-pushed the tizen-armel-build-ci branch from 1a03b2a to e00cba4 Compare August 9, 2021 02:31
@clamp03
Copy link
Member Author

clamp03 commented Aug 9, 2021

Oops. I pushed the old patch by mistake. Resolved now.

safern
safern reviewed Aug 9, 2021
View reviewed changes
@safern
Copy link
Member

safern commented Aug 9, 2021

This is currently just adding support for the coreclr build job template. Do we want to protect the dotnet/runtime end-to-end build? i.e, building all the subsets and producing a runtime pack for tizen?

@clamp03 clamp03 force-pushed the tizen-armel-build-ci branch 2 times, most recently from 54a6181 to 5259807 Compare August 10, 2021 01:32
@clamp03
Copy link
Member Author

clamp03 commented Aug 10, 2021 •
edited
Loading

This is currently just adding support for the coreclr build job template. Do we want to protect the dotnet/runtime end-to-end build? i.e, building all the subsets and producing a runtime pack for tizen?

I think the others are handled by arm32 (hardfp) build.
If armel build fails again, I will update it next time.
Thank you.

@clamp03 clamp03 force-pushed the tizen-armel-build-ci branch 2 times, most recently from 1f5fb96 to afc7249 Compare August 10, 2021 03:23
@clamp03 clamp03 force-pushed the tizen-armel-build-ci branch from 0317434 to 53f21c0 Compare October 5, 2021 03:31
@clamp03
Copy link
Member Author

clamp03 commented Oct 5, 2021

I think failures are not related to this PR. Could you please review and merge?
Thank you.

@jkotas
Copy link
Member

jkotas commented Oct 5, 2021 •
edited
Loading

@safern Could you please take final look and merge if it looks good?

safern
safern reviewed Oct 6, 2021
View reviewed changes
eng/pipelines/common/platform-matrix.yml Outdated
Copy link
Member

@safern safern Oct 6, 2021

Choose a reason for hiding this comment

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

Last question. It is currently showing up on the UI as Linux armel as the name, do we want it to be that or Tizen armel to make it more clear?

Copy link
Member

@jkotas jkotas Oct 7, 2021

Choose a reason for hiding this comment

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

Yes, it would be nice to have more distinct name for this (Tizen armel).

Copy link
Member Author

@clamp03 clamp03 Oct 7, 2021

Choose a reason for hiding this comment

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

Thank you for the review. I updated the name on the UI.

Copy link
Member Author

@clamp03 clamp03 Oct 7, 2021

Choose a reason for hiding this comment

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

@safern @jkotas I think test failures are not caused by this PR. Please review again. Thank you.

Copy link
Member Author

@clamp03 clamp03 Oct 12, 2021

Choose a reason for hiding this comment

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

Could you please review it?

This was referenced Oct 7, 2021
Closed
Closed
@safern
Copy link
Member

safern commented Oct 12, 2021 •
edited
Loading

@clamp03 I see that there is an error on the build coreclr runtime step:

/__w/1/s/eng/native/init-distro-rid.sh: line 44: [: missing `]'
/__w/1/s/eng/native/init-distro-rid.sh: line 44: tizen: command not found

Also, I see that in that step the distro rid is resolved as: __DistroRid: tizen.6.5.0-armel and in Build managed product components and packages step it is resolved as: __DistroRid: linux-armel

I think we need to fix this?

@clamp03 clamp03 force-pushed the tizen-armel-build-ci branch from fac10bf to 49699d6 Compare October 12, 2021 06:34
@clamp03
Copy link
Member Author

clamp03 commented Oct 12, 2021

@safern Thank you for the review

@clamp03 I see that there is an error on the build coreclr runtime step:

/__w/1/s/eng/native/init-distro-rid.sh: line 44: [: missing `]'
/__w/1/s/eng/native/init-distro-rid.sh: line 44: tizen: command not found

Sorry. I missed error messages.
This is not related to this Patch. It is because of #59178.
I fixed and added a patch in this PR.

Also, I see that in that step the distro rid is resolved as: __DistroRid: tizen.6.5.0-armel and in Build managed product components and packages step it is resolved as: __DistroRid: linux-armel

I think we need to fix this?

In our Tizen, we use clr and libs.native with portable build for tizen and the others for common linux-armel.
So I think we don't need to fix this.

am11
am11 reviewed Oct 12, 2021
View reviewed changes
@clamp03 clamp03 force-pushed the tizen-armel-build-ci branch from 49699d6 to aef39e1 Compare October 13, 2021 00:05
@clamp03
Copy link
Member Author

clamp03 commented Oct 13, 2021 •
edited
Loading

An error (multiline comment) in runtime (CoreCLR GCC Product BUild Linux x86 Checked) is not related to this PR.
It is by #58727. There is \ in the end of comment line 10915. I think it is better to fix by the author of PR 58727 in other PR. CC @benjamin-hodgson

runtime/src/coreclr/jit/morph.cpp

Lines 10914 to 10916 in 80e843f

// op1 op2 ==> tree
// | | / \
// x y x y

@safern @jkotas Could you please review this PR and merge if it is good?

@am11
Copy link
Member

am11 commented Oct 13, 2021

I pushed a commit to fix gcc build in #60328 (period after trailing \ on single-line comment line is how we fixed issues in dotnet/coreclr in morph.cpp and other places).

@akoeplinger akoeplinger mentioned this pull request Oct 13, 2021
Merged
@safern
Copy link
Member

safern commented Oct 13, 2021

@hoyosjs @jkoritzinsky it seems like the GCC build is broken?

@safern
Copy link
Member

safern commented Oct 13, 2021

It seems like that break was fixed on: #60333.

@safern safern merged commit 0cb73e8 into dotnet:main Oct 13, 2021
@safern
Copy link
Member

safern commented Oct 13, 2021

Thanks, @clamp03 🎉

@clamp03 clamp03 deleted the tizen-armel-build-ci branch October 14, 2021 01:13
@ghost ghost locked as resolved and limited conversation to collaborators Nov 13, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@jkotas jkotas jkotas left review comments

@safern safern safern approved these changes

+1 more reviewer

@am11 am11 am11 left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

area-Infrastructure-coreclr community-contribution Indicates that the PR has been added by a community member

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@clamp03 @safern @jkotas @am11 @eerhardt