Bug 220418

Created attachment 417182 [details] Patch

Built and run test tests normally. Just had an issue with gst-build unrelated to this and already present in the current tree.

Committed r271518: <https://trac.webkit.org/changeset/271518>

<rdar://problem/73246162>

Comment on attachment 417182 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=417182&action=review > Tools/buildstream/elements/sdk/cargo-c.bst:17 > + cargo --offline build --release Wouldn't the cargo invocation belong into “build-commands”? Something like: config: build-commands: - | cargo --offline build --release install-commands: - | install -D -m a+rx -t "%{install-root}%{bindir}" ./target/release/cargo-cbuild This allows Buildstream to better handle caching of build artifacts. For example it may choose to reuse build artifacts in a subsequent run and only run the installation phase if that was interrupted for some reason (IIRC there were other cases in which install may re-run without full build as well.) The same goes for the other .bst files which are invoking cargo in the installation commands :)

(In reply to Adrian Perez from comment #5) > Comment on attachment 417182 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=417182&action=review > > > Tools/buildstream/elements/sdk/cargo-c.bst:17 > > + cargo --offline build --release > > Wouldn't the cargo invocation belong into “build-commands”? Something like: > > config: > build-commands: > - | > cargo --offline build --release > install-commands: > - | > install -D -m a+rx -t "%{install-root}%{bindir}" > ./target/release/cargo-cbuild > > This allows Buildstream to better handle caching of build artifacts. For > example it may > choose to reuse build artifacts in a subsequent run and only run the > installation phase > if that was interrupted for some reason (IIRC there were other cases in > which install > may re-run without full build as well.) > > The same goes for the other .bst files which are invoking cargo in the > installation > commands :) Yes, good point. I'll follow-up as I mistakenly landed this too early. Sorry ;)

(In reply to Philippe Normand from comment #6) > (In reply to Adrian Perez from comment #5) > > Comment on attachment 417182 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=417182&action=review > > > > > Tools/buildstream/elements/sdk/cargo-c.bst:17 > > > + cargo --offline build --release > > > > Wouldn't the cargo invocation belong into “build-commands”? Something like: > > > > config: > > build-commands: > > - | > > cargo --offline build --release > > install-commands: > > - | > > install -D -m a+rx -t "%{install-root}%{bindir}" > > ./target/release/cargo-cbuild > > > > This allows Buildstream to better handle caching of build artifacts. For > > example it may > > choose to reuse build artifacts in a subsequent run and only run the > > installation phase > > if that was interrupted for some reason (IIRC there were other cases in > > which install > > may re-run without full build as well.) > > > > The same goes for the other .bst files which are invoking cargo in the > > installation > > commands :) > > Yes, good point. I'll follow-up as I mistakenly landed this too early. Sorry > ;) No problem at all!