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

Meson improvements #9

Merged
merged 6 commits into from
Jul 26, 2021
Merged

Conversation

Ericson2314
Copy link

@Ericson2314 Ericson2314 commented Jul 25, 2021

See each commit for details. I took this from my old branches.

I think this is strictly better except for one issue: by default (with stdenv.mkDerivation and Meson) we enable all optional features, and thus enable the s3 store. But the AWS pkg-config file is very bad in specifying -fno-exceptionsandstd=c++11`, and this breaks the build.

That means the build with AWS was always failing, but now with AWS is the effective default and thus the CI build will fail.

CC @Pamplemousse

N.B. Perhaps we should reframe from rebasing the PR while multiple people are working on it, and we are not sure if/when @edolstra is interested, as it was somewhat difficult to dredge up these changes.

auto options are better for allowing autodetect, in addition to on and
off.
This helps avoid merge conflicts, since git is line-oriented.
The whole point of the `files` function is that it remembers the current
directory. Otherwise we could just use plain lists.
Copy link
Owner

@p01arst0rm p01arst0rm left a comment

Choose a reason for hiding this comment

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

personally, i think meson looks a lot neater when you dont newline the closers :')

Copy link
Owner

@p01arst0rm p01arst0rm left a comment

Choose a reason for hiding this comment

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

im pretty sure files were added like this for a reason; albeit i cant remember that reason now! i think it was something to do with a bug in referencing file objects in different directories

Copy link
Owner

@p01arst0rm p01arst0rm left a comment

Choose a reason for hiding this comment

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

this file looks excessively messy. Could you give it a do-over maybe? improve formatting n such

@p01arst0rm
Copy link
Owner

@Ericson2314 there is mixed usage of with_[option] and enable_[option], could you standardise these all to the same syntax?

@Pamplemousse
Copy link

personally, i think meson looks a lot neater when you dont newline the closers :')

I strongly disagree:

  • it makes it harder to read (quickly scan with eyes where a list/dict ends)
  • it makes VCS logs confusing whenever something is added / removed at the end of the list (more line updates than necessary)

@Pamplemousse
Copy link

LGTM: This seems to be a good improvement of the original PR 👍

@p01arst0rm
Copy link
Owner

not yet, still need to standardise the with/enable flags

@p01arst0rm
Copy link
Owner

guess i could just do that myself :)

@p01arst0rm p01arst0rm merged commit b2c9b76 into p01arst0rm:meson Jul 26, 2021
@Ericson2314 Ericson2314 deleted the meson-improvements branch July 26, 2021 16:52
@Ericson2314
Copy link
Author

Thanks!

@Ericson2314
Copy link
Author

Thanks!!

Ericson2314 added a commit that referenced this pull request Apr 25, 2024
Add a GC test, fix hardlinking issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants