electron icon indicating copy to clipboard operation
electron copied to clipboard

build: extract ninja cflags_cc from electron_app.ninja in spec runners

Open ckerr opened this issue 3 years ago • 1 comments

Discussed at https://github.com/electron/electron/blob/d67532ee9f836857bc43989fbb8376d1fe769a48/script/nan-spec-runner.js#L34-L53 but not implemented. Would have prevented the need for https://github.com/electron/electron/pull/33906/commits/41a03a5799a8f40f31555d73d20ea865acfcd192.

Most importantly, cxxflags -std= and -nostdinc++ are needed.

It's possible that the includes and isystem also need to be pulled in if it's necessary for the chromium/v8 and the 3rd party modules to be built with the same system system headers

ckerr avatar May 12 '22 14:05 ckerr

I'm not sure how to proceed with this ticket:

The original scope was to sync cflags between the Electron build and the specs. In that situation where we know the build sandbox is available, we could do something like gn desc /path/to/src/out //electron:electron_app cflags_cc to pull out the build cflags.

But it looks like there's also the question of syncing it between Electron and electron-rebuild as discussed at https://github.com/electron/electron-rebuild/pull/1022 which maybe makes things a little more complicated? I'm not aware of a good way to shuttle these cflags (e.g. --std=c++17 vs --std=c++14) from Electron to electron-rebuild so that they'll be in sync.

Options:

  • Maybe someone has an idea of how to cleanly sync between electron/electron and electron/electron-rebuild. (@MarshallOfSound? @nornagon?)

  • If all else fails, we could hardcode everything to --std=c++17. V8 FTBFS without that flag, so it's a hard requirement for E20. That would unblock this issue for E20 (it's marked as blocks-stable) and would clear us until Chromium starts using `--std=c++20 sometime in 2025 :smile:, but it's not as satisfying as a non-hardcoded fix.

ckerr avatar Jul 27 '22 22:07 ckerr

This issue has been automatically marked as stale. If this issue is still affecting you, please leave any comment (for example, "bump"), and we'll keep it open. If you have any new additional information—in particular, if this is still reproducible in the latest version of Electron or in the beta—please include it with your comment!

github-actions[bot] avatar Oct 26 '22 02:10 github-actions[bot]

This issue has been closed due to inactivity, and will not be monitored. If this is a bug and you can reproduce this issue on a supported version of Electron please open a new issue and include instructions for reproducing the issue.

github-actions[bot] avatar Nov 26 '22 02:11 github-actions[bot]

bump

Govind-gupta75 avatar Dec 13 '22 05:12 Govind-gupta75