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

Build error for nim-git #17907

Closed
zqqw opened this issue Apr 30, 2021 · 17 comments · Fixed by Homebrew/homebrew-core#78297
Closed

Build error for nim-git #17907

zqqw opened this issue Apr 30, 2021 · 17 comments · Fixed by Homebrew/homebrew-core#78297

Comments

@zqqw
Copy link

zqqw commented Apr 30, 2021

Building nim-git with Pakku just now resulted in this, tried with a recent nim-git also a 1.4 release.

/tmp/pakku-/nim-git/src/Nim/compiler/semtypes.nim(259, 78) Error: undeclared field: 'isNaN' for type system.BiggestFloat [declared in /tmp/pakku-/nim-git/src/Nim/lib/system.nim(1402, 3)] 
FAILURE
==> ERROR: A failure occurred in build().
    Aborting...
error: failed to build 'nim-git'

Possibly related to
3192995
#16646
(It builds OK at the previous commit)

@alaviss
Copy link
Collaborator

alaviss commented May 2, 2021

I've left a comment on the nim-git AUR package. You should be able to build the latest nim devel after the package author updates their script.

@zqqw
Copy link
Author

zqqw commented May 2, 2021

OK, So I tried:

$ diff -u origPKGBUILD.bak PKGBUILD 
--- origPKGBUILD.bak	2021-05-02 14:00:07.545279028 +0100
+++ PKGBUILD	2021-05-02 14:02:01.839771883 +0100
@@ -24,7 +24,7 @@
 makedepends=('git')
 source=(
   'git+https://github.com/nim-lang/Nim'
-  'git+https://github.com/nim-lang/csources'
+  'git+https://github.com/nim-lang/csources_v1'
   'git+https://github.com/nim-lang/nimble'
   'makepkg-conf.patch'
 )
@@ -61,12 +61,12 @@
 prepare() {
   cd Nim
 
-  [[ -d ./csources ]] && rm -rf ./csources
+  [[ -d ./csources_v1 ]] && rm -rf ./csources_v1
 
-  cp -r "${srcdir}/csources" .
+  cp -r "${srcdir}/csources_v1" .
 
   # Remove `-O3` from build.sh's COMP_FLAGS:
-  patch ./csources/build.sh \
+  patch ./csources_v1/build.sh \
   --strip=1                 \
   --fuzz 5                  \
   -N                        \
@@ -78,7 +78,7 @@
 
   # Build the pre-generated C sources of the Nim compiler which aid in
   # bootstrapping:
-  cd csources
+  cd csources_v1
     ./build.sh
   cd -

which looked OK until:
==> Starting build()...
/tmp/test2/nim-git/PKGBUILD: line 82: ./build.sh: Permission denied
==> ERROR: A failure occurred in build().
Aborting...

Now that is caused by this line:
cd csources_v1
./build.sh

and the reason for that is because build.sh in csources_v1 lacks executable permissions, so I wonder if that should be changed in csources_v1, or whether the PKGBUILD should simply chmod it first? But you would sort of expect a file called build.sh to be an executable script I suppose.

@alaviss
Copy link
Collaborator

alaviss commented May 2, 2021

You can just call the shell interpreter to run the script: sh build.sh

@zqqw
Copy link
Author

zqqw commented May 2, 2021

Yes, certainly, but the file permissions have changed from the last version:

/tmp$ ls -l old/csources/build.sh 
-rwxr-xr-x 1 me me 640637 May  2 19:06 old/csources/build.sh
/tmp$ ls -l new/csources_v1/build.sh 
-rw-r--r-- 1 me me 981419 May  2 19:06 new/csources_v1/build.sh

So if that's left unchanged then every other distro's build scripts are going to encounter this. It just looked like an oversight unless it's been done deliberately for some reason. (Windows doesn't use this sort of permission so some people might not have seen this.)

@zqqw
Copy link
Author

zqqw commented May 2, 2021

This seems to build and work, also I've noticed recent versions of nim-git seem to lack nimfind too, at least the PKGBUILD can't find it:

--- oldPKGBUILD.bak	2021-05-02 19:56:30.783919211 +0100
+++ PKGBUILD	2021-05-02 20:28:45.364416939 +0100
@@ -7,10 +7,10 @@
 # Contributor: Zion Nimchuk <zionnimchuk@gmail.com>
 
 pkgbase='nim-git'
-pkgname=('nim-git' 'nimble-git' 'nimsuggest-git' 'nimpretty-git' 'nimfind-git' 'nim-gdb-git')
+pkgname=('nim-git' 'nimble-git' 'nimsuggest-git' 'nimpretty-git' 'nim-gdb-git')
 pkgdesc='Nim is a statically typed compiled systems programming language. It combines successful concepts from mature languages like Python, Ada and Modula. Its design focuses on efficiency, expressiveness, and elegance (in that order of priority).'
 epoch=1
-pkgver=1.4.2.r591.44ceefa9f
+pkgver=1.4.6.r1281.78e2d299d
 pkgrel=1
 arch=('x86_64')
 groups=('nim')
@@ -24,7 +24,7 @@
 makedepends=('git')
 source=(
   'git+https://github.com/nim-lang/Nim'
-  'git+https://github.com/nim-lang/csources'
+  'git+https://github.com/nim-lang/csources_v1'
   'git+https://github.com/nim-lang/nimble'
   'makepkg-conf.patch'
 )
@@ -61,12 +61,12 @@
 prepare() {
   cd Nim
 
-  [[ -d ./csources ]] && rm -rf ./csources
+  [[ -d ./csources_v1 ]] && rm -rf ./csources_v1
 
-  cp -r "${srcdir}/csources" .
+  cp -r "${srcdir}/csources_v1" .
 
   # Remove `-O3` from build.sh's COMP_FLAGS:
-  patch ./csources/build.sh \
+  patch ./csources_v1/build.sh \
   --strip=1                 \
   --fuzz 5                  \
   -N                        \
@@ -78,7 +78,8 @@
 
   # Build the pre-generated C sources of the Nim compiler which aid in
   # bootstrapping:
-  cd csources
+  cd csources_v1
+  chmod a+x build.sh
     ./build.sh
   cd -
 
@@ -199,16 +200,6 @@
   install -Dm 755 'Nim/bin/nimpretty' -t "${pkgdir}/usr/bin"
 }
 
-package_nimfind-git() {
-  pkgdesc='Nimfind is a tool that helps to give editors IDE like capabilities.'
-  url='https://github.com/nim-lang/Nim'
-  license=('MIT')
-  provides=('nimfind')
-  conflicts=('nimfind')
-
-  install -Dm 755 'Nim/bin/nimfind' -t "${pkgdir}/usr/bin"
-}
-
 package_nim-gdb-git() {
   pkgdesc='GDB pretty printing for Nim language.'
   url='https://github.com/nim-lang/Nim'

So I got nim-git-1:1.4.6.r1281.78e2d299d-1-x86_64.pkg.tar.xz which now causes a new Pakku toHashSet build error!

@jayvdb
Copy link

jayvdb commented May 4, 2021

I also encountered this when using brew install nim --HEAD , c.f. https://github.com/Homebrew/homebrew-core/blob/HEAD/Formula/nim.rb and Homebrew/homebrew-core#76550

@timotheecour
Copy link
Member

timotheecour commented May 4, 2021

the portable way to build nim from sources from any version moving forward is:

  • sh build_all.sh (builds from scratch csources(_v1), koch, nim, tools)
  • . ci/funs.sh && nimBuildCsourcesIfNeeded (builds from scratch a csources repo)

this allows forward/backward compatibility and building nim from source from any version moving forward even if csources repo/url/hash gets updated in the future

anything else is an implementation detail subject to change, in particular this is not future proof:

git clone https://github.com/nim-lang/csources_v1
cd csources_v1
make

because csources_v1 may get updated in the future (to address issues related to boostrapping by upgrading csources bootstrap version), and fetching HEAD may break a given version of nim.

is there a way you can you one of those 2 approaches? if not, what is missing, so that we can add support for it?

links

@zqqw
Copy link
Author

zqqw commented May 4, 2021

@timotheecour
Copy link
Member

using submodules for that in nim repo has been discussed before several times and rejected, you can search for it in github issues

@timotheecour
Copy link
Member

it seems like this issue should belong in nim-git repo (where is is?) rather than here; are the suggestions provided in #17907 (comment) enough to fix the OP's issue?

@zqqw
Copy link
Author

zqqw commented May 7, 2021

Unless I am experiencing Git user problems HEAD on this repo has been changed to an older commit:
commit f52aa6b (HEAD -> devel, origin/devel, origin/HEAD)
Date: Mon Apr 26 11:32:41 2021 -0700
fix typo in test name undeclared_routime.nim => undeclared_routine.nim (#17861)
So possibly it isn't a problem now, for the moment. (Edit - that was my local issue)
nim-git is in the AUR:
https://aur.archlinux.org/packages/nim-git/

@timotheecour
Copy link
Member

timotheecour commented May 7, 2021

well the problem will resurface once you update the commit to a newer one.

there are a few things in https://aur.archlinux.org/cgit/aur.git/tree/PKGBUILD?h=nim-git that IMO should be changed if you want it to keep working with future nim versions. I see you're avoiding sh build_all.sh and . ci/funs.sh && nimBuildCsourcesIfNeeded because you want to apply a patch to csources (patch ./csources/build.sh) is that correct?

if so, at least start from this:

. config/build_config.txt

which defines (for now) those variables:

nim_comment="key-value pairs for windows/posix bootstrapping build scripts"
nim_csourcesDir=csources_v1
nim_csourcesUrl=https://github.com/nim-lang/csources_v1.git
nim_csourcesHash=a8a5241f9475099c823cfe1a5e0ca4022ac201ff

then you can use those urls/hashes to at least build the correct version of csources (more future proof)

other than this, ok to close this issue since it looks like nim-git specific? (unless you have a specific suggestion for adding other APIs to ci/funs.sh to facilitate custom csources builds)

@kaushalmodi
Copy link
Contributor

Great! So I am not the only one facing this issue. I was finally finding some time to fix my long broken nim devel builds on Travis.

I thought my workaround for #15843 would work, but now am gated by this same error there: travis fail log

@0x647262
Copy link

0x647262 commented May 7, 2021

I see you're avoiding sh build_all.sh and . ci/funs.sh && nimBuildCsourcesIfNeeded because you want to apply a patch to csources (patch ./csources/build.sh) is that correct?

I wasn't using them out of ignorance, honestly. I've updated the PKGBUILD using the suggestions you've provided:

https://aur.archlinux.org/cgit/aur.git/tree/PKGBUILD?h=nim-git

@timotheecour
Copy link
Member

i see you're now using:

local -r hash="$(grep 'Hash' config/build_config.txt | grep -Eio '[0-9a-z]{40}')"

up to you but config/build_config.txt is meant to be source-able, so that you can instead use:

. config/build_config.txt
hash=$nim_csourcesHash

or similar

@zqqw
Copy link
Author

zqqw commented May 8, 2021

Sourcing PKGBUILD's was one of the reasons yaourt was deprecated, it's best avoided for security reasons, also other practical reasons like name space collisions or other unforseen accidental stuff:
https://security.stackexchange.com/questions/179481
So you probably wouldn't want to do that in a PKGBUILD - if it could be avoided - as parsing is safer AFAIK.

@timotheecour
Copy link
Member

how about:

local -r hash="$(grep 'Hash' config/build_config.txt | grep -Eio '[0-9a-z]{40}')"
=>
local -r hash="$(grep 'nim_csourcesHash' config/build_config.txt | grep -Eio '[0-9a-z]{40}')"

that way if other hash are later added, it'll still work

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants