Bug 211032

Summary: [Flatpak SDK] Misc flatpakutils.py fixes
Product: WebKit Reporter: Patrick Griffis <pgriffis>
Component: WebKitGTKAssignee: Patrick Griffis <pgriffis>
Status: RESOLVED FIXED    
Severity: Normal CC: aperez, bugs-noreply, lmoura, pnormand
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch none

Description Patrick Griffis 2020-04-25 15:11:05 PDT
Just a few fixes for issues I've hit.
Comment 1 Patrick Griffis 2020-04-25 15:12:10 PDT
Created attachment 397589 [details]
Patch
Comment 2 Patrick Griffis 2020-04-25 15:13:38 PDT
Created attachment 397590 [details]
Patch
Comment 3 Patrick Griffis 2020-04-25 15:35:46 PDT
Created attachment 397592 [details]
Patch
Comment 4 Patrick Griffis 2020-04-25 15:59:17 PDT
Created attachment 397594 [details]
Patch
Comment 5 Patrick Griffis 2020-04-26 11:48:45 PDT
Created attachment 397624 [details]
Patch
Comment 6 Philippe Normand 2020-04-27 02:20:25 PDT
ERROR: Tools/flatpak/flatpakutils.py:131:  [flatpak_check_output] Module 'subprocess' has no 'run' member  [pylint/E1101] [5]
Comment 7 Lauro Moura 2020-04-29 04:01:47 PDT
(In reply to Philippe Normand from comment #6)
> ERROR: Tools/flatpak/flatpakutils.py:131:  [flatpak_check_output] Module
> 'subprocess' has no 'run' member  [pylint/E1101] [5]

subprocess.run was added in Python 3.5, per https://docs.python.org/3/library/subprocess.html#subprocess.run

Use the old Popen instead?
Comment 8 Adrian Perez 2020-05-01 07:08:24 PDT
(In reply to Lauro Moura from comment #7)
> (In reply to Philippe Normand from comment #6)
> > ERROR: Tools/flatpak/flatpakutils.py:131:  [flatpak_check_output] Module
> > 'subprocess' has no 'run' member  [pylint/E1101] [5]
> 
> subprocess.run was added in Python 3.5, per
> https://docs.python.org/3/library/subprocess.html#subprocess.run
> 
> Use the old Popen instead?

Do we want to support Python 3 versions older than 3.5? I say no because:

 - Even Debian's “oldstable” has 3.5
 - 3.5 is considered one of the first Python 3 versions which are
   actually really usable (all is rosy with 3.6 or newer).
 - Support for 3.5 will end on 2020-09-13

Instead, I would figure out the issue with Pylint and keep using
“subprocess.run()”.
Comment 9 Lauro Moura 2020-05-01 08:04:34 PDT
(In reply to Adrian Perez from comment #8)
> (In reply to Lauro Moura from comment #7)
> > (In reply to Philippe Normand from comment #6)
> > > ERROR: Tools/flatpak/flatpakutils.py:131:  [flatpak_check_output] Module
> > > 'subprocess' has no 'run' member  [pylint/E1101] [5]
> > 
> > subprocess.run was added in Python 3.5, per
> > https://docs.python.org/3/library/subprocess.html#subprocess.run
> > 
> > Use the old Popen instead?
> 
> Do we want to support Python 3 versions older than 3.5? I say no because:
> 
>  - Even Debian's “oldstable” has 3.5
>  - 3.5 is considered one of the first Python 3 versions which are
>    actually really usable (all is rosy with 3.6 or newer).
>  - Support for 3.5 will end on 2020-09-13
> 
> Instead, I would figure out the issue with Pylint and keep using
> “subprocess.run()”.

The issue would not be with Python3 itself, but with Python2. Don't we still need to support both versions in webkitpy?

(subprocess.run is not available in python2: https://docs.python.org/2/library/subprocess.html )
Comment 10 Philippe Normand 2020-05-01 08:38:14 PDT
Right, webkitpy is not yet fully ported to Python3.
Comment 11 Patrick Griffis 2020-05-02 17:46:09 PDT
Created attachment 398300 [details]
Patch
Comment 12 Philippe Normand 2020-05-03 01:29:38 PDT
Comment on attachment 398300 [details]
Patch

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

> Tools/flatpak/flatpakutils.py:845
> +            icc_version_filename, = re.findall(rb'.*creating (.*)', tmpfile.read())

Breaks EWS:

Traceback (most recent call last):
  File "Tools/Scripts/update-webkit-flatpak", line 25, in <module>
    from flatpakutils import WebkitFlatpak
  File "./Tools/flatpak/flatpakutils.py", line 851
    icc_version_filename, = re.findall(rb'.*creating (.*)', tmpfile.read())
                                                         ^
SyntaxError: invalid syntax
Comment 13 Patrick Griffis 2020-05-03 13:34:18 PDT
Created attachment 398328 [details]
Patch
Comment 14 EWS 2020-05-04 00:07:37 PDT
Committed r261072: <https://trac.webkit.org/changeset/261072>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 398328 [details].