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

Broken release #231

Open
AuHau opened this issue Oct 29, 2024 · 10 comments · May be fixed by #232
Open

Broken release #231

AuHau opened this issue Oct 29, 2024 · 10 comments · May be fixed by #232

Comments

@AuHau
Copy link
Member

AuHau commented Oct 29, 2024

With the 0.5.0 I can not install nim-json-rpc with Nimble getting:

Downloading https://github.com/status-im/nim-json-rpc using git
       Tip: 904 messages have been suppressed, use --verbose to show them.
     Error: Could not read package info file in /tmp/nimble_1874/githubcom_statusimnimjsonrpc_0.4.0_0.5.0/json_rpc.nimble;
        ...   Reading as ini file failed with: 
        ...     Invalid section: .
        ...   Evaluating as NimScript file failed with: 
        ...     Unable to parse dependency version range: Unexpected char in version range ' ^= 4.0.3': ^.

https://github.com/codex-storage/nim-ethers/actions/runs/11573879127/job/32216914275?pr=85

Nim 1.6.20

@arnetheduck
Copy link
Member

probably you're using an old nimble - you need 0.16.2

@arnetheduck
Copy link
Member

actually this is caused by nim 1.6

@AuHau
Copy link
Member Author

AuHau commented Oct 29, 2024

Btw. even if you decide to drop Nim 1.6 for this package it will still stay broken for us. Even if we try to use the earlier version of this package by specifically pinning those commits, thanks to the fact that there are non-pinned dependencies that has the same changes you introduced here (eq. pinning Chronos dependency) we won't be able to install it.

So I guess the only solution is to fork this repo in order to pin the deps into version before your patches.

@arnetheduck
Copy link
Member

Actually, it's not a nim issue - it's indeed a nimble version issue.

@arnetheduck
Copy link
Member

arnetheduck@praeceps:~/tmp/xxx/nim-json-rpc$ PATH=/home/arnetheduck/src/Nim-v1.6/bin:$PATH ~/bin/nimble --use-system-nim test
     Info:  Dependency on unittest2@any version already satisfied
     Info:  Dependency on unittest2@any version already satisfied
     Info:  Dependency on results@any version already satisfied
     Info:  Dependency on unittest2@any version already satisfied
     Info:  Dependency on stew@>= 0.1.0 already satisfied
     Info:  Dependency on stew@any version already satisfied
     Info:  Dependency on results@any version already satisfied
     Info:  Dependency on unittest2@any version already satisfied
     Info:  Dependency on results@any version already satisfied
     Info:  Dependency on stew@any version already satisfied
     Info:  Dependency on bearssl@>= 0.2.5 already satisfied
     Info:  Dependency on httputils@any version already satisfied
     Info:  Dependency on unittest2@any version already satisfied
     Info:  Dependency on stew@any version already satisfied
     Info:  Dependency on unittest2@any version already satisfied
     Info:  Dependency on faststreams@any version already satisfied
     Info:  Dependency on unittest2@any version already satisfied
     Info:  Dependency on stew@any version already satisfied
     Info:  Dependency on serialization@any version already satisfied
     Info:  Dependency on stew@any version already satisfied
     Info:  Dependency on testutils@any version already satisfied
     Info:  Dependency on json_serialization@any version already satisfied
     Info:  Dependency on stew@any version already satisfied
     Info:  Dependency on chronos@ ^= >= 4.0.3 already satisfied
     Info:  Dependency on httputils@>= 0.2.0 already satisfied
     Info:  Dependency on chronicles@>= 0.10.2 already satisfied
     Info:  Dependency on stew@>= 0.1.0 already satisfied
     Info:  Dependency on nimcrypto@any version already satisfied
     Info:  Dependency on bearssl@any version already satisfied
     Info:  Dependency on zlib@any version already satisfied
     Info:  Dependency on stew@any version already satisfied
     Info:  Dependency on nimcrypto@any version already satisfied
     Info:  Dependency on stint@any version already satisfied
     Info:  Dependency on chronos@4.0.3 already satisfied
     Info:  Dependency on httputils@0.3.0 already satisfied
     Info:  Dependency on chronicles@any version already satisfied
     Info:  Dependency on websock@any version already satisfied
     Info:  Dependency on json_serialization@any version already satisfied
     Info:  Dependency on unittest2@any version already satisfied
  Executing task test in /home/arnetheduck/tmp/xxx/nim-json-rpc/json_rpc.nimble
[NimScript] exec: nim c  --styleCheck:usages --styleCheck:error --verbosity:0 --hints:off --skipParentCfg --skipUserCfg --outdir:build --nimcache:build/nimcache -f --threads:on -d:chronicles_log_level=ERROR   --mm:refc -r tests/all

works with nim 1.6 and a recent enough nimble

@emizzle
Copy link

emizzle commented Oct 30, 2024

@arnetheduck, could we please re-open this issue? For those using the packaged nimble with 1.6.x, the latest patch means that json_rpc cannot be installed.

^= Install the latest compatible version according to semver.

Instead, a simple fix to this is to use a more explicit versioning range, eg chronos >= 0.4.0 & < 0.5.0. I'm more than happy to open a PR for this.

Without this fix in, we are basically screwed and will be forced to fork quite a few libraries just to update their nimble files, and then need to maintain those forks. How is this acceptable?

@arnetheduck
Copy link
Member

For those using the packaged nimble with 1.6.x, the latest patch means that json_rpc cannot be installed.

This will become an issue with most, if not all packages quite soon - supporting nimble <0.16.2 and using versioning is simply not possible at scale because that nimble uses a broken (greedy) algorithm for selecting versions - it's not just about parsing the versions but also what happens after - the reason it has worked so far is because we haven't really been using versioning - neither json-rpc nor any packages it depended on had any version constraints to speak of and indeed no releases either - incidentally, this is what enabled codex to do its own thing and manage packages in a different way than the others, because it was at the tail end of the process.

As the number of actually versioned packages grow, there will also be a growing number of packages that fail in a complex way, due to ordering of requires and similar issues that you won't be able to solve with a patch.

nimbus-build-system already has the features to build a different nimble, and nimble can now also build nim (we've made sure it works with nim 1.6 so that codex can migrate to the new nimble while staying on nim 1.6) - the path forward lies here.

If you supply and test a PR, it will be accepted of course as a bridge solution, but this is not sustainable over time and the process will repeat for the whole graph of packages pretty soon.

@arnetheduck arnetheduck reopened this Oct 30, 2024
@arnetheduck
Copy link
Member

an option here is that we tag some random commit earlier in the release history as "0.4.x" - this might allow you to use a constraint on the versions to use whatever it was that got downloaded by nimble before.

@emizzle emizzle linked a pull request Oct 31, 2024 that will close this issue
@emizzle
Copy link

emizzle commented Oct 31, 2024

Yes, agreed the lack of versioning has caused a lot of headaches, and it will continue to as versions are added. However, this particular issue doesn't seem to be an issue with adding versioning, it seems to be with using version selector operators that nimble ≤0.13.1 doesn't support. Specifically, it seems that nimble ≤0.13.1 does not support the ^= selector operator when specifying version ranges. While I understand this is an annoying limitation for nim libs to not be able to use certain selector operators, I actually think it's more explicit to avoid using ^= because it is unclear which version will be installed when using this operator, especially when libs may or may not be following semver.

the path forward lies here

Codex is not necessarily the issue for us, as it uses NBS. The issue is with nim-ethers, which relies on nimble and supports 1.6.x. nim-ethers cannot be the only library supporting 1.6.x in the nim ecosystem, so my assumption is that this would affect plenty of other libs out there. Hence the (unhelpful) frustration (my apologies 🙏) that this issue was closed with the reasoning that this is a nimble issue and it's not going to be fixed.

If you supply and test a PR, it will be accepted of course as a bridge solution, but this is not sustainable over time and the process will repeat for the whole graph of packages pretty soon.

PR here: #232. Eventually nimble ≤0.13.1 support will be dropped, but in the meantime, using supported selector operators is an easy win imo.

an option here is that we tag some random commit earlier in the release history as "0.4.x" - this might allow you to use a constraint on the versions to use whatever it was that got downloaded by nimble before.

We tried using version 0.4.4 but this also did not work for us, failing with the same error. The assumption is that the websock dependency is using #head in 0.4.4, and websock#head was also recently updated to use unsupported selector operators (PR to fix this).

@arnetheduck
Copy link
Member

cannot be the only library supporting 1.6.x

it's a dying breed - 2 major releases since and no more security updates. but - with this, we're not dropping nim 1.6 support: only old nimble support - upgrading nimble can be done independently of nim (just like in python, people upgrade pip independently of python even if by default, a particular version is bundled for bootstrap).

websock#head

incidentally, the next nimble contains fixes for combining packages that use #head versions too - these are overly strict in all versions of nimble, including those with broken pm:s.

especially when libs may or may not be following semver.

yeah, this is a good/important point - one has to be careful to apply it only for such packages. I think however that by and large we want to follow semver for our packages given the status quo of there not being a better idea. it's also a tricky balance because "most of the time" even a major version upgrade doesn't really break things more than a "feature addition" might, in nim (due to its module system) - adding an "upper version" constraint can do some harm as well.

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 a pull request may close this issue.

3 participants