WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
139387
[GTK] Generate the make dist manifest from a CMake template file
https://bugs.webkit.org/show_bug.cgi?id=139387
Summary
[GTK] Generate the make dist manifest from a CMake template file
Michael Catanzaro
Reported
2014-12-08 01:49:16 PST
If we generate the install manifest from a CMake template file, we can use CMake variables in the manifest. This would be useful for
bug #139369
.
Attachments
Patch
(9.72 KB, patch)
2014-12-08 02:15 PST
,
Michael Catanzaro
no flags
Details
Formatted Diff
Diff
Patch
(14.02 KB, patch)
2015-01-14 15:01 PST
,
Michael Catanzaro
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Carlos Garcia Campos
Comment 1
2014-12-08 01:53:14 PST
Our manifest is not for installation, but to decide what files to include in the tarball for releasing.
Michael Catanzaro
Comment 2
2014-12-08 02:15:49 PST
Created
attachment 242792
[details]
Patch
Michael Catanzaro
Comment 3
2014-12-17 07:08:12 PST
(In reply to
comment #1
)
> Our manifest is not for installation, but to decide what files to include in > the tarball for releasing.
Yes, but I need more control over what files are included in the tarball for releasing to fix
bug #139369
.
Carlos Garcia Campos
Comment 4
2015-01-13 00:28:16 PST
The thing is that we already have a custom variable substitution mechanism for the manifest. I think we should either add the abi-version variable to the existing mechanism, or remove the custom code an use CMake for the other variables. Martin, you wrote that code, what do you think?
Michael Catanzaro
Comment 5
2015-01-13 06:58:18 PST
(In reply to
comment #4
)
> The thing is that we already have a custom variable substitution mechanism > for the manifest. I think we should either add the abi-version variable to > the existing mechanism, or remove the custom code an use CMake for the other > variables. Martin, you wrote that code, what do you think?
OK. make-dist.py is passed the right sourcedir and builddir when it is called, which it needs anyway, so it also incidentally replaces $source and $build in the manifest. For it to be able to replace other variables, values would need to be passed as arguments to the script, which doesn't seem like a good approach. $source is never used in the manifest, and $build is only used on the two lines that I modify in
bug #139369
. So I would generally prefer to replace $build with ${CMAKE_BINARY_DIR} on those two lines and remove the code in make-dist.py that expands $build and $source. But, on the other hand, I see that our API version is hardcoded in make-dist.py. There is no value in using a CMake variable for it in the manifest but not in make-dist.py, so we should add an argument to the script for it regardless (or just hardcode it in both places, but let's be better than that). This would obviate the need to use CMake variables in the manifest (at least for now, but I can't see why else we would need it in the future).
Martin Robinson
Comment 6
2015-01-13 10:09:01 PST
(In reply to
comment #4
)
> The thing is that we already have a custom variable substitution mechanism > for the manifest. I think we should either add the abi-version variable to > the existing mechanism, or remove the custom code an use CMake for the other > variables. Martin, you wrote that code, what do you think?
It does seem that using the CMake preprocessor could simplify the script quite a bit. I think this patch should also rip out the variable replacement code in the script and use the CMake preprocessor for everything though.
Michael Catanzaro
Comment 7
2015-01-14 11:37:59 PST
Like I said in
comment #5
, I no longer see value in using a CMake template for the manifest so long as we still hardcode our API version in make-dist.py. We should either pass the API version as an argument to make-dist.py (in which case there is no point in using a CMake template for the manifest), or else make make-dist.py itself a CMake template. I don't see any significant advantages or disadvantages to either approach; using CMake templates makes the make-dist script shorter, which I prefer, so I will upload a patch for that, but it also makes the script less flexible in case you want to play with calling the script manually (as I have removed the srcdir and builddir arguments), though I don't see why you would want to do that.
Martin Robinson
Comment 8
2015-01-14 11:41:07 PST
(In reply to
comment #7
)
> Like I said in
comment #5
, I no longer see value in using a CMake template > for the manifest so long as we still hardcode our API version in > make-dist.py. We should either pass the API version as an argument to > make-dist.py (in which case there is no point in using a CMake template for > the manifest), or else make make-dist.py itself a CMake template. I don't > see any significant advantages or disadvantages to either approach; using > CMake templates makes the make-dist script shorter, which I prefer, so I > will upload a patch for that, but it also makes the script less flexible in > case you want to play with calling the script manually (as I have removed > the srcdir and builddir arguments), though I don't see why you would want to > do that.
It should be possible to get rid of the hard-coded version by simply looking for the latest file with the pattern webkit2gtk-*.pc.
Michael Catanzaro
Comment 9
2015-01-14 14:25:51 PST
(In reply to
comment #8
)
> It should be possible to get rid of the hard-coded version by simply looking > for the latest file with the pattern webkit2gtk-*.pc.
That's not really any better than just passing it as an argument to the script. I'll do that in a separate bug since it's not necessary here. I do have a patch to turn make-dist.py into a CMake template, but it's larger than I expected and I don't really like it, so I will just turn the manifest into a template and remove the variable replacement code from make-dist.py (not very much code) like you requested.
Michael Catanzaro
Comment 10
2015-01-14 15:01:03 PST
Created
attachment 244647
[details]
Patch
Michael Catanzaro
Comment 11
2015-01-14 15:10:34 PST
(In reply to
comment #9
)
> That's not really any better than just passing it as an argument to the > script. I'll do that in a separate bug since it's not necessary here.
Actually, I'll leave it be and get back to real work, because the API version is only used in the script to call pkg-config to come up with a default version to use if --version is not passed to the script (which is not how we use it).
Martin Robinson
Comment 12
2015-01-19 13:15:12 PST
Comment on
attachment 244647
[details]
Patch Okay. Thanks!
WebKit Commit Bot
Comment 13
2015-01-19 13:58:02 PST
Comment on
attachment 244647
[details]
Patch Clearing flags on attachment: 244647 Committed
r178672
: <
http://trac.webkit.org/changeset/178672
>
WebKit Commit Bot
Comment 14
2015-01-19 13:58:08 PST
All reviewed patches have been landed. Closing bug.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug