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.

Sign up for GitHub

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

Jump to bottom

Disable autodownload by default #2953

Merged
merged 34 commits into from
Sep 22, 2021
Merged

Disable autodownload by default #2953

Changes from 1 commit
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
a6de026
Disable autodownload be default
shrit May 24, 2021
ce4ac37
Download deps for windows and linux
shrit May 27, 2021
228d8bb
Add ensmallen to macos
shrit May 27, 2021
c64f338
Merge branch 'master' into disable_autodownloader
shrit Jun 16, 2021
453df1d
Fix the cereal download and installation
shrit Jun 16, 2021
a5cbc72
Fix linux cereal installation
shrit Jun 17, 2021
d9f995f
Installing ensmallen and STB manually
shrit Jun 20, 2021
781e146
Setup the path for cereal
shrit Jun 20, 2021
6692049
Update .ci/linux-steps.yaml
shrit Jun 23, 2021
d5ad6a3
Update .ci/linux-steps.yaml
shrit Jun 23, 2021
e4b328e
Let us try those already and see what will happen
shrit Jun 24, 2021
1d10f4a
Let us see if we can install ensmallen
shrit Jun 29, 2021
4d19b40
Specify the version of ensmallen
shrit Aug 7, 2021
1267024
Use the entire word
shrit Aug 7, 2021
2229767
Fix ensmallen Path
shrit Aug 7, 2021
fce8b6b
Fix missing space and macoS path
shrit Aug 9, 2021
bea7057
Fix STB and tree ensmallen
shrit Aug 10, 2021
a195608
Fix ensmallen Path
shrit Aug 10, 2021
9f54a4b
Again...
shrit Aug 10, 2021
daaea02
Change the name of the dependency option
shrit Aug 16, 2021
c01f61f
Print armadillo libraries to see where are they
shrit Aug 20, 2021
3e21add
Try tmp/include instead of include/
shrit Aug 31, 2021
a59f1f4
Merge branch 'master' into disable_autodownloader
shrit Sep 7, 2021
45a67d0
Merge branch 'master' into disable_autodownloader
shrit Sep 7, 2021
612b0ab
Extract correctly armadillo
shrit Sep 7, 2021
98e7467
Delete packaged libraries, not required for windows
shrit Sep 8, 2021
24edc54
Fix the remove command in power shell
shrit Sep 8, 2021
379b03e
Use tmp\include dir in order to find armadillo lib
shrit Sep 16, 2021
6dc9c7c
Let us verify if armadillo has gone
shrit Sep 19, 2021
3ee6afa
Remove armadillo headers since they might be causing the issue
shrit Sep 19, 2021
71924f8
Remove all aramdillo
shrit Sep 20, 2021
681e4ea
Update .ci/windows-steps.yaml
shrit Sep 21, 2021
f316e2d
Update CMakeLists.txt
shrit Sep 21, 2021
435bfad
Update CMakeLists.txt
shrit Sep 21, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ option(ARMA_EXTRA_DEBUG "Compile with extra Armadillo debugging symbols." OFF)
option(TEST_VERBOSE "Run test cases with verbose output." OFF)
option(BUILD_TESTS "Build tests." ON)
option(BUILD_CLI_EXECUTABLES "Build command-line executables." ON)
option(DISABLE_DOWNLOADS "Disable downloads of dependencies during build." OFF)
option(DISABLE_DOWNLOADS "Disable downloads of dependencies during build." ON)
shrit marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

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

The use of a negative option here can be confusing -- using "ON" to actually disable something.

For ease of understanding (and easier maintenance) it's generally best to have a simple on/off for a given functionality. I recommend to change the option to ENABLE_DOWNLOADS or more accurately DOWNLOAD_DEPENDENCIES.

Using "ON" would then clearly mean to enable downloading the dependencies.

Copy link
Member

Choose a reason for hiding this comment

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

Agreed, this would be a nice improvement. 👍

option(DOWNLOAD_ENSMALLEN "If ensmallen is not found, download it." ON)
option(DOWNLOAD_STB_IMAGE "Download stb_image for image loading." ON)
option(BUILD_GO_SHLIB "Build Go shared library." OFF)
Expand Down