search

PLUS-POSTECH / soma

Cross-platform CTF problem container manager
Apache License 2.0
24 stars 3 forks source link

Alternative implementation for Path handling #104

Closed Qwaz closed 5 years ago

Qwaz commented 5 years ago

  1. Windows Path도 슬래시로 끊긴 경로 잘 처리합니다. 단, Unix와 다르게 슬래시와 백슬래시에서 둘 다 끊기는데, PathBufExt의 from_slash도 내부 표현만 다를 뿐 동작은 동일합니다. 따라서 외부 crate 대신 std를 쓰는 원래 방향으로 변경했습니다.

  2. 98의 취지가 String 대신 Path(Buf)를 쓰자인데, 오히려 String을 쓰는 부분이 늘어나고 인코딩-디코딩을 무의미하게 반복하는 부분이 있어 이를 원래 PathBuf 구현으로 되돌렸습니다.

KSAlpha commented 5 years ago

사실 #98의 취지는 String 대신 PathBuf를 쓰자가 아니라, format("{}/{}", path_a, path_b)를 쓰지 말자가 조금 더 정확합니다. 그리고 저는 Linux 경로들 (work_dir, target_path)가 PathBuf로 serde 되는게 잘못된 구현이라고 판단합니다. 위의 두가지 이유로 기존 구현이 String->PathBuf->String의 복잡한 변환 구조를 가지게 된 것입니다. 하지만 from_slash가 제 예상과는 동작을 보이는 것 같아 실패한 것 같네요 ㅠ

근본적인 해결은 Windows와 Linux 두 OS에서 같은 동작을 하도록 만드는 것인데, 약간 겉도는 느낌이 듭니다...

KSAlpha commented 5 years ago

한편, 전반적인 수정들은 가져올게 많아 보이네요 PathBuf 쪽만 합의하고 머지하셔도 좋을 것 같습니다.

Qwaz commented 5 years ago

그리고 저는 Linux 경로들 (work_dir, target_path)가 PathBuf로 serde 되는게 잘못된 구현이라고 판단합니다.

저도 어떤 의미로 PathBuf ser/de가 잘못되었다고 말하시는지는 알지만, 이를 개선하기 위한 방향으로 String ser/de를 사용하는건 기존 구현보다 더 나쁜 방향이라고 생각합니다. 올바른 from_slash 구현이 있다는 가정 하에 #[serde(deserialize_with = "path")]를 사용하든지, UnixPathBuf가 있다는 가정 하에 UnixPathBuf를 사용하는게 옳은 방향이라고 생각합니다...만 두 가지 다 불가능한 현재 상황에서는 PathBuf를 사용하는게 차악이라고 생각합니다.

Qwaz commented 5 years ago

확인하고 댓글 남겨주시면 머지할게요.

KSAlpha commented 5 years ago

흙흙... 최후의 발악을 시도했으나... 해결책을 찾을 수 없었습니다... 머지합니다.