googlefonts / fontations

Reading and writing font files
Apache License 2.0
362 stars 22 forks source link

no-std builds are not working and are checked incorrectly #1006

Closed waywardmonkeys closed 1 month ago

waywardmonkeys commented 1 month ago

CI checks no-std builds for font-types, read-fonts and skrifa. However, it uses the default target which has an std, so this isn't a comprehensive check.

These jobs need to specify a target and that needs to not have an std implementation. I tend to use one of thumbv7m-none-eabi, riscv64imac-unknown-none-elf, or x86_64-unknown-none.

The rust-toolchain action will have to install this target:

  check-no-std:
    name: cargo check no std
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v4

      - name: install stable toolchain
        uses: dtolnay/rust-toolchain@stable
            with:
                targets: riscv64imac-unknown-none-elf

      - name: cargo check font-types
        run: cargo check --manifest-path=font-types/Cargo.toml --no-default-features --target riscv64imac-unknown-none-elf

      - name: cargo check read-fonts
        run: cargo check --manifest-path=read-fonts/Cargo.toml --no-default-features --features=libm --target riscv64imac-unknown-none-elf

      - name: cargo check skrifa
        run: cargo check --manifest-path=skrifa/Cargo.toml --no-default-features --features=libm --target riscv64imac-unknown-none-elf

Once you do that, you'll see that int-set doesn't build for no-std and therefore read-fonts doesn't build for no-std.

cmyr commented 1 month ago

huh, that seems counterintuitive to me, if the std feature is disabled why does the presence of std on the target matter?

In any case thank you for the report, will take a look...

dfrg commented 1 month ago

I think the issue is that even if you include the no_std attribute, rustc will happily pull in std if it is used in your dependency graph. Using a std-less target checks the whole compilation.

waywardmonkeys commented 1 month ago

When int-set was incorporated into read-fonts, it was done so behind an std flag, so that part of things works now. Thanks!

@dfrg was correct. PR #1049 addresses all of this hopefully.