Upgrade IronRDP Dependency #59661
Upgrade IronRDP Dependency #59661
Conversation
rhammonds-teleport
force-pushed
the
rhammonds/desktop-latest-ironrdp
branch
from
2762b73 to
e09c1ec
Compare
Makefile
Outdated
| .PHONY: rdpclient | ||
| rdpclient: | ||
| ifeq ("$(with_rdpclient)", "yes") | ||
| @rustup -q override unset |
There was a problem hiding this comment.
Previous revisions of the Makefile used rustup override set during the build process. These directory level overrides are evaluated before our toolchain file. Since these overrides are set for toolchain 1.81.0, they'll also see compile errors. Clearing the override will fix that.
There was a problem hiding this comment.
Can't we tell people to undo that manually, so we can actually use the override feature for local development without having to modify the makefile?
There was a problem hiding this comment.
It's definitely not ideal, and the same thought crossed my mind, but... I'm thinking this choice is the lesser of two inconveniences. Without clearing the override, this change is going to break local builds for most (all?) Teleport devs once they pull it in. It's easy to fix, but it'll still be frustrating for the majority of devs (who aren't concerned with our Rust projects and just want their build to succeed). I would rather they have a seamless upgrade experience and instead shift any inconvenience the smaller group of devs who work in Rust.
I'm open to removing it. Just wanted to share my thoughts.
There was a problem hiding this comment.
Especially if we backport this PR (or at least the "leave the global overrides alone" part) it's going to be a one time thing in response to an unmissable build error, so my vote is to get rid of the automatic (and hidden!) rustup override unset. Perhaps we can add a message if cargo build fails suggesting to try the command by hand?
There was a problem hiding this comment.
Fair enough. I added a pre-req to the rdp client and wasm targets that inspects the active toolchain and prints a big red warning if it doesn't match the version defined in the toolchain file. The message explains how to clear directory overrides as well.
| build-ironrdp-wasm: ensure-wasm-deps | ||
| cargo build --package ironrdp --lib --target $(CARGO_WASM_TARGET) --release | ||
| @rustup -q override unset | ||
| RUSTFLAGS='--cfg getrandom_backend="wasm_js"' cargo build --package ironrdp --lib --target $(CARGO_WASM_TARGET) --release |
There was a problem hiding this comment.
New rustflag required by getrandom for wasm targets.
https://docs.rs/getrandom/latest/getrandom/#opt-in-backends
rhammonds-teleport
force-pushed
the
rhammonds/desktop-latest-ironrdp
branch
3 times, most recently
from
b05ddab to
b479632
Compare
rhammonds-teleport
force-pushed
the
rhammonds/desktop-latest-ironrdp
branch
2 times, most recently
from
0e18531 to
fff9551
Compare
rhammonds-teleport
force-pushed
the
rhammonds/desktop-latest-ironrdp
branch
from
3eec4d0 to
ee9abe3
Compare
rhammonds-teleport
force-pushed
the
rhammonds/desktop-latest-ironrdp
branch
from
ee9abe3 to
8e1f4f2
Compare
rhammonds-teleport
requested review from
espadolini,
fspmarshall,
probakowski,
r0mant,
rosstimothy,
russjones and
zmb3
as code owners
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
github-actions
bot
requested review from
aadc-dev,
danielashare,
flyinghermit and
kimlisa
public-teleport-github-review-bot
bot
removed request for
aadc-dev,
danielashare,
flyinghermit and
kimlisa
rhammonds-teleport
force-pushed
the
rhammonds/desktop-latest-ironrdp
branch
from
ab2c3d1 to
fd4df2d
Compare
* resolve conflicting 'getrandom' dependencies. * Add necessary RUSTFLAG env var to ironrdp-wasm compilation. Needed by 'getrandom' dependency to enable its wasm_js feature. * Configure client to use remotefx codec. * Bump rust toolchain version to 1.86.0. * Unset existing rustup overrides before building rdpclient or ironrdp wasm module. * Fix lint warnings after Rust toolchain upgrade.
…ere revealed after updating IronRDP
…g if it does not match the version defined in our toolchain file.
rhammonds-teleport
force-pushed
the
rhammonds/desktop-latest-ironrdp
branch
from
fd4df2d to
fd9b839
Compare
* * Update to latest commit of IronRDP. * resolve conflicting 'getrandom' dependencies. * Add necessary RUSTFLAG env var to ironrdp-wasm compilation. Needed by 'getrandom' dependency to enable its wasm_js feature. * Configure client to use remotefx codec. * Bump rust toolchain version to 1.86.0. * Unset existing rustup overrides before building rdpclient or ironrdp wasm module. * Fix lint warnings after Rust toolchain upgrade. * Simplify BitmapFrame rendering and fix some rendering glitches that were revealed after updating IronRDP * fix typo * Try to determine the active Rust toolchain and print a helpful warning if it does not match the version defined in our toolchain file.
* * Update to latest commit of IronRDP. * resolve conflicting 'getrandom' dependencies. * Add necessary RUSTFLAG env var to ironrdp-wasm compilation. Needed by 'getrandom' dependency to enable its wasm_js feature. * Configure client to use remotefx codec. * Bump rust toolchain version to 1.86.0. * Unset existing rustup overrides before building rdpclient or ironrdp wasm module. * Fix lint warnings after Rust toolchain upgrade. * Simplify BitmapFrame rendering and fix some rendering glitches that were revealed after updating IronRDP * fix typo * Try to determine the active Rust toolchain and print a helpful warning if it does not match the version defined in our toolchain file.
Upgrade our IronRDP dependencies to the latest commit of IronRDP. In addition to general maintenance, this brings in a few fixes and features contributed by the desktop access team that we would like to take advantage of:
This PR also moves us to rust 1.86.0.
closes #54303
changelog: Fixed an issue causing desktop sessions abruptly terminate with "PDU error" message when opening certain applications that make use of hardware security keys.