Bigtop
  1. Bigtop
  2. BIGTOP-1045

Be consistent with shells, bigtop-detect-javahome, and bigtop-utils versions

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.7.0
    • Component/s: None
    • Labels:
      None

      Description

      A while ago we decided to standardize on using /bin/bash (as opposed to /bin/sh), and to keep bigtop-detect-javahome in a centralized location. In addition to JAVA_HOME, there are now other scripts in bigtop-utils that are needed, so we should require the latest version.

      Until now, we've just been replacing /bin/sh when it's been a problem, but it's never obvious that it is the problem and is a low-risk change, so I'd like to just do a mass change now. With regards to bigtop-detect-javahome, we previously made this as a mass change, but several large patches were submitted soon thereafter and had been based off of older examples.

        Issue Links

          Activity

          Hide
          Sean Mackrory added a comment -

          I have not tested this exhaustively, but I've tested several components on several platforms and they are all working correctly. It was an automated search / replace that is easily verified visually, and the previous changes of this nature have not caused any problems since they were made.

          Show
          Sean Mackrory added a comment - I have not tested this exhaustively, but I've tested several components on several platforms and they are all working correctly. It was an automated search / replace that is easily verified visually, and the previous changes of this nature have not caused any problems since they were made.
          Hide
          Peter Linnell added a comment -

          Lovely +1 to committing and seeing where there might be breakage, which I doubt will happen. Only caveat is to make sure Ubuntu has bash really installed and not just /bin/dash.

          Thanks!

          Show
          Peter Linnell added a comment - Lovely +1 to committing and seeing where there might be breakage, which I doubt will happen. Only caveat is to make sure Ubuntu has bash really installed and not just /bin/dash. Thanks!
          Hide
          Sean Mackrory added a comment -

          Good point, Peter - although I don't think that is a cause for concern. According to Ubuntu's documentation, Bash is still the default login shell, it's just not the target of /bin/sh. I've checked several Ubuntu versions and they all have Bash installed. If it ever does become a concern, I suppose we could just add bash as a dependency to bigtop-utils.

          Show
          Sean Mackrory added a comment - Good point, Peter - although I don't think that is a cause for concern. According to Ubuntu's documentation, Bash is still the default login shell, it's just not the target of /bin/sh. I've checked several Ubuntu versions and they all have Bash installed. If it ever does become a concern, I suppose we could just add bash as a dependency to bigtop-utils.
          Hide
          Peter Linnell added a comment -

          OK, thanks for checking. That said, I see no harm in adding bash as a dependency to bigtop-utils... You know a belts and suspenders kind of thing

          Show
          Peter Linnell added a comment - OK, thanks for checking. That said, I see no harm in adding bash as a dependency to bigtop-utils... You know a belts and suspenders kind of thing
          Hide
          Sean Mackrory added a comment -

          Looks like you already did that, actually: BIGTOP-118

          Show
          Sean Mackrory added a comment - Looks like you already did that, actually: BIGTOP-118
          Hide
          Peter Linnell added a comment -
          Show
          Peter Linnell added a comment - We have this too: https://issues.apache.org/jira/browse/BIGTOP-716
          Hide
          Sean Mackrory added a comment -

          Committed - thanks, Peter!

          Show
          Sean Mackrory added a comment - Committed - thanks, Peter!

            People

            • Assignee:
              Sean Mackrory
              Reporter:
              Sean Mackrory
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development