Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(1847)

Issue 29527808: Noissue - Use meson to build the C++ code

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 month, 3 weeks ago by hub
Modified:
8 hours, 44 minutes ago
CC:
Sebastian Noack
Base URL:
https://hg.adblockplus.org/adblockpluscore/
Visibility:
Public.

Description

Noissue - Use meson to build the C++ code

Patch Set 1 #

Patch Set 2 : npm test now works #

Patch Set 3 : Rebased. Now compile output to the source directory #

Total comments: 15

Patch Set 4 : Reworked. Now build source files one by one. #

Total comments: 16

Patch Set 5 : Now build objects one by one, with dependencies, and share them. #

Total comments: 13

Patch Set 6 : Last bits of feedback #

Total comments: 9
Unified diffs Side-by-side diffs Delta from patch set Stats (+187 lines, -8 lines) Patch
M .gitignore View 1 2 1 chunk +1 line, -2 lines 0 comments Download
M .hgignore View 1 2 1 chunk +2 lines, -3 lines 0 comments Download
M README.md View 1 2 3 4 5 1 chunk +16 lines, -3 lines 4 comments Download
A meson.build View 1 2 3 4 5 1 chunk +166 lines, -0 lines 5 comments Download
A meson_options.txt View 1 2 3 1 chunk +2 lines, -0 lines 0 comments Download

Messages

Total messages: 18
hub
1 month, 3 weeks ago (2017-08-25 17:17:51 UTC) #1
hub
not really for a review but more for a "here is what I came up ...
1 month, 3 weeks ago (2017-08-25 17:24:29 UTC) #2
hub
I believe Eric's CMake build system it better. meson has serious limitations like the impossibility ...
1 week, 3 days ago (2017-10-10 20:18:28 UTC) #3
Wladimir Palant
I actually find this approach nicer than CMake, Meson build files seem to be more ...
1 week, 2 days ago (2017-10-11 10:00:53 UTC) #4
hub
With all the suggestions, this looks better. https://codereview.adblockplus.org/29527808/diff/29532724/meson.build File meson.build (right): https://codereview.adblockplus.org/29527808/diff/29532724/meson.build#newcode1 meson.build:1: project('adblockpluscore', license: ...
1 week, 2 days ago (2017-10-11 18:23:17 UTC) #5
Wladimir Palant
I wonder whether creating a tiny Python wrapper around the meson command would be worth ...
1 week, 2 days ago (2017-10-11 21:01:24 UTC) #6
Wladimir Palant
Alternatively, we could have a script generate a cross-compiling configuration that could then be passed ...
1 week, 2 days ago (2017-10-11 21:03:46 UTC) #7
Wladimir Palant
https://codereview.adblockplus.org/29527808/diff/29573838/README.md File README.md (right): https://codereview.adblockplus.org/29527808/diff/29573838/README.md#newcode30 README.md:30: mkdir build This step is unnecessary, meson will create ...
1 week, 1 day ago (2017-10-12 08:46:09 UTC) #8
hub
https://codereview.adblockplus.org/29527808/diff/29573838/README.md File README.md (right): https://codereview.adblockplus.org/29527808/diff/29573838/README.md#newcode30 README.md:30: mkdir build On 2017/10/12 08:46:09, Wladimir Palant wrote: > ...
1 week, 1 day ago (2017-10-12 18:12:11 UTC) #9
hub
I am much more optimistic about using meson now - still quirky but it actually ...
1 week, 1 day ago (2017-10-12 19:50:07 UTC) #10
Wladimir Palant
A few minor issues but altogether this is looking good. https://codereview.adblockplus.org/29527808/diff/29574659/README.md File README.md (right): https://codereview.adblockplus.org/29527808/diff/29574659/README.md#newcode33 ...
1 week ago (2017-10-13 09:45:40 UTC) #11
hub
https://codereview.adblockplus.org/29527808/diff/29574659/README.md File README.md (right): https://codereview.adblockplus.org/29527808/diff/29574659/README.md#newcode33 README.md:33: make a release build. On 2017/10/13 09:45:39, Wladimir Palant ...
1 week ago (2017-10-13 18:16:54 UTC) #12
Wladimir Palant
LGTM As far as I am concerned, this can land. However, I'd like to hear ...
1 week ago (2017-10-13 18:54:06 UTC) #13
sergei
On 2017/10/11 21:01:24, Wladimir Palant wrote: > I wonder whether creating a tiny Python wrapper ...
4 days, 13 hours ago (2017-10-16 15:27:13 UTC) #14
Eric
On 2017/10/11 10:00:53, Wladimir Palant wrote: > Also, initial CMake call > requiring a special ...
13 hours, 33 minutes ago (2017-10-20 15:20:04 UTC) #15
Eric
On 2017/10/13 18:54:06, Wladimir Palant wrote: > However, I'd like to hear other > people's ...
11 hours, 58 minutes ago (2017-10-20 16:54:39 UTC) #16
sergei
On 2017/10/20 16:54:39, Eric wrote: > On 2017/10/13 18:54:06, Wladimir Palant wrote: > > However, ...
9 hours, 16 minutes ago (2017-10-20 19:36:24 UTC) #17
Wladimir Palant
8 hours, 44 minutes ago (2017-10-20 20:08:14 UTC) #18
On 2017/10/20 15:20:04, Eric wrote:
> This reflects a misunderstanding of how CMake operates. CMake and meson are
> essentially the same here. The first call to CMake, just as with meson,
creates
> build-files for a build-specific tool.

No misunderstanding here, it's merely more confusing with CMake - one has to
understand that the two calls to CMake are completely different.

> The present meson script reads directly from the configuration file. It
> would be possible to have CMake work analogously.

Yes, I understand that. But so far the main advantage of CMake appears to be the
fact that Emscripten provides a toolchain for it. If it isn't really usable (or
overly complicated to use) then this advantage is void.

But Sergei is right of course, this isn't a discussion for a review.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 87257f5