Do not ignore uncommitted changes in repo

[rgommers]

meson-python should pick up uncommitted changes to files that are under version control. xref scipy/scipy#16187 with a lot of details. The main reasons are:

1. It is very confusing to silently ignore changes, it will lead to lost time by users and contributors.
2. Even if you understand it, it's quite cumbersome to have to commit changes before running pip install . (or similar) each time.
3. For complex packages, it is very common that patches have to be applied before building sdists, wheels, or other packages (conda/rpm/deb/etc.). It is now extremely hard to do so.

Reproducible releases should be made through a CI job anyway, so "the current behavior is more reproducible" doesn't really apply. And I don't think there is any other reason to keep the current behavior?

[FFY00]

The behavior is inherited from Meson. There are very good reasons for this behavior, but I understand it can also be annoying in certain scenarios. I am not opposed to changing this behavior, but I think we need some pretty good reasons for it.

The main issue I am seeing is that your git tag will not include the changes. This results in explicitly different build when using the sdist or the git tag/tarball. Have you considered this?

Overall, I think we should have an option to switch this behavior, but making it the default behavior is something I feel more conflicted about.

[rgommers]

The main issue I am seeing is that your git tag will not include the changes. This results in explicitly different build when using the sdist or the git tag/tarball. Have you considered this?

Yes. This is always the case, also today. And it will be the case for every single redistributor. As I said above, it's very common (and necessary) to apply patches that are packaging system specific. Those clearly cannot be part of a git tag.

[rgommers]

I am not opposed to changing this behavior, but I think we need some pretty good reasons for it.

I gave some above. Also, did you look at the build-pypi-artifacts.py script in https://github.com/scipy/scipy/pull/16187/files? It was quite painful to write, it's dangerous to use (adding a commit, then dropping it - if something goes wrong, you can lose work), and even with all that work there is no way to write something that is actually correct (it will at least have a non-existing commit hash in the scipy.version content).

There are very good reasons for this behavior,

Are there any beyond reproducibility/safety?

I think we are in a very different situation here than both meson dist and flit:

• For meson dist, that is basically only invoked as a release archive at release time, not for building binaries nor during development. So it's not really a problem that you cannot apply patches.
• For flit, that is for (usually simple) pure Python packages only. Those are way less likely to need any patches.

[FFY00]

I am aware, but this is only a problem when building sdists, and distributors do not need to build sdists, only wheels. But regardless, I agree that we should provide an option for this, I am just unsure that it should be the default.

Yes. This is always the case, also today.

That seems like an issue. AFAICT from my experience packaging things, that is not common, and undesirable.

Also, did you look at the build-pypi-artifacts.py script in scipy/scipy#16187 (files)?

Yes, I have some thoughts on it, but I don't think they are particularly relevant here. I can get into them if you want.

Are there any beyond reproducibility/safety?

My main issue, as I pointed above, is that the actual release will include uncommitted changes.

I can implement this if you want though.

[rgommers]

My main issue, as I pointed above, is that the actual release will include uncommitted changes.

Only if that's needed (or you are doing it wrong by accident of course), which it is sometimes. Here's an example if you are interested: scipy/scipy#10247.

I am aware, but this is only a problem when building sdists, and distributors do not need to build sdists, only wheels.

Is this true? If it is, I agree it's less urgent/painful. However, build builds wheels via sdist, and I believe pip has a plan to do the same in the future.

[FFY00]

Only if that's needed (or you are doing it wrong by accident of course), which it is sometimes. Here's an example if you are interested: scipy/scipy#10247.

That example is pretty unproblematic because we are talking about the LICENSE file.

If there are changes that are needed for the release, why aren't them being included in the git tag/tarball?

Is this true? If it is, I agree it's less urgent/painful. However, build builds wheels via sdist, and I believe pip has a plan to do the same in the future.

If you pass no target to pypa/build, it will build the wheel via sdist, this is the default behavior. If you select the sdist and/or wheel targets manually, it will build them standalone, so python -m build -w will build the wheel directly from the current source.

We do this in pypa/build because it highlights issues with the sdists build not being usable. pip's story is different, the goal is to have thing work as much as possible, so this behavior is probably undesirable there.

[rgommers]

That example is pretty unproblematic because we are talking about the LICENSE file.

If there are changes that are needed for the release, why aren't them being included in the git tag/tarball?

For the license file, the tl;dr is:

• The main LICENSE.txt file must match the standard BSD-3 license for ~98% or various license checkers get unhappy (and we cannot ignore those checkers unfortunately for ). That's why we cannot commit changes.
• For a release, we must append the (non BSD-3) licenses for license correctness reasons.

Another, more important change is: for a release we vendor libraries (libopenblas, libquadmath, the gfortran runtime); that then triggers more license amendments as well as a .py file to add the .libs library to the DLL search path. This is not part of the sdist, but is part of the wheels so it gets included if we build a wheel via an sdist.

pip's story is different, the goal is to have thing work as much as possible, so this behavior is probably undesirable there.

Exactly. So it may be okay to leave the current default unchanged, but if pip does indeed start defaulting to building a wheel via an sdist, we certainly have to change it because then it affects even casual contributors.

We do this in pypa/build because it highlights issues with the sdists build not being usable.

Related question: why don't you raise an error and abort the build if uncommitted changes are detected? That seems much better if you want to go for maximum safety. Silently ignoring changes is a recipe for lots of lost time and frustration.

[rgommers]

Never mind that last question, forgot that that is the default now.

[rgommers]

Discussed:

• it's fine to ignore untracked files, that's quite unlikely to be an issue in practice (and patch files typically touch existing build/code files).
• @FFY00 will add an approach to add uncommitted changes to tracked files
• One will be the default, the other will be a flag that meson-python understands. What should be the default?
  • The default staying as it is now (erroring out) is okay if (and only if) there can be a clear message to the user about what flag to add to the build command to pick up uncommitted changes. That's not easy right now, because the behavior we're seeing is implemented by meson and not meson-python
  • Let's take a staged approach:
    • for now let's switch the default to "include uncommitted changes"
    • try to implement the clear warning message
    • if that works, decide on the final default (both should be fine from my/SciPy's perspective I believe). if that doesn't work, the default stays "include uncommitted changes"