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.

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

Move development directly into pkg/nvml instead of gen/nvml #109

Merged
merged 6 commits into from
Apr 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
*.swp
*.swo
pkg/nvml/
.cache
coverage.out*
15 changes: 9 additions & 6 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -139,12 +139,11 @@ bindings: .create-bindings .strip-autogen-comment .strip-nvml-h-linenumber
.create-bindings: $(PKG_BINDINGS_DIR)/nvml.h $(SOURCES) | $(PKG_BINDINGS_DIR)
cp $(GEN_BINDINGS_DIR)/nvml.yml $(PKG_BINDINGS_DIR)
c-for-go -out $(PKG_DIR) $(PKG_BINDINGS_DIR)/nvml.yml
Copy link
Member

Choose a reason for hiding this comment

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

Question: Instead of copying the yml file in the previous step, could we specify $(GEN_BINDINGS_DIR)/nvml.yml as input here? Then we also wouldn't need to remove it later?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When I do that I get an incorrect pkg/types_gen.so file that gets generated. It's unclear why this is the case, but the fact that it doesn't "just work" when simplifying this means that it likely warrants a new PR to investigate.

cp $(GEN_BINDINGS_DIR)/*.go $(PKG_BINDINGS_DIR)
cd $(PKG_BINDINGS_DIR); \
go tool cgo -godefs types.go > types_gen.go; \
go fmt types_gen.go; \
cd -> /dev/null
rm -rf $(PKG_BINDINGS_DIR)/nvml.yml $(PKG_BINDINGS_DIR)/types.go $(PKG_BINDINGS_DIR)/_obj
rm -rf $(PKG_BINDINGS_DIR)/nvml.yml $(PKG_BINDINGS_DIR)/cgo_helpers.go $(PKG_BINDINGS_DIR)/types.go $(PKG_BINDINGS_DIR)/_obj

.strip-autogen-comment: SED_SEARCH_STRING := // WARNING: This file has automatically been generated on
.strip-autogen-comment: SED_REPLACE_STRING := // WARNING: THIS FILE WAS AUTOMATICALLY GENERATED.
Expand All @@ -160,12 +159,16 @@ bindings: .create-bindings .strip-autogen-comment .strip-nvml-h-linenumber

test-bindings: bindings
clean-bindings:
rm -rf $(PKG_BINDINGS_DIR)
git checkout $(PKG_BINDINGS_DIR)
rm -rf $(PKG_BINDINGS_DIR)/nvml.h
rm -f $(PKG_BINDINGS_DIR)/cgo_helpers.go
rm -f $(PKG_BINDINGS_DIR)/cgo_helpers.h
rm -f $(PKG_BINDINGS_DIR)/const.go
rm -f $(PKG_BINDINGS_DIR)/doc.go
rm -f $(PKG_BINDINGS_DIR)/nvml.go
rm -f $(PKG_BINDINGS_DIR)/nvml.h
rm -f $(PKG_BINDINGS_DIR)/types_gen.go

# Update nvml.h from the Anaconda package repository
update-nvml-h: JQ ?= $(DOCKER) run -i --rm -v "$(PWD):$(PWD)" -w "$(PWD)" stedolan/jq:latest
update-nvml-h: JQ ?= $(DOCKER) run -i --rm -v "$(PWD):$(PWD)" -w "$(PWD)" backplane/jq:latest
update-nvml-h: NVML_DEV_PACKAGES_INFO := $(shell \
wget -qO - https://api.anaconda.org/package/nvidia/cuda-nvml-dev/files | \
$(JQ) '.[] | select(.attrs.subdir=="linux-64") | .version + "@" + .upload_time[:19] + "@" + .full_name' | \
Expand Down
20 changes: 10 additions & 10 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,7 @@ symbols include the following (as defined in `nvml.h`):
The actual versions that these API calls are assigned to will depend on the
version of the NVIDIA driver (and hence the version of `libnvidia-ml.so` that
you have linked in). These updates happen in the `updateVersionedSymbols()`
function of `gen/nvml/lib.go` as seen below.
function of `pkg/nvml/lib.go` as seen below.

```go
// Default all versioned APIs to v1 (to infer the types)
Expand Down Expand Up @@ -366,8 +366,8 @@ The files below define a set of "glue" code between the auto-generated bindings
from `c-for-go` and the manual wrappers providing a more user-friendly API to
the end user.

- `gen/nvml/cgo_helpers.go`
- `gen/nvml/return.go`
- `pkg/nvml/cgo_helpers_atatic.go`
- `pkg/nvml/return.go`

The `cgo_helpers.go` file defines functions that help in dealing with the types
coming out of the C API and turning them into more usable Go types. It is
Expand All @@ -391,12 +391,12 @@ bindings from `c-for-go`. Only these manual wrappers are expected as part of
the API for the package -- the auto-generated bindings are only available for
internal use.

- `gen/nvml/init.go`
- `gen/nvml/system.go`
- `gen/nvml/event_set.go`
- `gen/nvml/vgpu.go`
- `gen/nvml/unit.go`
- `gen/nvml/device.go`
- `pkg/nvml/init.go`
- `pkg/nvml/system.go`
- `pkg/nvml/event_set.go`
- `pkg/nvml/vgpu.go`
- `pkg/nvml/unit.go`
- `pkg/nvml/device.go`

These wrappers add boiler-plate code around the auto-generated bindings so that
the end-user doesn't have to do this themselves every time a call is made.
Expand Down Expand Up @@ -475,7 +475,7 @@ should be straightforward so long as we keep good pace with each new release.

At present, all test code is under the following file:

- `gen/nvml/nvml_test.go`
- `pkg/nvml/nvml_test.go`

The test coverage is fairly sparse and could be greatly improved.

Expand Down
37 changes: 0 additions & 37 deletions gen/nvml/api.go

This file was deleted.

Loading