OK... I see what this does, but I don't think it's where we should be going. In general I believe that non-checksummable versions should never be installed by default. Without a better Spack security model (proposed but not yet implemented), we can't really enforce this. But we don't want to encourage the opposite.

In general I believe that non-checksummable versions should never be installed by default.

should not this still be the case when the only version is from a github repo like develop? Otherwise I am fine to remove the second unit test and only keep the one which has both released version and develop.

@tgamblin @adamjstewart ping.

I could see where someone might want to mark develop preferred, but seems like to me that is the only time it should be installed by default, and it should probably warn about that.

whether or not we warn about installation of develop is probably irrelevant to this PR, which only adds unit tests for Spec object.

But why should we unit test a case we want to avoid?
On Oct 21, 2016 3:46 AM, "Denis Davydov" [email protected] wrote:

whether or not we warn about installation of develop is probably
irrelevant to this PR, which only adds unit tests for Spec object.

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#2064 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AB1cdwoL-y-4tG-zeIyBtFYl3sr2kxqtks5q2G3mgaJpZM4KceTG
.

@citibeth : I am confused, you opened an issue which says add unit tests (see OP, essentially coming from this comment #1933 (comment) by @tgamblin from your PR ), I did exactly this here.

With a new Spack security model, I envision a --secure mode (should be on
by default) in which such a warning would be an error. I believe that
Spack will get better acceptance if we can tell sysadmins that it NEVER
does an insecure install (when being run in secure mode). Not "most of the
time" or "usually", but "never."

On Thu, Oct 20, 2016 at 5:39 PM, Todd Gamblin [email protected]
wrote:

I could see where someone might want to mark develop preferred, but seems
like to me that is the only time it should be installed by default, and it
should probably warn about that.

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#2064 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AB1cd0eowjB2Sp2cR_VCG1JkGgIqhoDuks5q1999gaJpZM4KceTG
.

Until we get a better security model, I'm of the opinion that package PRs
without at least one checksummable version should not be merged.
Unfortunately, I think a few have slipped through anyway (Julia). This
should not be encouraged.

On Thu, Oct 20, 2016 at 4:01 PM, Denis Davydov [email protected]
wrote:

In general I believe that non-checksummable versions should never be
installed by default.

should not this still be the case when the only version is from a github
repo like develop? Otherwise I am fine to remove the second unit test and
only keep the one which has both released version and develop.

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#2064 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AB1cd24ZFujuM5m1mefzFMm3HlHV2lXwks5q18ingaJpZM4KceTG
.

I would prefer it be removed. But a this point... I've communicated my
concern and been heard. And this is not preference I will fight to the
death over. I believe you should do what you think is best.

On Tue, Oct 25, 2016 at 2:59 PM, Denis Davydov [email protected]
wrote:

@davydden commented on this pull request.

In var/spack/repos/builtin.mock/packages/develop-test-2/package.py:

+# it under the terms of the GNU Lesser General Public License (as
+# published by the Free Software Foundation) version 2.1, February 1999.
+#
+# This program is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the IMPLIED WARRANTY OF
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the terms and
+# conditions of the GNU Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public
+# License along with this program; if not, write to the Free Software
+# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+##############################################################################
+from spack import *
+
+
+class DevelopTest2(Package):

@citibeth https://github.com/citibeth do you want me to remove this
unit test? I am fine to do that, but we certainly need another one!

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#2064 (review), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AB1cd-VC2Al8D18QrtpUh5CJq9_b3FPiks5q3lGcgaJpZM4KceTG
.

I would prefer it be removed.

done.

@tgamblin : i think this unit-test only PR is ready to be merged as soon as Travis is happy.