Skip to content
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

Add full-install script #54

Closed
wants to merge 1 commit into from

Conversation

balr0g
Copy link
Contributor

@balr0g balr0g commented Jul 12, 2014

This adds a script that installs the required prereqs and binwalk to /opt/binwalk, then creates a quick-start script in /usr/local/bin which sets PYTHONPATH and runs the installed binwalk.

This script requires #51 to correctly function, and requires autoreconf to be run.

@devttys0
Copy link
Collaborator

The Makefile has a new target named 'startup-script', which drops your quick-start script into the current directory; the end-user can then move/rename the script as they see fit.

It is not installed during a 'make install', as I would prefer not to assume that the user wants it to be placed in /usr/local/bin, or any other hard-coded directory, particularly if they've gone to the trouble of explicitly specifying a non-default install path with --prefix. It could be installed to the prefix directory, but then it couldn't be named 'binwalk', as that file already exists, obviously. Some other name could be chosen, like 'binwalk-start' or the like, but again, if the user wants to customize the install locations I think it's better to leave that decision to the user.

@devttys0 devttys0 closed this Jul 13, 2014
@balr0g
Copy link
Contributor Author

balr0g commented Jul 14, 2014

This script is written taking into account that libmagic and libfuzzy are not in the Makefile. I haven't tested them with the build scripts are they currently are, so I can't guarantee that it will work. The goal of this was to take the third-party libs out of the source tree while keeping the convenience of a one-command install.

It is not installed during a 'make install', as I would prefer not to assume that the user wants it to be placed in /usr/local/bin, or any other hard-coded directory, particularly if they've gone to the trouble of explicitly specifying a non-default install path with --prefix.

This script isn't supposed to be called by make — it's a quick-install script that itself installs the prereqs into the prefix, runs ./configure and make to install binwalk into the prefix (/opt/binwalk), and then creates the quick-start script to make it easy to run binwalk. This way binwalk doesn't interfere with anything the user may have installed into /usr/local.

It could be installed to the prefix directory, but then it couldn't be named 'binwalk', as that file already exists, obviously. Some other name could be chosen, like 'binwalk-start' or the like, but again, if the user wants to customize the install locations I think it's better to leave that decision to the user.

The quick start script is not supposed to go into the prefix directory.

The whole point of this full-install script is to negate the more difficult install caused by removing the libraries from the source tree and from the ./configure and make scripts. If a user wants to use ./configure and make manually, they are responsible for installing the required prerequisites (correct versions and all), and ./configure will bail out if these prereqs are not installed. This is how every other program that uses ./configure and make scripts works. (Some programs have sub-projects but those are used at compile time and are not installed.)

If a user wants to install everything "one-command," they can use the full-install script. This script does the following:
• Creates the /opt/binwalk prefix
• Downloads, compiles and installs both dependencies in the prefix
• Configures binwalk to link to the dependencies in the prefix (as opposed to those installed system-wide, as CFLAGS and LDFLAGS are used first)
• Compiles and installs binwalk in the prefix
• Places a quick-start script in /usr/local/bin so the user does not need to add the prefix bin directory to their path.

I feel that keeping everything that binwalk installs in its own prefix makes maintainability a lot easier for the user, as only a single file gets placed in /usr/local. If the user wants to uninstall binwalk, they only need to rm -rf /opt/binwalk && rm -f /usr/local/bin/binwalk.

@balr0g
Copy link
Contributor Author

balr0g commented Jul 14, 2014

In addition, the Python changes that remove /opt/local/lib/ and /usr/local/Cellar/ from the search paths are fine because these paths will be in the LDFLAGS that get parsed and stored at install-time and written to libpaths.py.

Your fix using prefix.conf is halfway there, but the prefix is the wrong place to look for the libs. While compress/miniz will be installed in $PREFIX/lib, the other libs may very well not be.

The paths searched for libs must not be only the compile-time value of $PREFIX as this is the install prefix of binwalk (and the two "internal" libs), not the install prefix of the other libs specified in LDFLAGS. The libs may be in any of the directories specified in the $LDFLAGS compile-time variable, which is formatted as follows:

LDFLAGS="-L/path/to/prefix1/libs -L/path/to/prefix2/libs ..."

If I wanted to have separate prefixes for libmagic and libfuzzy I could do that and it should still work.

I've updated the commit (Currently at balr0g@96f78ac) to properly handle this.

@balr0g
Copy link
Contributor Author

balr0g commented Jul 14, 2014

Still a little unsure on how to break the LDFLAGS variable into separate paths, one on each line.

My current perl command is as follows:

echo $(LDFLAGS) | perl -pe 's/((^-L)|((\ )-L))/$4/g' | perl -pe 's/\ /\n/g'

This mostly works, but it breaks down if LDFLAGS is as follows:

LDFLAGS='-L/path/t\ o/p1/lib -L"/path/t o/p2/lib" -L/path/t-Lo/p3/lib --blah -q'

The above LDFLAGS should produce the following separated output:

/path/t o/p1/lib
/path/t o/p2/lib
/path/t-Lo/p3/lib

And these need to be properly interpreted as whole paths. In the Python script this would be easy; rather than splitting on whitespace we'd only want to split on newline. Not sure how to do this in Makefile.am though.

@devttys0
Copy link
Collaborator

The goal of this was to take the third-party libs out of the source tree

As mentioned in issue #51, at least at this time, I prefer to keep them in.

This way binwalk doesn't interfere with anything the user may have installed into /usr/local

This is already the case.

If the user wants to uninstall binwalk, they only need to rm -rf /opt/binwalk && rm -f /usr/local/bin/binwalk

I have no particular issue with installing binwalk to /opt/binwalk, or some other location, but again, this is already a solved problem as 'make uninstall' will remove binwalk and the libs.

Your fix using prefix.conf is halfway there, but the prefix is the wrong place to look for the libs. While compress/miniz will be installed in $PREFIX/lib, the other libs may very well not be.

Currently, all libraries should be installed to the prefix provided to the configure script via --prefix. Is this not the case?

@balr0g
Copy link
Contributor Author

balr0g commented Jul 14, 2014

As mentioned in issue #51, at least at this time, I prefer to keep them in.

This completely breaks convention and recommended practices.
You stated that the reason for the included libraries is to allow for easy installation, and to prevent breakage due to library changes. The full-install script is intended to mitigate these issues — it allows for very easy installation and the library versions are specified in the script. The issue of older library versions is prevented by the checks I added in configure.ac. If someone does compile against a newer library and it doesn't work, they should create a bug report, as happened for the libmagic changes. (Chances are that bug report would have been looked at by the Debian package maintainer and a pull request would be filed here, but I saw it first and beat them out on it.)

Currently, all libraries should be installed to the prefix provided to the configure script via --prefix. Is this not the case?

No. Keg-only libraries in Homebrew will be installed in /usr/local/Cellar/$PACKAGENAME/$VERSION/lib, and $PACKAGENAME is not necessarily the same as the lib prefix (i.e. ssdeep and libfuzzy — but it works for ssdeep because it isn't keg-only). These lib paths are provided within $LDFLAGS so that value should be stored. Other than the issue of parsing out the paths from $LDFLAGS, balr0g@96f78ac properly handles this.

@devttys0
Copy link
Collaborator

This completely breaks convention and recommended practices.

I'm OK with that, though perhaps someday in the future I'll see the light.

The full-install script is intended to mitigate these issues

The only real difference that I can see is that full-install dynamically downloads the tarballs from the internet. Other than that, it's fundamentally the same: it installs libmagic/libfuzzy to unique locations, which can then be loaded by binwalk instead of whatever other libmagic/libfuzzy libraries may (or may not) be installed on the system. I have a couple of issues with full-install though:

Building with full-install requires an internet connection, which is actually quite important. I really do not want to require internet connectivity in order to do a basic build.

If a user specifies an install prefix, dropping a script into a hardcoded /usr/local location would be unexpected behavior. Yes, they could change it in the script, but this still is not what prefix is supposed to do.

Worse, if the user wants binwalk installed in a standard prefix like /usr/local and sets that as the prefix, your script is now overwriting any existing libmagic/libfuzzy libraries that may be installed there (which, especially for libmagic, has a good chance of breaking other tools relying on those libraries). Further, due to the way that libmagic and libfuzzy are built/installed by full-install, this will also overwrite any installed file and ssdeep binaries, which is completely unexpected and should obviously not be done.

I don't particularly like needing a shell script to start binwalk. It assumes the user wants binwalk run with python (they may want python2 or python3 instead), and it prevents the user from specifying python options when running binwalk (e.g., you can't do something like 'python -O /usr/local/bin/binwalk', which is useful for debugging).

While I'm sure most these issues could be resolved, requiring an internet connection is really a non-starter and the bottom line is, full-install is not capable of dealing with these issues which have already ben encountered and addressed with the current build system. Currently, no internet connection is required to build. Regardless of what your install prefix is, no existing libraries or unintended binaries will be overwritten. configure options allow for specifying the preferred python interpreter. Binwalk is just a python script, not a shell wrapper, so it can be invoked directly via a python interpreter, as would be expected of a python script.

Ultimately, the only advantage I see to full-install is that libmagic/libfuzzy are no longer in the binwalk repo, but I am comfortable with them living in the repo. Thanks to your suggestions/patches, prefix options work properly now, and the building of local libraries can be disabled, so it seems that the original issues from which this stemmed have been addressed, which is why the issue was closed.

Keg-only libraries in Homebrew will be installed in /usr/local/Cellar/$PACKAGENAME/$VERSION/lib, and $PACKAGENAME is not necessarily the same as the lib prefix

Does /usr/local/Cellar/$PACKAGENAME/$VERSION/lib get added as part of the standard system library search path? If so, I would think that ctypes should find it. If not, how does Homebrew usually deal with this? Do all packages/tools require modification to search in these directories?

@balr0g
Copy link
Contributor Author

balr0g commented Jul 14, 2014

The only real difference that I can see is that full-install dynamically downloads the tarballs from the internet. Other than that, it's fundamentally the same: it installs libmagic/libfuzzy to unique locations, which can then be loaded by binwalk instead of whatever other libmagic/libfuzzy libraries may (or may not) be installed on the system.

There's another major difference: full-install takes the job of installing the tarballs out of the configure and make scripts, where it doesn't belong. A make prereqs that triggers a script to download (if necessary), unpack, compile, and install the prereqs would be cleaner.

Building with full-install requires an internet connection, which is actually quite important. I really do not want to require internet connectivity in order to do a basic build.

An option would be informing the user to place the tarballs into the same directory as the script, then having the script detect the tarballs if they already exist and using them.

If a user specifies an install prefix, dropping a script into a hardcoded /usr/local location would be unexpected behavior. Yes, they could change it in the script, but this still is not what prefix is supposed to do.

If a user specified an install prefix using ./configure and make, this script isn't going to be dropped. The whole purpose of the script in /usr/local/bin is for an "easy-install." The user might have their own libs in /usr/local that conflict with the binwalk-required ones. If a user is installing binwalk into /usr/local, they are responsible for providing the dependencies, which the configure script will verify exist, and the user should not use the "full-install" script.

Worse, if the user wants binwalk installed in a standard prefix like /usr/local and sets that as the prefix, your script is now overwriting any existing libmagic/libfuzzy libraries that may be installed there (which, especially for libmagic, has a good chance of breaking other tools relying on those libraries).
Again, if the user wants the entirety of binwalk installed into the standard prefix, they are responsible for handling prereqs and installing it the standard way using ./configure and make install.

Further, due to the way that libmagic and libfuzzy are built/installed by full-install, this will also overwrite any installed file and ssdeep binaries, which is completely unexpected and should obviously not be done.

full-install specified a prefix of /opt/binwalk when building/installing libmagic and libfuzzy, so the binaries go into /opt/binwalk/bin, not /usr/local/bin or /usr/bin. Therefore this is not an issue.

I don't particularly like needing a shell script to start binwalk. It assumes the user wants binwalk run with python (they may want python2 or python3 instead), and it prevents the user from specifying python options when running binwalk (e.g., you can't do something like 'python -O /usr/local/bin/binwalk', which is useful for debugging).

Good point — the whole purpose of that script was a convenience script for starting binwalk when it is in a /opt prefix. I think this could be solved by replacing the shell script with a python script.

Currently, no internet connection is required to build. Regardless of what your install prefix is, no existing libraries or unintended binaries will be overwritten.

This is fairly easy to resolve, as explained above.

configure options allow for specifying the preferred python interpreter. Binwalk is just a python script, not a shell wrapper, so it can be invoked directly via a python interpreter, as would be expected of a python script.

Ultimately, the only advantage I see to full-install is that libmagic/libfuzzy are no longer in the binwalk repo, but I am comfortable with them living in the repo.

Do you have any good reasons other than convenience to keep them in the repo?

Thanks to your suggestions/patches, prefix options work properly now, and the building of local libraries can be disabled, so it seems that the original issues from which this stemmed have been addressed, which is why the issue was closed.

Well, mostly work — there are a couple of issues remaining. For one, the default for ./configure and make should be to check what libraries are already installed, and the current default behaviour of installing the libs should be non-default behaviour triggered by an option (if you do insist on keeping this in the ./configure and make scripts). Secondly, the LDFLAGS aren't used for library paths, as explained below.

Does /usr/local/Cellar/$PACKAGENAME/$VERSION/lib get added as part of the standard system library search path? If so, I would think that ctypes should find it. If not, how does Homebrew usually deal with this? Do all packages/tools require modification to search in these directories?

No. Normally (with compiled programs) the linker parses the LDFLAGS and writes the library paths into the binary at link time. As long as the LDFLAGS are set correctly, which Homebrew appends to when it's told that a package depends on another, the libraries will be found. This won't work with this method of opening libraries from Python though, as those scripts aren't handled the same way.

The changes in the referenced commit mostly work, but the parsing of LDFLAGS doesn't properly handle escaped and quoted paths, neither does it drop unused LDFLAGS flags — only the -Lpath flags should be extracted. I'm unsure of the best way to parse these out.

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.

None yet

2 participants