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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

dlang dstep: init at 1.0.4 #299797

Merged
merged 2 commits into from Apr 8, 2024
Merged

dlang dstep: init at 1.0.4 #299797

merged 2 commits into from Apr 8, 2024

Conversation

imrying
Copy link
Contributor

@imrying imrying commented Mar 28, 2024

Description of changes

From github: DStep is a tool for automatically generating D bindings for C and Objective-C libraries. This is implemented by processing C or Objective-C header files and output D modules. DStep uses the Clang compiler as a library (libclang) to process the header files.

Things done

I packaged the binary from releases since they are statically linked, and the configuration flow does not allow for easily updating to nix-style due to linker flags have to being set. If possible I think we could generate the linker_flags.txt manually and use this. Let me know if it is okay to package the binaries directly :-).

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • 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/)
  • 24.05 Release Notes (or backporting 23.05 and 23.11 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.

Add a 馃憤 reaction to pull requests you find important.

Copy link
Member

@AndersonTorres AndersonTorres left a comment

Choose a reason for hiding this comment

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

It deserves a full rewrite.

pkgs/by-name/ds/dstep/package.nix Outdated Show resolved Hide resolved
pkgs/by-name/ds/dstep/package.nix Outdated Show resolved Hide resolved
pkgs/by-name/ds/dstep/package.nix Outdated Show resolved Hide resolved
pkgs/by-name/ds/dstep/package.nix Outdated Show resolved Hide resolved
pkgs/by-name/ds/dstep/package.nix Outdated Show resolved Hide resolved
pkgs/by-name/ds/dstep/package.nix Outdated Show resolved Hide resolved
@symphorien
Copy link
Member

I packaged the binary from releases since they are statically linked, and the configuration flow does not allow for easily updating to nix-style due to linker flags have to being set. If possible I think we could generate the linker_flags.txt manually and use this. Let me know if it is okay to package the binaries directly :-).

yes building from source would be more preferable. What is this linker_flags.txt you are referring to? I found no mention of this file in the github repo of dstep. I only see in the readme that you need to run configure to the path where libclang lives:

$ ./configure --llvm-path /usr/lib/llvm-4.0

so

configurePhase = ''
./configure --llvm-path ${clang.cc.lib}/lib
'';

probably works.

@imrying
Copy link
Contributor Author

imrying commented Mar 28, 2024

@AndersonTorres thanks for the feedback. I've updated it know. Quite new with building packages and saw it used in some other place though i probably misunderstood :-)

@imrying
Copy link
Contributor Author

imrying commented Mar 28, 2024

I packaged the binary from releases since they are statically linked, and the configuration flow does not allow for easily updating to nix-style due to linker flags have to being set. If possible I think we could generate the linker_flags.txt manually and use this. Let me know if it is okay to package the binaries directly :-).

yes building from source would be more preferable. What is this linker_flags.txt you are referring to? I found no mention of this file in the github repo of dstep. I only see in the readme that you need to run configure to the path where libclang lives:

$ ./configure --llvm-path /usr/lib/llvm-4.0

so

configurePhase = ''
./configure --llvm-path ${clang.cc.lib}/lib
'';

probably works.

Will give it a try tommorow

@lolbinarycat
Copy link
Contributor

technically it should be lib.getLib clang.cc not clang.lib, in case anyone ever wants to change the outputs of clang (although that is unlikely)

Copy link
Member

@AndersonTorres AndersonTorres left a comment

Choose a reason for hiding this comment

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

Reword the commits

  1. "maintainers: add imrying"
  2. "dstep: init at 1.0.0"

pkgs/by-name/ds/dstep/package.nix Outdated Show resolved Hide resolved
pkgs/by-name/ds/dstep/package.nix Show resolved Hide resolved
pkgs/by-name/ds/dstep/package.nix Outdated Show resolved Hide resolved
pkgs/by-name/ds/dstep/package.nix Outdated Show resolved Hide resolved
pkgs/by-name/ds/dstep/package.nix Outdated Show resolved Hide resolved
pkgs/by-name/ds/dstep/package.nix Outdated Show resolved Hide resolved
@imrying
Copy link
Contributor Author

imrying commented Mar 29, 2024

I guess if it should not be packaged as a binary it will make sense to wait for this PR to be merged in order to have consistency as to how dub packages are built. Even though it does not have any dub dependencies

pkgs/by-name/ds/dstep/package.nix Outdated Show resolved Hide resolved
pkgs/by-name/ds/dstep/package.nix Outdated Show resolved Hide resolved
@imrying
Copy link
Contributor Author

imrying commented Apr 5, 2024

Will update to use buildDubPackage therefore marking as draft until finished

@imrying imrying changed the title dlang dstep: init at 1.0.0 dlang dstep: init at 1.0.4 Apr 6, 2024
@imrying imrying force-pushed the dstep branch 4 times, most recently from bd91c50 to dcafbb9 Compare April 6, 2024 14:12
@imrying imrying marked this pull request as ready for review April 6, 2024 14:24
pkgs/by-name/ds/dstep/package.nix Outdated Show resolved Hide resolved
pkgs/by-name/ds/dstep/package.nix Outdated Show resolved Hide resolved
pkgs/by-name/ds/dstep/package.nix Outdated Show resolved Hide resolved
Copy link
Member

@symphorien symphorien left a comment

Choose a reason for hiding this comment

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

thank you for converting to a source build!

Copy link
Contributor

@jtbx jtbx left a comment

Choose a reason for hiding this comment

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

Tested converting a C header. Thanks!

@symphorien symphorien merged commit 2aa023b into NixOS:master Apr 8, 2024
27 checks passed
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.

None yet

6 participants