Uploaded image for project: 'Data Management'
  1. Data Management
  2. DM-11949

newinstall.sh should warn about env vars known to cause problems

    Details

      Attachments

        Issue Links

          Activity

          Hide
          jhoblitt Joshua Hoblitt added a comment -

          Jonathan Sick Is this approximately what you had in mind?

          WARNING: the following environment variables are defined that will effect
          the operation of the LSST build tooling.
           
          LSST_HOME="foo"
          EUPS_PATH="bar"
          EUPS_PKGROOT="baz"
          REPOSITORY_PATH="qux"
           
          It is recommend that they are undefined before running this script.
           
          unset LSST_HOME EUPS_PATH EUPS_PKGROOT REPOSITORY_PATH
          

          Show
          jhoblitt Joshua Hoblitt added a comment - Jonathan Sick Is this approximately what you had in mind? WARNING: the following environment variables are defined that will effect the operation of the LSST build tooling.   LSST_HOME= "foo" EUPS_PATH= "bar" EUPS_PKGROOT= "baz" REPOSITORY_PATH= "qux"   It is recommend that they are undefined before running this script.   unset LSST_HOME EUPS_PATH EUPS_PKGROOT REPOSITORY_PATH
          Hide
          jsick Jonathan Sick added a comment -

          Sure. Will this exit the script, prompt for confirmation to continue, or just continue the newinstall script anyways?

          Show
          jsick Jonathan Sick added a comment - Sure. Will this exit the script, prompt for confirmation to continue, or just continue the newinstall script anyways?
          Hide
          jhoblitt Joshua Hoblitt added a comment -

          The current implementation prints the warning early on and then continues. I thought that was the requested behavior on slack this morning?

          Show
          jhoblitt Joshua Hoblitt added a comment - The current implementation prints the warning early on and then continues. I thought that was the requested behavior on slack this morning?
          Hide
          jsick Jonathan Sick added a comment -

          I wonder if only a warning is insufficient? People will miss the warning in the stream and I'll still have to document it.

          What do you think about having the warning block the script for confirmation (unless -b is used)?

          Show
          jsick Jonathan Sick added a comment - I wonder if only a warning is insufficient? People will miss the warning in the stream and I'll still have to document it. What do you think about having the warning block the script for confirmation (unless -b is used)?
          Hide
          jhoblitt Joshua Hoblitt added a comment -

          I'm fairly ambivalent about it. I'll make make the warning fatal if not in batch mode.

          Show
          jhoblitt Joshua Hoblitt added a comment - I'm fairly ambivalent about it. I'll make make the warning fatal if not in batch mode.
          Hide
          jsick Jonathan Sick added a comment -

          Perfect :+1:

          Show
          jsick Jonathan Sick added a comment - Perfect :+1:
          Hide
          jhoblitt Joshua Hoblitt added a comment -

          I've been looking through the lsst github org and I don't think there are any hard dependencies on newinstall.sh's loadLSST. files declaring LSST_HOME but I'm not 100% sure. However, I can't think of a reason why the loadLSST. files would need to respect a pre-declared LSST_HOME. I think it we remove that behavior, we don't need to doc or warn about setting LSST_HOME.

          Show
          jhoblitt Joshua Hoblitt added a comment - I've been looking through the lsst github org and I don't think there are any hard dependencies on newinstall.sh's loadLSST. files declaring LSST_HOME but I'm not 100% sure. However, I can't think of a reason why the loadLSST. files would need to respect a pre-declared LSST_HOME . I think it we remove that behavior, we don't need to doc or warn about setting LSST_HOME .
          Hide
          jhoblitt Joshua Hoblitt added a comment -

          merged.

          Show
          jhoblitt Joshua Hoblitt added a comment - merged.

            People

            • Assignee:
              jhoblitt Joshua Hoblitt
              Reporter:
              jhoblitt Joshua Hoblitt
              Watchers:
              Jonathan Sick, Joshua Hoblitt
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Summary Panel