This is an archive of the discontinued LLVM Phabricator instance.

Rework go bindings so that binding tests work fine
AbandonedPublic

Authored by serge-sans-paille on Feb 13 2020, 3:09 AM.

Details

Summary

Since llvm-go is no longer part of the distribution, the binding testing have started to fail.

This is a straight forward attempt at fixing that. Note that I'm by no mean a go developer, which is probably reflected by the commit.

Diff Detail

Unit TestsFailed

Event Timeline

Herald added a project: Restricted Project. · View Herald TranscriptFeb 13 2020, 3:09 AM
serge-sans-paille edited the summary of this revision. (Show Details)Feb 13 2020, 3:09 AM

So this is just using the system go compiler instead of llvm-go and all the files move to src/ because that's how the normal go compiler expects the workspace to look like?

So this is just using the system go compiler instead of llvm-go and all the files move to src/ because that's how the normal go compiler expects the workspace to look like?

yeah, that's basically it. Plus a few lit config stuff to make sure everything works. Which is not settled yet per the validation :-)

teemperor accepted this revision.Feb 13 2020, 4:32 AM

Also works on my machine now, so LGTM. Thanks!

This revision is now accepted and ready to land.Feb 13 2020, 4:32 AM
This revision was automatically updated to reflect the committed changes.
pcc reopened this revision.Feb 13 2020, 8:35 AM

This broke users of the go bindings who were using build.sh, and changed the import path for the bindings unnecessarily. Can we please just bring back llvm-go as I requested before?

This revision is now accepted and ready to land.Feb 13 2020, 8:35 AM
In D74540#1874534, @pcc wrote:

This broke users of the go bindings who were using build.sh, and changed the import path for the bindings unnecessarily. Can we please just bring back llvm-go as I requested before?

@pcc Are you the best one fixing this issue properly :) ?

I guess this patch fixed the /mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/build/test/Bindings/Go/Output/go.test.script: line 1: llvm-go: command not found diagnostic we continuously get from the premerge bot, so I still appreciate Serge's work.... https://reviews.llvm.org/harbormaster/unit/view/3381/

pcc added a comment.Feb 24 2020, 9:21 AM

Now that llvm-go has been brought back, I've reverted this change.

serge-sans-paille abandoned this revision.Feb 26 2020, 1:16 AM

llvm-go is still a thing, this patch is no longer needed

llvm/bindings/go/llvm/transforms_scalar.go