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

Added noop operations for js/wasm build target #478

Merged
merged 2 commits into from
Jun 29, 2022
Merged

Added noop operations for js/wasm build target #478

merged 2 commits into from
Jun 29, 2022

Conversation

taufik-rama
Copy link
Contributor

Details

Added basic compilation support for js/wasm build target.

Currently, when we try to compile the project to be used for WASM, it will result in error

taufik-rama@dev:~/programming/go/src/github.com/newrelic/go-agent$ GOOS=js GOARCH=wasm go build
# github.com/newrelic/go-agent/internal/sysinfo
internal/sysinfo/usage_posix.go:20:17: undefined: syscall.Getrusage
internal/sysinfo/usage_posix.go:20:35: undefined: syscall.RUSAGE_SELF

This changes just add basic support so that at least sysinfo.GetUsage() & sysinfo.PhysicalMemoryBytes() can be used, even though currently it will just return "unsupported" error

@taufik-rama taufik-rama changed the title Added noop 'GetUsage' for js/wasm build target Added noop operations for js/wasm build target Apr 6, 2022
@iamemilio
Copy link
Contributor

Thanks for bringing this to our attention and submitting a pull request! We will take a look and get back to you as soon as we can.

@CLAassistant
Copy link

CLAassistant commented Apr 21, 2022

CLA assistant check
All committers have signed the CLA.

@taufik-rama
Copy link
Contributor Author

I've signed the CLA & got the confirmation page, don't really know if there's any issue since it's still on Pending status.

I did registered my Github email after the confirmation, since I just so happen to use different email on the commit, don't know if this relates to that

@iamemilio
Copy link
Contributor

iamemilio commented May 3, 2022

we just pushed a bunch of test fixes, do you mind re-running them? Also, your CLA is good to go. I switched the pull request to be against develop, which is where we stage all the code changes that are ahead of the current release, which is in master.

@iamemilio iamemilio changed the base branch from master to develop May 3, 2022 20:10
@iamemilio
Copy link
Contributor

iamemilio commented May 25, 2022

Hi @taufik-rama we are going to close and then re-open this PR in order to allow us to re-run the tests :)

@iamemilio iamemilio closed this May 25, 2022
@iamemilio iamemilio reopened this May 25, 2022
@taufik-rama
Copy link
Contributor Author

Sure!

CMIIW it seems that some tests were not passing for previous versions. Unfortunately I'm currently in the middle of some work and doesn't have some free time to take a look into those, though I'll be sure to come back to this

Copy link
Contributor

@nr-swilloughby nr-swilloughby left a comment

Choose a reason for hiding this comment

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

The PR is approved; in addition it looks like some work is needed to clue in the automated checks that not all modules are compiled on every platform, hence the false positives about redefined functions.

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.

4 participants