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

Disable autodownload by default #2953

Merged
merged 34 commits into from
Sep 22, 2021
Merged
Show file tree
Hide file tree
Changes from 11 commits
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
23 changes: 20 additions & 3 deletions .ci/linux-steps.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ steps:
inputs:
versionSpec: '3.7'

# Install build dependencies
# Install build dependencies.
- script: |
git clone --depth 1 https://github.com/mlpack/jenkins-conf.git conf

Expand Down Expand Up @@ -42,9 +42,26 @@ steps:
sudo make install && \
cd ..

# Install ensmallen.
wget https://ensmallen.org/files/ensmallen-latest.tar.gz
tar -xvzpf ensmallen-latest.tar.gz # Unpack into ensmallen-*/.
cd ensmallen-* && \
sudo cp -vr include/* /usr/include/ && \
cd ..

# Install STB.
wget https://mlpack.org/files/stb.tar.gz
tar -xvzpf stb.tar.gz # Unpack into stb/.
cd stb &&\
sudo cp -vr include/* /usr/include/ && \
cd ..

# Install cereal.
wget https://github.com/USCiLab/cereal/archive/v1.3.0.tar.gz
tar -xvzpf v1.3.0.tar.gz # Unpack into cereal-1.3.0/.
tar -xvzpf v1.3.0.tar.gz # Unpack into cereal-1.3.0/.
cd cereal-1.3.0 && \
sudo cp -vr include/cereal /usr/include/ && \
cd ..

displayName: 'Install Build Dependencies'

Expand All @@ -56,7 +73,7 @@ steps:
export GOPATH=$PWD/src/mlpack/bindings/go
go get -u -t gonum.org/v1/gonum/...
fi
cmake $(CMakeArgs) -DPYTHON_EXECUTABLE=`which python` -DCEREAL_INCLUDE_DIR=../cereal-1.3.0/include/ ..
cmake $(CMakeArgs) -DPYTHON_EXECUTABLE=`which python` -DCEREAL_INCLUDE_DIR=/usr/include/ ..
displayName: 'CMake'

# Build mlpack
Expand Down
9 changes: 8 additions & 1 deletion .ci/macos-steps.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ steps:
set -e
sudo xcode-select --switch /Applications/Xcode_12.2.app/Contents/Developer
unset BOOST_ROOT
brew install libomp openblas armadillo boost cereal
brew install libomp openblas armadillo boost cereal ensmallen
shrit marked this conversation as resolved.
Show resolved Hide resolved

if [ "$(binding)" == "python" ]; then
pip install --upgrade pip
Expand All @@ -25,6 +25,13 @@ steps:
brew install --cask julia
fi

# Install STB.
wget https://mlpack.org/files/stb.tar.gz
tar -xvzpf stb.tar.gz # Unpack into stb/.
cd stb &&\
sudo cp -vr include/* /usr/include/ && \
shrit marked this conversation as resolved.
Show resolved Hide resolved
cd ..

git clone --depth 1 https://github.com/mlpack/jenkins-conf.git conf
displayName: 'Install Build Dependencies'

Expand Down
3 changes: 2 additions & 1 deletion .ci/windows-steps.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ steps:
nuget install boost -o $(Agent.ToolsDirectory) -Version 1.60.0
nuget install boost_random-vc140 -o $(Agent.ToolsDirectory) -Version 1.60.0
nuget install boost_math_c99-vc140 -o $(Agent.ToolsDirectory) -Version 1.60.0
nuget install OpenBLAS -o $(Agent.ToolsDirectory)
rcurtin marked this conversation as resolved.
Show resolved Hide resolved
nuget install unofficial-flayan-cereal -o $(Agent.ToolsDirectory)
nuget install ensmallen:x64-windows -o $(Agent.ToolsDirectory)
shrit marked this conversation as resolved.
Show resolved Hide resolved

mkdir -p $(Agent.ToolsDirectory)/boost_libs
cp $(Agent.ToolsDirectory)/boost_math_c99-vc140.1.60.0.0/lib/native/address-model-64/lib/*.* $(Agent.ToolsDirectory)/boost_libs
rcurtin marked this conversation as resolved.
Show resolved Hide resolved
Expand Down Expand Up @@ -62,6 +62,7 @@ steps:
-DBOOST_INCLUDEDIR=$(Agent.ToolsDirectory)\boost.1.60.0.0\lib\native\include `
Copy link
Member

Choose a reason for hiding this comment

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

What happens if you change \include to \tmp\include for ARMADILLO_INCLUDE_DIR?

Copy link
Member

Choose a reason for hiding this comment

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

Or maybe you can just leave ARMADILLO_LIBRARY out?

-DBOOST_LIBRARYDIR=$(Agent.ToolsDirectory)\boost_libs `
-DCEREAL_INCLUDE_DIR=$(Agent.ToolsDirectory)\unofficial-flayan-cereal.1.2.2\build\native\include `
-DENSMALLEN_INCLUDE_DIR=$(Agent.ToolsDirectory)\ensmallen.2.16.2\include`
shrit marked this conversation as resolved.
Show resolved Hide resolved
-DBUILD_JULIA_BINDINGS=OFF `
-DCMAKE_BUILD_TYPE=Release ..
displayName: 'Configure mlpack'
Expand Down
2 changes: 1 addition & 1 deletion CMakeLists.txt
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(BUILD_GO_SHLIB "Build Go shared library." OFF)
option(BUILD_DOCS "Build doxygen documentation (if doxygen is available)." ON)

Expand Down