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

Fix Windows Builds #7

Merged
merged 12 commits into from
Nov 23, 2021
Merged

Fix Windows Builds #7

merged 12 commits into from
Nov 23, 2021

Conversation

wfondrie
Copy link
Collaborator

This PR adds the Thermo MSFileReader installation to our Windows build. Currently it is a draft while I figure it out.

@mammmals
Copy link
Collaborator

Looks like a new certificate issue. Thoughts @wfondrie?

@wfondrie
Copy link
Collaborator Author

wfondrie commented Nov 22, 2021

I was just about to ask you and @jke000. Unfortunately I don't have much experience with building things with Visual Studio.

Here are the two errors I'm seeing:

C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\MSBuild\Current:\Bin\Microsoft.Common.CurrentVersion.targets(3326,5): error MSB3326: Cannot import the following key file: . The key file may be password protected. To correct this, try to import the certificate again or import the certificate manually into the current user’s personal certificate store. [D:\a\Comet\Comet\CometUI\CometUI.csproj]

C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\MSBuild\Current\Bin\Microsoft.Common.CurrentVersion.targets(3326,5): error MSB3321: Importing key file "CometUI.pfx" was canceled. [D:\a\Comet\Comet\CometUI\CometUI.csproj]

Any suggestions you have would be great!

@mammmals
Copy link
Collaborator

mammmals commented Nov 22, 2021

Hmm, looks like it's only an issue for the CometUI. We could potentially just ignore that in the MSBuild and only build the CLI exe for now?

@wfondrie
Copy link
Collaborator Author

Maybe - I'll see if I can configure it that way 🤔

@mammmals
Copy link
Collaborator

@mammmals
Copy link
Collaborator

mammmals commented Nov 22, 2021

Looks like the certificates are line 79-91 in https://github.com/UWPR/Comet/blob/master/CometUI/CometUI.csproj
Not sure if this would have other issues though...

@mhoopmann
Copy link
Collaborator

mhoopmann commented Nov 22, 2021

Try disabling the ClickOnce security setting in the Solution properties for CometUI.

image

image

@wfondrie
Copy link
Collaborator Author

Do you know how to do this either in the code or from the command line? I'm on a Mac.

Like @mammmals mentioned, it seems like changing the following to false would accomplish this:

<SignManifests>true</SignManifests>

@mhoopmann
Copy link
Collaborator

Sorry, no.

@jke000
Copy link
Collaborator

jke000 commented Nov 22, 2021

@wfondrie, instead of building the entire solution, are you able to just build individual projects? If so, simply build the Comet project to get Comet.exe. If not, I can explore ClickOnce certificates more to see if it can be generated without the password.
Or if any workaround is working, that's cool also.

@wfondrie
Copy link
Collaborator Author

It wouldn't be a problem to only build Comet.exe. However, I was able to make the change above---disabling SignManifests--- and now everything works. What do you think is the better solution?

@jke000
Copy link
Collaborator

jke000 commented Nov 22, 2021

If it's working now, leave as-is. Thanks!

@wfondrie wfondrie marked this pull request as ready for review November 22, 2021 23:15
@wfondrie wfondrie requested a review from mammmals November 22, 2021 23:16
Copy link
Collaborator

@mammmals mammmals left a comment

Choose a reason for hiding this comment

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

Looks good, we can do Compress-Archive if needed as well for the zip.
There's GH specific release actions as well, but now that we've got it working let's get it to the main repo.

@mammmals mammmals merged commit e114c7d into master Nov 23, 2021
@wfondrie wfondrie deleted the windows-build branch November 23, 2021 00:19
@wfondrie
Copy link
Collaborator Author

Yeah hopefully those release actions work as expected 🤞.

Unfortunately I don't know of a great way to test them, aside from creating a new release and seeing if it works. The good news is that I have used similar actions before, so I'm fairly sure these will work.

@mammmals
Copy link
Collaborator

Looks like the upload release assets isn't working for some reason.

@mammmals
Copy link
Collaborator

Very exciting, first run failed for a tag issue, but latest test worked like a charm! @wfondrie could you test on a mac/linux?

@wfondrie
Copy link
Collaborator Author

I just tried the MacOS and Linux versions and they seem to work 👍

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.

4 participants