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.

Sign up for GitHub

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

Jump to bottom

swiftformat: init at 0.44.0 #67226

Merged
merged 1 commit into from Jan 25, 2020
Merged

swiftformat: init at 0.44.0 #67226

merged 1 commit into from Jan 25, 2020

Conversation

bdesham
Copy link
Contributor

@bdesham bdesham commented Aug 22, 2019

Motivation for this change

SwiftFormat is a code formatting and linting tool for Swift. I requested a derivation for it in #67221; it turned out to be pretty easy to package, with the caveat that it impurely uses the user’s Xcode toolchain. I would welcome an approach that doesn’t require such a hack.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nix-review --run "nix-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@mmahut
Copy link
Member

mmahut commented Aug 22, 2019

@GrahamcOfBorg build swiftformat

@mmahut
Copy link
Member

mmahut commented Aug 22, 2019 •
edited

Ofborg fails with:

xcode-select: error: no developer tools were found at '/Applications/Xcode.app', and no install could be requested (perhaps no UI is present), please install manually from 'developer.apple.com'.

On my mac mini, I get:

/nix/store/ffl5ag647lkglhgd2yjdisajn5zzrr78-stdenv-darwin/setup: line 1325: xcodebuild: command not found.

@bdesham
Copy link
Contributor Author

bdesham commented Aug 24, 2019

@mmahut Darn, maybe this is even more fragile than I had expected. I’m curious: are you able to build the macvim derivation on your machine? I believe I’m using the same trick as that formula, where we’re cheating and copying /usr/bin/xcodebuild into the Nix build environment. But if you don’t have Xcode installed then this won’t work at all.

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/request-for-help-packaging-an-xcode-based-program/3828/1

@lilyball
Copy link
Member

As mentioned on Discourse, the MacVim trick won't work in the sandbox.

bennofs
bennofs requested changes Jan 24, 2020
View reviewed changes
Copy link
Contributor

@bennofs bennofs left a comment

Choose a reason for hiding this comment

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

build currently fails in sandbox?

@lilyball
Copy link
Member

The build relies on Xcode, so yes, it will fail in the sandbox.

@bdesham You can set meta.hydraPlatforms = []; to avoid having hydra waste time building it. This is what macvim does.

@lilyball
Copy link
Member

You may also want to consider borrowing macvim's sandboxProfile as well, which means you can build it locally with sandbox = relaxed. The sandboxProfile in this case will still block network access, and is written to block access to /usr/local (to avoid accidental dependencies on Homebrew).

@bdesham
Copy link
Contributor Author

bdesham commented Jan 25, 2020

@lilyball Thanks for the suggestions! I had assumed that a PR for an impurely-built app would be rejected out of hand, but if not, then maybe this one has a chance. I’ve force-pushed a new version that steals even more from the MacVim derivation and I updated to SwiftFormat 0.44.0 while I was at it.

@NicholasTD07
Copy link

NicholasTD07 commented Jan 25, 2020 via email

@lilyball
Copy link
Member

The PR title needs to be updated

lilyball
lilyball reviewed Jan 25, 2020
View reviewed changes
pkgs/development/tools/swiftformat/default.nix Outdated
let
buildSymlinks = runCommand "swiftformat-build-symlinks" {} ''
mkdir -p $out/bin
ln -s /usr/bin/xcodebuild $out/bin
Copy link
Member

Choose a reason for hiding this comment

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

Do we actually need this? You're not invoking some Makefile that references xcodebuild or anything like that, you're just invoking xcodebuild directly in buildPhase, so I would imagine you could just invoke /usr/bin/xcodebuild instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed you’re right. I’ve force-pushed a new commit that removes that.

@lilyball
Copy link
Member

This passes nix-review on my macOS 10.14.6 machine.

https://github.com/NixOS/nixpkgs/pull/67226
1 package built:
swiftformat

@bdesham bdesham changed the title swiftformat: init at 0.40.11 swiftformat: init at 0.44.0 Jan 25, 2020
@bennofs bennofs merged commit 645a816 into NixOS:master Jan 25, 2020
@bdesham bdesham deleted the swiftformat branch January 25, 2020 23:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lilyball lilyball lilyball left review comments

@mmahut mmahut Awaiting requested review from mmahut

@bennofs bennofs Awaiting requested review from bennofs

Assignees
No one assigned
Labels
8.has: package (new) 10.rebuild-darwin: 0 10.rebuild-linux: 0 11.by: package-maintainer
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@bdesham @mmahut @nixos-discourse @lilyball @NicholasTD07 @bennofs