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

Automatically replace XLL version information with data read from an assembly #15

Open
augustoproiete opened this issue Aug 27, 2015 · 35 comments

Comments

@augustoproiete
Copy link
Member

This is a (slightly modified) feature request that was asked in the mailing-list, so just adding as an issue so we can track this better and possibly encourage someone to implement and send a PR.

Original requests:

My assumption is that the developer (or ideally the continuous integration system) should be responsible for setting the right version information about the add-in, in the main assembly (or in all of them). With that said, we should be able to describe in the Excel-DNA XML file which assembly should be used to read version information about the add-in (version number, copyright, product name, etc.) and any information available in this assembly should automatically replace what is in the XLL file, during the build of the project.

I would suggest this to be implemented as a regular MSBuild Task, rather than implementing this as part of the ExcelDnaPack utility given that people might not care about packing their add-ins - and still would like to get version information in the XLL that gets copied during the build.

@govert
Copy link
Member

govert commented Aug 27, 2015

I think it's a good idea, but not something I'm likely to work on myself anytime soon.

Is there a standard MSBuild task that can do a version update for a .dll?

If not, I think the code for the version update would fit well into the ExcelDnaPack executable, since that already modifies the .xll file. Maybe ExcelDnaPack should have an option to update the Version resource only (without packing the .dna file). That would eliminate the need for another binary to update the Version resource, but still give you the option to only do a version update.
I'm happy that a build step might call ExcelDnaPack for the version update, or even a custom built target (though I wouldn't know how to make such a thing).

I would like to discuss details of the .dna changes before you implement that aspect. What attributes do you plan to add etc...

@augustoproiete
Copy link
Member Author

Hi Govert,

I understand this is not in your short-term roadmap, which is why I added the issue here to surface it to others as you said you'd be open to a PR (I'll take a stab on this in a few days, if no one does).

I'm not aware of any built-in MSBuild task that can do that. I was thinking about implementing a custom one that would be included in the NuGet package and run as part of the build (via a custom build target). This MSBuild task would just modify the version resource of the .XLL file(s) to set the corresponding version info, all using Windows APIs, similar to what you already do in ExcelDnaPack.

Regarding changes to the .dna file, I was thinking about adding a new attribute (name to be defined) to the "DnaLibrary" tag, that would tell the MSBuild task to use that assembly to read version information.

e.g.

<DnaLibrary Name="MyAwesomeExcelAddIn" UseVersionFrom="MyXLAddInAssembly.dll">
    <ExternalLibrary Path="MyXLAddInAssembly.dll" LoadFromBytes="true" />
</DnaLibrary>

Or alternatively, add an attribute to the "ExternalLibrary" tag.

e.g.

<DnaLibrary Name="MyAwesomeExcelAddIn">
    <ExternalLibrary Path="MyXLAddInAssembly.dll" LoadFromBytes="true" CopyVersionToXLL="true" />
</DnaLibrary>

I prefer something along the lines of the the first approach as it is more clear/direct. The second approach would allow a developer to add the same attribute to multiple tags, and we would have to define some rules to pick which one to use, etc.

Thanks,
Caio Proiete

@govert
Copy link
Member

govert commented Aug 28, 2015

I don't know how the MSBuild task would call the Windows APIs, but I'm suggesting that instead of an extra .dll for the build task, that those API calls go into ExcelDnaPack. Then MSBuild could call ExcelDnaPack with some parameters to update the Version. But I'm open to other ideas.

In fact, what I think would be first prize is for all the packing calls to made via build targets, thus replacing the post-build events that do the packing now. But I've not been brave enough to figure out the build targets yet.

I'm fine with the .dna file getting the extra attribute as you suggest - I agree adding to the DnaLibrary tag makes more sense.

@augustoproiete
Copy link
Member Author

@govert: Makes sense. Opened issue #17 to tackle that. Will replace build events with custom build tasks first, and then revisit this issue to implement the version thing as part of the custom build target

@govert
Copy link
Member

govert commented Aug 29, 2015

I suggest this sequence of work items:

  • Implement a switch in ExcelDnaPack that updates the version resource in the .xll, taking the path of a .dll to use. We might have an additional switch or option that does not do the default packing of the .dna and .xll.config files.
  • Implement the attribute in the .dna file to specify which ExternalLibrary to take the version from. Update ExcelDnaPack to use the attribute if no version update is specified on the command-line.
  • Implement a build targets option to update the version for the unpacked .xll file that is written to the Output directory. At first I'd prefer this to still use the ExcelDnaPack executable (via a standard Exec build task).
  • Then reconsider how this interacts with the build implementation and whether to split ExcelDnaPack into MSBuild-friendly task library and separate executable at that stage.

@govert
Copy link
Member

govert commented Sep 4, 2015

@augustoproiete I understand that making a separate .dll file with build tasks is probably the end goal. I'm just trying to sequence things so that I can live with the intermediate states.

If you're OK doing the version stuff first, then I suggest doing it inside ExcelDnaPack first, and leaving the refactoring for later. Something like the sequence I suggest here.

Otherwise, you might like to do the refactoring clean-up of ExcelDnaPack first, into a .dll (ExcelDna.Build.dll ?) which does all the work (and could be used to host build tasks), and a small command-line executable ExcelDnaPack.exe around it. For this step the main issue would probably be to sort out all the messages and stuff.

So I'm OK with either direction, just not the hybrid where some build stuff is in a .dll and some still in the executable.

@augustoproiete
Copy link
Member Author

@govert: Thanks.

The sequence you propose above is fine, especially if I was tackling this issue in isolation, and it would make sense to extend ExcelDnaPack first (quick win) and later worry about wrapping it into an MSBuild task + console app. No worries there.

However, in terms of priority, I think #18 and #19 are much more important - currently a pain for many devs, which makes debugging harder. I'd rather spend time on #18 and #19 next (after we finalize #17) and leave this issue here for later.

What do you think?

Maybe we go with a variation of your second suggestion, and try to wrap ExcelDnaPack into an MSBuild task doing the minimum amount of refactoring possible. Just enough to make it work (whilst I work on #18 and #19), and leave refactoring for another issue?

@govert
Copy link
Member

govert commented Sep 5, 2015

@augustoproiete Your help is greatly appreciated, and of course you can take on the issues in the sequence that would help you most.

Having said that, I've been asked about the version things about five times. I've never had a single question about the debug startup stuff in #18, and #19.

I'm also not sure how you'd approach #18 - I don't know if it's just another MSBuild property and the issue is just finding the right path, or whether it requires some other way of updating the project files.

MSBuild has build-in tasks for running executables. So if ExcelDnaPack.exe is able to update the version, then getting it to run as part of the build should be easy with no custom build task library. Or is there something I'm missing?

@davidandersen
Copy link

davidandersen commented May 18, 2016

@govert / @augustoproiete -- Versioning the xll for Windows Installer would be super helpful for us. We need to manage publishing of various add-ins and versions thereof. If we could resolve this during packing and have the .xll indicate the version of a packed binary then that would be awesome. Any idea on if and when this feature would be available?

@govert
Copy link
Member

govert commented May 18, 2016

@davidandersen It's not something I'm planning to do at this stage, but I'd be happy to accept a contribution for this.

I suggest the first step for this would be to implement it as a switch in ExcelDnaPack, which updates the version resource in the .xll, taking the path of a .dll to use.
Once that works, we can easily incorporate it into the new build mechanism #27.

Would that work for your case?
Would you be able to work on this?

Alternatively, it might be possible to add a call to a utility like verpatch in you post-build script.

@augustoproiete
Copy link
Member Author

augustoproiete commented May 18, 2016

Some thoughts, just to add to the conversation:

  1. Developers that don't use ExcelDnaPack to pack the add-in would expect their regular .xll files (AddIn.xll and AddIn64.xll) to have the version updated as well - and not just the -packed.xll (which is when using ExcelDnaPack.exe (instead of something like ExcelDnaBuild.exe etc) to do this work feels a little bit odd, but we can live with that) e.g.

    ExcelDnaPack.exe update-file-version MyAddIn.xll

  2. In addition to set the .XLL version resource with the version from an assembly, we should also create a separate resource in the .XLL to store the ExcelDna version which is valuable info. Ideally we should have a way to query this hidden version without having to install a separate resource viewer, etc. Maybe something like:

    ExcelDnaPack.exe show-file-version MyAddIn.xll
    ExcelDnaPack.exe show-dna-version MyAddIn.xll

Maybe we just change the C++ project to include a new resource e.g. "dna_version" which by default is the same as the version resource. Therefore nothing special needs to be done by ExcelDnaPack to keep the "dna_version" there.

@govert
Copy link
Member

govert commented May 19, 2016

I agree with the first point - so maybe ExcelDnaPack is not the right place.
What about adding it to the ExcelDna.AddIn.Tasks build utility? Since we now have that library anyway, it might be a better fit.

I now (finally?) agree that the ExcelDnaPack code should probably go in there too, leaving ExcelDnaPack.exe as a thin wrapper that just calls the library.

I'll have to think about the ExcelDna version. Maybe it can just be a comment or description written in the regular version resource.

On 19 May 2016, at 01:35, Caio Proiete <notifications@github.commailto:notifications@github.com> wrote:

Some thoughts, just to add to the conversation:

  1. Developers that don't use ExcelDnaPack to pack the add-in would expect their regular .xll files (AddIn.xll and AddIn64.xll) to have the version updated as well - and not just the -packed.xll (which is when using ExcelDnaPack.exe (instead of something like ExcelDnaBuild.exe etc) to do this work feels a little bit odd, but we can live with that) e.g.

ExcelDnaPack.exe update-file-version MyAddIn.xll

So maybe we just change the C++ project to include a new resource to "dna_version" which by default is

  1. In addition to set the .XLL version resource with the version from an assembly, we should also create a separate resource in the .XLL to store the ExcelDna version which is valuable info. Ideally we should have a way to query this hidden version without having to install a separate resource viewer, etc. Maybe something like:

ExcelDnaPack.exe show-file-version MyAddIn.xll
ExcelDnaPack.exe show-dna-version MyAddIn.xll


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHubhttps://github.com//issues/15#issuecomment-220188475

@augustoproiete
Copy link
Member Author

I'll have to think about the ExcelDna version. Maybe it can just be a comment or description written in the regular version resource.

Indeed, looks like we have a few options to choose in the VERSIONINFO resource
https://msdn.microsoft.com/en-us/library/windows/desktop/aa381049(v=vs.85).aspx

image

We'll have to decide which one is best.

@konne
Copy link
Contributor

konne commented May 19, 2016

Hi augustoproiete,

you can just use that tool as a build task:
http://www.codeproject.com/Articles/37133/Simple-Version-Resource-Tool-for-Windows

All the other ways doesn't make sense in my eyes, because nobody want to compile the xll on every build again.

bye Konrad

@govert
Copy link
Member

govert commented May 19, 2016

@konne The idea is certainly not to recompile the .xll. It's just about updating the embedded version resource, similar to how ExcelDnaPack updates embedded resources.

Something that would work in a post-build step at the moment is using a powershell command or something to read the version from the managed assembly, and pipe that to the verpatch tool.

I think @augustoproiete and @davidandersen are just looking for something a bit more elegant to set up in their (many?) add-ins.

@augustoproiete
Copy link
Member Author

@konne: What @govert said.

@konne
Copy link
Contributor

konne commented May 19, 2016

@augustoproiete and @govert
If you look to the source of the verpatch you see that is much more complex to update the Versionattributes ond a exe because there are no Win32 APIs for it.
So I see only the possibility to add this feature if somebody takes the time and implemnet first the verpatch.exe in C# and than it can be easily added into dnapack.

Then next issues is how the dll is defined who contains the version infos. For example I pack all in all 20 dlls into the xll. What are your ideas?

@augustoproiete
Copy link
Member Author

@konne

If you look to the source of the verpatch you see that is much more complex to update the Versionattributes ond a exe because there are no Win32 APIs for it

Interesting. I haven't looked at the source of verpatch until you mentioned it, and I thought it was simpler (i.e. calling a Win32 API or two), but you're right, it will def. be tricky to implement all that in C#.

Assuming there is indeed no Win32 API to do that, maybe the easiest thing for us would be to absorb the verpatch code in C++ as is, but turn it into a Win32 DLL, so that we can call via P/Invoke from ExcelDnaPack (or the MSBuild task down the road).

We can include this "verpatch.dll" in the NuGet package, or we can embed it in the ExcelDnaPack executable (or, again, the MSBuild task down the road).

Then next issues is how the dll is defined who contains the version infos. For example I pack all in all 20 dlls into the xll. What are your ideas?

I was thinking about adding a new attribute (name to be defined) to the "DnaLibrary" tag, that would tell the MSBuild task to use that assembly to read version information.

e.g.

<DnaLibrary Name="MyAwesomeExcelAddIn" UseVersionFrom="MyXLAddInAssembly.dll">
    <ExternalLibrary Path="MyXLAddInAssembly.dll" LoadFromBytes="true" />
</DnaLibrary>

@konne
Copy link
Contributor

konne commented May 19, 2016

thing for us would be to absorb the verpatch code in C++ as is, but turn it into a Win32 DLL
good Idea, can you do this or govert? I'm fit in C# but have no experience in c++ dlls.

good Idea, @govert what do you think?

If you provide me the verpatch as dll, I can update the ExcelDNAPack

@govert
Copy link
Member

govert commented May 19, 2016

I can't image it being hard to do in C# - ExcelDnaPack does pretty much the same thing with updating resources already.

I'd certainly not want that part to be in C++, or incorporate code from the verpatch project.

What about an attribute on the ExternalLibrary (and we use the first/last):

<DnaLibrary Name="MyAwesomeExcelAddIn">
    <ExternalLibrary Path="MyXLAddInAssembly.dll" LoadFromBytes="true" AddInVersion="true" />
</DnaLibrary>

That way we don't have to repeat the name?

@davidandersen
Copy link

@govert -- I like that. That makes sense.

For what it's worth all that we're doing right now is calling a post-build executable that wraps verpatch and we add that to the end of the post-build events in Visual Studio. It looks something like below.

"$(BuildRoot)Tools\ExcelDnaPostBuild.exe" -p "$(TargetDir)SomethingCooler-AddIn64-packed.xll" -v "$(TargetDir)SomethingCool.dll" -n "Something Different"

Where:

-p -- the packed .xll for which we want to update version
-v -- the file from which to pull the version information
-n -- (optional) an alternative name for the final packed .xll

In the end, the above creates a new .xll named Something Different.xll which is a copy of SomethingCooler-AddIn64-packed.xll, but with version information from SomethingCool.dll.

This is pretty ugly and not as elegant as having Excel DNA taking care of this natively, but it does the trick.

btw -- this is really great stuff.

Thanks.

@govert
Copy link
Member

govert commented May 19, 2016

Here's one example of a C# library for updating version (and other)
resources: https://github.com/dblock/resourcelib

On the other hand, I find the one-line solution of @davidandersen rather attractive. Maybe we just need to document it somewhere?

@konne
Copy link
Contributor

konne commented May 19, 2016

@govert thanks for the lib.
Do you have a problem if I integrate this into exceldnapack and make ar PR?

I don't like the other approach to have a seperate tool (like davidandersen)

@davidandersen
Copy link

@govert / @konne -- I'm not a huge fan of what we're currently doing either, and would much rather have it native to ExcelDnaPack.

On a related note, one thing that I think would be nice is to move all of the post-build events to a single executable, perhaps named something like ExcelDnaPostBuild, and requiring minimal inputs.

@govert
Copy link
Member

govert commented May 19, 2016

@davidandersen The work that @augustoproiete has done in #27 on the build is the first step to an improved build story, removing the post-build event and replacing with custom build targets with tasks in the new ExcelDna.AddIn.Tasks library. It's probably the right place for this too.

@fandrei
Copy link
Contributor

fandrei commented Dec 7, 2016

I wonder where can I get this ExcelDnaPostBuild.exe tool? Or is it already superseded?

@govert
Copy link
Member

govert commented Dec 7, 2016

The only update I have on this issue is that the new post-build approach and its ExcelDna.AddIn.Tasks library are now in the master branch (and the v 0.34 beta test). That doesn't (yet) contain anything like the version updating.

@augustoproiete
Copy link
Member Author

This issue is partially resolved with PR #114.

It introduces a new (optional) attribute in the ExternalLibrary element in the XML structure inside each .dna called UseVersionAsOutputVersion that allows the user to define which assembly to copy the version to the XLL.

It also updates the XLL file resulting from running ExcelDnaPack to have the same version as the assembly indicated in the .dna file.


The only thing left to close this issue, is to perform the same operation above as part of the build, so that the XLL files copied to the output folder also have the version of the assembly indicated (if any) - when the user is not using ExcelDnaPack

@fandrei
Copy link
Contributor

fandrei commented Mar 27, 2017

@augustoproiete, cool. Is this feature included in the build 0.34.4-rc3?

@augustoproiete
Copy link
Member Author

augustoproiete commented Mar 28, 2017

@fandrei Yes, it is included. The NuGet package 0.34.4-rc3 ships with ExcelDnaPack 0.34.4.1, which includes the code for copying the version, introduced in pull-request associated with this issue.

Let us know if you find any bugs!

@fandrei
Copy link
Contributor

fandrei commented Mar 28, 2017

@augustoproiete, unfortunately I can't get it working. I've added a UseVersionAsOutputVersion="true" attribute to ExternalLibrary of my primary assembly, but the resulting packed xll doesn't contain any version number.

@govert
Copy link
Member

govert commented Mar 28, 2017

@fandrei How are you checking?
The .xll won't directly show a version if you show its properties. You need to rename to a .dll first, then check the properties. If the UseVersionAsOutputVersion... did not work, you'll still see the Excel-DNA version info on the .xll.

@fandrei
Copy link
Contributor

fandrei commented Mar 28, 2017

@govert

You need to rename to a .dll first, then check the properties.

Ah I see, thanks.

@govert govert closed this as completed Jan 18, 2018
@augustoproiete
Copy link
Member Author

@govert As per my comment above, the UseVersionAsOutputVersion feature is only available for users using ExcelDnaPack, so we still need to implement the same thing for the regular .XLL copied to the output folder as part of the build.

We should either re-open this issue or create a new one to track that.

@govert
Copy link
Member

govert commented Jan 19, 2018

@augustoproiete OK - I'll reopen it, but the packing case seemed like the important one.

@govert govert reopened this Jan 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants