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

Rebuild for python310 (redux) #40

Closed
wants to merge 15 commits into from
Closed

Conversation

h-vetinari
Copy link
Member

@h-vetinari h-vetinari commented Mar 21, 2022

Continues from #39

Closes #49

@conda-forge-linter
Copy link
Contributor

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

@h-vetinari
Copy link
Member Author

h-vetinari commented Mar 21, 2022

@conda-forge/rust
I don't know why PYO3_CROSS_PYTHON_VERSION does not seem to take effect here. I'm following the same approach as cryptography, where this doesn't run into the problem below:

     Compiling matrixmultiply v0.2.4
       Running `rustc --crate-name matrixmultiply /home/conda/feedstock_root/build_artifacts/tokenizers_1647840012296/_build_env/.cargo/registry/src/github.com-1ecc6299db9ec823/matrixmultiply-0.2.4/src/lib.rs --error-format=json --json=diagnostic-rendered-ansi,artifacts,future-incompat --crate-type lib --emit=dep-info,metadata,link -C opt-level=3 -C embed-bitcode=no --cfg 'feature="default"' --cfg 'feature="std"' -C metadata=94143911fe9bd63f -C extra-filename=-94143911fe9bd63f --out-dir /home/conda/feedstock_root/build_artifacts/tokenizers_1647840012296/work/target/powerpc64le-unknown-linux-gnu/release/deps --target powerpc64le-unknown-linux-gnu -C linker=/home/conda/feedstock_root/build_artifacts/tokenizers_1647840012296/_build_env/bin/powerpc64le-conda-linux-gnu-cc -L dependency=/home/conda/feedstock_root/build_artifacts/tokenizers_1647840012296/work/target/powerpc64le-unknown-linux-gnu/release/deps -L dependency=/home/conda/feedstock_root/build_artifacts/tokenizers_1647840012296/work/target/release/deps --extern rawpointer=/home/conda/feedstock_root/build_artifacts/tokenizers_1647840012296/work/target/powerpc64le-unknown-linux-gnu/release/deps/librawpointer-5a526271f6b8f05d.rmeta --cap-lints allow -C link-arg=-Wl,-rpath-link,/home/conda/feedstock_root/build_artifacts/tokenizers_1647840012296/[...]/lib -C link-arg=-Wl,-rpath,/home/conda/feedstock_root/build_artifacts/tokenizers_1647840012296/[...]/lib`
       Running `/home/conda/feedstock_root/build_artifacts/tokenizers_1647840012296/work/target/release/build/pyo3-e872c55050f3b1f9/build-script-build`
  error: failed to run custom build command for `pyo3 v0.12.4`

  Caused by:
    process didn't exit successfully: `/home/conda/feedstock_root/build_artifacts/tokenizers_1647840012296/work/target/release/build/pyo3-e872c55050f3b1f9/build-script-build` (exit status: 1)
    --- stderr
    Error: "Python 3 required version is 3.5, current version is 3.1"

@isuruf
Copy link
Member

isuruf commented Mar 21, 2022

@h-vetinari
Copy link
Member Author

See conda-forge/cryptography-feedstock#81 (comment)

Ah, I had forgotten about that - many thanks! Weird that they're so far behind with pyo3. Patching it is then...

@h-vetinari
Copy link
Member Author

Yay, a fixture that enforces running from a specific directory...

    @pytest.fixture(scope="session")
    def data_dir():
>       assert os.getcwd().endswith("python")
E       AssertionError

@h-vetinari
Copy link
Member Author

aarch + 3.10 is blocked on conda-forge/multiprocess-feedstock#37 (due to test dep).

@h-vetinari
Copy link
Member Author

Woohoo, a test suite that runs through in emulation 🥳
(still with 1 failure that's still being investigated in huggingface/tokenizers#956, but an much better than failing at import already)

@h-vetinari
Copy link
Member Author

OK, we're down to:

  • Failing tests with python 3.10 due to changed error message huggingface/tokenizers#957 for python 3.10 everywhere
  • remaining type error being worked on in Upgrade pyo3 to 0.16 huggingface/tokenizers#956
  • new error on windows, apparently due to Rerender to get 3.10 aarch builds multiprocess-feedstock#37:
      File "C:\Miniconda\lib\site-packages\conda\exports.py", line 236, in <listcomp>
        actions['LINK'] = [index[d] for d in actions['LINK']]
    KeyError: Dist(channel='conda-forge', dist_name='multiprocess-0.70.12.2-py37hcc03f2d_2', name='multiprocess', fmt='.tar.bz2', version='0.70.12.2', build_string='py37hcc03f2d_2', build_number=2, base_url=None, platform=None)
    
  • An encoding error on windows (currently hidden by the previous error):
    input = array([['My', 'name', 'is', 'John'],
           ['My', 'name', 'is', 'Georges']], dtype='<U7')
    is_pretokenized = True
         def test_single(input, is_pretokenized=False):
            output = tokenizer.encode_batch(input, is_pretokenized=is_pretokenized)
    >       assert format(output) == result_single
    E       AssertionError: assert [['[CLS]', 'm..., '##r', ...]] == [['[CLS]', 'm...es', '[SEP]']]
    E         At index 1 diff: ['[CLS]', 'my', 'name', 'is', 'geo', '##r', '[SEP]'] != ['[CLS]', 'my', 'name', 'is', 'georges', '[SEP]']
    E         Use -v to get more diff
    
    tests\bindings\test_tokenizer.py:177: AssertionError
    

@h-vetinari
Copy link
Member Author

So this is going to need a bunch of upstream work that's now been done but not merged yet. I'm not comfortable in backporting that, so I proposed to wait for the new release that's planned in the next 1-3 weeks or so.

@conda-forge-linter
Copy link
Contributor

Hi! This is the friendly automated conda-forge-linting service.

I wanted to let you know that I linted all conda-recipes in your PR (recipe) and found some lint.

Here's what I've got...

For recipe:

  • Failed to even lint the recipe, probably because of a conda-smithy bug 😢. This likely indicates a problem in your meta.yaml, though. To get a traceback to help figure out what's going on, install conda-smithy and run conda smithy recipe-lint . from the recipe directory.

@conda-forge-linter
Copy link
Contributor

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

switch to github sources to do so
@h-vetinari
Copy link
Member Author

h-vetinari commented May 26, 2022

Fun... Running more extensive tests now runs into the missing cryptography builds for openssl3 through requests->urllib3->cryptography...

@h-vetinari
Copy link
Member Author

h-vetinari commented May 26, 2022

OK, aside from openssl 3 problems, tentatively trying to backport huggingface/tokenizers#956 to 0.12.1 (now merged and applies essentially without conflicts) runs into some more cross-compilation issues:

    --- stderr
    error: The `auto-initialize` feature is enabled, but your python installation only supports embedding the Python interpreter statically. If you are attempting to run tests, or a binary which is okay to link dynamically, install a Python distribution which ships with the Python shared library.

    Embedding the Python interpreter statically does not yet have first-class support in PyO3. If you are sure you intend to do this, disable the `auto-initialize` feature.

    For more information, see https://pyo3.rs/v0.16.2/building_and_distribution.html#embedding-python-in-rust

I'm trying to follow what the cryptography feedstock is doing, but it obviously doesn't seem to be working... The link above to the pyo3 docs also doesn't get me much further.

Tips welcome.

PS. for pip check, we need conda-forge/multiprocess-feedstock#40 (and probably updating some other package that forces pulling in an old dill).

@h-vetinari
Copy link
Member Author

@conda-forge-admin, please rerender

@h-vetinari
Copy link
Member Author

@conda-forge/rust
The build instructions here had worked earlier this year, but now there's widespread fall-out in all cross-compilation jobs. Anything that changed in the setup, or is there something obvious I'm missing here?

@h-vetinari
Copy link
Member Author

Funnily enough this sorted itself out in #53 🤷 🙃

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.

4 participants