Skip to content

Support Sisco - #17

Open
gmloose wants to merge 14 commits into
masterfrom
support-sisco
Open

gmloose wants to merge 14 commits into
masterfrom
support-sisco

Conversation

@gmloose

@gmloose gmloose commented Jun 22, 2026

Copy link
Copy Markdown
Collaborator

In order to build sisco, the package libdeflate must be installed.

In order to build `sisco`, the package `libdeflate` must be installed.
@gmloose gmloose self-assigned this Jun 22, 2026
gmloose added 12 commits June 22, 2026 17:57
Hopefully fixes Formula for casacore with new `sisco` build option.
I forgot to remove the conditional test for `sisco`.
Conditional packages need to go into their own group, after required packages.
Changed the option to `without-sisco` to be in line with `without-python`.
Bumped versions to `ubuntu-24.04` and `macos-15`.
Some actions use soon deprecated NodeJS 20; bumped to latest version.
Homebrew/actions switched default branch from `master` to `main`. Update the GitHub workflow that uses these.
Overlooked a couple of usages of `master`; these have now also been changed to `main`.
Comment thread Formula/casacore.rb Outdated
depends_on "readline"
depends_on "wcslib"

depends_on "libdeflate" => :optional

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't know what this optional does, I don't think you've told homebrew that the libdeflate dependency is related to sisco. Since libdeflate is in homebrew, I would be fine with making the dependency non-optional.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

This seems to be the modern way (according to ChatGPT) to use a conditional. I first tried:

    if build.with?("sisco")
        depends_on("libdeflate")
    endif

But I received errors and warning that I didn't understand. The version with ":optional" passed the test.

Comment thread Formula/casacore.rb Outdated
head "https://github.com/casacore/casacore.git"

option "without-python", "Build without Python bindings"
option "without-sisco", "Build without Sisco compression storage manager"

@gmloose gmloose Jun 23, 2026 •

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Also I don't know exactly how this works. Whether this means "with" or "without" if nothing is specified on the command line. I'm not even sure we need all this if we enable Sisco by default in the casacore build. Then probably just the following single line suffices:

depends_on "libdeflate"

And we don't need to add the CMake argument on line 52 either. What do you think?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I suggest to make the dependency non-optional, and just build sisco always. There are many other parts of casacore that could be built optional in homebrew and we don't do that either.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

OK, so shall I then convert this PR to just a one-liner that adds "libdeflate" as dependency?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yes please

Reverted all the changes to the `casacore.rb` Formula, and simply added `libdeflate` as dependency.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants