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

feat(ci): support building python on windows #1885

Open
wants to merge 16 commits into
base: main
Choose a base branch
from
Open

feat(ci): support building python on windows #1885

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
f928890
feat(ci): support building python on windows
An-DJ Oct 14, 2024
375f5d7
Merge remote-tracking branch 'upstream/main' into python-win-build
An-DJ Nov 2, 2024
2647e8e
Empty-Commit
An-DJ Nov 2, 2024
939a37e
Empty-Commit
An-DJ Nov 2, 2024
3ee5666
fix(ci): use (git) bash to run python steps on Windows
An-DJ Nov 2, 2024
8085249
fix(ci): add conda/bin to Windows system path
An-DJ Nov 2, 2024
5e9a753
remove conda/bin from python ci windows runner
An-DJ Nov 2, 2024
eb6af9a
fix(cpp): add missing header
An-DJ Nov 2, 2024
0637214
fix(cpp): rename FuryLogLevel::ERROR to FuryLogLevel::ERR
An-DJ Nov 2, 2024
8efdc83
fix(python): rename pyx built dynamic lib name to '*.pyd'
An-DJ Nov 2, 2024
5ec5b12
fix(python): rename .so to .pyd on windows
An-DJ Nov 2, 2024
4d15089
fix(misc): add *.pyd to .gitignore
An-DJ Nov 2, 2024
fa649b9
fix wrong shell syntax
An-DJ Nov 2, 2024
f14f0f4
fix wrong shell syntax
An-DJ Nov 2, 2024
64b91bb
fix wrong shell syntax
An-DJ Nov 2, 2024
6582f9c
revert
An-DJ Nov 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@
name: Rust CI
strategy:
matrix:
os: [ubuntu-latest, macos-12, macos-14] # macos-12: x86, macos-14: arm64

Check warning on line 188 in .github/workflows/ci.yml

View workflow job for this annotation

GitHub Actions / 🍏 YAML 

188:49 [comments] too few spaces before comment
runs-on: ${{ matrix.os }}
timeout-minutes: 45
steps:
Expand All @@ -201,7 +201,7 @@
name: C++ CI
strategy:
matrix:
os: [ubuntu-latest, macos-12, macos-14, windows-2022] # macos-12: x86, macos-14: arm64

Check warning on line 204 in .github/workflows/ci.yml

View workflow job for this annotation

GitHub Actions / 🍏 YAML 

204:63 [comments] too few spaces before comment
runs-on: ${{ matrix.os }}
steps:
- uses: actions/checkout@v4
Expand All @@ -215,10 +215,11 @@
name: Python CI
# Fix python 3.6 install issue, see
# https://github.com/rwth-i6/returnn/commit/38ecab17d781c4b74db6a174c8097187380b4ddc
runs-on: ubuntu-20.04
runs-on: ${{ matrix.os }}
strategy:
matrix:
python-version: [3.7, 3.12]
os: [ubuntu-20.04, windows-2022]
steps:
- uses: actions/checkout@v4
- name: Set up Python ${{ matrix.python-version }}
Expand All @@ -228,6 +229,7 @@
- name: Install bazel
run: ./ci/run_ci.sh install_bazel
- name: Run Python CI
shell: bash
run: ./ci/run_ci.sh python

go:
Expand Down
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
**/*.cpp
**/*.so
**/*.dylib
**/*.pyd
bazel-*
.whl
python/.cache
Expand Down
17 changes: 13 additions & 4 deletions BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,10 +102,19 @@ genrule(
set -e
set -x
WORK_DIR=$$(pwd)
cp -f $(location python/pyfury/_util.so) "$$WORK_DIR/python/pyfury"
cp -f $(location python/pyfury/lib/mmh3/mmh3.so) "$$WORK_DIR/python/pyfury/lib/mmh3"
cp -f $(location python/pyfury/format/_format.so) "$$WORK_DIR/python/pyfury/format"
cp -f $(location python/pyfury/_serialization.so) "$$WORK_DIR/python/pyfury"
u_name=`uname -s`
if [ "$${u_name: 0: 4}" == "MING" ] || [ "$${u_name: 0: 4}" == "MSYS" ]
then
cp -f $(location python/pyfury/_util.so) "$$WORK_DIR/python/pyfury/_util.pyd"
cp -f $(location python/pyfury/lib/mmh3/mmh3.so) "$$WORK_DIR/python/pyfury/lib/mmh3/mmh3.pyd"
cp -f $(location python/pyfury/format/_format.so) "$$WORK_DIR/python/pyfury/format/_format.pyd"
cp -f $(location python/pyfury/_serialization.so) "$$WORK_DIR/python/pyfury/_serialization.pyd"
else
cp -f $(location python/pyfury/_util.so) "$$WORK_DIR/python/pyfury"
cp -f $(location python/pyfury/lib/mmh3/mmh3.so) "$$WORK_DIR/python/pyfury/lib/mmh3"
cp -f $(location python/pyfury/format/_format.so) "$$WORK_DIR/python/pyfury/format"
cp -f $(location python/pyfury/_serialization.so) "$$WORK_DIR/python/pyfury"
fi
echo $$(date) > $@
""",
local = 1,
Expand Down
4 changes: 2 additions & 2 deletions ci/deploy.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,8 @@ VERSIONS=("3.7"
"3.11"
"3.12")

source $(conda info --base)/etc/profile.d/conda.sh

create_py_envs() {
source $(conda info --base)/etc/profile.d/conda.sh
for version in "${VERSIONS[@]}"; do
conda create -y --name "py$version" python="$version"
done
Expand Down Expand Up @@ -94,6 +93,7 @@ deploy_jars() {
}

deploy_python() {
source $(conda info --base)/etc/profile.d/conda.sh
if command -v pyenv; then
pyenv local system
fi
Expand Down
1 change: 1 addition & 0 deletions cpp/fury/thirdparty/MurmurHash3.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#define FORCE_INLINE __forceinline

#include <cstdint>
#include <cstdlib>

#define ROTL32(x, y) _rotl(x, y)
#define ROTL64(x, y) _rotl64(x, y)
Expand Down
4 changes: 2 additions & 2 deletions cpp/fury/util/logging.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ std::string GetCallTrace() {

std::unordered_map<FuryLogLevel, std::string> log_level_to_str = {
{FuryLogLevel::DEBUG, "DEBUG"}, {FuryLogLevel::INFO, "INFO"},
{FuryLogLevel::WARNING, "WARNING"}, {FuryLogLevel::ERROR, "ERROR"},
{FuryLogLevel::WARNING, "WARNING"}, {FuryLogLevel::ERR, "ERROR"},
{FuryLogLevel::FATAL, "FATAL"},
};

Expand All @@ -77,7 +77,7 @@ FuryLogLevel FuryLog::GetLogLevel() {
} else if (data == "warning") {
severity_threshold = FuryLogLevel::WARNING;
} else if (data == "error") {
severity_threshold = FuryLogLevel::ERROR;
severity_threshold = FuryLogLevel::ERR;
} else if (data == "fatal") {
severity_threshold = FuryLogLevel::FATAL;
} else {
Expand Down
2 changes: 1 addition & 1 deletion cpp/fury/util/logging.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ enum class FuryLogLevel {
DEBUG = -1,
INFO = 0,
WARNING = 1,
ERROR = 2,
ERR = 2,
Copy link
Member

@PragmaTwice PragmaTwice Dec 13, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Rename FuryLogLevel::ERROR to FuryLogLevel::ERR. I don't know why this enum make build failed, but it is successed if renaming it to FuryLogLevel::ERR

Could you attach the error message/log you face about the original ERROR identifier?

Copy link
Member

Choose a reason for hiding this comment

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

Hmm it seems here: https://github.com/apache/fury/actions/runs/11643078405/job/32423399925#step:5:1894

Copy link
Member

Choose a reason for hiding this comment

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

Just took a glance, and I think that maybe the Cython generated code defines a macro named ERROR and it's included before this header so that weird thing happens. Not familiar with related code here, maybe @penguin-wwy has ideas about this.

FATAL = 3
};

Expand Down
Loading