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

New cask for minecraft-server #13893

Closed

Conversation

chrisRidgers
Copy link

I've added a new cask file which pulls down the minecraft-server.1.8.8.jar
minecraft.net and accepts the eula, as well as creating a binary to run
from the command line. Future work could include testing for 64 bit
architecure and passing the appropriate flag to the java call (-d64);
allowing the passing of options to the binary; and better control of the
version of minecraft-server to be pulled down. I'm unsure what Mojang's
release pattern is.

I've created a new cask that pulls down the minecraft server jar and
set's up a binary linking to it.  Some potential changes for anyone
reviewing it include testing for 64bit architecture as part of
the binary and if present passing '-d64' to the java call as an
argument'.  Also, if there's a sane way to allow for the application to
run in the background, that would be a nice default, along with the
ability to pass options through to the java call.

I've added in an extra line to the caveats.
@chrisRidgers
Copy link
Author

Not got a lot of experience with pull requests and CI builds. Can anyone advise on what I should do: far as I can tell I shouldn't be setting a ruby version anywhere in my proposed cask file, but that's what it looks like travis failed on.

@victorpopkov
Copy link
Member

There are most likely some issues with Travis CI servers at the moment, but the cask itself seems good in most parts.

When the version is known, you can use string interpolation in download URL. In addition, if the homepage and download URL hosts differ the comment specifying that should be included:

  # amazonaws.com is the official download host per the vendor homepage
  url "https://s3.amazonaws.com/Minecraft.Download/versions/#{version}/minecraft_server.#{version}.jar"

Furthermore, if the version is specified, the sha256 also needs to be set in most cases.

In caveats block you can use #{token} and #{staged_path}:

  caveats do
    <<-EOS.undent
      To run this app, type "#{token}" in terminal.
      To configure the server take a look at the files staged at #{staged_path}
    EOS
  end

@victorpopkov
Copy link
Member

Thank you for the contribution. It needed some fixes, so they were made in 2b9660d. Your contribution is still included (and still credited to you), with the appropriate modifications. Please feel free to ask about any of the changes.

@chrisRidgers
Copy link
Author

Looks good,

I can see some of the changes. Cheers for merging.

I've seen version numbers being used before for urls, but I'm not sure how consistent mojang will be with the server releases.

@jawshooah
Copy link
Contributor

Wish I had seen this earlier. While we have agreed that shim scripts are a good thing (#9203), I think we should wait until we have augmented the DSL with helpers to generate them before accepting submissions like this.

We can leave this cask as-is for now, since the issue has been linked to the discussion as of this comment, but we should hold off on future submissions that include shims until we have a stable implementation in place.

@jawshooah
Copy link
Contributor

Pinging @vitorgalvao for his thoughts.

@vitorgalvao
Copy link
Member

Reading again bits and pieces from the shim discussions, I got reminded we do still have some solutions like this one hanging around in old PRs, some of those yours, @jawshooah, and it’s a shame that they all work but are being held of.

In the spirit of organising issues1, I wouldn’t be opposed to actually merging those PRs that currently have similar solutions to this one, and having a new issue similar to #12858 that keeps track of them, with a future label. Advantages:

  1. Get those casks working now.
  2. Have less old issues hanging around.
  3. We get a list of real cases that need shim scripts.
  4. When shim script helpers are actually implemented, we have a central location to know which casks to update.

Point 3 is sounding really good to me, actually, and is the one convincing me the most. If we have real use cases of casks that need shim scripts and are working, then we can better design the helpers since we have real cases where those have to fit.

How does that sound?


1 Finding old ones with a huge discussion, grabbing the main points and opening a new issue just with those, so we can actually start working on them without having to go through all of the cruft of how a decision was made.

@Homebrew Homebrew locked and limited conversation to collaborators May 8, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants