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

nim: update to 1.6.0, fix non-x86_64/i686 packages, improve template #34106

Merged
merged 1 commit into from
Dec 4, 2021

Conversation

EliteTK
Copy link
Contributor

@EliteTK EliteTK commented Nov 16, 2021

Testing the changes

  • I tested the changes in this PR: YES

Local build testing

  • I built this PR locally for my native architecture, (glibc)
  • I cross-built it for a few things (aarch64 and arm all glibc)

I think it is about ready.

Old comments below this line.


I was mainly putting this up here to gather comments. For background, things which have changed between what is available in the repositories and 1.6.0:

  • Nim no longer bundles fusion. It provides a koch command to nimble install it at a specific commit. It might be worth providing it as a separate package but maybe not worth it.
  • Nim now uses csources_v1 instead of csources. This is now built with make. We used to pass --cpu to build.sh based on XBPS_TARGET_MACHINE. I removed this temporarily, I think it would have to be re-added setting the ucpu variable instead but I would like to see what CI says first.
  • Nim now comes with testament which I am installing like the rest of things
  • Nim no longer comes with examples

Other notes:

  • I bumped the nimble version but actually I think it would be more correct to figure out which exact commit nim 1.6.0 uses in build_all.sh and mirror that. I simply haven't gotten around to this yet. (update: Looks like it uses 0.13.1 so the template is correct now)
  • I noticed build_all.sh passes --skipUserCfg --skipParentCfg --hints:off everywhere. I'm not sure if we should mirror that. I think the first two options would be irrelevant inside the build environment and the last option is just removing diagnostics. They're not essential options but I also don't think they hurt.
  • It would be nice if someone else had a glance at this. I don't really do void packaging but I think I did almost everything correctly.

@EliteTK
Copy link
Contributor Author

EliteTK commented Nov 16, 2021

Well, the CI didn't complain about the lack of passing ucpu=i686 to the bootstrap compiler build when compiling for i686. Would anyone know why that would ever be necessary?

@EliteTK EliteTK marked this pull request as draft November 17, 2021 23:40
@EliteTK EliteTK changed the title [RFC] nim: update to 1.6.0 nim: update to 1.6.0 Nov 17, 2021
@EliteTK
Copy link
Contributor Author

EliteTK commented Nov 17, 2021

Since github has drafts now, I've made this one.

I got to thinking about the i686 build and I think I understand how it works.

The cpu is set to i686 because nim doesn't really support cross-building the compiler. The problem is this doesn't actually work when you try to run the compile locally (and it didn't work for the previous csources) so I'm confused there. I pushed to see what CI says about it.

I think actually the correct solution would be to do for i686 what we're currently doing for the other non x86_64 arches an I'm not sure why that isn't the case already.

But in the meanwhile, if this does end up working in CI, I would like to keep it that way and get this version bump merged. Then doing i686 builds properly can be a task for later.

That being said I think ALL builds should be done the way that they are for non-x86_64/i686 and that's so you can cross-compile a nim void package from a non-x86_64 machine. But I don't know what the void policy is there.

@EliteTK EliteTK force-pushed the nim-1.6.0 branch 2 times, most recently from b4c4955 to 7a320bd Compare November 18, 2021 09:24
@EliteTK
Copy link
Contributor Author

EliteTK commented Nov 18, 2021

Okay, that worked, I checked the existing packages for nim in the repositories for other arches and they are compiled correctly. The void cross-building system confuses me but I've squashed the revert and I'm going to mark this as ready for review.

@EliteTK EliteTK marked this pull request as ready for review November 18, 2021 09:26
@EliteTK
Copy link
Contributor Author

EliteTK commented Nov 18, 2021

Okay, so I figured out that the current (and this bump doesn't change this) packages which ship for voidlinux come with a nim.cfg which sets the compiler for multiple architectures to be the cross compiler for the package architecture on non-x86_64 and non-i686 packages.

I verified this by just downloading an aarch64 package and checking the config.

There's an easy workaround for this I can put into a separate commit.

@EliteTK EliteTK changed the title nim: update to 1.6.0 nim: update to 1.6.0, fix non-x86_64/i686 packages, improve template Nov 18, 2021
@EliteTK
Copy link
Contributor Author

EliteTK commented Nov 18, 2021

I've now pushed the cross-platform build fixes as well as a few template improvements.

@EliteTK EliteTK force-pushed the nim-1.6.0 branch 4 times, most recently from 452efcd to 0b43bbd Compare November 19, 2021 23:13
Version bump to 1.6.0 including the following changes:

- Nim now comes with testament
- Fusion library is no longer bundled with nim
- Examples no longer come with nim
- FIX - non-x86_64/i686 nim.cfg compiler configuration
  Non-x86_64/i686 builds no longer end up with a broken nim.cfg which
  will try to use the package build time $CC to build packages for the
  package architecture as well as a bunch of others (which will simply
  not work).
- FIX - arm nim.cfg compiler configuration
  The default arm.linux.gcc.(linker)?exe variables are now commented on
  non-x86_64/i686 builds to ensure that arm packages don't end up with
  the same issue as above.
- CLEANUP - use koch to build non-x86_64/i686 tools
  The loop was unnecessary.
  - note: This means nimsuggest is now built without -d:release on all
    builds (not just x86_64/i686 builds). I am not sure if this is
    intentional or an upstream bug, should be investigated at some
    point.
- CLEANUP - use make to build the bootstrap compiler
@EliteTK
Copy link
Contributor Author

EliteTK commented Nov 19, 2021

Squashed per instruction on irc.

@ericonr
Copy link
Member

ericonr commented Dec 4, 2021

But in the meanwhile, if this does end up working in CI, I would like to keep it that way and get this version bump merged. Then doing i686 builds properly can be a task for later.

FWIW i686 is built natively.

@ericonr ericonr merged commit d6763d1 into void-linux:master Dec 4, 2021
@ericonr
Copy link
Member

ericonr commented Dec 4, 2021

Sorry for the delay, great PR!

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 4, 2022
@EliteTK EliteTK deleted the nim-1.6.0 branch June 19, 2022 23:35
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants