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

[staging] treewide: cmake buildInputs to nativeBuildInputs, minor cleanups #108022

Merged
merged 2 commits into from
Jan 1, 2021

Conversation

siraben
Copy link
Member

@siraben siraben commented Dec 31, 2020

Motivation for this change

It is very likely that cmake should be in nativeBuildInputs rather than buildInputs, which would matter when cross-compiling, cmake should be built on the build platform but not the host.

Additionally I am performing minor changes to the expressions that should not change the outcome, for instance

  • pkgconfig -> pkg-config
  • fetchFromGit -> fetchFromGitHub
  • adding missing license
  • gpl(2|3) -> gpl(2|3)(Only|Plus)
  • remove enableParallelBuilding = true (already defaults to true when using cmake)

Edit: after seeing the affected packages approach 1000+ I have made much more conservative changes, future PRs should be done to check if packages like flex, bison, doxygen etc. are placed in the correct inputs list.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@SuperSandro2000
Copy link
Member

Can you also add a minor ceanups like treewide: move cmake from buildInputs to nativeBuildInputs, minor cleanups?

@siraben siraben changed the title treewide: move cmake from buildInputs to nativeBuildInputs treewide: cmake buildInputs to nativeBuildInputs, minor cleanups Dec 31, 2020
@siraben
Copy link
Member Author

siraben commented Dec 31, 2020

Does anyone know of situations where you do want cmake to be in buildInputs?

Pending packages that trip up the regex:

ag -l "buildInputs.*cmake" -f pkgs
pkgs/applications/video/kodi/plugins.nix
pkgs/applications/networking/instant-messengers/pidgin-plugins/purple-vk-plugin/default.nix
pkgs/applications/audio/sorcer/default.nix
pkgs/applications/audio/artyFX/default.nix
pkgs/applications/audio/eq10q/default.nix
pkgs/applications/audio/game-music-emu/default.nix
pkgs/applications/audio/ympd/default.nix
pkgs/applications/misc/sqliteman/default.nix
pkgs/applications/misc/dfilemanager/default.nix
pkgs/applications/misc/kiwix/default.nix
pkgs/applications/misc/opencpn/default.nix
pkgs/applications/misc/posterazor/default.nix
pkgs/applications/misc/keepassx/2.0.nix
pkgs/applications/graphics/pbrt/default.nix
pkgs/applications/graphics/screencloud/default.nix
pkgs/applications/graphics/autopanosiftc/default.nix
pkgs/applications/graphics/smartdeblur/default.nix
pkgs/applications/graphics/scantailor/default.nix
pkgs/applications/graphics/freepv/default.nix
pkgs/applications/version-management/sit/default.nix
pkgs/applications/science/biology/cmtk/default.nix
pkgs/applications/science/biology/somatic-sniper/default.nix
pkgs/applications/science/robotics/yarp/default.nix
pkgs/applications/science/logic/monosat/default.nix
pkgs/applications/science/logic/mcrl2/default.nix
pkgs/applications/science/logic/lean2/default.nix
pkgs/applications/science/logic/avy/default.nix
pkgs/applications/science/electronics/qucs/default.nix
pkgs/applications/science/electronics/qfsm/default.nix
pkgs/applications/science/misc/root/5.nix
pkgs/applications/science/misc/openmodelica/default.nix
pkgs/applications/science/misc/vite/default.nix
pkgs/applications/editors/bonzomatic/default.nix
pkgs/applications/editors/emacs-modes/melpa-packages.nix
pkgs/applications/office/tagainijisho/default.nix
pkgs/development/python-modules/ovito/default.nix
pkgs/development/ocaml-modules/llvm/default.nix
pkgs/development/tools/build-managers/arpa2cm/default.nix
pkgs/development/tools/xcbuild/default.nix
pkgs/development/tools/minizinc/default.nix
pkgs/development/tools/solarus-quest-editor/default.nix
pkgs/development/tools/misc/automoc4/default.nix
pkgs/development/tools/misc/xc3sprog/default.nix
pkgs/development/tools/analysis/ikos/default.nix
pkgs/development/tools/msgpack-tools/default.nix
pkgs/development/tools/rucksack/default.nix
pkgs/development/interpreters/proglodyte-wasm/default.nix
pkgs/development/interpreters/self/default.nix
pkgs/development/interpreters/falcon/default.nix
pkgs/development/ruby-modules/gem-config/default.nix
pkgs/development/libraries/assimp/default.nix
pkgs/development/libraries/qjson/default.nix
pkgs/development/libraries/libtcod/default.nix
pkgs/development/libraries/ctpp2/default.nix
pkgs/development/libraries/libsnark/default.nix
pkgs/development/libraries/csfml/default.nix
pkgs/development/libraries/mygui/default.nix
pkgs/development/libraries/lucene++/default.nix
pkgs/development/libraries/kf5gpgmepp/default.nix
pkgs/development/libraries/curlcpp/default.nix
pkgs/development/libraries/PlistCpp/default.nix
pkgs/development/libraries/cpp-ipfs-api/default.nix
pkgs/development/libraries/libdynd/default.nix
pkgs/development/libraries/ptex/default.nix
pkgs/development/libraries/cpp-netlib/default.nix
pkgs/development/libraries/properties-cpp/default.nix
pkgs/development/libraries/glbinding/default.nix
pkgs/development/libraries/libbladeRF/default.nix
pkgs/development/libraries/freeglut/default.nix
pkgs/development/libraries/liquidfun/default.nix
pkgs/development/libraries/rnnoise-plugin/default.nix
pkgs/development/libraries/libresample/default.nix
pkgs/development/libraries/libLAS/default.nix
pkgs/development/libraries/audio/libgme/default.nix
pkgs/development/libraries/audio/jamomacore/default.nix
pkgs/development/libraries/nanoflann/default.nix
pkgs/development/libraries/grib-api/default.nix
pkgs/development/libraries/flann/default.nix
pkgs/development/libraries/alure/default.nix
pkgs/development/libraries/embree/2.x.nix
pkgs/development/libraries/qimageblitz/default.nix
pkgs/development/libraries/cegui/default.nix
pkgs/development/libraries/unittest-cpp/default.nix
pkgs/development/libraries/liblaxjson/default.nix
pkgs/development/libraries/libjreen/default.nix
pkgs/development/libraries/gdcm/default.nix
pkgs/development/libraries/vxl/default.nix
pkgs/development/libraries/libharu/default.nix
pkgs/development/libraries/opentracing-cpp/default.nix
pkgs/development/libraries/cppdb/default.nix
pkgs/development/libraries/google-cloud-cpp/default.nix
pkgs/development/libraries/grantlee/default.nix
pkgs/development/libraries/libuecc/default.nix
pkgs/development/libraries/rabbitmq-c/default.nix
pkgs/development/libraries/portmidi/default.nix
pkgs/development/libraries/science/robotics/ispike/default.nix
pkgs/development/libraries/science/math/superlu/default.nix
pkgs/development/libraries/science/math/metis/default.nix
pkgs/development/libraries/science/math/parmetis/default.nix
pkgs/development/libraries/vigra/default.nix
pkgs/development/libraries/grpc/default.nix
pkgs/development/libraries/cppcms/default.nix
pkgs/development/libraries/libebur128/default.nix
pkgs/development/libraries/libjson-rpc-cpp/default.nix
pkgs/development/libraries/NSPlist/default.nix
pkgs/development/libraries/nss_wrapper/default.nix
pkgs/development/libraries/opencollada/default.nix
pkgs/development/libraries/vrpn/default.nix
pkgs/development/libraries/libmusicbrainz/default.nix
pkgs/development/libraries/libmusicbrainz/5.x.nix
pkgs/development/libraries/libnabo/default.nix
pkgs/development/libraries/libbluedevil/default.nix
pkgs/development/libraries/libgroove/default.nix
pkgs/development/compilers/dale/default.nix
pkgs/development/compilers/seexpr/default.nix
pkgs/development/compilers/mono/llvm.nix
pkgs/development/compilers/ponyc/default.nix
pkgs/os-specific/darwin/swift-corelibs/libdispatch.nix
pkgs/os-specific/darwin/darling/default.nix
pkgs/misc/my-env/default.nix
pkgs/misc/screensavers/xss-lock/default.nix
pkgs/misc/emulators/hatari/default.nix
pkgs/data/misc/shared-desktop-ontologies/default.nix
pkgs/tools/filesystems/irods/common.nix
pkgs/tools/filesystems/unionfs-fuse/default.nix
pkgs/tools/compression/lzham/default.nix
pkgs/tools/admin/tightvnc/default.nix
pkgs/tools/audio/acoustid-fingerprinter/default.nix
pkgs/tools/misc/wv2/default.nix
pkgs/tools/misc/aspcud/default.nix
pkgs/tools/security/haka/default.nix
pkgs/tools/inputmethods/fcitx/fcitx-configtool.nix
pkgs/tools/inputmethods/fcitx-engines/fcitx-table-other/default.nix
pkgs/tools/inputmethods/fcitx-engines/fcitx-hangul/default.nix
pkgs/tools/inputmethods/fcitx-engines/fcitx-unikey/default.nix
pkgs/tools/inputmethods/fcitx-engines/fcitx-table-extra/default.nix
pkgs/tools/inputmethods/fcitx-engines/fcitx-libpinyin/default.nix
pkgs/tools/inputmethods/fcitx-engines/fcitx-cloudpinyin/default.nix
pkgs/tools/inputmethods/fcitx-engines/fcitx-chewing/default.nix
pkgs/tools/inputmethods/fcitx-engines/fcitx-rime/default.nix
pkgs/tools/inputmethods/fcitx-engines/fcitx-anthy/default.nix
pkgs/tools/inputmethods/fcitx-engines/fcitx-m17n/default.nix
pkgs/tools/inputmethods/ibus-engines/ibus-table-chinese/default.nix
pkgs/tools/cd-dvd/vobsub2srt/default.nix
pkgs/tools/cd-dvd/cdrkit/default.nix
pkgs/servers/rippled/default.nix
pkgs/servers/uhub/default.nix
pkgs/servers/sql/percona/5.6.x.nix

pkgs/applications/audio/rkrlv2/default.nix Outdated Show resolved Hide resolved
pkgs/applications/audio/rkrlv2/default.nix Outdated Show resolved Hide resolved
pkgs/games/blobby/default.nix Outdated Show resolved Hide resolved
pkgs/games/blobby/default.nix Outdated Show resolved Hide resolved
pkgs/games/megaglest/default.nix Outdated Show resolved Hide resolved
pkgs/games/rigsofrods/default.nix Outdated Show resolved Hide resolved
pkgs/games/spring/default.nix Outdated Show resolved Hide resolved
pkgs/games/trackballs/default.nix Show resolved Hide resolved
pkgs/games/trackballs/default.nix Show resolved Hide resolved
@ofborg ofborg bot added the 8.has: package (new) This PR adds a new package label Dec 31, 2020
@SuperSandro2000
Copy link
Member

SuperSandro2000 commented Dec 31, 2020

Does anyone know of situations where you do want cmake to be in buildInputs?

Pending packages that trip up the regex:

ag -l "buildInputs.*cmake" -f pkgs

There exists maybe one but I am not aware of any right now and if a package really needs cmake in buildInputs we should comment that.

Also in the past similar things where made with pkg-config and autoReconfHook.

531e4b8

Having cmake and other buildTools in nativeBuildInputs is really important for cross compiling and matters there the most.

But changing all the other cmake into nativeBuildInputs could be a part two PR to get this PR merged in the next days.

@siraben siraben force-pushed the cmake-native-build-inputs branch 4 times, most recently from cbbabd7 to 99de0d3 Compare December 31, 2020 07:58
@siraben
Copy link
Member Author

siraben commented Dec 31, 2020

@Ericson2314 do you still have that sed expression you used in 531e4b8?

@siraben
Copy link
Member Author

siraben commented Dec 31, 2020

Bumped trackballs from 1.3.1 to 1.3.2, game builds and runs.

@siraben
Copy link
Member Author

siraben commented Dec 31, 2020

I can target staging instead if the rebuilds are too much.

@FRidh
Copy link
Member

FRidh commented Dec 31, 2020

master is fine for now

@siraben
Copy link
Member Author

siraben commented Dec 31, 2020

I've noticed that in several cases cmake being in buildInputs can be indicative of a misunderstanding of the two buildInputs, since other packages like automake, gnumake, texinfo end up in the buildInputs as well.

@ofborg ofborg bot added the 6.topic: emacs Text editor label Dec 31, 2020
@FRidh
Copy link
Member

FRidh commented Dec 31, 2020

When making these changes its good for your sanity to also set strictDeps = true; to prevent regressions. Ideally we do this in stdenv.mkDerivation, but that will break people's code outside Nixpkgs.

@siraben
Copy link
Member Author

siraben commented Dec 31, 2020

I do not have enough computing resources to do a proper nix-review, but I've tested several of the smaller packages and they build fine.

@siraben siraben force-pushed the cmake-native-build-inputs branch 2 times, most recently from a7eec40 to 5fd50b1 Compare December 31, 2020 12:06
@jonringer
Copy link
Contributor

this is also getting to the point that we should move this to staging:

$ ./maintainers/scripts/rebuild-amount.sh HEAD
Estimating rebuild amount by counting changed Hydra jobs.
   1549 x86_64-darwin
   1138 x86_64-linux

@siraben
Copy link
Member Author

siraben commented Dec 31, 2020

Ok, I will rebase on staging. Which of the staging branches should I rebase on?

@siraben
Copy link
Member Author

siraben commented Dec 31, 2020

I think this is enough changes for now, so we can see what regressions were introduced and remediate them. Thanks all for the feedback so far.

@jonringer
Copy link
Contributor

Ok, I will rebase on staging. Which of the staging branches should I rebase on?

staging is the staging branch for unstable

@jonringer
Copy link
Contributor

/rebase-staging

@siraben siraben changed the base branch from master to staging December 31, 2020 18:12
@siraben siraben force-pushed the cmake-native-build-inputs branch 3 times, most recently from 343bb73 to 47ad347 Compare December 31, 2020 18:15
@github-actions
Copy link
Contributor

@siraben
Copy link
Member Author

siraben commented Dec 31, 2020

Hah, didn't know it was going to be done automatically, I just did it and force-pushed.

@Ericson2314
Copy link
Member

@Ericson2314 do you still have that sed expression you used in 531e4b8?

digs through fish history

begin 
  git ls-files | xargs sed -i -E \
    -e 's%( *buildInputs.*=.*\[.*)autoreconfHook%\1FOO_FOO_FOO%g' \
    -e 's%( *buildInputs.*=.*\[.*)pkgconfig%\1BAR_BAR_BAR%g'
  git ls-files | xargs sed -i -E \
     -e '/FOO_FOO_FOO.*BAR_BAR_BAR/i \  nativeBuildInputs = [ autoreconfHook pkgconfig ];' \
     -e '/BAR_BAR_BAR.*FOO_FOO_FOO/i \  nativeBuildInputs = [ autoreconfHook pkgconfig ];'
  git ls-files | xargs sed -i -E -e 's% *FOO_FOO_FOO(.*) +BAR_BAR_BAR%\1%g' -e 's% *BAR_BAR_BAR(.*) +FOO_FOO_FOO%\1%g'
  git ls-files | xargs sed -i -E \
     -e '/FOO_FOO_FOO/i \  nativeBuildInputs = [ autoreconfHook ];' \
     -e '/BAR_BAR_BAR/i \  nativeBuildInputs = [ pkgconfig ];'
  git ls-files | xargs sed -i -E -e 's% *FOO_FOO_FOO%%g' -e 's% *BAR_BAR_BAR%%g'
end

But this is an abomination and someone should use hnix to write something better :).

@Ericson2314
Copy link
Member

Ericson2314 commented Dec 31, 2020

Thanks for doing this grunt work, @siraben!

@jonringer
Copy link
Contributor

Hah, didn't know it was going to be done automatically, I just did it and force-pushed.

It was recently implemented, and I forgot about it when I made my first comment

@jonringer jonringer changed the title treewide: cmake buildInputs to nativeBuildInputs, minor cleanups [staging] treewide: cmake buildInputs to nativeBuildInputs, minor cleanups Dec 31, 2020
@ofborg ofborg bot removed the 6.topic: python label Jan 1, 2021
@FRidh FRidh merged commit 3f08495 into NixOS:staging Jan 1, 2021
@siraben
Copy link
Member Author

siraben commented Jan 1, 2021

@FRidh does staging have different merge requirements? Will the regressions be fixed later?

@FRidh
Copy link
Member

FRidh commented Jan 1, 2021

@siraben I wanted to get this in to avoid merge conflicts. Regressions still need to be fixed though, preferably as soon as possible, but I would expect them to be few compared to the amount of changes made here already. Maybe I am wrong and I should not have merged this yet!

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

Successfully merging this pull request may close these issues.

7 participants