Bug 179154 - [WPE][GTK] Avoid duplicating code for dist and distcheck targets
Summary: [WPE][GTK] Avoid duplicating code for dist and distcheck targets
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WPE WebKit (show other bugs)
Version: Other
Hardware: PC Linux
: P2 Normal
Assignee: Michael Catanzaro
URL:
Keywords:
Depends on:
Blocks: 178894
  Show dependency tree / graph
 
Reported: 2017-11-01 21:02 PDT by Michael Catanzaro
Modified: 2017-11-15 12:07 PST (History)
7 users (show)

See Also:

Attachments
Patch (33.41 KB, patch)
2017-11-01 22:39 PDT, Michael Catanzaro 		no flags Details | Formatted Diff | Diff
Patch (33.44 KB, patch)
2017-11-01 22:40 PDT, Michael Catanzaro 		no flags Details | Formatted Diff | Diff
Patch (33.43 KB, patch)
2017-11-02 05:28 PDT, Michael Catanzaro 		no flags Details | Formatted Diff | Diff
Patch (33.43 KB, patch)
2017-11-02 05:28 PDT, Michael Catanzaro 		no flags Details | Formatted Diff | Diff
Show Obsolete (3) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Catanzaro 2017-11-01 21:02:43 PDT 
Avoid duplicating code for dist and distcheck targets between GTK and WPE
Comment 1 Michael Catanzaro 2017-11-01 22:39:26 PDT 
Created attachment 325682 [details]
Patch
Comment 2 Michael Catanzaro 2017-11-01 22:40:15 PDT 
Created attachment 325683 [details]
Patch
Comment 3 Carlos Garcia Campos 2017-11-01 23:35:24 PDT 
Comment on attachment 325683 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=325683&action=review

> Tools/Scripts/make-dist:1
> +#!/usr/bin/env python2

Why did you change this? Does this fail to run with python3?
Comment 4 Michael Catanzaro 2017-11-02 05:19:02 PDT 
(In reply to Carlos Garcia Campos from comment #3)
> Comment on attachment 325683 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=325683&action=review
> 
> > Tools/Scripts/make-dist:1
> > +#!/usr/bin/env python2
> 
> Why did you change this? Does this fail to run with python3?

I didn't try, but I assume all our scripts will fail to run with python3 unless tested otherwise. I can leave it as-is.
Comment 5 Michael Catanzaro 2017-11-02 05:19:40 PDT 
Comment on attachment 325683 [details]
Patch

Both EWS are broken
Comment 6 Michael Catanzaro 2017-11-02 05:28:09 PDT 
Created attachment 325710 [details]
Patch
Comment 7 Michael Catanzaro 2017-11-02 05:28:46 PDT 
Created attachment 325711 [details]
Patch
Comment 8 Carlos Alberto Lopez Perez 2017-11-02 06:32:30 PDT 
(In reply to Michael Catanzaro from comment #4)
> I didn't try, but I assume all our scripts will fail to run with python3
> unless tested otherwise. I can leave it as-is.

This is a change that can be done in another bug. You can for example seize and globally replace "python" with "python-2.7" in all python shebang (as we discussed a while ago on the webkit-dev mailing lists).

On Debian, the python namespace still defaults to 2.7
Comment 9 Michael Catanzaro 2017-11-02 07:38:50 PDT 
Comment on attachment 325711 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=325711&action=review

> Tools/Scripts/make-dist:1
> +#!/usr/bin/env python

I switched it back, since I agree it's a problem for another bug.
Comment 10 Adrian Perez 2017-11-02 13:35:26 PDT 
Comment on attachment 325711 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=325711&action=review

Informally reviewing: Looks good to me / r+.

> Source/cmake/WebKitDist.cmake:24
> +        VERBATIM

#TIL about “VERBATIM” — certainly nicer that interspersing double-quote
characters all around :-)
Comment 11 Michael Catanzaro 2017-11-02 13:42:01 PDT 
(In reply to Adrian Perez from comment #10) 
> #TIL about “VERBATIM” — certainly nicer that interspersing double-quote
> characters all around :-)

Yeah, Konstantin wants us to always use it for all custom targets and commands.
Comment 12 WebKit Commit Bot 2017-11-03 11:42:58 PDT 
Comment on attachment 325711 [details]
Patch

Clearing flags on attachment: 325711

Committed r224410: <https://trac.webkit.org/changeset/224410>
Comment 13 WebKit Commit Bot 2017-11-03 11:43:00 PDT 
All reviewed patches have been landed.  Closing bug.