Skip to content

pkgs/sage-conf_pypi: Skip configure/make when SAGE_CONF_FILE or SAGE_CONF_ENV_FILE are set #37620

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

Open
wants to merge 6 commits into
base: develop
Choose a base branch
from

Conversation

mkoeppe
Copy link
Contributor

@mkoeppe mkoeppe commented Mar 16, 2024

The distribution package sage-conf as found on PyPI is the version from pkgs/sage-conf_pypi, which creates a SAGE_ROOT and then runs configure and make.

Here we make the same distribution package usable in a different way: Set SAGE_CONF_FILE or/and SAGE_CONF_ENV_FILE to whatever static file has your configuration, or just /dev/null, and that will be taken.

This may be a useful simplification for downstream packagers.

We also add another environment variable: SAGE_CONF_CONFIGURE_ARGS and add some documentation.

📝 Checklist

  • The title is concise and informative.
  • The description explains in detail what this PR is about.
  • I have linked a relevant issue or discussion.
  • I have created tests covering the changes.
  • I have updated the documentation accordingly.

⌛ Dependencies

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 16, 2024

@kiwifb Is this roughly what would enable you to use sage-conf from PyPI?

@kiwifb
Copy link
Member

kiwifb commented Mar 16, 2024

I'll have to try it, but on the description, that would avoid most of the patching I do. That could motivate me, to fix all the issues that come with trying to ship the sage-config script.

else:
# build_scripts skips empty files, so create something nonempty
with open(os.path.join(HERE, 'bin', 'sage-env-config'), 'w') as f:
print("# -*- shell-script -*-", file=f)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am perfectly happy with it being skipped. I am currently not shipping this file in any way shape or form and everything is fine. The main consumer at runtime is the sage script and it can cope with it not being present

# As of Issue #22731, sage-env-config is optional.

So, I'd rather skip it rather than having a token file.

Copy link
Contributor Author

@mkoeppe mkoeppe Mar 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem is that after skipping the installation, setuptools gives an error because the file is named in the metadata but is not present...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably why I currently remove it from setup.cfg. I must have seen that error some time ago. I guess, that acceptably would avoid the patching then.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd rather skip it rather than having a token file.

Done in d85432a

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's get this in please.

Copy link

github-actions bot commented Mar 24, 2024

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

@mkoeppe mkoeppe force-pushed the sage_conf_merge branch 2 times, most recently from 54e22ea to 97de68a Compare April 1, 2024 00:52
@mkoeppe mkoeppe force-pushed the sage_conf_merge branch from 26dbbfb to b10fcb3 Compare May 12, 2024 06:45
@mkoeppe mkoeppe force-pushed the sage_conf_merge branch from b10fcb3 to 7ccab80 Compare May 25, 2024 18:19
@mkoeppe mkoeppe force-pushed the sage_conf_merge branch from 7ccab80 to 857af39 Compare June 9, 2024 17:49
@mkoeppe mkoeppe changed the title pkgs/sage-conf_pypi: Skip configure/make when SAGE_CONF_FILE or SAGE_CONF_ENV_FILE are set pkgs/sage-conf_pypi: Skip configure/make when SAGE_CONF_FILE or SAGE_CONF_ENV_FILE are set Sep 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants