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 windows-specific build instructions to readme #1742

Merged
merged 1 commit into from
Jul 4, 2023

Conversation

benjaminwinger
Copy link
Collaborator

Added a short description of additional requirements for building on windows to the README (see #1736 (comment)).

@codecov
Copy link

codecov bot commented Jun 29, 2023

Codecov Report

Patch coverage has no change and project coverage change: -0.01 ⚠️

Comparison is base (5cd883c) 91.42% compared to head (715a9fd) 91.42%.

❗ Current head 715a9fd differs from pull request most recent head cc02b38. Consider uploading reports for the commit cc02b38 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1742      +/-   ##
==========================================
- Coverage   91.42%   91.42%   -0.01%     
==========================================
  Files         766      766              
  Lines       27509    27509              
==========================================
- Hits        25151    25149       -2     
- Misses       2358     2360       +2     

see 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@benjaminwinger
Copy link
Collaborator Author

I amended the note about the visual studio build since just running make arrow won't also configure the main cmake build to use ninja. Actually building isn't necessary since visual studio will build in a separate directory, but it's the simplest way to do it at the moment, even if it's somewhat slow. I feel like further details like this about the build process could be helpful, but it would probably be better to have dedicated build documentation and keep the section in the readme short and simple.

@LowLevelMahn
Copy link

LowLevelMahn commented Jun 29, 2023

what "Windows" Make should be used for Windows?

the 2006 GnuWin32 Version: https://gnuwin32.sourceforge.net/packages/make.htm
or from this project: https://github.com/mbuilov/gnumake-windows

Make is very uncommon on Windows so it would be good to state whats a good source for the binary

@benjaminwinger
Copy link
Collaborator Author

That version might work, depending on the differences between the versions. I don't think it's been tested with that old of a version of make.

What we've used in CI is the version packaged by chocolatey: https://community.chocolatey.org/packages/make. It looks like the person who packaged that is distributing the binaries themselves along with the package (the exe can be found here).

@LowLevelMahn
Copy link

What we've used in CI is the version packaged by chocolatey

could you add that info+url to the short build description?

@benjaminwinger
Copy link
Collaborator Author

could you add that info+url to the short build description?

Sure. Or rather, I updated it to link chocolatey and listed the package names in an example install command, since that also includes ninja and might be a little more straightforward than linking the chocolatey packages individually. Is that sufficient?

@LowLevelMahn
Copy link

could you add that info+url to the short build description?

Sure. Or rather, I updated it to link chocolatey and listed the package names in an example install command, since that also includes ninja and might be a little more straightforward than linking the chocolatey packages individually. Is that sufficient?

yes - seems to be more "complete" then

@benjaminwinger benjaminwinger merged commit 7684461 into master Jul 4, 2023
9 checks passed
@benjaminwinger benjaminwinger deleted the windows-readme-update branch July 4, 2023 19:22
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.

None yet

3 participants