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

Add github actions compile test #131

Merged
merged 15 commits into from
Sep 9, 2024
Merged

Add github actions compile test #131

merged 15 commits into from
Sep 9, 2024

Conversation

will-v-pi
Copy link

Add compilation test across variations of OS, generator, and with/without Libusb, MbedTLS, and pre-compiled xip-ram-perms and flash-id executables

@will-v-pi will-v-pi added this to the 2.0.1 milestone Sep 2, 2024
@ndabas
Copy link

ndabas commented Sep 2, 2024

If you wanted to be a bit more minimal about the Windows build, the Windows runner already has a bunch of stuff installed so you don't need to install the choco packages:

  • CMake is preinstalled.
  • MinGW is likely not needed? It seems you're just using bash at some point but PowerShell could be used too. If you're installing ninja anyway you wont need make either.
    • msys2 is preinstalled (but not added to PATH) if you really need it.
  • 7zip is preinstalled.
  • You could use curl instead of wget because curl is included with Windows anyway.

@will-v-pi
Copy link
Author

  • CMake is preinstalled.

  • MinGW is likely not needed? It seems you're just using bash at some point but PowerShell could be used too. If you're installing ninja anyway you wont need make either.

    • msys2 is preinstalled (but not added to PATH) if you really need it.

The reason for installing CMake and MinGW from chocolatey is so we have known compilers, and a know installation process that we can suggest to users, rather than just using the versions built into GitHub actions runners

  • 7zip is preinstalled.
  • You could use curl instead of wget because curl is included with Windows anyway.

Thanks for these suggestions, I will switch the action to use curl and the pre-installed 7zip

@lurch
Copy link
Contributor

lurch commented Sep 2, 2024

Looks like the pre-installed 7zip is causing the Windows tests to fail? 😕

@ndabas
Copy link

ndabas commented Sep 2, 2024

Looks like the pre-installed 7zip is causing the Windows tests to fail? 😕

I guess you need to call curl as curl.exe on Windows because curl is an alias to Invoke-WebRequest or something silly in PowerShell by default. I mean I think it's not the 7zip that's failing, it's the curl before that.

@will-v-pi
Copy link
Author

I guess you need to call curl as curl.exe on Windows because curl is an alias to Invoke-WebRequest or something silly in PowerShell by default. I mean I think it's not the 7zip that's failing, it's the curl before that.

I agree it looks like the curl that's failing, but it looks like it is invoking curl.exe, as Invoke-WebRequest doesn't give the % Total % Received etc printout.

@ndabas
Copy link

ndabas commented Sep 2, 2024

I guess you need to call curl as curl.exe on Windows because curl is an alias to Invoke-WebRequest or something silly in PowerShell by default. I mean I think it's not the 7zip that's failing, it's the curl before that.

I agree it looks like the curl that's failing, but it looks like it is invoking curl.exe, as Invoke-WebRequest doesn't give the % Total % Received etc printout.

Yes, you're right, it is invoking curl, I guess you just need a flag to allow redirects with curl.

@will-v-pi
Copy link
Author

Yes, you're right, it is invoking curl, I guess you just need a flag to allow redirects with curl.

Thanks, that's it - I'll use curl -L

@ndabas
Copy link

ndabas commented Sep 2, 2024

The reason for installing CMake and MinGW from chocolatey is so we have known compilers, and a know installation process that we can suggest to users, rather than just using the versions built into GitHub actions runners

On that topic, unrelated to this particular PR, it might make sense to write a small PowerShell script that installs these dependencies the same way the VS Code extension does? Basically just use the manifest files that are used there, download and add to PATH for command line builds. I see the SDK, examples, and now this (picotool) have copies of this choco-based install, which are not exactly the same; and one of the major differences from the VS Code ext is the ARM GCC specified for choco is much older (even pico-setup-windows had a newer build.)

@will-v-pi will-v-pi merged commit 3ff7c3e into develop Sep 9, 2024
31 checks passed
@will-v-pi will-v-pi deleted the actions-test branch September 9, 2024 10:08
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.

3 participants