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.

Sign up for GitHub

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

Jump to bottom

Really fix Ubuntu/Debian setup when $PATH contains spaces #9370

Merged
merged 1 commit into from
Jun 11, 2020
Merged

Really fix Ubuntu/Debian setup when $PATH contains spaces #9370

merged 1 commit into from
Jun 11, 2020

Conversation

sigprof
Copy link
Contributor

@sigprof sigprof commented Jun 11, 2020 •
edited by fauxpark
Loading

Description

PR #9307 fixed the immediately visible problem (the command that was added to $HOME/.bashrc was incorrect because of missing quotes around paths with spaces). However, the modified command is still wrong — it captures the value of $PATH at the setup time, and the resulting command written out to $HOME/.bashrc will overwrite $PATH with that captured value, ignoring any changes in the environment. This may be especially important for WSL, where the initial value of $PATH in Linux includes everything which has been added to %PATH% on the Windows side; after adding that command to $HOME/.bashrc the WSL environment will no longer pick up any changes made by newly installed Windows software.

Instead of that, use single quotes around the command, so that the environment variables are not expanded at the setup time, and the command that is added to $HOME/.bashrc becomes exactly this:

PATH="$HOME/.local/bin:$PATH"

This command will use the $HOME and $PATH environment variable values at the time the command is executed, not at the time the QMK setup is performed, so any further updates to $PATH are taken into account. Double quotes also ensure that the command is safe even if the values of those environment variables contain spaces.

Types of Changes

  • Core
  • Bugfix
  • New feature
  • Enhancement/optimization
  • Keyboard (addition or update)
  • Keymap/layout/userspace (addition or update)
  • Documentation

Checklist

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • I have tested the changes and verified that they work and don't break anything (as well as I can manage).

@sigprof
Really fix Ubuntu/Debian setup when $PATH contains spaces
6a18aa6 
PR qmk#9307 fixed the immediately visible problem (the command that was
added to $HOME/.bashrc was incorrect because of missing quotes around
paths with spaces).  However, the modified command is still wrong - it
captures the value of $PATH at the setup time, and the resulting command
written out to $HOME/.bashrc will overwrite $PATH with that captured
value, ignoring any changes in the environment.  This may be especially
important for WSL, where the initial value of $PATH in Linux includes
everything which has been added to %PATH% on the Windows side; after
adding that command to $HOME/.bashrc the WSL environment will no longer
pick up any changes made by newly installed Windows software.

Instead of that, use single quotes around the command, so that the
environment variables are not expanded at the setup time, and the
command that is added to $HOME/.bashrc becomes exactly this:

    PATH="$HOME/.local/bin:$PATH"

This command will use the $HOME and $PATH environment variable values at
the time the command is executed, not at the time the QMK setup is
performed, so any further updates to $PATH are taken into account.
Double quotes also ensure that the command is safe even if the values of
those environment variables contain spaces.
@fauxpark fauxpark requested a review from a team June 11, 2020 10:40
@zvecr zvecr merged commit 0cb4da2 into qmk:master Jun 11, 2020
@zvecr
Copy link
Member

zvecr commented Jun 11, 2020

Thanks!

nesth pushed a commit to nesth/qmk_firmware that referenced this pull request Jun 11, 2020
Merge remote-tracking branch 'upstream/master'
2ecd1de 
* upstream/master: (82 commits)
  Fix my personal keymap // Custom keymap for Kbdfans/kbd67/rev2 with improvements on Accessibility (qmk#9207)
  Murcielago: improve default keymap (qmk#9363)
  Really fix Ubuntu/Debian setup when $PATH contains spaces (qmk#9370)
  Fix Configurator layout data for clueboard/2x1800/2019 (qmk#9373)
  Fixing Iron165 VIA Keymap (qmk#9298)
  Fix Ubuntu/Debian setup when $PATH contains spaces (qmk#9307)
  Add documentation for selecting an Arm MCU (qmk#9046)
  [Docs] Fixed the hyperlink to `/users/_example/`. (qmk#9326)
  [keyboard] Project Keyboard Signature 87 (qmk#9062)
  Include `pointing_device_send` in docs (qmk#9185)
  Fix one shot swaphands compiler error when NO_ACTION_ONESHOT is defined (qmk#9296)
  [Keymap] WPM-responsive OLED animation in personal keymap (qmk#9264)
  Add bat43 rev2 (qmk#9319)
  [Keymap] Add dual layer keymap for xd002 macropad (qmk#9222)
  [Keyboard] Wazowski 23-19 PCB Support (qmk#9198)
  adds support for the atmega328 (qmk#9043)
  fix rgb mode selection and lighting increments for the 1894 (qmk#9336)
  Add Sinc keyboard (qmk#8986)
  Add new iris keymap 'fluffactually' (qmk#9325)
  kbdfans/kbd67/rev2: Fix ISO layout macro (qmk#9329)
  ...
Shinichi-Ohki added a commit to Shinichi-Ohki/qmk_firmware that referenced this pull request Jun 12, 2020
@Shinichi-Ohki
Merge branch 'master' of https://github.com/qmk/qmk_firmware
35ed72f 
* 'master' of https://github.com/qmk/qmk_firmware: (461 commits)
  Fix my personal keymap // Custom keymap for Kbdfans/kbd67/rev2 with improvements on Accessibility (qmk#9207)
  Murcielago: improve default keymap (qmk#9363)
  Really fix Ubuntu/Debian setup when $PATH contains spaces (qmk#9370)
  Fix Configurator layout data for clueboard/2x1800/2019 (qmk#9373)
  Fixing Iron165 VIA Keymap (qmk#9298)
  Fix Ubuntu/Debian setup when $PATH contains spaces (qmk#9307)
  Add documentation for selecting an Arm MCU (qmk#9046)
  [Docs] Fixed the hyperlink to `/users/_example/`. (qmk#9326)
  [keyboard] Project Keyboard Signature 87 (qmk#9062)
  Include `pointing_device_send` in docs (qmk#9185)
  Fix one shot swaphands compiler error when NO_ACTION_ONESHOT is defined (qmk#9296)
  [Keymap] WPM-responsive OLED animation in personal keymap (qmk#9264)
  Add bat43 rev2 (qmk#9319)
  [Keymap] Add dual layer keymap for xd002 macropad (qmk#9222)
  [Keyboard] Wazowski 23-19 PCB Support (qmk#9198)
  adds support for the atmega328 (qmk#9043)
  fix rgb mode selection and lighting increments for the 1894 (qmk#9336)
  Add Sinc keyboard (qmk#8986)
  Add new iris keymap 'fluffactually' (qmk#9325)
  kbdfans/kbd67/rev2: Fix ISO layout macro (qmk#9329)
  ...
turky pushed a commit to turky/qmk_firmware that referenced this pull request Jun 13, 2020
@sigprof @turky
Really fix Ubuntu/Debian setup when $PATH contains spaces (qmk#9370)
700bbe9 
PR qmk#9307 fixed the immediately visible problem (the command that was
added to $HOME/.bashrc was incorrect because of missing quotes around
paths with spaces).  However, the modified command is still wrong - it
captures the value of $PATH at the setup time, and the resulting command
written out to $HOME/.bashrc will overwrite $PATH with that captured
value, ignoring any changes in the environment.  This may be especially
important for WSL, where the initial value of $PATH in Linux includes
everything which has been added to %PATH% on the Windows side; after
adding that command to $HOME/.bashrc the WSL environment will no longer
pick up any changes made by newly installed Windows software.

Instead of that, use single quotes around the command, so that the
environment variables are not expanded at the setup time, and the
command that is added to $HOME/.bashrc becomes exactly this:

    PATH="$HOME/.local/bin:$PATH"

This command will use the $HOME and $PATH environment variable values at
the time the command is executed, not at the time the QMK setup is
performed, so any further updates to $PATH are taken into account.
Double quotes also ensure that the command is safe even if the values of
those environment variables contain spaces.
@sigprof sigprof deleted the docs-debian-ubuntu-path-fix branch June 15, 2020 14:36
jakobaa pushed a commit to jakobaa/qmk_firmware that referenced this pull request Jul 7, 2020
@sigprof @jakobaa
Really fix Ubuntu/Debian setup when $PATH contains spaces (qmk#9370)
a152a4e 
PR qmk#9307 fixed the immediately visible problem (the command that was
added to $HOME/.bashrc was incorrect because of missing quotes around
paths with spaces).  However, the modified command is still wrong - it
captures the value of $PATH at the setup time, and the resulting command
written out to $HOME/.bashrc will overwrite $PATH with that captured
value, ignoring any changes in the environment.  This may be especially
important for WSL, where the initial value of $PATH in Linux includes
everything which has been added to %PATH% on the Windows side; after
adding that command to $HOME/.bashrc the WSL environment will no longer
pick up any changes made by newly installed Windows software.

Instead of that, use single quotes around the command, so that the
environment variables are not expanded at the setup time, and the
command that is added to $HOME/.bashrc becomes exactly this:

    PATH="$HOME/.local/bin:$PATH"

This command will use the $HOME and $PATH environment variable values at
the time the command is executed, not at the time the QMK setup is
performed, so any further updates to $PATH are taken into account.
Double quotes also ensure that the command is safe even if the values of
those environment variables contain spaces.
BorisTestov pushed a commit to BorisTestov/qmk_firmware that referenced this pull request May 23, 2024
@sigprof
Really fix Ubuntu/Debian setup when $PATH contains spaces (qmk#9370)
1442686 
PR qmk#9307 fixed the immediately visible problem (the command that was
added to $HOME/.bashrc was incorrect because of missing quotes around
paths with spaces).  However, the modified command is still wrong - it
captures the value of $PATH at the setup time, and the resulting command
written out to $HOME/.bashrc will overwrite $PATH with that captured
value, ignoring any changes in the environment.  This may be especially
important for WSL, where the initial value of $PATH in Linux includes
everything which has been added to %PATH% on the Windows side; after
adding that command to $HOME/.bashrc the WSL environment will no longer
pick up any changes made by newly installed Windows software.

Instead of that, use single quotes around the command, so that the
environment variables are not expanded at the setup time, and the
command that is added to $HOME/.bashrc becomes exactly this:

    PATH="$HOME/.local/bin:$PATH"

This command will use the $HOME and $PATH environment variable values at
the time the command is executed, not at the time the QMK setup is
performed, so any further updates to $PATH are taken into account.
Double quotes also ensure that the command is safe even if the values of
those environment variables contain spaces.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fauxpark fauxpark fauxpark approved these changes

@zvecr zvecr zvecr approved these changes

Assignees
No one assigned
Labels
bug documentation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@sigprof @zvecr @fauxpark