Skip to content

CDRIVER-5509 Use Bash and shebangs whenever able #1680

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

Merged
merged 11 commits into from
Jul 31, 2024

Conversation

eramongodb
Copy link
Contributor

Summary

Addresses CDRIVER-5509. Verified by this patch.

For the most part, our scripts all use Bash already following prior on EVG config refactoring and script hygiene improvements such as in #1193 and #1187.

This PR continues the prior work to ensure consistent use of shebangs and script invocation in our EVG, build, and utility scripts.

Shebangs

This PR applies #!/usr/bin/env bash to all our shebangs for consistency with the pattern being used by DET scripts: mongodb-labs/drivers-evergreen-tools#458

Scripts related to Debian packaging are left unmodified due to the special environment requirements. These may also be updated for consistency if it turns out not to be an issue.

Script Invocation

This PR adds the executable permission bit to all shell scripts and invokes them directly (e.g. ./script.sh instead of bash script.sh) to respect shebangs. Otherwise our invocation risks overriding the preferred shell for which the script is intended to be used.

License Notice in Uninstall Script

#1655 overlooked some minor formatting issues with the license notice embedded in the generated uninstall script.

#!/bin/sh
# * Mongo C Driver uninstall program, generated with CMake
# * 
# * Copyright 2009-present MongoDB, Inc.
# * 
# * Licensed under the Apache License, Version 2.0 (the \"License\")
# * 
# * you may not use this file except in compliance with the License.
# * You may obtain a copy of the License at
# * 
# *   http://www.apache.org/licenses/LICENSE-2.0
# * 
# * Unless required by applicable law or agreed to in writing, software
# * distributed under the License is distributed on an \"AS IS\" BASIS,
# * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# * See the License for the specific language governing permissions and
# * limitations under the License.
# * 
set -eu

...

This PR tidies it up a bit to look as follows:

#!/usr/bin/env bash
#
# Mongo C Driver uninstall program, generated with CMake
#
# Copyright 2009-present MongoDB, Inc.
#
# Licensed under the Apache License, Version 2.0 (the "License")
#
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
#   http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

set -eu

...

Similarly for the Windows CMD script, before:

@echo off
rem Mongo C Driver uninstall program, generated with CMake
rem 
rem Copyright 2009-present MongoDB, Inc.
rem 
rem Licensed under the Apache License, Version 2.0 (the \"License\")
rem 
rem you may not use this file except in compliance with the License.
rem You may obtain a copy of the License at
rem 
rem   http://www.apache.org/licenses/LICENSE-2.0
rem 
rem Unless required by applicable law or agreed to in writing, software
rem distributed under the License is distributed on an \"AS IS\" BASIS,
rem WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
rem See the License for the specific language governing permissions and
rem limitations under the License.
rem 
call :init

...

and after:

@echo off

rem Mongo C Driver uninstall program, generated with CMake
rem
rem Copyright 2009-present MongoDB, Inc.
rem
rem Licensed under the Apache License, Version 2.0 (the "License")
rem
rem you may not use this file except in compliance with the License.
rem You may obtain a copy of the License at
rem
rem   http://www.apache.org/licenses/LICENSE-2.0
rem
rem Unless required by applicable law or agreed to in writing, software
rem distributed under the License is distributed on an "AS IS" BASIS,
rem WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
rem See the License for the specific language governing permissions and
rem limitations under the License.

call :init

...

@eramongodb eramongodb self-assigned this Jul 24, 2024
@eramongodb
Copy link
Contributor Author

Fixed the install-uninstall-check task by ensuring the uninstall.sh script is always executable even prior to installation. Verified by this patch.

Copy link
Contributor

@rcsanchez97 rcsanchez97 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, with a few spacing tweaks.

@@ -72,24 +70,46 @@ function(append_line line)
file(APPEND "${UNINSTALL_WRITE_FILE}" "${line}\n")
endfunction()

# Ensure generated uninstall script has executable permissions.
if ("${CMAKE_VERSION}" VERSION_GREATER_EQUAL "3.19.0")
file(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
file(
file (

else ()
# Workaround lack of file(CHMOD).
get_filename_component(_script_filename "${UNINSTALL_WRITE_FILE}" NAME)
file(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
file(
file (

GROUP_READ GROUP_EXECUTE
WORLD_READ WORLD_EXECUTE
)
file(RENAME "${_script_filename}.d/${_script_filename}" "${_script_filename}")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
file(RENAME "${_script_filename}.d/${_script_filename}" "${_script_filename}")
file (RENAME "${_script_filename}.d/${_script_filename}" "${_script_filename}")

@eramongodb
Copy link
Contributor Author

Additionally removed the find-cmake.sh script, which appears to be unused by any of the C Driver, CXX Driver, or libmongocrypt repos.

@eramongodb eramongodb merged commit 46ceeba into mongodb:master Jul 31, 2024
43 of 46 checks passed
@eramongodb eramongodb deleted the cdriver-5509 branch July 31, 2024 21:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants