Skip to content

Comments

WIP: Remove custom parser in favor of another package#17

Closed
baronfel wants to merge 8 commits intomainfrom
refactor-to-csharp-lib
Closed

WIP: Remove custom parser in favor of another package#17
baronfel wants to merge 8 commits intomainfrom
refactor-to-csharp-lib

Conversation

@baronfel
Copy link
Contributor

@baronfel baronfel commented Jun 3, 2022

This PR

  • removes the fparsec-based Changelog parser
  • migrates the task to C#
  • Makes the ChangelogFile parameter no longer inferred, so users have to supply it

I did this because

  • maintaining a parser is a pain, and not actually core to these tasks
  • VS support for multi FSharp.Core references seems to be quite hard
  • It makes this repo more readable for folks that don't know F#, so I can use it more as a template/example for prospective task authors

Current problems:

  • the packaging isn't correct. When multitargeting the ResolveReferences target doesn't run when I expect and so no BuildOutputInPackage items are written for the net6.0 TFM. This results in the following incorrect package layout:
    image

@baronfel
Copy link
Contributor Author

baronfel commented Jun 3, 2022

image

Here we can see that GenerateNuspec is building the _GetBuildOutputfilesWithTfm target with a TFM set for both net472 and net6.0, as expected. My custom targets cause ResolveReferences to run, however only the net472 ResolveReferences is used?

@baronfel baronfel force-pushed the refactor-to-csharp-lib branch from 6a893ff to 514fc04 Compare June 3, 2022 15:22
@baronfel
Copy link
Contributor Author

baronfel commented Jun 3, 2022

Rainer helped me figure out what was going on:

  • no package references runtime files were being marked as CopyLocal, because there's a line in the SDK that defaults CopyLocalLockFileReferences to false unless you meet a condition. That condition is to set <EnableDynamicLoading>true</EnableDynamicLoading>.

This same property also results in deps.json files being generated, so we should just simplify to that property.

It's pretty cool that with that property, there's no difference in targeting Full framework vs .NET MSBuild, from a packaging perspective. Really sweet stuff.

The package itself needs testing (which I'll do manually now that the package is valid), but we're almost ready to merge this.

@albertwoo
Copy link

Any plan to merge this PR? I am facing below issue on VisualStudio 2022, but use dotnet cli is working as expected. I think this PR may solve my issue.

ionide.keepachangelog.tasks\0.1.8\build\Ionide.KeepAChangelog.Tasks.targets(29,9): error MSB4018: The "Ionide.KeepAChangelog.Tasks.ParseChangeLogs" task failed unexpectedly.
ionide.keepachangelog.tasks\0.1.8\build\Ionide.KeepAChangelog.Tasks.targets(29,9): error MSB4018: System.IO.FileNotFoundException: Could not load file or assembly 'FParsec, Version=1.1.1.0, Culture=neutral, PublicKeyToken=null' or one of its dependencies. The system cannot find the file specified.
ionide.keepachangelog.tasks\0.1.8\build\Ionide.KeepAChangelog.Tasks.targets(29,9): error MSB4018: File name: 'FParsec, Version=1.1.1.0, Culture=neutral, PublicKeyToken=null'
ionide.keepachangelog.tasks\0.1.8\build\Ionide.KeepAChangelog.Tasks.targets(29,9): error MSB4018:    at KeepAChangelog.Tasks.ParseChangelogs.Execute()
ionide.keepachangelog.tasks\0.1.8\build\Ionide.KeepAChangelog.Tasks.targets(29,9): error MSB4018:    at Microsoft.Build.BackEnd.TaskExecutionHost.Microsoft.Build.BackEnd.ITaskExecutionHost.Execute()
ionide.keepachangelog.tasks\0.1.8\build\Ionide.KeepAChangelog.Tasks.targets(29,9): error MSB4018:    at Microsoft.Build.BackEnd.TaskBuilder.<ExecuteInstantiatedTask>d__26.MoveNext()

@baronfel
Copy link
Contributor Author

Closing as @tboby has superseded this.

@baronfel baronfel closed this Oct 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants