Conversation
|
Interesting. It looks like Homebrew solves this problem by combining both in the same package: I think I like this solution better, but let's wait for others to comment. |
That solution leaves an obviously wrong Combining the builds means you require a lot of X11 stuff just to run TCL. Some people might not want that; for example, TCL-based web servers. That's probably by TCL and Tk were separate to begin with. |
|
@adamjstewart Is any further review needed on this PR? |
|
@adamjstewart ping |
Still waiting on others to comment on this PR. I don't know of any other packages that install their own source, so I'm hesitant to just merge it. |
@scheibelp @tgamblin @alalazo Can anyone else chime in on this? |
|
Didn't we, at some point discuss, whether |
|
Yes and it was merged. This PR uses its code. The difference here is,
installing the source is a required part of the package.
…On Tue, May 22, 2018 at 3:52 AM healther ***@***.***> wrote:
Didn't we, at some point discuss, whether spack install should have a
general option that installs the sources into prefix? I'm too unfamiliar
with the history and potential legal reasons for why the sources aren't
generally kept around, but there have been a number of times where I wished
for such an option (which I'd want to default to True at least on our CI
stuff)
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#8153 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AB1cdyfImDYSMjaIxUBlKJY-Ks0yAGrlks5t08PQgaJpZM4UBiNd>
.
|
|
when you say "This PR uses its code" shouldn't that mean you just have to do a function call? |
|
The other PR doesn’t have a function to call.
…On Tue, May 22, 2018 at 7:15 AM healther ***@***.***> wrote:
when you say "This PR uses its code" shouldn't that mean you just have to
do a function call?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#8153 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AB1cd_xIZ277DOfKOFLUBsQ9NbomAtayks5t0_NbgaJpZM4UBiNd>
.
|
|
@tgamblin @alalazo @scheibelp Can one of you please comment on this PR? @adamjstewart would like more eyes looking at it, due to the unusual nature of keeping sources around. |
|
@citibeth Now that you are calling the For the record, I'm in favor of this type of solution. IMHO, the primary task of spack is to provide a fully functioning installation without the need for extra arguments. If Tcl's source is needed in the installation directory to allow Tk to find/link properly, then the default recipe should install the sources (even if |
|
On Wed, Jun 6, 2018 at 10:28 AM, Kelly (KT) Thompson < ***@***.***> wrote:
@citibeth <https://github.com/citibeth> Now that you are calling the
install-headers make target, do you still need to copy all of the sources
to the install location?
Yes. The `install-headers` target simply makes version 8.6 work as
versions through 8.5 always did:
http://wiki.tcl.tk/17463
Adding the call to `make install-headers` is the right thing to do, but it
didn't solve the larger problem of building Tk.
For the record, I'm in favor of this type of solution. IMHO, the primary
task of spack is to provide a fully functioning installation without the
need for extra arguments. If Tcl's source is needed in the installation
directory to allow Tk to find/link properly, then the default recipe should
install the sources (even if --source isn't requested).
I agree. Spack works on the principle that two installed packages with the
same hash have the same functionality. `--source` was designed to not
change the hash, whether you use it or not --- under the assumption that
packages don't need the source installed, this is just an option for people
who want it there for whatever reason. In this case, lack of source DOES
change the functionality. Or in other words... the `--source` option is
not relevant to the problem here. But I did design it so `tcl` installs
sources in the same place as the `--source` option would. This way, we
don't needlessly end up with TWO copies of the source installed!
|
|
Thanks! In general there may be a concern that the same problem comes up several times across several packages and it would create clutter if it were solved differently each time. I'm trying to help recognize patterns by recording strange behavior at https://github.com/spack/spack/wiki/Packaging-Special-Cases (the point being that if multiple packages start doing something then it will become apparent by viewing/updating that page). |
This fixes a bug in the Tcl/Tk installation #8151. Previously, the recipe for
tkwould erroneously pick up the system-installedtcl.hfile. This "worked" for newer systems that had TCL 8.6 installed. The bug was unmasked building on SLES 11, which comes with an older version of TCL.Long story short, the
tclbuild references the original sources upon install, and thetkbuild requires thetclsources. TCL sources were installed following the example of #4102. If the user installs withspack install --source, then the sources would be copied twice. This should not be a serious problem.@junghans @davydden @tgamblin from #4083:
Note that this is a case in which the presence of the source is structural, not an install option. If it were optional (it's not), then a
+sourcevariant would be warranted here.*** In addition, a
make install-headerswas added, as per advice for TCL 8.6