Re: Wno-conversion changes

[Shai Barack]

Hi Andrey,

Thank you for your work! Adding +discuss in case more folks have ideas.

I wish there was a way to build everything. As it stands, GN takes arguments (indirectly via `fx set`, or directly via `fx args`), then applies them, then generates ninja files which is what decides what work is done on `fx build`. Building everything requires regenerating and running ninja for all viable combinations of arguments. Plus, for instance you can't build any Mac host binaries unless you have a Mac, because the Mac toolchain is not hermetic yet (uses your local xcode install, not limited exclusively to Fuchsia toolchain prebuilts that are downloaded on `jiri update`).

What I do instead is use this as a first approximation:

fx set core.x64 --with //bundles/buildbot:core

For large-scale changes I run any sort of cleanup on my machine with this configuration. Then I upload the change to gerrit and trigger the commit queue to identify any issues that only happen on other configurations. Then I work out what is the `fx set` command to run to reproduce any failures on CQ bots using `fx repro`.

The fact that there's no simple way is the reason why we haven't automated this cleanup of stale -Wno-conversion's yet. I'm hoping you could come up with a creative solution, or at least identify a way to clean up just code that's in the scope of a particular build configuration that captures most of the sources. That would be very valuable too!

If you have more changes then I'm happy to continue reviewing and committing them. Or we can figure out how to give you committer status.

https://fuchsia.dev/fuchsia-src/contribute/community/contributor-roles#become-a-committer

On Tue, Jan 18, 2022 at 3:40 AM Andrey Andreev <a.d.an...@gmail.com> wrote:

Hello,

I have been submitting changes for the Wno-conversion issue which I found on the getting started guide:

https://fuchsia.dev/fuchsia-src/contribute/open_projects/cpp/wconversion#completing_a_task 

I wanted to ask if there is a build command that will build everything.

I have currently done:

fx set workstation.qemu-x64

I tried to automate the removal of the unneeded Wno-conversions.

I wrote a script that checks each BUILD.gn file, deletes the config line with Wno-conversion, runs "fx build" and if it fails restores the file.

However, sometimes the build will succeed even if the target shouldn't build. I tried running "fx clean-build", but that also passes, even though individual targets fail.

An example of such a target is the `garnet/bin/insntrace:bin`. Removing the Wno-conversion for it and running `fx clean-build` passes, but compiling the specific target with `fx build garnet/bin/insntrace:bin` fails. I guess this means that the target itself isn't built when running the `fx clean-build` command and that's why it passes. 

I wanted to ask if there is a build command that will rebuild all affected targets, as that would make the automated replacing a lot simpler. Another idea I had was to parse the files and see which targets change (probably using some of the tools in the gn build system for inspiration).

Thank you for your help.

Kind regards,

Andrey

[James Tucker]

On Tue, Jan 18, 2022 at 9:34 AM 'Shai Barack' via discuss <dis...@fuchsia.dev> wrote:

Hi Andrey,

Thank you for your work! Adding +discuss in case more folks have ideas.

I wish there was a way to build everything. As it stands, GN takes arguments (indirectly via `fx set`, or directly via `fx args`), then applies them, then generates ninja files which is what decides what work is done on `fx build`. Building everything requires regenerating and running ninja for all viable combinations of arguments. Plus, for instance you can't build any Mac host binaries unless you have a Mac, because the Mac toolchain is not hermetic yet (uses your local xcode install, not limited exclusively to Fuchsia toolchain prebuilts that are downloaded on `jiri update`).

This isn't entirely true, as we already have https://fuchsia.dev/fuchsia-src/concepts/source_code/layout?hl=en#canonical_targets applied to many areas of the build. This is as it happens, why the gen phase is so very horrendously slow for us. The canonical group targets result in typically every tree being completely depth-first traversed in every single toolchain. The sad thing is that to date, we don't have a way to make use of that.

Well, no-more, ask and ye may receive, at least in part: https://fuchsia-review.googlesource.com/c/fuchsia/+/633486

What this provides is not a way to "build maximal targets", the kitchen_sink build is likely the closest we have to that still, though it may no longer build as it's typically untested. As noted in the above CL there are many configurations that do not build - because there are targets in our build that are mutually exclusive of other targets. Typically we should consider these build-bugs, and we should go and fix them, common examples are packages that are given the same name, but build in different configurations from different labels. Per our goals, these ideally should have different names, though we have some architecture challenges to address to make that easy (such as the ability to adjust more parts of the topology in the build, or to have a way to rewrite some urls based on a build-stage product configuration) - I know there's lots of work ongoing in these areas.

For now this immediately highlights some issues with targets in the Zircon build that were recently broken with re-arrangements of //third_party/boringssl. It would be great if the build or engprod teams would be interested in picking up the follow-up work from this patch, in trying to get to a world where it works. I think this is more easy to maintain as a "check the build kitchen sink" solution than the kitchen_sink bundle target - particularly as it should be able to _build everything_, something which product/image builds can't do until we can do multi-product builds. The large product builds such as buildbot:core and/or kitchen_sink still need to exist for running functional tests against configurations, but they have a different coverage range than "build everything" which is what this target provides.

 

What I do instead is use this as a first approximation:

fx set core.x64 --with //bundles/buildbot:core

For large-scale changes I run any sort of cleanup on my machine with this configuration. Then I upload the change to gerrit and trigger the commit queue to identify any issues that only happen on other configurations. Then I work out what is the `fx set` command to run to reproduce any failures on CQ bots using `fx repro`.

The fact that there's no simple way is the reason why we haven't automated this cleanup of stale -Wno-conversion's yet. I'm hoping you could come up with a creative solution, or at least identify a way to clean up just code that's in the scope of a particular build configuration that captures most of the sources. That would be very valuable too!

If you have more changes then I'm happy to continue reviewing and committing them. Or we can figure out how to give you committer status.

https://fuchsia.dev/fuchsia-src/contribute/community/contributor-roles#become-a-committer

On Tue, Jan 18, 2022 at 3:40 AM Andrey Andreev <a.d.an...@gmail.com> wrote:

Hello,

I have been submitting changes for the Wno-conversion issue which I found on the getting started guide:

https://fuchsia.dev/fuchsia-src/contribute/open_projects/cpp/wconversion#completing_a_task 

I wanted to ask if there is a build command that will build everything.

I have currently done:

fx set workstation.qemu-x64

I tried to automate the removal of the unneeded Wno-conversions.

I wrote a script that checks each BUILD.gn file, deletes the config line with Wno-conversion, runs "fx build" and if it fails restores the file.

However, sometimes the build will succeed even if the target shouldn't build. I tried running "fx clean-build", but that also passes, even though individual targets fail.

An example of such a target is the `garnet/bin/insntrace:bin`. Removing the Wno-conversion for it and running `fx clean-build` passes, but compiling the specific target with `fx build garnet/bin/insntrace:bin` fails. I guess this means that the target itself isn't built when running the `fx clean-build` command and that's why it passes. 

I wanted to ask if there is a build command that will rebuild all affected targets, as that would make the automated replacing a lot simpler. Another idea I had was to parse the files and see which targets change (probably using some of the tools in the gn build system for inspiration).

Thank you for your help.

Kind regards,

Andrey

--
All posts must follow the Fuchsia Code of Conduct https://fuchsia.dev/fuchsia-src/CODE_OF_CONDUCT or may be removed.
---
You received this message because you are subscribed to the Google Groups "discuss" group.
To unsubscribe from this group and stop receiving emails from it, send an email to discuss+u...@fuchsia.dev.
To view this discussion on the web visit https://groups.google.com/a/fuchsia.dev/d/msgid/discuss/CAK0PkCEy8p%3D2m5zyE5%2Bk_1LU5r1Ux-QdmjiK-x81rrS%2BxVbPRg%40mail.gmail.com.

[Andrey Andreev]

Hello Shai,

Thank you for the pointers.

For now I will try with the configuration you showed and probably add the executables in the same BUILD.gn file, as those are relatively easy to find.

Kind regards,

Andrey