Skip to content
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.

Sign up for GitHub

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

Jump to bottom

add documentation and build script for Go wrapper #626

Closed
wants to merge 3 commits into from
Closed

add documentation and build script for Go wrapper #626

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d44e19d
add documentation and build script for Go wrapper
tangowithfoxtrot Feb 22, 2024
04c8a9b
`npm i`
tangowithfoxtrot Feb 22, 2024
c046b53
add `--release` arg
tangowithfoxtrot Feb 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions languages/go/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@ managing projects and secrets, as well as a client interface to facilitate opera
- Go installed
- C environment to run CGO

## Building the SDK

`cargo` and `npm` are required to build the SDK. To build, run the `./build.sh` script.

## Installation

Download the SDK files and place them in your Go project directory.
Expand Down
52 changes: 52 additions & 0 deletions languages/go/build.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
#!/usr/bin/env bash
Copy link

Choose a reason for hiding this comment

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

why do we need a build.sh script that actually does not build the Go lang wrapper, but instead builds SDK ?
Feels like that's something for the SDK itself, which we already have documented in the root README.md.

Copy link
Member

Choose a reason for hiding this comment

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

I would agree, @tangowithfoxtrot maybe we can discuss what the heart of the PR is? I have an idea that I think you'll like 😄


# Usage: ./build.sh [--release]

check_command() {
if ! command -v "$1" &>/dev/null; then
printf '%s\n' "$1 is required to build locally. Please install $2."
exit 1
fi
}

check_command cargo Rust
check_command go Go
check_command npm Node.js
Comment on lines +5 to +14
Copy link
Member

Choose a reason for hiding this comment

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

We generally presume the environment is correctly bootstraped. These checks can provide a false confidence since we don't validate the version of the scripts.


REPO_ROOT="$(git rev-parse --show-toplevel)"
Copy link

Choose a reason for hiding this comment

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

instead of relying on git, how about obtaining an absolute path of the script location and traversing back to parent ? Something like$(realpath "$0/../../")

GO_LIB_DIR="$REPO_ROOT/languages/go/internal/cinterface/lib"

pushd "$REPO_ROOT" || exit 1
printf '%s\n\n' "Cleaning old builds..."
rm -f languages/go/example/example && rm -rf "$GO_LIB_DIR"

printf '%s\n\n' "Building binaries..."

if [ "$1" = "--release" ]; then
cargo build --release
else
cargo build
fi

npm i && npm run schemas
Copy link
Member

Choose a reason for hiding this comment

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

We never want to run npm i, and ci would re-install every dependency. I suggest presuming the environment is. already bootstrapped like our other build scripts.


printf '%s\n\n' "Copying Go bindings to $GO_LIB_DIR..."
mkdir -p "$GO_LIB_DIR"
find target/debug -maxdepth 1 -type f -name "libbitwarden_c.*" -exec cp {} "$GO_LIB_DIR"/ \;
Copy link
Member

Choose a reason for hiding this comment

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

There's a reference to the target/debug dir here, which should be target/release when building in release mode.

Also, the libbitwarden_c.* wildcard copies four files for me (libbitwarden_c.{a,d,dylib,rlib}), while only the dylib seems to be needed on Mac.

I don't think the extra copies are necessarily a concern, but only *.dylib and *.a are in the gitignore file, so at the very least we should add the two other types to not clutter the working directory (*.d and *.rlib).


printf '%s\n\n' "Build complete!"
printf '%s\n' "To run the Go example, set the following environment variables:
Copy link

Choose a reason for hiding this comment

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

shouldn't this all be part of README.md instead ?

export API_URL=\"http://localhost:4000\" # your Bitwarden API URL
export IDENTITY_URL=\"http://localhost:33656\" # your Bitwarden Identity URL
export ACCESS_TOKEN=\"your-access-token\" # your Bitwarden access token
export STATE_PATH=\"your-absolute-path\" # the absolute path to your state file
export ORGANIZATION_ID=\"your-org-id\" # your Bitwarden organization ID
export PROJECT_NAME=\"your-project-name\" # an arbitrary project name
"

printf '%s\n' "Then, run the example with:
pushd $REPO_ROOT/languages/go/example
Copy link

Choose a reason for hiding this comment

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

cd will be more familiar command to the users, even if it means changing directories.

go mod tidy
go run example.go
popd
"