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

Loosen dependency contraints #1963

Open
wants to merge 9 commits into
base: dev
Choose a base branch
from
Open

Loosen dependency contraints #1963

wants to merge 9 commits into from

Conversation

n8henrie
Copy link
Contributor

@n8henrie n8henrie commented May 4, 2024

Loosen dependency contraints to allow API-compatible dependency updates, update
Cargo.lock accordingly

Update Cargo.lock with new constraints. This should allow several crates
to update and make it easier for use to update major versions of several
dependencies.

See also: espanso#1825
@n8henrie
Copy link
Contributor Author

n8henrie commented May 4, 2024

There are several stragglers I can add to this PR as well, assuming the tests pass (locally was hanging on tests::ipc_multiple_clients).

@AucaCoyan
Copy link
Member

Great! This looks promising. I merged the clippy fixes and skipped that test it's causing you problems. In my machine took longer than a couple of minutes (4-5?), so probably something is going south when cargo tries to run that specific test.
Try merging dev again and see if you have better results 💯

@n8henrie
Copy link
Contributor Author

n8henrie commented May 5, 2024

I should have mentioned but I am able to recreate Cargo.lock without any trouble here (#1953). Just rm Cargo.lock, cargo clean, then cargo build and Bob's your uncle.

@AucaCoyan
Copy link
Member

Oh (click soud) nice!
CI is acting weird, I think every machine is downloading different versions of the crates, and that collides with the code we were using (chrono, for example).
Is it possible to you to make the CI green? Watch out this line. You can delete that if it's causing trouble, and then we re-add it (compile times lasts aeons)

@AucaCoyan
Copy link
Member

Hi! Just as another comment, you can check my pr trying this same steps of updating chrono and the like here #1872

@n8henrie
Copy link
Contributor Author

n8henrie commented May 5, 2024

Looks like most of the failures were just a clippy lint. Not sure what's up with Windows though.

@AucaCoyan
Copy link
Member

...
 Compiling espanso v2.2.1 (D:\a\espanso\espanso\espanso)
error: linking with `link.exe` failed: exit code: 1169
  |
  = note: "C:\\Program Files\\Microsoft Visual Studio\\2022\\Enterprise\\VC\\Tools\\MSVC\\14.39.33519\\bin\\HostX64\\x64\\link.exe" "/NOLOGO" 
...
   = note: wxmsw31u_core.lib(corelib_gdiplus.obj) : error LNK2005: GdipAlloc already defined in windows.0.52.0.lib(gdiplus.dll)
          wxmsw31u_core.lib(corelib_gdiplus.obj) : error LNK2005: GdipCloneImage already defined in windows.0.52.0.lib(gdiplus.dll)
          wxmsw31u_core.lib(corelib_gdiplus.obj) : error LNK2005: GdipCreateBitmapFromFile already defined in windows.0.52.0.lib(gdiplus.dll)
          wxmsw31u_core.lib(corelib_gdiplus.obj) : error LNK2005: GdipCreateBitmapFromFileICM already defined in windows.0.52.0.lib(gdiplus.dll)
          wxmsw31u_core.lib(corelib_gdiplus.obj) : error LNK2005: GdipCreateHBITMAPFromBitmap already defined in windows.0.52.0.lib(gdiplus.dll)
          wxmsw31u_core.lib(corelib_gdiplus.obj) : error LNK2005: GdipDisposeImage already defined in windows.0.52.0.lib(gdiplus.dll)
          wxmsw31u_core.lib(corelib_gdiplus.obj) : error LNK2005: GdipFree already defined in windows.0.52.0.lib(gdiplus.dll)
          wxmsw31u_core.lib(corelib_gdiplus.obj) : error LNK2005: GdiplusShutdown already defined in windows.0.52.0.lib(gdiplus.dll)
          wxmsw31u_core.lib(corelib_gdiplus.obj) : error LNK2005: GdiplusStartup already defined in windows.0.52.0.lib(gdiplus.dll)
          D:\a\espanso\espanso\target\debug\deps\espanso.exe : fatal error LNK1169: one or more multiply defined symbols found
          

error: could not compile `espanso` (bin "espanso") due to 1 previous error

Oh damn. I hitted this error many times, and I don't have any idea why it happens. The log says that it can't link the librearies, or it doesn't find the linker (link.exe). I mildly assume that is something related to the C++ code, in which I have totally null experience 😢
I asked Federico once, and he didn't had a solution for this error

@n8henrie
Copy link
Contributor Author

The find_tool function from the cc crate doesn't seem to be working in CI / GitHub Actions, at least on newer versions.

@n8henrie
Copy link
Contributor Author

cc >= 1.0.84 requires a full target (including arch): rust-lang/cc-rs#1071

@n8henrie
Copy link
Contributor Author

I hitted this error many times, and I don't have any idea why it happens.

@AucaCoyan It looks like @federico-terzi may have used the avoid-gdi to work around this issue:

$ git log --grep gdiplus
commit 04720212e571e7f1e22a4f9587a1bb9fd3d73e3a
Author: Federico Terzi <federico-terzi@users.noreply.github.com>
Date:   Fri May 21 22:00:02 2021 +0200

    feat(ui): add feature to avoid double linking gdiplus

commit ccb3e11d9329d17583a3d30b1849b9675d11f995
Author: Federico Terzi <federico-terzi@users.noreply.github.com>
Date:   Fri May 21 21:59:29 2021 +0200

    feat(clipboard): add feature to avoid double linking gdiplus

@AucaCoyan
Copy link
Member

It had never occurred to me we could grep the issue in git history. Thank you for that TIL!
I'm on the phone now, but later I would like to what changes introduced those 2 commits and if they are necessary again in these new deps.

Great work here!

@n8henrie
Copy link
Contributor Author

Just checking in -- still have this on my mind, but still unclear on how to fix the windows gdiplus linking issues.

@n8henrie
Copy link
Contributor Author

Even prior to this PR, we've known that some dependency is breaking windows if we run cargo update (even without bumping any version number in Cargo.toml: #1953

To try to flesh this out, I ran the script below, which starts from a working Cargo.lock and then:

  1. runs cargo update --dry-run and filters for any dependencies that would have been updated
  2. updates each of the deps individually (cargo update depname)
  3. makes a commit including the name of the updated dependency
  4. pushes to GitHub to try to build for windows in CI
  5. resets back to the working Cargo.lock
  6. continues with the next dependency
#!/usr/bin/env bash

set -x

main() {
  while read -r dep; do
    git reset --hard c25d419d8fd233751dfa6f97d4385afe78febb98
    cargo update "${dep}"
    git add Cargo.lock
    git commit -m "Update ${dep}"
    git push --force-with-lease
  done < <(cargo update --dry-run |& awk '/Updating/ && /->/ { print $2 }')
}
main "$@"

You can see my pages of results at: https://github.com/n8henrie/espanso/actions/workflows/ci.yml

Unfortunately it's not immediately obvious how to tie a specific workflow run back to the culprit commit, but some work with the (authenticated) gh CLI tool helps:

$ gh api --paginate  -H "Accept: application/vnd.github+json"   -H "X-GitHub-Api-Version: 2022-11-28"   /repos/n8henrie/espanso/actions/workflows/ci.yml/runs

I'll need to jq through this a bit to see if I can tease apart which ones succeeded and which broke, but hopefully will have a little more info soon.

@AucaCoyan
Copy link
Member

Thanks for the thorough explanation!

@n8henrie
Copy link
Contributor Author

I'm no pro at jq, but I was able to muck through.

I took the gh api command from above and piped its output to a file named runs.json. Its output was 2 separate json objects (first of length 100 and second of length 90), so I had to use the jq --slurp option (not sure if I did something wrong with the --paginate flag to the gh command), which wraps it into an array to allow it to be processed in the same jq filter.

Eventually I came up with this:

$ jq -s '[
    .[].workflow_runs[] |
    select(.head_commit.message | test("^Update \\w+$")) |
    {(.head_commit.id): {"conclusion": .conclusion, "message": .head_commit.message}}
    ]'
    < runs.json

which outputs something like:

[
  {
    "4063d6b8cb8dcc44fa3e3b43581243983d077450": {
      "conclusion": "success",
      "message": "Update zeroize"
    }
  },
  {
    "451eb8126e05830148223bd6d6da121d480027fe": {

where the key is the git commit:

$ git show 451eb8126e05830148223bd6d6da121d480027fe
commit 451eb8126e05830148223bd6d6da121d480027fe
Author: Nathan Henrie <nate@n8henrie.com>
Date:   Tue Jun 11 16:23:04 2024 -0600

    Update xkeysym


Cargo.lock
───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────

───────────────────────┐
3839: dependencies = [ │
───────────────────────┘

[[package]]
name = "xkeysym"
version = "0.2.0"
version = "0.2.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "054a8e68b76250b253f671d1268cb7f1ae089ec35e195b2efb2a4e9a836d0621"
checksum = "b9cc00251562a284751c9973bace760d86c0276c471b4be569fe6b068ee97a56"
dependencies = [
 "bytemuck",
]

With a minor variation, I can select for only the runs that failed:

$ jq -s '[.[].workflow_runs[] | select(.head_commit.message | test("^Update \\w+$")) | select(.conclusion == "failure")  | {(.head_commit.id): {"conclusion": .conclusion, "message": .head_commit.message}}]' < runs.json
[
  {
    "258ee2c2934fa961176d6f809a95e0545a39b1fe": {
      "conclusion": "failure",
      "message": "Update tokio"
    }
  },
  {
    "1585debaa1fee264a30d2d4f9dfc151829771f78": {
      "conclusion": "failure",
      "message": "Update tempfile"
    }
  },
  {
    "36747fe907889fc99745be30cc38136ca3465402": {
      "conclusion": "failure",
      "message": "Update schannel"
    }
  },
  {
    "06401ef4ca163ee27632710cb2be55f0bb0fafeb": {
      "conclusion": "failure",
      "message": "Update filetime"
    }
  },
  {
    "31a722aedbb65bfbb23e4bd73a0a9a9c9aec0c01": {
      "conclusion": "failure",
      "message": "Update chrono"
    }
  }
]

or with a tiny bit more work can just show all of the respective commits that failed:

$ jq -r -s '.[].workflow_runs[] | select(.head_commit.message | test("^Update \\w+$")) | select(.conclusion == "failure") | .head_commit.id' < runs.json | xargs git show
commit 258ee2c2934fa961176d6f809a95e0545a39b1fe
Author: Nathan Henrie <nate@n8henrie.com>
Date:   Tue Jun 11 16:21:15 2024 -0600

    Update tokio

diff --git a/Cargo.lock b/Cargo.lock
index fcceab7..9738656 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -2,6 +2,15 @@
 # It is not intended for manual editing.
 version = 3
 
+[[package]]
+name = "addr2line"
+version = "0.21.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "8a30b2e23b9e17a9f90641c7ab1549cd9b44f296d3ccbf309d2863cfe398a0cb"
+dependencies = [
+ "gimli",
+]
+
 [[package]]
 name = "adler"
 version = "1.0.2"
@@ -70,6 +79,21 @@ version = "1.1.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "d468802bab17cbc0cc575e9b053f41e72aa36bfa6b7f55e3529ffa43161b97fa"
 
+[[package]]
+name = "backtrace"
+version = "0.3.69"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "2089b7e3f35b9dd2d0ed921ead4f6d318c27680d4a5bd167b3ee120edb105837"
+dependencies = [
+ "addr2line",
+ "cc",
+ "cfg-if 1.0.0",
+ "libc",
+ "miniz_oxide 0.7.3",
+ "object",
+ "rustc-demangle",
+]
+
 [[package]]
 name = "base64"
 version = "0.13.0"
@@ -976,7 +1000,7 @@ dependencies = [
  "cfg-if 1.0.0",
  "crc32fast",
  "libc",
- "miniz_oxide",
+ "miniz_oxide 0.4.4",
 ]
 
 [[package]]
@@ -1179,6 +1203,12 @@ dependencies = [
  "wasi 0.10.0+wasi-snapshot-preview1",
 ]
 
+[[package]]
+name = "gimli"
+version = "0.28.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "4271d37baee1b8c7e4b708028c57d816cf9d2434acb33a549475f78c181f6253"
+
 [[package]]
 name = "glob"
 version = "0.3.0"
@@ -1332,7 +1362,7 @@ dependencies = [
  "httpdate",
  "itoa 1.0.6",
  "pin-project-lite",
- "socket2",
+ "socket2 0.4.9",
  "tokio",
  "tower-service",
  "tracing",
@@ -1678,6 +1708,15 @@ dependencies = [
  "autocfg",
 ]
 
+[[package]]
+name = "miniz_oxide"
+version = "0.7.3"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "87dfd01fe195c66b572b37921ad8803d010623c0aca821bea2302239d155cdae"
+dependencies = [
+ "adler",
+]
+
 [[package]]
 name = "mio"
 version = "0.6.23"
@@ -1699,14 +1738,13 @@ dependencies = [
 
 [[package]]
 name = "mio"
-version = "0.8.6"
+version = "0.8.11"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "5b9d9a46eff5b4ff64b45a9e316a6d1e0bc719ef429cbec4dc630684212bfdf9"
+checksum = "a4a650543ca06a924e8b371db273b2756685faae30f8487da1b56505a8f78b0c"
 dependencies = [
  "libc",
- "log",
  "wasi 0.11.0+wasi-snapshot-preview1",
- "windows-sys 0.45.0",
+ "windows-sys 0.48.0",
 ]
 
 [[package]]
@@ -1922,6 +1960,15 @@ dependencies = [
  "objc",
 ]
 
+[[package]]
+name = "object"
+version = "0.32.2"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "a6a622008b6e321afc04970976f62ee297fdbaa6f95318ca343e3eebb9648441"
+dependencies = [
+ "memchr",
+]
+
 [[package]]
 name = "once_cell"
 version = "1.19.0"
@@ -2513,6 +2560,12 @@ dependencies = [
  "crossbeam-utils",
 ]
 
+[[package]]
+name = "rustc-demangle"
+version = "0.1.24"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "719b953e2095829ee67db738b3bfa9fa368c94900df327b3f07fe6e794d2fe1f"
+
 [[package]]
 name = "rustix"
 version = "0.38.32"
@@ -2755,6 +2808,16 @@ dependencies = [
  "winapi 0.3.9",
 ]
 
+[[package]]
+name = "socket2"
+version = "0.5.7"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "ce305eb0b4296696835b71df73eb912e0f1ffd2556a501fcede6e0c50349191c"
+dependencies = [
+ "libc",
+ "windows-sys 0.52.0",
+]
+
 [[package]]
 name = "spin"
 version = "0.5.2"
@@ -3023,17 +3086,17 @@ checksum = "cda74da7e1a664f795bb1f8a87ec406fb89a02522cf6e50620d016add6dbbf5c"
 
 [[package]]
 name = "tokio"
-version = "1.28.2"
+version = "1.38.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "94d7b1cfd2aa4011f2de74c2c4c63665e27a71006b0a192dcd2710272e73dfa2"
+checksum = "ba4f4a02a7a80d6f274636f0aa95c7e383b912d41fe721a31f29e29698585a4a"
 dependencies = [
- "autocfg",
+ "backtrace",
  "bytes",
  "libc",
- "mio 0.8.6",
+ "mio 0.8.11",
  "num_cpus",
  "pin-project-lite",
- "socket2",
+ "socket2 0.5.7",
  "windows-sys 0.48.0",
 ]
 
@@ -3525,15 +3588,6 @@ dependencies = [
  "windows_x86_64_msvc 0.24.0",
 ]
 
-[[package]]
-name = "windows-sys"
-version = "0.45.0"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "75283be5efb2831d37ea142365f009c02ec203cd29a3ebecbc093d52315b66d0"
-dependencies = [
- "windows-targets 0.42.2",
-]
-
 [[package]]
 name = "windows-sys"
 version = "0.48.0"
@@ -3552,21 +3606,6 @@ dependencies = [
  "windows-targets 0.52.4",
 ]
 
-[[package]]
-name = "windows-targets"
-version = "0.42.2"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "8e5180c00cd44c9b1c88adb3693291f1cd93605ded80c250a75d472756b4d071"
-dependencies = [
- "windows_aarch64_gnullvm 0.42.2",
- "windows_aarch64_msvc 0.42.2",
- "windows_i686_gnu 0.42.2",
- "windows_i686_msvc 0.42.2",
- "windows_x86_64_gnu 0.42.2",
- "windows_x86_64_gnullvm 0.42.2",
- "windows_x86_64_msvc 0.42.2",
-]
-
 [[package]]
 name = "windows-targets"
 version = "0.48.5"
@@ -3597,12 +3636,6 @@ dependencies = [
  "windows_x86_64_msvc 0.52.4",
 ]
 
-[[package]]
-name = "windows_aarch64_gnullvm"
-version = "0.42.2"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "597a5118570b68bc08d8d59125332c54f1ba9d9adeedeef5b99b02ba2b0698f8"
-
 [[package]]
 name = "windows_aarch64_gnullvm"
 version = "0.48.5"
@@ -3615,12 +3648,6 @@ version = "0.52.4"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "bcf46cf4c365c6f2d1cc93ce535f2c8b244591df96ceee75d8e83deb70a9cac9"
 
-[[package]]
-name = "windows_aarch64_msvc"
-version = "0.42.2"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "e08e8864a60f06ef0d0ff4ba04124db8b0fb3be5776a5cd47641e942e58c4d43"
-
 [[package]]
 name = "windows_aarch64_msvc"
 version = "0.48.5"
@@ -3639,12 +3666,6 @@ version = "0.24.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "c0866510a3eca9aed73a077490bbbf03e5eaac4e1fd70849d89539e5830501fd"
 
-[[package]]
-name = "windows_i686_gnu"
-version = "0.42.2"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "c61d927d8da41da96a81f029489353e68739737d3beca43145c8afec9a31a84f"
-
 [[package]]
 name = "windows_i686_gnu"
 version = "0.48.5"
@@ -3663,12 +3684,6 @@ version = "0.24.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "bf0ffed56b7e9369a29078d2ab3aaeceea48eb58999d2cff3aa2494a275b95c6"
 
-[[package]]
-name = "windows_i686_msvc"
-version = "0.42.2"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "44d840b6ec649f480a41c8d80f9c65108b92d89345dd94027bfe06ac444d1060"
-
 [[package]]
 name = "windows_i686_msvc"
 version = "0.48.5"
@@ -3687,12 +3702,6 @@ version = "0.24.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "384a173630588044205a2993b6864a2f56e5a8c1e7668c07b93ec18cf4888dc4"
 
-[[package]]
-name = "windows_x86_64_gnu"
-version = "0.42.2"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "8de912b8b8feb55c064867cf047dda097f92d51efad5b491dfb98f6bbb70cb36"
-
 [[package]]
 name = "windows_x86_64_gnu"
 version = "0.48.5"
@@ -3705,12 +3714,6 @@ version = "0.52.4"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "5eee091590e89cc02ad514ffe3ead9eb6b660aedca2183455434b93546371a03"
 
-[[package]]
-name = "windows_x86_64_gnullvm"
-version = "0.42.2"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "26d41b46a36d453748aedef1486d5c7a85db22e56aff34643984ea85514e94a3"
-
 [[package]]
 name = "windows_x86_64_gnullvm"
 version = "0.48.5"
@@ -3729,12 +3732,6 @@ version = "0.24.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "9bd8f062d8ca5446358159d79a90be12c543b3a965c847c8f3eedf14b321d399"
 
-[[package]]
-name = "windows_x86_64_msvc"
-version = "0.42.2"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "9aec5da331524158c6d1a4ac0ab1541149c0b9505fde06423b02f5ef0106b9f0"
-
 [[package]]
 name = "windows_x86_64_msvc"
 version = "0.48.5"

commit 1585debaa1fee264a30d2d4f9dfc151829771f78
Author: Nathan Henrie <nate@n8henrie.com>
Date:   Tue Jun 11 16:20:54 2024 -0600

    Update tempfile

diff --git a/Cargo.lock b/Cargo.lock
index fcceab7..3a9fba2 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -955,6 +955,12 @@ dependencies = [
  "winrt-notification 0.5.1",
 ]
 
+[[package]]
+name = "fastrand"
+version = "2.1.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "9fc0510504f03c51ada170672ac806f1f105a88aa97a5281117e1ddc3368e51a"
+
 [[package]]
 name = "filetime"
 version = "0.2.14"
@@ -2911,16 +2917,14 @@ dependencies = [
 
 [[package]]
 name = "tempfile"
-version = "3.2.0"
+version = "3.10.1"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "dac1c663cfc93810f88aed9b8941d48cabf856a1b111c29a40439018d870eb22"
+checksum = "85b77fafb263dd9d05cbeac119526425676db3784113aa9295c88498cbf8bff1"
 dependencies = [
  "cfg-if 1.0.0",
- "libc",
- "rand 0.8.3",
- "redox_syscall 0.2.5",
- "remove_dir_all",
- "winapi 0.3.9",
+ "fastrand",
+ "rustix",
+ "windows-sys 0.52.0",
 ]
 
 [[package]]

commit 36747fe907889fc99745be30cc38136ca3465402
Author: Nathan Henrie <nate@n8henrie.com>
Date:   Tue Jun 11 16:19:56 2024 -0600

    Update schannel

diff --git a/Cargo.lock b/Cargo.lock
index fcceab7..abacead 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -2564,12 +2564,11 @@ dependencies = [
 
 [[package]]
 name = "schannel"
-version = "0.1.19"
+version = "0.1.23"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "8f05ba609c234e60bee0d547fe94a4c7e9da733d1c962cf6e59efa4cd9c8bc75"
+checksum = "fbc91545643bcf3a0bbb6569265615222618bdf33ce4ffbbd13c4bbd4c093534"
 dependencies = [
- "lazy_static",
- "winapi 0.3.9",
+ "windows-sys 0.52.0",
 ]
 
 [[package]]

commit 06401ef4ca163ee27632710cb2be55f0bb0fafeb
Author: Nathan Henrie <nate@n8henrie.com>
Date:   Tue Jun 11 16:15:36 2024 -0600

    Update filetime

diff --git a/Cargo.lock b/Cargo.lock
index fcceab7..d5d44a8 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -957,14 +957,14 @@ dependencies = [
 
 [[package]]
 name = "filetime"
-version = "0.2.14"
+version = "0.2.23"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "1d34cfa13a63ae058bfa601fe9e313bbdb3746427c1459185464ce0fcf62e1e8"
+checksum = "1ee447700ac8aa0b2f2bd7bc4462ad686ba06baa6727ac149a2d6277f0d240fd"
 dependencies = [
  "cfg-if 1.0.0",
  "libc",
- "redox_syscall 0.2.5",
- "winapi 0.3.9",
+ "redox_syscall 0.4.1",
+ "windows-sys 0.52.0",
 ]
 
 [[package]]

commit 31a722aedbb65bfbb23e4bd73a0a9a9c9aec0c01
Author: Nathan Henrie <nate@n8henrie.com>
Date:   Tue Jun 11 16:14:18 2024 -0600

    Update chrono

diff --git a/Cargo.lock b/Cargo.lock
index fcceab7..8616a16 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -253,15 +253,16 @@ checksum = "baf1de4339761588bc0619e3cbc0120ee582ebb74b53b4efbf79117bd2da40fd"
 
 [[package]]
 name = "chrono"
-version = "0.4.19"
+version = "0.4.20"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "670ad68c9088c2a963aaa298cb369688cf3f9465ce5e2d4ca10e6e0098a1ce73"
+checksum = "6127248204b9aba09a362f6c930ef6a78f2c1b2215f8a7b398c06e1083f17af0"
 dependencies = [
- "libc",
+ "js-sys",
  "num-integer",
  "num-traits",
  "pure-rust-locales",
  "time",
+ "wasm-bindgen",
  "winapi 0.3.9",
 ]

@n8henrie
Copy link
Contributor Author

Next I'm hoping to filter the output logs from the failed runs to see which (if any) of these result in the duplicated gdiplus linking errors from above. But first a trip to the store for some ingredients for dinner.

@n8henrie
Copy link
Contributor Author

Downloading the logs (which come as zip files), unzipping into a structured subdirectory:

#!/usr/bin/env bash

main() {

  while read -r run; do
    mkdir -p logs/"${run}"
    gh api \
      -H "Accept: application/vnd.github+json" \
      -H "X-GitHub-Api-Version: 2022-11-28" /repos/n8henrie/espanso/actions/runs/"${run}"/logs > logs/"${run}/${run}.zip" &
  done < <(jq -s '
        .[].workflow_runs[] |
        select(.head_commit.message |
        test("^Update \\w+$")) |
        select(.conclusion == "failure")  |
        .id
      ' < runs.json)
  wait

  while read -r zipfile; do
    pushd "$(dirname "${zipfile}")"
    unzip *.zip
    popd
  done < <(find logs -name '*.zip')
}
main "$@"

Here are the runs that correspond to the gpiplus error:

$ rg -l gdiplus logs/ | awk -F'/' '{ print $2 }' | sort  -u
9473477655
9473525880
9473536486
9473541040

@n8henrie
Copy link
Contributor Author

So looks like that is these dependencies that seem to provoke the gdiplus error:

[
  {
    "id": 9473541040,
    "commit": "258ee2c2934fa961176d6f809a95e0545a39b1fe",
    "conclusion": "failure",
    "message": "Update tokio"
  },
  {
    "id": 9473536486,
    "commit": "1585debaa1fee264a30d2d4f9dfc151829771f78",
    "conclusion": "failure",
    "message": "Update tempfile"
  },
  {
    "id": 9473525880,
    "commit": "36747fe907889fc99745be30cc38136ca3465402",
    "conclusion": "failure",
    "message": "Update schannel"
  },
  {
    "id": 9473477655,
    "commit": "06401ef4ca163ee27632710cb2be55f0bb0fafeb",
    "conclusion": "failure",
    "message": "Update filetime"
  }
]

I wonder if there is some common subdependency that is getting updated and provokes the issue?

@n8henrie
Copy link
Contributor Author

image

So it looks like tempfile had a pretty small diff and made a fair target for trying to hunt things down.

Sure enough, as one can see at their changelog, they added windows-sys in 3.5.0, which sounded like a possible culprit, and updated windows-sys to 0.52 (from 0.48 I think) in version 3.9.0, which also seemed suspicious. As per the screenshot above, the build fails with tempfile 3.9.0 but works with the previous release 3.8.1, so I think we're getting somewhere.

@AucaCoyan
Copy link
Member

AucaCoyan commented Jun 14, 2024

This is amazing work man. Not only because it's digging a really really deep into the codebase, but also because you document it very well for future issues 🙌.

If you ask me, I'm on board to get rid of tempfile and do everything manually. That's about changing 1 relevant file
https://github.com/search?q=repo%3Aespanso%2Fespanso%20tempfile&type=code

@n8henrie
Copy link
Contributor Author

If you ask me, I'm on board to get rid of tempfile

Yeah, I was hoping to find a different solution, but not sure if I'll get there.

I'm thinking the duplicate symbols errors are due to the GdiPlus stuff in windows-sys: https://github.com/search?q=repo%3Amicrosoft%2Fwindows-rs%20GdiPlus&type=code

It looks like there is a GdiPlus feature, but it doesn't look like that feature is enabled, so I'm not sure why those symbols are being generated.

@n8henrie
Copy link
Contributor Author

Oh interesting, it looks like tempfile is only used for test code. So it might even be reasonable to just pin to an older release? It seems less likely to expose users to e.g. security vulnerabilities from an outdated dependency if it's only used in test code...

@n8henrie
Copy link
Contributor Author

Unfortunately just removing tempfile (and cargo update) still failed -- likely other dependencies using the culprit version of windows-sys.

I also tried adding the USE_GDIPLUS=0 flag to the vendored build of wxWidgets (https://github.com/wxWidgets/wxWidgets/blob/master/docs/msw/install.md), this still fails :/

@AucaCoyan
Copy link
Member

AucaCoyan commented Jun 14, 2024

Yes, I knew about that dep because in the past I browsed cargo.toml to see if I could delete some of them (and I did with a few!).
Btw, I think clippy is happy now in espanso:dev, try pulling the latest changes! Nervermind, I see you did the changes 😊

Does it sound reasonable to repeat this process and see which deps have a dependency of windows-sys? We may be on a path to solve multiple problems, not only this one 🙌

@n8henrie
Copy link
Contributor Author

Does it sound reasonable to repeat this process and see which deps have a dependency of windows-sys?

Already done:

$ cargo update
$ cargo tree --target x86_64-pc-windows-msvc -i windows-sys@0.52.0
windows-sys v0.52.0
├── filetime v0.2.23
│   └── notify v4.0.17
│       └── espanso v2.2.1 (/Users/n8henrie/git/espanso/espanso)
├── schannel v0.1.23
│   └── native-tls v0.2.12
│       ├── hyper-tls v0.5.0
│       │   └── reqwest v0.11.27
│       │       └── espanso-package v0.1.0 (/Users/n8henrie/git/espanso/espanso-package)
│       │           └── espanso v2.2.1 (/Users/n8henrie/git/espanso/espanso)
│       ├── reqwest v0.11.27 (*)
│       └── tokio-native-tls v0.3.1
│           ├── hyper-tls v0.5.0 (*)
│           └── reqwest v0.11.27 (*)
├── socket2 v0.5.7
│   ├── hyper v0.14.29
│   │   ├── hyper-tls v0.5.0 (*)
│   │   └── reqwest v0.11.27 (*)
│   └── tokio v1.38.0
│       ├── h2 v0.3.26
│       │   ├── hyper v0.14.29 (*)
│       │   └── reqwest v0.11.27 (*)
│       ├── hyper v0.14.29 (*)
│       ├── hyper-tls v0.5.0 (*)
│       ├── reqwest v0.11.27 (*)
│       ├── tokio-native-tls v0.3.1 (*)
│       └── tokio-util v0.7.11
│           └── h2 v0.3.26 (*)
├── tempfile v3.10.1
│   └── dialoguer v0.8.0
│       └── espanso v2.2.1 (/Users/n8henrie/git/espanso/espanso)
│   [dev-dependencies]
│   └── espanso-config v0.1.0 (/Users/n8henrie/git/espanso/espanso-config)
│       └── espanso v2.2.1 (/Users/n8henrie/git/espanso/espanso)
└── winapi-util v0.1.8
    ├── same-file v1.0.6
    │   └── walkdir v2.5.0
    │       ├── espanso-config v0.1.0 (/Users/n8henrie/git/espanso/espanso-config) (*)
    │       ├── espanso-migrate v0.1.0 (/Users/n8henrie/git/espanso/espanso-migrate)
    │       │   └── espanso v2.2.1 (/Users/n8henrie/git/espanso/espanso)
    │       └── notify v4.0.17 (*)
    ├── termcolor v1.1.3
    │   └── simplelog v0.9.0
    │       └── espanso v2.2.1 (/Users/n8henrie/git/espanso/espanso)
    └── walkdir v2.5.0 (*)

@n8henrie
Copy link
Contributor Author

Perhaps we should see if things still work if we can compile wxWidgets without linking the GDI stuff. Unfortunately the USE_GDIPLUS=0 flag didn't seem to do anything.

@AucaCoyan
Copy link
Member

Mmmmmm... I see.
Federico said in discord that wxWidgets is like a daft solution. It's not clean or maintained nor it thought carefully. If we are able, we would change the GUI entirely, the problem is that I don't know a thing of C++ and there aren't many GUIs in rust with a list of requirements, look at this comment in discord. I quote it here:

GUI (wxWidgets) is vendorized and locked in an old version. Can we move on with tao?

AFAIK Tao provides only the windowing logic. wxWidgets is currently used to render the UI components of Espanso, not only the window. A transition to another GUI technology is possible (eg. Tauri). The reason I initially chose wxWidgets is that it's efficiency, performance and system integration is hard to compete with, but it also comes with several downsides (eg. >90% of the Espanso compile times on a fresh build are spent on wxWidgets atm)

@n8henrie
Copy link
Contributor Author

Fair. I think the biggest limitation might be cross-platform; I've looked into egui and iced a decent amount, both seem interesting, not sure how well they'll work with Windows.

It does seem to be actively maintained, last commit 5 days ago: https://github.com/wxWidgets/wxWidgets

@AucaCoyan
Copy link
Member

Yes! We could also update the dependency. It's a well known pattern in espanso org updating deps that are a couple of years old.
We can keep wxWidgets, nothing wrong with that

@AucaCoyan
Copy link
Member

Today we vendor it though, we would need to "keep vendoring" unless we change the mode in rust relates to it

@n8henrie
Copy link
Contributor Author

I started an issue at wx and their team was remarkably responsive, has ideas for a possible fix on their side: wxWidgets/wxWidgets#24613

@n8henrie
Copy link
Contributor Author

n8henrie commented Jul 3, 2024

The wx team was kind enough to remove the conflicting gdiplus stuff in the thread above.

Unfortunately, wx has moved on quite a bit since we vendored it, and the new version (containing this fix) seems t have an entirely new cmake-based build system.

Especially as I don't know cmake, it seems that reconfiguring espanso-module to use cmake-rs instead of cc-rs may be a nontrivial effort.

@AucaCoyan
Copy link
Member

Nice! Thank you for taking the issue forward 🧑‍🚒.
I think it would be nice to upgrade wxWidgets OR develop a GUI that we are most comfortable with. At this point in time I think they take similar amount of effort, since sinking time in understanding how wxWidgets work and how it interacts with espanso is close to a re-write, don't you think? (my guess, coming from a non cpp developer 🤣).
If you feel adventurous, tauri had a couple of mentions in the past in the discord. We could have an exploration of the two and see what offer do we like the most. (I'm referring to the comment here)

Today is the montly meeting to discuss anything about espanso. If you want, you can join the discord and have a voice chat with others!. The meeting is 2:30 hours away

@n8henrie
Copy link
Contributor Author

n8henrie commented Jul 3, 2024

I would love to join but will be at work during that time.

Tauri is interesting, but I've long thought that iced seemed like the most promising. Unfortunately I'm terrible with GUIs! But I'll try to look into iced and bring back what I find.

@AucaCoyan
Copy link
Member

Woah, I didn't crossed path with Iced before. It even has a Wayland desktop environment!
image

Take your time, this will be really really useful, but we are not in a hurry 😊

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants