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

Solve the space in base path problem issue #75 #417

Closed
wants to merge 2 commits into from

Conversation

gromain
Copy link
Contributor

@gromain gromain commented Jun 5, 2020

Closed previous PR #411 and reopening because of mis-management on my side.

I believe this would solve the root cause of #75.
However, this does not solve the problem upstream with debootstrap.
Maybe a check should be added so that if there is a space in the base path, debootstrap does not fail silently. Or to recommend using the docked build script. I believe explicit errors are better than undocumented behavior because of an upstream.

If there is a space in the base path anywhere, the scripts fails to start.
This is due to the default separator used in bash (which is a space).
This default separator can be change by setting the variable $IFS.
In build.sh, the old IFS variable is saved and then replaced by
\n\b. At the end of the script, the saved IFS is set back.

In build-docker.sh, a simple double quote was enough.

If there is a space in the base anywhere, the scripts fails to start. 
This is due to the default separator used in bash (which is a space). 
This default separator can be change by setting the variable $IFS.
In `build.sh`, the old IFS variable is saved and then replaced by 
`\n\b`. At the end of the script, the saved IFS is set back.

In `build-docker.sh`, a simple double quote was enough.
@gromain
Copy link
Contributor Author

gromain commented Sep 11, 2020

Any comments on this?

@XECDesign
Copy link
Member

Shouldn't the variable assignments be double quoted?

@gromain
Copy link
Contributor Author

gromain commented Dec 3, 2021

I just pushed the change you requested (sorry, it fell through the cracks, and I completely forgot about it!).

Is there anything else?

@gromain gromain marked this pull request as draft December 4, 2021 01:38
@gromain
Copy link
Contributor Author

gromain commented Dec 4, 2021

This PR creates an issue during the image creation steps (more specifically, line 302 when transforming the collected EXPORT_DIRS array). I'm going to have about ways to solve this.

@XECDesign
Copy link
Member

It's probably not worth spending too much time on this while debootstrap itself doesn't support spaces. Maybe your suggestion of just flagging up the issue before the build starts is the way to go for now?

@gromain
Copy link
Contributor Author

gromain commented Feb 1, 2022

Yes, I agree. I'll close and recreate a PR with something in the README somewhere, if that's good for you.
Maybe we can put a check too in the build script, to check for the presence of a space in the path and avoid building in that case?

@XECDesign
Copy link
Member

Yes, I agree. I'll close and recreate a PR with something in the README somewhere, if that's good for you.

Yeah, sounds great. No need to re-create the PR, you can just modify your existing branch and it will update the PR.

Maybe we can put a check too in the build script, to check for the presence of a space in the path and avoid building in that case?

Yup, that's what I meant.

@gromain
Copy link
Contributor Author

gromain commented Feb 2, 2022

Shellcheck is complaining about a few word-splitting issues, do you want me to fix them here too or should I create another PR to keep things clean?

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