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

Add truffleruby-head to CI #132

Merged
merged 1 commit into from Aug 21, 2020
Merged

Add truffleruby-head to CI #132

merged 1 commit into from Aug 21, 2020

Conversation

bjfish
Copy link
Contributor

@bjfish bjfish commented Aug 21, 2020

Please add truffleruby-head to travis CI so we can see the status of the latest TruffleRuby.

@fxn
Copy link
Owner

fxn commented Aug 21, 2020

Awesome, looking forward to a green build! ❤️

@fxn fxn merged commit 1790e82 into fxn:master Aug 21, 2020
@eregon
Copy link

eregon commented Aug 22, 2020

Seems like truffleruby-head passed on this run:
https://travis-ci.com/github/fxn/zeitwerk/jobs/376296186
But failed here:
https://travis-ci.com/github/fxn/zeitwerk/jobs/376301423

So there looks like there is something transient.

@eregon
Copy link

eregon commented Aug 22, 2020

That could be some unsynchronized modification of $LOADED_FEATURES, because the line failing is
https://github.com/oracle/truffleruby/blob/cc7f184c7c153b29f113c727a1695bec1af0c62a/src/main/ruby/truffleruby/core/truffle/feature_loader.rb#L145
which means $LOADED_FEATURES[i] is nil.

But I don't see any Thread in that test:


so it sounds unlikely.

So maybe the @loaded_features_index simply points to a removed entry from $LOADED_FEATURES?

@bjfish I'd suggest to try reproducing locally, and change
next if loaded_feature.size < feature.size
to
next if loaded_feature.nil? || loaded_feature.size < feature.size (+ a comment explaining why it can be nil)

@fxn
Copy link
Owner

fxn commented Aug 27, 2020

In case it helps, Zeitwerk mutates $LOADED_FEATURES using Array#reject! here.

The motivation for that is that reloading is implemented as "remove constant + load its definition again". Since Module#autoload loads with require, we need to mutate $LOADED_FEATURES to workaround its idempotency.

@bjfish bjfish deleted the truffleruby-ci branch August 27, 2020 14:47
@bjfish
Copy link
Contributor Author

bjfish commented Aug 27, 2020

@fxn We found a bug related to the test failures and a fix has been pushed: oracle/truffleruby@985b21c. I expect future builds to pass for truffleruby-head.

@fxn
Copy link
Owner

fxn commented Aug 28, 2020

@bjfish I am curious, why the emphasis on pop/shift? The API to mutate arrays has several methods, and you could combine them to edit the thing in place at arbitrary positions and preserve the size.

@eregon
Copy link

eregon commented Aug 28, 2020

Other mutations will cause a copy of the the underlying storage (which is copy-on-write).
pop/shift do not need to modify the underlying storage, and so they just adjust the size or offset.

@eregon
Copy link

eregon commented Aug 28, 2020

@fxn Assuming truffleruby-head passes the CI reliably now, do you think we could remove it from allow_failures?

@fxn
Copy link
Owner

fxn commented Aug 29, 2020

@eregon Removed and green 🎉.

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.

None yet

3 participants