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

xdg-ninja 0.2.0.1 (new formula) #104762

Closed
wants to merge 2 commits into from

Conversation

superatomic
Copy link
Contributor

  • Have you followed the guidelines for contributing?
  • Have you ensured that your commits follow the commit style guide?
  • Have you checked that there aren't other open pull requests for the same formula update/change?
  • Have you built your formula locally with brew install --build-from-source <formula>, where <formula> is the name of the formula you're submitting?
  • Is your test running fine brew test <formula>, where <formula> is the name of the formula you're submitting?
  • Does your build pass brew audit --strict <formula> (after doing brew install --build-from-source <formula>)? If this is a new formula, does it pass brew audit --new <formula>?

xdg-ninja is a tools that checks your $HOME for unwanted files and directories

@BrewTestBot BrewTestBot added the new formula PR adds a new formula to Homebrew/homebrew-core label Jun 30, 2022
Copy link
Member

@carlocab carlocab left a comment

Choose a reason for hiding this comment

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

Why not also install xdgnj?

Formula/xdg-ninja.rb Show resolved Hide resolved
Formula/xdg-ninja.rb Outdated Show resolved Hide resolved
@superatomic
Copy link
Contributor Author

Why not also install xdgnj?

xdgnj is a developer tool for contributing to xdg-ninja. As such, the average user of xdg-ninja will never use xdgnj.

I would assume that most contributors would prefer to clone the xdg-ninja repository to contribute instead of installing xdgnj via Homebrew as a standalone without the repository (the xdgnj tool might not even be useful for contributing if it is installed with Homebrew).

If you believe that this formula should provide the development tools for xdg-ninja alongside of xdg-ninja itself, I can try to add it regardless, even though I believe it probably shouldn't be installed.

@superatomic superatomic requested a review from carlocab June 30, 2022 08:12
@carlocab
Copy link
Member

Well, we try to have formulae that are feature-complete which means catering to some niche use-cases (but not necessarily all of them).

If the build doesn't take too long and the xdgnj binary isn't too large, then it probably doesn't hurt to build it. That's assuming xdgnj is usable after cloning the xdg-ninja repository separately -- though I don't see why it wouldn't be. (It would be very silly for there to be restrictions about where the xdgnj binary is located on the filesystem relative to the xdg-ninja repository. After all, a developer could put it anywhere after building it from source upon cloning the repo.)

@carlocab carlocab removed their request for review June 30, 2022 09:43
@superatomic
Copy link
Contributor Author

Great! I'll add the xdgnj binary to the formula, although it will be a few days before I will be able to implement it.

Should I also add the name "xdgnj" as a name alias for this formula?

@superatomic
Copy link
Contributor Author

I tried to add xdgnj to the formula, but I run into the following error:

==> stack install --system-ghc --no-install-ghc --local-bin-path=/usr/local/Cellar/xdg-ninja/0.2.0.1/bin
Last 15 lines from /Users/Ethan/Library/Logs/Homebrew/xdg-ninja/01.stack:
2022-07-08 00:05:22 +0000

stack
install
--system-ghc
--no-install-ghc
--local-bin-path=/usr/local/Cellar/xdg-ninja/0.2.0.1/bin

No compiler found, expected minor version match with ghc-9.0.2 (x86_64) (based on resolver setting in /private/tmp/xdg-ninja-20220707-6745-h8ymcy/xdg-ninja-0.2.0.1/stack.yaml).
To install the correct GHC into /private/tmp/xdg-ninja-20220707-6745-h8ymcy/xdg-ninja-0.2.0.1/.brew_home/.stack/programs/x86_64-osx/, try running "stack setup" or use the "--install-ghc" flag. To use your system GHC installation, run "stack config set system-ghc --global true", or use the "--system-ghc" flag.

This is the additional line I have added to the install method in addition to adding depends_on "ghc" => :build and depends_on "haskell-stack" => :build:

system "stack", "install", "--system-ghc", "--no-install-ghc", "--local-bin-path=#{bin}"

If I let stack install it's own version of ghc instead of using Homebrew's version (ghc@9) by removing --system-ghc and --no-install-ghc flags, the build will work, but then I'm pulling a resource from an external source to build, and I don't think that's allowed.

I don't actually know Haskell and I've never used stack (or cabal), so I don't know if there is an easy way to fix this, but I'd imagine that for this to work either ghc-9.0.2 specifically has to be downloaded by the formula, or the dependency file needs to be patched to be less restrictive.

Unless somebody knows how to use a specific version of ghc or how to build successfully while using Homebrew's version of ghc, I'd recommend that xdgnj is simply not installed in this formula.

@carlocab
Copy link
Member

carlocab commented Jul 8, 2022

Yup, fine with leaving it out. Thanks for looking at it!

@carlocab carlocab added the ready to merge PR can be merged once CI is green label Jul 8, 2022
@BrewTestBot
Copy link
Member

:shipit: @chenrui333 has triggered a merge.

@chenrui333
Copy link
Member

@superatomic, thanks for your contribution to Homebrew! 🎉 🥇

Without awesome contributors like you, it would be impossible to maintain Homebrew to the high level of quality users have come to expect. Thank you!!!!

@github-actions github-actions bot added the outdated PR was locked due to age label Aug 9, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 9, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
new formula PR adds a new formula to Homebrew/homebrew-core outdated PR was locked due to age ready to merge PR can be merged once CI is green
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants