Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a MonoDoc nuget package #246

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Conversation

mhutch
Copy link
Member

@mhutch mhutch commented May 2, 2018

Fixes #228

@Therzok
Copy link

Therzok commented May 4, 2018

Does this fix #227 ?

Otherwise, it won't work

@mhutch
Copy link
Member Author

mhutch commented May 4, 2018

No, it doesn't

Copy link
Member

@joelmartinez joelmartinez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like there were a lot of changes in addition to just adding a nuget package ... and the build is failing, seems like a nuget reference issue.

@mhutch can you maybe give me a bit more details about what the intent was/is here? I would love to get this added and published on nuget

@@ -8,12 +8,11 @@ all: build
build: $(MDOC)

$(MDOC):
$(MSBUILD) apidoctools.sln /p:Configuration=$(CONFIGURATION);
$(MSBUILD) apidoctools.sln /r /p:Configuration=$(CONFIGURATION);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does an MSBuild NuGet restore

@@ -26,6 +25,7 @@ check-mdoc:

nuget:
nuget pack mdoc/mdoc.nuspec -outputdirectory bin/Nuget
$(MSBUILD) monodoc/monodoc.csproj /p:Configuration=$(CONFIGURATION) /t:Pack
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This runs the MSBuild pack target

@@ -0,0 +1,11 @@
<Project>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

General package metadata

@@ -9,8 +9,6 @@ Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "monodoc", "monodoc\monodoc.
EndProject
Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Monodoc.Test", "monodoc\Test\Monodoc.Test.csproj", "{1EE70E2C-A289-4C36-AD0A-3D0C6CE56615}"
EndProject
Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "ICSharpCode.SharpZipLib", "external\SharpZipLib\ICSharpCode.SharpZipLib.NET45\ICSharpCode.SharpZipLib.csproj", "{0E7413FF-EB9E-4714-ACF2-BE3A6A7B2FFD}"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Switched this to a NuGet reference so it could be a package dependency

@@ -126,7 +126,7 @@ public void NoSupport_DefaultParameters()
TestMethodSignature(CSharpTestLib, "Mono.DocTest.Widget", "Default", null);
}

[TestFixtureTearDown]
[OneTimeTearDown]
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tracking NUnit API change

<Reference Include="System.ValueTuple">
<HintPath>..\packages\System.ValueTuple.4.3.1\lib\netstandard1.0\System.ValueTuple.dll</HintPath>
</Reference>
<PackageReference Include="FSharp.Core" Version="4.3.4" />
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Switched these to PackageReferences for consistency

@@ -7,7 +7,7 @@
<OutputType>Library</OutputType>
<RootNamespace>mdoc.Test</RootNamespace>
<AssemblyName>mdoc.Test</AssemblyName>
<TargetFrameworkVersion>v4.6.1</TargetFrameworkVersion>
<TargetFrameworkVersion>v4.7.1</TargetFrameworkVersion>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unlike 4.6.1, 4.7.1 doesn't require a pile of polyfill to be netstandard 2.0 compatible

<Reference Include="Mono.Cecil.Rocks, Version=0.10.0.0, Culture=neutral, PublicKeyToken=50cebf1cceb9d05e, processorArchitecture=MSIL">
<HintPath>..\packages\Mono.Cecil.0.10.0-beta5\lib\net40\Mono.Cecil.Rocks.dll</HintPath>
</Reference>
<PackageReference Include="Mono.Cecil" Version="0.10.0" PrivateAssets="all" />
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are private so that they are local copied and mdoc works

@@ -1,6 +0,0 @@
using System.Reflection;
Copy link
Member Author

@mhutch mhutch May 25, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file is autogenerated when using sdk style projects

@@ -1,59 +1,11 @@
<?xml version="1.0" encoding="utf-8"?>
<Project DefaultTargets="Build" ToolsVersion="4.0" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
<Project Sdk="Microsoft.NET.Sdk">
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SDK style projects are much nicer to hand edit :)

@@ -1,713 +1,27 @@
<?xml version="1.0" encoding="utf-8"?>
<Project DefaultTargets="Build" ToolsVersion="4.0" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
<Project Sdk="Microsoft.NET.Sdk">
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Had to switch to SDK style to get netstandard 2.0 support and the MSBuild Pack target.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also allows MASSIVELY simplifying the project file

<PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Release|AnyCPU' ">
<Optimize>true</Optimize>
<OutputPath>..\bin\Release</OutputPath>
<TargetFramework>netstandard2.0</TargetFramework>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yay netstandard 2.0!

<Content Include="monodoc.dll.config">
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
</Content>
<PackageReference Include="SharpZipLib" Version="1.0.0-alpha2" />
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One of the nice things about PackageReference is that the Pack target automatically converts it into a NuGet package dependency.

<Name>ICSharpCode.SharpZipLib</Name>
</ProjectReference>
<Compile Include="..\external\Lucene.Net.Light\src\core\**\*.cs" Link="Lucene\%(Filename)%(Extension)" />
<Compile Remove="Mono.Utilities\MemoryLRU.cs" />
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These files needed to be explicitly removed from the wildcard to match the old project's explicit file list

@@ -10,7 +10,7 @@ namespace Monodoc.Generators.Html
public class Ecmaspec2Html : IHtmlExporter
{
static string css_ecmaspec;
static XslTransform ecma_transform;
static XslCompiledTransform ecma_transform;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updating obsoleted API

@mhutch
Copy link
Member Author

mhutch commented May 25, 2018

@joelmartinez I added a bunch of comments :)

@mhutch
Copy link
Member Author

mhutch commented May 25, 2018

Currently failing with

 /usr/local/share/dotnet/sdk/2.1.200/Sdks/NuGet.Build.Tasks.Pack/build/NuGet.Build.Tasks.Pack.targets(198,5): warning : A stable release of a package should not have a prerelease dependency. Either modify the version spec of dependency "SharpZipLib [1.0.0-alpha2, )" or update the version field in the nuspec. [/Users/builder/jenkins/workspace/Docs-MonoApiDocTools/monodoc/monodoc.csproj]

@joelmartinez
Copy link
Member

Ok, I took a look at this, and it seems like the newer version of nunit changed something about the way paths are resolved (maybe the default working directory, or something) ... so as a result, a bunch of tests that have to load some assemblies are failing.

I played around and got some of them passing by making this one change:

diff --git a/mdoc/mdoc.Test/BasicTests.cs b/mdoc/mdoc.Test/BasicTests.cs
index cddbcbd..ef136aa 100644
--- a/mdoc/mdoc.Test/BasicTests.cs
+++ b/mdoc/mdoc.Test/BasicTests.cs
@@ -1,6 +1,7 @@
 using System;
 using System.Collections.Generic;
 using System.Linq;
+using System.IO;
 using Mono.Cecil;
 
 namespace mdoc.Test
@@ -18,7 +19,10 @@ namespace mdoc.Test
 
             if (!moduleCash.ContainsKey(filepath))
             {
-                var readModule = ModuleDefinition.ReadModule(filepath);
+                var testAssemblyFilename = this.GetType ().Assembly.Modules.First().FullyQualifiedName;
+                var testDirectory = Path.GetDirectoryName (testAssemblyFilename);
+                var assemblyToOpenPath = Path.Combine (testDirectory, filepath);
+                var readModule = ModuleDefinition.ReadModule(assemblyToOpenPath);
                 moduleCash.Add(filepath, readModule);
             }

But there are still some other tests failing ... I've got to focus on putting a release together, so I'm probably not going to be able to look at this for another two weeks or so (given upcoming travel, etc).

Is there a way that maybe the scope of changes could be scaled back a bit? I fully understand and agree that SDK style projects are much nicer, but maybe that should be a standalone change/upgrade from the monodoc nuget? same for some of the other changes in this PR ...

@mhutch
Copy link
Member Author

mhutch commented May 29, 2018

@joelmartinez the reason I migrated to SDK style projects was specifically because it makes it much easier to define/build the nuget packages though. I can't really separate them.

@joelmartinez
Copy link
Member

@mhutch ok, fair enough ... then could it perhaps be decoupled from upgrading nunit (assuming that was the change that otherwise broke the unit tests)?

@mhutch
Copy link
Member Author

mhutch commented May 29, 2018

@joelmartinez unfortunately nunit 2.6.4 doesn't support netstandard... :(

@joelmartinez
Copy link
Member

@mhutch gotcha ... ok, I'll try to keep working on getting the unit tests passing here, but as I mentioned above, this will probably land after I'm able to get 5.7 out (ie. in a few weeks) :)

@mhutch
Copy link
Member Author

mhutch commented May 29, 2018

fair enough!

@kekyo
Copy link

kekyo commented Jul 3, 2018

Ping: I'm waiting packaged monodoc :) what suspended reason?

@joelmartinez
Copy link
Member

@kekyo It is on my agenda to revisit this soon ... no specific ETA, but it's definitely on the agenda. The PR needs some changes to ensure the unit tests still pass, not to mention some recent master changes that will have to be carefully merged

@joelmartinez
Copy link
Member

I'm going to be breaking out some of these changes into separate/individual commits ... starting with updating to the latest nunit. This way I can make sure that all the unit tests continue passing as we subsequently move to SDK style projects, etc

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[NuGet] Ship monodoc.dll in build directory
4 participants