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.

Sign up for GitHub

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

Jump to bottom

2.0.4/6 backport candidates #3

Closed
arnetheduck opened this issue Mar 25, 2024 · 14 comments
Closed

2.0.4/6 backport candidates #3

arnetheduck opened this issue Mar 25, 2024 · 14 comments

Comments

@arnetheduck
Copy link

arnetheduck commented Mar 25, 2024 •
edited by narimiran
Loading 

git log --oneline --pretty="format:%m https://github.com/nim-lang/Nim/commit/%H %s" --left-right  --cherry-pick devel...origin/version-2-0
@narimiran narimiran mentioned this issue Apr 19, 2024
Closed
@narimiran
Copy link
Member

narimiran commented Apr 23, 2024 •
edited
Loading

@ringabout Even with new backports, the nightlies are still failing with:
os.nim(678, 30) Error: getApplOpenBsd() can raise an unlisted exception: ref OSError

Since this happens only on the nightlies, and not with the regular Nim CIs or to me locally, it is hard for me to reproduce.

Last successful nightlies build was before I started doing backports from this list, so I'm guessing the problematic commit is somewhere in the bottom 1/3 of this list (the oldest commit is at the bottom). Based on the commit messages in the list, can you maybe tell what might be causing it or, alternatively, how to fix the error?

@narimiran
Copy link
Member

Found it!!
I need to backport nim-lang/Nim#22585, which deals with exactly this error.

@narimiran
Copy link
Member

@arnetheduck The work on this list of commits has been completed. The "missing backports" (onec without a checkmark on the list) are commits which cause problems on the version-2-0 branch (each has been backported and then tested).

As it is more than a month since you made this list, if you want — make another list with recent devel commits which you would like to see in Nim 2.0.x, and I can work on backporting them.

@arnetheduck
Copy link
Author

@narimiran great, thanks! I'll take a look in a bit

@metagn nim-lang/Nim@9416595 - what would it take to backport this one?

@metagn
Copy link

metagn commented May 2, 2024

@metagn nim-lang/Nim@9416595 - what would it take to backport this one?

Should be no trouble afaik, a direct cherrypick could work

@narimiran
Copy link
Member

Should be no trouble afaik, a direct cherrypick could work

Ok, let me try to backport it again. Maybe this time (with more commits backported) it'll work.

@narimiran
Copy link
Member

Should be no trouble afaik, a direct cherrypick could work

When I cherry pick your commit, tests/concepts/tmatrixconcept.nim fails with Error: unhandled exception: index 1 not in 0 .. 0 [IndexDefect].
That test file is the same in devel, so it's probably some other commit(s) that need to be backported for this to work.

@metagn
Copy link

metagn commented May 2, 2024 •
edited
Loading

Then I would say nim-lang/Nim@e8092a5 might fix it, but there may have been a backport attempt on this already?

@narimiran
Copy link
Member

Then I would say nim-lang/Nim@e8092a5 might fix it, but there may have been a backport attempt on this already?

I've tried it again now, on top of the previously mentioned commit, and it still doesn't fix the error in tmatrixconcept.nim.

Next idea? :)

@metagn
Copy link

metagn commented May 2, 2024

None off the top of my head, I'll try to figure out what's failing from the stacktrace when I get the chance

@metagn
Copy link

metagn commented May 2, 2024 •
edited
Loading

The issue is with sempass2 not being able to handle the weirdness of the type of the type keyword, changing:

https://github.com/nim-lang/Nim/blob/fcb8461efab2ef7bdd976f82af8c7d1390f502ac/compiler/sempass2.nim#L891

to:

    if a.kind != nkSym or a.sym.magic notin {mNBindSym, mFinished, mExpandToAst, mQuoteAst, mType}:

(i.e. adding mType to the excluded sym magics) fixes it. My guess is the "type refactor" PRs dealt with this some other way but they are pretty big. We could push this change to just 2.0 or we can add it to devel then backport it for symmetry as it should be harmless.

I forgot to mention nim-lang/Nim@9416595 also has a followup nim-lang/Nim@4b1a841 and probably needs another one down the line to address nim-lang/Nim#23386.

@narimiran
Copy link
Member

I forgot to mention nim-lang/Nim@9416595 also has a followup nim-lang/Nim@4b1a841

Now I backported that followup + added mType to the line you mention.

tests/concepts/tmatrixconcept.nim now passes on my machine, but let's see if CIs find any other problem.

@narimiran
Copy link
Member

let's see if CIs find any other problem.

No problem found. All mentioned commits are now available in version-2-0 branch.

@narimiran
Copy link
Member

With @metagn's fix (nim-lang/Nim#23571) backported, I consider this list of commits done.

@arnetheduck, please open a new issue with the recent commits you would like to see backported.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@arnetheduck @metagn @narimiran