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

sqitch 1.3.1 (new formula) #129128

Closed
wants to merge 1 commit into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
82 changes: 82 additions & 0 deletions Formula/sqitch.rb
@@ -0,0 +1,82 @@
class Sqitch < Formula
desc "Sensible database change management"
homepage "https://sqitch.org/"
url "https://www.cpan.org/authors/id/D/DW/DWHEELER/App-Sqitch-v1.3.1.tar.gz"
sha256 "f5e768d298cd4047ee2ae42319782e8c2cda312737bcbdbfaf580bd47efe8b94"
license "MIT"
head "https://github.com/sqitchers/sqitch.git", branch: "develop"
theory marked this conversation as resolved.
Show resolved Hide resolved

depends_on "cpm" => :build
depends_on "libiodbc"
depends_on "libpq"
depends_on "mysql-client"
depends_on "perl"
uses_from_macos "sqlite"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I changed this from depends_on at the recommendation of brew audit --new, but as there is an sqlite formula that will install a newer version than on the OS, ISM it might be better kept depends_on.


if build.head?
depends_on "gettext" => :build
depends_on "openssl@3" => :build
end

def install
# Download Module::Build and Menlo::CLI::Compat.
cpm_args = %w[install --local-lib-contained instutil --no-test]
cpm_args.push("--verbose") if verbose?
system "cpm", *cpm_args, "Menlo::CLI::Compat", "Module::Build"
Comment on lines +22 to +25
Copy link
Member

Choose a reason for hiding this comment

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

These should be resources 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.

You mean like this? Woof, that would be a lot.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, otherwise it could be installing anything and we want versioned dependencies.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense, I'm working on something to generate it from that cpan file. I presume I can put additional dependencies required to build --HEAD inside a if build.head? block and they'll only be downloaded for head builds, yes?


ENV["PERL5LIB"] = "#{buildpath}/instutil/lib/perl5"
ENV["PERL_MM_OPT"] = "INSTALLDIRS=vendor"
ENV["PERL_MB_OPT"] = "--installdirs vendor"

if build.head?
# Need to tell the compiler where to find Gettext.
ENV.prepend_path "PATH", Formula["gettext"].opt_bin

# Download Dist::Zilla and plugins, then make and cd into a build dir.
system "cpm", *cpm_args, "Dist::Zilla"
system "./instutil/bin/dzil authordeps --missing | xargs cpm " + cpm_args.join(" ")
system "./instutil/bin/dzil", "build", "--in", ".brew"
Dir.chdir ".brew"
end

# Assemble the Build.PL args, including supported native and ODBC engines.
# Oracle and Firebird not currently supported.
args = %W[
Build.PL --install_base #{prefix} --etcdir #{etc}/sqitch
--with postgres --with sqlite --with mysql
--with vertica --with exasol --with snowflake
]
args.push("--verbose") if verbose?
args.push("--quiet") if quiet?

# Build and bundle (install).
system "perl", *args
system "./Build", "bundle"

# Wrap the binary in client paths.
mkdir_p libexec
mv bin/"sqitch", libexec/"sqitch"
paths = [
Formula["libpq"].opt_bin,
Formula["mysql-client"],
]
(bin/"sqitch").write_env_script libexec/"sqitch", PATH: "#{paths.join(":")}:$PATH"

# Move the man pages from #{prefix}/man to man.
mkdir share
mv "#{prefix}/man", man
end

def caveats
<<~EOS
This Sqitch install includes clients to manage Postgres, MySQL, and SQLite.
See https://sqitch.org/download/macos/ to install the client libraries
required to manage Snowflake, Exasol, and Vertica. Use the sqitchers/sqitch
tap to manage Oracle or Firebird.
EOS
end
Comment on lines +70 to +77
Copy link
Member

Choose a reason for hiding this comment

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

We don't want external references here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hrm. Should I instead throw in a file and tell the user to read it for more details?

Copy link
Member

Choose a reason for hiding this comment

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

A lot of users skip the caveats, might be easier to just put something to this extend in your documentation.


test do
system bin/"sqitch", "--version"
Copy link
Member

Choose a reason for hiding this comment

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

We need a test that exercises the some of the functionality of the app. Version checks or usage checks (foo --version or foo --help) are not sufficient, as explained in the formula cookbook.

In most cases, a good test would involve running a simple test case: run #{bin}/foo input.txt.

  • Then you can check that the output is as expected (with assert_equal or assert_match on the output of shell_output)
  • You can also check that an output file was created, if that is expected: assert_predicate testpath/"output.txt", :exist?

Some advice for specific cases:

  • If the formula is a library, compile and run some simple code that links against it. It could be taken from upstream's documentation / source examples.
  • If the formula is for a GUI program, try to find some function that runs as command-line only, like a format conversion, reading or displaying a config file, etc.
  • If the software cannot function without credentials, a test could be to try to connect with invalid credentials (or without credentials) and confirm that it fails as expected.
  • Same if the software requires a virtual machine, docker instance, etc. to be running.

Copy link
Contributor Author

@theory theory Apr 24, 2023

Choose a reason for hiding this comment

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

I could run ./Build test there if you don't mind the overhead.

Copy link
Member

Choose a reason for hiding this comment

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

No, it should be something a user would run.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, this is more like an install test; no source here I presume. Will think about it.

end
end