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

Windows Path on Bootstrap #8669

Merged
merged 5 commits into from
Jan 16, 2020
Merged

Windows Path on Bootstrap #8669

merged 5 commits into from
Jan 16, 2020

Conversation

xorima
Copy link
Contributor

@xorima xorima commented Jun 18, 2019

Fix issue with windows path during bootstrap

Description

As a windows user when I bootstrap a node I expect the path to not have items removed as my cookbooks may depend on executables not stored on common system paths, such as sqlcmd which will break and cause the initial chef run to fail

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

Checklist:

  • I have read the CONTRIBUTING document.
  • I have run the pre-merge tests locally and they pass.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • All commits have been signed-off for the Developer Certificate of Origin.

As a windows user when I bootstrap a node I expect the path to not have items removed as my cookbooks may depend on executables not stored on common system paths, such as sqlcmd which will break and cause the initial chef run to fail

Signed-off-by: Jason Field <[email protected]>
@tas50 tas50 requested a review from marcparadise June 24, 2019 23:41
@tas50
Copy link
Contributor

tas50 commented Jun 24, 2019

Another one for you @marcparadise

@xorima
Copy link
Contributor Author

xorima commented Jun 25, 2019

I think the reason this was changed was too long file paths but it has other unintended causes,
I guess the question is how can we validate the length of the path without it getting too long, and as we are using a cmd file it limits the options we would have with say powershell

Updated path as per feedback
@xorima xorima requested review from a team as code owners August 7, 2019 07:41
@xorima
Copy link
Contributor Author

xorima commented Aug 7, 2019

Resolved as per feedback @marcparadise apologies for the delay

1 similar comment
@xorima
Copy link
Contributor Author

xorima commented Aug 7, 2019

Resolved as per feedback @marcparadise apologies for the delay

@xorima
Copy link
Contributor Author

xorima commented Aug 8, 2019

tests failing on linux so doubt it is due to this one?

xorima added 2 commits August 12, 2019 11:47
Fixes for bad copy paste

Signed-off-by: Jason Field <[email protected]>
@xorima
Copy link
Contributor Author

xorima commented Oct 3, 2019

Nudge @marcparadise could this be looked at please?

@lamont-granquist lamont-granquist merged commit 1c671d1 into chef:master Jan 16, 2020
Copy link
Contributor

@robbkidd robbkidd left a comment

Choose a reason for hiding this comment

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

tenor-204362819

@xorima xorima deleted the windows_path branch January 16, 2020 21:54
@lock
Copy link

lock bot commented Jan 30, 2020

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Jan 30, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants