Skip to content
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

Fix build pipeline #262

Closed
wants to merge 2 commits into from
Closed

Conversation

ejose19
Copy link

@ejose19 ejose19 commented Feb 23, 2021

No description provided.

@ejose19
Copy link
Author

ejose19 commented Feb 23, 2021

@Morganamilo can you trigger a pipeline to confirm if this fixes the issue? So #259 can cherry-pick the 2nd commit and merge.

@Morganamilo
Copy link
Owner

You should be able to test this yourself by just pushing a tag starting with v.

@ejose19 ejose19 force-pushed the ej/fixBuildPipeline branch 5 times, most recently from 569dcc7 to 2c636a1 Compare February 23, 2021 22:10
@Morganamilo
Copy link
Owner

I see a lot of extra stuff in this PR stuff that we were exploring in #259. Is that stuff actually needed for this to work or are you just building on that?

@ejose19
Copy link
Author

ejose19 commented Feb 23, 2021

Was waiting for pipeline (https://github.com/ejose19/paru/actions/runs/594038571) to finish to confirm you, but I managed to get it to work. Main issue is actions/runner-images#2658, which first resolution was to use actions/runner-images#2698 (comment), however that didn't work as seem gh is using an older host, so the other resolution was to use a patched version of glibc that works for linux4 (based on PR lxqt/lxqt-panel#1562) which solves the issue for x86_64 (arm build doesn't seems to be affected).

Regarding the pacman.conf changes, yes they're needed. Seems github screwed another thing so these two must be kept until solved upstream.

EDIT: Pipeline completed successfully.

@Morganamilo
Copy link
Owner

I very much hate how awful the hacks are here but it works.

@ejose19
Copy link
Author

ejose19 commented Feb 24, 2021

I agree with you, also I'm not very comfortable using an unsigned package in the base image of paru build. So either:

  • Wait for an official package of glibc-linux4
  • Wait for github to solve this (this started over 2 weeks, so who knows how long it will take)
  • Migrate building to a more mature solution (travis, circleci)

I will close this PR for now.

@ejose19
Copy link
Author

ejose19 commented Feb 24, 2021

@Morganamilo I've continue to investigate about the issue, seems that buildx is not fully compatible with the patch of runc, when I removed buildx the 1st workaround worked without doing anything extra, so I filed moby/buildkit#1995 to monitor the progress of that particular issue.

Regarding the other issues (ro mounts), it's buildx again with moby/buildkit#1267, and applying the workaround specified in comments allows to continue with the simplest change in Dockerfile.

So either we keep waiting for a proper solution, or we split the build.yml so aarch64 is built with buildx (aarch64 is not affected by glibc bug) and x86_64 is built using docker build directly (on which the latest runc works and there won't be a need to edit pacman.conf)

@Morganamilo
Copy link
Owner

The build worked for without the last commit, what does the last commit actually fix?

@Morganamilo
Copy link
Owner

My hope is this will be fixed by the next release. At least some of the issuess. I used this PR to build 1.3.0, hoping the next release will be a while off.

@ejose19
Copy link
Author

ejose19 commented Feb 24, 2021

This is still a work in progress, as I said I'm waiting for a reply on moby/buildkit#1995 to finish this PR. The previous "commit" included the 2nd workaround but it was only a proof of concept to confirm the issue solved, I'm pretty sure it's not the solution we want and didn't thought you would publish that one.

So I'll mark this as a draft instead but wanted to share my advances on this with you. And I doubt this will be completely fixed by next release as buildx clearly stated they won't fix the readonly mounts, so at least that fix must be included in this PR.

@ejose19 ejose19 marked this pull request as draft February 24, 2021 21:37
@ejose19 ejose19 force-pushed the ej/fixBuildPipeline branch 2 times, most recently from 0450e1b to a143b53 Compare February 25, 2021 01:55
@ejose19 ejose19 marked this pull request as ready for review February 25, 2021 03:11
@ejose19
Copy link
Author

ejose19 commented Feb 25, 2021

The folks at moby already solved moby/buildkit#1995, so this PR is complete and you can merge it (there's no need to use any patch now). Here's the pipeline to confirm it.

I would recommend releasing another version and deprecate/hide current latest one at that one was built with an unsigned package, and I'm sure paru users will appreciate using only trusted builds.

@ejose19 ejose19 force-pushed the ej/fixBuildPipeline branch 2 times, most recently from db071d0 to cbfbbe2 Compare February 26, 2021 13:10
@ejose19
Copy link
Author

ejose19 commented Feb 26, 2021

They already updated the default moby/buildkit:buildx-stable-1 that comes with setup-buildx-action, so I removed the reference to latest, leaving this PR with only the "essential" changes.

@Morganamilo Do you need anything else to be done here before merging?

@ejose19 ejose19 closed this Apr 22, 2021
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