Skip to content
This repository has been archived by the owner on May 13, 2024. It is now read-only.

Update Windows README + gitignore test executable #29

Merged
merged 3 commits into from
Nov 13, 2019

Conversation

pamtaro
Copy link
Contributor

@pamtaro pamtaro commented Nov 9, 2019

Checklist

🚨Please review the guidelines for contributing to this repository.

  • 🤔 CONSIDER adding a unit test if your PR resolves an issue.
  • DO check open PR's to avoid duplicates.
  • DO keep pull requests small so they can be easily reviewed.
  • DO build locally before pushing.
  • DO make sure tests pass.
  • DO make sure any new changes are documented.
  • DO make sure not to introduce any compiler warnings.
  • AVOID breaking the continuous integration build.
  • AVOID making significant changes to the overall architecture.

Description

Updated documentation to run on Windows. If CMAKE was never previously installed on the Windows machine (either from pacman inside of MSYS or in the Windows side from cmake.org), then using the cmake commands with -G "MSYS Makefiles" will fail with Error: Could not create named generator MSYS Makefiles. This adds that package to be installed via pacman during the initial setup of MSYS2. This also updates the cmake command's arguments to specify the source and build directories from the root of the project and run make through cmake for consistency. This PR also gitignores the simple.exe that is generated from by the example build.

.gitignore Outdated
ElectionGuardConfig.cmake
simple.exe
Copy link
Contributor

Choose a reason for hiding this comment

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

Good addition!

Copy link
Contributor

Choose a reason for hiding this comment

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

👉 hide whole simple folder

Copy link
Contributor

@keithrfung keithrfung left a comment

Choose a reason for hiding this comment

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

Leaving review up to @echumley-msft , but left comments to help along.

cmake -G "MSYS Makefiles" ..
make
cmake -S . -B build -G "MSYS Makefiles" ..
cmake --build build
Copy link
Contributor

Choose a reason for hiding this comment

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

Good addition, the original commands were using an older version of cmake and building with make.
The newer version allows for autocreation of the folder.


3. You should now have a `libelectionguard.a`.
3. You should now have a `electionguard.a` or `electionguard.dll` (depending on the how cmake was configured).
Copy link
Contributor

Choose a reason for hiding this comment

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

The commands listed in Step 2 should only build an artifact (.a).
It would only build a .dll if -DBUILD_SHARED_LIBS=ON is passed with command.

cmake -G "MSYS Makefiles" ..
make
cmake -S examples/simple -B simple_build -G "MSYS Makefiles"
cmake --build simple_build --target simple
Copy link
Contributor

Choose a reason for hiding this comment

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

Good improvement.

@keithrfung keithrfung added this to the Phase 3 milestone Nov 11, 2019
@echumley-msft echumley-msft merged commit 0a878be into Election-Tech-Initiative:master Nov 13, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants