Repair "sage -b" broken by #29411

[mkoeppe]

(from jhpalmieri, #30123)

Note that sage -b doesn't work any more: it tries to do cd "$SAGE_SRC" && $MAKE, but SAGE_SRC/Makefile.in was removed in #29411. (If you have done an incremental build, the Makefile is still there.)

Depends on #30128

CC: @jhpalmieri @orlitzky @kliem

Component: build

Author: Matthias Koeppe, John Palmieri, Michael Orlitzky

Branch: 2d95def

Reviewer: John Palmieri, Matthias Koeppe

Issue created by migration from https://trac.sagemath.org/ticket/30153

[mkoeppe]

Branch: u/mkoeppe/repair__sage__b__broken_by__29411

[mkoeppe]

New commits:

95c0b68	build/make/Makefile.in: Add target SPKG-no-deps for all SPKGs
cb4262f	src/bin/sage: Repair ./sage -b etc. by using make sagelib-clean, make sagelib-no-deps

[mkoeppe]

Commit: cb4262f

[mkoeppe]

Author: Matthias Koeppe

[orlitzky]

comment:3

Can you base this on #30128 and then source sage-env-config before sage-env in that new Makefile target?

[jhpalmieri]

comment:4

• If I touch a .pyx file and do sage -b, nothing happens: make: `sagelib-no-deps' is up to date.
• Same with sage --ba-force: nothing happens, even though this is supposed to force rebuilding everything in the Sage library.
• It looks like the logger is invoked, and it didn't used to be. This might be a good change, not sure, but it is a change.
• git status reports that the empty file build/make/sagelib-no-deps is present and untracked. Does sagelib-no-deps need to be added as a phony target, or something like that?

[mkoeppe]

comment:5

Replying to @orlitzky:

Can you base this on #30128 and then source sage-env-config before sage-env in that new Makefile target?

Sure, will do

[mkoeppe]

comment:6

Replying to @jhpalmieri:

• git status reports that the empty file build/make/sagelib-no-deps is present and untracked. Does sagelib-no-deps need to be added as a phony target, or something like that?

Yes... will fix it

[sagetrac-git]

Changed commit from cb4262f to 03a2dbe

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

2f141c8	Trac #30128: always source sage-env-config before sage-env.
554282a	Trac #30128: enforce sourcing of sage-env-config before src/bin/sage-env.
7a91c05	build/make/Makefile.in: Add target SPKG-no-deps for all SPKGs
4fc909c	src/bin/sage: Repair ./sage -b etc. by using make sagelib-clean, make sagelib-no-deps
03a2dbe	build/make/Makefile.in: Make SPKG-no-deps targets phony; for script packages, create the correct installation stamp file

[sagetrac-git]

Changed commit from 03a2dbe to 62ddd02

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

62ddd02

build/make/Makefile.in [SPKG-no-deps]: Do not forget to source sage-env-config

[mkoeppe]

comment:9

Replying to @jhpalmieri:

• If I touch a .pyx file and do sage -b, nothing happens: make: `sagelib-no-deps' is up to date.
• Same with sage --ba-force: nothing happens, even though this is supposed to force rebuilding everything in the Sage library.

This should be fixed now.

• It looks like the logger is invoked, and it didn't used to be. This might be a good change, not sure, but it is a change.

Yes, it was convenient to go through the Makefile, and I think this change is good as it improves uniformity.

[mkoeppe]

Dependencies: #30128

[jhpalmieri]

comment:11

I think this works. The first time I got a file build/make/sagelib-no-deps, but maybe I forgot to run ./bootstrap/.configure/etc. It's working now.

• Can we add a comment in Makefile.in explaining what $(1)-no-deps is and how it should be used? I'm guessing that it tries to build the package but ignores its dependencies. Does that have any uses aside from the Sage library? I've made an attempt.

• For pip packages, you have added $(1)-clean a second time to the .PHONY list. Should there be a $(1)-uninstall target for these? In that case, change one $(1)-clean to $(1)-uninstall.

Could we change the build rules so that

$$(INST)/$(1)-$(2)

just does $(1)-build-deps and $(1)-no-deps? It would cut out a tiny bit of code duplication. I guess we can't force the dependencies to be built before the package, so maybe not.

[jhpalmieri]

Changed branch from u/mkoeppe/repair__sage__b__broken_by__29411 to u/jhpalmieri/repair__sage__b__broken_by__29411

[jhpalmieri]

comment:13

By the way, we should deprecate sage --ba-force: #30160.

---

New commits:

830843b

trac 30153: add, clarify some comments

[jhpalmieri]

Changed commit from 62ddd02 to 830843b

[mkoeppe]

comment:14

Replying to @jhpalmieri:

• Can we add a comment in Makefile.in explaining what $(1)-no-deps is and how it should be used? I'm guessing that it tries to build the package but ignores its dependencies. Does that have any uses aside from the Sage library? I've made an attempt.

Thanks very much, this is perfect.

Regarding other uses - we could reimplement sage -p in terms of this. I just wasn't sure if we want to do this on the same ticket.

• For pip packages, you have added $(1)-clean a second time to the .PHONY list. Should there be a $(1)-uninstall target for these? In that case, change one $(1)-clean to $(1)-uninstall.

Thanks for catching this. I think you fixed it already. For deprecating/renaming -clean to -uninstall, we have #29097. Probably best not to do everything on one ticket.

Could we change the build rules so that

$$(INST)/$(1)-$(2)

just does $(1)-build-deps and $(1)-no-deps? It would cut out a tiny bit of code duplication. I guess we can't force the dependencies to be built before the package, so maybe not.

I don't see an elegant way to do this, unfortunately.

[mkoeppe]

Changed author from Matthias Koeppe to Matthias Koeppe, John Palmieri

[jhpalmieri]

comment:16

This looks okay to me. Positive review on your contributions.

[jhpalmieri]

Reviewer: John Palmieri

[mkoeppe]

comment:17

Thanks!

[mkoeppe]

Changed reviewer from John Palmieri to John Palmieri, Matthias Koeppe

[orlitzky]

comment:18

Late to the party, sorry. I think my new commit factors out the no-deps code as requested.

If I missed something or if you'd rather do it on another ticket, just put the old branch back and mark it as positive review again. Everything is working now.

[orlitzky]

Changed commit from 830843b to 1a0e5c3

[orlitzky]

Changed branch from u/jhpalmieri/repair__sage__b__broken_by__29411 to u/mjo/ticket/30153

[mkoeppe]

Changed author from Matthias Koeppe, John Palmieri to Matthias Koeppe, John Palmieri, Michael Orlitzky

[mkoeppe]

comment:19

Thanks.

[mkoeppe]

Changed branch from u/mjo/ticket/30153 to u/mkoeppe/ticket/30153

[mkoeppe]

comment:21

Merged 9.2.beta6

---

New commits:

2d95def

Merge tag '9.2.beta6' into t/30153/repair__sage__b__broken_by__29411

[mkoeppe]

Changed commit from 1a0e5c3 to 2d95def

[vbraun]

Changed branch from u/mkoeppe/ticket/30153 to 2d95def

[mkoeppe]

Changed commit from 2d95def to none

[mkoeppe]

comment:23

I think the recursive invocation of $(MAKE) leads to builds with extremely high parallel load when MAKE="make -j8" as recommended in Sage build documentation. I think this is part of why lately many builds on GH Actions are failing. To be fixed in #30345