cadl compile failed when package.json doesn't include all emitters defined in cadl-project.yaml

[dw511214992]

How to reproduce

1. Get my cadl project
2. npm install
3. npx cadl compile . --emit @azure-tools/cadl-java --arg "java-sdk-folder=/tmp/java"

Result

Encounter Error:

error import-not-found: Couldn't resolve import "@azure-tools/cadl-python"
error emitter-not-found: Emitter with name @azure-tools/cadl-python is not found.

Found 2 errors.

Expect

Generate java sdk succesfully.

Root cause analysis:

The package.json's dependencies doesn't include emitter @azure-tools/cadl-python, which is defined in cadl-project.yaml.
As discussed with @weshaggard and @allenjzhang , package.json doesn't include any sdk emitters by default. When running in sdk automation pipeline, we only install the target sdk emitter in package.json. For example, I want to generate java sdk and I have installed @azure-tools/cadl-java in package.json, and then run command npx cadl compile . --emit @azure-tools/cadl-java --arg "java-sdk-folder=/tmp/java", cadl should generate java sdk and don't check whether @azure-tools/cadl-python exist or not when I specify the java emitter in command.
I also test this issue in cadl v0.37.* and it works well. So this issue only exists in v0.38.*.

[dw511214992]

@timotheeguerin

[allenjzhang]

Ack. @timotheeguerin, in order for this to work, we would need to skip loading verification of libraries that is not listed or being emitted.

[raych1]

@dw511214992 validated this issue was introduced from v0.38.*

[timotheeguerin]

This was done by design here #1211. Adding @m-nash and @tjprescott which both got frustrated when this wasn't being validated and had a wrong emitter name.

Now we have to weight the 2 side of this issue and I see a few options:

1. Only do the validation on the --option flag not in the config
   • Pro: We still get to fail the cli use
   • Cons: We are now inconsistent + not sure if we can easily do that as the validation is later in the process
2. Provide a flag strict-emitter which can be turned on/off(Can decide on the default)
   • Pro: This could give an escape hatch for azure sdk repo
   • Cons: Adds a new flag

Thing is this scenario feels very specific to azure specs repo flow. With option 1 we are making it worse for all user of Cadl to satisfy a workaround. I think option 2 with a default of strict-emitter: true would be better but also this brings a flag that might become obsolete for our use case if we ever get to be able to import urls.

[timotheeguerin]

Proposal

Add a new cadl config property

model CadlConfig {
  "validate-emitter-options"?:
    | "strict"          // Validate everything name & options
    | "installed-only"  // Skip uninstalled emitters - (Do we need this one?)
    | "emit-only"       // Only validate options for emitter running
    | "off"             // Skip uninstalled emitters and don't validate the options
    = "strict";         // default to "strict"
}

emit:
  - cadl-autorest
options:
  cadl-autorest:
    opt1: "abc"
    unknown-opt: "abc"

  cadl-java:
    opt1: "abc"
    unknown-opt: "abc"

  cadl-python:
    opt1: "abc"
    unknown-opt: "abc"

{
  "dependencies": {
    "cadl-autorest": "latest",
    "cadl-java": "latest"
  }
}

strict - Validate everything name & options

• complain cadl-java is not installed
• complain cadl-csharp is not installed
• complain cadl-autorest.unknown-opt doesn't exist
• complain cadl-java.unknown-opt doesn't exist (even though it is not run)

installed-only - Skip uninstalled emitters - (Do we need this one?)

• complain cadl-autorest.unknown-opt doesn't exist
• complain cadl-java.unknown-opt doesn't exist (even though it is not run)

emit-only - Only validate options for emitter running

• complain cadl-autorest.unknown-opt doesn't exist

off - Skip uninstalled emitters and don't validate the options

No errors

Alternative

If we don't want a new flag then the only solution is to really just disable validation and act like emit-only from above.

[timotheeguerin]

Updated the error to be a warning to mitigate the issue #1460 until resolution of above design.

[tjprescott]

I ran into this as well and it annoyed me. The "fix" was to comment out the emitter options for the offending emitter. However, since I wasn't trying to emit that particular language, I was surprised it even checked those options. But given the details @timotheeguerin shared, I'm thinking the warning may be the best option. The problem before was that it just silently failed and you had no idea anything was wrong (except that it didn't do what you expected).

My vote would probably be for no new flag and just emit the warning instead of error.

[dw511214992]

Thanks @timotheeguerin I like this solution which only adds a warning. Do you want to close this issue? Or let's keep it open if you want to figure out a better solution?

[timotheeguerin]

Yeah lets keep it open, my guess is that a warning is not going to be working if you want to prevent specs from being merged with warnings in the future.

[markcowl]

• the error was made a warning to unblock this usage
• design discussion needed for proposal