Skip to content
This repository has been archived by the owner on Jul 4, 2023. It is now read-only.

Add RDM tests option to OLA #48575

Closed
wants to merge 4 commits into from
Closed

Add RDM tests option to OLA #48575

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
321e1ff
Make libusb and liblo optional
peternewman Jan 29, 2016
ed1b7b7
Add an rdm-tests option
peternewman Jan 29, 2016
fa7a04f
ola: fix PR comments
peternewman Jan 31, 2016
0256134
OLA: Fix the remaining comment
peternewman Mar 1, 2016
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
13 changes: 11 additions & 2 deletions Library/Formula/ola.rb
View file
Open in desktop
Expand Up @@ -20,14 +20,16 @@ class Ola < Formula

option :universal
option "with-ftdi", "Install FTDI USB plugin for OLA."
option "with-rdm-tests", "Install RDM Tests for OLA."
# RDM tests require protobuf-c --with-python to work

depends_on "pkg-config" => :build
depends_on "cppunit"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is there a :test dependency flag? We only need cppunit for the tests.

Copy link
Member

Choose a reason for hiding this comment

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

No, there isn't.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay thanks. Any plans to do so? If I understand your bottle concept correctly, will that not still drag the test deps in, despite the fact your unlikely to bother testing the binary?

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'd like to add a TODO if there's an issue covering adding a :test dependency flag.

depends_on "protobuf-c"
depends_on "libmicrohttpd"
depends_on "libusb"
depends_on "liblo"
depends_on "ossp-uuid"
depends_on "libusb" => :recommended
depends_on "liblo" => :recommended
depends_on :python => :optional
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is this line redundant, now its been duplicated on line 44?

Copy link
Member

Choose a reason for hiding this comment

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

I chopped it in post before merging.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah yes, just spotted that. Thanks.

depends_on "doxygen" => :optional

Expand All @@ -36,6 +38,12 @@ class Ola < Formula
depends_on "libftdi0"
end

if build.with? "rdm-tests"
depends_on :python if MacOS.version <= :snow_leopard
else
depends_on :python => :optional
end

def install
ENV.universal_binary if build.universal?

Expand All @@ -47,6 +55,7 @@ def install
]

args << "--enable-python-libs" if build.with? "python"
args << "--enable-rdm-tests" if build.with? "rdm-tests"
args << "--enable-doxygen-man" if build.with? "doxygen"

system "autoreconf", "-fvi" if build.head?
Expand Down