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

Move packaged buildpack directory out of target/ #583

Merged
merged 6 commits into from
Aug 18, 2023
Merged

Move packaged buildpack directory out of target/ #583

merged 6 commits into from
Aug 18, 2023

Conversation

Malax
Copy link
Member

@Malax Malax commented Jun 29, 2023 •
edited

cargo libcnb package currently puts the packaged buildpacks in Cargo's target directory. This requires a deep path to avoid clashing with Cargo itself. Modifiying the target directory might also interfere with other Rust tooling that doesn't expect non-Cargo files in that directory. One case is caching via https://github.com/Swatinem/rust-cache.

This PR moves the packaged buildpacks into a dedicated directory. The default name and location for that directory is $WORKSPACE_ROOT/libcnb-packaged. This can be changed via the new --package-dir option of cargo libcnb package.

See: #580 (comment)

Closes GUS-W-13813150

@Malax Malax force-pushed the malax/dist-directory branch 2 times, most recently from 6f36d7b to a01466d Compare June 29, 2023 10:28
Malax
Malax commented Jun 29, 2023
View reviewed changes
libcnb-cargo/src/cli.rs Outdated Show resolved Hide resolved
@colincasey colincasey mentioned this pull request Jul 19, 2023
Closed
@colincasey colincasey mentioned this pull request Jul 26, 2023
Merged
@Malax Malax mentioned this pull request Aug 17, 2023
Merged
@Malax Malax changed the base branch from main to malax/refactor-libcnb-cargo-tests August 17, 2023 15:38
@Malax Malax marked this pull request as ready for review August 17, 2023 15:55
@Malax Malax requested a review from a team as a code owner August 17, 2023 15:55
@colincasey
Copy link
Contributor

@Malax there is some overlap with #632 here. can we resolve this PR after that one or would you prefer this one first?

@Malax
Copy link
Member Author

Malax commented Aug 17, 2023

@colincasey I understand. The thing is that we want to cut a release and get this breaking change in since the new release will have breaking changes already. If we postpone we have two releases with breaking changes. Happy to help out getting #632 back on track after this PR is merged - is that good enough for you?

Base automatically changed from malax/refactor-libcnb-cargo-tests to main August 17, 2023 16:27
@colincasey
Copy link
Contributor

maybe we should also prioritize #631, #632, #633 now then? all those changes to support composite buildpack testing will mean we'll still have two releases with breaking changes, right?

edmorley
edmorley reviewed Aug 17, 2023
View reviewed changes
CHANGELOG.md Outdated Show resolved Hide resolved
libcnb-cargo/src/cli.rs Show resolved Hide resolved
libcnb-package/src/lib.rs Show resolved Hide resolved
libcnb-cargo/tests/test.rs Outdated Show resolved Hide resolved
@edmorley
Copy link
Member

all those changes to support composite buildpack testing will mean we'll still have two releases with breaking changes, right?

Manuel was referring to breaking changes to the output directory of cargo libcnb package.

There is already one breaking change to that path (as of #580), which will mean needing the adjust the hardcoded paths in:
https://github.com/heroku/languages-github-actions

If we release libcnb 0.14.0 without this PR, then once libcnb 0.15.0 is released, we'll have to update the hardcoded paths in languages-github-actions a second time.

edmorley
edmorley reviewed Aug 17, 2023
View reviewed changes
.gitignore Outdated Show resolved Hide resolved
@edmorley edmorley changed the title Move packaged buildpack directory out of target Move packaged buildpack directory out of target/ Aug 17, 2023
@Malax Malax requested a review from edmorley August 18, 2023 08:39
@Malax Malax merged commit ec90a43 into main Aug 18, 2023
4 checks passed
@Malax Malax deleted the malax/dist-directory branch August 18, 2023 08:44
@Malax Malax mentioned this pull request Aug 18, 2023
Merged
colincasey added a commit that referenced this pull request Aug 18, 2023
@colincasey
Merge branch 'main' into libcnb-package/find_cargo_workspace
2a2b0dd 
* main:
  Bump buildpacks/github-actions from 5.3.1 to 5.4.0 (#647)
  Prepare release v0.14.0 (#646)
  Pin intra-libcnb* crate dependencies to exact versions (#644)
  Rename libcnb-cargo integration test file (#645)
  Add version links in the changelog (#643)
  Update Quick Start Guide (#640)
  Run `cargo upgrade` as part of preparing libcnb releases (#641)
  Move packaged buildpack directory out of `target/` (#583)
  Refactor libcnb-cargo integration tests (#637)
  libcnb-test: Improve error messages for `address_for_port` (#636)
  libcnb-test: Implement `fmt::Display` for `LogOutput` (#635)

# Conflicts:
#	CHANGELOG.md
#	libcnb-cargo/src/package/command.rs
colincasey added a commit that referenced this pull request Aug 18, 2023
@colincasey
Merge branch 'main' into libcnb-package/buildpack_output_directory_lo…
8e8483e 
…cator

* main:
  Bump buildpacks/github-actions from 5.3.1 to 5.4.0 (#647)
  Prepare release v0.14.0 (#646)
  Pin intra-libcnb* crate dependencies to exact versions (#644)
  Rename libcnb-cargo integration test file (#645)
  Add version links in the changelog (#643)
  Update Quick Start Guide (#640)
  Run `cargo upgrade` as part of preparing libcnb releases (#641)
  Move packaged buildpack directory out of `target/` (#583)
  Refactor libcnb-cargo integration tests (#637)
  libcnb-test: Improve error messages for `address_for_port` (#636)
  libcnb-test: Implement `fmt::Display` for `LogOutput` (#635)

# Conflicts:
#	CHANGELOG.md
#	libcnb-cargo/src/package/command.rs
#	libcnb-cargo/src/package/error.rs
#	libcnb-cargo/tests/test.rs
#	libcnb-package/src/lib.rs
colincasey added a commit that referenced this pull request Aug 18, 2023
@colincasey
Merge branch 'libcnb-package/buildpack_output_directory_locator' into…
0ce28f5 
… libcnb-package/assembling_buildpack_directories

* libcnb-package/buildpack_output_directory_locator:
  Bump buildpacks/github-actions from 5.3.1 to 5.4.0 (#647)
  Prepare release v0.14.0 (#646)
  Pin intra-libcnb* crate dependencies to exact versions (#644)
  Rename libcnb-cargo integration test file (#645)
  Add version links in the changelog (#643)
  Update Quick Start Guide (#640)
  Run `cargo upgrade` as part of preparing libcnb releases (#641)
  Move packaged buildpack directory out of `target/` (#583)
  Refactor libcnb-cargo integration tests (#637)
  libcnb-test: Improve error messages for `address_for_port` (#636)
  libcnb-test: Implement `fmt::Display` for `LogOutput` (#635)

# Conflicts:
#	CHANGELOG.md
#	libcnb-cargo/src/package/command.rs
#	libcnb-package/src/output.rs
@edmorley edmorley mentioned this pull request Aug 19, 2023
Merged
edmorley added a commit that referenced this pull request Aug 21, 2023
@edmorley
libcnb-test: Rename test to match new function name (#648)
65a41fc 
In #583 the function `get_buildpack_target_dir` was renamed to
`get_buildpack_package_dir`.

This renames the corresponding test to match.
colincasey added a commit that referenced this pull request Sep 13, 2023
@colincasey
Ignore file support when finding buildpacks
d149b15 
Previously we could rely on knowing that packaged buildpacks would be written to a location within the `target` directory configured in Crate. This was easy to determine through program code and could be supplied to `find_buildpack_dirs` so that packaged buildpacks weren't accidentally included when searching for buildpacks.

With the changes introduced in [#583](#583) it is no longer possible to easily determine the output location for packaged buildpacks. This PR adds the [ignore](https://crates.io/crates/ignore) crate to provide directory iteration that respects standard ignore files. This will give the user the ability to configure where we search within a project for buildpacks.
@colincasey colincasey mentioned this pull request Sep 13, 2023
Merged
colincasey added a commit that referenced this pull request Sep 19, 2023
@colincasey
Ignore file support when finding buildpacks (#673)
31ec5af 
* Ignore file support when finding buildpacks

Previously we could rely on knowing that packaged buildpacks would be written to a location within the `target` directory configured in Crate. This was easy to determine through program code and could be supplied to `find_buildpack_dirs` so that packaged buildpacks weren't accidentally included when searching for buildpacks.

With the changes introduced in [#583](#583) it is no longer possible to easily determine the output location for packaged buildpacks. This PR adds the [ignore](https://crates.io/crates/ignore) crate to provide directory iteration that respects standard ignore files. This will give the user the ability to configure where we search within a project for buildpacks.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@colincasey colincasey colincasey left review comments

@edmorley edmorley edmorley approved these changes

Assignees

@Malax Malax

Labels
breaking change libcnb-cargo libcnb-package
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@Malax @colincasey @edmorley