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

Windows build #22

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Windows build #22

wants to merge 8 commits into from

Conversation

2-5
Copy link

@2-5 2-5 commented Dec 7, 2022

I'm trying to make Aim work on Windows, and I've managed to build aimrocks.

All the tests pass, except test_unicode_path. I tried fixing it, but somewhere between Python -> C Extension -> rocksdb the Unicode path name gets mangled despite Python supporting Unicode paths on Windows.

The big idea is using vcpkg to build rocksdb and all it's dependencies (zlib, ...) on Windows.

Build Instructions

Install a Windows C++ compiler

I used Visual Studio 2022 Community Edition (free). I'm not sure where the other Python packages with Windows binaries get the compiler and how they integrate it in the CI/CD.

Clone vcpkg

git clone https://github.com/Microsoft/vcpkg.git

Checkout supported version of rocksdb

The current version (HEAD) of vcpkg has rocksdb-7.7.3 which is incompatible with aimrocks. vcpkg was updated to use this version on 30 Dec 2021, so we need to use a vcpkg snapshot before that, which has rocksdb-6.27.3

cd vcpkg
git checkout tags/2022.11.14

Bootstrap vcpkg

-disableMetrics disables telemetry.

cd vcpkg
bootstrap-vcpkg.bat -disableMetrics

Build rocksdb and dependencies

We build the static library version of everything (x64-windows-static-md) so that we don't have .DLLs to manage. The -md at the end uses the DLL version of the Microsoft C Runtime (just like Python).

cd vcpkg
vcpkg install rocksdb[bzip2,lz4,snappy,zlib,zstd]:x64-windows-static-md

Clone aimrocks

Clone next to the vcpkg directories, so they are siblings:

/workdir
  /aimrocks
  /vcpkg
git clone https://github.com/aimhubio/aimrocks.git

Create virtual env

cd aimrocks
py -3.10 -m venv venv
venv\Scripts\activate.bat

Install required Python packages

python -m pip install -U pip setuptools wheel

pip install Cython==3.0.0.a11
pip install pytest

Build and install aimrocks

python setup.py build
python setup.py install

Run tests

pytest tests

@gorarakelyan
Copy link

@2-5 this is huge! Running Aim on windows has been one of the most requested features since the day Aim v3 was launched. Would you mind also submitting the instructions on how to run Aim on windows to the official docs? https://github.com/aimhubio/aim/tree/main/docs/source/using

Copy link
Member

@alberttorosyan alberttorosyan left a comment

Choose a reason for hiding this comment

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

@2-5 thanks for contribution! Amazing work! I think we can go a step further and add windows builds to the packaging pipeline (preferable to do a separate PR)
Waiting for @mahnerak to approve two.

@2-5
Copy link
Author

2-5 commented Dec 8, 2022

I've added a few more commits to this PR to bundle the libs to make it work with the aim package.

Now both aim and aimrocks build.

@mihran113
Copy link
Contributor

Hey @2-5! Thanks again for the huge contribution! I've managed to install aimrocks on my end, and done some research on unicode path issue, despite python supporting unicode paths on Windows, rocksdb doesn't support utf-8 encoding out of the box on Windows. here is a discussion from rocksdb repo:
facebook/rocksdb#3408
There are also instructions on custom build on how to make it work.
I guess we can skip this for now, not a major issue, as we're only creating rocksdbs only with our generated hashes (the custom hashes are deprecated already).

@hadim hadim mentioned this pull request Jan 10, 2023
5 tasks
@CLAassistant
Copy link

CLAassistant commented Feb 6, 2023

CLA assistant check
All committers have signed the CLA.

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.

5 participants