Excel-DNA / ExcelDna

Excel-DNA - Free and easy .NET for Excel. This repository contains the core Excel-DNA library.
https://excel-dna.net
zlib License
1.3k stars 276 forks source link

Use Dot Net Core SDK direct from the install tree rather than a copy. #396

Open davidhunter22 opened 3 years ago

davidhunter22 commented 3 years ago

There seems to be a copy of the 32 bit libnethost.lib and header files inside ExcelDna.Host. An alternative would be to reference them from the SDK install tree. I don't think it's unreasonable to say that installing the SDk is a prerequisite of building ExcelDNA itself. I will create a PR so you can take a look, this will include getting the 64 bit build of ExcelDna.Host to work as well.

You mentioned dealing with the packed ExcelDna.ManagedHost.dll file. When you say "dealing with" do you mean loading into the CLR in the ExcelDna.Host code?

I do have otehr build suggestions, the following are roughly in order of which I think might be most impactful.

1) Generate the nuget packages as part of the csproj files rather than using nuspec files and batch commands, this uses . This way the packages generation is simply part of the normal build process with no other messing around. This could also include symbols via . Note this is orthogonal to the issues of users consuming the nuget package via rather than packages.config. 2) Factor out more build stuff into Directory.Build.props 3) Put all build output into a common solution wide output directory rather than into each project and then copying stuff around 4) Select a C# compiler version, via , I would suggest 9.0 5) Switch on nullabel reference types. 6) use /W4 and treat warnings as erros. 7) Use static analysis tools

govert commented 3 years ago

I think it's simpler to keep a separate copy of the nethost files. There is a way to reference a C++ package to get them, but that seems more complicated. I'm trying to keep stuff less complicated for now. Referencing from the SDK has all kinds of version issues.

I suggest we not fiddle too much with the build stuff for now, please. I definitely can't handle large-scale changes to the project (at least for now), so I think you'll be wasting your time there - sorry. And I don't want to refactor the managed code right now, unless we need to make functional changes. The managed code mostly needs to run under .NET 4.5.2 too.

There is a good opportunity to help with the .NET 5+ hosting, since that's not baked at all yet. I've created a good issue to start with here: https://github.com/Excel-DNA/ExcelDna/issues/398

govert commented 3 years ago

Another issue that can do with help without affecting the existing stuff is the whole PackageReference story: https://github.com/Excel-DNA/ExcelDna/issues/363

I don't know exactly how that should work yet, but you might have some suggestions based on my first dump of ideas in that issue.

And I think your suggestion 1. above is a good one, but it seemed to me that building the packages as part of the csproj files would be hard. So maybe a separate project to build the packages would be feasible.

davidhunter22 commented 3 years ago

OK I'll look at the issues, I assume you mean users of being able to use rather than package.config.

BTW I would suggest that you target .NET Core 6 which is in preview rather than 5. Version 5 will be end of lifed 3 months after 6 comes out in November. 6 is an LTS version so will supported for 3 years.

govert commented 3 years ago

The current packages have a Powershell install.ps1 script that manipulates the project file, but the PackageReference style references (which are required in SDK-style project files) do not run the script. So we need a different mechanism make it so that an SDK-style project can easily use the Excel-DNA package to create an add-in. Most of the issue has to do with the .dna file which directs the add-in, and setting up the debugging to work.

I think you're right that we might target .NET 6 for a release of the .NET 5+ support (since .NET 5+ doesn't support side-by-side runtimes), but for now we might as well work with the current version. I don't know of any relevant differences at the moment.

davidhunter22 commented 3 years ago

Yes I had already looked at the script.

Given the script and the manipulation of the csproj file which is obviously very different in and SDK style project are you thinking of two nuget packages, one for SDK and for for legacy style csproj files or do you want to try to do one that works for both?

govert commented 3 years ago

I think for a user to change from an old-style project file to an SDK-style project is quite disruptive anyway. So if there are incompatible changes or a different package name, then it would not be a big problem. So for me the main downside of a different package is that it becomes harder to document, discover and pick the right one.

So I'd say for figuring out how to do it a separate package is easier, and then we can see whether there is a way to incorporate support for SDK-style projects in the existing package.

sbolofsson commented 3 years ago

I think you're right that we might target .NET 6 for a release of the .NET 5+ support (since .NET 5+ doesn't support side-by-side runtimes), but for now we might as well work with the current version.

@govert, just from a technical standpoint, I'm curious: does .NET 6 support loading multiple versions of the CLR into the same process? If not, then what's the use case for supporting .NET 6 in Excel-DNA?

Just curious because we're investigating the options for upgrading our VSTO addin from .NET Framework to .NET 5 (or 6). But it seems like we're blocked due to these issues:

I.e. it's not feasible for us to target .NET Core/5/6 if it doesn't support loading multiple versions of the runtime into the same process, since we will potentially break other addins targeting .NET Core/5/6 (though, it seems like loading .NET Core/5/6 and .NET Framework side-by-side is fine besides some relatively minor issues that you also comment on in the first issue that I linked to).

So hence my question: what's the use case for Excel-DNA in supporting .NET 6? Have things changed in that version of .NET with regards to side-by-side loading? (I haven't been able to find any documentation confirming that). Isn't it "dangerous" to go down that route?

govert commented 3 years ago

@sbolofsson My understanding and experience so far is that .NET Framework 4.x can be loaded at the same time as .NET 5, and that this is in fact a supported scenario. I would presume that nothing in this regard will change with .NET 6, so that .NET Framework 4.x and .NET 6 can also be loaded in the same process. However, .NET 5 and .NET 6 won't be able to load in the same process, as you point out.

Since Excel-DNA does not support .NET 5 or .NET 6, and possible support for these is still a while away, it will probably make sense to only support .NET 6 (which is also a long-term support release). That way the side-by-side issue between .NET 5 and .NET 6 never appears, and we can reevaluate in later years how to deal with future versions .NET 7+. But being 'stuck' on .NET 6 might be a viable option for a few years, and I can imagine workarounds after that if need be.

sbolofsson commented 3 years ago

@govert, totally follow you in the scenario with .NET 6 and .NET Framework 4.x being able to be loaded into the same process simultaneously - that's good. And also on the part about being 'stuck' on .NET 6, which is an LTS release thereby making it okay to just 'skip/disregard' .NET 5, since it will sooner be unsupported anyway.

But there could be other addins (both Excel-DNA and non Excel-DNA) targeting different minor versions of .NET 6 loaded into the same process, thereby causing issues. What are your thoughts on this?

I don't know if future minor versions of .NET 6 will imply a different version of the runtime. I haven't been following .NET Core/5/6 closely enough to be sure of how they version the runtime vs. the framework. (In the old .NET Framework it was easier to follow due to the almost never changing runtime version - but I guess that's exactly not a design goal of the new .NET).

govert commented 3 years ago

Yes, I guess there might eventually be problems dealing with different .NET 5+ versions.

I can't tell yet whether we'll be able to support any such version with Excel-DNA, for both technical and effort reasons, so the theoretical version issues seem like a bit of a luxury to consider now.

sbolofsson commented 3 years ago

Got it. I thought this issue was about supporting .NET Core/5/6 in which case I think the version issues are not theoretical but a necessity to discuss :)

None the less, I was also trying to draw parallels from Excel-DNA to our own situation. And yes we also use Excel-DNA - Many thanks for an ingenious piece of software 🙂

gmichaud commented 3 years ago

Are contributions from the community welcome regarding .NET Core support? Is there anything in particular where we could help @govert and @augustoproiete?

govert commented 3 years ago

Hi @gmichaud - yes certainly any help would be welcome, though you have to take it a bit slow with me.

Your first step would be to figure out the current state of the .NET 5+ support. The current source tree has both the native host project (ExcelDna.Host) and a test add-in (ExcelDna.Test). On my machine I can set the ExcelDna.Test project as the startup and it works. You might need to tune the launchsettings.json, or the pre / post build steps to get the right files set up.

So for me this add-in loads and UDF functions, RTD and the ribbon all work.

Once you have the basics working, there are some specific themes that need attention:

Native host bootstrapping and error handling.

Currently the managed bootstrapper, .dna file and runtimeconfig files are not packed into the .xll. So you have to have these as separate file for the add-in. See https://github.com/Excel-DNA/ExcelDna/issues/398 image

SDK-style project file / PackageReference support

This is best addressed in issue https://github.com/Excel-DNA/ExcelDna/issues/363 As a first step, some discussion and thoughts about the design, compatibility etc. for how this should work would be very welcome.


In general it helps me if you can open or extend an issue with some discussion on the intended scope of changes, before making a pull request. You are also welcome to get in touch with me directly and set up a chat.