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.

Sign up for GitHub

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

Jump to bottom

github_repo fails when revision is a non-semver version number #2829

Open
chrisnovakovic opened this issue Jun 19, 2023 · 2 comments · May be fixed by #2832
Open

github_repo fails when revision is a non-semver version number #2829

chrisnovakovic opened this issue Jun 19, 2023 · 2 comments · May be fixed by #2832
Labels
bug

Comments

@chrisnovakovic
Copy link
Contributor

MWE:

## java/snappy-java/BUILD

github_repo(
    name = "snappy-java",
    repo = "xerial/snappy-java",
    revision = "v1.1.10.1",
    hashes = ["b5dcf03f9618bd33fd9f424d9acf711a175aecd690cfca31440954723064d449"],
    build_file = "snappy-java.build",
)

$ plz build //java/snappy-java:snappy-java
Build stopped after 60ms. 1 target failed:
    //java/snappy-java:snappy-java
Error building target //java/snappy-java:snappy-java: exit status 1
mv: cannot move 'java/snappy-java/snappy-java.build' to '[...]/plz-out/tmp/java/snappy-java/snappy-java._build/snappy-java/BUILD': No such file or directory

This happens because the first command github_repo runs - namely, the GitHub archive zip extraction operation using arcat - fails because arcat is given the wrong prefix to strip by the rule, but exits successfully:

bash-5.1$ unzip -l $SRCS_REMOTE | head -n5
Archive:  java/snappy-java/snappy-java_v1.1.10.1.zip
737f3973ff392c351da2744820a23a0521c6cc77
  Length      Date    Time    Name
---------  ---------- -----   ----
        0  2023-06-14 22:51   snappy-java-1.1.10.1/
bash-5.1$ $TOOL x $SRCS_REMOTE -o "$OUT" -s snappy-java-v1.1.10.1
bash-5.1$ echo $?
0

GitHub doesn't publish the algorithm they use to decide when to strip the v prefix from a tag name during the archival process, so the rule currently assumes that any revision that looks like a semver will have any leading v stripped from the top-level directory name in the exported archive. In reality the logic appears to be more complicated than that. It's probably safe to assume that any dotted version number format will also cause the v prefix to be stripped on GitHub's side.

In addition, perhaps it's worth changing arcat so that it warns/errors if any value given by -s results in no files being extracted - the underlying cause would then become clearer here.
chrisnovakovic added a commit to chrisnovakovic/please that referenced this issue Jun 19, 2023
@chrisnovakovic
github_repo: strip v prefix if revision is any version number
51cb95e 
`github_repo` currently strips the `v` prefix from the file names in the
zip file downloaded from GitHub if `revision` is a semver, since GitHub
silently does the same if asked to archive the code for semver-like
tags. It looks like this logic extends to more than just semvers,
though - the same behaviour has been observed for strings that are
simple dotted version numbers with any number of components (e.g.
`1.1.10.1`). Account for this in the prefix-stripping logic.

Fixes thought-machine#2829.
@chrisnovakovic chrisnovakovic linked a pull request Jun 19, 2023 that will close this issue
Open
chrisnovakovic added a commit to chrisnovakovic/please that referenced this issue Jun 19, 2023
@chrisnovakovic
github_repo: strip v prefix if revision is any version number
9a9a9b9 
`github_repo` currently strips the `v` prefix from the file names in the
zip file downloaded from GitHub if `revision` is a semver, since GitHub
silently does the same if asked to archive the code for semver-like
tags. It looks like this logic extends to more than just semvers,
though - the same behaviour has been observed for strings that are
simple dotted version numbers with any number of components (e.g.
`v1.1.10.1`). Account for this in the prefix-stripping logic.

Fixes thought-machine#2829.
@stale
Copy link

stale bot commented Sep 17, 2023

This issue has been automatically marked as stale because it has not had any recent activity in the past 90 days. It will be closed if no further activity occurs. If you require additional support, please reply to this message. Thank you for your contributions.

@stale stale bot added the wontfix label Sep 17, 2023
@chrisnovakovic
Copy link
Contributor Author

Not stale.

@chrisnovakovic chrisnovakovic mentioned this issue Dec 27, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

github_repo: strip v prefix if revision is any version number chrisnovakovic/please
1 participant
@chrisnovakovic