Skip to content

Commit

Permalink
fix linenoise regression (#13395)
Browse files Browse the repository at this point in the history
* fix nightlies linenoise regression

* fix other installers
  • Loading branch information
timotheecour authored Feb 12, 2020
1 parent 90491ea commit 2b368bc
Showing 1 changed file with 19 additions and 11 deletions.
30 changes: 19 additions & 11 deletions tools/niminst/niminst.nim
Original file line number Diff line number Diff line change
Expand Up @@ -505,12 +505,20 @@ proc writeInstallScripts(c: var ConfigData) =
writeFile(deinstallShFile, generateDeinstallScript(c), "\10")
inclFilePermissions(deinstallShFile, {fpUserExec, fpGroupExec, fpOthersExec})

template gatherFiles(fun, libpath, outDir) =
block:
template copySrc(src) =
let dst = outDir / extractFilename(src)
when false: echo (dst, dst)
fun(src, dst)

for f in walkFiles(libpath / "lib/*.h"): copySrc(f)
copySrc(libpath / "lib/wrappers/linenoise/linenoise.h")

proc srcdist(c: var ConfigData) =
if not existsDir(getOutputDir(c) / "c_code"):
createDir(getOutputDir(c) / "c_code")
for x in walkFiles(c.libpath / "lib/*.h"):
when false: echo(getOutputDir(c) / "c_code" / extractFilename(x))
copyFile(dest=getOutputDir(c) / "c_code" / extractFilename(x), source=x)
let cCodeDir = getOutputDir(c) / "c_code"
if not existsDir(cCodeDir): createDir(cCodeDir)
gatherFiles(copyFile, c.libpath, cCodeDir)
var winIndex = -1
var intel32Index = -1
var intel64Index = -1
Expand Down Expand Up @@ -603,8 +611,9 @@ when haveZipLib:
addFile(z, proj / makeFile, "build" / makeFile)
addFile(z, proj / installShFile, installShFile)
addFile(z, proj / deinstallShFile, deinstallShFile)
for f in walkFiles(c.libpath / "lib/*.h"):
addFile(z, proj / "c_code" / extractFilename(f), f)

template addFileAux(src, dst) = addFile(z, dst, src)
gatherFiles(addFileAux, c.libpath, proj / "c_code")
for osA in 1..c.oses.len:
for cpuA in 1..c.cpus.len:
var dir = buildDir(osA, cpuA)
Expand Down Expand Up @@ -647,8 +656,8 @@ proc xzDist(c: var ConfigData; windowsZip=false) =
processFile(proj / makeFile, "build" / makeFile)
processFile(proj / installShFile, installShFile)
processFile(proj / deinstallShFile, deinstallShFile)
for f in walkFiles(c.libpath / "lib/*.h"):
processFile(proj / "c_code" / extractFilename(f), f)
template processFileAux(src, dst) = processFile(dst, src)
gatherFiles(processFileAux, c.libpath, proj / "c_code")
for osA in 1..c.oses.len:
for cpuA in 1..c.cpus.len:
var dir = buildDir(osA, cpuA)
Expand Down Expand Up @@ -722,8 +731,7 @@ proc debDist(c: var ConfigData) =
copyNimDist(makeFile, makeFile)
copyNimDist(installShFile, installShFile)
createDir(workingDir / upstreamSource / "build")
for f in walkFiles(c.libpath / "lib/*.h"):
copyNimDist(f, "build" / extractFilename(f))
gatherFiles(copyNimDist, c.libpath, "build")
for osA in 1..c.oses.len:
for cpuA in 1..c.cpus.len:
var dir = buildDir(osA, cpuA)
Expand Down

6 comments on commit 2b368bc

@drjdn
Copy link

@drjdn drjdn commented on 2b368bc Feb 12, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@timotheecour after this commit I'm getting the following error:

niminst csource main.ini -d:release
/home/jdn/nim/tools/niminst/niminst.nim(764) niminst
/home/jdn/nim/tools/niminst/niminst.nim(516) srcdist
/home/jdn/nim/lib/pure/os.nim(1650) copyFile
/home/jdn/nim/lib/pure/includes/oserr.nim(94) raiseOSError
Error: unhandled exception: No such file or directory
Additional info: "lib/wrappers/linenoise/linenoise.h" [OSError]
make: *** [Makefile:20: standalone] Error 1

when using niminst. I'm on a Debian 10 x86-64 system.

@timotheecour
Copy link
Member Author

@timotheecour timotheecour commented on 2b368bc Feb 12, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@drjdn it's hard to fix this without know what to run; could you please provide instructions for how to reproduce this from a fresh Nim clone?

here's what i tried (on OSX but doubt error I'm getting below is related to osx):

git clone -q https://github.com/nim-lang/Nim
cd Nim
sh build_all.sh
choosenim . # avoid other nim clashing with this
cd tools/niminst
nim c niminst
./niminst csource main.ini -d:release
cannot open: main.ini

(also full logs would help)

@drjdn
Copy link

@drjdn drjdn commented on 2b368bc Feb 12, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@timotheecour I've added a simple hello world example that fails as above at my github repo. There is a copy of nimbase.h in lib/. I use a soft link but here I just copied the current version from mylocation/nim/lib.

@timotheecour
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like you're using a non-standard configuration so IMO this isn't a regression

doesn't look like a good idea to copy Nim/lib/nimbase.h into another repo https://github.com/drjdn/hello_standalone/blob/master/lib/nimbase.h since it causes duplication; instead a build tool that would automatically copy files seems like a better idea (you're missing other files too if you're gonna go this route, eg lib/cycle.h). Using a softlink is better indeed, but still should use a build tool for that.

in your specific case, I guess all you'd need is for your build tool to also copy lib/wrappers/linenoise/linenoise.h, or (even better) add a tool to niminst that returns a json string/file that specifies all the files that need to be copied as part of a proper install, to avoid duplicating any logic

@drjdn
Copy link

@drjdn drjdn commented on 2b368bc Feb 13, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@timotheecour this is a regression as the repo that I put up for you to debug the issue worked fine before your commit and doesn't now. niminst creates standalone c code that can run without nim. That c code needs nimbase.h to build. In practice I soft link to my current systems nimbase.h but in the repo I just copied the current version from nim's git head. Once the c code is generated you can ship it with that version of nimbase.h and it will build correctly. If you can explain to me why my hello world example needs the header file to a line editing library to work I'll agree this isn't a regression.

@timotheecour
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

=> PR #13413

Please sign in to comment.