-
Notifications
You must be signed in to change notification settings - Fork 91
Adding bloodhound #463
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Adding bloodhound #463
Conversation
This should cover #305 :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
some inline comments for your next PR, but i have to say that i'm pretty disappointed that you basically took the pkgbuild from blackarch.
@@ -0,0 +1,75 @@ | |||
# This file is part of BlackArch Linux ( https://www.blackarch.org/ ). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
... really?
this pkgbuild is basically copied with minor changes
|
||
_npmname=BloodHound | ||
pkgname=bloodhound | ||
pkgver=705.124ecee |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
doesn't fit out packaging guideline. if you're going to get $pkgver from git
, please rename the pkg accordingly to bloodhound-git
cp -a *.js *.json *.sh *.html *.yml src $_bin_path *.graphdb dist "$pkgdir/usr/share/$pkgname" | ||
|
||
cat > "$pkgdir/usr/bin/$pkgname" << EOF | ||
#!/bin/sh |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#!/usr/bin/env bash
EOF | ||
|
||
chmod +x "$pkgdir/usr/bin/$pkgname" | ||
chmod +x "$pkgdir/usr/share/$pkgname/$_bin_path/BloodHound" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
chmod 755 instead of chmod +x
pkgrel=1 | ||
pkgdesc='Six Degrees of Domain Admin' | ||
groups=('archstrike') | ||
arch=('any') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'any' arch doesn't work here since it needs to be built differently for different arches. you need to explicitly write them
Hi torxed, thanks for submitting. There are a few things that need changing. One of the big ones changing the maintainer and contributor lines:
We have no issues with contributions between various repos, we prefer to keep it repo focused. Also, our packaging infrastructures are different. Have you tried testing that PKGBUILD in a clean chroot?
Do you have any ARM devices that can also be used for testing? Thanks. |
I see. Let me get back to you on those things. I'll test the stuff, been a busy week and will probably be a bit busy hence forth, but I'll do my best :) |
|
Ah, you mean it does dependency-lookups? Not something you necessarily need for the PKGBUILD to pass? I thought you removed |
|
I apologize for dropping the ball and not following up. Where are we with this? I took a peek and don't see a change to the Maintainer and contributor lines - want to give proper credit. |
No tests have been made, not sure what the normal test procedure is.
I currently have no machine to test this on since they're all running bloodhound instances currently.
Will update the PR once I get around to testing.