Sitelet https://web.archive.org/web/20210701131023/https://github.com/PowerShell/PowerShell/pull/15180
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

Use Unix line endings for shell scripts #15180

Merged
merged 2 commits into from Apr 12, 2021
Merged

Use Unix line endings for shell scripts #15180

merged 2 commits into from Apr 12, 2021

Conversation

@xtqqczze
Copy link
Contributor

@xtqqczze xtqqczze commented Apr 7, 2021

PR Summary

  • Specify line ending behaviour in EditorConfig like in dotnet/runtime
  • Update line endings in *.sh

PR Context

These changes fix SC1017

PR Checklist

* Specify line ending behaviour in EditorConfig like in dotnet/runtime
* Update line endings

These changes fix [SC1017](https://github.com/koalaman/shellcheck/wiki/SC1017)
@msftbot msftbot bot assigned rjmholt Apr 7, 2021
@iSazonov
Copy link
Collaborator

@iSazonov iSazonov commented Apr 7, 2021 •

We already use core.autocrlf=true in git config. So the files already have LF in repository and on Unix.
https://stackoverflow.com/questions/1967370/git-replacing-lf-with-crlf

@rjmholt
rjmholt approved these changes Apr 7, 2021
@rjmholt rjmholt requested review from andschwa and SteveL-MSFT Apr 7, 2021
@iSazonov
Copy link
Collaborator

@iSazonov iSazonov commented Apr 8, 2021

See the history https://github.com/PowerShell/PowerShell/search?q=autocrlf&type=issues
Conclusion was to delegate this to Git autocrlf

Copy link
Member

@andschwa andschwa left a comment

I agree with @iSazonov that auto-crlf in Git ought to be ensuring this already happens, but I've had issues with that working 100% (you can forcibly add files, etc., it's just not perfect), so I see no harm setting this explicitly for shell scripts.

@iSazonov iSazonov added this to the 7.2.0-preview.5 milestone Apr 9, 2021
@msftbot msftbot bot removed this from the 7.2.0-preview.5 milestone Apr 9, 2021
@msftbot
Copy link

@msftbot msftbot bot commented Apr 9, 2021

Open PRs should not be assigned to milestone, so they are not assigned to the wrong milestone after they are merged. For backport consideration, use a backport label.

@xtqqczze
Copy link
Contributor Author

@xtqqczze xtqqczze commented Apr 9, 2021 •

The issue is that shell scripts should never contain \r\n line terminators.

https://github.com/dotnet/runtime/blob/main/.gitattributes has:
*.sh text eol=lf

Apparently if this was not the case there could be issues if a shell script was accessed in Unix via a file share from Windows.

@rjmholt rjmholt merged commit e118f7e into PowerShell:master Apr 12, 2021
31 checks passed
31 checks passed
@github-actions
Analyze (csharp)
Details
@github-code-scanning
CodeQL No new or fixed alerts
Details
CodeFactor No issues found.
Details
@azure-pipelines
PowerShell-CI-linux Build #PR-15180-20210407.02 succeeded
Details
@azure-pipelines
PowerShell-CI-linux (Build for Linux linux Build) Build for Linux linux Build succeeded
Details
@azure-pipelines
PowerShell-CI-linux (CodeCoverage and Test Packages CodeCoverage and Test Packages) CodeCoverage and Test Packages CodeCoverage and Test Packages succeeded
Details
@azure-pipelines
PowerShell-CI-linux (Test for Linux Linux Test - ElevatedPesterTests - CI) Test for Linux Linux Test - ElevatedPesterTests - CI succeeded
Details
@azure-pipelines
PowerShell-CI-linux (Test for Linux Linux Test - ElevatedPesterTests - Others) Test for Linux Linux Test - ElevatedPesterTests - Others succeeded
Details
@azure-pipelines
PowerShell-CI-linux (Test for Linux Linux Test - UnelevatedPesterTests - CI) Test for Linux Linux Test - UnelevatedPesterTests - CI succeeded
Details
@azure-pipelines
PowerShell-CI-linux (Test for Linux Linux Test - UnelevatedPesterTests - Others) Test for Linux Linux Test - UnelevatedPesterTests - Others succeeded
Details
@azure-pipelines
PowerShell-CI-linux (Test for Linux Verify xUnit Results) Test for Linux Verify xUnit Results succeeded
Details
@azure-pipelines
PowerShell-CI-macos Build #PR-15180-20210407.02 succeeded
Details
@azure-pipelines
PowerShell-CI-macos (Build for macOS macOS Build) Build for macOS macOS Build succeeded
Details
@azure-pipelines
PowerShell-CI-macos (CodeCoverage and Test Packages CodeCoverage and Test Packages) CodeCoverage and Test Packages CodeCoverage and Test Packages succeeded
Details
@azure-pipelines
PowerShell-CI-macos (Test for macOS Verify xUnit Results) Test for macOS Verify xUnit Results succeeded
Details
@azure-pipelines
PowerShell-CI-macos (Test for macOS mac Test - ElevatedPesterTests - CI) Test for macOS mac Test - ElevatedPesterTests - CI succeeded
Details
@azure-pipelines
PowerShell-CI-macos (Test for macOS mac Test - ElevatedPesterTests - Others) Test for macOS mac Test - ElevatedPesterTests - Others succeeded
Details
@azure-pipelines
PowerShell-CI-macos (Test for macOS mac Test - UnelevatedPesterTests - CI) Test for macOS mac Test - UnelevatedPesterTests - CI succeeded
Details
@azure-pipelines
PowerShell-CI-macos (Test for macOS mac Test - UnelevatedPesterTests - Others) Test for macOS mac Test - UnelevatedPesterTests - Others succeeded
Details
@azure-pipelines
PowerShell-CI-static-analysis Build #PR-15180-20210407.02 succeeded
Details
@azure-pipelines
PowerShell-CI-static-analysis (CI Compliance) CI Compliance succeeded
Details
@azure-pipelines
PowerShell-CI-static-analysis (Markdown and Common Tests) Markdown and Common Tests succeeded
Details
@azure-pipelines
PowerShell-CI-windows Build #PR-15180-20210407.02 succeeded
Details
@azure-pipelines
PowerShell-CI-windows (Build for Windows Windows Build) Build for Windows Windows Build succeeded
Details
@azure-pipelines
PowerShell-CI-windows (Test for Windows Verify xUnit Results) Test for Windows Verify xUnit Results succeeded
Details
@azure-pipelines
PowerShell-CI-windows (Test for Windows Windows Test - ElevatedPesterTests - CI) Test for Windows Windows Test - ElevatedPesterTests - CI succeeded
Details
@azure-pipelines
PowerShell-CI-windows (Test for Windows Windows Test - ElevatedPesterTests - Others) Test for Windows Windows Test - ElevatedPesterTests - Others succeeded
Details
@azure-pipelines
PowerShell-CI-windows (Test for Windows Windows Test - UnelevatedPesterTests - CI) Test for Windows Windows Test - UnelevatedPesterTests - CI succeeded
Details
@azure-pipelines
PowerShell-CI-windows (Test for Windows Windows Test - UnelevatedPesterTests - Others) Test for Windows Windows Test - UnelevatedPesterTests - Others succeeded
Details
@wip
WIP Ready for review
Details
@microsoft-cla
license/cla All CLA requirements met.
Details
@iSazonov iSazonov added this to the 7.2.0-preview.5 milestone Apr 12, 2021
@xtqqczze xtqqczze deleted the xtqqczze:sh-lf branch Apr 12, 2021
@msftbot
Copy link

@msftbot msftbot bot commented Apr 14, 2021

🎉v7.2.0-preview.5 has been released which incorporates this pull request.🎉

Handy links:

TravisEz13 added a commit that referenced this pull request Apr 16, 2021
[7.2.0-preview.5] - 2021-04-14

* Breaking Changes

- Make PowerShell Linux deb and RPM packages universal (#15109)
- Enforce AppLocker Deny configuration before Execution Policy Bypass configuration (#15035)
- Disallow mixed dash and slash in command line parameter prefix (#15142) (Thanks @davidBar-On!)

* Experimental Features

- `PSNativeCommandArgumentPassing`: Use `ArgumentList` for native executable invocation (breaking change) (#14692)

* Engine Updates and Fixes

- Add `IArgumentCompleterFactory` for parameterized `ArgumentCompleters` (#12605) (Thanks @powercode!)

* General Cmdlet Updates and Fixes

- Fix SSH remoting connection never finishing with misconfigured endpoint (#15175)
- Respect `TERM` and `NO_COLOR` environment variables for `$PSStyle` rendering (#14969)
- Use `ProgressView.Classic` when Virtual Terminal is not supported (#15048)
- Fix `Get-Counter` issue with `-Computer` parameter (#15166) (Thanks @krishnayalavarthi!)
- Fix redundant iteration while splitting lines (#14851) (Thanks @hez2010!)
- Enhance `Remove-Item -Recurse` to work with OneDrive (#14902) (Thanks @iSazonov!)
- Change minimum depth to 0 for `ConvertTo-Json` (#14830) (Thanks @kvprasoon!)
- Allow `Set-Clipboard` to accept empty string (#14579)
- Turn on and off `DECCKM` to modify keyboard mode for Unix native commands to work correctly (#14943)
- Fall back to `CopyAndDelete()` when `MoveTo()` fails due to an `IOException` (#15077)

* Code Cleanup

<details>

<summary>

<p>We thank the following contributors!</p>
<p>@xtqqczze, @iSazonov, @ZhiZe-ZG</p>

</summary>

<ul>
<li>Update .NET to <code>6.0.0-preview.3</code> (#15221)</li>
<li>Add space before comma to hosting test to fix error reported by <code>SA1001</code> (#15224)</li>
<li>Add <code>SecureStringHelper.FromPlainTextString</code> helper method for efficient secure string creation (#14124) (Thanks @xtqqczze!)</li>
<li>Use static lambda keyword (#15154) (Thanks @iSazonov!)</li>
<li>Remove unnecessary <code>Array</code> -&gt; <code>List</code> -&gt; <code>Array</code> conversion in <code>ProcessBaseCommand.AllProcesses</code> (#15052) (Thanks @xtqqczze!)</li>
<li>Standardize grammar comments in Parser.cs (#15114) (Thanks @ZhiZe-ZG!)</li>
<li>Enable <code>SA1001</code>: Commas should be spaced correctly (#14171) (Thanks @xtqqczze!)</li>
<li>Refactor <code>MultipleServiceCommandBase.AllServices</code> (#15053) (Thanks @xtqqczze!)</li>
</ul>

</details>

* Tools

- Use Unix line endings for shell scripts (#15180) (Thanks @xtqqczze!)

* Tests

- Add the missing tag in Host Utilities tests (#14983)
- Update `copy-props` version in `package.json` (#15124)

* Build and Packaging Improvements

<details>

<summary>

<p>We thank the following contributors!</p>
<p>@JustinGrote</p>

</summary>

<ul>
<li>Fix <code>yarn-lock</code> for <code>copy-props</code> (#15225)</li>
<li>Make package validation regex accept universal Linux packages (#15226)</li>
<li>Bump NJsonSchema from 10.4.0 to 10.4.1 (#15190)</li>
<li>Make MSI and EXE signing always copy to fix daily build (#15191)</li>
<li>Sign internals of EXE package so that it works correctly when signed (#15132)</li>
<li>Bump Microsoft.NET.Test.Sdk from 16.9.1 to 16.9.4 (#15141)</li>
<li>Update daily release tag format to  work with new Microsoft Update work (#15164)</li>
<li>Feature: Add Ubuntu 20.04 Support to install-powershell.sh (#15095) (Thanks @JustinGrote!)</li>
<li>Treat rebuild branches like release branches (#15099)</li>
<li>Update WiX to 3.11.2 (#15097)</li>
<li>Bump NJsonSchema from 10.3.11 to 10.4.0 (#15092)</li>
<li>Allow patching of preview releases (#15074)</li>
<li>Bump Newtonsoft.Json from 12.0.3 to 13.0.1 (#15084, #15085)</li>
<li>Update the <code>minSize</code> build package filter to be explicit (#15055)</li>
<li>Bump NJsonSchema from 10.3.10 to 10.3.11 (#14965)</li>
</ul>

</details>

* Documentation and Help Content

- Merge `7.2.0-preview.4` changes to master (#15056)
- Update `README` and `metadata.json` (#15046)
- Fix broken links for `dotnet` CLI (#14937)

[7.2.0-preview.5]: v7.2.0-preview.4...v7.2.0-preview.5
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants