ginkgo-project / ginkgo

Numerical linear algebra software package
https://ginkgo-project.github.io/
BSD 3-Clause "New" or "Revised" License
414 stars 88 forks source link

Cannot commit to my own fork due to #1728

Closed jrobcary closed 2 weeks ago

jrobcary commented 2 weeks ago

At

vcloud/.../ginkgo$ git remote -v
origin  https://github.com/Tech-XCorp/ginkgo.git (fetch)
origin  https://github.com/Tech-XCorp/ginkgo.git (push)
upstream    https://github.com/ginkgo-project/ginkgo.git (fetch)
upstream    https://github.com/ginkgo-project/ginkgo.git (push)
cary@vcloud/.../ginkgo$ git branch -v
* clang_cl          9e53640d4 alias in template will lead an issue
  develop           e518aee3a Merge Fix Typo Clonable
  upstream-clang_cl 9e53640d4 alias in template will lead an issue
  upstream-develop  dc8cfeb0a Merge Fix complex solver benchmark

ie, my own fork. When I try to commit I get

vcloud/.../ginkgo$ git commit -m "Cannot redefine a using in a struct if it is defined in the scopt of that struct."
Please only commit to Ginkgo when GINKGO_DEVEL_TOOLS is enabled in CMake.
This can be set in your initial invocation of CMake by using
 -DGINKGO_DEVEL_TOOLS=ON or by editing the CMakeCache.txt file.

I did configure (in another directory) with -DGINKGO_DEVEL_TOOLS=ON.

How can I turn off this check for my own fork?

jrobcary commented 2 weeks ago

--no-verify

yhmtsai commented 1 week ago

Hi @jrobcary , You can also turn off the check by deleting .git/hook/pre-commit and .git/hook/pre-commit.legacy. I think the issue is from our own pre-commit hook to pre-commit package. The package will turns the old one to pre-commit.legacy (still active) to keep the original pre-commit step, which leads this issue. From your commit message, is "using T = T;" in the nested structure disallowed in your side?

jrobcary commented 1 week ago

The diff that allows me to build is

diff --git a/core/test/matrix/csr_builder.cpp b/core/test/matrix/csr_builder.cpp
index 69e0c9632..1b9fd906d 100644
--- a/core/test/matrix/csr_builder.cpp
+++ b/core/test/matrix/csr_builder.cpp
@@ -59,7 +59,9 @@ TYPED_TEST(CsrBuilder, UpdatesSrowOnDestruction)
     using value_type = typename TestFixture::value_type;
     using index_type = typename TestFixture::index_type;
     struct mock_strategy : public Mtx::strategy_type {
+#if !defined(__clang__) && !defined(__GNUC__)
         using Mtx = Mtx;
+#endif
         virtual void process(const gko::array<index_type>&,
                              gko::array<index_type>*) override
         {

I chose the ifdef to minimize impact, as I assumed this was working with all compilers besides the one I am using.

This is relative to the clang_cl branch of the main (upstream) repo.

yhmtsai commented 1 week ago

I see. I have added in that branch

#if defined(_MSC_VER) && defined(__clang__)
...

which should only trigger this workaround in clang_cl in windows