Revert gh-40765 which breaks builds from the git repo

[vbraun]

#40765 breaks builds from a clean repo. Revert it until it can be fixed.

[github-actions]

Documentation preview for this PR (built with commit 07a6950; changes) is ready! 🎉
This preview will update shortly after each push to this PR.

[dimpase]

I'm currently testing the alternative #40327

[tobiasdiez]

What's the issue? A bit more context would be helpful.

[dimpase]

basically, sage_conf is a standard package, so it gets installed, but it needs _conf.py:

make --no-print-directory all-sage-docs all-sage
make[3]: Nothing to be done for 'all-sage-docs'.
/usr/bin/python3 /home/dima/software/sage/build/bin/sage-venv --system-site-packages "/home/dima/software/sage/local/var/lib/sage/venv-python3.13"
make[3]: *** No rule to make target '/home/dima/software/sage/pkgs/sage-conf/_sage_conf/_conf.py', needed by '/home/dima/software/sage/local/var/lib/sage/venv-python3.13/var/lib/sage/installed/sage_conf-10.8.beta4'.  Stop.
make[3]: *** Waiting for unfinished jobs....
rm -f "/home/dima/software/sage/local/var/lib/sage/venv-python3.13/var/lib/sage/installed"/python3_venv-*
touch "/home/dima/software/sage/local/var/lib/sage/venv-python3.13/var/lib/sage/installed/python3_venv-3.13--usr-bin-python3--system-site-packages"
make[2]: *** [Makefile:3013: all-start] Error 2

[tobiasdiez]

Yes, then this is indeed better fixed by #40327 (ready for review now).

(or if you want to have a hotfix, just restore the .py.in file - no need to revert everything else).

[dimpase]

Yes, then this is indeed better fixed by #40327 (ready for review now).

(or if you want to have a hotfix, just restore the .py.in file - no need to revert everything else).

that's not enough, as you also need to restore the generation of .py from .py.in. Namely the line
584 in configure.ac removed in #40765

[vbraun]

The issue is https://groups.google.com/g/sage-release/c/7LhB72fmw6k/m/QINoQj8YAgAJ

I'm not going to attempt to fix this by another giant patch that touches 71 other files. Which is much larger than the poorly-tested pr that got us into the problem in the first place. You are welcome to suggest a narrow PR that just fixes the breakage introduced in #40765

[tobiasdiez]

The issue is https://groups.google.com/g/sage-release/c/7LhB72fmw6k/m/QINoQj8YAgAJ

Thanks!

I'm not going to attempt to fix this by another giant patch that touches 71 other files.

I've created a minimal version of it in #40882 that should fix the problem (at least CI seems to pass now).

Which is much larger than the poorly-tested pr that got us into the problem in the first place.

The underlying problem why this was not discovered during testing was that sage-the-distro didn't recognize that one of it's packages was changed and thus had to be reinstalled.

[vbraun]

I've created a minimal version of it in #40882 that should fix the problem (at least CI seems to pass now).

How is that minimal, it still touches more files than just reverting the offending PR

Also as you have seen you need to build from a clean checkout and not just trust the CI here.

[dimpase]

I've created a minimal version of it in #40882 that should fix the problem (at least CI seems to pass now).

How is that minimal, it still touches more files than just reverting the offending PR

it removes an spkg - all 8 files in the spkg.

Also as you have seen you need to build from a clean checkout and not just trust the CI here.

I'm testing it now from the clean checkout on macOS and on Linux.

You might consider to make sure that one of the fast buildbots does git clean -fdx before each test run for a PR touching anything outside src/

[tobiasdiez]

I've created a minimal version of it in #40882 that should fix the problem (at least CI seems to pass now).

How is that minimal, it still touches more files than just reverting the offending PR

It adds two lines, and removes 40 lines. That's very minimal, especially compared to +78 −186 in this PR here. It's also mostly deleting a single folder (which manifests the workaround to use --disable-sage-conf that was given on sage-dev). Perhaps more importantly than some line/file count, #40765 actually fixed an issue for downstream packages (#40726) that would be reintroduced by the revert.

Also as you have seen you need to build from a clean checkout and not just trust the CI here.

The changes in https://github.com/sagemath/sage/pull/40882/files#diff-625c2dc0791820d3e14dfd284ea78767685e0eb75f28eb67fce0304504608f12 (ci-distro.yml) exactly make sure that the full CI (that runs from a clean checkout) are activated. As you can see at https://github.com/sagemath/sage/actions/runs/17995565127/job/51194107594?pr=40882, all of those tests are succeeding (modulo the small doctest issue that dima just pointed out as well).

---

I honestly have trouble understanding the bigger problem here. Sure #40765 introduced an annoying bug that should be fixed before the next stable release - and I'm very sorry that this issue was overlooked and made it into the last beta. But thinks like this, while unfortunate, are very normal and happen all the time in development (otherwise one would be eventually living in a bug free world). So let's work together to find a solution to the new issue.

[dimpase]

let's close this one in favour of #40882

[jhpalmieri]

If the release manager says that we should revert a PR, we should revert the PR. Any other changes can be built on top of this one.

[dimpase]

The release manager does not have to say anything if he could just revert it.

There is currently a search for the optimal way forward going on. Actually Volker asked for it, @jhpalmieri

Let us wait for his response.

[vbraun]

I was hoping to merge #40882 instead, but unfortunately it fails. I'll go ahead with reverting it for 10.8.beta5

[dimpase]

@vbraun - please, a couple of long tests there fails for some reason I couldn't reproduce.