Skip to content

Adding asmjit#31498

Open
das-intensity wants to merge 1 commit into
conda-forge:mainfrom
das-intensity:asmjit
Open

Adding asmjit#31498
das-intensity wants to merge 1 commit into
conda-forge:mainfrom
das-intensity:asmjit

Conversation

@das-intensity
Copy link
Copy Markdown
Contributor

Checklist

  • Title of this PR is meaningful: e.g. "Adding my_nifty_package", not "updated meta.yaml".
  • License file is packaged (see here for an example).
  • Source is from official source.
  • Package does not vendor other packages. (If a package uses the source of another package, they should be separate packages or the licenses of all packages need to be packaged).
  • If static libraries are linked in, the license of the static library is packaged.
  • Package does not ship static libraries. If static libraries are needed, follow CFEP-18.
  • Build number is 0.
  • A tarball (url) rather than a repo (e.g. git_url) is used in your recipe (see here for more details).
  • GitHub users listed in the maintainer section have posted a comment confirming they are willing to be listed there.
  • When in trouble, please check our knowledge base documentation before pinging a team.

@github-actions
Copy link
Copy Markdown
Contributor

Hi! This is the staged-recipes linter and I found some lint.

File-specific lints and/or hints:

  • recipes/asmjit/meta.yaml:
    • lints:
      • The following maintainers have not yet confirmed that they are willing to be listed here: dahm. Please ask them to comment on this PR if they are.

@conda-forge-admin
Copy link
Copy Markdown
Contributor

conda-forge-admin commented Nov 16, 2025

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 (recipes/asmjit/meta.yaml) and found it was in an excellent condition.

I do have some suggestions for making it better though...

For recipes/asmjit/meta.yaml:

  • ℹ️ The recipe is not parsable by parser conda-souschef (grayskull). This parser is not currently used by conda-forge, but may be in the future. We are collecting information to see which recipes are compatible with grayskull.
  • ℹ️ The recipe is not parsable by parser conda-recipe-manager. The recipe can only be automatically migrated to the new v1 format if it is parseable by conda-recipe-manager.

This message was generated by GitHub Actions workflow run https://github.com/conda-forge/conda-forge-webservices/actions/runs/20698709109. Examine the logs at this URL for more detail.

@github-actions
Copy link
Copy Markdown
Contributor

Hi! This is the staged-recipes linter and your PR looks excellent! 🚀

@das-intensity
Copy link
Copy Markdown
Contributor Author

@conda-forge/help-c-cpp ready for review

Comment thread recipes/asmjit/meta.yaml
Comment on lines +2 to +4
{% set commit_date = "20251012" %}
{% set commit_hash = "5134d396bd00c1b63259387acdbb12dfdf009f9b" %}
{% set version = "0.0.0.dev" + commit_date + "+" + commit_hash[:8] %}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is there an upstream tag or the possibility to get upstream to tag a release?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Is there an upstream tag

Nope, no releases or even tags https://github.com/asmjit/asmjit/tags

or the possibility to get upstream to tag a release?

Unlikely per this:

In general no. Considering the current support from the ecosystem there will probably never be any releases.

@traversaro
Copy link
Copy Markdown
Contributor

Thanks for the contribution @das-intensity ! As asmjit seems to be related/vendored in PyTorch, perhaps it is good to have an opinion on this from @conda-forge/pytorch-cpu .

@das-intensity
Copy link
Copy Markdown
Contributor Author

@traversaro Sounds good. I took a brief look in e.g. pytorch-2.7.1-cuda128_mkl_py312_ha31c84e_304.conda and didn't see any files containing asmjit in the name.

However looking in the recipe, it looks like the tests, only for windows, expect asmjit.dll and fbgemm.dll:

    {% set torch_libs = [
        ...
    ] + target_platform.startswith("win") * [
        "asmjit", "fbgemm"
    ]
    %}
    {% for each_lib in torch_libs %}
    - test -f $PREFIX/lib/lib{{ each_lib }}.so              # [linux]
    - test -f $PREFIX/lib/lib{{ each_lib }}.dylib           # [osx]
    - if not exist %LIBRARY_BIN%\{{ each_lib }}.dll exit 1  # [win]

Interestingly on this point, the reason that I put up this asmjit PR is because I'm working on a PR for fbgemm, and it was pulling in asmjit source and compiling it to lib/python3.12/site-packages/fbgemm_gpu/asmjit.so. I am hoping this PR could enable us to avoid that.

Perhaps (hopefully) where we'll end up is with both asmjit and fbgemm standalone and actually depended on by pytorch, but in a very odd way:

  1. asmjit and fbgemm would be standalone
  2. Pytorch would depend on both of these, at least for windows builds
  3. fbgemm-gpu and fbgemm-gpu-genai (different sources to fbgemm, although same repo)

See FBGEMM repo packages here

@h-vetinari
Copy link
Copy Markdown
Member

Perhaps (hopefully) where we'll end up is with both asmjit and fbgemm standalone and actually depended on by pytorch, but in a very odd way:

In pytorch 2.9 we're actually dropping asmjit and fbgemm on windows (don't know the backstory there, perhaps @mgorny can comment).

Overall, I'm very much in favour of unvendoring stuff like this though (still sad that #19103 never made it, which included asmjit and fbgemm as well).

fbgemm-gpu and fbgemm-gpu-genai (different sources to fbgemm, although same repo)

I don't understand what you're saying here. Based on the README in https://github.com/pytorch/FBGEMM, these would be depending on fbgemm and should then obviously be built on the same feedstock.

@das-intensity
Copy link
Copy Markdown
Contributor Author

I was saying that based on:

  1. Pytorch would depend on both of these, at least for windows builds

because (at time of writing), pytorch 2.8 still relied on fbgemm, leaving the dep chain fbgemm <- pytorch <- fbgemm-gpu. With this, we wouldn't be able to build fbgemm and fbgemm-gpu at the same time (since one must be built before pytorch, and the other after). Hope that makes sense.

Regardless, if you're dropping the vendoring of fbgemm in pytorch 2.9 completely, then this is resolved, and all fbgemm packages can be happily built together in a single feedstock.

@danielnachun
Copy link
Copy Markdown
Contributor

This is a good effort to devendor pytorch, despite the upstream lack of interest in making this easier. I had a few thoughts/ questions.

I think you can override c_stdlib on linux_aarch64 pre-emptively by just adding a conda_build_config.yaml with these contents:

c_stdlib_version:
	- 2.28 # [linux and aarch64]

When the feedstock is rendered, this should just override the default in .ci_support. That saves you the need to skip aarch64.

Are you planning to test macOS at least in here? It would be nice to see how this builds on that platform since it should be very similar to Linux. Windows would also be good to enable, though if it ends up being too much of a headache to debug, then you can just skip it for now.

Lastly, do we have any long term ideas on how to keep the versions in sync here? Since upstream doesn't seem to want to tag versions of dependencies, will someone have to manually update the commits for every new Pytorch? I guess Pytorch isn't exactly the kind of software that we will ever just automatically update anyway, but it would be good to have a documented process for how to choose the right commits etc.

@mgorny
Copy link
Copy Markdown
Contributor

mgorny commented Nov 24, 2025

Long story short, PyTorch 2.9 disabled building fbgemm (and therefore asmjit) on Windows. I've tried reenabling it but it failed to compile, so I just assumed fbgemm no longer works on Windows, so it was disabled deliberately, and moved on.

FWICS PyTorch currently doesn't support using system fbgemm. Of course, this is potentially something we could fix if the releases are reasonably compatible with whatever PyTorch is vendoring.

@das-intensity
Copy link
Copy Markdown
Contributor Author

@mgorny I tried to add conda_build_config.yaml but it didn't apply to the local build. My hope here is:

  1. Land this as-is with aarch64, osx, and win disabled, to ensure everything else in my recipe is correct
  2. Now that the feedstock is created, add in the ``conda_build_config.yamlto enableaarch64`, knowing the PR CI will build it properly
  3. Ideally try similar for osx/osx_arm/win since I can't get them to build locally (I might be able to dust off a windows pc to build win)
  4. Then potentially update to latest version

I'm unsure on (4) since fbgemm pinned a specific version, so I'm always nervous that it's specifically dependent on that exact version, and upgrading version would make the next fbgemm upgrade tick fail due to this.

it would be good to have a documented process for how to choose the right commits etc.

My thought is to check which version fbgemm and/or pytorch uses, and stick to that. In the ideal case we push the version fbgemm and pytorch use on their master branches before upgrading to said version here.

@mgorny
Copy link
Copy Markdown
Contributor

mgorny commented Nov 24, 2025

Yeah, I was wondering about the version matching. I would personally probably go the other way around, i.e. start with fbgemm with vendored asmjit and then unvendor it, but either way is fine, as long as it matches and works.

Did you try the complete asmjit - fbgemm - pytorch chain locally by any chance?

@das-intensity
Copy link
Copy Markdown
Contributor Author

start with fbgemm with vendored asmjit and then unvendor it

Sounds good. I'm 99% of the way there, will put up the PR for fbgemm in the coming days.

PyTorch currently doesn't support using system fbgemm

You mean regular package-manager-installed fbgemm? Or did someone try building fbgemm for conda separately and that didn't work?

@das-intensity
Copy link
Copy Markdown
Contributor Author

Now that "a few days" have passed since 24th Nov, I've put up the fbgemm recipe PR: #31820

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants