Skip to content
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

every EmitOption field in std.Build.Step.Compile (for example: emit_docs) lacks FileSource integration #16351

Closed
andrewrk opened this issue Jul 7, 2023 · 2 comments
Closed

every EmitOption field in std.Build.Step.Compile (for example: emit_docs) lacks FileSource integration #16351

andrewrk opened this issue Jul 7, 2023 · 2 comments
Labels
breaking Implementing this issue could cause existing code to no longer compile or have different behavior. bug Observed behavior contradicts documented or intended behavior contributor friendly This issue is limited in scope and/or knowledge of Zig internals. enhancement Solving this issue will likely involve adding new logic or components to the codebase. zig build system std.Build, the build runner, `zig build` subcommand, package management

Comments

@andrewrk
Copy link
Member

andrewrk commented Jul 7, 2023

zig/lib/std/Build/Step/Compile.zig

Lines 49 to 58 in b9fc0d2

emit_analysis: EmitOption = .default,
emit_asm: EmitOption = .default,
emit_bin: EmitOption = .default,
emit_docs: EmitOption = .default,
emit_implib: EmitOption = .default,
emit_llvm_bc: EmitOption = .default,
emit_llvm_ir: EmitOption = .default,
// Lots of things depend on emit_h having a consistent path,
// so it is not an EmitOption for now.
emit_h: bool = false,

Instead of passing strings directly to the Zig CLI, the build system API should have a function for adding emission of each of these artifacts which returns a FileSource which can then be used as inputs in other parts of the build system API.

As an example, getOutputSource already does this for the CLI equivalent of -femit-bin.

As a bonus, the build system should pass -fno-emit-bin unless getOutputSource() is called on the Compile Step. This will happen automatically in the case of adding an install step for the build artifact, which passes the result of getOutputSource() to an Install Step.
@andrewrk andrewrk added bug Observed behavior contradicts documented or intended behavior enhancement Solving this issue will likely involve adding new logic or components to the codebase. contributor friendly This issue is limited in scope and/or knowledge of Zig internals. breaking Implementing this issue could cause existing code to no longer compile or have different behavior. zig build system std.Build, the build runner, `zig build` subcommand, package management labels Jul 7, 2023
@andrewrk andrewrk added this to the 0.12.0 milestone Jul 7, 2023
@andrewrk andrewrk mentioned this issue Jul 7, 2023
Closed
@ikskuh
Copy link
Contributor

ikskuh commented Jul 19, 2023

I'll try resolving this with a PR together with #16353

@ikskuh ikskuh mentioned this issue Jul 19, 2023
Merged
andrewrk added a commit that referenced this issue Jul 22, 2023
@andrewrk
move installation logic to the build script where it belongs
3965f72 
* build.zig: introduce `-Dflat` option which makes the installation
  match what we want to ship for our download tarballs. This allows
  deleting a bunch of shell script logic from the CI.
  - for example it puts the executable directly in prefix/zig rather
    than prefix/bin/zig and it additionally includes prefix/LICENSE.
* build.zig: by default also install std lib documentation to doc/std/
  - this can be disabled by `-Dno-autodocs` similar to how there is
    already `-Dno-langref`.
* build.zig: add `std-docs` and `langref` steps which build and install
  the std lib autodocs and langref to prefix/doc/std and
  prefix/doc/langref.html, respectively.

* std.Build: implement proper handling of `-femit-docs` using the
  LazyPath system. This is a breaking change.
  - this is a partial implementation of #16351
* frontend: fixed the handling of Autodocs with regards to caching and
  putting the artifacts in the proper location to integrate with the
  build system.
  - closes #15864

* CI: delete the logic for autodocs since it is now handled by build.zig
  and is enabled by default.
  - in the future we should strive to have nearly all the CI shell
    script logic deleted in favor of `zig build` commands.
* CI: pass `-DZIG_NO_LIB=ON`/`-Dno-lib` except for the one command where
  we want to actually generate the langref and autodocs. Generating the
  langref takes 14 minutes right now (why?!) so we don't want to do that
  more times than necessary.

* Autodoc: fixed use of a global variable. It works fine as a local
  variable instead.
  - note that in the future we will want to make Autodoc run
    simultaneously using the job system, but for now the principle of
    YAGNI dictates that we don't have an init()/deinit() API and instead
    simply call the function that does the things.
* Autodoc: only do it when there are no compile errors
@andrewrk andrewrk mentioned this issue Jul 22, 2023
Merged
andrewrk added a commit that referenced this issue Jul 22, 2023
@andrewrk
move installation logic to the build script where it belongs
414a076 
* build.zig: introduce `-Dflat` option which makes the installation
  match what we want to ship for our download tarballs. This allows
  deleting a bunch of shell script logic from the CI.
  - for example it puts the executable directly in prefix/zig rather
    than prefix/bin/zig and it additionally includes prefix/LICENSE.
* build.zig: by default also install std lib documentation to doc/std/
  - this can be disabled by `-Dno-autodocs` similar to how there is
    already `-Dno-langref`.
* build.zig: add `std-docs` and `langref` steps which build and install
  the std lib autodocs and langref to prefix/doc/std and
  prefix/doc/langref.html, respectively.

* std.Build: implement proper handling of `-femit-docs` using the
  LazyPath system. This is a breaking change.
  - this is a partial implementation of #16351
* frontend: fixed the handling of Autodocs with regards to caching and
  putting the artifacts in the proper location to integrate with the
  build system.
  - closes #15864

* CI: delete the logic for autodocs since it is now handled by build.zig
  and is enabled by default.
  - in the future we should strive to have nearly all the CI shell
    script logic deleted in favor of `zig build` commands.
* CI: pass `-DZIG_NO_LIB=ON`/`-Dno-lib` except for the one command where
  we want to actually generate the langref and autodocs. Generating the
  langref takes 14 minutes right now (why?!) so we don't want to do that
  more times than necessary.

* Autodoc: fixed use of a global variable. It works fine as a local
  variable instead.
  - note that in the future we will want to make Autodoc run
    simultaneously using the job system, but for now the principle of
    YAGNI dictates that we don't have an init()/deinit() API and instead
    simply call the function that does the things.
* Autodoc: only do it when there are no compile errors
andrewrk added a commit that referenced this issue Jul 22, 2023
@andrewrk
move installation logic to the build script where it belongs
5f6fb95 
* build.zig: introduce `-Dflat` option which makes the installation
  match what we want to ship for our download tarballs. This allows
  deleting a bunch of shell script logic from the CI.
  - for example it puts the executable directly in prefix/zig rather
    than prefix/bin/zig and it additionally includes prefix/LICENSE.
* build.zig: by default also install std lib documentation to doc/std/
  - this can be disabled by `-Dno-autodocs` similar to how there is
    already `-Dno-langref`.
* build.zig: add `std-docs` and `langref` steps which build and install
  the std lib autodocs and langref to prefix/doc/std and
  prefix/doc/langref.html, respectively.

* std.Build: implement proper handling of `-femit-docs` using the
  LazyPath system. This is a breaking change.
  - this is a partial implementation of #16351
* frontend: fixed the handling of Autodocs with regards to caching and
  putting the artifacts in the proper location to integrate with the
  build system.
  - closes #15864

* CI: delete the logic for autodocs since it is now handled by build.zig
  and is enabled by default.
  - in the future we should strive to have nearly all the CI shell
    script logic deleted in favor of `zig build` commands.
* CI: pass `-DZIG_NO_LIB=ON`/`-Dno-lib` except for the one command where
  we want to actually generate the langref and autodocs. Generating the
  langref takes 14 minutes right now (why?!) so we don't want to do that
  more times than necessary.

* Autodoc: fixed use of a global variable. It works fine as a local
  variable instead.
  - note that in the future we will want to make Autodoc run
    simultaneously using the job system, but for now the principle of
    YAGNI dictates that we don't have an init()/deinit() API and instead
    simply call the function that does the things.
* Autodoc: only do it when there are no compile errors
@andrewrk andrewrk mentioned this issue Jul 22, 2023
Closed
andrewrk added a commit that referenced this issue Jul 22, 2023
@andrewrk
move installation logic to the build script where it belongs
6e4fff6 
* build.zig: introduce `-Dflat` option which makes the installation
  match what we want to ship for our download tarballs. This allows
  deleting a bunch of shell script logic from the CI.
  - for example it puts the executable directly in prefix/zig rather
    than prefix/bin/zig and it additionally includes prefix/LICENSE.
* build.zig: by default also install std lib documentation to doc/std/
  - this can be disabled by `-Dno-autodocs` similar to how there is
    already `-Dno-langref`.
* build.zig: add `std-docs` and `langref` steps which build and install
  the std lib autodocs and langref to prefix/doc/std and
  prefix/doc/langref.html, respectively.

* std.Build: implement proper handling of `-femit-docs` using the
  LazyPath system. This is a breaking change.
  - this is a partial implementation of #16351
* frontend: fixed the handling of Autodocs with regards to caching and
  putting the artifacts in the proper location to integrate with the
  build system.
  - closes #15864

* CI: delete the logic for autodocs since it is now handled by build.zig
  and is enabled by default.
  - in the future we should strive to have nearly all the CI shell
    script logic deleted in favor of `zig build` commands.
* CI: pass `-DZIG_NO_LIB=ON`/`-Dno-lib` except for the one command where
  we want to actually generate the langref and autodocs. Generating the
  langref takes 14 minutes right now (why?!) so we don't want to do that
  more times than necessary.

* Autodoc: fixed use of a global variable. It works fine as a local
  variable instead.
  - note that in the future we will want to make Autodoc run
    simultaneously using the job system, but for now the principle of
    YAGNI dictates that we don't have an init()/deinit() API and instead
    simply call the function that does the things.
* Autodoc: only do it when there are no compile errors
@andrewrk
Copy link
Member Author

Duplicate of #14971

@andrewrk andrewrk closed this as not planned Won't fix, can't repro, duplicate, stale Jul 22, 2023
@andrewrk andrewrk removed this from the 0.12.0 milestone Jul 22, 2023
@andrewrk andrewrk mentioned this issue Jul 24, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
breaking Implementing this issue could cause existing code to no longer compile or have different behavior. bug Observed behavior contradicts documented or intended behavior contributor friendly This issue is limited in scope and/or knowledge of Zig internals. enhancement Solving this issue will likely involve adding new logic or components to the codebase. zig build system std.Build, the build runner, `zig build` subcommand, package management
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@andrewrk @ikskuh