Skip to content

fix: read CARGO_MANIFEST_DIR at runtime#90

Merged
tustvold merged 2 commits intoinfluxdata:mainfrom
tomgr:feature/improve-build-compatibility
Sep 17, 2023
Merged

fix: read CARGO_MANIFEST_DIR at runtime#90
tustvold merged 2 commits intoinfluxdata:mainfrom
tomgr:feature/improve-build-compatibility

Conversation

@tomgr
Copy link
Copy Markdown
Contributor

@tomgr tomgr commented Jan 12, 2023

Using env! inside the build.rs script makes it difficult to build this package with other build tools like bazel which sandbox execution, since the source directory might be at different paths between when the build.rs file is compiled, and when the resulting build script is executed.

Cargo recommends that CARGO_MANIFEST_DIR is read at runtime instead: https://doc.rust-lang.org/cargo/reference/environment-variables.html#environment-variables-cargo-sets-for-build-scripts

The above links also includes the promise that Cargo will set the cwd to the crate's directory. This patch uses this to find the descriptors.bin file.

Using env! inside the build.rs script makes it difficult to build this
package with other build tools like bazel which sandbox execution, since
the source directory might be at different paths between when the
build.rs file is compiled, and when the resulting build script is
executed.

Cargo recommends that CARGO_MANIFEST_DIR is read at runtime instead:
https://doc.rust-lang.org/cargo/reference/environment-variables.html#environment-variables-cargo-sets-for-build-scripts

The above links also includes the promise that Cargo will set the cwd to
the crate's directory. This patch uses this to find the descriptors.bin
file.
@tomgr tomgr force-pushed the feature/improve-build-compatibility branch from b0f6825 to a4f760e Compare January 12, 2023 12:06
@echochamber
Copy link
Copy Markdown

echochamber commented Jun 5, 2023

Any chance this can get merged. I just tested the CI builds on my fork and the tests pass, The CI actions that are failing are :

  • Lint - only failing because of using a deprecated methods from the chrono crate,
  • Audit - Failing because the rustc version on the image is too old for toml_edit, one of the deps for cargo audit.

I think neither of the above need to block this pull request since they are unrelated to this issue and can be fixed in their own separate PRs

@tustvold tustvold changed the title Follow cargo best practice in build.rs script fix: read CARGO_MANIFEST_DIR at runtime Sep 17, 2023
@tustvold tustvold merged commit 2889fe4 into influxdata:main Sep 17, 2023
Farhad-Shabani added a commit to informalsystems/pbjson that referenced this pull request Dec 20, 2023
* chore: fix CI builds (influxdata#104)

* chore: update prost to 0.12 (influxdata#111)

* chore: update prost to 0.12

* chore: fix lint

* chore: update descriptors

---------

Co-authored-by: Raphael Taylor-Davies <r.taylordavies@googlemail.com>

* fix: parsing numbers more than i32 and u32 (influxdata#87)

* fix: parsing numbers more than i32 and u32

* fix: f64::MANTISSA_DIGITS instead of 53

---------

Co-authored-by: Raphael Taylor-Davies <r.taylordavies@googlemail.com>

* fix: Resolve Envoy xDS proto issues (influxdata#103)

* fix: Escape Self since it is a reserved keyword

* fix: Allow duplicate numbers in enums when aliases are enabled

* fix: Avoid field name collisions with the map ident

* fix: follow cargo best practice in build.rs script (influxdata#90)

Using env! inside the build.rs script makes it difficult to build this
package with other build tools like bazel which sandbox execution, since
the source directory might be at different paths between when the
build.rs file is compiled, and when the resulting build script is
executed.

Cargo recommends that CARGO_MANIFEST_DIR is read at runtime instead:
https://doc.rust-lang.org/cargo/reference/environment-variables.html#environment-variables-cargo-sets-for-build-scripts

The above links also includes the promise that Cargo will set the cwd to
the crate's directory. This patch uses this to find the descriptors.bin
file.

Co-authored-by: Raphael Taylor-Davies <r.taylordavies@googlemail.com>

* chore(deps): update itertools requirement from 0.10 to 0.11 (influxdata#98)

Updates the requirements on [itertools](https://github.com/rust-itertools/itertools) to permit the latest version.
- [Changelog](https://github.com/rust-itertools/itertools/blob/master/CHANGELOG.md)
- [Commits](rust-itertools/itertools@v0.10.0...v0.11.0)

---
updated-dependencies:
- dependency-name: itertools
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Raphael Taylor-Davies <r.taylordavies@googlemail.com>

* chore(deps): update base64 requirement from 0.13 to 0.21 (influxdata#89)

* chore(deps): update base64 requirement from 0.13 to 0.21

Updates the requirements on [base64](https://github.com/marshallpierce/rust-base64) to permit the latest version.
- [Release notes](https://github.com/marshallpierce/rust-base64/releases)
- [Changelog](https://github.com/marshallpierce/rust-base64/blob/master/RELEASE-NOTES.md)
- [Commits](marshallpierce/rust-base64@v0.13.0...v0.21.0)

---
updated-dependencies:
- dependency-name: base64
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <support@github.com>

* fix: fix removed methods

* fix: indifferent padding

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Raphael Taylor-Davies <r.taylordavies@googlemail.com>

* chore: prepare 0.6.0 (influxdata#112)

* fix: prost deprecations (influxdata#113)

* chore: patch release pjson-build 0.6.2 (influxdata#114)

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: Bob Aman <sporkmonger@users.noreply.github.com>
Co-authored-by: Jeffrey Smith II <jeffreyssmith2nd@gmail.com>
Co-authored-by: Raphael Taylor-Davies <r.taylordavies@googlemail.com>
Co-authored-by: Nikolay Alexandrov <nick.alexandrov@gmail.com>
Co-authored-by: Thomas Gibson-Robinson <thomas.gibsonrobinson@gmail.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Raphael Taylor-Davies <1781103+tustvold@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants