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(build): fix version number when building with old Sapling #712

Closed
wants to merge 1 commit into from

Conversation

vegerot
Copy link
Contributor

@vegerot vegerot commented Aug 25, 2023

fix(build): fix version number when building with old Sapling

Summary:
Fix a bug (#711) in Sapling that causes sl --version to not work when
building Sapling with an old version of Sapling.

The way setup.py gets the version number is by first finding the path to
the vc exe
(findhg) and then running the sl log ... (or git show ...). findhg
interprets the hint[old-version] as an error and aborts.

This commit fixes the bug by simply adding hint[old-version] to filterhgerr

Test Plan:
It doesn't seem straightforward to write an automated test for this since we
don't have tests for setup.py. The manual steps to test would be:

  1. get an old sl
$ sl go "bsearch(date('-50'),main)"
- verify it's old by running any `sl` command and make sure you get
  `hint[old-version]: WARNING! ...`
  1. build Sapling (make oss)
  2. ./sl --version

Before this commit:

$ sl --version
Saping 4.4.2

This commit:

$ sl --version
Sapling <date-and-stuff>

Closes #711

Summary:
Fix a bug (facebook#711) in Sapling that causes `sl --version` to not work when
building Sapling with an old version of Sapling.

The way `setup.py` gets the version number is by first **finding the path to
the vc exe** (`findhg`) and then running the `sl log ...` (or `git show ...`).  `findhg`
interprets the `hint[old-version]` as an error and aborts.

This commit fixes the bug by simply adding `hint[old-version]` to `filterhgerr`

Test Plan:
It doesn't seem straightforward to write an automated test for this since we
don't have tests for `setup.py`.  The manual steps to test would be:

1. get an old `sl`
```sh
$ sl go "bsearch(date('-50'),main)"
```
    - verify it's old by running any `sl` command and make sure you get
      `hint[old-version]: WARNING! ...`
2. build Sapling (`make oss`)
3. `./sl --version`

Before this commit:

```sh
$ sl --version
Saping 4.4.2
```

This commit:
```sh
$ sl --version
Sapling <date-and-stuff>
```

Closes facebook#711
@zzl0
Copy link
Contributor

zzl0 commented Aug 25, 2023

LGTM! Thanks for fixing it.

@facebook-github-bot
Copy link
Contributor

@zzl0 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@zzl0 merged this pull request in 892ef54.

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

Successfully merging this pull request may close these issues.

sl --version breaks when building Sapling from an old Sapling
3 participants