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 to bootstrap.sh after move into submodule #1

Merged
merged 3 commits into from
Dec 12, 2022

Conversation

dbanks12
Copy link
Contributor

@dbanks12 dbanks12 commented Dec 9, 2022

Fixes to bootstrap and .gitignore

Had an issue with the .gitignore, and needed to fix the pre-commit section of bootstrap.sh.

.gitignore

For some reason the cpp/build* directories were not properly being ignored by the line ./build* in cpp/.gitignore. So, I changed it to /build*/ and now it seems to work properly. I am not sure why this broke in the first place though...

pre-commit-generation broke in ./bootstrap.sh

The pre-commit hook lives in a different place when the barretenberg is a submodule, so this logic basically uses git rev-parse --git-path hooks to find it, then uses git rev-parse --show-toplevel inside of the generated pre-commit file to ensure pre-commit can find its way back to the submodule and run ./format.sh.

Concern

We will also likely want this pre-commit content for the parent repo itself as there will be other cpp code in the circuits/ subdirectory. Do we just want to setup a static pre-commit configuration? Is there a benefit to dynamically generating it via inside cpp/bootstrap.sh?

@dbanks12 dbanks12 requested a review from charlielye December 9, 2022 18:17
@dbanks12 dbanks12 self-assigned this Dec 9, 2022
@charlielye charlielye merged commit ca5a3c7 into defi-bridge-project Dec 12, 2022
@charlielye charlielye deleted the db/submodule_fixes branch December 16, 2022 11:25
charlielye added a commit that referenced this pull request Dec 16, 2022
* Fix to bootstrap.sh after move into submodule (#1)

* Updated bootstrap now that bberg may be a submodule, and now that cpp subdir exists

* fix gitignore, wasnt properly ignoring cpp/build*

* fix the barretenberg cpp formatting pre-commit hook now that barretenberg iss its own repo and may be a submodule

* Contrived change.

* Revert "Contrived change."

This reverts commit 68aff9c.

* Revert "Revert "Contrived change.""

This reverts commit 3eee2fd.

* Introduce cpp to paths.

* More cpp path.

* Cl/mum merge (#4)

* Initial merge. Still has conflicts.

* Tidied up bigfield and biggroup

* Builds.

* Order constants.

* Fix.

* Make more tests pass.

* Update circuit change detection stuff to be local to tests.

* Include header.

* Cleanup and settings.

* Path updates.

* Path updates.

Co-authored-by: Rumata888 <isennovskiy@gmail.com>

Co-authored-by: David Banks <47112877+dbanks12@users.noreply.github.com>
Co-authored-by: Rumata888 <isennovskiy@gmail.com>
dbanks12 added a commit that referenced this pull request Jan 24, 2023
* Updated bootstrap now that bberg may be a submodule, and now that cpp subdir exists

* fix gitignore, wasnt properly ignoring cpp/build*

* fix the barretenberg cpp formatting pre-commit hook now that barretenberg iss its own repo and may be a submodule
dbanks12 added a commit that referenced this pull request Jan 24, 2023
* Updated bootstrap now that bberg may be a submodule, and now that cpp subdir exists

* fix gitignore, wasnt properly ignoring cpp/build*

* fix the barretenberg cpp formatting pre-commit hook now that barretenberg iss its own repo and may be a submodule
dbanks12 added a commit that referenced this pull request Jan 26, 2023
* Fix to bootstrap.sh after move into submodule (#1)

* Updated bootstrap now that bberg may be a submodule, and now that cpp subdir exists

* fix gitignore, wasnt properly ignoring cpp/build*

* fix the barretenberg cpp formatting pre-commit hook now that barretenberg iss its own repo and may be a submodule

* Contrived change.

* Revert "Contrived change."

This reverts commit 68aff9c.

* Revert "Revert "Contrived change.""

This reverts commit 3eee2fd.

* Introduce cpp to paths.

* More cpp path.

* Cl/mum merge (#4)

* Initial merge. Still has conflicts.

* Tidied up bigfield and biggroup

* Builds.

* Order constants.

* Fix.

* Make more tests pass.

* Update circuit change detection stuff to be local to tests.

* Include header.

* Cleanup and settings.

* Path updates.

* Path updates.

Co-authored-by: Rumata888 <isennovskiy@gmail.com>

Co-authored-by: David Banks <47112877+dbanks12@users.noreply.github.com>
Co-authored-by: Rumata888 <isennovskiy@gmail.com>
dbanks12 added a commit that referenced this pull request Jan 27, 2023
* Fix to bootstrap.sh after move into submodule (#1)

* Updated bootstrap now that bberg may be a submodule, and now that cpp subdir exists

* fix gitignore, wasnt properly ignoring cpp/build*

* fix the barretenberg cpp formatting pre-commit hook now that barretenberg iss its own repo and may be a submodule

* Contrived change.

* Revert "Contrived change."

This reverts commit 68aff9c.

* Revert "Revert "Contrived change.""

This reverts commit 3eee2fd.

* Introduce cpp to paths.

* More cpp path.

* Cl/mum merge (#4)

* Initial merge. Still has conflicts.

* Tidied up bigfield and biggroup

* Builds.

* Order constants.

* Fix.

* Make more tests pass.

* Update circuit change detection stuff to be local to tests.

* Include header.

* Cleanup and settings.

* Path updates.

* Path updates.

Co-authored-by: Rumata888 <isennovskiy@gmail.com>

Co-authored-by: David Banks <47112877+dbanks12@users.noreply.github.com>
Co-authored-by: Rumata888 <isennovskiy@gmail.com>
ludamad pushed a commit to AztecProtocol/aztec-packages that referenced this pull request Jul 22, 2023
* Fix to bootstrap.sh after move into submodule (AztecProtocol/barretenberg#1)

* Updated bootstrap now that bberg may be a submodule, and now that cpp subdir exists

* fix gitignore, wasnt properly ignoring cpp/build*

* fix the barretenberg cpp formatting pre-commit hook now that barretenberg iss its own repo and may be a submodule

* Contrived change.

* Revert "Contrived change."

This reverts commit f01a24e76362ed98875b7ad027ccce32802f3584.

* Revert "Revert "Contrived change.""

This reverts commit e500666fdcb9fb7d684a5fb717268fd0c6c88aea.

* Introduce cpp to paths.

* More cpp path.

* Cl/mum merge (AztecProtocol/barretenberg#4)

* Initial merge. Still has conflicts.

* Tidied up bigfield and biggroup

* Builds.

* Order constants.

* Fix.

* Make more tests pass.

* Update circuit change detection stuff to be local to tests.

* Include header.

* Cleanup and settings.

* Path updates.

* Path updates.

Co-authored-by: Rumata888 <isennovskiy@gmail.com>

Co-authored-by: David Banks <47112877+dbanks12@users.noreply.github.com>
Co-authored-by: Rumata888 <isennovskiy@gmail.com>
ludamad pushed a commit to AztecProtocol/aztec-packages that referenced this pull request Jul 24, 2023
* Fix to bootstrap.sh after move into submodule (AztecProtocol/barretenberg#1)

* Updated bootstrap now that bberg may be a submodule, and now that cpp subdir exists

* fix gitignore, wasnt properly ignoring cpp/build*

* fix the barretenberg cpp formatting pre-commit hook now that barretenberg iss its own repo and may be a submodule

* Contrived change.

* Revert "Contrived change."

This reverts commit a474f39b35fd024cc8be65dd8668124913da909e.

* Revert "Revert "Contrived change.""

This reverts commit 7a2ea1bcf35f1fa8b6d42bed7cf36d112ccafd45.

* Introduce cpp to paths.

* More cpp path.

* Cl/mum merge (AztecProtocol/barretenberg#4)

* Initial merge. Still has conflicts.

* Tidied up bigfield and biggroup

* Builds.

* Order constants.

* Fix.

* Make more tests pass.

* Update circuit change detection stuff to be local to tests.

* Include header.

* Cleanup and settings.

* Path updates.

* Path updates.

Co-authored-by: Rumata888 <isennovskiy@gmail.com>

Co-authored-by: David Banks <47112877+dbanks12@users.noreply.github.com>
Co-authored-by: Rumata888 <isennovskiy@gmail.com>
@ludamad ludamad mentioned this pull request Mar 26, 2024
AztecBot pushed a commit that referenced this pull request Jun 7, 2024
Overview:
If you are in a scenario where you have a failing call to check_circuit
and wish to get more information out of it than just the gate index, you
can use this feature to get a stack trace, see example below.

Usage instructions:
- On ubuntu (or our mainframe accounts) use `sudo apt-get install
libdw-dev` to support trace printing
- Use `cmake --preset clang16-dbg-fast-circuit-check-traces` and `cmake
--build --preset clang16-dbg-fast-circuit-check-traces` to enable the
backward-cpp dependency through the CHECK_CIRCUIT_STACKTRACES CMake
variable.
- Run any case where you have a failing check_circuit call, you will now
have a stack trace illuminating where this constraint was added in code.

Caveats:
- This works best for code that is not overly generic, i.e. where just
the sequence of function calls carries a lot of information. It is
possible to tag extra data along with the stack trace, this can be done
as a followup, please leave feedback if desired.
- There are certain functions like `assert_equals` that can cause gates
that occur _before_ them to fail. If this would be useful to
automatically report, please leave feedback.

Example:
```
[ RUN      ] standard_circuit_constructor.test_check_circuit_broken
Stack trace (most recent call last):
#4    Source "_deps/gtest-src/googletest/src/gtest.cc", line 2845, in Run
       2842:   if (!Test::HasFatalFailure() && !Test::IsSkipped()) {
       2843:     // This doesn't throw as all user code that can throw are wrapped into
       2844:     // exception handling code.
      >2845:     test->Run();
       2846:   }
       2847: 
       2848:   if (test != nullptr) {
#3    Source "_deps/gtest-src/googletest/src/gtest.cc", line 2696, in Run
       2693:   // GTEST_SKIP().
       2694:   if (!HasFatalFailure() && !IsSkipped()) {
       2695:     impl->os_stack_trace_getter()->UponLeavingGTest();
      >2696:     internal::HandleExceptionsInMethodIfSupported(this, &Test::TestBody,
       2697:                                                   "the test body");
       2698:   }
#2  | Source "_deps/gtest-src/googletest/src/gtest.cc", line 2657, in HandleSehExceptionsInMethodIfSupported<testing::Test, void>
    |  2655: #if GTEST_HAS_EXCEPTIONS
    |  2656:     try {
    | >2657:       return HandleSehExceptionsInMethodIfSupported(object, method, location);
    |  2658:     } catch (const AssertionException&) {  // NOLINT
    |  2659:       // This failure was reported already.
      Source "_deps/gtest-src/googletest/src/gtest.cc", line 2621, in HandleExceptionsInMethodIfSupported<testing::Test, void>
       2618:   }
       2619: #else
       2620:   (void)location;
      >2621:   return (object->*method)();
       2622: #endif  // GTEST_HAS_SEH
       2623: }
#1    Source "/mnt/user-data/adam/aztec-packages/barretenberg/cpp/src/barretenberg/circuit_checker/standard_circuit_builder.test.cpp", line 464, in TestBody
        461:     uint32_t d_idx = circuit_constructor.add_variable(d);
        462:     circuit_constructor.create_add_gate({ a_idx, b_idx, c_idx, fr::one(), fr::one(), fr::neg_one(), fr::zero() });
        463: 
      > 464:     circuit_constructor.create_add_gate({ d_idx, c_idx, a_idx, fr::one(), fr::neg_one(), fr::neg_one(), fr::zero() });
        465: 
        466:     bool result = CircuitChecker::check(circuit_constructor);
        467:     EXPECT_EQ(result, false);
#0    Source "/mnt/user-data/adam/aztec-packages/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/standard_circuit_builder.cpp", line 22, in create_add_gate
         19: {
         20:     this->assert_valid_variables({ in.a, in.b, in.c });
         21: 
      >  22:     blocks.arithmetic.populate_wires(in.a, in.b, in.c);
         23:     blocks.arithmetic.q_m().emplace_back(FF::zero());
         24:     blocks.arithmetic.q_1().emplace_back(in.a_scaling);
         25:     blocks.arithmetic.q_2().emplace_back(in.b_scaling);
gate number4
```
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