[Storage] Initial v0.7.0 codegen

[vincenttran-msft]

This is currently pointed to vincenttran/blob-tsp-test2, which contains the necessary stop-gap changes from feature/blob-tsp to allow this typespec definition to be ingested.

1. Axing pageable operations (currently still NYI, expected behavior, emitter-stopping)
2. Adding a crate-version (this is required by the emitter)
3. Change the name from storage_blob to azure_storage_blob and combining with our handwritten wrapper code

[vincenttran-msft]

storage_blob doesn't exist in that scenario. You'd have a azure_storage_blobs (plural; same naming convention as every other language) crate where the clients live in the root. Again, like most other languages. Implementations should be idiomatic, yes, but naming should be idiomatically consistent (i.e. the same, ignoring casing differences). That means for "packages" (nupkgs, wheels, jars, crates, etc.) as well as namespaces/modules.

Originally posted by @heaths in #1959 (comment)

Hi Heath, just wanted to touch base as to whether this is your expected directory structure? If so, I will look to merge our old initial scaffolding (which is the azure_storage_blob singular namespace) with the generated output. Thanks!

[heaths]

Looks like it. Basically, you should follow the naming convention you use for other languages (but idiomatic, so underscores here). Looking at https://azure.github.io/azure-sdk/, I see both "Azure Storage Blobs" and "Azure Storage Blob". I guess pick which makes the most sense to you. Personally, I prefer the plural "blobs" since it works with Storage Blobs - not a blob.

[vincenttran-msft]

Looks like it. Basically, you should follow the naming convention you use for other languages (but idiomatic, so underscores here). Looking at https://azure.github.io/azure-sdk/, I see both "Azure Storage Blobs" and "Azure Storage Blob". I guess pick which makes the most sense to you. Personally, I prefer the plural "blobs" since it works with Storage Blobs - not a blob.

Hi Heath, that sounds good to me, I also started an email thread to loop in our PMs to see if they have any strong feelings or guidance wrt the naming and will land on something after all that feedback is received. Thanks!

[vincenttran-msft]

This is ready for review, but is failing CI for several duplicates of the same issue:

error: field `container_name` is never read
  --> sdk/storage/azure_storage_blob/src/generated/clients/blob_blob_client.rs:16:16
   |
15 | pub struct BlobBlobClient {
   |            -------------- field in this struct
16 |     pub(crate) container_name: String,
   |                ^^^^^^^^^^^^^^

This issue will be resolved as a result of finding a solution for: https://github.com/Azure/typespec-rust/issues/218 , so if that solves in a timely manner we can wait for it, otherwise curious to hear what the path forward should be.

In the meantime, we will be continuing to work on fleshing out the get blob properties API in a separate branch based off this branch.

[jhendrixMSFT]

The unused container_name field is a known issue. I've asked @catalinaperalta to look into it as it might simply be a matter of removing it from the tsp.

Also, it looks like you need to execute cargo fmt on the generated code.

[heaths]

@jhendrixMSFT said,

Also, it looks like you need to execute cargo fmt on the generated code.

Can we have the tool do that? It's not something authors should have to know/remember to do.

[jhendrixMSFT]

I would really prefer to have the emitter just emit code and not have any dependency on 3rd party tools to perform any post-processing. This is why in the Go repo we have build.go files to execute the emitter and perform any post-processing (especially since a lot of SDKs have unique post-processing to modify generated code).

Is there a mechanism we can use to invoke some post-generation command, or do we need to wrap building in a script to do this?

[catalinaperalta]

FYI I removed the containerName definition from client.tsp. This might change in a few days however as the client.tsp is restructured to output a better generated client design.

[heaths]

I would really prefer to have the emitter just emit code and not have any dependency on 3rd party tools to perform any post-processing. This is why in the Go repo we have build.go files to execute the emitter and perform any post-processing (especially since a lot of SDKs have unique post-processing to modify generated code).

Is there a mechanism we can use to invoke some post-generation command, or do we need to wrap building in a script to do this?

Using a build.rs file will increase build times for people that do not need to generate code, which we should not have the dev simply importing our libraries do because they'd need to install a bunch of TypeScript and TypeSpec libraries, Node.js, etc. Even if the build.rs file does nothing in main if not passed an env var or whatever as @RickWinter did in #1966, Rust still compiles it and executes the resulting binary even before it compiles the crate. This has a downstream impact.

We could have a script or cargo xtask or something do this, but I'm curious to know why you call cargo fmt a third-party tool? It's part of the rust toolchain itself and automatically installed whenever you run a cargo command based on our root rust-toolchain.toml. Or did you just mean to say it's external to the TypeSpec toolchain and rust emitter?

[jhendrixMSFT]

My mental model is that any process external to the TypeSpec toolchain/emitter is a "3rd party tool". Maybe a "hidden dependency" is a better way to phrase it. When you pnpm install the emitter and its dependencies, this doesn't install the Rust toolchain. So, how does one know that it's a requirement to generate code? And if it's not present, what do we do? Fail? Warn/ignore?

Agreed that build.rs isn't an appropriate mechanism. I'd like to explore other possibilities before deciding that we should incorporate cargo fmt in the emitter.

[heaths]

We have #1972 tracking that currently.

[jhendrixMSFT]

Can you please resolve the merge conflicts?

[vincenttran-msft]

Can you please resolve the merge conflicts?

Definitely, resolving that right now. Spinning on generating the lockfile.

[vincenttran-msft]

Alrighty, I have resolved all the merge conflicts and cspell issues. The remaining issues are with the "Run source analysis" step (sample except):

error: unresolved link to `Update`
   --> sdk/storage/azure_storage_blob/src/generated/clients/blob_blob_client.rs:119:10
    |
119 |     /// [Update] The Lease Blob operation establishes and manages a lock on a blob for write and delete operations.
    |          ^^^^^^ no item named `Update` in scope
    |
    = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]`

Any insight on what is the correct path forward? This should be our final blocker 😄

[jhendrixMSFT]

It's coming from the doc comments in the tsp. example

Presumably the doc comments need to be updated. At least I think that's the preferred solution.

[jhendrixMSFT]

I updated the tsp in the feature branch to clean up these doc comments.