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

Filter target build-tags if user specified an overriding option #3357

Merged
merged 3 commits into from
Feb 14, 2023

Conversation

anuraaga
Copy link
Contributor

I noticed that when building for wasi, the runtime_memhash_ option cannot be overridden because the leveldb option always gets added, but only one file is allowed to be compiled

❯ ~/go/bin/tinygo build -scheduler=none -target=wasi -opt=2 -o mapbench.wasm -tags="runtime_memhash_tsip" .
# runtime
/Users/anuraag/git/tinygo/src/runtime/memhash_tsip.go:18:6: ptrToSlice redeclared in this block
/Users/anuraag/git/tinygo/src/runtime/memhash_leveldb.go:16:6: 	other declaration of ptrToSlice
/Users/anuraag/git/tinygo/src/runtime/memhash_tsip.go:47:6: hash64 redeclared in this block
/Users/anuraag/git/tinygo/src/runtime/memhash_leveldb.go:82:6: 	other declaration of hash64
/Users/anuraag/git/tinygo/src/runtime/memhash_tsip.go:119:6: hash32 redeclared in this block
/Users/anuraag/git/tinygo/src/runtime/memhash_leveldb.go:34:6: 	other declaration of hash32

@deadprogram
Copy link
Member

Looks good to me @anuraaga just wondering if you could perhaps write a test for the new function? I realize that the compileopts subpackage is a bit light on tests now... 👼

@anuraaga
Copy link
Contributor Author

anuraaga commented Feb 2, 2023

Sorry for the huge delay, finally added unit tests

@deadprogram
Copy link
Member

Now going to squash/merge. Thanks @anuraaga

@deadprogram deadprogram merged commit e0a5fc2 into tinygo-org:dev Feb 14, 2023
waj334 pushed a commit to waj334/tinygo that referenced this pull request Feb 24, 2023
…go-org#3357)

compileopts: Filter target build-tags if user specified an overriding option
conejoninja pushed a commit to conejoninja/tinygo that referenced this pull request Mar 22, 2023
…go-org#3357)

compileopts: Filter target build-tags if user specified an overriding option
deadprogram pushed a commit that referenced this pull request Mar 28, 2023
compileopts: Filter target build-tags if user specified an overriding option
aykevl added a commit that referenced this pull request May 13, 2023
This basically reverts #3357
and replaces it with a different mechanism to get to the same goal.

I do not think filtering tags like this is a good idea: it's the wrong
part of the compiler to be concerned with such tags (that part sets
tags, but doesn't modify existing tags). Instead, I've written the
//go:build lines in such a way that it has the same effect: WASI
defaults to leveldb, everything else defaults to fnv, and it's possible
to override the default using build tags.
aykevl added a commit that referenced this pull request May 13, 2023
This basically reverts #3357
and replaces it with a different mechanism to get to the same goal.

I do not think filtering tags like this is a good idea: it's the wrong
part of the compiler to be concerned with such tags (that part sets
tags, but doesn't modify existing tags). Instead, I've written the
//go:build lines in such a way that it has the same effect: WASI
defaults to leveldb, everything else defaults to fnv, and it's possible
to override the default using build tags.
deadprogram pushed a commit that referenced this pull request May 17, 2023
This basically reverts #3357
and replaces it with a different mechanism to get to the same goal.

I do not think filtering tags like this is a good idea: it's the wrong
part of the compiler to be concerned with such tags (that part sets
tags, but doesn't modify existing tags). Instead, I've written the
//go:build lines in such a way that it has the same effect: WASI
defaults to leveldb, everything else defaults to fnv, and it's possible
to override the default using build tags.
LiuJiazheng pushed a commit to LiuJiazheng/tinygo that referenced this pull request Aug 20, 2023
…go-org#3357)

compileopts: Filter target build-tags if user specified an overriding option
LiuJiazheng pushed a commit to LiuJiazheng/tinygo that referenced this pull request Aug 20, 2023
This basically reverts tinygo-org#3357
and replaces it with a different mechanism to get to the same goal.

I do not think filtering tags like this is a good idea: it's the wrong
part of the compiler to be concerned with such tags (that part sets
tags, but doesn't modify existing tags). Instead, I've written the
//go:build lines in such a way that it has the same effect: WASI
defaults to leveldb, everything else defaults to fnv, and it's possible
to override the default using build tags.
deadprogram pushed a commit that referenced this pull request Sep 17, 2023
This basically reverts #3357
and replaces it with a different mechanism to get to the same goal.

I do not think filtering tags like this is a good idea: it's the wrong
part of the compiler to be concerned with such tags (that part sets
tags, but doesn't modify existing tags). Instead, I've written the
//go:build lines in such a way that it has the same effect: WASI
defaults to leveldb, everything else defaults to fnv, and it's possible
to override the default using build tags.
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

2 participants