rbtying / shengji

An online version of shengji (a.k.a. tractor) and zhaopengyou (a.k.a. Finding Friends)
https://robertying.com/shengji/
MIT License
85 stars 44 forks source link

fix: `yarn types` crash when /tmp is cross-device #454

Open elldritch opened 5 days ago

elldritch commented 5 days ago

This PR modifies json-schemabin/src/main.rs so that the script attempts to fs::copy if the fs::rename fails. On my machine, this fails because I'm running Arch Linux and /tmp is mounted on a different device than /home.

You can see another example of this error in https://github.com/rust-lang/rustup/issues/1239.

Ideally, we would match on e.kind(), but unfortunately the CrossesDevices error enum value is not yet available in stable, only nightly (see https://doc.rust-lang.org/stable/std/io/enum.ErrorKind.html#variant.CrossesDevices).

Another simpler alternative here would be to always do a copy instead of a rename, although this might slow down builds a little bit on unaffected machines.

elldritch commented 1 day ago

Done. I've also rebased on to latest master.

The diff also contains changes to the generated files (from when I ran yarn types), but I don't think any of those changes are semantic. Here's what difftastic says about the AST diff:

➜ git --no-pager -c diff.external='difft --display inline' diff HEAD^
frontend/json-schema-bin/src/main.rs --- Rust
55                return;
56            }
57        }
58
59        - std::fs::rename(&tmp_path, path).unwrap()
   59     + std::fs::copy(&tmp_path, path).unwrap();
   60 }

frontend/src/gen-types.d.ts --- 1/4 --- TypeScript
160          PlayCardsWithHint: [Card[], TrickUnit[]];
161        };
162    export type Number = string;
163    export type FriendSelectionPolicy =
164    -   | "Unrestricted"
171    -   | "JokerOrHigherSuit"
179    -   | "BothTwoOrMore"
   163 + export type FriendSelectionPolicy = "Unrestricted" | "TrumpsIncluded" | "HighestCardNotAllowed" | "PointCardNotAllowed";
   166 + export type BidPolicy = "JokerOrHigherSuit" | "JokerOrGreaterLength" | "GreaterLength";
   171 + export type JokerBidPolicy = "BothTwoOrMore" | "BothNumDecks" | "LJNumDecksHJNumDecksLessOne" | "Disabled";
   172 export type MaxRank = string;

frontend/src/gen-types.d.ts --- 2/4 --- TypeScript
189            [k: string]: unknown;
190          };
191        };
192    export type AdvancementPolicy =
193    -   | "Unrestricted"
197    -   | "NoBonusLevel"
   181 + export type AdvancementPolicy = "Unrestricted" | "FullyUnrestricted" | "DefendPoints";
   182 + export type BonusLevelPolicy = "NoBonusLevel" | "BonusLevelForSmallerLandlordTeam";
   183 export type KittyPenalty = "Times" | "Power";
   184 export type KittyBidPolicy = "FirstCard" | "FirstCardOfLevelOrHighest";
   185 export type TrickDrawPolicy =

frontend/src/gen-types.d.ts --- 3/4 --- TypeScript
242          };
243        };
244    export type Suit = string;
245    export type EffectiveSuit =
246    -   | "Unknown"
   229 + export type EffectiveSuit = "Unknown" | "Clubs" | "Diamonds" | "Spades" | "Hearts" | "Trump";

frontend/src/gen-types.d.ts --- 4/4 --- TypeScript
724       * A parallel array to `played_cards` which contains the units corresponding to played cards that match the `trick_format`, or `None` if they don't match.
725       *
726       * TODO: remove default deserialization attribute in a few days.
727       */
728    - played_card_mappings?: Array<TrickUnit[] | null>;
   706 + played_card_mappings?: (TrickUnit[] | null)[];
   707   played_cards: PlayedCards[];
   708   player_queue: number[];
   709   trick_format?: TrickFormat | null;
   710   trump: Trump;

frontend/src/gen-types.schema.json --- JSON
No syntactic changes.

It removed some leading |s from sum types, and changed an Array<T> to a T[].

rbtying commented 1 day ago

@elldritch I think you might be missing a yarn prettier --write

elldritch commented 1 day ago

@rbtying Good catch! Pushed.

EDIT: It looks like this still chose to prefer T[] over Array<T>. Not sure why, given that the prettier version in the lockfile hasn't been updated since 2022 and the last regeneration of this file was 2024. Here's the version my system is using:

➜ yarn prettier --version
yarn run v1.22.22
$ prettier src --version
2.5.1
Done in 0.63s.

➜ node --version
v23.1.0
rbtying commented 1 day ago

Hm, perhaps we should update prettier here since it fails eslint now!

/home/runner/work/shengji/shengji/frontend/src/gen-types.d.ts
  728:26  error  Array type using 'T[]' is forbidden for non-simple types. Use 'Array<T>' instead  @typescript-eslint/array-type

Let me take a stab at this later, though -- thank you for getting it this far!