dotnet / docfx

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

MergeCommand failing (null pointer exception) #3067

Open artemlos opened 6 years ago

artemlos commented 6 years ago

DocFX Version Used: 2.37.0.0

Template used: default

Steps to Reproduce:

  1. Add the following to docfx.json
    "metadata": [
    {
      "src": "*.csproj",
      "dest": "temp/api/netstandard2.0",
      "properties": {
          "TargetFramework": "netstandard2.0"
      }
    },
      {
        "src": "*.csproj",
        "dest": "temp/api/net40",
        "properties": {
            "TargetFramework": "net40"
        }
    },
      {
        "src": "*.csproj",
        "dest": "temp/api/net46",
        "properties": {
            "TargetFramework": "net46"
        }
    }
    ],
    "merge": {
    "content": [
        {
            "files": "*.yml",
            "src": "temp/api/net40"
        },
        {
            "files": "*.yml",
            "src": "temp/api/net46"
        },
        {
          "files": "*.yml",
          "src": "temp/api/netstandard2.0"
        }
    ],
    "fileMetadata": {
        "platform": {
            "temp/api/net40/*.yml": [
                "net40"
            ],
            "temp/api/net46/*.yml": [
                "net46"
            ],
            "temp/api/netstandard2.0/*.yml": [
              "netstandard2.0"
          ]
        }
    },
    "dest": "api"
    }
  2. Run docfx.exe metadata
  3. Run docfx.exe docfx.json --serve

Expected Behavior: API documentation generated (i.e. merge command should have moved the files into the api folder)

Actual Behavior: An error was thrown and and the API documentation files were not generated.

[18-07-12 03:10:53.343]Info:[MetadataCommand]Completed Scope:MetadataCommand in 17778.9324 milliseconds.
[18-07-12 03:10:53.387]Info:[MergeCommand.Merge Metadata]Start merge metadata...
[18-07-12 03:10:55.891]Error:[MergeCommand]System.NullReferenceException: Object reference not set to an instance of an object.
   at Microsoft.DocAsCode.Build.Engine.HostServiceCreator.LoadModels(IEnumerable`1 files, DocumentBuildParameters parameters, IDocumentProcessor processor)
   at Microsoft.DocAsCode.Build.Engine.HostServiceCreator.CreateHostService(DocumentBuildParameters parameters, TemplateProcessor templateProcessor, IMarkdownService markdownService, IEnumerable`1 metadataValidator, IDocumentProcessor processor, IEnumerable`1 files)
   at Microsoft.DocAsCode.Build.Engine.SingleDocumentBuilder.Build(IDocumentProcessor processor, DocumentBuildParameters parameters, IMarkdownService markdownService)
   at Microsoft.DocAsCode.SubCommands.MetadataMerger.MergePageViewModel(MetadataMergeParameters parameters)
   at Microsoft.DocAsCode.SubCommands.MetadataMerger.Merge(MetadataMergeParameters parameters)
   at Microsoft.DocAsCode.SubCommands.MergeCommand.MergeDocument(String baseDirectory, String outputDirectory)
[18-07-12 03:10:55.912]Info:[MergeCommand]Completed Scope:MergeCommand in 2568.157 milliseconds.
viceice commented 6 years ago

Maybe it is related to #2286 and #2287 ?

theggelund commented 6 years ago

I got the same error while running merge on my config. After investigation the issue I found that the error is in SingleDocumentBuilder

HostServiceCreator ctor parameter DocumentBuildContext is null but it's required because LoadModels uses DocumentBuildContext.MaxParallelism but since DocumentBuildContext is null, it fails with NullReferenceException

Edit After creating context from DocumentBuildParameters ctor argument I got another NullReferenceException because of more missing dependencies. LinkPhaseHandler also has missing requirements. It is missing TemplateProcessor.

stale[bot] commented 5 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in a week if no further activity occurs.

dotMorten commented 5 years ago

I'm hitting this too :-( If you need additional repros, I can provide one.

dotMorten commented 5 years ago

The bug appears to be here because it parses a null-context into the constructor: https://github.com/dotnet/docfx/blob/27f259127b94dac20186cb6002497ae3dadc63f5/src/Microsoft.DocAsCode.Build.Engine/SingleDocumentBuilder.cs#L34 This later causes a crash here: https://github.com/dotnet/docfx/blob/27f259127b94dac20186cb6002497ae3dadc63f5/src/Microsoft.DocAsCode.Build.Engine/HostServiceCreator.cs#L117

Null is passed in as context, but the code in the class is expecting it to be non-null. Even if you try and do a null-check and just pass 1 as a fallback, it'll fail later when it gets here: https://github.com/dotnet/docfx/blob/27f259127b94dac20186cb6002497ae3dadc63f5/src/Microsoft.DocAsCode.Build.Engine/LinkPhaseHandler.cs#L96 because that's also created with a null-context.

This is a good example of why someone should enable nullable checks in their code.

Here's the quick change I made to SingleDocumentBuilder.cs's Build command. No crash no (but also no output): image

Mike-E-angelo commented 4 years ago

This is a good example of why someone should enable nullable checks in their code.

+1

After sidestepping #2284 I am now hitting this error, even with only one entry in the metadata and merge elements. Not off to a good start here. 😞

dotMorten commented 4 years ago

Any progress on this? Multi-targeting is the name of the game these days, and the merge command has been broken for at least a year-and-a-half.

gimlichael commented 4 years ago

Any progress on this? Multi-targeting is the name of the game these days, and the merge command has been broken for at least a year-and-a-half.

Yeah, I am starting to loose faith in this project. I have multi-target projects - since targetframeworks is not supported, they recommend this merge operation .. which is broken, and has been for more than 1 year.

Please prioritize a fix of this; given migration from .NET Standard 2.0 --> 2.1 --> 3.0 we urgently need support for multiple frameworks and document it accordingly.

Thanks.

dotMorten commented 4 years ago

@gimlichael I've opted to just generate the doc over and over again for each framework. One benefit of that is that I can actually create doc that's specific for a platform if I need to. It does have a downside though as it creates a gazillion warnings, and links are wrong. I fix the links with a subsequent powershell command.

Here's an example of the multi-platform generated doc: https://dotmorten.github.io/NmeaParser/api/index.html

Here's how I generate for each TFM: https://github.com/dotMorten/NmeaParser/blob/master/docs/docfx.json#L2-L70

Here's where I call the powershell script after building to fix links: https://github.com/dotMorten/NmeaParser/blob/master/.github/workflows/ghpages.yml#L43 And the powershell script: https://github.com/dotMorten/NmeaParser/blob/master/docs/FixApiRefLinks.ps1

But it really shouldn't be this hard - even if I would like separate doc for each platform

gimlichael commented 4 years ago

@dotMorten thank you for the work-around; i can see, that great minds think a-like ;-)

In my frustration over this ooooold bug, I was actually testing out the same way that you presented here .. it's just not as smooth as if they fixed the bug.

My approach - although very similar to yours - was to have multiple docfx.json .. one for each target framework and then one containing the actual build configuration.

I will check later which approach is the smoothest .. i tend to lean towards your implementation.

Thanks for sharing.

Zastai commented 4 years ago

Would PRs be accepted for a file-by-file adding of #nullable enable, and then moving it to <Nullable>enable</Nullable> in the project whenever a full project source set is completed? Wouldn't fix the issue, but would help spot it (and similar things) much much earlier.

dotMorten commented 4 years ago

@Zastai An approach that worked better for us was to add #nullable disabled instead to all files, enable it for the project, and then progressively start removing them file by file

Zastai commented 4 years ago

That seems a very reasonable choice; I can see the benefit. Either way, I’d need to know whether a PR of that nature would be accepted before I start the work. Plus, I haven’t looked at the codebase yet - if it’s full of code where I couldn’t help myself and would clean up lots of other things at the same time, it’s better if I just don’t start and aim for more localised functional fixes.

dotMorten commented 4 years ago

I even wrote a commandline tool that'll give you stats on how many lines of code you got left per-folder to help track progress: https://www.nuget.org/packages/dotMorten.NullabilityStats/

macrogreg commented 2 years ago

Using merge is described as a preferred workaround for enabling docs for libraries that target multiple frameworks. This issue blocks such usage, but it was not addressed for years. I respect the prioritization decisions of the DocFx team, however, would it be possible to learn whether there is any hope for this issue to be addressed in the foreseeable future?

If not, is there a recommended working workaround for the multitargeting issue that would permit generating docs for libraries targeting multiple frameworks? (See also https://github.com/dotnet/docfx/issues/1254#issuecomment-294080535.) Is DocFx the right choice for such use cases?

dotMorten commented 2 years ago

@macrogreg I'm able to generate doc for multiple frameworks without needing to merge at all. Here's one example: https://developers.arcgis.com/net/api-reference/index.html. It creates individual docs for each TFM (hence the index) and it has a little extra post-build magic to create the dropdown to switch between target frameworks when you're looking at a member, as well as generating the "applies to" table for each page to where each member is available. I use a slightly simpler version here: https://dotmorten.github.io/NmeaParser/api/index.html You can see all my doc scripts for that on github here: https://github.com/dotMorten/NmeaParser/tree/main/docs The main trick is to specify the source multiple times, each with a different target framework set: https://github.com/dotMorten/NmeaParser/blob/main/docs/docfx.json#L11

At this point I'm not even sure what the merge is supposed to do. Perhaps it was from before you could specify multiple targetframeworks and had to run docfx build multiple times, then merge it, but as shown that isn't really necessary now.

rst4 commented 12 months ago

I experience the same issue while trying to target multiple platforms. It's the end of 2023 and still no update on the issue.