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

ansible-cmdb 1.6 (new formula) #45344

Closed
wants to merge 1 commit into from
Closed

Conversation

hryamzik
Copy link
Contributor

No description provided.

@dunn
Copy link
Contributor

dunn commented Oct 27, 2015

Looks like the makefile is trying to write directly to /usr/local/lib instead of to the Cellar: http://bot.brew.sh/job/Homebrew%20Pull%20Requests/35392/version=el_capitan/testReport/junit/brew-test-bot/el_capitan/install_ansible_cmdb/

depends_on "libyaml"

def install
system "make", "install"
Copy link
Contributor

Choose a reason for hiding this comment

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

The solution is probably just to pass a prefix argument to make.

@hryamzik
Copy link
Contributor Author

@dunn so does it have to be patched?

@dunn
Copy link
Contributor

dunn commented Oct 27, 2015

If the Makefile doesn't allow the install prefix to be customized, then yeah.

desc "Ansible-cmdb takes the output of Ansible's fact gathering and converts it into a static HTML overview page containing system configuration information."
homepage "https://github.com/fboender/ansible-cmdb"
url "https://github.com/fboender/ansible-cmdb/releases/download/1.6/ansible-cmdb-1.6.zip"
version "1.6"
Copy link
Member

Choose a reason for hiding this comment

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

Suspect this is redundant. We can handle reading simple URLs for version numbers.

@hryamzik
Copy link
Contributor Author

@DomT4 you mean version could be omitted or used as a variable?

@hryamzik
Copy link
Contributor Author

 * Non-libraries were installed to "/usr/local/Cellar/ansible-cmdb/1.6/lib"
Installing non-libraries to "lib" is discouraged.
The offending files are:

Ok, I'll inreplace this. Is there anything else that has to be fixed?

@MikeMcQuaid
Copy link
Member

@hryamzik brew audit lists a few other issues: https://travis-ci.org/Homebrew/homebrew/jobs/87652927#L382

@hryamzik
Copy link
Contributor Author

@MikeMcQuaid got it, thanks, now cleaning up locally.

ansible-cmdb:
 * Don't need to interpolate "prefix" with inreplace
 * Don't need to interpolate "bin" with inreplace

Any suggestions on this? I don't see any other way to replace full paths.

@MikeMcQuaid
Copy link
Member

@hryamzik That's telling you that "#{prefix}" is the same as prefix

@hryamzik
Copy link
Contributor Author

@MikeMcQuaid indeed, thanks!

depends_on "libyaml"

def install
Dir.mkdir "#{bin}"
Copy link
Member

Choose a reason for hiding this comment

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

Can just do bin.mkpath here.

@hryamzik
Copy link
Contributor Author

@DomT4 thanks, done.

@apjanke
Copy link
Contributor

apjanke commented Nov 26, 2015

Not supporting PREFIX in a Makefile sounds worthy of an upstream bug report; that's pretty basic.

Looks like the audit problems are all fixed now. Could you squash this to a single commit to avoid cluttering up Homebrew's git history?

Also, where did the "6.3" in the issue title come from? Looks like this program is v 1.6. (1.8 is available now, too.)

@MikeMcQuaid
Copy link
Member

Not supporting PREFIX in a Makefile sounds worthy of an upstream bug report; that's pretty basic.

👍

@hryamzik hryamzik changed the title ansible-cmdb 6.3 (new formula) ansible-cmdb 1.6 (new formula) Nov 27, 2015
@hryamzik
Copy link
Contributor Author

@apjanke, all fixed, I'll take a look at 1.8 a bit later and will make a new PR.

@apjanke
Copy link
Contributor

apjanke commented Nov 27, 2015

Good to go. Merged. Thanks for your contribution to Homebrew!

@apjanke apjanke closed this in 3dcf732 Nov 27, 2015
@Homebrew Homebrew locked and limited conversation to collaborators Jul 10, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants