-
-
Notifications
You must be signed in to change notification settings - Fork 491
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
./configure + homebrew: recommend the script .homebrew-build-env #29504
Comments
comment:1
Questions:
I guess in zsh, the correct invocation with |
comment:2
Replying to @jhpalmieri:
Good idea. We should recommend No |
comment:3
References: _ environment variables - What is the difference between '.' and 'source' in shells? - Unix & Linux Stack Exchange bash - running script with ". " and with "source " - Unix & Linux Stack Exchange |
New commits:
|
Author: Matthias Koeppe |
This comment has been minimized.
This comment has been minimized.
Commit: |
comment:6
Thank you for working on this, but this only prints the message if there are uninstalled homebrew packages. Otherwise, this code gets executed before the message has a chance to be printed:
|
Replying to @jhpalmieri:
If one sources this file, one would always need to make sure that it is sourced whenever one runs |
comment:8
Replying to @mwageringel:
Are you suggesting deleting the file altogether, or automatically running it every time? |
comment:9
Replying to @jhpalmieri:
I am not suggesting to delete it, as it is used by the tox system, but, if my understanding is correct, then it certainly seems less error-prone if Sage could be configured to source it during the build process. |
Branch pushed to git repo; I updated commit sha1. New commits:
|
comment:11
How about this version?
|
comment:12
Replying to @mwageringel:
I would like to keep all interactions with the system package manager on the level of hints issued to the user. |
comment:13
My initial reaction is that this won't work, since I have multiple Sage builds in different directories, but then I realized that the same script The other option would be that running |
comment:14
Replying to @jhpalmieri:
OK, I think this would actually be best handled via #29507 rather than making it homebrew-specific. Something for 9.2. |
comment:15
I would suggest waiting until #29507 is resolved, instead of adding this message now and then possibly changing it after #29507. (But if others feel strongly that it should go in now, I won't object. I like the wording in comment:11.) |
comment:16
The only thing that would change with the #29507 is the last sentence; and it won't be a problem if users have already put this line in their shell profiles. So I would be in favor of getting this into 9.1. Otherwise, this will be an FAQ on sage-support |
comment:19
Replying to @mkoeppe:
Okay. I think it will still be an FAQ, since people are not in the habit of running I still don't see the message at the end of
|
comment:20
I tried and saw the message at the end of By the way, for some reason I made a |
comment:21
Replying to @mkoeppe:
Ok, fair enough. The new message is better indeed. Adding the |
comment:22
Replying to @dcoudert:
We have two tickets for reducing the number of times that |
comment:23
Thank you so much ! :)) |
comment:24
Replying to @jhpalmieri:
Right, sorry, there's one more place to fix |
comment:25
what is this |
comment:26
Replying to @dimpase:
The loop runs once with |
comment:27
Replying to @jhpalmieri:
That's right. |
comment:28
Replying to @jhpalmieri:
Wait... why do you want to see a message here? Everything is in order |
comment:29
Replying to @mkoeppe:
thanks. That was too creative for me :-) |
comment:30
Replying to @jhpalmieri:
Well, we could do #29316 (Require ./configure before make)... At least we should make sure that |
comment:31
Replying to @mkoeppe:
That's a fair question. Maybe I don't need to see the message? Are we convinced that if A minor point: when I see the message, I see
It would be nice if it gave an absolute path so I could cut and paste it into my shell profile. But this is not a big deal: someone who has been installing homebrew packages probably knows enough to change this. But would this change be helpful? diff --git a/build/bin/sage-print-system-package-command b/build/bin/sage-print-system-package-command
index 7802d447df..ee3d3c0c5b 100755
--- a/build/bin/sage-print-system-package-command
+++ b/build/bin/sage-print-system-package-command
@@ -45,7 +45,7 @@ case $system:$command in
homebrew*:install)
echo "# To automatically take care of homebrew messages regarding "
echo "# keg-only packages for the current shell session:"
- [ -n "$SAGE_ROOT" ] || SAGE_ROOT=.
+ [ -n "$SAGE_ROOT" ] || SAGE_ROOT=`pwd`
echo "# $ source $SAGE_ROOT/.homebrew-build-env"
echo "# Add this to your shell profile if you want it to persist between shell sessions."
;; |
comment:32
Replying to @jhpalmieri:
Did you run bootstrap? There is already a change that does that |
comment:33
Running bootstrap fixes it. When does that need to be run? The only information I see says to run it if |
comment:34
Replying to @jhpalmieri:
This strong statement is not true because of #29448 (configure finds libpng but matplotlib does not) and perhaps also #27205 (Sage ignores libs from pkgconfig). But a weaker statement is true: The set of settings that I make in |
comment:35
Replying to @jhpalmieri:
You only need to run it on the ticket because the configure tarball is not current. |
comment:36
Okay, let's proceed with this. Thank you for being so patient with all of my questions. |
Reviewer: John Palmieri |
comment:38
Thank you! |
Changed branch from u/mkoeppe/__configure___homebrew__recommend_the_script__homebrew_build_env to |
When homebrew is detected, the "hints" at the end of running
./configure
should suggest running the script.homebrew-build-env
.[Marking it as critical because it has the potential of reducing the number of support questions for release 9.1]
CC: @mkoeppe @dimpase @dcoudert
Component: build: configure
Author: Matthias Koeppe
Branch/Commit:
ec64fb6
Reviewer: John Palmieri
Issue created by migration from https://trac.sagemath.org/ticket/29504
The text was updated successfully, but these errors were encountered: