catboost-cli 1.2.1 (new formula) #140960
catboost-cli 1.2.1 (new formula) #140960
Conversation
github-actions
bot
added
the
new formula
Formula/catboost-cli.rb
Outdated
There was a problem hiding this comment.
I've added the version to the binary name because when you ask catboost for its version, you get this:
Arc info:
Branch: unknown-vcs-branch
Commit: 0577215664901532860606512090082402431042
Author: ordinal
Summary: No VCS
So, no info about its version is reported.
There was a problem hiding this comment.
Can we pass the version information to the build somehow?
There was a problem hiding this comment.
I've changed the strategy to download the source code.
By cloning the git repository, the VCS information is properly added:
Git info:
Commit: d03b246cae23490dcf991cf822be110d6f818665
Branch: tags/v1.2.1
Author: akhropov <[email protected]>
Summary: Release 1.2.1..
git-svn info:
Last Changed Rev: 9293085
So now, I'm testing:
assert_match %r{Branch: tags/v#{version}}, shell_output("#{bin}/catboost --version")
cdalvaro
force-pushed
the
catboost-cli
branch
4 times, most recently
from
8c92721 to
fcaa17b
Compare
cdalvaro
changed the title
|
Since we have already sharded all the formulae, this formula should go to the |
Formula/catboost-cli.rb
Outdated
There was a problem hiding this comment.
no need to add livecheck, the default one just works out fine.
| livecheck do | |
| url :stable | |
| strategy :github_latest | |
| end |
Formula/catboost-cli.rb
Outdated
There was a problem hiding this comment.
| on_linux do | |
| depends_on "llvm" => :build | |
| end | |
| uses_from_macos "llvm" => :build |
Formula/catboost-cli.rb
Outdated
There was a problem hiding this comment.
we need a better test
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.
There was a problem hiding this comment.
I've managed to add a test that checks the installed binary is working as expected.
cdalvaro
force-pushed
the
catboost-cli
branch
from
fcaa17b to
9e6d9a0
Compare
Formula/c/catboost-cli.rb
Outdated
There was a problem hiding this comment.
This is cleaner than adding a ninja dependency. What is the use of CMAKE_TOOLCHAIN_FILE?
| extra_cmake_args = [ | |
| "-S", | |
| buildpath.to_s, | |
| "-B", | |
| "#{buildpath}/brew-build", | |
| "-G", | |
| "Ninja", | |
| "-DCATBOOST_COMPONENTS=app", | |
| "-DHAVE_CUDA=NO", | |
| "-DCMAKE_TOOLCHAIN_FILE=#{buildpath}/build/toolchains/clang.toolchain", | |
| ] | |
| system "cmake", *extra_cmake_args, *std_cmake_args | |
| system "ninja", "-C", "#{buildpath}/brew-build", "catboost" | |
| args = [ | |
| "-DCATBOOST_COMPONENTS=app", | |
| "-DHAVE_CUDA=NO", | |
| "-DCMAKE_TOOLCHAIN_FILE=#{buildpath}/build/toolchains/clang.toolchain", | |
| ] | |
| system "cmake", "-S", ".", "-B", "build", *args, *std_cmake_args | |
| system "cmake", "--build", "build" | |
| system "cmake", "--install", "build" |
There was a problem hiding this comment.
Done!
Although system "cmake", "--install", "build" does nothing. So I've replaced this line by:
bin.install "catboost"There was a problem hiding this comment.
I've reverted this change, and now the formula is compiling right using ninja...
cdalvaro
force-pushed
the
catboost-cli
branch
8 times, most recently
from
90080a3 to
e3f45dd
Compare
|
Thank you @chenrui333 and @SMillerDev for all your comments! |
cdalvaro
force-pushed
the
catboost-cli
branch
2 times, most recently
from
be040a8 to
27fb92a
Compare
|
I'm not sure why the formula is installing on my Mac but not passing the CI... Apparently, when compiling the binary inside the CI environment, the following error arises: But I don't understand why I'm not seeing this issue when building the formula from source on local. Edit: Using Ninja as compiler the formula is working |
cdalvaro
force-pushed
the
catboost-cli
branch
from
27fb92a to
b94cc66
Compare
|
Catboost bundles a bunch of third-party libraries (see https://github.com/catboost/catboost/tree/master/contrib/libs). |
These are the third-party packages and libraries installed with Conan: Although I don't known how Conan works and if there is a method to force Conan to use system (homebrew) libraries instead of downloading them. Full log attached. |
cdalvaro
force-pushed
the
catboost-cli
branch
from
b94cc66 to
514d571
Compare
Right, and along with this, I didn't find a way to override these to system dependencies in the documentation (https://catboost.ai/en/docs/installation/build-native-artifacts), or an obvious way in cmake files. I think, at this point |
OK, I understand. I'll keep using |
HOMEBREW_NO_INSTALL_FROM_API=1 brew install --build-from-source <formula>, where<formula>is the name of the formula you're submitting?brew test <formula>, where<formula>is the name of the formula you're submitting?brew audit --strict <formula>(after doingHOMEBREW_NO_INSTALL_FROM_API=1 brew install --build-from-source <formula>)? If this is a new formula, does it passbrew audit --new <formula>?