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

Appveyor configuration for a Visual Studio 2015 build #64

Closed
wants to merge 1 commit into from

Conversation

haata
Copy link
Contributor

@haata haata commented Aug 18, 2016

  • Required adding a property sheet to pass C defines from the command line
  • Builds with libusb-win32 and libusbK
  • Builds 4 different permutations currently (x86, x64 /w Debug, Release)
  • Support for pushing tagged releases (only Release/Win32 currently) to GitHub Releases

Notes:

  • Appveyor also has Cygwin available so it's likely possible to setup some Cygwin builds as well
  • To fully utilize Appveyor you'll need to. The Win32 Release build seemed like the most useful overall, so I started there.
    • Create an Appveyor project for libwdi
    • Change the "Build Status" link in the README.md file (currently it points to my Appveyor project)
    • Add your own GitHub auth key (https://www.appveyor.com/docs/deployment/github/), mine won't work on your repo
  • Example of the README.md (https://github.com/haata/libwdi)
  • Example of GitHub Artifacts (https://github.com/haata/libwdi/releases), it's possible to add more files, platforms and/or add them to .zip files

- Required adding a property sheet to pass C defines from the command line
- Builds with libusb-win32 and libusbK
- Builds 4 different permutations currently (x86, x64 /w Debug, Release)
- Support for pushing tagged releases (only Release/Win32 currently) to GitHub Releases

Notes:
- Appveyor also has Cygwin available so it's likely possible to setup some Cygwin builds as well
@pbatard
Copy link
Owner

pbatard commented Aug 18, 2016

Thanks!

  • Required adding a property sheet to pass C defines from the command line

Makes sense. No issue there.

  • Builds with libusb-win32 and libusbK

Very nice! 😃

  • Builds 4 different permutations currently (x86, x64 /w Debug, Release)

That's great. But I think I'll want to add MinGW and MinGW-w64 builds, since this is what I use to produce the actual releases of Zadig for instance. I've done something like this for Rufus very recently, so hopefully that won't be too difficult. I'll probably add that later, after I've integrated your proposal.

  • Support for pushing tagged releases (only Release/Win32 currently) to GitHub Releases

This is the only item I have a small issue with. I cannot see myself releasing an application that requires elevation without it being digitally signed (which of course, is not something that can be automated, as it would require AppVeyor to have the private key for my signing cert). So pushing something like the Zadig executable built by AppVeyor for user consumption is a big NO_NO. It's not that I don't trust the AppVeyor servers, but you have to consider that, as soon as there exists an unsigned version of Zadig x.y.z in the open, anybody can create a malicious version that can pass as Zadig x.y.z, and if users fall into the habit of downloading and running unsigned versions, because they are officially published as artefacts, it will end up putting users at risk.

An app or library that installs drivers, and that you didn't build yourself, should always have a digital signature to validate its author. If it doesn't, you should not run it.

And I understand that it's possible to use artifacts to push code archives, but I think any developer who wants the latest source will be familiar enough with git already, so I don't see the need to create source archive for each commit.

  • Appveyor also has Cygwin available so it's likely possible to setup some Cygwin builds as well

I've deliberately dropped compatibility for Cygwin in libwdi a long time ago. It was just too much work to ensure the code was Cygwin compatible (From various projects, I found that Cygwin was always a lot more work compared to MinGW, and that if there's a chance for anything to break, it'll usually be Cygwin).

At any rate, thank you very much for this pull request. It looks fine for integration (I'll apply the changes I mentioned myself -- no need to do that, unless you really want to), and, just to give you an ETA, I think I'll be applying it within a week or so.

@pbatard pbatard self-assigned this Aug 18, 2016
@haata
Copy link
Contributor Author

haata commented Aug 19, 2016

Cool!
Yeah, this all started with me needing the zadic.exe binary, but not being available by the general release means. I'm generally on Linux, so it's usually a pain to get all this built in a convenient manner.
In my use case I'm just trying to automate the WinUSB assignment for a custom DFU device.

Feel free to pull-out anything. Part of this was to experiment with the GitHub Release system which I've never used before.

@pbatard pbatard closed this in 8e0a99b Aug 24, 2016
@pbatard
Copy link
Owner

pbatard commented Aug 24, 2016

Mmm, unfortunately, the SourceForge download services is really hit and miss, which means we get build failures simply because the SourceForge mirrors refuse to server downloadable content. This already happened for 2 builds... 😞

I'd have to have to create my own mirror of the libusbK and libusb-win32 files just so that AppVeyor can pass...

@haata
Copy link
Contributor Author

haata commented Aug 24, 2016

Yeah...I've noticed that as well. On the plus side, you can be sure that the binary doesn't magically change on you.

@pbatard
Copy link
Owner

pbatard commented Aug 24, 2016

Yeah...I've noticed that as well.

Please avoid leaving these kind of details out. If you know something may not work in a patch you're proposing, at least mention it, instead of leaving it for the committer to find it and address.

Since I have no time for dealing with all of SourceForge's bullshit (I moved my projects out of SF a long time ago, and never looked back), I have now placed the files on my own server.

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