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

The Autodownloader. #2927

Merged
merged 2 commits into from Apr 26, 2021
Merged

The Autodownloader. #2927

merged 2 commits into from Apr 26, 2021

Conversation

shrit
Copy link
Member

@shrit shrit commented Apr 24, 2021

This pull request provides the Autodownloader for mlpack dependencies.
It can be used with current and future dependencies.
This one is born from working with #2531, with help from @rcurtin and @zoq.

It is ready to merge, with minor reviews are always welcome.

This pull request provides the Autodownloader for mlpack dependencies.
It can be used with current and future dependencies.
This one is born from working with mlpack#2531, with help from @rcurtin and @zoq.

It is ready to merge, with minor reviews are always welcome.

Signed-off-by: Omar Shrit <omar@shrit.me>
Signed-off-by: Omar Shrit <omar@shrit.me>
Copy link
Member

@zoq zoq left a comment

Choose a reason for hiding this comment

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

Looks good to me, thanks for putting this together 👍

Copy link

@mlpack-bot mlpack-bot bot left a comment

Choose a reason for hiding this comment

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

Second approval provided automatically after 24 hours. 👍

Copy link
Member

@rcurtin rcurtin left a comment

Choose a reason for hiding this comment

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

Nice, looks good! 👍

@rcurtin rcurtin merged commit 1f74685 into mlpack:master Apr 26, 2021
Copy link
Member

@Yashwants19 Yashwants19 left a comment

Choose a reason for hiding this comment

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

Ahh, I guess I am late. 😅

else()
find_package(Armadillo "${ARMADILLO_VERSION}")
if (NOT ARMADILLO_FOUND)
get_deps(http://files.mlpack.org/armadillo-10.3.0.tar.gz armadillo armadillo-10.3.0.tar.gz)
Copy link
Member

Choose a reason for hiding this comment

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

I am not sure, But I guess we can maintained a copy of the latest armadillo tarball(armadillo-latest.tar.gz) same as ensmallen at our website. Let me know what you think??

else()
find_package(cereal "${CEREAL_VERSION}")
if (NOT CEREAL_FOUND)
get_deps(https://github.com/USCiLab/cereal/archive/refs/tags/v1.3.0.tar.gz cereal cereal-1.3.0.tar.gz)
Copy link
Member

Choose a reason for hiding this comment

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

Can we some how fetch the latest version of cereal before downloading it???

else()
find_package(Boost "${BOOST_VERSION}")
if (NOT Boost_FOUND)
get_deps(https://dl.bintray.com/boostorg/release/1.75.0/source/boost_1_75_0.tar.gz boost boost_1_75_0.tar.gz)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
get_deps(https://dl.bintray.com/boostorg/release/1.75.0/source/boost_1_75_0.tar.gz boost boost_1_75_0.tar.gz)
list(GET Boost_ADDITIONAL_VERSIONS 0 Boost_LATEST_VERSION_1)
string(REPLACE "." "_" Boost_LATEST_VERSION_2 ${Boost_LATEST_VERSION_1})
get_deps(https://dl.bintray.com/boostorg/release/${Boost_LATEST_VERSION_1}/source/boost_${Boost_LATEST_VERSION_2}.tar.gz boost boost_${Boost_LATEST_VERSION_2}.tar.gz)

As latest version of boost is maintained by actions, we can rely on Boost_ADDITIONAL_VERSIONS, and Hence we can get rid of updating the version of boost regularly here. Let me know what you think???

@rcurtin
Copy link
Member

rcurtin commented Apr 26, 2021

Ahh, I guess I am late. 😅

Sorry about that! I guess I hit merge too fast 😄

@shrit
Copy link
Member Author

shrit commented Apr 26, 2021

@rcurtin @Yashwants19 no worries I can open another pull request to change later to the latest version for cereal and armadillo, but we need to host the last version on mlpack.org.
Considering boost, frankly, it does not worth the modifications since it will be removed in a couple of months.
Let me know what do you think

This was referenced Oct 14, 2022
@rcurtin rcurtin mentioned this pull request Oct 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants