emscripten-core / emscripten-core/emscripten

Warn users that a flag is set multiple times with different values and being overwritten

Open
#21,294 0 comments 0 reactions 1 assignee Claimed by @sbc100 View on GitHub
Dominant language
C++
Stars
27.6k
Forks
3.6k
Avg merge
1d 1h
Merged PRs (30d)
105

Description

In some build systems (e.g. bazel) flags can be set on dependencies that will take precedence over final binary flags.

```python
cc_library(
name = "test_lib",
srcs = ["test.cc"],
linkopts = [
"-s EXPORTED_RUNTIME_METHODS='[\"FS\"]'",
"-s MODULARIZE=0",
],
)

cc_library(
name = "test2_lib",
srcs = ["test2.cc"],
linkopts = [
"-s EXPORTED_RUNTIME_METHODS='[\"MEMFS\"]'",
],
)

cc_binary(
name = "test_bin",
linkopts = [
"-s EXPORTED_RUNTIME_METHODS='[\"ccall\"]'",
"-s MODULARIZE=1",
"-s EXPORT_NAME=createWasmModule",
],
deps = [
":test2_lib",
":test_lib",
],
)

wasm_cc_binary(
name = "test_wasm",
cc_target = ":test_bin",
)
```
In this case, the order of flags will be:

test_bin
test2_lib
test_lib

So the end is

```
-s EXPORTED_RUNTIME_METHODS='[\"ccall\"]'
-s MODULARIZE=1
-s EXPORT_NAME=createWasmModule
-s EXPORTED_RUNTIME_METHODS='[\"MEMFS\"]'
-s EXPORTED_RUNTIME_METHODS='[\"FS\"]'
-s MODULARIZE=0
```

which currently equals to

```
-s EXPORT_NAME=createWasmModule
-s EXPORTED_RUNTIME_METHODS='[\"FS\"]'
-s MODULARIZE=0
```

As a result, test_bin lost access to ccall, test2_lib won't get MEMFS and test_lib will prevent modularization. If test_lib is deep enough in the dependency tree, it becomes insanely hard to debug as there is literary no warning that it's happening.

Given that changing the behavior might be too invasive for now, I suggest we warn users that it's happening so they can rearrange the code accordingly. So something like

```python
for s in settings_changes:
key, value = s.split('=', 1)
key, value = normalize_boolean_setting(key, value)
if key in user_settings and user_settings[key] != value:
diagnostics.warning('unused-command-line-argument', f'{key} was provided multiple times with different values. Previous value: {user_settings[key]}. New value: {value}.')
user_settings[key] = value
```

might do the trick.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.