Created attachment 166007 [details]
comms/openzwave testport 10.2 amd64 output

Comment 2 Kubilay Kocak 2016-01-23 15:23:00 UTC

Thanks Johan

Review items:

* Remove "Open-zwave - An " from COMMENT. portlint should have picked this up (You can also enable PORTLINT in poudriere)

* Look like the PORTVERSION should be 1.4 (or theyre not tagging their repo properly)
* Use DISTVERSIONPREFIX=v, then you wont need to override GH_TAGNAME
* DOXYGEN option should be named the standard: DOCS. What DOCS depends on is immaterial.
* Use an OPTIONS helper instead of the .if ${PORT_OPTIONS:MDOXYGEN} block
* Whitespace align Makefile entries
* Add LICENSE_FILE if one exists in WRKSRC

Hi, thanks for quick feedback!

re PORTLINT in poudriere, I build all my ports there so a global config to do lint is not what I'd prefer (I guess).

PORTVERSION: Yes, it is tagged 1.4, but i.e. openzwave.spec (RHEL) has version 1.4.. The build script needs an env var VERSION_REV, which rhel/debian scripts derive from version nr. So I think it makes sense to use 1.4.0 even though the tag is invalid. Will forward this for future tags. Agree?

DISTVERSIONPREFIX: ah! Though, I guess I need to override anyway if I set PORTVERSION=1.4.0

DOXYGEN: check!

OPTIONS helper: can you clarify what an options helper is in this context?

Comment 4 Kubilay Kocak 2016-01-23 16:01:12 UTC

(In reply to johan from comment #3)

Then ensure you run portlint manually, and confirm in your issues that you've run it and it passes, or WARNS are false positives.

I'll leave version up to you, though it would be nice if there weren't two formats of versions. Any reason you decided to use github sources, instead of the actual release tarballs, which presumably have this version magic already prr-processed?

"OPTIONS helpers" are those defined in Mk/bsd.options.mk. For details see that file, and Porters Handbook, Section 5.12.3. Options Helpers

But basically, you ought to be able to do:

<OPTIONNAME>_PORTDOCS=blah instead of the if/BLAH=/endif block

portlint -AC did not warn on 'Open-zwave - An open-source...', but it *does* warn on 'An open-source...'. Fixed.

Hm, actually I missed the fact that there where release tarballs (which *does* have revision, even a 1.4.1 which there is no tag for). I've been running a local openzwave-devel port which fetched from GH before, so this one was based on that. Will check the tarballs instead, and also investigate what 1.4.1 has that 1.4.0 lacks, and which one corresponds to v1.4 tag on GH.

Ah, will fix the portdocs things. 
One more thing: is it recommended/required to have DOCS enabled by default? On this port, it requires that both doxygen and graphiviz is installed, making the build a looooot heavier in terms of build-deps.

Will look into the details tomorrow. Thanks for very quick feedback!

Regards
Johan

Comment 6 Kubilay Kocak 2016-01-23 16:17:03 UTC

(In reply to johan from comment #5)

OPTIONS_DEFAULT it as:

* Package users don't need build dependencies
* Ports users can OPTIONS_UNSET=DOCS, so they have a choice (Hence my recommendation for renaming the OPTION :])

(In reply to Kubilay Kocak from comment #6)

Ah, of course! Now uses DOCS_PORTDOCS=.. but one if-blocks for docs is left in post-install.

Ok, so I've reviewed the openzwave-1.4.1 tar-ball, and it is almost identical to the v1.4 tag. The only difference is the cpp/src/vers.cpp file which indicates version 1.4.1, and some windows build scripts which differs.
Fixed up so we use the proper tar-ball now, no more github!

portlint -AC output:

WARN: Makefile: [45]: use of != in assignments is almost never a good thing to do.  Try to avoid using them.  See http://lists.freebsd.org/pipermail/freebsd-ports/2008-July/049777.html for some helpful hints on what to do instead.
0 fatal errors and 1 warning found.

Created attachment 166053 [details]
comms/openzwave 1.4.1 port

Cleaned up port

Hello,

A small ping on this port. I need it to link www/domoticz with it :D

Regards,
Xavier

Any news ? I really needs this to update www/domoticz to latest version.

Comment 11 Kubilay Kocak 2016-07-24 13:36:49 UTC

Apologies for the delay Johan, landing this now

Comment 12 Kubilay Kocak 2016-07-24 14:51:38 UTC

@Johan, I'm seeing the following during QA and testing:

===>   Registering installation for openzwave-1.4.1
(openzwave-1.4.1) /wrkdirs/usr/ports/comms/openzwave/work/stage//usr/local/bin/MinOZW - required shared library /wrkdirs/usr/ports/comms/openzwave/work/openzwave-1.4.1/libopenzwave.so not found
[103amd64-user] Installing openzwave-1.4.1...
===========================================================================
====>> Checking shared library dependencies
 0x0000000000000001 (NEEDED)             Shared library: [/wrkdirs/usr/ports/comms/openzwave/work/openzwave-1.4.1/libopenzwave.so]
 0x0000000000000001 (NEEDED)             Shared library: [libc++.so.1]
 0x0000000000000001 (NEEDED)             Shared library: [libc.so.7]
 0x0000000000000001 (NEEDED)             Shared library: [libcxxrt.so.1]
 0x0000000000000001 (NEEDED)             Shared library: [libgcc_s.so.1]
 0x0000000000000001 (NEEDED)             Shared library: [libm.so.5]
 0x0000000000000001 (NEEDED)             Shared library: [libthr.so.3]
 0x0000000000000001 (NEEDED)             Shared library: [libusb.so.3]
=======================<phase: deinstall      >============================

[user@CURRENT-amd64:/usr/home/user/repos/freebsd/ports/comms/openzwave] MinOZW
Cannot open "/usr/home/user/repos/freebsd/ports/comms/openzwave/work/openzwave-1.4.1/libopenzwave.so"

I've tried to tinker with the LIBDIR variable (pointing into STAGEDIR/LOCALBASE/lib, leaving it out completely) to try to isolate the cause and a possible fix, but I've come up trumps.

You'll want to set DEVELOPER=yes in /etc/make.conf and test it with poudriere.

It would also be fantastic improving the Makefiles upstream to enable verbose building to allow for easy debugging. For some examples, see:

http://stackoverflow.com/questions/3861911/ignoring-at-symbol-in-makefiles

Comment 13 Kubilay Kocak 2016-07-24 15:05:29 UTC

I've temporarily remove the silencing @'s in support.mk and we're also going to need to clean up *FLAGS appending that's happening:

 -c -Wall -Wno-unknown-pragmas -Werror -Wno-format -Wno-error=sequence-point -Wno-sequence-point -O3 -DNDEBUG

In particular -Werror shouldn't be in release flags and *FLAGS should all honour user supplied values (prepended to, not appended to)

Just wanted to check on the status of this port.  Thanks! 

Bryce

Sorry, haven't had any time to look further at this. If anyone else want's to take a stab at it, feel free to do so!

Since I need this port... How can I help ?

(In reply to Kubilay Kocak from comment #13)

Kubilay -

Where can I get to the changes you reference (in support.mk, etc)?  I'd like to take a shot at trying to create a working port submission.  Thanks!

Bryce

Comment 18 Kubilay Kocak 2016-10-09 09:41:43 UTC

(In reply to Bryce Edwards from comment #17)

I believe support.mk is part of the upstream sources (in WRKSRC after make extract'ing)

Ping ? My www/domoticz port is waiting _for_ages_ to this to be committed.

Can someone check and see what is the issue ? 

Regards!

Hi,

good thing you wrote.. I actually had the thought to pick this up a few days ago, so now I did!

So, I have identified the issue. patch-cpp_build_Makefile replaced the LDFLAGS and accidentally left out -Wl,-soname,libopenzwave.so.$(VERSION).
Re-adding that fixed the linking.


Now, there have been a lot of changes since 1.4.1.. These are listed in the CHANGELOG file under v1.5... But I'm not sure if there have a proper release of 1.5 since, there is a 1.5 tag which points to 2 commits after 1.4.

While commiting the above fix upstream [1], I asked about the status of 1.5. If no release is imminent, I submit an updated patch for 1.4 here, and we can go on. Otherwise we might want to go with 1.5 (or whatever it might be).



[1] https://github.com/OpenZWave/open-zwave/pull/1034

Created attachment 176764 [details]
openzwave 1.4.2238

The github master is 1.4 maintenance branch, and official version numbers are the github incremental version [1].
So as of this moment, the latest 1.4 version is 1.4.2238.

The attached port (1.4.2238) seems to build fine without any warnings or issue from testport.
All previous patches are merged upstream.

I have not taken a stab on rewriting CFLAGS etc as previously discussed, I don't feel that we need to do that specifically for the FreeBSD port but it may rather be something to be done upstream.

I hope this is good enough to make it into the tree now, let me know otherwise so we can finally close this one.. :)


[1] https://github.com/OpenZWave/open-zwave/pull/1034

Ping @ Kubilay Kocak, or someone else who might be willing to take over..

Any progress?

Created attachment 179124 [details]
comms/openzwave shar - version 1.4.2360

Here's an updated shar file for a more recent version that works.

Wasn't sure if I should put myself as maintainer, but I don't mind if that is acceptable.

One change I made to tweak the XML files to got to DATADIR instead of ETCDIR as they are more appropriate to go into /usr/local/share/PORTNAME.

Great with some more interested people :)

I have no problem with giving over maintainership to someone else.

One thing about your DATADIR change; this should be reflected in the code/build, so that it can find the default location.

Currently this happens in cpp/src/Options.cpp, and it looks in the following dirs:

* app-specific dir
* "./config"
* "/etc/openzwave"
* SYSCONFDIR

cpp/build/Makefile defines SYSCONFDIR as $(PREFIX)/etc/openzwave/

Thus, I propose adding a Makefile patch to use set SYSCONFDIR to %%DATADIR%% instead (expanded by the build process).

Created attachment 179165 [details]
Latest comms/openzwave shar

Here's the latest version.  I left the xml files in ETCDIR for now, as the application expects that and I'll work to  tweak it upstream.  At that time, i'll incorporate into a future port version.

Created attachment 179170 [details]
comms/openzwave shar - version 1.4.2360 with fixes

One last time, fixed a lint error.

Comment 28 Kubilay Kocak 2017-08-27 01:53:19 UTC

Looking at this again today

ping!

Just broke 2 year since I created ticket ...:)

Fyi, 1.4 is still latest stable release, at this moment 1.4-2939 is tip of master aka latest release.

May I ask the status of this PR? Personally I'm to soon start working on my home automation, and it would be really nice to have openzwave in the ports tree.

Comment 32 Kubilay Kocak 2018-07-20 04:00:18 UTC

I've just fixed a couple of outstanding issues in the port (see below) and am QA'ing again

- verbose builds (the build is silenced without ability to override it without patching)
- the build clobbers user/system supplied *FLAGS (RELEASE_CFLAGS has -Werror in it for example, and fails to build because of a warning)

Comment 33 Kubilay Kocak 2018-07-20 06:33:53 UTC

Updated patch available at https://reviews.freebsd.org/D16362

Changes:

- Patch build files to make DOCS and/or DOT optional, which facilitates removing post-install-DOCS-off target, allowing...
- Remove of dot as a dependency

- Fix port compliance issues:

  * Section ordering
  * Use of !=
  * Respect *FLAGS (user/system supplied), allowing ..
  * Removal (overriding) of -Werror
  * Verbosify build

The last three issues above and allowing DOCS/DOT to be enabled via make env/args should be fixed upstream, either using the patches provided, although the 'verbosify build' patch is a quick hack, and needs to be turned into a command line toggle like `make V=(0|1)` or similar.

Can all interested parties please test this at your ends

@Bryce, are you happy to continue to be MAINTAINER?

If so,

@Johan, can you please approve the 'change of maintainer' to Bryce

Approving Bryce as maintainer (without nothing to base that on besides me not being maintainer is OK for me).

Regarding the patch, it looks good but is based on a pretty old version. I'd recommend to use the latest snapshot from http://old.openzwave.com/snapshots/ aka tip of master. This is what is considered more or less stable, development work takes place in the Dev branch and master mostly gets minor fixes and device information updates (which is required and wanted for usage with newly released HW).

Comment 35 Kubilay Kocak 2018-07-20 11:51:33 UTC

(In reply to Johan Ström from comment #34)

I'd prefer to get the version we have here committed, unless someone can use my latest patch as a base to:

1) update it to whatever latest version they want
2) confirm thorough QA passing (poudriere across freebsd {10,11,12}{i386,amd64} minimally, and 
3) confirm it is *solely* a PORTVERSION/distinfo update

Re: the Bryce as maintainer question, if you'd like to maintain the port, as the original submitter, just say the word. Alternatively organise between yourself and Bryce based on who has the time/commitment to give to the port (ideally its someone who uses it), and we'll go from there. 

Note: Nothing is set in stone, and nothing prevents both of you working collaboratively to keep it updated and in good working order.

(In reply to Kubilay Kocak from comment #35)

I'm fine with commiting the existing update and then do another update with version/distinfo only (although a few XML files with device descriptions might be updated/added).

Re maint, I have no need to be maintainer so if Bryce want's to take it, I'm totally fine with that :)

I'm no longer using this port, so Jonan you are welcome to keep maintainer status if that is OK with you.

Comment 38 Kubilay Kocak 2018-07-24 03:43:51 UTC

(In reply to Bryce Edwards from comment #37)

Thanks for the update Bryce, I'll revert maintainer back to Johan in the review diff.

Would still like people to test the review patch at there end, in particular for/at run-time

This bug is approaching 3 years old, can we please go ahead and close it?

Just tested the D16362 patch (from URLs), and it does not build due to unused fields in code.

If I recall correctly, that was some other version which was submitted here, that included patches for that. but more importantly, it has been fixed upstream in newer versions: https://github.com/OpenZWave/open-zwave/pull/1580

I suggest moving the D16362 patch to the latest OZW 1.4 version, and submitting that.

I wait also on this port to add Official OpenZwave support to www/domoticz (My own internal port work with OZW for more than 2 years).

Created attachment 199476 [details]
OpenZWave 1.4.3254

This patch replaces any prior ones. 

The Makefile is based on the one in https://reviews.freebsd.org/D16362, but this uses the latest 1.4 version (1.4.3254 == current master @ f339aa6) which builds fine on FreeBSD without any modifications.

Comment 43 Kubilay Kocak 2019-03-26 13:08:53 UTC

(In reply to Johan Ström from comment #42)

Will re-look at this this week

(In reply to Johan Ström from comment #42)

Ran portlint: OK
Ran poudriere testport 12 amd64: OK

Manually built and installed in a seperate jail: OK

Comment 45 commit-hook 2019-03-27 06:00:51 UTC

A commit references this bug:

Author: koobs
Date: Wed Mar 27 06:00:13 UTC 2019
New revision: 496935
URL: https://svnweb.freebsd.org/changeset/ports/496935

Log:
  comms/openzwave: Open-source interface to Z-Wave networks

  Free software library that interfaces with selected Z-Wave PC controllers,
  allowing anyone to create applications that manipulate and respond to
  devices on a Z-Wave network, without requiring in-depth knowledge of the
  Z-Wave protocol.

  WWW: http://www.openzwave.net/

  PR:		206526
  Submitted by:	Johan Str?m <johan stromnet se> (with changes)

Changes:
  head/comms/Makefile
  head/comms/openzwave/
  head/comms/openzwave/Makefile
  head/comms/openzwave/distinfo
  head/comms/openzwave/files/
  head/comms/openzwave/files/patch-cpp_build_Makefile
  head/comms/openzwave/files/patch-cpp_build_support.mk
  head/comms/openzwave/pkg-descr
  head/comms/openzwave/pkg-plist

Wow! Finally, thanks :)

The now commited version is 1.4.3254. The latest one today is .3429, but looking at the version that other distros is having (http://old.openzwave.com/snapshots/), .3355 seems to be popular.

I'll check with the community if there is any specific reason for having that one, or what the best practices are.

Comment 47 Kubilay Kocak 2019-03-27 06:09:15 UTC

patches contained in review but not in attachment 199476 [details] to cpp/build/* needed to be rebased and re-added

Other changes:

- PLIST_SUB removed (not used in pkg-plist)
- Use DOXYGEN as option, as DOCS shouldn't install large deps. Add DOCS option, necessary to use PORTDOCS variable, and implied (set) when enabling DOXYGEN.

Maintainer, please work with the openzwave project to upstream the local patches:

- respecting user *FLAGS (never append to *FLAGS)
- not using -Werror in release builds
- Making build verbose by default, optionally silent (make V=0)