Skip to content

Make Snappy technically an optional dependency#1715

Merged
lbergelson merged 1 commit intomasterfrom
lb_separate_snappy_loading
Sep 17, 2024
Merged

Make Snappy technically an optional dependency#1715
lbergelson merged 1 commit intomasterfrom
lb_separate_snappy_loading

Conversation

@lbergelson
Copy link
Member

  • Separate SnappyLoader into SnappyLoader and SnappyLoaderInternal
    This allows SnappyLoader to function (and correctly respond that snappy is not available)
    in the case where the Snappy library is completely missing. Previously it identified and
    worked around cases where the native code was missing or incompatible, but the java code was required.

  • Snappy is still marked as a dependency in gradle so downstream projects have to actively opt out of it.

  • This is needed to work around notarization issues in the IGV mac builds which strongly discourage native code.

* Separate SnappyLoader into SnappyLoader and SnappyLoaderInternal
  This allows SnappyLoader to function (and correclty respond that snappy is not available)
  in the case where the Snappy libraryis completely missing.  Previously it identified and
  worked around cases where the native code was missing or incompatable, but the java code was required.

* Snappy is still marked as a dependency in gradle so downstream projects have to actively opt out of it.
Copy link
Contributor

@droazen droazen left a comment

Choose a reason for hiding this comment

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

This looks fine to me @lbergelson , provided you've tested both the snappy and no-snappy cases.

@lbergelson
Copy link
Member Author

I tested a good combination of options. IGV with Snappy, snappy excluded but not disabled, and snappy excluded and disabled.

Thank you.

@lbergelson lbergelson merged commit 966737b into master Sep 17, 2024
@lbergelson lbergelson deleted the lb_separate_snappy_loading branch September 17, 2024 16:59
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