setup.py: generate auto-generated files in setup()

[mkoeppe]

Follow-up on #21613 and #22094. Ticket for embray.

CC: @embray @jdemeyer @kiwifb

Component: build

Author: Erik Bray

Branch/Commit: d9d6e54

Reviewer: Jeroen Demeyer

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

[embray]

comment:2

So basically you undid my work and then assigned me a ticket to redo it? Ok, thanks. <== (Sorry, that wasn't meant to be sarcastic; having the ticket is actually helpful).

[embray]

comment:3

Since I'm back from vacation now can you just let me fix this for 7.5?

[kiwifb]

comment:4

I'll take full responsibility for the undoing. I don't know what is Volker timing for 7.5, I must say, but it was on my mind that we couldn't get 7.5 in that state. Blame me if it was put forward.

[kiwifb]

comment:5

You may want to make it a blocker if you want to attract Volker's attention.

[embray]

comment:6

7.5 isn't released yet so I don't know why it can't wait.

"I don't know what is Volker timing for 7.5"

Yeah neither, it seems, does anybody. Maybe Sage should have some kind of release schedule...

[embray]

Branch: u/embray/ticket-22106

[embray]

Commit: e82c80c

[embray]

Author: Erik Bray

[embray]

comment:7

Here's a workaround that deals with the issue without running code at the setup.py module level. It's related, somewhat, to #21682, which should now depend on this.

I still see this as something of a stop-gap solution. My longer-term plan is to implement something that allows Sage's individual sub-packages to provide configuration details on a per-package basis, to be collected by the main setup.py in a consistent manner (similar to, but not exactly the same as http://docs.astropy.org/en/stable/development/building.html#customizing-setup-build-for-subpackages). This will make it easier for individual packages to inject steps into the build process (sort of like sub-makes) at the appropriate build stages.

[mkoeppe]

comment:8

There's no commit on the branch

[jdemeyer]

comment:9

Personally, I am really against pushing this to 7.5.

The current 7.5.rc1 is working fine, I would not want to risk to break stuff again. Especially build-system issues might take a while to be discovered.

I also don't see the urgency for this ticket. I agree that something needs to be done, but surely it can wait for Sage 7.6?

[jdemeyer]

comment:10

Replying to @embray:

So basically you undid my work

To be fair, we only undid your work because it broke stuff.

[embray]

comment:11

Yeah, it's fine. I don't think it's urgent now.

[sagetrac-git]

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

3bb5e73

Generate auto-generated sources as the earliest step in the 'build' command.

[sagetrac-git]

Changed commit from e82c80c to 3bb5e73

[embray]

New commits:

3bb5e73

Generate auto-generated sources as the earliest step in the 'build' command.

[jdemeyer]

Changed branch from u/embray/ticket-22106 to u/jdemeyer/ticket-22106

[jdemeyer]

comment:15

I made some cosmetic changes.

One more thing: is it a problem if some packages appear more than once? Should we do something like

for pkg in autogen_all():
    if pkg not in self.distribution.packages:
        self.distribution.packages.append(pkg)

---

New commits:

1286549

Cosmetic changes

[jdemeyer]

Changed commit from 3bb5e73 to 1286549

[jdemeyer]

Reviewer: Jeroen Demeyer

[embray]

comment:17

I thought about that too, and even had that in an earlier version, but concluded that it's not a problem. If you find some case where it is a problem it could be changed.

[jdemeyer]

comment:18

Given that we both thought about it, you should add a small comment explaining that duplicated entries are potentially a problem but actually aren't a problem.

If this branch actually works (I have not tested it sufficiently yet), this can get positive_review from me.

[embray]

comment:19

Well, you changed the branch to your branch so you're free to do that. It seems to me like a silly thing to comment on--at that point you might as well put the check in anyways just in case.

[jdemeyer]

comment:20

Replying to @embray:

at that point you might as well put the check in anyways just in case.

OK, let's do that then.

[embray]

comment:21

Will you do it or shall I? I don't mind, it's just you already switched to your branch.

[jdemeyer]

comment:22

I'm currently doing other things. So if you have time, please go ahead.

[sagetrac-git]

Changed commit from 1286549 to d9d6e54

[sagetrac-git]

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

abf86ee	Generate auto-generated sources as the earliest step in the 'build' command.
d311e0e	Cosmetic changes
d9d6e54	Only add packages which did not exist already

[jdemeyer]

comment:24

Rebased to 7.6 + added the changes discussed in [comment:15]

[embray]

comment:26

LGTM, thanks!

[vbraun]

Changed branch from u/jdemeyer/ticket-22106 to d9d6e54