Support cadl generation from dotnet language repo #32540
Conversation
…or-net into mnash-cadlGeneration
Cadl build must refer to the checked-out spec folder
| @@ -0,0 +1,152 @@ | |||
| [CmdletBinding()] | |||
There was a problem hiding this comment.
Just checking - if I'm on a plane with no network access and have a copy of all the spec files needed already sync'd to local.... I can still run codegen, right?
There was a problem hiding this comment.
No we would need to add in some "offline" mode. Right now it will always try to pull the changes in for the commit sha since it doesn't know if the local files are up to date or not.
we could do this by some flag in the cadl-location.yaml such as commit: local to indicate don't sync just use my local files.
Do you mind filing an issue for this and we can address after this PR unless you think its a requirement for v1. Trying to keep this to MVP as the dpg timelines are very sensitive right now.
There was a problem hiding this comment.
We could potentially add some configuration option that allows folks to point at a local specs repo clone as I do think that will be an interesting scenario even outside of the offline scenario.
add gitignore for temp cadl files folder
add cleanup flag
cadl-compile needs a main definition
this allows other langauges to share sync
no manual tweaks
|
API change check APIView has identified API level changes in this PR and created following API reviews. |
|
|
||
| # Temporary cadl folders for cadl generation | ||
| TempCadlFiles/ |
There was a problem hiding this comment.
Any reason to put these in a new top level folder instead of under artifacts like everything else?
There was a problem hiding this comment.
not sure I follow, they are currently inside each project folder, I have the ignore so they are never accidentally checked in.
There was a problem hiding this comment.
I guess my main question is can we redirect these temp folders under a place like artifacts that is already in the ignore list instead of adding random temp folders throughout the repo.
There was a problem hiding this comment.
It's because cleanup: false in cadl-location.yaml. I remove that property in another PR, and TempCadlFiles will be removed after dotnet build /t:GenerateFiles
| } | ||
|
|
||
| function InitializeSparseGitClone([string]$repo) { | ||
| git clone --no-checkout --filter=tree:0 $repo . |
There was a problem hiding this comment.
In order to get this to work for the private repo we are going to either need to use a PAT from our bot account or perhaps figure out how to use the credentials from the agent. @benbp did you ever do any investigation with the persistCredentials option https://learn.microsoft.com/en-us/azure/devops/pipelines/yaml-schema/steps-checkout?view=azure-pipelines while doing your sparse checkout work?
There was a problem hiding this comment.
I think the approach currently used in test proxy where we filter on the PAT existing and change the repo url accordingly could work here as well. I recall initially trying to get pipeline sparse checkout with persist credentials working, but the format/encoding of the token was set up in some way where I couldn't figure out how to consume it. Some working examples may have been posted here.
| @@ -0,0 +1,6 @@ | |||
| directory: specification/adp/DataManagement | |||
| additionalDirectories: | |||
There was a problem hiding this comment.
We might need to figure out if we want to do this via additionalDirectories or simply clone the entire adp directory instead. I don't think the extra files will cause much of an issue, whereas the additionalDirectories will always guarantee more manual configuration which likely means a failure that requires manual intervention.
There was a problem hiding this comment.
it won't always be in the same service directory. We will need the ability to point at additional sub directories.
There was a problem hiding this comment.
My assumption is that we should block source references outside of the same service folder but you are correct that we might not be able to do that in all cases.
|
|
||
| $tempFolder = "$ProjectDirectory/TempCadlFiles" | ||
| $npmWorkingDir = Resolve-Path $tempFolder/$innerFolder | ||
| $mainCadlFile = Resolve-Path "$npmWorkingDir/main.cadl" |
There was a problem hiding this comment.
We might need another parameter to indicate the entry cadl file. It doesn't have to be main.cadl. client.cadl is a common practice.
| } | ||
| } | ||
|
|
||
| function CopySpecToProjectIfNeeded([string]$specCloneRoot, [string]$mainSpecDir, [string]$dest, [string[]]$specAdditionalSubDirectories) { |
There was a problem hiding this comment.
I think we should clean the folder before copying.
| Push-Location $npmWorkingDir | ||
| NpmInstallForProject $npmWorkingDir | ||
| Write-Host("npx cadl compile $mainCadlFile --emit `"`@azure-tools/cadl-csharp`" --output-path `"$resolvedProjectDirectory`" --option @azure-tools/cadl-csharp.clear-output-folder=true") | ||
| npx cadl compile $mainCadlFile --emit "@azure-tools/cadl-csharp" --output-path "$resolvedProjectDirectory" --option @azure-tools/cadl-csharp.clear-output-folder=true |
There was a problem hiding this comment.
Why the options are hard-coded here? Shouldn't it be in the cadl-project.yaml?
There was a problem hiding this comment.
And my understanding is there should be logic some where to move all the parts only leaving "@azure-tools/cadl-csharp" part. Is this feature included?
Contributing to the Azure SDK
Please see our CONTRIBUTING.md if you are not familiar with contributing to this repository or have questions.
For specific information about pull request etiquette and best practices, see this section.