1
\$\begingroup\$

Let it be clear that in this question I ask for further guidance with my current version of Makefile used for my own Shell OpenSSL file en-/decryption scripts, not those scripts themselves.

I did my best, but this is my own first Makefile, so I better give it to you for review. I may only think the work is done, there may be many issues I don't know about.

Requirements:

  • The implicit destination path must be easily overridden without having to edit the Makefile

  • The implicit target must check for integrity (hashsum) stored in the file SHA512SUMS

  • The file must account for non-standard Linux environments like Cygwin

  • I myself tested it under normal circumstances in Linux Mint 18.3 and Cygwin

  • All files can be simply acquired from GitHub page of this little project

The Makefile follows:

DESTDIR?=/usr/local/bin
install_path=$(DESTDIR)

script_name_1=encrypt-file-aes256
script_name_2=decrypt-file-aes256

user_id=$(shell whoami)
group_id=$(shell id -gn)

.PHONY: check
.PHONY: install
.PHONY: uninstall

check:
    [ -f $(script_name_1) ] && [ -f $(script_name_2) ] && sha512sum --check SHA512SUMS --status && echo "OK: Files are prepared. You may use make install command now." || echo "ERROR: Files are missing and / or hashsum mismatch!"

install:
    install --verbose --mode=0755 --owner=$(user_id) --group=$(group_id) --target-directory=$(install_path) $(script_name_1) $(script_name_2)

uninstall:
    rm $(install_path)/$(script_name_1) $(install_path)/$(script_name_2)
\$\endgroup\$

2 Answers 2

Reset to default
1
\$\begingroup\$

The check target always succeeds (because the last command in the pipeline is echo). We'd like it to fail if any of the tests fail, so we should just write them one per line. Instead of testing for the existence of the files, we should make the check target depend on them:

check: $(script_name_1)  $(script_name_2)
    sha512sum --check SHA512SUMS
    @echo "OK: Files are prepared. You may use make install command now."

BTW, script_name_1 and script_name_2 are completely unhelpful variable names. How about encrypt and decrypt, for example?

I think the install target should depend on check - we really don't want to install if the check fails:

install: check

I'm not convinced that this is a good thing:

user_id=$(shell whoami)
group_id=$(shell id -gn)

When we install software, we normally make it owned by an appropriate user on the system, not the person doing the installing. In most cases, programs are owned by root, so that no ordinary user can accidentally overwrite them (or any other harmful action).

Make provides a $(RM) definition to give you a more portable command for deleting files.

It's great that you've thought about install location and about overridability of such parameters. You probably ought to know the conventional variables that are used for these:

  • DESTDIR specifies where the root directory is. This is usually empty, but packaging tools set this to a suitable working directory to build a (partial) filesystem image that can be archived.
  • PREFIX works somewhat like your DESTDIR or install_path (I'm not sure why you have two names for the same thing).

This is a rare Makefile that doesn't need .DELETE_ON_ERROR: (since it contains no non-PHONY targets).

Modified code

This is what I ended up with:

PREFIX = /usr/local

install_path = $(DESTDIR)$(PREFIX)/bin

encrypt = encrypt-file-aes256
decrypt = decrypt-file-aes256

check: $(encrypt)  $(decrypt)
    sha512sum --check SHA512SUMS
    @echo "OK: Files are prepared. You may use make install command now."

install: check
    install --d -D $(install_path)
    install -m 0755 -t $(install_path) $(encrypt) $(decrypt)

uninstall:
    $(RM) $(install_path)/$(encrypt) $(install_path)/$(decrypt)


.PHONY: check install uninstall

Note that Make variables specified on the command line will override those in the Makefile, so you can write

make install PREFIX="$(HOME)/testing"

to install to your test directory, for example.

\$\endgroup\$
0
0
\$\begingroup\$

Write the check target's commands with lines in between

You can easily enhance the user experience by:

Inserting a blank line in between the command itself and its output.

Write a distrib target to automatically make a package for further distribution

If you ever intend to distribute this code, it has to be easily packageable, so it should:

  • Remove possibly, not necessarily existing package files.

  • Create some distribution directory.

  • Compute the SHA512SUMS file.

  • Copy only what is necessary - the scripts and the Makefile.

  • The rest (LICENSE and README.md) may change and should be downloaded directly from GitHub.

  • On Cygwin for instance, all created files on the NTFS file system are marked executable. Thus, you should ensure proper permissions are in place.

  • tar the directory with your favorite compression method.

  • And finally, delete the temporary directory.

Toby Speight has pointed out a few things I adopted and enhanced in some cases

Instead of testing for the existence of the files, we should make the check target depend on them.

  • I also added SHA512SUMS file, which is needed too.

script_name_1 and script_name_2 are completely unhelpful variable names.

  • I agree, renamed to encrypt_script and decrypt_script.

The install target should depend on check.

  • I agree, dependency added.

I'm not convinced that this is a good thing: user_id and group_id.

  • After testing in Cygwin and Linux Mint, it seems these are unnecessary. Commented out, just in case, someone will provide a reason to use it.

DESTDIR specifies where the root directory is... + PREFIX works somewhat like your DESTDIR...

  • After reading some articles, it seems you are right, code changed.

Things I changed myself for a reason

  • Those .PHONYs seem to be possible to declate in one line.

  • Target uninstall tries to remove the directory also in case it is empty.

  • And a few nice touches.

Enhanced re-write based on the above information

PREFIX?=/usr/local/bin
install_path=$(DESTDIR)$(PREFIX)

encrypt_script=encrypt-file-aes256
decrypt_script=decrypt-file-aes256
distrib_name=openssl-file-encryption-decryption-shell-scripts

#user_id=$(shell id --name --user)
#group_id=$(shell id --name --group)

.PHONY: check install uninstall distrib

check: $(encrypt_script) $(decrypt_script) SHA512SUMS
    echo && sha512sum --check --status SHA512SUMS && ( echo "Ok. You may use 'sudo make install' or '(sudo) make install PREFIX=SomeDir' command now." ) || ( echo "ERROR: Files hash sum mismatch!" && echo && exit 1 )

install: check
    echo && [ -d $(install_path) ] || mkdir --parents $(install_path)
    install --verbose --mode=0755 --target-directory=$(install_path) $(encrypt_script) $(decrypt_script) # --owner=$(user_id) --group=$(group_id)

uninstall:
    rm $(install_path)/$(encrypt_script) $(install_path)/$(decrypt_script)
    rmdir --ignore-fail-on-non-empty $(install_path)

distrib: $(encrypt_script) $(decrypt_script) Makefile
    rm --force $(distrib_name).tar.xz
    rm --force --recursive $(distrib_name)
    mkdir $(distrib_name)
    sha512sum $(encrypt_script) $(decrypt_script) > $(distrib_name)/SHA512SUMS
    cp $(encrypt_script) $(decrypt_script) Makefile $(distrib_name)
    wget --quiet --output-document=$(distrib_name)/LICENSE https://raw.githubusercontent.com/burianvlastimil/openssl-file-encryption-decryption-shell-scripts/master/LICENSE
    wget --quiet --output-document=$(distrib_name)/README.md https://raw.githubusercontent.com/burianvlastimil/openssl-file-encryption-decryption-shell-scripts/master/README.md
    chmod 755 $(distrib_name)/$(encrypt_script) $(distrib_name)/$(decrypt_script)
    chmod 644 $(distrib_name)/Makefile $(distrib_name)/SHA512SUMS $(distrib_name)/LICENSE $(distrib_name)/README.md
    tar --create --xz --file=$(distrib_name).tar.xz $(distrib_name)/*
    rm --force --recursive $(distrib_name)
\$\endgroup\$
0

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.