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

Configuration load doesn't work when used as a dynamic library due to commandLineParams() #31

Open
oskarth opened this issue Jun 14, 2021 · 4 comments

Comments

@oskarth
Copy link

oskarth commented Jun 14, 2021

commandLineParams doesn't work when used in a dynamic library:

/home/oskarth/git/status-im/nim-waku/vendor/nim-confutils/confutils.nim(784, 16) Error: commandLineParams() unsupported by dynamic libraries; usage of 'commandLineParams' is an {.error.} defined at /home/oskarth/git/status-im/nim-waku/vendor/nimbus-build-system/vendor/Nim/lib/pure/os.nim(2775, 3)

One way to get around this would be something like

+var getCLIParams: proc(): seq[TaintedString]
+
+# On Posix there is no portable way to get the command
+# line from a DLL and thus the proc isn't defined in this environment.
+# See https://nim-lang.org/docs/os.html#commandLineParams
+when declared(paramCount):
+  getCLIParams = commandLineParams
+else:
+  getCLIParams = proc(): seq[TaintedString] =
+    var emptyTaintedSeq: seq[TaintedString]
+    return emptyTaintedSeq
+
 proc load*(Configuration: type,
-           cmdLine = commandLineParams(),
+           cmdLine = getCLIParams(),
            version = "",

When paramCount is not declared, this is equivalent to cmdLine being empty.

However, this seems like a bit of a hack and there are probably better solutions here?

@oskarth
Copy link
Author

oskarth commented Jun 14, 2021

cc @zah for ideas on simple proper solution, or if something like this seems workable

@oskarth
Copy link
Author

oskarth commented Jun 14, 2021

To give some context, the goal is to load default config options in a shared library https://github.com/status-im/nim-waku/pull/614/files#r651031823

@oskarth
Copy link
Author

oskarth commented Jun 14, 2021

I also notice that the above hack, while it makes libwaku compile, it causes our chat2 app error with:

/home/oskarth/git/status-im/nim-waku/vendor/nim-chronos/chronos/asyncmacro2.nim(67, 17) Error: ':anonymous' is not GC-safe as it accesses 'nameIterVar`gensym198915109' which is a global using GC'ed memory
stack trace: (most recent call last)
/home/oskarth/git/status-im/nim-waku/vendor/nimbus-build-system/vendor/Nim/lib/system/nimscript.nim(416, 18)
/home/oskarth/git/status-im/nim-waku/waku.nimble(86, 15) chat2Task
/home/oskarth/git/status-im/nim-waku/waku.nimble(35, 8) buildBinary
/home/oskarth/git/status-im/nim-waku/vendor/nimbus-build-system/vendor/Nim/lib/system/nimscript.nim(260, 7) exec
/home/oskarth/git/status-im/nim-waku/vendor/nimbus-build-system/vendor/Nim/lib/system/nimscript.nim(260, 7) Error: unhandled exception: FAILED: nim c --out:build/chat2 -d:chronicles_log_level=DEBUG -d:chronicles_sinks=textlines[file] -d:ssl --verbosity:0 --hints:off -d:usePcreHeader --passL:-lpcre -d:release examples/v2/chat2.nim [OSError]

vitvly pushed a commit to vitvly/nim-confutils that referenced this issue Oct 29, 2022
vitvly pushed a commit to vitvly/nim-confutils that referenced this issue Mar 10, 2023
@zah
Copy link
Contributor

zah commented Mar 16, 2023

I'm sorry for not noticing this issue earlier. It's actually quite easy to fix. We need to create a low-level overload of load that doesn't have a default cmdLine parameter and then we'll include the high level convenient interface that wraps the lower-level API with the additional defaults only when compiling regular apps.

vitvly pushed a commit to vitvly/nim-confutils that referenced this issue May 15, 2023
vitvly added a commit to vitvly/nim-confutils that referenced this issue Jan 25, 2024
arnetheduck added a commit that referenced this issue Jan 25, 2024
* Dynlib fix as suggested in #31

* Update confutils.nim

* Declare empty commandLineParams if the standard one is not declared.

---------

Co-authored-by: Jacek Sieka <[email protected]>
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

No branches or pull requests

2 participants