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

neko 2.1.0 #982

Closed
wants to merge 1 commit into from
Closed

neko 2.1.0 #982

wants to merge 1 commit into from

Conversation

andyli
Copy link
Contributor

@andyli andyli commented May 8, 2016

It has just been released and is available at http://nekovm.org/download.

@andyli andyli mentioned this pull request May 8, 2016
8 tasks
@MikeMcQuaid
Copy link
Member

Test and bottling failures here unfortunately.

@apjanke
Copy link
Contributor

apjanke commented May 8, 2016

Failed build here.

        ==> Determining neko bottle revision...
==> Bottling neko-2.1.0.yosemite.bottle.tar.gz...
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/install_name_tool: the __LINKEDIT segment does not cover the end of the file (can't be processed) in: /usr/local/Cellar/neko/2.1.0/bin/nekoc
Error: undefined method `write' for nil:NilClass
Please report this bug:
    https://git.io/brew-troubleshooting
/usr/local/Library/Homebrew/cmd/bottle.rb:258:in `block (2 levels) in bottle_formula'
/usr/local/Library/Homebrew/utils.rb:481:in `ignore_interrupts'
/usr/local/Library/Homebrew/cmd/bottle.rb:257:in `ensure in block in bottle_formula'
/usr/local/Library/Homebrew/cmd/bottle.rb:257:in `block in bottle_formula'
/usr/local/Library/Homebrew/keg.rb:245:in `block in lock'
/usr/local/Library/Homebrew/formula_lock.rb:29:in `with_lock'
/usr/local/Library/Homebrew/keg.rb:241:in `lock'
/usr/local/Library/Homebrew/cmd/bottle.rb:193:in `bottle_formula'
/usr/local/Library/Homebrew/cmd/bottle.rb:410:in `block in bottle'
/usr/local/Library/Homebrew/cmd/bottle.rb:409:in `each'
/usr/local/Library/Homebrew/cmd/bottle.rb:409:in `bottle'
/usr/local/Library/brew.rb:87:in `<main>'

Looks like the neko build might need header name padding? That can be done by passing -headerpad_max_install_names to the linker. See shapelib or search this repo for "headerpad" for examples. You wouldn't have encountered this locally since local builds don't normally have a bottling step.

And then it looks like brew bottle has an internal bug where its cleanup code is referencing a possibly-unitialized variable. That's on us, and it should go away here once the header padding is fixed.

That test failure looks like it might just be a result of the link rewriting failure that's causing the bottle step to fail. Try fixing the header padding and see if it goes away.

@andyli
Copy link
Contributor Author

andyli commented May 9, 2016

The binary that has issue with install_name_tool, nekoc, is not created by a C compiler. It is in fact created by the nekotools boot command, which combines the neko vm binary with a chunk of neko bytecode. It doesn't "fix" the Mach-O header so it would be invalid in the eye of install_name_tool, but it will just work fine.

I guess the use of install_name_tool is to make the executables relocatable? Can it be skipped for now, e.g. to create non-relocatable bottles, until the issue has been fixed upstream?

@MikeMcQuaid
Copy link
Member

We don't have any current mechanism to do that skipping; we we install_name_tool for more than just relocatable bottles, unfortunately.

@UniqMartin
Copy link
Contributor

I guess the use of install_name_tool is to make the executables relocatable? Can it be skipped for now, e.g. to create non-relocatable bottles, until the issue has been fixed upstream?

We cannot create non-relocatable bottles, but what we can do is disable bottle creation, effectively forcing every Homebrew user of neko to build the formula from source. This might be a viable option to get this version out before the upstream issue is fixed. This would mean replacing the bottle do block with:

# Remove when resolved: https://github.com/HaxeFoundation/neko/issues/130
bottle :disable, "Neko binaries cannot be edited with 'install_name_tool'."

However, that's something we do extremely rarely and I'm not sure other maintainers will approve.


Sadly, there's another breaking (and unrelated) issue with 2.1.0: Libraries are installed to #{prefix}/lib and binaries to #{prefix}/bin. The latter link to @rpath/libneko.2.dylib and have an embedded RPATH of @executable_path/ so none of the binaries actually work out of the box.

@UniqMartin UniqMartin added the upstream issue An upstream issue report is needed label May 9, 2016
@MikeMcQuaid
Copy link
Member

However, that's something we do extremely rarely and I'm not sure other maintainers will approve.

Yeh, I'd rather see us stick on an old version than do that, personally.

url "https://github.com/HaxeFoundation/neko.git", :revision => "22c49a89b56b9f106d7162710102e9475227e882"
version "2.0.0-22c49a8"
revision 2
url "https://github.com/HaxeFoundation/neko.git", :tag => "v2-1-0", :revision => "63c8ff1a20e72e295456a1b3083209efec1e8ab2"
Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason not to use the GitHub release tarball https://github.com/HaxeFoundation/neko/archive/v2-1-0.tar.gz instead? We prefer that for tagged releases instead of making every user do a Git clone.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just revised the formula to use the tarball from http://nekovm.org/. It was created by git archive, so it should be the same as the Github one.

@andyli
Copy link
Contributor Author

andyli commented May 11, 2016

I have just used a workaround to avoid using nekotools boot to build binaries. The detail is explained in HaxeFoundation/neko#130 (comment).

For @executable_path/, it was used because the binary archive distributed in http://nekovm.org/download is using a flat directory structure. I've turned it off with -DRELOCATABLE=OFF now to let homebrew handle it.

@andyli
Copy link
Contributor Author

andyli commented May 11, 2016

The test now failed with

failed: brew readall --aliases --syntax
Stacktrace

        /usr/local/Library/Homebrew/cmd/pull.rb:555: warning: possibly useless use of a variable in void context

Any idea?

@UniqMartin UniqMartin removed the upstream issue An upstream issue report is needed label May 12, 2016
@UniqMartin
Copy link
Contributor

This was a silly mistake on our side that has since been resolved and isn't caused by your build. Thanks for fixing the problem upstream!

Merged in 8292d2e. Thank you for this contribution to Homebrew, @andyli! 🎉

UniqMartin added a commit that referenced this pull request May 12, 2016
Patch was already applied upstream, thus only apply it to stable build.
Fixes an oversight in my review of #982.
@UniqMartin
Copy link
Contributor

@andyli FYI, c512ffa – Sorry I missed that in my final review of this PR.

@andyli
Copy link
Contributor Author

andyli commented May 12, 2016

@UniqMartin ah! Thanks for fixing that!

@Homebrew Homebrew locked and limited conversation to collaborators May 4, 2018
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.

4 participants