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.

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

NativeLink flake module should make --remote_upload_local_results configurable #1371

Closed
aaronmondal opened this issue Sep 27, 2024 · 0 comments
Assignees
Labels
bug Something isn't working enhancement New feature or request

Comments

@aaronmondal
Copy link
Member

aaronmondal commented Sep 27, 2024

Otherwise users running with read-only keys will get confusing errors about failed uploads.

Currently, a workaround is to set --remote_upload_local_results=false manually for local builds.

@aaronmondal aaronmondal added bug Something isn't working enhancement New feature or request labels Sep 27, 2024
@aaronmondal aaronmondal self-assigned this Sep 27, 2024
aaronmondal added a commit to aaronmondal/nativelink that referenced this issue Sep 27, 2024
…#1369)"

This partially reverts commit 9600839.

The original idea was to implement a `readonly` setting to dynamically
configure `--remote_upload_local_results`. While this is fairly
straightforward to implement it turned out that the UX implications
around the environment variables/scripts/files to configure this are
nontrivial and we need to evaluate the different approaches in more
depth first.

For now, revert to readonly by default and also add a small modifier to
the relevant workflow so that the write-access workflow retains the
ability to write artifacts on pushes to main.

Fixes TraceMachina#1371
aaronmondal added a commit to aaronmondal/nativelink that referenced this issue Sep 29, 2024
* Revert "Allow nativelink flake module to upload results (TraceMachina#1369)" (TraceMachina#1372)

This partially reverts commit 9600839.

The original idea was to implement a `readonly` setting to dynamically
configure `--remote_upload_local_results`. While this is fairly
straightforward to implement it turned out that the UX implications
around the environment variables/scripts/files to configure this are
nontrivial and we need to evaluate the different approaches in more
depth first.

For now, revert to readonly by default and also add a small modifier to
the relevant workflow so that the write-access workflow retains the
ability to write artifacts on pushes to main.

Fixes TraceMachina#1371

* Introduce reproducible branch-based coverage

Reports may now be build via:

```nix
nix build .#nativelinkCoverageForHost
```

The `result` symlink then contains the contents of a webpage to view the
existing reports.

Pushes to main publish the site at tracemachina.github.io/nativelink.

Reports are built in release mode to closely resemble production coverage
of the testsuite. This also means that most worker tests are ignored via
a new `nix` feature to make the testsuite run in nix sandboxes. It's not
ideal, but accurately reflects our production coverage guarantees. A
future resolution for this might be  to implement more elaborate mocking
functionality for `nativelink-worker`.

Coverage leverages nix caching, but not Bazel caching. While Bazel has
builtin support for coverage, the reports were not satisfactory as they
rely on outdated gcov toolchains with vague hermeticity guarantees and
unsatisfactory implementations for llvm-cov-based workflows (e.g. no
branch-based coverage for rust and no story around coverage for
heterogeneous code). Mid-term we should implement "fast development"
coverage via Bazel alongside the "slow production" coverage via Nix.

As next steps we should continuously publish the generated HTML pages
via the web infrastructure and add hooks to report regressions.
aaronmondal added a commit to aaronmondal/nativelink that referenced this issue Sep 30, 2024
* Revert "Allow nativelink flake module to upload results (TraceMachina#1369)" (TraceMachina#1372)

This partially reverts commit 9600839.

The original idea was to implement a `readonly` setting to dynamically
configure `--remote_upload_local_results`. While this is fairly
straightforward to implement it turned out that the UX implications
around the environment variables/scripts/files to configure this are
nontrivial and we need to evaluate the different approaches in more
depth first.

For now, revert to readonly by default and also add a small modifier to
the relevant workflow so that the write-access workflow retains the
ability to write artifacts on pushes to main.

Fixes TraceMachina#1371

* Introduce reproducible branch-based coverage

Reports may now be build via:

```nix
nix build .#nativelinkCoverageForHost
```

The `result` symlink then contains the contents of a webpage to view the
existing reports.

Pushes to main publish the site at tracemachina.github.io/nativelink.

Reports are built in release mode to closely resemble production coverage
of the testsuite. This also means that most worker tests are ignored via
a new `nix` feature to make the testsuite run in nix sandboxes. It's not
ideal, but accurately reflects our production coverage guarantees. A
future resolution for this might be  to implement more elaborate mocking
functionality for `nativelink-worker`.

Coverage leverages nix caching, but not Bazel caching. While Bazel has
builtin support for coverage, the reports were not satisfactory as they
rely on outdated gcov toolchains with vague hermeticity guarantees and
unsatisfactory implementations for llvm-cov-based workflows (e.g. no
branch-based coverage for rust and no story around coverage for
heterogeneous code). Mid-term we should implement "fast development"
coverage via Bazel alongside the "slow production" coverage via Nix.

As next steps we should continuously publish the generated HTML pages
via the web infrastructure and add hooks to report regressions.
aaronmondal added a commit to aaronmondal/nativelink that referenced this issue Sep 30, 2024
* Revert "Allow nativelink flake module to upload results (TraceMachina#1369)" (TraceMachina#1372)

This partially reverts commit 9600839.

The original idea was to implement a `readonly` setting to dynamically
configure `--remote_upload_local_results`. While this is fairly
straightforward to implement it turned out that the UX implications
around the environment variables/scripts/files to configure this are
nontrivial and we need to evaluate the different approaches in more
depth first.

For now, revert to readonly by default and also add a small modifier to
the relevant workflow so that the write-access workflow retains the
ability to write artifacts on pushes to main.

Fixes TraceMachina#1371

* Introduce reproducible branch-based coverage

Reports may now be build via:

```nix
nix build .#nativelinkCoverageForHost
```

The `result` symlink then contains the contents of a webpage to view the
existing reports.

Pushes to main publish the site at tracemachina.github.io/nativelink.

Reports are built in release mode to closely resemble production coverage
of the testsuite. This also means that most worker tests are ignored via
a new `nix` feature to make the testsuite run in nix sandboxes. It's not
ideal, but accurately reflects our production coverage guarantees. A
future resolution for this might be  to implement more elaborate mocking
functionality for `nativelink-worker`.

Coverage leverages nix caching, but not Bazel caching. While Bazel has
builtin support for coverage, the reports were not satisfactory as they
rely on outdated gcov toolchains with vague hermeticity guarantees and
unsatisfactory implementations for llvm-cov-based workflows (e.g. no
branch-based coverage for rust and no story around coverage for
heterogeneous code). Mid-term we should implement "fast development"
coverage via Bazel alongside the "slow production" coverage via Nix.

As next steps we should continuously publish the generated HTML pages
via the web infrastructure and add hooks to report regressions.
aaronmondal added a commit to aaronmondal/nativelink that referenced this issue Sep 30, 2024
* Revert "Allow nativelink flake module to upload results (TraceMachina#1369)" (TraceMachina#1372)

This partially reverts commit 9600839.

The original idea was to implement a `readonly` setting to dynamically
configure `--remote_upload_local_results`. While this is fairly
straightforward to implement it turned out that the UX implications
around the environment variables/scripts/files to configure this are
nontrivial and we need to evaluate the different approaches in more
depth first.

For now, revert to readonly by default and also add a small modifier to
the relevant workflow so that the write-access workflow retains the
ability to write artifacts on pushes to main.

Fixes TraceMachina#1371

* Introduce reproducible branch-based coverage

Reports may now be build via:

```nix
nix build .#nativelinkCoverageForHost
```

The `result` symlink then contains the contents of a webpage to view the
existing reports.

Pushes to main publish the site at tracemachina.github.io/nativelink.

Reports are built in release mode to closely resemble production coverage
of the testsuite. This also means that most worker tests are ignored via
a new `nix` feature to make the testsuite run in nix sandboxes. It's not
ideal, but accurately reflects our production coverage guarantees. A
future resolution for this might be  to implement more elaborate mocking
functionality for `nativelink-worker`.

Coverage leverages nix caching, but not Bazel caching. While Bazel has
builtin support for coverage, the reports were not satisfactory as they
rely on outdated gcov toolchains with vague hermeticity guarantees and
unsatisfactory implementations for llvm-cov-based workflows (e.g. no
branch-based coverage for rust and no story around coverage for
heterogeneous code). Mid-term we should implement "fast development"
coverage via Bazel alongside the "slow production" coverage via Nix.

As next steps we should continuously publish the generated HTML pages
via the web infrastructure and add hooks to report regressions.
aaronmondal added a commit to aaronmondal/nativelink that referenced this issue Sep 30, 2024
* Revert "Allow nativelink flake module to upload results (TraceMachina#1369)" (TraceMachina#1372)

This partially reverts commit 9600839.

The original idea was to implement a `readonly` setting to dynamically
configure `--remote_upload_local_results`. While this is fairly
straightforward to implement it turned out that the UX implications
around the environment variables/scripts/files to configure this are
nontrivial and we need to evaluate the different approaches in more
depth first.

For now, revert to readonly by default and also add a small modifier to
the relevant workflow so that the write-access workflow retains the
ability to write artifacts on pushes to main.

Fixes TraceMachina#1371

* Introduce reproducible branch-based coverage

Reports may now be build via:

```nix
nix build .#nativelinkCoverageForHost
```

The `result` symlink then contains the contents of a webpage to view the
existing reports.

Pushes to main publish the site at tracemachina.github.io/nativelink.

Reports are built in release mode to closely resemble production coverage
of the testsuite. This also means that most worker tests are ignored via
a new `nix` feature to make the testsuite run in nix sandboxes. It's not
ideal, but accurately reflects our production coverage guarantees. A
future resolution for this might be  to implement more elaborate mocking
functionality for `nativelink-worker`.

Coverage leverages nix caching, but not Bazel caching. While Bazel has
builtin support for coverage, the reports were not satisfactory as they
rely on outdated gcov toolchains with vague hermeticity guarantees and
unsatisfactory implementations for llvm-cov-based workflows (e.g. no
branch-based coverage for rust and no story around coverage for
heterogeneous code). Mid-term we should implement "fast development"
coverage via Bazel alongside the "slow production" coverage via Nix.

As next steps we should continuously publish the generated HTML pages
via the web infrastructure and add hooks to report regressions.
aaronmondal added a commit to aaronmondal/nativelink that referenced this issue Sep 30, 2024
* Revert "Allow nativelink flake module to upload results (TraceMachina#1369)" (TraceMachina#1372)

This partially reverts commit 9600839.

The original idea was to implement a `readonly` setting to dynamically
configure `--remote_upload_local_results`. While this is fairly
straightforward to implement it turned out that the UX implications
around the environment variables/scripts/files to configure this are
nontrivial and we need to evaluate the different approaches in more
depth first.

For now, revert to readonly by default and also add a small modifier to
the relevant workflow so that the write-access workflow retains the
ability to write artifacts on pushes to main.

Fixes TraceMachina#1371

* Introduce reproducible branch-based coverage

Reports may now be build via:

```nix
nix build .#nativelinkCoverageForHost
```

The `result` symlink then contains the contents of a webpage to view the
existing reports.

Pushes to main publish the site at tracemachina.github.io/nativelink.

Reports are built in release mode to closely resemble production coverage
of the testsuite. This also means that most worker tests are ignored via
a new `nix` feature to make the testsuite run in nix sandboxes. It's not
ideal, but accurately reflects our production coverage guarantees. A
future resolution for this might be  to implement more elaborate mocking
functionality for `nativelink-worker`.

Coverage leverages nix caching, but not Bazel caching. While Bazel has
builtin support for coverage, the reports were not satisfactory as they
rely on outdated gcov toolchains with vague hermeticity guarantees and
unsatisfactory implementations for llvm-cov-based workflows (e.g. no
branch-based coverage for rust and no story around coverage for
heterogeneous code). Mid-term we should implement "fast development"
coverage via Bazel alongside the "slow production" coverage via Nix.

As next steps we should continuously publish the generated HTML pages
via the web infrastructure and add hooks to report regressions.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant