Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1530713006:
Don't use pulse/alsa in embedded config (Closed)
DescriptionDon't use pulse/alsa in embedded config
The embedded check was removed in CL
https://codereview.chromium.org/1528533002 which broke our builds.
BUG=
Committed: https://crrev.com/45266a85b3ea49d9e9e942e52b239677f042c6e9
Cr-Commit-Position: refs/heads/master@{#368304}
Patch Set 1 #Patch Set 2 : Readd embedded for midi.gyp as well #
Messages
Total messages: 22 (9 generated)
Description was changed from ========== Don't use pulse/alsa in embedded config The embedded check was removed in CL https://codereview.chromium.org/1528533002 which broke our builds. BUG= ========== to ========== Don't use pulse/alsa in embedded config The embedded check was removed in CL https://codereview.chromium.org/1528533002 which broke our builds. BUG= ==========
landell@opera.com changed reviewers: + dalecurtis@chromium.org, sandersd@chromium.org
@reviewers: Don't know why the embedded check was removed but it's needed for patforms that don't have alsa/pulseadio that are not chromecast.
Ah, spang@ was the previous owner of this var, so I assumed he had reviewed this change and was happy. lgtm
slan@chromium.org changed reviewers: + slan@chromium.org
lgtm My apologies for the breakage - I have been operating under the assumption for a long time that chromecast builds were the only user of 'embedded'. For future reference, what other types of build rely on this flag? Please be aware that the 'embedded' flag has not been ported to GN. Do you plan to rely on GN support for these builds in the future?
You will also want to make the same change here: https://code.google.com/p/chromium/codesearch#chromium/src/media/midi/midi.gy...
On 2015/12/17 18:51:06, slan wrote: > lgtm > > My apologies for the breakage - I have been operating under the assumption for a > long time that chromecast builds were the only user of 'embedded'. For future > reference, what other types of build rely on this flag? We're working on a downstream product, which targets embedded linux (mostly for TV/STB devices). > Please be aware that the 'embedded' flag has not been ported to GN. Do you plan > to rely on GN support for these builds in the future? We are migrating to GN of course, because eventually gyp will go away. We probably currently depend on the embedded variable in the gyp build in a few places, but we would prefer to use lower-level feature variables rather than a meta-variable like embedded. ie as long as GN variables like use_alsa remain arguments, we will be happy.
On 2015/12/17 19:02:00, slan wrote: > You will also want to make the same change here: > https://code.google.com/p/chromium/codesearch#chromium/src/media/midi/midi.gy... Done
lgtm
The CQ bit was checked by landell@opera.com
The patchset sent to the CQ was uploaded after l-g-t-m from dalecurtis@chromium.org Link to the patchset: https://codereview.chromium.org/1530713006/#ps20001 (title: "Readd embedded for midi.gyp as well")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1530713006/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1530713006/20001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_android_rel_ng on tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
The CQ bit was checked by landell@opera.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1530713006/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1530713006/20001
Message was sent while issue was closed.
Description was changed from ========== Don't use pulse/alsa in embedded config The embedded check was removed in CL https://codereview.chromium.org/1528533002 which broke our builds. BUG= ========== to ========== Don't use pulse/alsa in embedded config The embedded check was removed in CL https://codereview.chromium.org/1528533002 which broke our builds. BUG= ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== Don't use pulse/alsa in embedded config The embedded check was removed in CL https://codereview.chromium.org/1528533002 which broke our builds. BUG= ========== to ========== Don't use pulse/alsa in embedded config The embedded check was removed in CL https://codereview.chromium.org/1528533002 which broke our builds. BUG= Committed: https://crrev.com/45266a85b3ea49d9e9e942e52b239677f042c6e9 Cr-Commit-Position: refs/heads/master@{#368304} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/45266a85b3ea49d9e9e942e52b239677f042c6e9 Cr-Commit-Position: refs/heads/master@{#368304} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
