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

Fixes compilation with nim 2 and the test-script #58

Merged
merged 2 commits into from
Sep 18, 2023

Conversation

hoijui
Copy link
Contributor

@hoijui hoijui commented Sep 9, 2023

No description provided.

Using simply nim,
we may end up using different dependency versions,
which could have multiple unwanted effects.
@hoijui
Copy link
Contributor Author

hoijui commented Sep 9, 2023

This fixes issue #57.

@oprypin
Copy link
Member

oprypin commented Sep 9, 2023

Thanks. Just one issue:
You are supposedly fixing 2 things, but the continuous test has 0 errors - how can that be explained?
https://github.com/docopt/docopt.nim/actions/runs/6129309785/job/16637279989

@hoijui
Copy link
Contributor Author

hoijui commented Sep 9, 2023

The "testing: ..." fix was required on my machine, because without it, it would pickup regex 0.19.0, instead of regex 0.21.0. Both of these are installed on my system, but for some reason, nim picks the older, and nimble the newer version. On CI, we start with a bare system and install only what is needed, so only regex 0.21.0 is installed, and thus both nim and nimble work equally well. That is my explanation at least.

The GC-safe part however ... :/
strange. it at least happened for @RokkuCode and me, but apparently not on CI with the same nim version ...
I first saw the issue when trying to compile my software that uses docopt on CI, using nim 2.0.0.

@RokkuCode
Copy link

RokkuCode commented Sep 18, 2023

The GC-part. Because the tests are doing things differently. I made the following script with the minimal example of the Naval Fate - and it crashes. I run this script in a fresh docker container (debian bookworm). there was never a nim version installed because its fresh debian docker image.

put this test in a separate test step and look it it compiles. if it does not, there must an error. because where should the error in the code on this minimal example on a freshly installed os without any nim installed earlier.

#!/bin/bash

apt update && apt upgrade -y
apt install build-essential curl git wget
curl https://nim-lang.org/choosenim/init.sh -sSf | sh -s -- -y
export PATH=/root/.nimble/bin:$PATH
nimble install -y docopt
mkdir test && cd test
cat << EOF > ./test.nim
let doc = """
Naval Fate.

Usage:
  naval_fate ship new <name>...
  naval_fate ship <name> move <x> <y> [--speed=<kn>]
  naval_fate ship shoot <x> <y>
  naval_fate mine (set|remove) <x> <y> [--moored | --drifting]
  naval_fate (-h | --help)
  naval_fate --version

Options:
  -h --help     Show this screen.
  --version     Show version.
  --speed=<kn>  Speed in knots [default: 10].
  --moored      Moored (anchored) mine.
  --drifting    Drifting mine.
"""

import strutils
import docopt

let args = docopt(doc, version = "Naval Fate 2.0")
EOF

nim c test.nim

Result:

Hint: used config file '/root/.choosenim/toolchains/nim-2.0.0/config/nim.cfg' [Conf]
Hint: used config file '/root/.choosenim/toolchains/nim-2.0.0/config/config.nims' [Conf]
....................................................................................................................................................
/root/.nimble/pkgs2/docopt-0.7.0-21150284640b882fa91147181c52da3e5bb44df8/docopt.nim(608, 6) Error: 'docopt' is not GC-safe as it calls 'docopt_exc'

@oprypin oprypin merged commit bb4a4c7 into docopt:master Sep 18, 2023
1 check passed
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.

3 participants