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

Sed for linux #250

Merged
Merged

Conversation

BluThaitanium
Copy link
Contributor

@BluThaitanium BluThaitanium commented Oct 27, 2021

Fixes updating licensing by running the correct sed command corresponding to Linux and MacOS
For readability- Sed commands are in separate functions, called by each xxxx_comment() function in add_license_headers.sh during execution.
Throws error statement without exit code, indicating the command isn't working due to OS limitations of sed (i.e. user is using Windows)

Resolves #249

@ckadner

@BluThaitanium BluThaitanium force-pushed the sed_for_linux_ branch 2 times, most recently from ecc8cf5 to a17ce00 Compare October 27, 2021 21:27
Copy link
Member

@ckadner ckadner left a comment

Choose a reason for hiding this comment

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

Thanks Phu. This is great. We need to make a few more changes though.

  1. We need to exclude any *.bundle.js files. These are generated and meant to be as small as possible
  2. We need different replacements for the various file types since comments in Bash scripts are different from comments in HTML or CSS. So we should instead create one alias for the sed command, i.e. call it gsed at the beginning of the script and then use that and leave the hash_comment and slash_comment, ... functions unchanged otherwise.
if [[ "$OSTYPE" == "linux-gnu"* ]]; then  # Linux
    alias gsed="sed -i"
elif [[ "$OSTYPE" == "darwin"* ]]; then  # macOS
    alias gsed="sed -i ''"
elif [[ "$OSTYPE" == "cygwin" ]]; then  # POSIX compatible emulation for Windows
    alias gsed="sed -i"
else
    echo "FAILED. OS not compatible with script '/hack/add_license_headers.sh'"
    exit 1
fi

And then use gsed in the *_comment () functions:

hash_comment () {
  echo "$1"
  if ! grep -q "SPDX-License-Identifier" "$1"
  then
    gsed '1i\
    # Copyright 2021 The MLX Contributors\
    #\
    # SPDX-License-Identifier: Apache-2.0\
    ' "$1"
  fi
}

hack/regenerate_catalog_upload_json.py Show resolved Hide resolved
dashboard/origin-mlx/public/dashboard_lib.bundle.js Outdated Show resolved Hide resolved
@ckadner ckadner removed the request for review from animeshsingh October 27, 2021 22:31
Signed-off-by: Phu Thai <[email protected]>

macos syntax fix

Signed-off-by: Phu Thai <[email protected]>
Signed-off-by: Phu Thai <[email protected]>

removed bundle license

ignore *.bundle.js

Signed-off-by: Phu Thai <[email protected]>
Copy link
Member

@ckadner ckadner left a comment

Choose a reason for hiding this comment

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

Thanks Phu just a few more changes :-)

hack/add_license_headers.sh Outdated Show resolved Hide resolved
hack/add_license_headers.sh Outdated Show resolved Hide resolved
hack/add_license_headers.sh Outdated Show resolved Hide resolved
hack/add_license_headers.sh Outdated Show resolved Hide resolved
hack/add_license_headers.sh Outdated Show resolved Hide resolved
Copy link
Member

@ckadner ckadner left a comment

Choose a reason for hiding this comment

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

Thanks @BluThaitanium.

I just gave it a spin and came across a few issues.

hack/add_license_headers.sh Outdated Show resolved Hide resolved
hack/add_license_headers.sh Outdated Show resolved Hide resolved
hack/add_license_headers.sh Outdated Show resolved Hide resolved
hack/regenerate_catalog_upload_json.py Show resolved Hide resolved
Copy link
Member

@ckadner ckadner left a comment

Choose a reason for hiding this comment

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

/lgtm

Thank you @BluThaitanium

@mlx-bot
Copy link
Collaborator

mlx-bot commented Nov 2, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: BluThaitanium, ckadner

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@mlx-bot mlx-bot merged commit 0dd8c4f into machine-learning-exchange:main Nov 2, 2021
@ckadner ckadner added the RCOS Potential work items for RCOS student interns label Dec 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved lgtm RCOS Potential work items for RCOS student interns
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Generating Licensing Headers do not work on Ubuntu Linux
3 participants