Make demo and make local fails if CURDIR has spaces in it

[ysuarez]

I believe that if you have spaces in the path for the isle-dc repo on your system, make demo and make local will fail.

I was using a path similar to this...

/home/yamil/isle tests/isle-dc

This of course leads to Make setting the CURDIR to a path with spaces, and then also leads to the makefile generate-secrets target to run commands that fails since it gets parsed as...

docker run --rm -t \
	-v /home/yamil/isle tests/isle-dc/secrets:/secrets \
	-v /home/yamil/isle tests/isle-dc/scripts/generate-secrets.sh:/generate-secrets.sh \
	-w / \
	--entrypoint bash \
	islandora/drupal:1.0.0-alpha-11 -c "/generate-secrets.sh && chown -R `id -u`:`id -g` /secrets"

(I was able to see how the commands was parsed by running make generate-secrets -n)

On macOS Big Sur and Mojave I got this not too obvious error when running make demo or make local...

docker: invalid reference format: repository name must be lowercase.
See 'docker run --help'.
make: *** [Makefile:285: generate-secrets] Error 125

Not sure what is best:

1. to wrap relevant uses of $(CURDIR) in the Makefile with double quotes
2. OR just make it a requirement that the path to isle-dc repo cannot have paths (with a warning in the docs & README).
3. OR something else entirely

If folks decide to not change the Makefile code I can create a documentation PR to add a note/tip/warning about not having paths with spaces in the README and/or in the official docs.

For example, in this page that covers Installing ISLE Docker Compose and/or the section for Installing a Demo Server

Thoughts?

Thanks in advance.

[ysuarez]

BTW, if folks want to go with the approach of adding quotes to the use of $(CURDIR) I can create a PR for that change.

[ysuarez]

At today's tech call I proposed making a PR that will wrap the use of $(CURDIR) in double quotes, so that running make commands work even if the full path to isle-dc has spaces in it.

[ysuarez]

@DonRichards (and others) I have been experimenting with Makefile syntax (I had never used Makefiles this complex before), and wondered what you (all) thought of something.

my original plan was to just wrap the use of $(CURDIR) in double quotes from this...

-v $(CURDIR)/secrets:/secrets \

to this...

-v "$(CURDIR)/secrets":/secrets \

and I wondered how would you want to handle lines like 332 in the Makefile...

isle-dc/Makefile

Lines 327 to 334 in 3aa6e1f

	local: generate-secrets
	$(MAKE) download-default-certs ENVIROMENT=local
	$(MAKE) -B docker-compose.yml ENVIRONMENT=local
	$(MAKE) pull ENVIRONMENT=local
	mkdir -p $(CURDIR)/codebase
	if [ -z "$$(ls -A $(CURDIR)/codebase)" ]; then \
	docker container run --rm -v $(CURDIR)/codebase:/home/root $(REPOSITORY)/nginx:$(TAG) with-contenv bash -lc 'composer create-project drupal/recommended-project:^9.1 /tmp/codebase; mv /tmp/codebase/* /home/root; cd /home/root; composer config minimum-stability dev; composer require islandora/islandora:dev-8.x-1.x; composer require drush/drush:^10.3'; \
	fi

There could be a simple way to handle adding quotes to a snippet that is already in double quotes, but I could not figure it out yet.

For now I took the approach of creating a "target-specific variable" called QUOTED_CURDIR for getting around the double + double quotes...

isle-dc/Makefile

Lines 327 to 335 in 431976d

	local: QUOTED_CURDIR = "$(CURDIR)"
	local: generate-secrets
	$(MAKE) download-default-certs ENVIROMENT=local
	$(MAKE) -B docker-compose.yml ENVIRONMENT=local
	$(MAKE) pull ENVIRONMENT=local
	mkdir -p "$(CURDIR)/codebase"
	if [ -z "$$(ls -A $(QUOTED_CURDIR)/codebase)" ]; then \
	docker container run --rm -v "$(CURDIR)/codebase":/home/root $(REPOSITORY)/nginx:$(TAG) with-contenv bash -lc 'composer create-project drupal/recommended-project:^9.1 /tmp/codebase; mv /tmp/codebase/* /home/root; cd /home/root; composer config minimum-stability dev; composer require islandora/islandora:dev-8.x-1.x; composer require drush/drush:^10.3'; \
	fi

I was then able to partially build demo with the changes above while using a path (and thus $CURDIR) to the isle-dc repo with spaces. For example, the demo containers were created. Though I got errors when the makefile tried to call the drupal-public-files-import: $(SRC) target, and again the spaces in the path caused errors.

In this case the error was...

make[1]: *** No rule to make target '/TEST/PATH/WITH SPACES', needed by 'drupal-public-files-import'.  Stop.
make[1]: Leaving directory '/TEST/PATH/WITH SPACES'
make: *** [Makefile:314: demo] Error 2

Here is the relevant code...

The problem now is that the drupal-public-files-import target's prerequisite is not being matched because of the spaces in the path. It is being seen as multiple files, instead of SRC pointing to a single file.

isle-dc/Makefile

Lines 226 to 231 in 3aa6e1f

	drupal-public-files-import: $(SRC)
	ifndef SRC
	$(error SRC is not set)
	endif
	docker cp $(SRC) $$(docker-compose ps -q drupal):/tmp/public-files.tgz
	docker-compose exec -T drupal with-contenv bash -lc 'tar zxvf /tmp/public-files.tgz -C /var/www/drupal/web/sites/default/files && chown -R nginx:nginx /var/www/drupal/web/sites/default/files && rm /tmp/public-files.tgz'

I am not sure how to fix this, and it will probably take someone with more experience with Makefiles than me.

I am able to wrap "$(SRC)" in double quotes in the "recipe/commands" part of the target, and it works fine there in my early tests.

Thoughts?

Thanks in advance.

P.S. In the short term I can create updates to the README and official docs that the path to isle-dc currently cannot have spaces.

[misilot]

I believe this is resolved by #237 ?

[ysuarez]

In an attempt to be thorough I want to paste a comment that @misilot put in PR #237

"...
I believe this is limitation with Make per this really old bug report http://savannah.gnu.org/bugs/?712
..."

Based on the info above, If we are running into a limitation of Make, to have spaces in its path, then should I/we revert the parts of my PR #237 that tried to fix this issues incompletely? Then just keep the code that @DonRichards wrote that just stops the Makefile execution with an error if a space is detected in the path? That way we end up with less changes to the resulting Makefile?

[ysuarez]

I will close this ticket. @DonRichards code provides a pretty clear error message for folks that fall into this situation. @misilot thanks for adding more info to this issue.