Skip to content
This repository was archived by the owner on Jan 7, 2022. It is now read-only.

Temporary fix for CI #163

Merged
merged 2 commits into from
Jan 5, 2021
Merged

Temporary fix for CI #163

merged 2 commits into from
Jan 5, 2021

Conversation

vext01
Copy link
Member

@vext01 vext01 commented Jan 5, 2021

x.py install seems to be broken at the moment and this in turn broke our CI. This branch proposes a workaround.

The first commit is the best "easy" fix I can come up with. I'd like this to be a temporary workaround, as it's not optimal: copying the stage 2 dir copies a lot more stuff than you'd find in a typical sysroot. We could try and remove the bits we don't need, but it's difficult to know what, and we'd have to keep it in sync with upstream. I'm hoping upstream will fix x.py install soon...

Tested on ykjit/yk#206. And here is the forced build log.

The second commit allows us to run .buildbot.sh in scenarios where there isn't necessarily a bors merge commit on the top of the branch (i.e. during a manual or forced build). I needed this to test my changes and it seems generally useful.

.buildbot.sh Outdated
@@ -27,7 +28,12 @@ ulimit -d $((1024 * 1024 * 10)) # 10 GiB

# Build extended tools and install into TARBALL_TOPDIR.
mkdir -p ${TARBALL_TOPDIR}
/usr/bin/time -v ./x.py install --config .buildbot.config.toml
# We should be able to do this, but `x.py install` is broken.
Copy link
Member

Choose a reason for hiding this comment

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

What does "this" refer to here? [I eventually guessed, but it's a bit ambiguous.]

Copy link
Member Author

Choose a reason for hiding this comment

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

The following commented line. We can add some whitespace to make it more obvious?

Copy link
Member

Choose a reason for hiding this comment

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

It'll still be ambiguous I suspect: I think the comment needs rephrasing.

Copy link
Member Author

Choose a reason for hiding this comment

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

Give 5057cab a go.

.buildbot.sh Outdated
@@ -28,7 +28,7 @@ ulimit -d $((1024 * 1024 * 10)) # 10 GiB

# Build extended tools and install into TARBALL_TOPDIR.
mkdir -p ${TARBALL_TOPDIR}
# We should be able to do this, but `x.py install` is broken.
# `x.py install` currently broken, so we use a workaround for now.
Copy link
Member

Choose a reason for hiding this comment

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

"is currently"?

Copy link
Member Author

Choose a reason for hiding this comment

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

Indeed. Fixed.

@ltratt
Copy link
Member

ltratt commented Jan 5, 2021

Please squash.

vext01 added 2 commits January 5, 2021 14:46
This gets us going, but installs too many intermediate libraries. Once
upstream fixes `x.py install` we should revert to using it.
@vext01
Copy link
Member Author

vext01 commented Jan 5, 2021

Splat.

@vext01
Copy link
Member Author

vext01 commented Jan 5, 2021

Hold on a minute. I think there's a bug in the second commit.

@vext01
Copy link
Member Author

vext01 commented Jan 5, 2021

Sorry, that's rubbish, please carry on :)

@ltratt
Copy link
Member

ltratt commented Jan 5, 2021

bors r+

@bors
Copy link
Contributor

bors bot commented Jan 5, 2021

Build succeeded:

@bors bors bot merged commit e169bb4 into softdevteam:master Jan 5, 2021
@vext01 vext01 deleted the yk-fix-ci branch January 5, 2021 15:39
vext01 added a commit to vext01/ykrustc that referenced this pull request Jan 13, 2021
Upstream has fixed `x.py install` so we can go back to using it.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants