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

Readme: Fix command to build with Docker on Windows #7040

Merged
merged 3 commits into from
Dec 5, 2022
Merged

Readme: Fix command to build with Docker on Windows #7040

merged 3 commits into from
Dec 5, 2022

Conversation

avgrad
Copy link
Contributor

@avgrad avgrad commented Nov 10, 2022

The command from the README.md for building with Docker does not work on Windows.

$(pwd) is not valid. Neither in CMD nor in PowerShell.

This PR adds a separate listing and command for PowerShell with ${pwd} that works (braces instead of parentheses).

@ApCoder123 ApCoder123 added the bug Issue/PR highlights a bug. label Nov 10, 2022
Copy link
Member

@Carlgo11 Carlgo11 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$(pwd) could just be replaced with ./ for it to work on all platforms.

@Carlgo11
Copy link
Member

As the requested changes haven't been resolved yet I'm going to label this PR as inactive.
⚠️ Unless there's activity within the next 7 days I'll close the PR.

@Carlgo11 Carlgo11 added inactive Inactive issue/PR, soon to be closed unless the author responds. enhancement Issue/PR contains enhancements to the overall code of the site. and removed bug Issue/PR highlights a bug. labels Nov 23, 2022
@avgrad
Copy link
Contributor Author

avgrad commented Nov 27, 2022

$(pwd) could just be replaced with ./ for it to work on all platforms.

docker run -p 4000:4000 -v ./:/twofactorauth 2factorauth/twofactorauth

I tried this on Windows Powershell and CMD, both resulting in the following error:

docker: Error response from daemon: create .: volume name is too short, names should be at least two alphanumeric characters.
See 'docker run --help'.

@Carlgo11
Copy link
Member

Carlgo11 commented Nov 27, 2022

Ah okay.
I guess ${PWD} works in PS?
If so, replace $(pwd) with ${PWD} as it should be available cross platform.

Although the entire ordeal might be a bit of a waste due to #7091 removing that part of the README completely 🤷‍♂️

@avgrad
Copy link
Contributor Author

avgrad commented Dec 4, 2022

Yes for Windows PowerShell ${pwd} works.
I can't test ${pwd} on Linux and Mac.
I merged the commands to use ${pwd} for all platforms.

@Carlgo11 Carlgo11 removed the inactive Inactive issue/PR, soon to be closed unless the author responds. label Dec 5, 2022
@Carlgo11 Carlgo11 merged commit 77e2676 into 2factorauth:master Dec 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Issue/PR contains enhancements to the overall code of the site.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants