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

Update to Chronos V4 #673

Closed
wants to merge 47 commits into from
Closed

Update to Chronos V4 #673

wants to merge 47 commits into from

Conversation

gmega
Copy link
Member

@gmega gmega commented Jan 12, 2024

This PR contains the changes required to have Codex running on Chronos V4.

@gmega gmega force-pushed the feat/chronos-v4 branch 6 times, most recently from d491a82 to d8ee340 Compare January 22, 2024 13:58
@gmega
Copy link
Member Author

gmega commented Jan 22, 2024

OK, to be able to wrap this up, we need the following to happen:

Of course

  • having an actual V4 tag to pin to

wouldn't hurt either, but that's not a blocker. 🙂

@gmega gmega mentioned this pull request Jan 22, 2024
@gmega gmega self-assigned this Feb 29, 2024
@emizzle
Copy link
Contributor

emizzle commented Mar 14, 2024

nim-lang/Nim#23215 backported and released in a 1.16.x version as this branch is currently building Nim from a "private" fork (public repo on my account). The alternative being extirpating all use of range types from the codebase;
nim-lang/Nim#23354 backported and released in an 1.16.x version,

Where are we using these two in codex that is preventing us from getting a working solution for chronos v4?

@gmega
Copy link
Member Author

gmega commented Mar 14, 2024

nim-lang/Nim#23215 backported and released in a 1.16.x version as this branch is currently building Nim from a "private" fork (public repo on my account). The alternative being extirpating all use of range types from the codebase;
nim-lang/Nim#23354 backported and released in an 1.16.x version,

Where are we using these two in codex that is preventing us from getting a working solution for chronos v4?

These are compiler bugs which affect move and sink, respectively, both of which are key to Chronos v4; i.e., Chronos v4 uses them, not us. Both of them cause crashes in Codex due to memory corruption.

@elcritch
Copy link
Contributor

Heads up Chronos v4.0.1 was released that removes all sink params https://forum.nim-lang.org/t/10914#73721

v4.0.1 tagged:
removes usage of sink (currently broken in all Nim versions, both refc/arc)
fixes some circular refs in timers for better ARC support
fixes a bunch of random bugs

@gmega
Copy link
Member Author

gmega commented Mar 27, 2024

Awesome, this unblocks our migration path. The unfortunate thing is that this has been bumped to low prio as it's too late for it to make it into the testnet.

@jessiebroke jessiebroke added the Client See https://miro.com/app/board/uXjVNZ03E-c=/ for details label Apr 1, 2024
@dryajov
Copy link
Contributor

dryajov commented Apr 29, 2024

We should update this and plan on integrating it ASAP.

@gmega
Copy link
Member Author

gmega commented Jun 10, 2024

I'm closing this PR as we're going the v3 compat route (#814), which buys us some time to kick the can on debugging buggy deps down the road while still making progress on the v4 migration.

@gmega gmega closed this Jun 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Client See https://miro.com/app/board/uXjVNZ03E-c=/ for details
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants