Skip to content

Refactor ICU4X code to allow configured code by version #377

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

Merged
merged 12 commits into from
Jan 8, 2025
Merged

Conversation

sffc
Copy link
Member

@sffc sffc commented Jan 7, 2025

Unblocks #372

@sven-oly
Copy link
Collaborator

sven-oly commented Jan 7, 2025

Problems encountered in using this in all versions (1.3, 1.4, 1.5, and 2.0_beta)*

$ gh pr checkout 377
remote: Enumerating objects: 122, done.
remote: Counting objects: 100% (122/122), done.
remote: Compressing objects: 100% (51/51), done.
remote: Total 122 (delta 67), reused 119 (delta 64), pack-reused 0 (from 0)
Receiving objects: 100% (122/122), 27.33 KiB | 6.83 MiB/s, done.
Resolving deltas: 100% (67/67), c$ gh pr checkout 377
remote: Enumerating objects: 122, done.
remote: Counting objects: 100% (122/122), done.
remote: Compressing objects: 100% (51/51), done.
remote: Total 122 (delta 67), reused 119 (delta 64), pack-reused 0 (from 0)
Receiving objects: 100% (122/122), 27.33 KiB | 6.83 MiB/s, done.
Resolving deltas: 100% (67/67), completed with 3 local objects.
From github.com:unicode-org/conformance

  • [new branch] icu4x-refactor -> upstream/icu4x-refactor
    branch 'icu4x-refactor' set up to track 'upstream/icu4x-refactor'.
    Switched to a new branch 'icu4x-refactor'
    ccornelius@ccornelius3:/ICU_conformance/conformance/executors/node$ cd ..
    ccornelius@ccornelius3:
    /ICU_conformance/conformance/executors$ cd rust
    ccornelius@ccornelius3:/ICU_conformance/conformance/executors/rust$ cd 1.3
    ccornelius@ccornelius3:
    /ICU_conformance/conformance/executors/rust/1.3$ cargo build
    Compiling executor v0.0.0 (/usr/local/google/home/ccornelius/ICU_conformance/conformance/executors/rust/1.3)
    error[E0658]: using an imported function as entry point main is experimental
    --> build.rs:4:5
    |
    4 | use print_icu4x_versions_1_3::main;
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: see issue #28937 Tracking issue for Allow a re-export for main (RFC 1260) rust-lang/rust#28937 for more information

For more information about this error, try rustc --explain E0658.
error: could not compile executor (build script) due to previous error
ccornelius@ccornelius3:~/ICU_conformance/conformance/executors/rust/1.3$ cargo build --release
ompleted with 3 local objects.
From github.com:unicode-org/conformance

  • [new branch] icu4x-refactor -> upstream/icu4x-refactor
    branch 'icu4x-refactor' set up to track 'upstream/icu4x-refactor'.
    Switched to a new branch 'icu4x-refactor'
    ccornelius@ccornelius3:/ICU_conformance/conformance/executors/node$ cd ..
    ccornelius@ccornelius3:
    /ICU_conformance/conformance/executors$ cd rust
    ccornelius@ccornelius3:/ICU_conformance/conformance/executors/rust$ cd 1.3
    ccornelius@ccornelius3:
    /ICU_conformance/conformance/executors/rust/1.3$ cargo build
    Compiling executor v0.0.0 (/usr/local/google/home/ccornelius/ICU_conformance/conformance/executors/rust/1.3)
    error[E0658]: using an imported function as entry point main is experimental
    --> build.rs:4:5
    |
    4 | use print_icu4x_versions_1_3::main;
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: see issue #28937 Tracking issue for Allow a re-export for main (RFC 1260) rust-lang/rust#28937 for more information

For more information about this error, try rustc --explain E0658.
error: could not compile executor (build script) due to previous error
ccornelius@ccornelius3:~/ICU_conformance/conformance/executors/rust/1.3$ cargo build --release

@sven-oly
Copy link
Collaborator

sven-oly commented Jan 7, 2025

After rustup 1.80, this works fine.

@sven-oly sven-oly self-assigned this Jan 7, 2025
Copy link
Collaborator

@echeran echeran left a comment

Choose a reason for hiding this comment

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

LGTM

pub mod relativedatetime_fmt;

#[cfg(any(ver = "1.3", ver = "1.4", ver = "1.5"))]
#[path = "datetime_1.rs"]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Interesting. I like the way you did this. I didn't know that you can configure which file Cargo should look at to find a particular module, but that's really convenient for this use case.

@sffc sffc merged commit 2480e06 into main Jan 8, 2025
8 checks passed
@sffc sffc deleted the icu4x-refactor branch January 8, 2025 22:45
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.

3 participants