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

Multiple Issues when Building from Scratch #1257

Closed
rz137 opened this issue Jun 10, 2020 · 38 comments
Closed

Multiple Issues when Building from Scratch #1257

rz137 opened this issue Jun 10, 2020 · 38 comments

Comments

@rz137
Copy link
Contributor

rz137 commented Jun 10, 2020

I am creating this ticket to track multiple issues I have had building Macaulay2 from source. Having pushed the GitHub actions PR #1256, one finally has a way to demonstrate those issues. Most but not all have to do with the cmake build. All are on macOS. Thanks in advance!

cc @mahrud, @DanGrayson

In no particular order,

  • Some libraries are clearly present on my system — and are even reported as found, but are still locally built. Current examples are googletest, mpfr, ntl. Previously, the same thing was happening even for mpir.
  • The autotools build uses GMP by default, while the cmake one uses mpir. This introduces an unpleasant inconsistency.
  • Flint's required version 2.5.3 looks off. Its website lists versions 2.5.2 and 2.6.0. If that's a hack to use a patched variant, another approach is needed.
  • The target build-programs has never succeeded for me. It doesn't succeed on github actions either.
  • (Omitting build-programs or letting it fail should be OK for a very capable M2 binary, so that's what I did.)
  • I am unhappy with having to build M2-core after M2-binary, but that's a personal preference. The way I look at it, M2-core should refer to a core part and M2-binary to the end product.
  • I hate the religious adoption of Emacs, and accordingly hate that M2-emacs is the default target. In my experience, one should make a software system as modular as possible or useful, (but not more), and systems where logically independent components are too entangled together are never well designed. My view is that Emacs is a front-end and should be kept as one.
  • After building M2-core (and M2-binary), my interactive shell is missing history. For example, ⬆️ (up-key) doesn't bring back anything. I am guessing readline is not linked properly. In fact, otool -L M2-binary does not list readline.
  • The default gmake step in the autotools builds a whole lot. Is there a target that only builds the binary without performing any package installations and tests?

Thanks! I am pasting my cmake configuration output for reference.

radoslav@home ~/Projects/M2/M2/BUILD/gcc9rel (up-master)$ CXX=g++-9 CC=gcc-9 cmake -S../.. -B. -DCMAKE_BUILD_TYPE=Release
-- The C compiler identification is GNU 9.3.0
-- The CXX compiler identification is GNU 9.3.0
-- Checking whether C compiler has -isysroot
-- Checking whether C compiler has -isysroot - yes
-- Checking whether C compiler supports OSX deployment target flag
-- Checking whether C compiler supports OSX deployment target flag - yes
-- Check for working C compiler: /usr/local/bin/gcc-9
-- Check for working C compiler: /usr/local/bin/gcc-9 - works
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Detecting C compile features
-- Detecting C compile features - done
-- Checking whether CXX compiler has -isysroot
-- Checking whether CXX compiler has -isysroot - yes
-- Checking whether CXX compiler supports OSX deployment target flag
-- Checking whether CXX compiler supports OSX deployment target flag - yes
-- Check for working CXX compiler: /usr/local/bin/g++-9
-- Check for working CXX compiler: /usr/local/bin/g++-9 - works
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Detecting CXX compile features
-- Detecting CXX compile features - done
## Configure Macaulay2
     M2 version        = 1.15.1.0
     Git commit        = 22480f07d
     Install prefix    = /usr/local

     CMAKE_BUILD_TYPE  = Release
     BUILD_NATIVE      = ON
     BUILD_SHARED_LIBS = OFF
     BUILD_TESTING     = ON
     BUILD_DOCS        = OFF

     COVERAGE          = OFF
     PROFILING         = OFF

     DEVELOPMENT       = OFF
     EXPERIMENT        = OFF

## Host OS information
     ISSUE             = MacOS-10.15.5
     NODENAME          = MacBook-Pro.fios-router.home
     OS REL            = Darwin 19.5.0
     ARCH              = x86_64

## Staging area prefixes
     common            = /Users/radoslav/Projects/M2/M2/BUILD/gcc9rel/usr-dist/common
     exec              = /Users/radoslav/Projects/M2/M2/BUILD/gcc9rel/usr-dist/x86_64-Darwin-MacOS-10.15.5

## Compiler information
     C                 = GNU 9.3.0 (/usr/local/bin/gcc-9)
     C++               = GNU 9.3.0 (/usr/local/bin/g++-9)

--  Checking for existing libraries and programs
-- Could NOT find Doxygen (missing: DOXYGEN_EXECUTABLE) 
-- Could NOT find Sphinx (missing: SPHINX_EXECUTABLE) 
-- Looking for sgemm_
-- Looking for sgemm_ - not found
-- Found Threads: TRUE  
-- Looking for dgemm_
-- Looking for dgemm_ - found
-- Found BLAS: /Library/Developer/CommandLineTools/SDKs/MacOSX10.15.sdk/System/Library/Frameworks/Accelerate.framework  
-- Found MPIR: /usr/local/include (Required is at least version "3.0.0") 
-- Found BDWGC: /usr/local/include (Required is at least version "7.6.4") 
-- Found MPFR: /usr/local/include (Required is at least version "4.0.1") 
-- Found NTL: /usr/local/include (Required is at least version "10.5.0") 
-- Flint version 2.5.2 found in /usr/local/include, but at least version 2.5.3 is required
-- Could NOT find Flint (missing: FLINT_VERSION_OK) (Required is at least version "2.5.3")
-- Could NOT find Factory (missing: FACTORY_INCLUDE_DIR FACTORY_LIBRARIES FACTORY_VERSION_OK) (Required is at least version "4.1.0")
-- Could NOT find Frobby (missing: FROBBY_INCLUDE_DIR FROBBY_LIBRARIES) (Required is at least version "0.9.0")
-- Could NOT find CDDLIB (missing: CDDLIB_INCLUDE_DIR CDDLIB_LIBRARIES) 
-- Could NOT find MPSolve (missing: MPSOLVE_LIBRARIES MPSOLVE_INCLUDE_DIR) (Required is at least version "3.1.8")
-- Found GTest: /usr/local/lib/libgtest.dylib (Required is at least version "1.10") 
-- Could NOT find Memtailor (missing: MEMTAILOR_INCLUDE_DIR MEMTAILOR_LIBRARIES) (Required is at least version "1.0.0")
-- Could NOT find Mathic (missing: MATHIC_INCLUDE_DIR MATHIC_LIBRARIES) (Required is at least version "1.0.0")
-- Could NOT find Mathicgb (missing: MATHICGB_INCLUDE_DIR MATHICGB_LIBRARIES) (Required is at least version "1.0.0")
-- Found GLPK: /usr/local/include (Required is at least version "4.59.0") 
-- Checking for one of the modules 'fflas-ffpack>=2.3.2'
-- Checking for one of the modules 'givaro>=4.0.3'
-- Found LibXml2: /usr/lib/libxml2.dylib (found suitable version "2.9.4", minimum required is "2.9") 
--  Checking library compatibility
--  Checking library compatibility -  Libraries are compatible!
-- Submodule update
## External components
     Need to build:
       Libraries       = cddlib;factory;fflas_ffpack;flint;frobby;givaro;googletest;mathic;mathicgb;memtailor;mpfr;mpsolve;ntl
       Programs        = 4ti2;cohomcalg;csdp;gfan;lrslib;nauty;normaliz;topcom
     Already built:
       Libraries       = N/A
       Programs        = N/A

## Library information
     Linear Algebra    = /Library/Developer/CommandLineTools/SDKs/MacOSX10.15.sdk/System/Library/Frameworks/Accelerate.framework;-lm;-ldl
     MP Arithmetic     = /usr/local/lib/libmpirxx.dylib;/usr/local/lib/libmpir.dylib

--  Checking for existing libraries and programs -  Some components are missing
## Rerun build-libraries and build-programs targets first
-- Configuring done
-- Generating done
-- Build files have been written to: /Users/radoslav/Projects/M2/M2/BUILD/gcc9rel
@mahrud
Copy link
Member

mahrud commented Jun 10, 2020

  • Some libraries are clearly present on my system — and are even reported as found, but are still locally built. Current examples are googletest, mpfr, ntl. Previously, the same thing was happening even for mpir.
  • The autotools build uses GMP by default, while the cmake one uses mpir. This introduces an unpleasant inconsistency.

These two are related. mpfr, ntl, flint, factory, frobby, givaro link with gmp, so when we use mpir as a replacement, they all need to be built and linked with mpir instead. I'm not aware of any non-hacky ways to prevent this. Look in cmake/configure.cmake line 51 for where this happens.

The choice of gmp vs mpir is up to the person building it. I don't think it's any more unpleasant than having two build systems. The install guide tells you how to switch if you would like.

On some systems, using the installed googletest lead to issues. I can look into it again, but it's a low priority for me since googletest is very small and I'm more interested in switching to catch2 instead.

  • Flint's required version 2.5.3 looks off. Its website lists versions 2.5.2 and 2.6.0. If that's a hack to use a patched variant, another approach is needed.

Until a couple of months ago they were working on releasing 2.5.3, then they switched to 2.6.0. CMake build already switched to 2.6.0, see #1242 for autotools build.

  • The target build-programs has never succeeded for me. It doesn't succeed on github actions either.

What fails exactly? Have you tried the docker builds in BUILD/docker/ubuntu?

  • (Omitting build-programs or letting it fail should be OK for a very capable M2 binary, so that's what I did.)

I'd like this to be the case, but unfortunately the system can't handle not having gfan at the moment. See #1201.

  • I am unhappy with having to build M2-core after M2-binary, but that's a personal preference. The way I look at it, M2-core should refer to a core part and M2-binary to the end product.

Binary means the executable. Core means the barebone Macaulay2 top level (i.e. without packages). You need M2-binary to even parse the barebones top level scripts. In Julia they call this "Base" instead of "Core", probably the same for any other interpreted language.

  • I hate the religious adoption of Emacs, and accordingly hate that M2-emacs is the default target. In my experience, one should make a software system as modular as possible or useful, (but not more), and systems where logically independent components are too entangled together are never well designed. My view is that Emacs is a front-end and should be kept as one.

I used to agree, but I think it's natural for a project to develop at least one solid editor interface, then the community can add support for their editors of choice (vim, vscode, and atom support already exist). It's just convenience. In #1255 I'm trying to make this more democratic so you can write grammar files for other editors easily.

  • After building M2-core (and M2-binary), my interactive shell is missing history. For example, arrow_up (up-key) doesn't bring back anything. I am guessing readline is not linked properly. In fact, otool -L M2-binary does not list readline.

Hmm, maybe make a new issue on this. I've confirmed that this works at least on Catalina.

@rz137
Copy link
Contributor Author

rz137 commented Jun 10, 2020

thanks for the response and clarification, @mahrud!

  • Having two build systems is fine as long as they achieve the same, all be it in wildly different ways. That's why having different default options (GMP vs MPIR) on two bothered me a bit.
  • I am aware of some of the dependencies not using the M2-chosen MPZ library but (1) I am not convinced one has to rebuild as long as memory management and linking is done carefully; (2) even if rebuilds are necessary, it may be useful to emit some message about it — e.g. Libraries ... are found but are to be rebuild because of ....
  • I don't buy core meaning "Macaulay2 top level". I do like base though! (And technically, "binary" isn't any more synonymous to "executable" than to "end product".)
  • I still don't buy the Emacs part :) Having people using Emacs by default is fine, as long there's a sensible logical separation between between the two. Don't even start me on seeing the build instructions at the end of C++ files. ;)
  • If you have the time, you can inspect where build-programs failed from the output of the Github Actions trigger. But if I am not mistaken it's slightly earlier than on my Catalina. I usually get a failure around topcom. I'll double check.

@mahrud
Copy link
Member

mahrud commented Jun 10, 2020

  • Having two build systems is fine as long as they achieve the same, all be it in wildly different ways. That's why having different default options (GMP vs MPIR) on two bothered me a bit.

If the build systems did the exact same thing, what would be the point of having two?

  • I am aware of some of the dependencies not using the M2-chosen MPZ library but (1) I am not convinced one has to rebuild as long as memory management and linking is done carefully;

Don't take my word for it. Feel free to send a PR :)

(2) even if rebuilds are necessary, it may be useful to emit some message about it — e.g. Libraries ... are found but are to be rebuild because of ....

The CMake build does inform the user of this :)

--  Checking library compatibility -  Libraries are compatible!

And if they're incompatible (which happens when linking two libraries that are linked with different MP libraries), you get to see which libraries will be built one or two lines after that.

  • I don't buy core meaning "Macaulay2 top level". I do like base though! (And technically, "binary" isn't any more synonymous to "executable" than to "end product".)

I didn't invent the names "core" or "binary". The content of Macaulay2/m2 makes up the "Core" package and "M2-binary" is the executable that goes in the bin directory. If it helps, you can just forget about the M2-binary target and always use M2-core.

  • If you have the time, you can inspect where build-programs failed from the output of the Github Actions trigger. But if I am not mistaken it's slightly earlier than on my Catalina. I usually get a failure around topcom. I'll double check.

Looks like 4ti2 and normaliz failed on Catalina (search for "failed" in the logs), but on Ubuntu M2 built okay and got a segmentation fault .. I'm guessing it's because the workflow doesn't install the correct libraries (e.g. openblas is missing). See INSTALL-CMake.md :)

I would suggest trying to get ccache and Github's dependency catching to work before even fixing the build errors. 8 x 2 hours for every build is unconscionable (think of the carbon emissions!)

@rz137
Copy link
Contributor Author

rz137 commented Jun 10, 2020

OK, I really don't want to spend too much time in coffee-table conversations, but

  • You can see it found -- Found BLAS: /Applications/Xcode_11.4.1.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.15.sdk/System/Library/Frameworks/Accelerate.framework
  • The point of two systems achieving the same thing is who you integrate them in your workflow and where you apply them. For example, one might like cmake because it can be used with CLion, another might like autotools because they've used it for 30 years. What I am saying is that if you just build the two, the end result — that is the linked libraries, configure options, etc &mdash should be the same.
  • -- Checking library compatibility - Libraries are compatible! is rather meaningless, right? It did get printed yet mpfr;ntl are still being rebuilt.

I'll take care of the carbon footprint :) of the actions scripts and resubmit.

@DanGrayson
Copy link
Member

  • The autotools build uses GMP by default, while the cmake one uses mpir. This introduces an unpleasant inconsistency.

That should be fixed. And the same versions of everything should be used.

  • Flint's required version 2.5.3 looks off. Its website lists versions 2.5.2 and 2.6.0. If that's a hack to use a patched variant, another approach is needed.

The autotools build uses a recent flint snapshot, and will soon be updated to 2.6.0, as mentioned in #1242

  • The default gmake step in the autotools builds a whole lot. Is there a target that only builds the binary without performing any package installations and tests?

No. Would you find one useful?

@mahrud
Copy link
Member

mahrud commented Jun 11, 2020

  • You can see it found -- Found BLAS: /Applications/Xcode_11.4.1.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.15.sdk/System/Library/Frameworks/Accelerate.framework

Sorry for the confusion, I meant OpenBLAS is missing on Ubuntu. On mac we always use Accelerate (though it may not be the best idea).

What I am saying is that if you just build the two, the end result — that is the linked libraries, configure options, etc &mdash should be the same.

That wasn't my goal from the beginning. The goal was to produce a better build system. Plenty of build options for both M2 and libraries and programs are improved. Build processes for some libraries are very different. As I mentioned, the prerequisites are different as well.

For instance, the reason Normaliz is failing on Catalina currently is that I want to get Normaliz properly linked with OpenMP. If that means the build fails for now, that's okay, I'll fix it. But the end results will not be the same either way.

  • -- Checking library compatibility - Libraries are compatible! is rather meaningless, right? It did get printed yet mpfr;ntl are still being rebuilt.

That message is a guarantee that you won't get a linking conflict and the headache goes with it. As I said, mpfr and ntl are rebuilt because I forced them to be rebuilt with mpir. If you disable that in cmake/configure.cmake, then you'll get a linking conflict and a different message to go with it.

@rz137
Copy link
Contributor Author

rz137 commented Jun 11, 2020

My advice is to make sure the build system works first and replicates what the autotools one does. (Because a great thing about the latter is that it simply always works.) If the cmake one is better, people will adopt it, and then 3 months down the line you can easily modify to "produce a better build system" with "Plenty of build options for both M2 and libraries and programs [are] improved."

This issue is about the 3 points below. If you (@mahrud @DanGrayson @mikestillman) disagree, just close the ticket. It's easy as that.

  1. Have the cmake build system work, demonstrably. (The Github Actions jobs should succeed.)
  2. Have it by default link with the same libraries and versions as the autotools one.
  3. If you emit a message "Found: Library X" and then build it locally, clearly report why.

Ignore my other comments — they are minor and I wouldn't have mentioned them if the build succeeded.

@mahrud
Copy link
Member

mahrud commented Jun 11, 2020

  1. Have the cmake build system work, demonstrably. (The Github Actions jobs should succeed.)

As far as I can tell from the last build, currently all ubuntu builds succeed and only the mac ones fail due to an error in brew. Do you know what the issue is?

When you fix it, be sure to pull from the feature/cmake branch as well, which fixes the Normaliz issue.

  1. Have it by default link with the same libraries and versions as the autotools one.

That's tracked in a separate issue (#1243).

  1. If you emit a message "Found: Library X" and then build it locally, clearly report why.

This already exists. For instance:

--  Checking for existing libraries and programs
...
-- MPFR version 3.1.6 found in /usr/include, but at least version 4.0.1 is required
...

Or in the event that there's a conflict you'll see a message like this:

--  Checking library compatibility - Detected library incompatibilities; rerun the build-libraries target

And later you'll see:

## External components
     Need to build:
       Libraries       = ...;mpfr;...

Printing exactly which libraries have conflicts is not easy, but in my experience libraries linking with mpir are the number one culprit, which is why a few libraries are always built by us. Again, feel free to make a PR if you need something more specific, but I'd rather build a few extra libraries that consistently link correctly rather than wish for the best.

@rz137
Copy link
Contributor Author

rz137 commented Jun 11, 2020

You're looking at the latest build off my master, not the upstream master. the feature/actions is based off the upstream master, so that's why I referenced the specific commit before.

I am going to wait for your patch to make it to M2's master and then repeat the exercise.

@mahrud
Copy link
Member

mahrud commented Jun 12, 2020

Looks like installPackage("CohomCalg") is failing on every CMake build regardless of machine type and compiler. However, I can't replicate it anywhere, even on Catalina. @radoslavraynov if you can reproduce the bug on your machine let me know, or if you can add some artifacts to the workflow (e.g. an archive of all *.errors would be useful right now).

Also, I think it would be good to set the IgnoreExampleErrors=true environment variable so all packages are tested, instead of stopping after the first error.

@DanGrayson
Copy link
Member

The problem with setting IgnoreExampleErrors=true is that it will let the build "succeed".

@rz137
Copy link
Contributor Author

rz137 commented Jun 12, 2020

@mahrud, I get the exact same (CohomCalg) error when building from scratch locally.

@mikestillman
Copy link
Member

@radoslavraynov Can you send me (or place here) the contents of the .errors file that shows the failure?

@rz137
Copy link
Contributor Author

rz137 commented Jun 12, 2020

https://gist.github.com/radoslavraynov/dd868df3f54675255b0d31574d6daf06

@mahrud
Copy link
Member

mahrud commented Jun 12, 2020

Hmm, @radoslavraynov when you say "building from scratch locally", what do you mean exactly? I still can't replicate it on either Catalina or Ubuntu.

It looks like the virtual environment is missing some directories, but I can't tell what exactly:

i20 : elapsedTime hvecs = cohomCalg(X, D2)
shell-init: error retrieving current directory: getcwd: cannot access parent directories: No such file or directory

and

ERROR: Could not open temporary file for computation.
stdio:22:21:(3): error: close: process exited with code 256

But then it sometimes exists, in other examples .. I'm rather confused.

@rz137
Copy link
Contributor Author

rz137 commented Jun 12, 2020

Hmm, @radoslavraynov when you say "building from scratch locally", what do you mean exactly?

What do you think I mean? :)

@mahrud
Copy link
Member

mahrud commented Jun 12, 2020

What do you think I mean? :)

When you find an error, it's useful to provide more information so others can reproduce it; e.g. what brew packages you had installed, what compiler you used, or anything else that makes your build special.

@mikestillman
Copy link
Member

Are you building in a virtual machine? docker image? os? gcc version?

I'll look at this: perhaps somehow the package is deleting the directory by accident? Seems unlikely, but so is that error message!

@rz137
Copy link
Contributor Author

rz137 commented Jun 12, 2020

@mikestillman I don't think anything makes my setup special. It's — again — a several days-old installation of Catalina with homebrew packages of the tools and libraries analogous to the Actions. Same steps as on the Actions job. All the problems I've experienced locally have manifested on the Actions job.

@mahrud, to be fair, you haven't been able to reproduce any of the other build failures so far, so why should this be any different? In fact, I set up the Actions job precisely because you countered me with "It works for me."

@mahrud
Copy link
Member

mahrud commented Jun 12, 2020

@radoslavraynov I reproduced and fixed both the 4ti2 and normaliz issues, thanks to your Actions workflow, so I don't think that's fair at all.

@rz137
Copy link
Contributor Author

rz137 commented Jun 12, 2020

@mahrud, It's just an expression :D I don't think "fair" or "unfair" are at all relevant in the context of building Macaulay2.

@rz137
Copy link
Contributor Author

rz137 commented Jun 12, 2020

could it be just a permissions issue?

@mahrud
Copy link
Member

mahrud commented Jun 13, 2020

@mikestillman I got a similar error from Normaliz locally:

-- -*- M2-comint -*- hash: 1407851922

i1 : nmzFilename="polytope";

i2 : setNmzOption("allf",true);

i3 : R=ZZ/37[x,y,z];

i4 : ehrhartRing {x^0,x^2,y^3,z^5};
shell-init: error retrieving current directory: getcwd: cannot access parent directories: No such file or directory
stdio:4:1:(3): error: opening output file "polytope.in" failed: No such file or directory

I'm not sure why it happened, and on a second run it didn't fail again, but it makes me wonder if this is an issue with two packages creating the same temporary directory, then one of them deleting it and breaking the other package. In other words, a bug with installing packages in parallel.

@DanGrayson
Copy link
Member

The temporary filenames we create contain the pid, so two M2 processes aren't using the same one:

i1 : temporaryFileName ()

o1 = /var/folders/46/9b86vqxj4hjcngvy7kd7sb140000gn/T/M2-3152-0/0

i2 : processID ()

o2 = 3152

@mahrud
Copy link
Member

mahrud commented Jun 13, 2020

Well, something is definitely going wrong, because I was able to reproduce the error with:

ninja uninstall-Normaliz uninstall-CohomCalg && ninja install-Normaliz install-CohomCalg

Aha! Found it: afda220 cc: @d-torrance

@mahrud
Copy link
Member

mahrud commented Jun 13, 2020

Reverting that commit fixed the issue locally. I'll make a PR to see if it makes github happy 👯‍♂️

mahrud added a commit to mahrud/M2 that referenced this issue Jun 13, 2020
@d-torrance
Copy link
Member

Aha! Found it: afda220 cc: @d-torrance

Yikes! That commit was merely to remove some build paths that were appearing in the documentation. As I was working on it, I wondered if there was a reason we were doing everything in a temporary directory, and it appears that there is! I definitely didn't want to break anything, so revert away!

I'm currently working on replacing the canned examples that were included in that same PR, and when that's complete, it should be able to solve the same problem that afda220 was trying to solve.

@mahrud
Copy link
Member

mahrud commented Jun 13, 2020

@d-torrance no worries, and thanks for taking care of finding a different way!

@mahrud
Copy link
Member

mahrud commented Jun 13, 2020

For the record, I prefer using the build directory (or the ~/.Macaulay2 directory if we're running installPackage form an installed binary) instead of /tmp to make the examples as well.

@mahrud
Copy link
Member

mahrud commented Jun 13, 2020

Looks like all CMake builds are succeeding now:
image

I don't quite understand the issues with autotools on mac.

There's a sporadic segmentation fault with gcc 7.5.0 on Ubuntu, which I haven't been able to reproduce. The crash occurs right after M2-binary is compiled and is building tvalues.m2, so not even anything math related. Not sure how to track this down, but it's low priority for me.

@rz137
Copy link
Contributor Author

rz137 commented Jun 14, 2020

great, thanks @mahrud!

@mahrud
Copy link
Member

mahrud commented Jun 17, 2020

What is left in this issue?

@rz137
Copy link
Contributor Author

rz137 commented Jun 17, 2020

@mahrud, here's another issue I ran into. The cmake build by default tries to update the submodules. This is generally a bad idea. Rerunning a build should not change the state of your checkout-ed code. In particular:

-- Submodule update
error: could not lock config file /Users/radoslav/Projects/M2/.git/modules/M2/submodules/bdwgc/config: File exists
fatal: could not set 'core.worktree' to '../../../../../M2/submodules/bdwgc'
CMake Error at cmake/build-libraries.cmake:121 (message):
  git submodule update --init failed with 1, please checkout submodules
Call Stack (most recent call first):
  CMakeLists.txt:76 (include)

Running git submodule update --init --recursive didn't fix it.

@mahrud
Copy link
Member

mahrud commented Jun 17, 2020

This is generally a bad idea. Rerunning a build should not change the state of your checkout-ed code.

Why is it bad? It's only making sure that the submodules are checked out at the specified commit hash. If you rerun cmake it won't change it again.

Running git submodule update --init --recursive didn't fix it.

This is, in fact, exactly what CMake does for you:

if(GIT_FOUND AND EXISTS "${CMAKE_SOURCE_DIR}/../.git")
## Update submodules as needed
if(GIT_SUBMODULE)
message(STATUS "Submodule update")
execute_process(COMMAND ${GIT_EXECUTABLE} submodule update --init --recursive
WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}
RESULT_VARIABLE GIT_SUBMOD_RESULT)
if(NOT GIT_SUBMOD_RESULT EQUAL "0")
message(FATAL_ERROR "git submodule update --init failed with ${GIT_SUBMOD_RESULT}, please checkout submodules")
endif()
endif()
endif()

I don't know how this error occurred:

error: could not lock config file /Users/radoslav/Projects/M2/.git/modules/M2/submodules/bdwgc/config: File exists
fatal: could not set 'core.worktree' to '../../../../../M2/submodules/bdwgc'

Could you elaborate a bit? Did you have the repository open in gitk or git-gui or something like that?

I prefer CMake taking care of this to be honest, so unless it's causing widespread issues or making development harder, you can try passing in -DGIT_SUBMODULE=OFF to CMake to disable that step.

@rz137
Copy link
Contributor Author

rz137 commented Jun 17, 2020

It's up to you, obviously, and but I personally prefer once I check out some code for it to remain static git-wise.

I don't know how exactly the error occurred — and tbh didn't spend much time on it. I tried git submodule update --init --recursive, it didn't work, so I just commented out the problematic block. The issue must have occurred roughly like so:

  1. working on branch A; git status is clean
  2. switch to branch pr/actions tracking upstream/development; git pull
  3. make changes; git push
  4. git checkout A; notice a change in the submodules hashes and naively ignore it :D

Now any and all building fails with the above error, with or without that submodule hash change. But that's fine — just wanted to flag it here in case it happens again.

I won't be online for the next few days, so don't wait for my input on closing this, etc.

@mahrud mahrud closed this as completed Jun 17, 2020
@mahrud
Copy link
Member

mahrud commented Jun 17, 2020 via email

@DanGrayson
Copy link
Member

DanGrayson commented Jun 17, 2020

It's up to you, obviously, and but I personally prefer once I check out some code for it to remain static git-wise.

It should be possible to make it static by setting an option. For the autotools build system, I skip the submodule update if --enable-development is given to the configure script.

We handle downloads the other way: we download tarfiles only with the user's permission.

Maybe opting out is not enough and we should reconcile these two approaches: experienced computer users don't like it when build system download files from the internet without permission!

@DanGrayson DanGrayson reopened this Jun 17, 2020
@mahrud
Copy link
Member

mahrud commented Jun 17, 2020

@DanGrayson if you read my comment above you would see:

I prefer CMake taking care of this to be honest, so unless it's causing widespread issues or making development harder, you can try passing in -DGIT_SUBMODULE=OFF to CMake to disable that step.

Or if you take a look under build flags in INSTALL-CMake.md you would find:

  • GIT_SUBMODULE:BOOL=ON: update submodules during build

Maybe opting out is not enough and we should reconcile these two approaches: experienced computer users don't like it when build system download files from the internet without permission!

As I've said before, there are differences between the build systems. I've designed the CMake system with many factors in mind, and unless there is a bug that is causing an issue for you I'm against changing it.

If there's any issues left with building from scratch, please mention them, otherwise please close this issue.

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

No branches or pull requests

5 participants