fix bugs intruduced by my previous PR #50
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
gutjuri
commented
Jan 26, 2024
| @@ -16,12 +16,12 @@ def getJson(install, prefix, par): | |||
| osStr = (platform.system()) | |||
| cwd = os.getcwd() | |||
| folder = cwd + "/json" | |||
| url = "https://github.com/nlohmann/json.git " | |||
| url = "https://github.com/nlohmann/json.git" | |||
| json_branch_or_tag = "develop" | |||
There was a problem hiding this comment.
Also, the previous version of the script to install nlohmann/json didn't explicitly specify a branch or tag or commit-id to pull, so it just pulled the default branch (develop). i have made this explicit in this PR. However, I don't know whether or not develop is stable, so if you think that it is appropriate you can change this to some release tag (e.g. v3.11.3)
Suggested change
| json_branch_or_tag = "develop" | |
| json_branch_or_tag = "v3.11.3" |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I noticed that my PR broke your pipeline, so I investigated what the problem is.
Turns out that this command
aby3/thirdparty/getFunction2.py
Line 24 in f50a098
The reason for the failure is that cloning with --depth 1 means that the git history isn't downloaded. Only the current state of the master branch is. I fixed this by specifying the desired version of the library in the clone command itself (tag 4.1.0 is the same commit that was previously specified with the hash 3a0746... I wrote 4.1.0 because I think this is more readable).
Hope the pipeline now runs through and sorry for wasting your time this way... Nevertheless i hope this my PRs save you and other contributors time in the future.
I have now tested my change locally by building the project, so I am confident that it works now :D Also, the CI pipeline is green again.