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.

Sign up for GitHub

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

Jump to bottom

libdrm: build with meson #57773

Merged
merged 1 commit into from Jun 26, 2019
Merged

libdrm: build with meson #57773

merged 1 commit into from Jun 26, 2019

Conversation

colemickens
Copy link
Member

@colemickens colemickens commented Mar 17, 2019 •
edited

Motivation for this change

I'm testing this jointly with #56199 and assume it might be staged with it?

I'm doing this as the next libdrm release seemed to require it, similar to the next release of mesa.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • 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 nox --run "nox-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)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

colemickens
colemickens commented Mar 17, 2019
View reviewed changes
pkgs/development/libraries/libdrm/default.nix
++ stdenv.lib.optionals (stdenv.isAarch32 || stdenv.isAarch64)
[ "--enable-tegra-experimental-api" "--enable-etnaviv-experimental-api" ]
++ stdenv.lib.optional stdenv.isDarwin "-C"
Copy link
Member Author

Choose a reason for hiding this comment

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

please review this deletion. I'm not sure what it was for, or what it's meson equivalent is.

Copy link
Member

Choose a reason for hiding this comment

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

It can be dropped as well, it's asking configure to use the cache ( https://www.gnu.org/software/autoconf/manual/autoconf-2.65/html_node/Cache-Files.html ), presumably to pick up the value put there in preConfigure previously.

Copy link
Member

Choose a reason for hiding this comment

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

Not sure that the clock_gettime workaround is still useful (meson or no), may need some testing or investigation on that.

@colemickens colemickens changed the base branch from master to staging March 17, 2019 01:40
@colemickens colemickens changed the base branch from staging to master March 17, 2019 01:40
@colemickens
Copy link
Member Author

cc: @vcunat @dtzWill @eadwu

dtzWill
dtzWill requested changes Mar 17, 2019
View reviewed changes
Copy link
Member

@dtzWill dtzWill left a comment

Choose a reason for hiding this comment

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

Thanks! Agreed we'll need to move this to meson "soon" anyway.

pkgs/development/libraries/libdrm/default.nix Outdated

src = fetchgit {
url = "https://gitlab.freedesktop.org/mesa/drm";
inherit rev sha256;
Copy link
Member

Choose a reason for hiding this comment

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

Why the change of source?

Copy link
Member Author

Choose a reason for hiding this comment

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

It makes it easier to point at git revisions, but maybe not appropriate for this PR. I'll revert.

pkgs/development/libraries/libdrm/default.nix Outdated
@@ -25,13 +28,15 @@ stdenv.mkDerivation rec {
preConfigure = stdenv.lib.optionalString stdenv.isDarwin
"echo : \\\${ac_cv_func_clock_gettime=\'yes\'} > config.cache";
Copy link
Member

Choose a reason for hiding this comment

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

This should be removed-- if we're using meson it does nothing :).

pkgs/development/libraries/libdrm/default.nix Outdated
@@ -25,13 +28,15 @@ stdenv.mkDerivation rec {
preConfigure = stdenv.lib.optionalString stdenv.isDarwin
"echo : \\\${ac_cv_func_clock_gettime=\'yes\'} > config.cache";

configureFlags = [ "--enable-install-test-programs" ]
mesonFlags =
[ "-Dinstall-test-programs=true" ]
++ stdenv.lib.optionals (stdenv.isAarch32 || stdenv.isAarch64)
[ "--enable-tegra-experimental-api" "--enable-etnaviv-experimental-api" ]
Copy link
Member

Choose a reason for hiding this comment

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

"-Dtegra=true" "-Detnaviv=true" , at least that's what I came up with :).

pkgs/development/libraries/libdrm/default.nix
++ stdenv.lib.optionals (stdenv.isAarch32 || stdenv.isAarch64)
[ "--enable-tegra-experimental-api" "--enable-etnaviv-experimental-api" ]
++ stdenv.lib.optional stdenv.isDarwin "-C"
Copy link
Member

Choose a reason for hiding this comment

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

It can be dropped as well, it's asking configure to use the cache ( https://www.gnu.org/software/autoconf/manual/autoconf-2.65/html_node/Cache-Files.html ), presumably to pick up the value put there in preConfigure previously.

pkgs/development/libraries/libdrm/default.nix
++ stdenv.lib.optionals (stdenv.isAarch32 || stdenv.isAarch64)
[ "--enable-tegra-experimental-api" "--enable-etnaviv-experimental-api" ]
++ stdenv.lib.optional stdenv.isDarwin "-C"
Copy link
Member

Choose a reason for hiding this comment

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

Not sure that the clock_gettime workaround is still useful (meson or no), may need some testing or investigation on that.

@dtzWill
Copy link
Member

dtzWill commented Mar 17, 2019

Also, and maybe this is why I was asked to review ;), previously: #48218 . Been using it ever since, so far so good ^_^.

@colemickens
Copy link
Member Author

@dtzWill Thank you for the review. I actually hadn't seen that PR -- please let me know if you'd prefer to reopen your PR and drive it yourself. Otherwise, I'm happy to keep pushing this with your guidance.

@dtzWill
Copy link
Member

dtzWill commented Mar 18, 2019 via email

@colemickens colemickens force-pushed the libdrm-meson branch 2 times, most recently from 41a49f5 to e7408ca Compare March 22, 2019 01:28
@colemickens
Copy link
Member Author

I've updated this per the requested changes. Note, I don't have Darwin devices, so I can't test anything this affects in Darwin.

@FRidh FRidh requested a review from dtzWill April 16, 2019 17:05
@colemickens
Copy link
Member Author

Rebased, pushed. 19.1's release is coming very soon. Shall we try to get this and the mesa/meson change pushed?

@colemickens colemickens mentioned this pull request Jun 13, 2019
Merged
10 tasks
@colemickens colemickens changed the base branch from master to staging June 26, 2019 21:44
@colemickens
Copy link
Member Author

Rebased on and re-pointed at staging. cc: @grahamc

@grahamc grahamc merged commit 2b248d5 into NixOS:staging Jun 26, 2019
@nh2 nh2 mentioned this pull request Aug 10, 2019
Closed
@nh2
Copy link
Contributor

nh2 commented Aug 10, 2019

This broke the build with musl:

#66441

@colemickens colemickens deleted the libdrm-meson branch January 30, 2020 09:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dtzWill dtzWill Awaiting requested review from dtzWill

Assignees
No one assigned
Labels
10.rebuild-darwin: 11-100 10.rebuild-linux: 501+ 10.rebuild-linux: 5001+
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@colemickens @dtzWill @nh2 @grahamc @GrahamcOfBorg