Nim2 compatibility #186
Merged
Nim2 compatibility #186
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
These are (currently) no longer supported by nim devel. For reference, see: - nim-lang/Nim#21435 - nim-lang/Nim#21192 It is likely that this behaviour will stay. Either way, there is no particular need to stick with [_], thus we shall proactively remove it, just to be safe.
Ndb was originally used for interacting with sqlite/postgres, as it properly handled NULL as opposed to std/db_* However, the package appears to be dead/less maintained. Therefore we shall move on to a maintained fork of it called lowdb.
I don't want to deal with docker-compose commands I won't remember anyway. Just give me nimble tasks.
Lowdb 0.1.0 had some structural issues. Thus 0.1.1 was brought out to fix those.
moigagoo
reviewed
Feb 25, 2023
1) Nim 2.0 does not like bare catches 2) Made it so connections are grabbed from environmental variables in postgres 3) Nim does not let you get away with forgetting that other parts of your code raise "Exception" if you use the raises pragma.
In accordance with nim 2.0, logs can raise "Exception" That has cascading effects on procs using "log", such as sqlite. Postgres has already been adjusted, the raises pragmas for sqlite need to be adjusted as well. They need to tell that their procs can also raise "Exception".
Nim 1.6.10 used to allow dispatching to a proc dbType(T: typedesc[int64]) when you passed in a model with field type "Positive". This changed in nim 2. Positive no longer matches int64 this way. HOWEVER, you also can not define a separate dbType proc for the "Positive" type. Because int64 still matches to the Positive type. So by doing so you would suddenly cause ambiguous situations where nim can't decide whether to call dbType for int64 or Positive when encountering a field of type int64. Making it a generic this way circumvents that problem.
By making dbType for int64 a generic it no longer matches int Therefore it needs to be added to the generic, so that a dbType for int is still defined.
Nim 2.0 has deprecated bare excepts. You are supposed to catch CatchableErorrs instead.
|
Okay, here we go. @moigagoo I had to significantly revamp the test-suite, solely so we can start testing multiple nim-versions there instead of just one. Couldn't figure out a way that left the calling code within a container itself. As was visible with the dbTypes issue, it was quite necessary to start testing for multiple versions. That did sacrifice the docker-compose commands, but I replaced them with (hopefully) as convenient nimble tasks. Anyway:
|
Nimibook can not be compiled with orc on 1.6.10 Only on devel as of 25.02.2023.
moigagoo
reviewed
Feb 26, 2023
This is not strictly speaking necessary, as any proc call looking to match `Natural` will match for `int`, but this way is more explicit that `Natural` is supported.
This is to make it clear that there's no real difference between them Should lead to less confusion and more explicitness.
Log should only throw LoggingError. However, the underlying procs in std/logging that log calls can throw `Exception` in general. That is undesired behaviour and thus to avoid having to track that in the raises system, we catch the exception and convert it into a LoggingError.
Sqlite procs like these can no longer raise Exception. They originally could because log could, but that has been changed. So they no longer can.
Postgres.nim contained procs previously able to throw raw exceptions. That was due to: - the log proc being able to do so - various DB-query procs being able to throw raw exceptions The log proc has previously been refactored to no longer do so. The DB-procs that can throw exceptions are now in try-catch blocks that convert the raw exception into a DBError
moigagoo
approved these changes
Feb 27, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
closes #182
Makes norm viable with nim devel:
[_]to[T]etc. due to breaking change introduced by underscores for routine parameters nim-lang/Nim#21192See: Improve error message of generic declarations using [_] as type, as
undeclared identifier _compiler error is hard to understand nim-lang/Nim#21435Unrelated to that:
I introduced nimble tasks to run the tests because I got tired of looking up the exact command after the third time and I'd prefer just having a nimble task for everything related to a package 😅