Bug 168036

Summary: [GTK] ImageDiff should be run by jhbuild-wrapper in case of using jhbuild
Product: WebKit Reporter: Fujii Hironori <Hironori.Fujii>
Component: Tools / TestsAssignee: Carlos Alberto Lopez Perez <clopez>
Status: RESOLVED FIXED    
Severity: Normal CC: bugs-noreply, buildbot, clopez, glenn, lforschler, mcatanzaro, rego, svillar
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=171268
Attachments:
Description Flags
Patch none

Description Fujii Hironori 2017-02-08 21:13:02 PST
[GTK][EFL] nrwt: ImageDiff should be run by jhbuild-wrapper in case of using jhbuild
Comment 1 Fujii Hironori 2017-02-08 21:28:12 PST
After installing EFL libs in my system, it works.

> sudo apt-get install libevas1 libecore1 libecore-evas1

Closed as INVALID.
Comment 2 Sergio Villar Senin 2017-03-23 08:55:58 PDT
I'm reopening this because I think the report is correct. I do also think that we should use jhbuild-wrapper because, among other things, ImageDiff links against a lot of things we build inside our jhbuild: cairo, libicu, gtk, pango, cairo...

It has been working because I did also have libicu55 in my system, but as soon as Debian unstable moved to libicu57 the problem arose.

We want ImageDiff to also use the same versions of cairo,gtk,pango... than webkit in order to provide a reliable testing platform.
Comment 3 Michael Catanzaro 2017-04-24 18:41:57 PDT
*** Bug 171255 has been marked as a duplicate of this bug. ***
Comment 4 Carlos Alberto Lopez Perez 2017-04-24 19:56:45 PDT
Created attachment 308050 [details]
Patch
Comment 5 Michael Catanzaro 2017-04-24 20:18:57 PDT
Comment on attachment 308050 [details]
Patch

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

Excellent! Nice tests.

> Tools/Scripts/webkitpy/port/base_unittest.py:402
> +        port._filesystem.maybe_make_directory(port.path_from_webkit_base('WebKitBuild', 'Dependencies%s' % port.port_name.upper()))

Nit: is there a non-maybe version of this function? If not, you should add one. It doesn't make sense to say "maybe" here when we want to assert that the path definitely does not already exist.

> Tools/Scripts/webkitpy/port/port_testcase.py:274
> +        port._filesystem.maybe_make_directory(port.path_from_webkit_base('WebKitBuild', 'Dependencies%s' % port.port_name.upper()))

Ditto.
Comment 6 Carlos Alberto Lopez Perez 2017-04-25 03:46:06 PDT
(In reply to Michael Catanzaro from comment #5)
> Comment on attachment 308050 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=308050&action=review
> 
> Excellent! Nice tests.
> 

Thanks for the review.

> > Tools/Scripts/webkitpy/port/base_unittest.py:402
> > +        port._filesystem.maybe_make_directory(port.path_from_webkit_base('WebKitBuild', 'Dependencies%s' % port.port_name.upper()))
> 
> Nit: is there a non-maybe version of this function? If not, you should add
> one. It doesn't make sense to say "maybe" here when we want to assert that
> the path definitely does not already exist.
> 

No, there isn't anyone.

Also the function will always create the virtual directory.

I guess it was named "maybe", because the function only creates it if its not already created.  In this sense I think it behaves exactly like "mkdir -p".

I also think is a bad name.

But changing that name is out of the scope of this patch. I have opened bug 171268 for requesting that rename to be done, and I'll land this patch as is.
Comment 7 Carlos Alberto Lopez Perez 2017-04-25 03:48:37 PDT
Comment on attachment 308050 [details]
Patch

Clearing flags on attachment: 308050

Committed r215727: <http://trac.webkit.org/changeset/215727>
Comment 8 Carlos Alberto Lopez Perez 2017-04-25 03:48:41 PDT
All reviewed patches have been landed.  Closing bug.
Comment 9 Carlos Alberto Lopez Perez 2017-04-25 09:10:11 PDT
Committed r215735: <http://trac.webkit.org/changeset/215735>