Skip to content

Conversation

@ahornby
Copy link
Owner

@ahornby ahornby commented Oct 21, 2024

Summary:

  • filter out hexdump dependency on macos, its a system util there
  • set brew dependency to python@3.11, not @3.8. Our pyo3 dependency is too old for python@3.13, and python@3.12 doesn't install all the symlinks necessary.
  • Makefile fixes:
    • macOS's old make doesn't like comment in continued shell line, remove it
    • fix sed job count parsing on macOS make

After this I can build and run cli without "unknown python exception", and I can start the test suite (although its very slow compared to my linux server, not got a full run yet).

Test Plan:

local build
install brew dependencies:

./build/fbcode_builder/getdeps.py install-system-deps build sapling

build

./build/fbcode_builder/getdeps.py --allow-system-packages build --src-dir=. sapling

test binary works without "unknown python exception":

Mon 21 Oct 2024 18:46:05 BST [Exit: 0 0] ahornby@ahornby-mbp:~/local/sapling-sl/eden/scm/tests
$ /Users/ahornby/.scratch/UsersZahornbyZlocalZsapling-sl/fbcode_builder_getdeps/installed/sapling/bin/sl --help | head -4
Mon 21 Oct 2024 18:46:07 BST
Sapling SCM

sl COMMAND [OPTIONS]

run tests
Before, failed to run any tests

After, runs tests slow and lots currently don't pass. We don't have CI up for macOS getdeps at moment, so can address that separarately

./build/fbcode_builder/getdeps.py --allow-system-packages test --retry 0 --num-jobs=16 --src-dir=. sapling

Summary:
   * filter out hexdump dependency on macos, its a system util there
   * set brew dependency to python@3.11, not @3.8.  Our pyo3 dependency is too old for python@3.13, and python@3.12 doesn't install all the symlinks necessary.
   * Makefile fixes:
      * macOS's old make doesn't like comment in continued shell line, remove it
      * fix sed job count parsing on macOS make

After this I can build and run cli without "unknown python exception", and I can start the test suite (although its very slow compared to my linux server, not got a full run yet).

Test Plan:

**local build**
install brew dependencies:
```
./build/fbcode_builder/getdeps.py install-system-deps build sapling
```
build
```
./build/fbcode_builder/getdeps.py --allow-system-packages build --src-dir=. sapling
```
test binary works without "unknown python exception":
```
Mon 21 Oct 2024 18:46:05 BST [Exit: 0 0] ahornby@ahornby-mbp:~/local/sapling-sl/eden/scm/tests
$ /Users/ahornby/.scratch/UsersZahornbyZlocalZsapling-sl/fbcode_builder_getdeps/installed/sapling/bin/sl --help | head -4
Mon 21 Oct 2024 18:46:07 BST
Sapling SCM

sl COMMAND [OPTIONS]
```

**run tests**
Before, failed to run any tests

After, runs tests slow and  lots currently don't pass.  We don't have CI up for macOS getdeps at moment, so can address that separarately
```
./build/fbcode_builder/getdeps.py --allow-system-packages test --retry 0 --num-jobs=16 --src-dir=. sapling
```
ahornby pushed a commit that referenced this pull request Nov 22, 2024
Summary:
Fixes this (which was polluting my `arc rust-check` output):

```
warning: unused variable: `time`
   --> fbcode/eden/scm/saplingnative/bindings/modules/pymetalog/src/lib.rs:119:38
    |
119 |     def commit(&self, message: &str, time: Option<u64> = None, pending: bool = false) -> PyResult<Bytes> {
    |                                      ^^^^
    |
help: `time` is captured in macro and introduced a unused variable
   --> third-party/rust/vendor/cpython-0.7.2/src/py_class/py_class.rs:478:1
    |
    = note: in this expansion of `py_class!` (#1)
   ::: third-party/rust/vendor/cpython-0.7.2/src/py_class/py_class.rs:537:9
    |
    = note: in this macro invocation (#2)
   ::: third-party/rust/vendor/cpython-0.7.2/src/py_class/py_class.rs:543:1
    |
    = note: in this expansion of `$crate::py_class_impl_item!` (#18)
   --> third-party/rust/vendor/cpython-0.7.2/src/py_class/py_class_impl3.rs:30:1
    |
    = note: in this expansion of `$crate::py_class_impl!` (#2)
    |
    = note: in this expansion of `$crate::py_class_impl!` (#3)
    |
    = note: in this expansion of `$crate::py_class_impl!` (#4)
    |
    = note: in this expansion of `$crate::py_class_impl!` (#5)
    |
    = note: in this expansion of `$crate::py_class_impl!` (#6)
    |
    = note: in this expansion of `$crate::py_class_impl!` (#7)
    |
    = note: in this expansion of `$crate::py_class_impl!` (#8)
    |
    = note: in this expansion of `$crate::py_class_impl!` (#9)
    |
    = note: in this expansion of `$crate::py_class_impl!` (#10)
    |
    = note: in this expansion of `$crate::py_class_impl!` (#11)
    |
    = note: in this expansion of `$crate::py_class_impl!` (#12)
    |
    = note: in this expansion of `$crate::py_class_impl!` (#13)
   ::: third-party/rust/vendor/cpython-0.7.2/src/py_class/py_class_impl3.rs:249:5
    |
    = note: in this macro invocation (#3)
    |
    = note: in this macro invocation (#4)
   ::: third-party/rust/vendor/cpython-0.7.2/src/py_class/py_class_impl3.rs:2392:5
    |
    = note: in this macro invocation (#5)
   ::: third-party/rust/vendor/cpython-0.7.2/src/py_class/py_class_impl3.rs:2970:5
    |
    = note: in this macro invocation (#7)
    |
    = note: in this macro invocation (#13)
   ::: third-party/rust/vendor/cpython-0.7.2/src/py_class/py_class_impl3.rs:2999:13
    |
    = note: in this macro invocation (#14)
   ::: third-party/rust/vendor/cpython-0.7.2/src/py_class/py_class_impl3.rs:3005:5
    |
    = note: in this macro invocation (#8)
    |
    = note: in this macro invocation (#11)
    |
    = note: in this macro invocation (#12)
   ::: third-party/rust/vendor/cpython-0.7.2/src/py_class/py_class_impl3.rs:3120:5
    |
    = note: in this macro invocation (#6)
    |
    = note: in this macro invocation (#9)
    |
    = note: in this macro invocation (#10)
   --> third-party/rust/vendor/cpython-0.7.2/src/argparse.rs:196:1
    |
    = note: in this expansion of `$crate::py_argparse_parse_plist_impl!` (#14)
    |
    = note: in this expansion of `$crate::py_argparse_parse_plist_impl!` (#15)
    |
    = note: in this expansion of `$crate::py_argparse_parse_plist_impl!` (#16)
    |
    = note: in this expansion of `$crate::py_argparse_parse_plist_impl!` (#17)
   ::: third-party/rust/vendor/cpython-0.7.2/src/argparse.rs:201:9
    |
    = note: in this macro invocation (#18)
   ::: third-party/rust/vendor/cpython-0.7.2/src/argparse.rs:271:9
    |
    = note: in this macro invocation (#15)
   ::: third-party/rust/vendor/cpython-0.7.2/src/argparse.rs:331:9
    |
    = note: in this macro invocation (#16)
    |
    = note: in this macro invocation (#17)
    |
   ::: fbcode/eden/scm/saplingnative/bindings/modules/pymetalog/src/lib.rs:36:1
    |
36  | /   py_class!(pub class metalog |py| {
37  | |       data log: Arc<RwLock<MetaLog>>;
38  | |       data fspath: String;
...   |
226 | |       }
227 | |   });
    | |____- in this macro invocation (#1)
    = note: `#[warn(unused_variables)]` on by default

```

Reviewed By: quark-zju

Differential Revision: D65218833

fbshipit-source-id: fa69c1a24a32b7eff857070528f6337ec0b3711c
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