nixos compat

[Artturin]

make building docs optional

look for pandoc to see a big red error instead of seeing a small text that says "pandoc not
found"

if CMAKE_INSTALL_LIBDIR is set then add that to default NGS_PATH

build-scripts: check if the CC variable is set and if it is not then use
cpp. when cross-compiling, the compiler is prefixed with the target

i'm packaging ngs for nixos/nixpkgs and encountered some issues

[Artturin]

currently the tests are failing with

ngs> +--------------------------------------------------------+
ngs> | Test failed: dir("/").path.has("/var"). Returned false |
ngs> +--------------------------------------------------------+

in nix we build packages in a sandbox so there are no fhs paths there

[Artturin]

@jirutka this will make your patch unnecessary https://git.alpinelinux.org/aports/tree/testing/ngs/cmakelists.patch

[Artturin]

Will rebase properly once I am on computer again

[ilyash-b]

Hello!

Happy to see your contribution! I will be looking into it.

The correct target branch is dev.

no fhs paths there

Any alternative to look for?

My main concern is: how do I know the build works? For all supported platforms, we have GitHub CI builds and tests in .github/workflows/build.yml. Is it possible to add build+test there for NixOS?

[Artturin]

no fhs paths there

Any alternative to look for?

there's these, however i'd recommend not checking that something has some path, the only safeish thing you can use is /.

ngs> ++++ ls /
ngs> bin  build  dev  etc  nix  proc  tmp
ngs> ++++ ls /bin /build /dev /etc /nix /proc /tmp
ngs> /bin:
ngs> sh
ngs> /build:  # this is the dir where build 
ngs> env-vars  ngs
ngs> /dev:
ngs> fd    kvm   ptmx  random  stderr  stdout  urandom
ngs> full  null  pts   shm        stdin   tty     zero
ngs> /etc:
ngs> group  hosts  passwd
ngs> /nix:
ngs> store
ngs> /proc:
ngs> 1     crypto         iomem        loadavg       pressure       thread-self
ngs> 19    devices        ioports      locks         scsi           timer_list
ngs> acpi          diskstats      irq          lrng_type     self           tty
ngs> asound        dma            kallsyms     meminfo       slabinfo       uptime
ngs> buddyinfo  driver    kcore        misc          softirqs       version
ngs> bus           dynamic_debug  key-users    modules       spl            vmallocinfo
ngs> cgroups    execdomains       keys         mounts        stat           vmstat
ngs> cmdline    fb                kmsg         mtrr          swaps          zoneinfo
ngs> config.gz  filesystems       kpagecgroup  net           sys
ngs> consoles   fs                kpagecount   pagetypeinfo  sysrq-trigger
ngs> cpuinfo    interrupts        kpageflags   partitions    sysvipc
ngs> /tmp:

dependencies and such are passed via env vars to the build.

here's a gist with the non-cross, static musl and a cross env-vars files which are sourced by the builder
https://gist.github.com/Artturin/d8658aca90355b11c2dae400286b7c78

My main concern is: how do I know the build works? For all supported platforms, we have GitHub CI builds and tests in .github/workflows/build.yml. Is it possible to add build+test there for NixOS?

sure i'll write ci

[Artturin]

Could you make the CI run on PRs?

here's a patch for it

diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml
index 55a4c53..e948546 100644
--- a/.github/workflows/build.yml
+++ b/.github/workflows/build.yml
@@ -1,5 +1,7 @@
 name: Build
-on: push
+on:
+  push
+  pull_request_target
 
 jobs:
   build-internal-apt:

[ilyash-b]

Could you make the CI run on PRs?
here's a patch for it

Applied in dev

there's these, however i'd recommend not checking that something has some path, the only safeish thing you can use is /.

I still prefer to check for something (than not having the test) so in dev the check is now looking for one of the following: "/var", "/etc", "/bin"

Anything else needed from my side?

I appreciate the love that CMakeLists.txt gets, it was not getting enough.

How do I know when the PR is ready for review?

[Artturin]

I'll undraft once ready

[Artturin]

sorry kinda busy right now. This pr is ready to be merged as is though, i'll add ci later.

[ilyash-b]

Roughly the same here, not getting to it

[ilyash-b]

Tried to merge. Did not go well. The issues are:

1. Does not find pcre.h on MacOS - https://github.com/ngs-lang/ngs/runs/6948954852?check_suite_focus=true#step:4:134
2. Overwrites lib/stdlib.ngs at file(WRITE lib/stdlib.ngs "${FILE_CONTENTS}") (for this, I would like to try to find more elegant mechanism, without modifying the file at all, probably through C code, which would expose the define value. I guess it's something I can try to do)
3. The value used to replace #@INSTALL_LIBDIR@ is lib/ngs while I would expect absolute path

1 and 3 here sound like would take me some time and I have the impression that you are more skilled at this so it would be great if you took a look

Additional info: the linked build is based on dev branch (more precisely ngs/nixoscompat is dev + your changes)

[ilyash-b]

I'm sorry this is stuck for a while now. I'll try to take a second look.

[ilyash-b]

Why install(DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/lib/ DESTINATION ${CMAKE_INSTALL_LIBDIR}/ngs) is unconditional while string(REPLACE "#@INSTALL_LIBDIR@" ${CMAKE_INSTALL_LIBDIR}/ngs FILE_CONTENTS "${FILE_CONTENTS}") is wrapped in if(CMAKE_INSTALL_LIBDIR)?

[ilyash-b]

Some lines only differ in spacing. It would be easier to review the PR without this change. If spacing needs to be fixed (I don't know whether it's the case), it's better to do it in a separate PR.

[Artturin]

Some lines only differ in spacing. It would be easier to review the PR without this change. If spacing needs to be fixed (I don't know whether it's the case), it's better to do it in a separate PR.

I'll rebase, I guess the me 2 years ago didn't know PR hygiene well yet.

[ilyash-b]

If it makes sense to you, @Artturin , you can also split this PR into two: one for optional pandoc build and one for the rest.

[Artturin]

Part 1 #669

[ilyash-b]

Part 1 #669

Reviewed and commented

[ilyash-b]

Can we close this one in favor of the smaller ones?