dotnet / docfx

Static site generator for .NET API documentation.
https://dotnet.github.io/docfx/
MIT License
4.09k stars 867 forks source link

[Bug] Docset.Build throws ArgumentNullException if `pdf` is set in docfx.json #8348

Closed chrisvanderpennen closed 1 year ago

chrisvanderpennen commented 1 year ago

Describe the bug At https://github.com/dotnet/docfx/blob/main/src/Microsoft.DocAsCode.App/RunPdf.cs#L18, config.BaseDirectory is passed as the first argument to Path.Combine but it is always null, so it throws ArgumentNullException.

Based on what RunBuild.cs is doing, I believe the fix would be to move https://github.com/dotnet/docfx/blob/main/src/Microsoft.DocAsCode.App/RunPdf.cs#L32-L34 to the top of the method, and pass EnvironmentContext.BaseDirectory to Path.Combine instead.

To Reproduce Steps to reproduce the behavior:

  1. Call Docset.Build, passing the path to a docfx.json defining PDF output.

Expected behavior A PDF to be generated.

Context (please complete the following information):

{
  "build": {
    "content": [
      {
        "src": "docs",
        "files": [
          "platform/**",
          "frontend/**",
          "content/**",
          "minutes/**",
          "index.md",
          "toc.yml"
        ],
        "exclude": ["**/.attachments/**"]
      },
      {
        "files": [
          "{frontend,platform}/README.md",
          "platform/*/README.md",
          "platform/*/docs/**.{md,yml}",
          "frontend/{apis,lib,web,desktop}/*/README.md",
          "frontend/{apis,lib,web,desktop}/*/docs/**.{md,yml}"
        ],
        "exclude": ["**/ci/**", "**/node_modules/**", "**/.attachments/**"]
      }
    ],
    "resource": [
      {
        "src": "docs",
        "files": ["content/.attachments/**", "content/**/.attachments/**", "minutes/.attachments/**", "minutes/**/.attachments/**"]
      },
      {
        "files": [
          "frontend/{apis,lib,web,desktop}/*/docs/.attachments/**",
          "platform/*/docs/.attachments/**",
          "frontend/{apis,lib,web,desktop}/*/docs/**/.attachments/**",
          "platform/*/docs/**/.attachments/**"
        ]
      }
    ],
    "dest": "docs/_site",
    "template": [
      "docs/template"
    ],
    "globalMetadata": {
      "_enableSearch": true,
      "_disableBreadcrumb": false
    },
    "fileMetadata": {
      "_disableContribution": {
        "**/swagger.json": true
      }
    },
    "globalMetadataFiles": [],
    "fileMetadataFiles": [],
    "postProcessors": [],
    "markdownEngineName": "markdig",
    "markdownEngineProperties": {
      "plantUml.renderingMode": "remote",
      "plantUml.remoteUrl": "http://localhost:8080/",
      "markdigExtensions": [
          "mathematics",
          "diagrams"
      ]
    },
    "noLangKeyword": false,
    "keepFileLink": false,
    "cleanupCacheHistory": true,
    "disableGitFeatures": false
  },
  "pdf": {
    "content": [
      {
        "src": "docs",
        "files": ["platform/**", "frontend/**", "minutes/**", "content/**", "index.md"],
        "exclude": ["**/.attachments/**"]
      },
      {
        "files": [
          "{frontend,platform}/README.md",
          "platform/*/README.md",
          "platform/*/docs/**.{md,yml}",
          "frontend/{apis,lib,web,desktop}/*/README.md",
          "frontend/{apis,lib,web,desktop}/*/docs/**.{md,yml}"
        ],
        "exclude": ["**/ci/**", "**/node_modules/**", "**/.attachments/**"]
      },
      {
        "src": "docs/pdf",
        "files": ["toc.yml"]
      }
    ],
    "resource": [
      {
        "src": "docs",
        "files": ["content/.attachments/**", "content/**/.attachments/**", "minutes/.attachments/**", "minutes/**/.attachments/**"]
      },
      {
        "files": [
          "frontend/{apis,lib,web,desktop}/*/docs/.attachments/**",
          "platform/*/docs/.attachments/**",
          "frontend/{apis,lib,web,desktop}/*/docs/**/.attachments/**",
          "platform/*/docs/**/.attachments/**"
        ]
      }
    ],
    "wkhtmltopdf": {
      "filePath": "/usr/bin/wkhtmltopdf",
      "additionalArguments": "--enable-local-file-access"
    },
    "dest": "docs/_site_pdf",
    "template": [
      "pdf.default"
    ],
    "globalMetadata": {
      "_enableSearch": false,
      "_disableBreadcrumb": false
    },
    "fileMetadata": {
      "_disableContribution": {
        "**/swagger.json": true
      }
    },
    "globalMetadataFiles": [],
    "fileMetadataFiles": [],
    "postProcessors": [],
    "markdownEngineName": "markdig",
    "markdownEngineProperties": {
      "plantUml.renderingMode": "remote",
      "plantUml.remoteUrl": "http://localhost:8080/",
      "markdigExtensions": [
          "mathematics",
          "diagrams"
      ]
    },
    "noLangKeyword": false,
    "keepFileLink": false,
    "cleanupCacheHistory": true,
    "disableGitFeatures": false
  }
}
System.ArgumentNullException: Value cannot be null. (Parameter 'path1')
   at System.IO.Path.Combine(String path1, String path2)
   at Microsoft.DocAsCode.RunPdf.Exec(PdfJsonConfig config)
   at Microsoft.DocAsCode.Docset.Build(String configPath)
   at Program.<Main>$(String[] args) in /home/chrisv/repos/redacted/docs-tools/docs-builder/Program.cs:line 3
❯ dotnet --info
.NET SDK:
 Version:   7.0.102
 Commit:    4bbdd14480

Runtime Environment:
 OS Name:     ubuntu
 OS Version:  22.04
 OS Platform: Linux
 RID:         ubuntu.22.04-x64
 Base Path:   /usr/share/dotnet/sdk/7.0.102/

Host:
  Version:      7.0.2
  Architecture: x64
  Commit:       d037e070eb

.NET SDKs installed:
  6.0.405 [/usr/share/dotnet/sdk]
  7.0.102 [/usr/share/dotnet/sdk]

.NET runtimes installed:
  Microsoft.AspNetCore.App 6.0.13 [/usr/share/dotnet/shared/Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 7.0.2 [/usr/share/dotnet/shared/Microsoft.AspNetCore.App]
  Microsoft.NETCore.App 6.0.13 [/usr/share/dotnet/shared/Microsoft.NETCore.App]
  Microsoft.NETCore.App 7.0.2 [/usr/share/dotnet/shared/Microsoft.NETCore.App]

Other architectures found:
  None

Environment variables:
  DOTNET_ROOT       [/usr/lib/dotnet/dotnet6-6.0.109]

global.json file:
  Not found

Learn more:
  https://aka.ms/dotnet/info

Download .NET:
  https://aka.ms/dotnet/download

Additional context contents of Program.cs:

// See https://aka.ms/new-console-template for more information

await Microsoft.DocAsCode.Docset.Build("docfx.json");
yufeih commented 1 year ago

Thank you for reporting this bug! Your analysis is absolutely correct, would you like to send a PR for a fix?

chrisvanderpennen commented 1 year ago

Sure, I can do that.

chrisvanderpennen commented 1 year ago

While testing I found this would also throw if the configuration didn't provide a path for wkhtmltopdf.

I've restored the logic used in 2.59.4, from https://github.com/dotnet/docfx/blob/v2.59.4/src/docfx/SubCommands/PdfCommand.cs#L30 and https://github.com/dotnet/docfx/blob/v2.59.4/src/docfx/Extensions/StringExtensions.cs, as a private static method on RunPdf - please let me know if you would like me to factor this differently :)