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

Less automatic build.rs shenanigans #3814

Merged
merged 12 commits into from
Oct 11, 2023
Merged

Less automatic build.rs shenanigans #3814

merged 12 commits into from
Oct 11, 2023

Conversation

emilk
Copy link
Member

@emilk emilk commented Oct 11, 2023

What

rust-analyzer will no longer run codegen

…if you configure rust-analyzer to use --all-features, which you should, and which we do in .vscode/settings.json

Same is true for stuff like bacon too.

No build-time and git branch/commit for local developers

I've removed the build-time and git branch/commit from:

  • rerun --version
  • Viewer Rerun Menu -> About
  • analytics

for local developers (that means you), who has cloned our repository.

This is to save us from annoying re-builds using complex detection code.

No build-time and git branch/commit for crate users

Users of our crates (i.e. users who put rerun in their Cargo.toml or build rerun using cargo install rerun) will also no longer see the build-time and git branch/commit in these places. Previously we would try to get the git branch/commit hash for whatever folder they happened to be in (😬 ), which was not a great idea. So that's a bug fix.

The build time is a bit sadder to let go, but it was always unreliable: if you ran cargo update for instance, the build time would not update even if the rerun crate was being rebuilt. So instead of showing an unreliable built-time, I think it is better to just remove it.

Checklist

  • I have read and agree to Contributor Guide and the Code of Conduct
  • I've included a screenshot or gif (if applicable)
  • I have tested demo.rerun.io (if applicable)
  • The PR title and labels are set such as to maximize their usefulness for the next release's CHANGELOG

@emilk emilk added 🧑‍💻 dev experience developer experience (excluding CI) include in changelog labels Oct 11, 2023
@@ -0,0 +1,65 @@
//! # Situations to consider regarding git
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is old code that has moved to its own file

Comment on lines +33 to +40
const EXPORT_BUILD_TIME_FOR_DEVELOPERS: bool = false;

/// Should we export the current git hash/branch for developers in the workspace?
///
/// It will be visible in analytics, in the viewer's about-menu, and with `rerun --version`.
///
/// To do so accurately may incur unnecessary recompiles, so only turn this on if you really need it.
const EXPORT_GIT_FOR_DEVELOPERS: bool = false;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I decided not to make these into environment variables right now. We can very easily do so now if we change our minds.

@emilk emilk marked this pull request as ready for review October 11, 2023 15:28
@Wumpf Wumpf self-requested a review October 11, 2023 16:08
Copy link
Member

@Wumpf Wumpf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tradeoffs make sense to me. Good step forward to saner builds! Haven't tried it locally yet. Will stay in the lookout for spurious rebuilds etc.!

Comment on lines +100 to +102
if !datetime.is_empty() {
write!(f, ", built {datetime}")?;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wouldn't it be more helpful to write something like built <unknown>?

Copy link
Member Author

@emilk emilk Oct 11, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it? Maybe "built at an unknown time", but then we could also add "and at an unknown place"

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the output you get from rerun --version btw

crates/re_types/Cargo.toml Show resolved Hide resolved
crates/re_types/build.rs Outdated Show resolved Hide resolved
@emilk emilk merged commit f6cdc38 into main Oct 11, 2023
30 checks passed
@emilk emilk deleted the emilk/build-rs-changes branch October 11, 2023 17:37
emilk added a commit that referenced this pull request Jan 23, 2024
### What
This is so we can test things out before the next release, and also get
in some new egui features for the plot aggregator and drag-and-drop.

* Closes #4716
* Closes #4794

### TODO
* [x] Fix hovering ListItems in blueprint panel

### wgpu changelog
https://github.com/gfx-rs/wgpu/blob/trunk/CHANGELOG.md#v0190-2024-01-17

### relevant egui changelog (so far)

#### eframe
* Keep `ViewportInfo::maximized` and `minimized` up-to-date on Windows
[#3831](emilk/egui#3831) (thanks
[@rustbasic](https://github.com/rustbasic)!)
* Update wgpu to 0.19 [#3824](emilk/egui#3824)
* Fix: handle `IconData::default()` without crashing
[#3842](emilk/egui#3842)

#### egui_extras
* Fix unwraps in SVG scaling
[#3826](emilk/egui#3826) (thanks
[@amPerl](https://github.com/amPerl)!)
* Update to ehttp 0.4 [#3834](emilk/egui#3834)

#### egui_plot
* Make `egui_plot::PlotMemory` public
[#3871](emilk/egui#3871)

#### egui
* Selectable text in Labels
[#3814](emilk/egui#3814)
* `ComboBox`: add builder method for height
[#3001](emilk/egui#3001) (thanks
[@hinto-janai](https://github.com/hinto-janai)!)
* Add keys `?`, `/`, `|`
[#3820](emilk/egui#3820)
* Fix clickable widgets blocking scrolling on touch screens
[#3815](emilk/egui#3815) (thanks
[@lucasmerlin](https://github.com/lucasmerlin)!)
* Fix `stable_dt` [#3832](emilk/egui#3832)
* Bug Fix : `Response::is_pointer_button_down_on` is now false the frame
the button is released [#3833](emilk/egui#3833)
(thanks [@rustbasic](https://github.com/rustbasic)!)
* Use runtime knowledge of OS for OS-specific text editing
[#3840](emilk/egui#3840)
* Refactor: move text selection logic to own module
[#3843](emilk/egui#3843)
* Fix: dragging to above/below a `TextEdit` or `Label` will select text
to begin/end [#3858](emilk/egui#3858)
* Add `Response::contains_pointer`
[#3859](emilk/egui#3859)
* Always set `response.hovered` to false when dragging another widget
[#3860](emilk/egui#3860)
* Add `Align2::anchor_size`
[#3863](emilk/egui#3863)
* Add `Context::debug_text`
[#3864](emilk/egui#3864)

#### epaint
* Add `Align2::anchor_size`
[#3863](emilk/egui#3863)

### Checklist
* [x] I have read and agree to [Contributor
Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and
the [Code of
Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md)
* [x] I've included a screenshot or gif (if applicable)
* [x] I have tested the web demo (if applicable):
* Using newly built examples:
[app.rerun.io](https://app.rerun.io/pr/4885/index.html)
* Using examples from latest `main` build:
[app.rerun.io](https://app.rerun.io/pr/4885/index.html?manifest_url=https://app.rerun.io/version/main/examples_manifest.json)
* Using full set of examples from `nightly` build:
[app.rerun.io](https://app.rerun.io/pr/4885/index.html?manifest_url=https://app.rerun.io/version/nightly/examples_manifest.json)
* [x] The PR title and labels are set such as to maximize their
usefulness for the next release's CHANGELOG

- [PR Build Summary](https://build.rerun.io/pr/4885)
- [Docs
preview](https://rerun.io/preview/eb1bce846c3adb29b99d04018b002475994ad213/docs)
<!--DOCS-PREVIEW-->
- [Examples
preview](https://rerun.io/preview/eb1bce846c3adb29b99d04018b002475994ad213/examples)
<!--EXAMPLES-PREVIEW-->
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)

---------

Co-authored-by: Andreas Reich <r_andreas2@web.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🧑‍💻 dev experience developer experience (excluding CI) include in changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants