Skip to content

Commit

Permalink
refactor!: simplify Makevars files and rename env vars for building t…
Browse files Browse the repository at this point in the history
…he library (#693)
  • Loading branch information
eitsupi committed Jan 20, 2024
1 parent 7c842ad commit bc675ca
Show file tree
Hide file tree
Showing 16 changed files with 75 additions and 119 deletions.
10 changes: 5 additions & 5 deletions .github/actions/setup/action.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ runs:
using: composite
steps:
- name: Update Rust
if: inputs.rust-nightly != 'true' && env.RPOLARS_FULL_FEATURES != 'true'
if: inputs.rust-nightly != 'true' && env.LIBR_POLARS_FEATURES != 'full_features'
shell: bash
run: |
rustup update
Expand All @@ -27,7 +27,7 @@ runs:
repo-token: ${{ inputs.token }}

- name: Set up Rust nightly toolchain
if: inputs.rust-nightly == 'true' || env.RPOLARS_FULL_FEATURES == 'true'
if: inputs.rust-nightly == 'true' || env.LIBR_POLARS_FEATURES == 'full_features'
shell: bash
run: |
task setup-rust-toolchain
Expand All @@ -39,10 +39,10 @@ runs:
rustup target add ${{ inputs.target }}
- name: Set up Rust profile
if: env.RPOLARS_PROFILE == ''
if: env.LIBR_POLARS_PROFILE == ''
shell: bash
run: |
echo "RPOLARS_PROFILE=release" >>"$GITHUB_ENV"
echo "LIBR_POLARS_PROFILE=release" >>"$GITHUB_ENV"
- name: Find default target
id: find-target
Expand All @@ -59,5 +59,5 @@ runs:
- name: Cache Rust
uses: Swatinem/rust-cache@v2
with:
shared-key: build-${{ env.RPOLARS_PROFILE }}-${{ inputs.target || steps.find-target.outputs.target }}
shared-key: build-${{ env.LIBR_POLARS_PROFILE }}-${{ inputs.target || steps.find-target.outputs.target }}
workspaces: "src/rust -> target"
4 changes: 2 additions & 2 deletions .github/workflows/check.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,8 @@ jobs:
- name: Set env vars for build option
if: matrix.config.full-features
run: |
echo "RPOLARS_FULL_FEATURES=true" >>$GITHUB_ENV
echo "RPOLARS_PROFILE=release" >>$GITHUB_ENV
echo "LIBR_POLARS_FEATURES=full_features" >>$GITHUB_ENV
echo "LIBR_POLARS_PROFILE=release" >>$GITHUB_ENV
- uses: ./.github/actions/setup
with:
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/docs.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,9 @@ concurrency:
cancel-in-progress: true

env:
RPOLARS_FULL_FEATURES: "true"
LIBR_POLARS_FEATURES: "full_features"
LIBR_POLARS_BUILD: "true"
RPOLARS_PROFILE: release
LIBR_POLARS_PROFILE: release

jobs:
documentation:
Expand Down
10 changes: 5 additions & 5 deletions .github/workflows/release-lib.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ defaults:

env:
LIB_NAME: libr_polars
RPOLARS_FULL_FEATURES: "true"
RPOLARS_PROFILE: release-optimized
LIBR_POLARS_FEATURES: "full_features"
LIBR_POLARS_PROFILE: release-optimized

jobs:
build:
Expand Down Expand Up @@ -74,11 +74,11 @@ jobs:
env:
NOT_CRAN: "true"
TARGET: ${{ matrix.target }}
RPOLARS_PROFILE: ${{ env.RPOLARS_PROFILE }}
LIBR_POLARS_PROFILE: ${{ env.LIBR_POLARS_PROFILE }}
LIBR_POLARS_BUILD: "true"
working-directory: src
run: |
LIB_PATH="$(pwd)/rust/target/${TARGET}/${RPOLARS_PROFILE}/${LIB_NAME}.a"
LIB_PATH="$(pwd)/rust/target/${TARGET}/${LIBR_POLARS_PROFILE}/${LIB_NAME}.a"
ARTIFACT_NAME="${LIB_NAME}-${LIB_VERSION}-${TARGET}.tar.gz"
if [[ ${{ runner.os }} == 'Windows' ]]; then
pushd ..
Expand All @@ -88,7 +88,7 @@ jobs:
else
make -f Makevars.in "${LIB_PATH}"
fi
tar -czf "../${ARTIFACT_NAME}" -C "rust/target/${TARGET}/${RPOLARS_PROFILE}" "${LIB_NAME}.a"
tar -czf "../${ARTIFACT_NAME}" -C "rust/target/${TARGET}/${LIBR_POLARS_PROFILE}" "${LIB_NAME}.a"
echo "ARTIFACT_NAME=${ARTIFACT_NAME}" >>"$GITHUB_ENV"
- name: upload artifact
Expand Down
6 changes: 3 additions & 3 deletions .github/workflows/release.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ defaults:
shell: bash

env:
RPOLARS_FULL_FEATURES: "true"
RPOLARS_PROFILE: release-optimized
LIBR_POLARS_FEATURES: "full_features"
LIBR_POLARS_PROFILE: release-optimized
NOT_CRAN: "true"

jobs:
Expand Down Expand Up @@ -74,7 +74,7 @@ jobs:
TARGET="$(rustc -vV | grep host | cut -d' ' -f2)"
fi
Rscript -e 'pkgbuild::compile_dll()'
echo "LIBR_POLARS_PATH=$(pwd)/src/rust/target/${TARGET}/${{ env.RPOLARS_PROFILE }}/libr_polars.a" >>$GITHUB_ENV
echo "LIBR_POLARS_PATH=$(pwd)/src/rust/target/${TARGET}/${{ env.LIBR_POLARS_PROFILE }}/libr_polars.a" >>$GITHUB_ENV
- name: make binary R package + test on host arch
if: matrix.config.target == ''
Expand Down
4 changes: 0 additions & 4 deletions DEVELOPMENT.md
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,3 @@ load_polars()
#to check package + reuses previous compilation in check, protects against deletion
check_polars() #assumes rust target at `paste0(getwd(),"/src/rust")`
```

- The `RPOLARS_RUST_SOURCE` environment variable allows **polars** to recover the Cargo cache even if source files have been moved. Replace with your own absolute path to your local clone!
- `filter_rcmdcheck.R` removes known warnings from final check report.
- `unlink("check")` cleans up.
10 changes: 10 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,16 @@

## polars (development version)

### Breaking changes

- The environment variables used when building the library have been changed. (#693)
This only affects selecting the feature flag and selecting profiles during source installation.
- `RPOLARS_PROFILE` is renamed to `LIBR_POLARS_PROFILE`
- `RPOLARS_FULL_FEATURES` is removed and `LIBR_POLARS_FEATURES` is added.
If want to select the `full_features`, we need to set `LIBR_POLARS_FEATURES="full_features"`.
- `RPOLARS_RUST_SOURCE`, which was used for development, has been removed.
If you want to use library binaries located elsewhere, use `LIBR_POLARS_PATH` instead.

### What's changed

- New method `$rolling()` for `DataFrame` and `LazyFrame`. When this is
Expand Down
4 changes: 1 addition & 3 deletions R/expr__string.R
Original file line number Diff line number Diff line change
Expand Up @@ -266,9 +266,7 @@ ExprStr_to_lowercase = function() {
#' @keywords ExprStr
#' @return Expr of String titlecase chars
#' @details
#' This method is only available with the feature flag "simd" which can
#' be set via envvar "RPOLARS_FULL_FEATURES" and it requires
#' Rust nightly toolchain to compile.
#' This method is only available with the "simd" feature.
#' See [polars_info] for more details.
#' @examplesIf polars_info()$features$simd
#' pl$lit(c("hello there", "HI, THERE", NA))$str$to_titlecase()$to_series()
Expand Down
2 changes: 1 addition & 1 deletion R/sql.R
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#' @title Run SQL queries against DataFrame/LazyFrame data.
#' @description Run SQL queries against DataFrame/LazyFrame data.
#' @details Currently, only available when built with the `full` feature.
#' @details Currently, only available when built with the "sql" feature.
#' See [polars_info] for more information.
#' @name SQLContext_class
#' @keywords SQLContext
Expand Down
2 changes: 1 addition & 1 deletion Taskfile.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ version: "3"
env:
NOT_CRAN: "true"
LIBR_POLARS_BUILD: "true"
RPOLARS_FULL_FEATURES: "true"
LIBR_POLARS_FEATURES: "full_features"

vars:
PACKAGE_NAME: polars
Expand Down
4 changes: 1 addition & 3 deletions man/ExprStr_to_titlecase.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion man/SQLContext_class.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

50 changes: 12 additions & 38 deletions src/Makevars.in
Original file line number Diff line number Diff line change
@@ -1,16 +1,12 @@
TARGET ?= @RUST_TARGET@
RPOLARS_PROFILE ?= release
RPOLARS_BUILD_COMMAND_BASE ?= cargo build --target=$(TARGET)
BUILD_COMMAND = $(RPOLARS_BUILD_COMMAND_BASE) --lib --profile $(RPOLARS_PROFILE) --manifest-path="$(RPOLARS_RUST_SOURCE)/Cargo.toml"
LIBR_POLARS_PROFILE ?= release
LIBR_POLARS_FEATURES ?=

LIBNAME = libr_polars.a
TARGET_DIR = $(CURDIR)/rust/target
LIBDIR = $(TARGET_DIR)/$(TARGET)/$(RPOLARS_PROFILE)
LIBDIR = $(TARGET_DIR)/$(TARGET)/$(LIBR_POLARS_PROFILE)
STATLIB = $(LIBDIR)/$(LIBNAME)
PKG_LIBS = -L$(LIBDIR) -lr_polars @ADDITIONAL_PKG_LIBS_FLAG@
RPOLARS_RUST_SOURCE ?= ./rust

rpolars_ext_binary = $(RPOLARS_RUST_SOURCE)/target/$(TARGET)/$(RPOLARS_PROFILE)/$(LIBNAME)

all: C_clean

Expand All @@ -25,41 +21,19 @@ $(STATLIB):
exit 0; \
fi && \
if [ "true" != "true" ]; then \
export CARGO_HOME=$(CARGOTMP); \
export CARGO_HOME="$(CARGOTMP)"; \
export CARGO_BUILD_JOBS=2; \
fi && \
export PATH="$(PATH):$(HOME)/.cargo/bin" && \
if [ "$(RPOLARS_FULL_FEATURES)" = "true" ]; then \
$(BUILD_COMMAND) --features "full_features"; \
else \
$(BUILD_COMMAND); \
fi
if [ "true" != "true" ]; then \
rm -Rf $(CARGOTMP) && \
rm -Rf $(LIBDIR)/build; \
fi
if [ -f "$(STATLIB)" ]; then \
echo "file is there: "; \
elif [ -f "$(rpolars_ext_binary)" ]; then \
echo "file is '$(rpolars_ext_binary)'"; \
mkdir -p $(LIBDIR) ; \
echo "trying to symlink in $(rpolars_ext_binary)"; \
ln -s $(rpolars_ext_binary) $(STATLIB) ; \
fi
export PATH="$(PATH):$(HOME)/.cargo/bin" && \
cargo build --lib --manifest-path="$(CURDIR)/rust/Cargo.toml" --target-dir "$(TARGET_DIR)" --target="$(TARGET)" \
--profile="$(LIBR_POLARS_PROFILE)" --features="$(LIBR_POLARS_FEATURES)"

if [ "$(RPOLARS_CARGO_CLEAN_DEPS)" = "true" ]; then \
echo "cleanup!!" ; \
mv $(STATLIB) $(LIBDIR)/../temp_binary.a; \
rm -rf $(LIBDIR); \
mkdir $(LIBDIR); \
mv $(LIBDIR)/../temp_binary.a $(STATLIB); \
rm -rf ./src/.cargo; \
else \
echo "hands off!!" ; \
if [ "true" != "true" ]; then \
rm -Rf "$(CARGOTMP)" "$(LIBDIR)/build"; \
fi


C_clean:
rm -Rf $(SHLIB) $(STATLIB) $(OBJECTS)
rm -Rf "$(SHLIB)" "$(STATLIB)" "$(OBJECTS)"

clean:
rm -Rf $(SHLIB) $(STATLIB) $(OBJECTS) rust/target
rm -Rf "$(SHLIB)" "$(STATLIB)" "$(OBJECTS)" "$(TARGET_DIR)"
64 changes: 19 additions & 45 deletions src/Makevars.win
Original file line number Diff line number Diff line change
@@ -1,16 +1,12 @@
TARGET ?= $(subst 64,x86_64,$(subst 32,i686,$(WIN)))-pc-windows-gnu
RPOLARS_PROFILE ?= release
RPOLARS_BUILD_COMMAND_BASE ?= cargo build --target=$(TARGET)
BUILD_COMMAND = $(RPOLARS_BUILD_COMMAND_BASE) --lib --profile $(RPOLARS_PROFILE) --manifest-path="$(RPOLARS_RUST_SOURCE)/Cargo.toml"
LIBR_POLARS_PROFILE ?= release
LIBR_POLARS_FEATURES ?=

LIBNAME = libr_polars.a
TARGET_DIR = $(CURDIR)/rust/target
LIBDIR = $(TARGET_DIR)/$(TARGET)/$(RPOLARS_PROFILE)
LIBDIR = $(TARGET_DIR)/$(TARGET)/$(LIBR_POLARS_PROFILE)
STATLIB = $(LIBDIR)/$(LIBNAME)
PKG_LIBS = -L$(LIBDIR) -lr_polars -lws2_32 -ladvapi32 -luserenv -lbcrypt -lole32 -lntdll -lpsapi -liphlpapi -lpdh -lpowrprof -loleaut32 -lnetapi32 -lsecur32 -t
RPOLARS_RUST_SOURCE ?= ./rust

rpolars_ext_binary = $(RPOLARS_RUST_SOURCE)/target/$(TARGET)/$(RPOLARS_PROFILE)/$(LIBNAME)

# Rtools42 doesn't have the linker in the location that cargo expects, so we
# need to overwrite it via configuration.
Expand All @@ -23,57 +19,35 @@ $(SHLIB): $(STATLIB)
CARGOTMP = $(CURDIR)/.cargo

$(STATLIB):
echo "RPOLARS_RUST_SOURCE is set to $(RPOLARS_RUST_SOURCE)"
mkdir -p $(TARGET_DIR)/libgcc_mock && touch $(TARGET_DIR)/libgcc_mock/libgcc_eh.a
# `rustc` adds `-lgcc_eh` flags to the compiler, but Rtools' GCC doesn't have
# `libgcc_eh` due to the compilation settings. So, in order to please the
# compiler, we need to add empty `libgcc_eh` to the library search paths.
#
# For more details, please refer to
# https://github.com/r-windows/rtools-packages/blob/2407b23f1e0925bbb20a4162c963600105236318/mingw-w64-gcc/PKGBUILD#L313-L316
mkdir -p "$(TARGET_DIR)/libgcc_mock"
touch "$(TARGET_DIR)/libgcc_mock/libgcc_eh.a"

# CARGO_LINKER is provided in Makevars.ucrt for R >= 4.2
if [ -f "$(CURDIR)/../tools/$(LIBNAME)" ]; then \
mkdir -p "$(LIBDIR)" ; \
mv "$(CURDIR)/../tools/$(LIBNAME)" "$(STATLIB)" ; \
exit 0; \
fi && \
if [ "true" != "true" ]; then \
export CARGO_HOME=$(CARGOTMP); \
export CARGO_HOME="$(CARGOTMP)"; \
export CARGO_BUILD_JOBS=2; \
fi && \
export CARGO_TARGET_X86_64_PC_WINDOWS_GNU_LINKER="$(CARGO_LINKER)" && \
export LIBRARY_PATH="$${LIBRARY_PATH};$(TARGET_DIR)/libgcc_mock" && \
if [ "$(RPOLARS_FULL_FEATURES)" = "true" ]; then \
$(BUILD_COMMAND) --features "full_features"; \
else \
$(BUILD_COMMAND); \
fi;
if [ "true" != "true" ]; then \
rm -Rf $(CARGOTMP) && \
rm -Rf $(LIBDIR)/build; \
fi
cargo build --lib --manifest-path="$(CURDIR)/rust/Cargo.toml" --target-dir "$(TARGET_DIR)" --target="$(TARGET)" \
--profile="$(LIBR_POLARS_PROFILE)" --features="$(LIBR_POLARS_FEATURES)"

# when RPOLARS_RUST_SOURCE is defined rust binary is compiled there
# if binary is not here, it can be symlinked instead.
# This solves both caching and R CMD check failing cargo build files
# see issue #28
if [ -f "$(STATLIB)" ]; then \
echo "file is there: "; \
elif [ -f "$(rpolars_ext_binary)" ]; then \
echo "file is '$(rpolars_ext_binary)'"; \
echo "binary not built here, trying to symlink in $(rpolars_ext_binary)"; \
mkdir -p $(LIBDIR) ; \
ln -s $(rpolars_ext_binary) $(STATLIB) ; \
fi

# CRAN might even need more files to be gone, delete them here...
if [ "$(RPOLARS_CARGO_CLEAN_DEPS)" = "true" ]; then \
echo "clean up target, to not let R CMD check fail on rust files"; \
mv $(STATLIB) $(LIBDIR)/../temp_binary.a; \
rm -rf $(LIBDIR); \
mkdir $(LIBDIR); \
mv $(LIBDIR)/../temp_binary.a $(STATLIB); \
rm -rf -v ./src/.cargo; \
else \
echo "skip cleaning" ; \
if [ "true" != "true" ]; then \
rm -Rf "$(CARGOTMP)" "$(LIBDIR)/build"; \
fi

C_clean:
rm -Rf $(SHLIB) $(STATLIB) $(OBJECTS)
rm -Rf "$(SHLIB)" "$(STATLIB)" "$(OBJECTS)"

clean:
rm -Rf $(SHLIB) $(STATLIB) $(OBJECTS) rust/target
rm -Rf "$(SHLIB)" "$(STATLIB)" "$(OBJECTS)" "$(TARGET_DIR)"
5 changes: 1 addition & 4 deletions src/rust/src/lazy/dsl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2448,10 +2448,7 @@ fn f_str_to_titlecase(expr: &RPolarsExpr) -> RResult<RPolarsExpr> {
return (Ok(expr.0.clone().str().to_titlecase().into()));

#[cfg(not(feature = "simd"))]
rerr().plain(
"$to_titlecase() is only available with 'simd' enabled. Try our github \
binary releases or compile with env var RPOLARS_FULL_FEATURES = 'true'",
)
rerr().plain("$to_titlecase() is only available with the 'simd' feature")
}

//allow proto expression that yet only are strings
Expand Down
13 changes: 11 additions & 2 deletions vignettes/install.Rmd
Original file line number Diff line number Diff line change
Expand Up @@ -120,8 +120,17 @@ polars_info()

At this time, the following environment variables can be used to change the Rust library build time options.

- `RPOLARS_FULL_FEATURES="true"` (Build with nightly feature enabled, requires Rust nightly toolchain `r rust_toolchain_version`)
- `RPOLARS_PROFILE="release-optimized"` (Build with more optimization)
#### Features

The feature flags can be set by the `LIBR_POLARS_FEATURES` environment variable.
For example, to enable the `full_features` feature, set the environment variable as like `LIBR_POLARS_FEATURES="full_features"`.

Note that these features requires the Rust nightly toolchain `r rust_toolchain_version`.

#### Profile

The profile can be set by the `LIBR_POLARS_PROFILE` environment variable.
For example, to build with more optimization, set the environment variable as like `LIBR_POLARS_PROFILE="release-optimized"`.

### Minimum Supported Rust Version (MSRV)

Expand Down

0 comments on commit bc675ca

Please sign in to comment.