Details

Reviewers

russ.haley_gmail.com

Group Reviewers

mono

Summary

This review has been split into multiple parts
This review remains as a meta review to track the multiple parts

Individual reviews:

https://reviews.freebsd.org/D13751: ancillary ports
https://reviews.freebsd.org/D13752: mono (and USES=mono)
https://reviews.freebsd.org/D14135: msbuild
https://reviews.freebsd.org/D16069: lang/fsharp: update to 10.0.2
https://reviews.freebsd.org/D16070: devel/monodevelop: update to 7.1.0.1297
https://reviews.freebsd.org/D13869: support linux ports for dotnet-core (CentOS)
https://reviews.freebsd.org/D13870: linux-dotnet (depends on support linux ports)
https://reviews.freebsd.org/D14006: lang/mono: update to 5.8.0.108
https://reviews.freebsd.org/D15780: lang/mono: Update to 5.10.1.47

Diff Detail

Repository

rP FreeBSD ports repository

Lint

Lint Skipped

Unit

Tests Skipped

Event Timeline

dbn created this revision.Sep 21 2017, 8:18 PM

Herald added a reviewer: portmgr. · View Herald TranscriptSep 21 2017, 8:18 PM

Herald added a subscriber: mat. · View Herald Transcript

Could you write a summary of what you are doing ? (one that is human readable, that is)

Also, could you use devel/arcanist so that we get the actual commits you are talking about in here, and the context, or at least generate a diff with full context like it does, with svn diff -x -U9999 or git diff -U9999.

romain added a subscriber: romain.Sep 22 2017, 9:00 AM

romain added a subscriber: woodsb02.Sep 22 2017, 9:02 AM

romain edited the summary of this revision. (Show Details)Sep 22 2017, 9:20 AM

romain edited the summary of this revision. (Show Details)

romain retitled this revision from Update mono to 5.2.0.215 to Update lang/mono to 5.2.0.215 and update related ports.Sep 22 2017, 9:42 AM

romain edited the summary of this revision. (Show Details)

Changes:

rebase patch to r450479
lang/mono: fix distinfo with ACCEPTANCE_TESTS
spelling fix in summary
update log in summary

Rebase to r453757

Nice job adding msbuild. That's going to come in handy for dotnet core 2. I'll try building this again asap.

Mk/Uses/mono.mk
57	Just a sanity check that mono_ARGS:Nnuget and mono_ARGS:Mnuget on line 77 are actually two different things and not a typo?
132	Can you please give me a brief on what this section is doing?
devel/monodevelop/Makefile
30	Are we pulling a nuget package called appveyor or is that the name of the site we pull nugetizer3000 from? If the later, it doesn't follow your "naming convention". Also, it seems the NUGET_URL is defined somewhere else. Is that on purpose or would it be better to put it here too?
devel/monodevelop/nupkg-nuget
124	What are these Linux packages for?

dbn marked 3 inline comments as done.Nov 25 2017, 2:35 PM

dbn added inline comments.

Mk/Uses/mono.mk
57	Yes, this line should use :N and line 77 should use :M. The former means "all that don't match" whereas the latter means "all that match". Sanity checked ;-)
132	Nuget "caches" (I think nuget uses a different term) the packages in an extracted format, plus a few extra files: the package stored at ${PACKAGE}.nupkg; and the package checksum stored at ${PACKAGE}.nupkg.sha512 The above line calculates the cache in base64 format, without trailing space. I would have used sha512 however our implemntationd doesn't support base64 output.
devel/monodevelop/Makefile
30	Neither :-/. We are pulling a package called NuGet.Build.Packaging. It is being pulled from a site called nugetizer3000? hosted at appveyor. What name do you think is intuitive?
devel/monodevelop/nupkg-nuget
124	I'll explain the full context of how this list is generated, but in brief: because that is what the port's build system expects. These packages are not used.

russ.haley_gmail.com added inline comments.Nov 27 2017, 5:21 AM

devel/monodevelop/Makefile
30	https://github.com/NuGet/NuGet.Build.Packaging Nuget.Build.Packaging is the namespace for the Nugetizer3000 software. If the AZURE_APPSERVICE_URL pulls the azure app service, the TEMPLATING_URL is for the templating engine, ROSLYN_URL pulls the compiler, then naming the url for Build Packaging for by the hosting site is incongruous with the other names. NUGETIZER3000_URL or NUGET_BUILD_PACK_URL or some variant there of would be more appropriate in my opinion. :)
devel/msbuild/files/csc.exe
1	csc.exe is the C# compiler. Can I ask why there is a python file named csc.exe in the files/ dir? Not that it's definitive, but I don't see anything like this in Microsoft.msbuild gh repo.

russ.haley_gmail.com added inline comments.Nov 27 2017, 5:29 AM

devel/monodevelop/nupkg-nuget
124	So the upstream package requires them but doesn't use them? That's fine I guess, I'm just being curious.

Hi David,

In the end, this review is just too massive for me to work with. I had some issues with the patch for mono but was able to get it to build. I'd like to help and submit that patch, but I can't work with all the other changes too. Can I suggest breaking this review up along the groupings in your comment and submitting each part separately (starting with the mono update)? I just don't think this update will ever get finished due to it's breadth.

dbn edited the summary of this revision. (Show Details)Jan 3 2018, 7:09 PM

dbn edited the test plan for this revision. (Show Details)

dbn edited the summary of this revision. (Show Details)Jan 3 2018, 7:27 PM

dbn edited the test plan for this revision. (Show Details)

In D12440#286673, @russ.haley_gmail.com wrote:

Hi David,

In the end, this review is just too massive for me to work with. I had some issues with the patch for mono but was able to get it to build. I'd like to help and submit that patch, but I can't work with all the other changes too. Can I suggest breaking this review up along the groupings in your comment and submitting each part separately (starting with the mono update)? I just don't think this update will ever get finished due to it's breadth.

TL;DR: I completely agree.

I suggest you review mono (https://reviews.freebsd.org/D13752)
I'll check/commit the ancillary updates
Please subscribe yourselves to the relevant dependent reviews (more to come - see summary)

I tried splitting up this patch, and yes, it is very big and complex :-(. What I've done is turn this review into a meta review and created two reviews for the pieces of work that can be committed (independently). I've kept the content of this review so we don't lose anything raised.

The other three items depend directly and indirectly on mono so I suggest we delay reviewing them until we get mono reviewed. I'm happy to manage this process as I somewhat understand.

dbn edited the summary of this revision. (Show Details)Jan 6 2018, 8:50 AM

dbn removed a reviewer: portmgr.

Herald added a reviewer: portmgr. · View Herald TranscriptJan 6 2018, 8:50 AM

Yay, ancillary ports have been committed. I'm slowly flushing out the backlog of changes that I've accumulated.

dbn retitled this revision from Update lang/mono to 5.2.0.215 and update related ports to Tracker for mono-related reviews.Jan 11 2018, 6:37 PM

dbn edited the summary of this revision. (Show Details)

dbn edited the summary of this revision. (Show Details)Jan 21 2018, 2:50 PM

dbn removed a reviewer: portmgr.

dbn added a subscriber: portmgr.

Herald added a reviewer: portmgr. · View Herald TranscriptJan 21 2018, 2:50 PM