Skip to content
This repository has been archived by the owner on Sep 19, 2021. It is now read-only.

[WIP] Add coverity and rebuild-deps build types. #8

Closed
wants to merge 3 commits into from

Conversation

fwalch
Copy link
Member

@fwalch fwalch commented Aug 3, 2014

Summary

This PR adds rebuilding of dependencies on Travis CI and pushes to neovim/deps. It also adds the coverity trigger from neovim/neovim and should remove the need for a coverity-scan branch there.

Two changes in neovim/neovim are necessary for this: neovim/neovim#1037, neovim/neovim#1033.

Coverity

Coverity is simply triggered for each build on this repo. Since this should happen only once per week (?) or if something is pushed, I hope that's okay. A commit that adds the encrypted COVERITY_SCAN_TOKEN would need to be added to make this work. If I understood correctly, with this it shouldn't be necessary to maintain a separate coverity-scan branch in neovim/neovim anymore.

Dependency rebuilding

Also added rebuilding of the dependencies to this PR. If this step is performed periodically, hopefully problems such as the ones related to luarocks (1 2) will be noticed more quickly and not only locally, but also on Travis.

This will unfortunately require a small change to neovim/neovim's travis.sh (neovim/neovim#1037) to change /opt/neovim-deps to /opt/neovim-deps/usr, or a change to third-party/CMakeLists.txt to be able to set DEPS_INSTALL_DIR etc. directly. Paths are hard-coded in some files, so just moving the files around after building doesn't work.

Helper script for local builds

Added a local.sh script to locally test builds more easily (see updated readme). Also testing builds on Travis should now be easier (see fwalch/bot-ci@f9bdabd).

Final notes

I think it's easier to review this commit by commit. If this is too much for one PR, I'll split it into multiple ones.

Regarding #4: I think that coverity triggering doesn't really belong into neovim/neovim. I know that there's already a dependency rebuilding script in neovim/deps, but I think it's easier to manage PRs if everything related to "bot builds" is centralized in this repo, and the "output repos" (neovim/doc, neovim/deps) contain as few non-autogenerated stuff as possible. I also think stuff like this should be run automatically and not rely on someone merging a branch to trigger a build.

Here is a Travis build; output in fwalch/doc and fwalch/deps. Here's a neovim build that uses the auto-generated dependencies from fwalch/deps. Both builds are using testing branches where I added a commit that changes the repositories from neovim/* to fwalch/*.

That was quite a lot of text, sorry.. please let me know what you think!

Ping @aktau @jszakmeister @justinmk @ZyX-I

@fwalch
Copy link
Member Author

fwalch commented Aug 4, 2014

Having trouble building 32 bit on Travis (noticed that output is actually x64), so marking [WIP].

@fwalch fwalch changed the title [RFC] Add coverity and rebuild-deps build types. [WIP] Add coverity and rebuild-deps build types. Aug 4, 2014
@fwalch
Copy link
Member Author

fwalch commented Aug 4, 2014

Okay, tried a few things, but the only way I got this working was with the workaround from @ZyX-I's build script (creating a GCC wrapper that sets -m32). I'll have a look how exactly it's done with cmake/i386-linux-gnu.toolchain.cmake in the main neovim build.

@fwalch fwalch changed the title [WIP] Add coverity and rebuild-deps build types. [WIP/RFC] Add coverity and rebuild-deps build types. Aug 4, 2014
@fwalch
Copy link
Member Author

fwalch commented Aug 5, 2014

Looking at the CMake source code, I can't see much special handling of CMAKE_C_COMPILER_ARG1 (where -m32 is set in the cmake toolchain file cmake/i386-linux-gnu.toolchain.cmake) other than that it's simply added as the first argument before the compiler is called. I didn't see it being concatenated to any other variables.

I therefore changed third-party/CMakeLists.txt to pass CMAKE_C_COMPILER_ARG1 and thus -m32 on to the dependency building process and got rid of the GCC wrapper in this PR. This is in neovim/neovim#1033.

@fwalch fwalch changed the title [WIP/RFC] Add coverity and rebuild-deps build types. [RFC] Add coverity and rebuild-deps build types. Aug 5, 2014
@jszakmeister
Copy link
Contributor

@fwalch

Looking at the CMake source code, I can't see much special handling of CMAKE_C_COMPILER_ARG1 other than that it's simply added as the first argument before the compiler is called. I didn't see it being concatenated to any other variables.

I therefore changed third-party/CMakeLists.txt to pass this on to the dependency building process and got rid of the GCC wrapper here. All neovim changes are in neovim/neovim#1033.

I'm sorry... I'm a bit worn out from traveling yesterday so I'm probably not firing on all cylinders, but why do you need to pass CMAKE_C_COMPILER_ARG1?

@ZyX-I
Copy link
Contributor

ZyX-I commented Aug 5, 2014

@jszakmeister To enforce target architecture.

I am not sure this is going to work with all luarocks stuff: AFAIR there were some issues with luarocks ignoring compiler settings.

@jszakmeister
Copy link
Contributor

@jszakmeister To enforce target architecture.

I am not sure this is going to work with all luarocks stuff: AFAIR there were some issues with luarocks ignoring compiler settings.

Thanks, but that didn't really explain where using CMAKE_C_COMPILER_ARG1--versus something else--was useful. Looking through the scripts more, it's because we're using the toolchain cmake file for the 32-bit build, and it's setting CMAKE_C_COMPILER_ARG1.

@fwalch
Copy link
Member Author

fwalch commented Aug 5, 2014

@jszakmeister Exactly, it was just because it was already set in the toolchain file. Sorry, I didn't explain this at all in my last comment, updated it.

This seems to have worked, at least the 32-bit Neovim build using these dependencies ran successfully, and file reported 32-bit for the files I checked.

I can see an error message, though:

/opt/neovim-deps/32/usr/bin/luarocks: 3: exec: /opt/neovim-deps/32/usr/bin/luajit: not found

This seems to be unrelated to my changes, however, because it's also in a build on neovim master:

/opt/neovim-deps/32/bin/luarocks: 3: exec: /opt/neovim-deps/32/bin/luajit: not found

@fwalch
Copy link
Member Author

fwalch commented Aug 7, 2014

Updated to add the color_terminal.lua script from neovim/deps#2 and copy it to the deps repository when building. Dependencies in fwalch/deps have been updated by Travis.

Here's a neovim build based on current neovim/neovim master and neovim/neovim#1037 that uses these dependencies.

@fwalch fwalch changed the title [RFC] Add coverity and rebuild-deps build types. [WIP] Add coverity and rebuild-deps build types. Aug 20, 2014
@fwalch fwalch mentioned this pull request Aug 22, 2014
26 tasks
@fwalch
Copy link
Member Author

fwalch commented Aug 29, 2014

Will redo this in two separate PRs after #14 is in.

@fwalch fwalch closed this Aug 29, 2014
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

3 participants