Skip to content

Improve fork check in Git hooks installation #268

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

Merged
merged 1 commit into from
Mar 17, 2025

Conversation

willy-liu
Copy link
Contributor

This change allows users to modify the repository name when working with a fork.

Modifications:

  • Introduced repository name to store the repository name dynamically.
  • Replaced fixed occurrences of lab0-c in API calls with current repository name.
  • Improved fork validation by:
    1. Checking if the repository is a fork.
    2. Verifying that the parent repository's full name.

Note:

  • This change requires a tool to parse the response from github.com.

@willy-liu
Copy link
Contributor Author

willy-liu commented Mar 13, 2025

The github api, https://api.github.com/repos/${ACCOUNT}/${REPO_NAME}, return with a JSON object like the one below:

{
  "id": 944494149,
  "node_id": "R_kgDOOEvWRQ",
  "name": "lab0-c_dev",
  "full_name": "willy-liu/lab0-c_dev",
  "fork": true,
  "parent": {
    "id": 149674932,
    "node_id": "MDEwOlJlcG9zaXRvcnkxNDk2NzQ5MzI=",
    "name": "lab0-c",
    "full_name": "sysprog21/lab0-c",
    }
}

The "fork" value indicates whether the repository is forked from another repository.

If "fork" is true, the API will also return the "parent" field, which contains details of the original repository. We can check .parent.full_name to verify that this repository is forked from "sysprog21/lab0-c".

However, since grep does not handle JSON format well, I added a jq installation check after checking for curl installation.

Comment on lines 42 to 46
JQ=$(which jq)
if [ $? -ne 0 ]; then
echo "jq not installed."
exit 1
fi
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't do that. The jq is not required for this case. Use sed/awk instead.

@willy-liu
Copy link
Contributor Author

  1. Correct the regular expressions for ACCOUNT and REPO_NAME.
  2. Remove checking the command jq
  3. Rewrite jq code with sed/awk.

@@ -31,7 +31,8 @@ fi
((CURRENT_STEP++))
progress "$CURRENT_STEP" "$TOTAL_STEPS"

ACCOUNT=$(git config -l | grep -w remote.origin.url | sed -e 's/^.*github.com[\/:]\(.*\)\/lab0-c.*/\1/')
ACCOUNT=$(git config --get remote.origin.url | awk -F'[:/]' '{print $(NF-1)}')
REPO_NAME=$(git config --get remote.origin.url | awk -F'[:/]' '{gsub(/\.git$/, "", $NF); print $NF}')
Copy link
Contributor

Choose a reason for hiding this comment

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

The homework assignment requires that the repository must always be named lab0-c. This naming convention serves as a strict identification mechanism to determine who is participating in the assignment.

*)
throw "Your repository MUST be forked from 'sysprog21/lab0-c'."
esac
RESPONSE=$(${CURL} -s -H "Accept: application/vnd.github.v3+json" \
Copy link
Contributor

Choose a reason for hiding this comment

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

You should ensure the content retrieved from the curl command is not empty.

@willy-liu
Copy link
Contributor Author

This change enforces that the repository name is "lab0-c" both locally
and on GitHub while ensuring it is correctly forked from
"sysprog21/lab0-c." It validates both the fork status and the parent
repository name using the GitHub API.

Modifications:

  • Replaced grep and sed parsing with Git commands and awk for extracting
    the repository owner and name.
  • Added validation to ensure the repository name is "lab0-c."
  • Improved fork verification by checking the GitHub API response for both
    fork status and parent repository name.

Copy link
Contributor

@jserv jserv left a comment

Choose a reason for hiding this comment

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

Refine the commit messages to avoid unnecessary bullets for modification items. Always use plain English sentences to describe.

This change ensures that the repository is named "lab0-c" both
locally and on GitHub while verifying that it is correctly forked
from "sysprog21/lab0-c." The validation now relies on the GitHub
API to check both the fork status and the parent repository name.

The previous approach using grep and sed for parsing has been replaced
with Git commands and awk to extract the repository owner and name.
Additional validation ensures the repository name is "lab0-c",
and fork verification has been improved by directly analyzing
the GitHub API response.
@jserv jserv merged commit fcacc73 into sysprog21:master Mar 17, 2025
1 of 2 checks passed
@jserv
Copy link
Contributor

jserv commented Mar 17, 2025

Thank @willy-liu for contributing!

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

Successfully merging this pull request may close these issues.

2 participants