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

Please don't clutter the target directory #594

Closed
DzenanJupic opened this issue Oct 13, 2022 · 5 comments · Fixed by #600
Closed

Please don't clutter the target directory #594

DzenanJupic opened this issue Oct 13, 2022 · 5 comments · Fixed by #600

Comments

@DzenanJupic
Copy link

Using the derive macros from 2.0.0-rc.1 leads to a target folder like the following:

$ ls target
AccessListItem_Decode.rs  Bytes13_Encode.rs  Bytes20_Decode.rs  Bytes27_Encode.rs  Bytes4_Decode.rs  H160_Decode.rs   I128_Encode.rs  I184_Decode.rs  I240_Encode.rs  I64_Decode.rs          U104_Encode.rs  U168_Decode.rs  U216_Encode.rs  U40_Decode.rs  U8_Encode.rs
AccessListItem_Encode.rs  Bytes14_Decode.rs  Bytes20_Encode.rs  Bytes28_Decode.rs  Bytes4_Encode.rs  H160_Encode.rs   I136_Decode.rs  I184_Encode.rs  I248_Decode.rs  I64_Encode.rs          U112_Decode.rs  U168_Encode.rs  U224_Decode.rs  U40_Encode.rs  U96_Decode.rs
Address_Decode.rs         Bytes14_Encode.rs  Bytes21_Decode.rs  Bytes28_Encode.rs  Bytes5_Decode.rs  H2048_Decode.rs  I136_Encode.rs  I192_Decode.rs  I248_Encode.rs  I72_Decode.rs          U112_Encode.rs  U16_Decode.rs   U224_Encode.rs  U48_Decode.rs  U96_Encode.rs
Address_Encode.rs         Bytes15_Decode.rs  Bytes21_Encode.rs  Bytes29_Decode.rs  Bytes5_Encode.rs  H2048_Encode.rs  I144_Decode.rs  I192_Encode.rs  I24_Decode.rs   I72_Encode.rs          U120_Decode.rs  U16_Encode.rs   U232_Decode.rs  U48_Encode.rs  debug
Block_Decode.rs           Bytes15_Encode.rs  Bytes22_Decode.rs  Bytes29_Encode.rs  Bytes6_Decode.rs  H256_Decode.rs   I144_Encode.rs  I200_Decode.rs  I24_Encode.rs   I80_Decode.rs          U120_Encode.rs  U176_Decode.rs  U232_Encode.rs  U56_Decode.rs  release
Block_Encode.rs           Bytes16_Decode.rs  Bytes22_Encode.rs  Bytes2_Decode.rs   Bytes6_Encode.rs  H256_Encode.rs   I152_Decode.rs  I200_Encode.rs  I256_Decode.rs  I80_Encode.rs          U128_Decode.rs  U176_Encode.rs  U240_Decode.rs  U56_Encode.rs  tmp
Bytes0_Decode.rs          Bytes16_Encode.rs  Bytes23_Decode.rs  Bytes2_Encode.rs   Bytes7_Decode.rs  H64_Decode.rs    I152_Encode.rs  I208_Decode.rs  I256_Encode.rs  I88_Decode.rs          U128_Encode.rs  U184_Decode.rs  U240_Encode.rs  U64_Decode.rs
Bytes0_Encode.rs          Bytes17_Decode.rs  Bytes23_Encode.rs  Bytes30_Decode.rs  Bytes7_Encode.rs  H64_Encode.rs    I160_Decode.rs  I208_Encode.rs  I32_Decode.rs   I88_Encode.rs          U136_Decode.rs  U184_Encode.rs  U248_Decode.rs  U64_Encode.rs
Bytes10_Decode.rs         Bytes17_Encode.rs  Bytes24_Decode.rs  Bytes30_Encode.rs  Bytes8_Decode.rs  I104_Decode.rs   I160_Encode.rs  I216_Decode.rs  I32_Encode.rs   I8_Decode.rs           U136_Encode.rs  U192_Decode.rs  U248_Encode.rs  U72_Decode.rs
Bytes10_Encode.rs         Bytes18_Decode.rs  Bytes24_Encode.rs  Bytes31_Decode.rs  Bytes8_Encode.rs  I104_Encode.rs   I168_Decode.rs  I216_Encode.rs  I40_Decode.rs   I8_Encode.rs           U144_Decode.rs  U192_Encode.rs  U24_Decode.rs   U72_Encode.rs
Bytes11_Decode.rs         Bytes18_Encode.rs  Bytes25_Decode.rs  Bytes31_Encode.rs  Bytes9_Decode.rs  I112_Decode.rs   I168_Encode.rs  I224_Decode.rs  I40_Encode.rs   I96_Decode.rs          U144_Encode.rs  U200_Decode.rs  U24_Encode.rs   U80_Decode.rs
Bytes11_Encode.rs         Bytes19_Decode.rs  Bytes25_Encode.rs  Bytes32_Decode.rs  Bytes9_Encode.rs  I112_Encode.rs   I16_Decode.rs   I224_Encode.rs  I48_Decode.rs   I96_Encode.rs          U152_Decode.rs  U200_Encode.rs  U256_Decode.rs  U80_Encode.rs
Bytes12_Decode.rs         Bytes19_Encode.rs  Bytes26_Decode.rs  Bytes32_Encode.rs  Bytes_Decode.rs   I120_Decode.rs   I16_Encode.rs   I232_Decode.rs  I48_Encode.rs   Transaction_Decode.rs  U152_Encode.rs  U208_Decode.rs  U256_Encode.rs  U88_Decode.rs
Bytes12_Encode.rs         Bytes1_Decode.rs   Bytes26_Encode.rs  Bytes3_Decode.rs   Bytes_Encode.rs   I120_Encode.rs   I176_Decode.rs  I232_Encode.rs  I56_Decode.rs   Transaction_Encode.rs  U160_Decode.rs  U208_Encode.rs  U32_Decode.rs   U88_Encode.rs
Bytes13_Decode.rs         Bytes1_Encode.rs   Bytes27_Decode.rs  Bytes3_Encode.rs   CACHEDIR.TAG      I128_Decode.rs   I176_Encode.rs  I240_Decode.rs  I56_Encode.rs   U104_Decode.rs         U160_Encode.rs  U216_Decode.rs  U32_Encode.rs   U8_Decode.rs

This seems a bit messy. A solution could be to either not have side effects and generate the code on the fly (I didn't read the proc macro code, so I'm not sure how they work concretely, and what the files are used for), or alternatively to use a dedicated directory.

@VictorKoenders
Copy link
Contributor

Previous (stale) issue: #531

The file exports are not needed, but I personally believe all macros should do this so developers can see what the code is that is generated. It's something that is copied from https://rtic.rs/1/book/en/

Would exporting them to a folder like target/bincode_derive_output work for you?

@DzenanJupic
Copy link
Author

That would already be a big improvement, yes

@DzenanJupic
Copy link
Author

How does this approach compare to cargo expand in your opinion?

@trevyn
Copy link
Contributor

trevyn commented Oct 15, 2022

In my experience, cargo-expand can be challenging to work with for two main reasons:

  1. All macros are expanded recursively, so if a macro expansion contains other macro invocations, those expansions can add confusion and reduce clarity.
  2. It can be tedious to find the specific invocation you're interested in, among all the other code.

See, for example:

dtolnay/cargo-expand#11
dtolnay/cargo-expand#68
dtolnay/cargo-expand#167

I've independently done a similar thing with my own proc macros, where they have the ability to write out their own expansions to the filesystem, and have found it very useful. I didn't think to unconditionally generate the outputs and put them in a subdirectory of the target directory though, that's a great solution! :)

@VictorKoenders
Copy link
Contributor

@trevyn virtue 0.0.11 is now released

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants