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

Fix a few syntax issues in shell scripts #62102

Merged
merged 4 commits into from
Nov 30, 2021

Conversation

am11
Copy link
Member

@am11 am11 commented Nov 27, 2021

This delta updates shell scripts for consistency. In particular, avoid mixing Bash and Unix shell syntax by replacing:

  • [ $x == $y ] and [[ $x = $y ]] with [[ $x == $y ]]
  • [[ $x ]] && [[ $y ]] and [ $x ] && [ $y ] with [[ $x && $y ]]
  • [ ! -z $x ] and [ $x != "" ] with [ -n $x ]
  • [ ! -n $x ] and [ $x == "" ] with [ -z $x ]
  • ! $x == $y with $x != $y
  • #!/bin/bash with #!/usr/bin/env bash
  • [[ with [ in files with /bin/sh shebang

@am11 am11 requested a review from marek-safar as a code owner November 27, 2021 22:46
@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

@am11 am11 force-pushed the feature/build/script-fixes branch from 9520955 to ab8553c Compare November 28, 2021 01:09
am11 added 2 commits November 28, 2021 19:11
```sh
# git remote add dotnet https://github.com/dotnet/runtime && git pull --rebase dotnet main

if uname 2>/devnull | grep -q Darwin; then
    space=" "
fi

git show --name-only --pretty="" HEAD...dotnet/main |\
    xargs -I{} sh -c "test -f {} && sed -i$space'' 's/[[:space:]]*$//' {}"
```
@am11 am11 force-pushed the feature/build/script-fixes branch from ab8553c to 17c6508 Compare November 28, 2021 17:19
@ghost
Copy link

ghost commented Nov 29, 2021

Tagging subscribers to this area: @dotnet/runtime-infrastructure
See info in area-owners.md if you want to be subscribed.

Issue Details

This delta updates shell scripts for consistency. In particular, avoid mixing Bash and Unix shell syntax by replacing:

  • [ $x == $y ] and [[ $x = $y ]] with [[ $x == $y ]]
  • [[ $x ]] && [[ $y ]] and [ $x ] && [ $y ] with [[ $x && $y ]]
  • [ ! -z $x ] and [ $x != "" ] with [ -n $x ]
  • [ ! -n $x ] and [ $x == "" ] with [ -z $x ]
  • ! $x == $y with $x != $y
  • #!/bin/bash with #!/usr/bin/env bash
  • [[ with [ in files with /bin/sh shebang
Author: am11
Assignees: -
Labels:

area-Infrastructure

Milestone: -

Copy link
Member

@hoyosjs hoyosjs left a comment

Choose a reason for hiding this comment

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

Just a couple minor questions.

Copy link
Member

@hoyosjs hoyosjs left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

@am11
Copy link
Member Author

am11 commented Nov 30, 2021

Resolved merge conflict with 85642f8.
@hoyosjs, I don't have permissions, could you please merge this? Thanks.

@safern
Copy link
Member

safern commented Nov 30, 2021

@radical could you please look at the staging failure?

@safern safern merged commit 6cc2d2e into dotnet:main Nov 30, 2021
@am11 am11 deleted the feature/build/script-fixes branch November 30, 2021 23:35
@ghost ghost locked as resolved and limited conversation to collaborators Dec 31, 2021
@antonfirsov
Copy link
Member

This change broke the (non-mandatory) SslStress pipeline.

When touching SslStress and HttpStress, please make sure to run the corresponding pipelines:

/azp run runtime-libraries stress-http
/azp run runtime-libraries stress-ssl

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants