Skip to content
This repository has been archived by the owner on Apr 12, 2023. It is now read-only.

UTF-8 BOM が不正な位置に存在する #92

Closed
ghost opened this issue Mar 30, 2021 · 16 comments · Fixed by #97
Closed

UTF-8 BOM が不正な位置に存在する #92

ghost opened this issue Mar 30, 2021 · 16 comments · Fixed by #97
Assignees
Labels
enhancement 新しい機能や改善のリクエスト

Comments

@ghost
Copy link

ghost commented Mar 30, 2021

不具合の内容 / Describe the bug

ファイル先頭以外の場所に UTF-8 BOM marker が記載されています

再現手順 / Steps to reproduce

たとえば Covid19Radar/Covid19Radar/AppSettings.cs では、以下のような場所に存在します

$ od -t x1 Covid19Radar/Covid19Radar/AppSettings.cs |head -n 15
0000000 2f 2a 20 54 68 69 73 20 53 6f 75 72 63 65 20 43
0000020 6f 64 65 20 46 6f 72 6d 20 69 73 20 73 75 62 6a
0000040 65 63 74 20 74 6f 20 74 68 65 20 74 65 72 6d 73
0000060 20 6f 66 20 74 68 65 20 4d 6f 7a 69 6c 6c 61 20
0000100 50 75 62 6c 69 63 0d 0a 20 2a 20 4c 69 63 65 6e
0000120 73 65 2c 20 76 2e 20 32 2e 30 2e 20 49 66 20 61
0000140 20 63 6f 70 79 20 6f 66 20 74 68 65 20 4d 50 4c
0000160 20 77 61 73 20 6e 6f 74 20 64 69 73 74 72 69 62
0000200 75 74 65 64 20 77 69 74 68 20 74 68 69 73 0d 0a
0000220 20 2a 20 66 69 6c 65 2c 20 59 6f 75 20 63 61 6e
0000240 20 6f 62 74 61 69 6e 20 6f 6e 65 20 61 74 20 68
0000260 74 74 70 73 3a 2f 2f 6d 6f 7a 69 6c 6c 61 2e 6f
0000300 72 67 2f 4d 50 4c 2f 32 2e 30 2f 2e 20 2a 2f 0d
0000320 0a 0d 0a ef bb bf 75 73 69 6e 67 20 53 79 73 74
0000340 65 6d 2e 43 6f 6c 6c 65 63 74 69 6f 6e 73 2e 47

期待される挙動 / Expected behavior

ファイル先頭に置くか、すべて削除する

スクリーンショット / Screenshots

動作環境 / Environments

  • デバイス:(例:Pixel 5、iPhone 6 等)
  • OS:(例:Android、iOS)
  • バージョン:(例:1.2.2)

その他 / Additional context

すべて削除で問題ないのだろうとは思うのですが VisualStudio で必須の場所がないか、
確認したほうがよいのかもしれません。
また、一括してスクリプトで処理するような場合には XD のファイルなどバイナリのものを
誤って壊さないように注意する必要はあります

@keiji keiji added enhancement 新しい機能や改善のリクエスト good first issue 初心者や、このリポジトリに対する深い経験が無い方向けのIssue labels Mar 31, 2021
@ghost
Copy link
Author

ghost commented Mar 31, 2021

削除したはいいけどVisualStudio(Windows版)で編集したらまた勝手につく、とかだと
つけたりとったりの差分が出てしまう可能性もあるので、ちゃんと調べてからやった方がよさそうです。
Web で検索すると https://qiita.com/magnet163/items/b1061be4fb1c1833c156 のような記事があるので、
.editorconfig の修正 (#90) で対応できるはずなのでしょうか?

@ghost
Copy link
Author

ghost commented Mar 31, 2021

@b-wind
ファイルの途中に混入しているものについては必要ということはあり得ないだろうと思うので
そこは考えなくていいと思っています(厳密に調べるなら 970bbf8 の時点で
ファイルの途中に混入していないか調べてもいいですが。。)

なので、

  1. BOM 無しにした場合に VisualStudio なり Azure なり既存の動作に影響がないことの確認はしておいた方がよい(ないと思うけど)
  2. 新規作成時や編集時にまたファイル先頭にもBOMが追加されないように対応しておいた方がよい
  3. 追記: もし設定で回避できないファイルがあるなら、最初から(先頭に)BOMをつけておいた方がよい

という2点 3点かなと思っています。

@ghost
Copy link
Author

ghost commented Mar 31, 2021

BOMを削除した方が手っ取り早いかなと思ったりはしたのですが、
安全に進めることを重視するならば、BOM の削除とか検証は後回しにして、
一旦いままで存在していたBOMは残して、先頭に移動するだけ、というのが安全なのではという
結論になりそうかなとは思いました。

@keiji
Copy link
Collaborator

keiji commented Mar 31, 2021

ありがとうございます。

これはBOMを削除してしまいましょう。
#89 がマージされれば以後フォーマッターを適用すれば(適用しなくても?)BOMなしで保存されるという理解なので、それを追う形でファイル先頭のBOMと、中途半端なところにあるものも含めて削除するcommitを準備します。

数が多そうなのと手元にスクリプトがあるのでぼくの方でやります。今度は気をつけて作業しますね。

削除後にクリーン状態からのビルドと、Android/iOSのバイナリが動くかの一通りののテストもやって報告します。

@keiji keiji removed the good first issue 初心者や、このリポジトリに対する深い経験が無い方向けのIssue label Mar 31, 2021
@keiji keiji self-assigned this Mar 31, 2021
@ghost
Copy link
Author

ghost commented Mar 31, 2021

@keiji
個人的には急ぐ理由がわかっていませんが、master を改変してしまっていて、
開発チームのソースとのマージに支障ないか確認いただくことはできますか?
もし conflict するならいったん revert することも検討してみてはどうでしょう?

現時点で(すでに想像からすると何週間も遅すぎるんですが)最優先するべきなのは #25 とかの改修版アプリを
正しくリリースすることで、ライセンス表記もプロジェクト運営も何もかもそれよりは後回しでいいと
思っているのですが。。

@keiji
Copy link
Collaborator

keiji commented Mar 31, 2021

ライセンス表記については後回しにできない問題と理解していますが、BOMについては急いではいないですね。
good first issueつけたけど、これ単純作業で、どんな種類のファイルがどこにあるかわかってないといけないので初心者向けではないので違うでしょ。誰かが手を上げて手数をかける前に引き取っておこうと思いました。

一回revertして、ライセンスヘッダーの付け直しcommitをfixupして入れるのはありですね。

@ghost
Copy link
Author

ghost commented Mar 31, 2021

ライセンス表記も、もちろん重要ではあるけれども、突然発生した問題でもないので
次のバージョンとかでもそんなに影響度は変わらないと思っています。

それこそ単純作業としては私でも誰でもやることもできるんですが、
開発チームとの conflict などの問題がないことの確認やリリースに向けての準備などは
IT室の方なり @keiji さんなりしかできないし、そう言った情報は公開もされていないので、
そちらを優先していただいた上で、手が空いたら着手で問題ないのではと思います

@keiji
Copy link
Collaborator

keiji commented Mar 31, 2021

もちろんそう言ったことはしながら動いています。
不都合が起こるかもしれないと危惧されるのは、ひとえにぼくの能力不足を痛感します。

それこそ単純作業としては私でも誰でもやることもできるんですが、

ありがたい申し出だと思います。それでは @zipperpull さんにこの件、お任せしてもよろしいでしょうか。

そちらを優先していただいた上で、手が空いたら着手で問題ないのではと思います

それを言っちゃうと、ぼくの手が空くときはずっと来ないので……自分から手を挙げて、やることリストに入れておかないといけないという事情もあります。

手を挙げてくださる方がいたらお任せしたい( #91 )。手がないようだったらぼくがやってしまおう。みたいな心持ちです。

@ghost
Copy link
Author

ghost commented Mar 31, 2021

何も考えずに動いているとまでは思っていないものの、外部からは結果としては
何も見えてないので圧倒的に説明不足になっていると感じています。

この件についてもお任せしたいはいいのですが、現時点の GitHub の master でいいので、
開発チームの持っている master とは conflict が起きるかどうかの回答をまずお願いします。

@keiji
Copy link
Collaborator

keiji commented Mar 31, 2021

すでにライセンス通知の表記についても調整済みです。
今回のケースも同様です。

@ghost
Copy link
Author

ghost commented Mar 31, 2021

対応します。ところで回答できるかできないかわかりませんが、
アプリのリリースビルドと審査申請は進んでいないのでしょうか?

ビルドしたソースとmaster の差分ができることは問題ないと考えているのか、
それとも master に揃ってから審査申請するのでまだまだリリースは先の話なのかで言うと
どちらなのでしょう?

@keiji
Copy link
Collaborator

keiji commented Mar 31, 2021

よろしくお願いします。

まことに心苦しいのですが、リリースに関する質問については、本Issueの趣旨から逸れますし、日程についても現時点でぼくからお答えすることはできません。

繰り返しになりますが、開発チームとは調整を重ねていて、ライセンス通知の添付に関係するPRや誤りを修正するPRは事実として取り込まれていますし、zipperpullさんにお願いしようとしている件に関しても同様に調整済みです。

@ghost
Copy link
Author

ghost commented Mar 31, 2021

日程そのものは回答しないで済むような質問にはしたのですが了解しました。
なるべく急いで対応します

This was referenced Mar 31, 2021
@ghost
Copy link
Author

ghost commented Mar 31, 2021

@keiji さん
遅くなってしまいましたが #97 or #98 のいずれかを確認いただければと思います。

#97 はこの issue で議論したとおり、BOM をファイル先頭に移動する対応
#98@keiji さんが書かれていたような、BOM を削除してしまう対応です

いずれも、単純に VisualStudio Windows 版で clean build した程度であれば BOM が追加されてしまう現象は
起きなかったのですが、現時点で .editorconfig を入れない予定であるならば #97 のほうが
安心なのではとは思っています。とはいえ BOM が追加されてしまったとしても
たいした問題ではないと思うので、どちらを利用してどちらを close するかなどはおまかせしますので
よろしくお願いします(もちろんすぐわかるような問題などあれば対応します)

@keiji
Copy link
Collaborator

keiji commented Mar 31, 2021

ありがとうございます。
いただいた2つパターン、まずは実際にビルドをしてみようとおもいます(iOSでのビルドチェックがCIでできていないので)

@keiji
Copy link
Collaborator

keiji commented Apr 1, 2021

開発チームに確認をして #97 BOMをファイル先頭へ移動の採用を希望しているのでそちらで進めますね。

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement 新しい機能や改善のリクエスト
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant