Skip to content

python310Packages.fasm: init at 0.0.2.post100 - #251333

Closed
hansfbaier wants to merge 5 commits into
NixOS:masterfrom
openXC7:fasm
Closed

python310Packages.fasm: init at 0.0.2.post100#251333
hansfbaier wants to merge 5 commits into
NixOS:masterfrom
openXC7:fasm

Conversation

@hansfbaier

@hansfbaier hansfbaier commented Aug 25, 2023

Copy link
Copy Markdown
Contributor

Description of changes

added nix packaging for fasm
This PR depends on https://github.com/NixOS/nixpkgs/pull/251333/commits
If somebody else wants to maintain this, please change the maintainer.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 23.11 Release Notes (or backporting 23.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

@github-actions github-actions Bot added the 6.topic: python Python is a high-level, general-purpose programming language. label Aug 25, 2023
@hansfbaier
hansfbaier force-pushed the fasm branch 3 times, most recently from 5be25f8 to ef3ac33 Compare August 25, 2023 09:32
@hansfbaier hansfbaier mentioned this pull request Aug 25, 2023

@fabaff fabaff left a comment

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.

The upstream repo contains tests. unittestCheckHook should be used. If they are not part of the source from PyPI then the source from GitHub should be used.

Please also add pythonImportsCheck.

Comment thread pkgs/development/python-modules/fasm/default.nix Outdated
Comment thread pkgs/development/python-modules/fasm/default.nix Outdated

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.

Using hash with a SRI hash is the preferred way nowadays.

Comment thread pkgs/development/python-modules/fasm/default.nix Outdated
@ofborg ofborg Bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. labels Aug 25, 2023
@hansfbaier
hansfbaier force-pushed the fasm branch 2 times, most recently from 76c0fcb to 51bbfd2 Compare September 8, 2023 05:50
@hansfbaier

hansfbaier commented Sep 8, 2023

Copy link
Copy Markdown
Contributor Author

@fabaff I applied your review notes, and built the packages.
But I noticed something strange: the output derivation only contains the executable binaries,
but no python code for the fasm python library.
For example if I install the python package locally with pip, I also get the following files:

~/.local/lib/python3.10/site-packages/fasm $ find .
.
./parser
./parser/antlr_to_tuple.pyx
./parser/fasm.tx
./parser/textx.py
./parser/antlr.py
./parser/__pycache__
./parser/__pycache__/__init__.cpython-310.pyc
./parser/__pycache__/textx.cpython-310.pyc
./parser/__pycache__/antlr.cpython-310.pyc
./parser/__init__.py
./tool.py
./output.py
./version.py
./__pycache__
./__pycache__/tool.cpython-310.pyc
./__pycache__/__init__.cpython-310.pyc
./__pycache__/output.cpython-310.pyc
./__pycache__/version.cpython-310.pyc
./__pycache__/model.cpython-310.pyc
./__init__.py
./model.py

Where would those files go in nix?

@hansfbaier

Copy link
Copy Markdown
Contributor Author

@fabaff I probably built the wrong derivation:
I used in my local copy of nixpkgs:

$ nix repl
:lf .
:bl outputs.legacyPackages.i686-linux.fasm

But I probably should have built something under outputs.legacyPackages.i686-linux.python310Packages.
The problem is, I can't find fasm there.
How would I build this package?

@jleightcap jleightcap mentioned this pull request Sep 27, 2023
12 tasks
@chuangzhu

Copy link
Copy Markdown
Contributor

The problem is, I can't find fasm there.

You should add an entry in pkgs/top-level/python-packages.nix:

+  fasm = callPackage ../development/python-modules/fasm { };

Comment thread pkgs/development/python-modules/fasm/default.nix Outdated
@jleightcap

Copy link
Copy Markdown
Contributor

This draft is good start, but a valid method of testing:

  1. add fasm to python-packages
  2. nix build .#python3Packages.fasm

Your use of the REPL is interesting, but unfortunately didn't execute the
package you had written here. I'm not positive but I think you might have been
building the other fasm (flat assembler) existing in nixpkgs. Hence there
being no python libraries in the result.

I've taken a stab at packaging this in an out-of-band repo for the time being:
openXC7/toolchain-nix#6

The upstream being inactive with a failing test suite also doesn't bode well for
this package being upstreamed, at the moment.

@hansfbaier

Copy link
Copy Markdown
Contributor Author

@jleightcap I pushed your packaging effort here. It will need to be adapted to use the openxc7 fork instead.

@wegank wegank added the 2.status: merge conflict This PR has merge conflicts with the target branch label Apr 5, 2024
@wegank wegank added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jul 4, 2024
@hansfbaier hansfbaier closed this Jun 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

2.status: merge conflict This PR has merge conflicts with the target branch 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md 6.topic: python Python is a high-level, general-purpose programming language. 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants