pact-foundation / pact-net

.NET version of Pact. Enables consumer driven contract testing, providing a mock service and DSL for the consumer project, and interaction playback and verification for the service provider project.
https://pact.io
MIT License
848 stars 234 forks source link

Added Msbuild script to download pact_ffi lib #490

Closed tmulkern closed 9 months ago

tmulkern commented 9 months ago

Hey there, I justed added Download.Native.Libs.targets, it downloads the correct pact_ffi lib needed and is part of the build process instead of using download-native-libs.sh seperatily

The Download.Native.Libs.targets contains properties matching the variables in download-native-libs.sh, some renamed as to not conflict with defined MsBuild properties and some derived from properties in the main PacNet proj file. It also contains five tasks

tmulkern commented 9 months ago

Hi @adamrodger ,

I agree with some points and disagree with others.

Firstly, when developing sometimes I have to use different versions of the FFI in order to try out new features or test when things are fixed, and this adds a layer of opacity to making that work.

That is as easy as changing the version in the FfiVersion property in the Download.Native.Libs.targets file, deleting the folder in the build directory and running build in Visual Studio. This IMO is not different to what it currently is today with the script.

  1. Change version number
  2. Delete folder/files
  3. Run script

Secondly, the Bash script is super simple and easy to audit, whereas MSBuild is much more complicated, as shown by having to embed C# within etc. I strongly disagree with the Bash script as being super 'simple', there are additional tools that need to be installed to make it work (gunzip and MinGW/WSL in the case of windows), the use of sed IMO does not make the easiest to audit.

MSBuild is as complicated as shell scripting is, however MSBuild scripting is native to Visual Studio and C# tooling in general and makes the solution cross platform, the Bash script is not the easiest to work with in Windows.

I will concede on the use of embed C# code in the MSBuild script, but this being a C# project I thought it would be ok

Thirdly, I dislike the concept that files are downloaded without the consent of the user, especially binary files which will be executed on the user's machine. The Bash script acts as a consent stage and the user is always free to disregard it and download the native libs themselves if they don't trust it. The MSBuild version happens opaquely without user consent.

The issue of user consent can be easy fixed, I made it part of the build because it will fail without the binary present in the correct folder

The best way to enact changes like this would be to raise an issue and start a discussion around potential approaches in order to avoid disappointment and wasting your time if the change you want doesn't align. Hopefully you found it interesting and learned some new stuff trying it out though so it's not a waste of time.

Very much happy to raise an issue and discuss this and no time wasted, this did not take me long to put together, I live in Visual Studio, MSBuild, Bash scripts.. it's my day job. for my sins :stuck_out_tongue_winking_eye:

adamrodger commented 9 months ago

Having just had to change the download script to deal with changes to how the FFI was packaged, I definitely prefer the shell script approach over the MSBuild approach, so I'm gonna stick with that.

Thanks for the contribution all the same.