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

Ensure rsync available in image #101

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

jonseymour
Copy link

@jonseymour jonseymour commented May 12, 2021

I recently checked out @ 2.0.0 and attempted to build it, but found the rsync step was failing because of a missing dependency in the image.

This likely would not have caused a visible issue (at least as far as the build artefacts were concerned) if the build was performed in a dirty environment in which rsync copied files had already been copied, however, it did cause issues in my (clean) environment where there were no such artefacts available. The issue was also apparent in the build messages themselves which showed that rsync was missing.

So, I fixed the Dockerfile to add the rsync dependency to the image.

I have also added a Makefile with a clean target to allow a pristine environment to be restored easily and idiomatically.

The code in this PR is built on the 2.0.0 tag but merges cleanly with master.

PR amended with commits to:

  • add .gitignore rules for the build products (mainly for cases where this module is used as a submodule)
  • fail the build in case any command fails
  • disable the docker run -i option in non-interactive environments
  • configure PROJ_LIB variable to reduce noise during test lambda execution

The final 2 commits are git foo to permit the same branch to be merged (at different points) with a branch derived from 2.0.0 or a branch derived from origin/master and both merges will succeed without conflict as will any merged derived from each.

I am, of course, happy to restructure the PR into a master only PR if this too complicated for others to follow, but until I get feedback I will leave it as is, is as this is the format I prefer to maintain it in.

For others: if you want these fixes on 2.0.0 variant, merge
c27bf0a. If you want a master variant, merge
7d8e184 (or 1c8d1f6)

If packaging was working before, it was only because of a dirty lambda/ directory
which had already received a copy of the copied files.

A subsequent commit will suggest the use of a clean target on a Makefile to make
cleaning dirty build directories easier to achieve.

Signed-off-by: Jon Seymour <jon@wildducktheories.com>
The clean function is justified because lack of an idiomatic way to
find and execute a clean target may have contributed to the error that that
previous commit addressed.

The use of a Makefile is justified by an idiomatic way to find and execute the clean function.

The use of the git ls-files is justified beccause of the need to exclude the one
source controlled file in package/lambda from cleanup.
Signed-off-by: Jon Seymour <jon@wildducktheories.com>
Helps to prevent (almost) silent errors frustrating test and deployment efforts.
Previously, the first docker run would fail with a short error message:

the input device is not a TTY

Now, if we are running in an automated environment like Jenkins or Circle CI
the interactive option is removed. It is still available, just in case
there is sometimes a reason to run the build interactively.

Signed-off-by: Jon Seymour <jon@wildducktheories.com>
…riable

Signed-off-by: Jon Seymour <jon@wildducktheories.com>
…J_LIB variable"

This reverts commit c27bf0a.

(This change, or an equivalent, is already in master.
@jonseymour jonseymour force-pushed the ensure-rsync-available-in-image branch from 7accb08 to 1c8d1f6 Compare May 14, 2021 02:07
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.

1 participant