Skip to content

unbreak on OpenBSD#259

Closed
buzzdeee wants to merge 3 commits intopuppetlabs:masterfrom
buzzdeee:master
Closed

unbreak on OpenBSD#259
buzzdeee wants to merge 3 commits intopuppetlabs:masterfrom
buzzdeee:master

Conversation

@buzzdeee
Copy link
Copy Markdown
Contributor

OpenBSD doesn't have /etc/environment, therefore there is no
need to fiddle with it. The default case statement, "do nothing"
is all fine here.

Facter::Util::Resolution.with_env is long time gone since
Facter > 2.x. That was even longer before introduced by me.
Remove that OpenBSD special case in the java_version fact,
and assume that $PATH is properly set in order to find
the 'java' binary.

Additionally confine the java_default_home and java_libjvm_path
to OpenBSD kernel as well. If I see posix specs right, 'readlink'
utility is not mentioned there, and the '-e' flag is not portable,
at least not available on the OpenBSD readlink binary.
Because of that, I switched that fact to use Ruby functions to
find the real path of the java binary.
I adapted the tests, but that drives me crazy, I cannot remember
how many attempts I made to get them all to be green.
With the current state, only one is failing for the java_default_home,
but I have no clue what's wrong with it. Any hint/help to get it
right would be much appreciated.

@bastelfreak
Copy link
Copy Markdown
Collaborator

Hi @buzzdeee, thanks for this PR! Can you please rebase against latest master?

need to fiddle with it. The default case statement, "do nothing"
is all fine here.

Facter::Util::Resolution.with_env is long time gone since
Facter > 2.x. That was even longer before introduced by me.
Remove that OpenBSD special case in the java_version fact,
and assume that $PATH is properly set in order to find
the 'java' binary.

Additionally confine the java_default_home and java_libjvm_path
to OpenBSD kernel as well. Add some flesh to the java_default_home
fact to allow it to find the java binary.
use Ruby in a more portable fashion.

adapt tests as well
@buzzdeee
Copy link
Copy Markdown
Contributor Author

@bastelfreak I rebased onto current master, still works for me.
Let me know what else I can do, If you have an idea about the
failing test, I'm all ears.

@bastelfreak
Copy link
Copy Markdown
Collaborator

No idea why it fails :( @hunner?

@wyardley
Copy link
Copy Markdown

wyardley commented Nov 8, 2017

I'm not sure if current FreeBSD has /etc/environment either, FWIW.

@willmeek willmeek mentioned this pull request Nov 15, 2017
@willmeek
Copy link
Copy Markdown

Hi @buzzdeee , thank you for your contribution.

I found that the unit test needed to mock out the realpath call, otherwise it was potentially trying to run this against a non-existent path and failing.

I have added this as a change with your functional changes in PR #265

However I have removed the addition of OpenBSD to the metadata, as we currently don't officially support the module on that platform.

Thank you!

@buzzdeee
Copy link
Copy Markdown
Contributor Author

@willmeek thanks for figuring that out!
I don't mind having OpenBSD listed in the metadata ;)
I'll likely come back in the case it breaks again in the future (:

@willmeek
Copy link
Copy Markdown

Thanks @buzzdeee :)

PR 265 was merged into master, which contained your changes (other than metadata) and the unit test fix.
This should go out with the next release.

Closing out this particular PR.

Thanks
Will

@willmeek willmeek closed this Nov 16, 2017
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.

4 participants