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

Add a --check argument to cargo codegen #3495

Closed
jleibs opened this issue Sep 27, 2023 · 1 comment · Fixed by #3918
Closed

Add a --check argument to cargo codegen #3495

jleibs opened this issue Sep 27, 2023 · 1 comment · Fixed by #3918
Assignees
@teh-cmc
Labels
codegen/idl 🧑‍💻 dev experience developer experience (excluding CI) enhancement New feature or request good first issue Good for newcomers 🏎️ Quick Issue Can be fixed in a few hours or less
Milestone
Triage

Comments

@jleibs
Copy link
Member

jleibs commented Sep 27, 2023

This should do a dry-run and return an error code if any of the files would be modified.

This would be a bit clearer and less error prone in the CI check:

rerun/.github/workflows/reusable_checks.yml

Lines 138 to 140 in 50222c2

- name: No Diffs From Running Codegen
run: |
git diff --exit-code

And would also be something that could be added to a git pre-push hook to reduce frequency of submitting code that's going to fail the codegen check.
@jleibs jleibs added enhancement New feature or request 🧑‍💻 dev experience developer experience (excluding CI) codegen/idl labels Sep 27, 2023
@jleibs jleibs added this to the 0.10 Polish milestone Sep 27, 2023
@emilk
Copy link
Member

emilk commented Oct 9, 2023

The various codegens should not write to the filesystem directly, but instead return a BTreeMap<Path, String> of filed it wants to write. This will then be trivial to implement. It will also make the codegen a lot nicer.

@emilk emilk added good first issue Good for newcomers 🏎️ Quick Issue Can be fixed in a few hours or less labels Oct 9, 2023
@teh-cmc teh-cmc mentioned this issue Oct 17, 2023
Merged
4 tasks
@teh-cmc teh-cmc self-assigned this Oct 17, 2023
teh-cmc added a commit that referenced this issue Oct 17, 2023
@teh-cmc
Make codegen I/O-free and agnostic to output location (#3888)
a364405 
**Commit by commit**

This is necessary refactoring work for the upcoming
`attr.rust.custom_crate` attribute, itself necessary for the upcoming
serde-codegen support, itself necessary for the upcoming blueprint
experimentations as well as #3741.

### Changes

1. The `CodeGenerator` trait as well as all post-processing helpers
(gitattributes, orphan detection...) are now I/O-free.
  ```rust
pub type GeneratedFiles =
std::collections::BTreeMap<camino::Utf8PathBuf, String>;
  
  pub trait CodeGenerator {
      fn generate(
          &mut self,
          reporter: &crate::Reporter,
          objects: &crate::Objects,
          arrow_registry: &crate::ArrowRegistry,
      ) -> GeneratedFiles;
  }
  ```
2. All post-processing helpers are now agnostic to the location output.
This is very important as it makes it possible to generate e.g. rust
code out of the `re_types` crate without everything crumbling down.
A side-effect is that gitattributes files are now finer-grained.
3. The Rust codegen pass is now crate agnostic: it is driven by the
workspace path rather than a specific crate path.
Necessary for the upcoming `attr.rust.custom_crate`.
4. All codegen passes now follow the exact same 4-step structure:
  ```
  // 1. Generate in-memory code files.
  let mut gen = MyGenerator::new();
  let mut files = gen.generate(reporter, objects, arrow_registry);
  // 2. Generate in-memory attribute files.
  generate_gitattributes_for_generated_files(&mut files);
  // 3. Write all in-memory files to disk.
  write_files(&gen.pkg_path, &gen.testing_pkg_path, &files);
  // 4. Remove orphaned files.
  crate::codegen::common::remove_orphaned_files(reporter, &files);
  ```
5. The documentation codegen pass now removes its orphans, which is why
some `md` files were removed in this PR.

---

- Unblocks #3741 
- Unblocks #3495
@teh-cmc teh-cmc mentioned this issue Oct 18, 2023
Merged
4 tasks
teh-cmc added a commit that referenced this issue Oct 19, 2023
@teh-cmc
Codegen: Multi-pass refactoring & support for --check (#3918)
22e0188 
- Adds a new `CodeFormatter` trait.
- Adds a new trait-based generic `generate_code` helper, and reimplement
all language-specific backends in terms of it.
- Adds `--check` flag.
- Use `--check` instead of git dirtyness checks on CI.

---

- Fixes #3495
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@teh-cmc teh-cmc

Labels
codegen/idl 🧑‍💻 dev experience developer experience (excluding CI) enhancement New feature or request good first issue Good for newcomers 🏎️ Quick Issue Can be fixed in a few hours or less
Projects
None yet
Milestone
Triage
Development

Successfully merging a pull request may close this issue.

Codegen: Multi-pass refactoring & support for --check rerun-io/rerun
3 participants
@emilk @teh-cmc @jleibs