search

michaelforney / samurai

ninja-compatible build tool written in C
Other
832 stars 47 forks source link

FreeBSD Ports exp-run results #70

Closed ghost closed 3 years ago

ghost commented 3 years ago

Hi,

the first round of the exp-run has finished. There were only a handful of failures.

  1. Our ninja package depends on Python and unsurprisingly there were many issues related to underspecified build dependencies as a result since samurai has no dependencies itself.

  2. Many build order bugs. I tried addressing a few:

    Remaining issues are

  3. One self-made issue with argument order in our framework. -v was always added as the last argument to Ninja but thanks to SAMUFLAGS we do not even need to do this with Samurai.

  4. A samurai/ninja behavior difference in sysutils/fluxengine. It has its own Ninja generator which creates a build file with extra whitespace. Ninja accepts it but Samurai does not. This was easy enough to fix but I wonder if this could be considered a bug in Samurai. Please see the commit for more details: https://github.com/freebsd/freebsd-ports/commit/7b1d905b03cc85d834d5cba7df8ab22f9f51e0ca

michaelforney commented 3 years ago

Thanks for the update! That sounds really promising. I'm really happy to see FreeBSD consider using samurai since you have such a large ports tree. It should really open the door for other operating systems to try it as well.

  1. Many build order bugs. I tried addressing a few:

Do you have plans to submit these patches upstream? It would be quite nice to get these sort of issues fixed for the benefit of everyone using samurai.

If you have any doubts about any of the patches, let me know and I can try to take a look.

  1. A samurai/ninja behavior difference in sysutils/fluxengine. It has its own Ninja generator which creates a build file with extra whitespace. Ninja accepts it but Samurai does not. This was easy enough to fix but I wonder if this could be considered a bug in Samurai. Please see the commit for more details: freebsd/freebsd-ports@7b1d905

I pushed a fix for this in aff20851, let me know if that works. I wish the ninja manual was a bit more detailed about the syntax :/

ghost commented 3 years ago

Do you have plans to submit these patches upstream?

If you have any doubts about any of the patches, let me know and I can try to take a look.

I have doubts about all of the patches but let's see what the upstreams say.

I have some trouble figuring out what the fbthrift issue even could be. It's a maze of CMake build files. I also cannot reproduce the failure on my machines...

I pushed a fix for this in aff2085, let me know if that works. I wish the ninja manual was a bit more detailed about the syntax :/

Yes, thanks. That seems to work fine.

michaelforney commented 3 years ago

fbthrift bug should be fixed by https://github.com/facebook/fbthrift/pull/422

Is it just bijiben that is left after this?

michaelforney commented 3 years ago

gnome-notes (bijiben) pull request submitted: https://gitlab.gnome.org/GNOME/gnome-notes/-/merge_requests/116

ghost commented 3 years ago

Thank you. I believe that bijiben was the last failure. I've applied your patches to the ports tree and have asked for another exp-run round.

ghost commented 3 years ago

I've noticed something. qt5-webengine's configure fails to recognize samurai as valid system ninja and builds a bundled ninja(!).

The check goes like this (configure.pri line 163):

defineTest(qtConfTest_detectNinja) {
    ninja = $$qtConfFindInPath("ninja$$EXE_SUFFIX")
    !isEmpty(ninja) {
        qtLog("Found ninja from path: $$ninja")
        qtRunLoggedCommand("$$ninja --version", version)|return(false)
        contains(version, "1\.([7-9]|1[0-9])\..*"): return(true)
        qtLog("Ninja version too old")
    }
    qtLog("Building own ninja")
    return(false)
}

It tries to run ninja --version and expects it to return a 3 point version which samu does not do.

$ samu --version
1.9

$ ninja --version
1.10.2

If I patch samu to return 1.9.0 instead it passes the check. Is it possible to change this?

michaelforney commented 3 years ago

If I patch samu to return 1.9.0 instead it passes the check. Is it possible to change this?

Done in ca5a6ba6.

ghost commented 3 years ago

The exp-run has finished and it looks good, only one new failure in deno. Something wrong with the bundled v8 build. Not sure what's going one with that. I might just hardcode ninja there for now to be done with it. http://package18.nyi.freebsd.org/data/122amd64-default-foo/2021-04-10_08h07m32s/logs/errors/deno-1.8.2_1.log

On a more general note I noticed that some escape sequences for color compiler diagnostics make it through to the build logs. I disabled that for Meson builds by passing -Db_colorout=never to them as per Samurai's README.md but many CMake or GN builds force on colors and every project does it slightly differently without any setting that I could find to disable that globally. :-( This isn't terribly important but do you have any advice what can be done here if anything? I realize that from samu's side there probably cannot be done much about it.

michaelforney commented 3 years ago

The exp-run has finished and it looks good, only one new failure in deno. Something wrong with the bundled v8 build. Not sure what's going one with that. I might just hardcode ninja there for now to be done with it. http://package18.nyi.freebsd.org/data/122amd64-default-foo/2021-04-10_08h07m32s/logs/errors/deno-1.8.2_1.log

Though this looks like the same type of error as other build-order issues (missing header error), I am a bit suspicious about this one since src/objects/function-syntax-kind.h is not a generated header, it is tracked in source control. Additionally, the appropriate include directory appears on the command-line: -I../../../deno-1.8.2/cargo-crates/rusty_v8-0.21.0/v8

Additionally, we have these lines

warning: c++: error: no such file or directory: 'src/vendor/SPIRV-Cross/spirv_cross.cpp'
warning: c++: error: no input files
warning: c++: error: no such file or directory: 'src/vendor/SPIRV-Cross/spirv_cross_parsed_ir.cpp'
warning: c++: error: no input files

The following warnings were emitted during compilation:

warning: cc: error: no such file or directory: 'c/freebsd.c'
warning: cc: error: no input files

Are you sure that the build with ninja does not have these problems? Can you reproduce the failure and poke around the build directory? I don't seem to be able to reproduce it locally when building rusty_v8 (but I tested on Linux since I don't have a FreeBSD install and the build took too long to run on sourcehut).

On a more general note I noticed that some escape sequences for color compiler diagnostics make it through to the build logs. I disabled that for Meson builds by passing -Db_colorout=never to them as per Samurai's README.md but many CMake or GN builds force on colors and every project does it slightly differently without any setting that I could find to disable that globally. :-( This isn't terribly important but do you have any advice what can be done here if anything? I realize that from samu's side there probably cannot be done much about it.

Sorry, I'm not sure. It seems these projects are hard-coding -fdiagnostics-color=always into their build files. Can you pipe the output into some tool/script that strips the escape sequences?

ghost commented 3 years ago

I went ahead and committed samurai support to the ports tree. It can be switched on by having DEFAULT_VERSIONS+=ninja=samurai in make.conf now.

Can you reproduce the failure and poke around the build directory?

I have been unable to reproduce it myself. In the mean time deno was also updated, so maybe the problem is already gone (but rusty_v8 wasn't updated so probably not). I've asked for a rebuilt on the package builder and for a copy of the working directory in case it fails again.

Can you pipe the output into some tool/script that strips the escape sequences?

Hmm, I'm not sure this is worth the trouble it might cause. It also feels like solving the problem at the wrong layer. What probably should happen is to implement something like b_colorout in CMake . It's just how it is at the moment then. The logs can be viewed with less -R anyway.

ghost commented 3 years ago

I think that's all. Thanks for all your help.

michaelforney commented 3 years ago

Thank you for tracking down all those failing packages :)