8000 refactor: do not use electron.gyp to verify ffmpeg by alexeykuzmin · Pull Request #14405 · electron/electron · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

refactor: do not use electron.gyp to verify ffmpeg #14405

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 3 commits into from
Sep 4, 2018
Merged

refactor: do not use electron.gyp to verify ffmpeg #14405

merged 3 commits into from
Sep 4, 2018

Conversation

alexeykuzmin
Copy link
Contributor
@alexeykuzmin alexeykuzmin commented Aug 31, 2018 •
edited
Loading
Description of Change

We use GN now.

Checklist
  • PR description included and stakeholders cc'd
  • npm test passes
  • PR title follows semantic commit guidelines
Release Notes

Notes: no notes

@alexeykuzmin alexeykuzmin requested review from a team and jkleinsc August 31, 2018 11:53
@alexeykuzmin alexeykuzmin changed the title [wip] refactor: do not use electron gyp for verify ffmpeg [wip] refactor: do not use electron.gyp to verify ffmpeg Aug 31, 2018
jkleinsc
jkleinsc requested changes Aug 31, 2018
View reviewed changes
Copy link
Member
@jkleinsc jkleinsc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall LGTM, just need some small tweaks including updating the CI config files.

@nornagon nornagon mentioned this pull request Aug 31, 2018
Merged
@alexeykuzmin alexeykuzmin force-pushed the do-not-use-electron-gyp-for-verify-ffmpeg branch from c673da2 to dd78837 Compare August 31, 2018 17:29
@alexeykuzmin
Copy link
Contributor 8000 Author
alexeykuzmin commented Sep 1, 2018 •
edited
Loading

@jkleinsc I fixed the configs.
But verify-ffmpeg fails on Windows trying to run gn from the Python script. Any idea why it can happen?

UPD I've already fixed it, never mind =)

Verifying non proprietary ffmpeg 
verify-ffmpeg: args.source_root = C:\projects\src
verify-ffmpeg: initial_app_path = C:\projects\src\out\Default
verify-ffmpeg: app_path = C:\projects\src\out\Default-no-proprietary-codecs
gn.run: out_dir = C:\projects\src\out\Default
gn.run: complete_args = gn, args, ., --list=electron_product_name, --short
Traceback (most recent call last):
  File "electron\script\verify-ffmpeg.py", line 95, in <module>
    sys.exit(main()) 
  File "electron\script\verify-ffmpeg.py", line 27, in main
    product_name = gn(initial_app_path).args().get_string('electron_product_name')
  File "C:\projects\src\electron\script\lib\gn.py", line 43, in get_string
    raw_value = self._get_raw_value(name)
  File "C:\projects\src\electron\script\lib\gn.py", line 30, in _get_raw_value
    ['--list={}'.format(name), '--short'])
  File "C:\projects\src\electron\script\lib\gn.py", line 17, in run
    return subprocess.check_output(complete_args)
  File "C:\Python27\lib\subprocess.py", line 212, in check_output
    process = Popen(stdout=PIPE, *popenargs, **kwargs)
  File "C:\Python27\lib\subprocess.py", line 390, in __init__
    errread, errwrite)
  File "C:\Python27\lib\subprocess.py", line 640, in _execute_child
    startupinfo)
WindowsError: [Error 2] The system cannot find the file specified
Command exited with code 1

@alexeykuzmin alexeykuzmin force-pushed the do-not-use-electron-gyp-for-verify-ffmpeg branch 2 times, most recently from b22c64f to 65f0913 Compare September 3, 2018 11:24
@alexeykuzmin
build: add a helper script to retreive data from a GN project
64542a6
@alexeykuzmin
refactor: do not use electron.gyp contents in the verify-ffmpeg script
b5408db 
Also run verify ffmpeg with cmd instead of powershell
@alexeykuzmin
ci: pass GN_EXTRA_ARGS to the ffmpeg build
397e9a6 
Both Electron and ffmpeg should have
the same value of the `target_cpu` build flag.
@alexeykuzmin alexeykuzmin force-pushed the do-not-use-electron-gyp-for-verify-ffmpeg branch from 65f0913 to 397e9a6 Compare September 4, 2018 11:00
@jkleinsc jkleinsc changed the title [wip] refactor: do not use electron.gyp to verify ffmpeg refactor: do not use electron.gyp to verify ffmpeg Sep 4, 2018
@jkleinsc jkleinsc merged commit 308316a into delete-gyp Sep 4, 2018
@release-clerk
Copy link
release-clerk bot commented Sep 4, 2018

Release Notes Persisted

no notes

@jkleinsc jkleinsc deleted the do-not-use-electron-gyp-for-verify-ffmpeg branch September 4, 2018 14:31
@alexeykuzmin alexeykuzmin mentioned this pull request Sep 9, 2018
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jkleinsc jkleinsc jkleinsc approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@alexeykuzmin @jkleinsc
0