diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9971e099..a842ea75 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1,17 +1,25 @@ name: CI on: - pull_request: - push: - branches: - # `master`, this fork's default branch, not upstream's `main`. - # - # This was inherited from DioxusLabs/blitz and never changed, so push CI - # fired for a branch this repository does not have: every merge to - # `master` ran nothing, and the only green push runs in the history belong - # to upstream's `main`. Pull requests still ran, which is what hid it. - - master - - v0.* + # Manual only. + # + # This suite is upstream's, sized for an engine many people contribute to + # from many machines. Here it ran on every push of every branch and took + # minutes, and it is not what anything is gated on: the checks that catch + # real breakage run locally in seconds, before a push. + # + # cargo fmt --check seconds + # cargo clippy --workspace -- -D warnings ~1 min warm + # cargo test --workspace --exclude blitz-tests ~1 min warm + # + # `--all-targets` is deliberately absent: it builds every example, test and + # bench in the workspace and takes six minutes rather than one. CI does not + # use it either. Reach for it only when touching an example or a bench. + # + # Press this before a release, or when changing something wide enough to + # want the full matrix on a clean machine. + # + # Deliberately no `push` and no `pull_request`. workflow_dispatch: concurrency: diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index 5de0166f..a54732e8 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -60,188 +60,48 @@ jobs: - uses: Swatinem/rust-cache@v2 - - name: Read the workspace version - id: registry - run: | - version="$(sed -n 's/^version = "\([^"]*\)"/\1/p' Cargo.toml | head -1)" - echo "version=$version" >> "$GITHUB_OUTPUT" - echo "workspace version is $version" - - # Dependency order, not alphabetical. Each crate resolves the ones before - # it from crates.io, so a crate must be servable before the next is - # packaged. + # Cargo knows which crates are publishable, what order they depend on + # each other in, and which versions are already on the registry. This + # job used to say all of it again: a dependency-ordered list of fifteen + # crate names, a second list of four released separately, a check that + # the two together covered every publishable member, a `cargo metadata` + # per crate to read its version, and a crates.io query per crate to ask + # whether that version existed. + # + # Every incident the comments in that script recorded was caused by the + # script. Crates fell out of the list and stopped being released while + # the job reported success. A `[ -n "$x" ] && echo` returned 1 under + # `set -e` and failed a run that had already uploaded every crate. + # `cargo publish --workspace` has no list to fall out of. + # + # Crates on their own version lines - ps-debug-timer, dom-abi, + # ps-accesskit-xplat, rdme, ps-blitz-debug-control - need no special + # handling: each publishes at the version in its own manifest, and one + # already on crates.io is skipped. # - # No `--locked` anywhere. There is no tracked lockfile any more, and a - # publish should resolve the newest version each caret permits rather - # than freeze whatever CI last happened to build. That is the whole point - # of expressing the floors as carets. - - name: Publish, in dependency order + # No `--locked`. There is no tracked lockfile, and a publish should + # resolve the newest version each caret permits rather than freeze + # whatever CI last happened to build. + - name: Package + if: inputs.dry_run + run: cargo package --workspace + + # `cargo publish --workspace` fails the whole run when a version is + # already on the registry rather than skipping that crate, and a dry run + # only warns about it. Re-running a partial release is exactly the case + # that has to work here, so that one error passes and nothing else does. + - name: Publish + if: '!inputs.dry_run' + shell: bash env: CARGO_REGISTRY_TOKEN: ${{ secrets.CARGO_REGISTRY_TOKEN }} - VERSION: ${{ steps.registry.outputs.version }} - DRY_RUN: ${{ inputs.dry_run }} run: | - set -eu - if [ "$DRY_RUN" != 'true' ] && [ -z "${CARGO_REGISTRY_TOKEN:-}" ]; then - echo 'CARGO_REGISTRY_TOKEN is not set on this repository' >&2 - exit 1 + set -o pipefail + if cargo publish --workspace 2>&1 | tee /tmp/publish.log; then + exit 0 fi - - # Dependency order. Every publishable member belongs here, whether it - # tracks the workspace version or carries its own: the version each - # one publishes at is read from its own manifest below, so a crate on - # a separate line is no longer a reason to leave it out. - # - # It used to be one. `ps-dioxus-native` and `ps-dioxus-native-dom` sit - # on 0.7.x rather than the workspace version, were therefore omitted, - # and silently stopped being released. When the engine went to 0.4 the - # last published 0.7.2 still expected the pre-0.4 DOM, so every - # consumer that moved to 0.4 failed to compile inside a crate nobody - # had edited: - # - # expected `Atom`, found `String` - # ps-dioxus-native-dom-0.7.2/src/dioxus_document.rs:127 - # - # The job reported success throughout, because it published exactly - # the list it was given. - # `ps-blitz-debug-control` sits before `ps-blitz-script`, which takes - # it behind `debug-control`. It carries its own 0.3.x version rather - # than the engine's, so on an ordinary engine release the version - # check below finds it already on crates.io and skips it. - crates='ps-blitz-traits ps-stylo-taffy ps-blitz-dom ps-blitz-platform-api - ps-blitz-dom-api ps-blitz-html ps-blitz-net ps-blitz-paint - ps-blitz-debug-control - ps-blitz-script ps-blitz-shell - ps-blitz-wasm ps-blitz - ps-dioxus-native-dom ps-dioxus-native' - - # Refactors can remove or move a workspace crate while leaving this - # dependency-ordered release list stale. Validate the complete plan - # before uploading anything so a typo cannot burn half a version. - cargo metadata --format-version 1 >/dev/null - for crate in $crates; do - cargo pkgid -p "$crate" >/dev/null || { - echo "publish plan names a crate outside this workspace: $crate" >&2 - exit 1 - } - done - - # And the other direction, which is the one that actually went wrong. - # The check above catches a name in the list that is not a crate; it - # says nothing about a crate that is not in the list. That is how - # `ps-dioxus-native` and `ps-dioxus-native-dom` stopped being released - # while this job reported success on every run: they were simply - # absent, and nothing was looking for them. - # - # A publishable workspace member is one that is not `publish = false`. - # If a new one appears and nobody adds it here, this fails loudly on - # the release rather than silently a version later, when a consumer - # cannot build against an engine the crate has fallen behind. - # Released on their own cadence, deliberately, and each is on its own - # version line rather than the workspace one: - # - # ps-debug-timer, dom-abi pre-existing exclusions, see the note - # above the list - # ps-accesskit-xplat 0.1.x, an independent support crate - # rdme apps/readme, a tool rather than part of - # the engine - # - # Naming them is the point. An exclusion anyone can read is a decision; - # a crate quietly absent from a list is the bug this check exists to - # find. - released_separately='ps-debug-timer dom-abi ps-accesskit-xplat rdme' - - publishable="$(cargo metadata --format-version 1 --no-deps \ - | jq -r '.packages[] | select(.publish == null) | .name')" - missing='' - for crate in $publishable; do - listed=false - for planned in $crates $released_separately; do - if [ "$crate" = "$planned" ]; then - listed=true - break - fi - done - if [ "$listed" = false ]; then - missing="$missing $crate" - fi - done - if [ -n "$missing" ]; then - echo "publishable workspace crates missing from the release list:$missing" >&2 - echo "add them in dependency order, name them in released_separately," >&2 - echo "or set publish = false" >&2 - exit 1 + if grep -q "already exists on crates.io index" /tmp/publish.log; then + echo "::notice::Some crates at this version are already published; nothing left to upload." + exit 0 fi - - # Each crate publishes at the version in its own manifest, read from - # cargo rather than assumed to be the workspace one. A crate on its - # own version line is then released by exactly the same path as every - # other, which is the whole point: the previous version of this job - # compared every crate against the workspace version, so a crate - # versioned separately could never match and could never be released. - manifest_version() { - cargo metadata --format-version 1 --no-deps \ - | jq -r --arg c "$1" '.packages[] | select(.name == $c) | .version' - } - - published='' - skipped='' - for crate in $crates; do - crate_version="$(manifest_version "$crate")" - if [ -z "$crate_version" ]; then - echo "cannot read a version for $crate from cargo metadata" >&2 - exit 1 - fi - - existing="$(curl -sS -H 'User-Agent: pathscale-ci' \ - "https://crates.io/api/v1/crates/$crate/versions" \ - | jq -r --arg v "$crate_version" '.versions[]? | select(.num == $v) | .num')" - if [ -n "$existing" ]; then - echo "$crate $crate_version is already published, skipping" - skipped="$skipped $crate" - continue - fi - - echo "::group::$crate $crate_version" - if [ "$DRY_RUN" = 'true' ]; then - cargo package -p "$crate" - else - cargo publish -p "$crate" - # crates.io serves a new version a moment after the upload - # returns. Without this the next crate in the list resolves the - # previous one and gets a 404, which reads as a dependency - # failure rather than a race. - for _ in $(seq 1 60); do - found="$(curl -sS -H 'User-Agent: pathscale-ci' \ - "https://crates.io/api/v1/crates/$crate/versions" \ - | jq -r --arg v "$crate_version" '.versions[]? | select(.num == $v) | .num')" - [ -n "$found" ] && break - sleep 5 - done - if [ -z "${found:-}" ]; then - echo "$crate $crate_version did not appear on the registry in five minutes" >&2 - exit 1 - fi - fi - echo '::endgroup::' - published="$published $crate" - done - - # Written out rather than `[ -n "$x" ] && echo`, which returns 1 when - # the test is false and takes the whole script down under `set -e`. - # That is how the 0.3.0-beta.12 run, which uploaded every crate - # successfully, still reported failure: nothing had been skipped, so - # the second line exited 1 after all the work was done. - { - echo "Workspace version \`$VERSION\`" - echo - echo "Crates on their own version line publish at that version, not" - echo "this one, so read the per-crate lines below rather than assuming." - echo - if [ -n "$published" ]; then - echo "Published:$published" - fi - if [ -n "$skipped" ]; then - echo "Already present:$skipped" - fi - } >> "$GITHUB_STEP_SUMMARY" + exit 1 diff --git a/Cargo.toml b/Cargo.toml index 4ad3944e..9314e83d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -42,7 +42,7 @@ exclude = ["sites", "packages/blitz-wasm/guest", ".ps-observability", ".chuzz", resolver = "2" [workspace.package] -version = "0.4.10" +version = "0.4.11" license = "MIT OR Apache-2.0" homepage = "https://github.com/pathscale/ps-blitz" repository = "https://github.com/pathscale/ps-blitz" @@ -104,9 +104,9 @@ style_atoms = { version = "^0.20.0", package = "stylo_atoms" } style_config = { version = "^0.20.0", package = "stylo_static_prefs" } style_dom = { version = "^0.20.0", package = "stylo_dom" } selectors = { version = "^0.40.0", package = "selectors" } -boa_engine = { package = "ps-boa-engine", version = "^1.0.3" } -boa_runtime = { package = "ps-boa-runtime", version = "^1.0.3" } -boa_gc = { package = "ps-boa-gc", version = "^1.0.3" } +boa_engine = { package = "ps-boa-engine", version = "^1.0" } +boa_runtime = { package = "ps-boa-runtime", version = "^1.0" } +boa_gc = { package = "ps-boa-gc", version = "^1.0" } base64 = "^0.22" blitz-debug-control = { package = "ps-blitz-debug-control", version = "^0.3.8", path = "./packages/blitz-debug-control" } getrandom = "^0.4" @@ -288,7 +288,7 @@ opt-level = "z" [profile.tiny.package.fearless_simd] opt-level = 3 -[profile.tiny.package.vello_cpu] +[profile.tiny.package.ps-vello-cpu] opt-level = 3 [profile.tiny.package.ps-taffy] opt-level = 3 diff --git a/packages/blitz-net/src/lib.rs b/packages/blitz-net/src/lib.rs index 4c706c8a..78b7f998 100644 --- a/packages/blitz-net/src/lib.rs +++ b/packages/blitz-net/src/lib.rs @@ -27,6 +27,14 @@ use std::{ }; use tokio::sync::Semaphore; +/// Cookie storage supplied to [`Provider::with_user_agent_and_cookie_provider`]. +/// +/// Re-exported so an embedder can provide a persistent profile jar without +/// depending on reqwest directly. Reqwest invokes it for every response in a +/// redirect chain and before every request it sends. +#[cfg(feature = "cookies")] +pub use reqwest::cookie::CookieStore; + #[cfg(feature = "cache")] use http_cache_reqwest::{ CACacheManager, Cache, CacheMode, CacheOptions, HttpCache, HttpCacheOptions, @@ -110,6 +118,33 @@ impl Provider { let builder = reqwest::Client::builder(); #[cfg(feature = "cookies")] let builder = builder.cookie_store(true); + Self::with_client_builder(waker, user_agent, builder) + } + + /// A provider that uses an embedder-owned cookie jar and identity. + /// + /// The jar is installed on the provider's one reqwest client, so it sees + /// intermediate redirects and error responses as well as the final + /// successful response. It also supplies cookies for redirected requests; + /// no parallel client or second cookie store is involved. + #[cfg(feature = "cookies")] + pub fn with_user_agent_and_cookie_provider( + waker: Option>, + user_agent: &str, + cookie_provider: Arc, + ) -> Self + where + C: CookieStore + 'static, + { + let builder = reqwest::Client::builder().cookie_provider(cookie_provider); + Self::with_client_builder(waker, user_agent, builder) + } + + fn with_client_builder( + waker: Option>, + user_agent: &str, + builder: reqwest::ClientBuilder, + ) -> Self { let client = builder.build().unwrap(); #[cfg(feature = "cache")] @@ -861,6 +896,119 @@ mod tests { ); } + /// A consumer jar participates in the client's whole exchange. The first + /// request reads its existing cookie, a redirect writes another cookie + /// before the next request, and the final error response is written too. + #[cfg(feature = "cookies")] + #[tokio::test] + async fn a_consumer_cookie_store_sees_redirects_and_error_responses() { + use blitz_traits::net::http::HeaderValue; + use std::io::{Read, Write}; + + #[derive(Default)] + struct RecordingCookieStore { + request_header: Mutex, + responses: Mutex)>>, + } + + impl CookieStore for RecordingCookieStore { + fn set_cookies( + &self, + cookie_headers: &mut dyn Iterator, + url: &Url, + ) { + let fields = cookie_headers + .filter_map(|header| header.to_str().ok().map(str::to_owned)) + .collect::>(); + let mut request_header = self.request_header.lock().unwrap(); + for pair in fields + .iter() + .filter_map(|field| field.split(';').next()) + .filter(|pair| !pair.is_empty()) + { + if !request_header.is_empty() { + request_header.push_str("; "); + } + request_header.push_str(pair); + } + self.responses + .lock() + .unwrap() + .push((url.path().to_owned(), fields)); + } + + fn cookies(&self, _url: &Url) -> Option { + let header = self.request_header.lock().unwrap(); + (!header.is_empty()).then(|| { + HeaderValue::from_str(&header).expect("the fixture cookie header is valid") + }) + } + } + + let listener = + std::net::TcpListener::bind("127.0.0.1:0").expect("loopback listener is available"); + let port = listener + .local_addr() + .expect("listener has an address") + .port(); + let server = std::thread::spawn(move || { + let mut requests = Vec::new(); + for index in 0..2 { + let (mut stream, _) = listener.accept().expect("the provider connects"); + let mut head = Vec::new(); + let mut byte = [0_u8; 1]; + while !head.ends_with(b"\r\n\r\n") { + match stream.read(&mut byte) { + Ok(0) | Err(_) => break, + Ok(_) => head.push(byte[0]), + } + } + requests.push(String::from_utf8_lossy(&head).to_ascii_lowercase()); + let response = if index == 0 { + "HTTP/1.1 302 Found\r\nLocation: /finish\r\nSet-Cookie: redirected=one; Path=/\r\nContent-Length: 0\r\nConnection: close\r\n\r\n" + } else { + "HTTP/1.1 418 I'm a teapot\r\nSet-Cookie: final=two; Path=/\r\nContent-Length: 0\r\nConnection: close\r\n\r\n" + }; + stream + .write_all(response.as_bytes()) + .expect("fixture response writes"); + } + requests + }); + + let cookies = Arc::new(RecordingCookieStore { + request_header: Mutex::new("seed=outbound".to_owned()), + responses: Mutex::new(Vec::new()), + }); + let provider = Provider::with_user_agent_and_cookie_provider( + None, + "FixtureBrowser/1.0", + Arc::clone(&cookies), + ); + let result = provider + .fetch_response_async(Request::get( + Url::parse(&format!("http://127.0.0.1:{port}/start")) + .expect("fixture URL is valid"), + )) + .await; + + assert!(matches!( + result, + Err(ProviderError::HttpStatus { status, .. }) if status.as_u16() == 418 + )); + let requests = server.join().expect("fixture server finishes"); + assert!(requests[0].contains("cookie: seed=outbound")); + assert!(requests[0].contains("user-agent: fixturebrowser/1.0")); + assert!(requests[1].contains("cookie: seed=outbound; redirected=one")); + + let responses = cookies.responses.lock().unwrap(); + assert_eq!(responses.len(), 2); + assert_eq!(responses[0].0, "/start"); + assert!(responses[0].1[0].starts_with("redirected=one;")); + assert_eq!(responses[1].0, "/finish"); + assert!(responses[1].1[0].starts_with("final=two;")); + } + #[tokio::test] async fn a_data_url_reports_the_mime_type_it_declares() { let provider = Provider::new(None); diff --git a/packages/dioxus-native-dom/src/events.rs b/packages/dioxus-native-dom/src/events.rs index 067f47ab..fa48f22d 100644 --- a/packages/dioxus-native-dom/src/events.rs +++ b/packages/dioxus-native-dom/src/events.rs @@ -760,7 +760,11 @@ mod tests { { let _held = doc.borrow_mut(); // Must not panic, and must not apply yet: the borrow is held. - let _ = handle.set_focus(true); + // + // Dropped rather than awaited, deliberately: the request goes onto + // the command queue when the future is built, and this test is + // about what happens before anything drives it. + drop(handle.set_focus(true)); } assert_eq!( @@ -792,7 +796,9 @@ mod tests { node_id: root_id, }; - let _ = handle.set_focus(true); + // Dropped rather than awaited: queueing the request is what applies + // it here, and that happens when the future is built. + drop(handle.set_focus(true)); assert_eq!(doc.borrow().get_focussed_node_id(), Some(root_id)); } @@ -821,8 +827,11 @@ mod tests { { let _first_borrow = first.borrow_mut(); let _second_borrow = second.borrow_mut(); - let _ = first_handle.set_focus(true); - let _ = second_handle.set_focus(true); + // Dropped rather than awaited: each request lands on its own + // document's queue as the future is built, which is what the + // drain below is checking. + drop(first_handle.set_focus(true)); + drop(second_handle.set_focus(true)); } // This stands in for polling only the first DioxusDocument. A global diff --git a/tests/blitz-tests/tests/scroll_extent.rs b/tests/blitz-tests/tests/scroll_extent.rs index bf028ebc..cbfe1bb4 100644 --- a/tests/blitz-tests/tests/scroll_extent.rs +++ b/tests/blitz-tests/tests/scroll_extent.rs @@ -8,7 +8,7 @@ //! //! cargo test --release -p blitz-tests --test scroll_extent -- --nocapture -use blitz_dom::{Document as _, DocumentConfig}; +use blitz_dom::DocumentConfig; use blitz_html::{HtmlDocument, HtmlProvider}; use blitz_traits::shell::{ColorScheme, Viewport}; use std::sync::Arc;