jsdelivr / bot

DEPRECATED The jerk of a bot that checks PRs and responds in comments
22 stars 14 forks source link

UTF-8 encoded files seems be interpreted as binary #55

Closed OlegKi closed 8 years ago

OlegKi commented 8 years ago

After forcing the usage of UTF-8 format in all JavaScript files I get messages like in the pool request:

Expected js/i18n/grid.locale-ja.js to be static content; found binary Expected js/i18n/grid.locale-ja.min.js to be static content; found binary Expected js/i18n/grid.locale-ja.min.map to be static content; found binary

and the merging of the new version will be done many days later.

I suppose that it's a bug in jsdelivrbot, which seems to be incompatible with the --charset "UTF-8" option of closureCompiler. It seems to have problems even with the source JavaScript files in UTF-8 format.

If it's not a bug, then I would like to know what can I do to prevent stopping of automatic pull request by jsdelivrbot.

Thanks in advance.

megawac commented 8 years ago

Dupe of https://github.com/jsdelivr/bot/issues/25. Unfortunately its a GH api issue, not much to be done here. I suppose the check could just be disabled?

OlegKi commented 8 years ago

Sorry, but if we are developers then the answer like this one sounds not serious enough. Every problem can be solved.

If you write

That's the github APIs

then you should write exactly which API you use and in which way. You could create small test case which demonstrates the bug in some specific "github API" which you use. You can and I mean you should post the bug report to github and to include the reference to the bug report in your answer.

The solution exist always! If you really can't do any above steps then you can at least test the first bytes of the file, verify whether it has UTF-8 format, and don't call "the buggy github API" for the files. See here for example a simple code fragments how one can do it in Python.

megawac commented 8 years ago

If you really can't do any above steps then you can at least test the first bytes of the file, verify whether it has UTF-8 format

The issue is that the github api begins truncating files randomly after some n number of files or data. They seem to switch up their truncation heuristics quite frequently.

The Github api documents that a changed file in a commit with 0 additions, changes, or deletions is a binary file. The js files that are being consdiered binary have these properties according to the api.

You can't simply check the first bytes as a) there isn't a simple reliable check that doesn't capture text files (had issues with this before) b) github api contents will often just give you empty files when they feel like it

OlegKi commented 8 years ago

Sorry, but it sounds still not serious if you write about "Github api" the name of the API method which you mean.

I included in my issue the exact reference to the report with a lot of files and included an example with one minimized file grid.locale-ja.min.js, the corresponding map-file grid.locale-ja.min.map and the source file grid.locale-ja.js referenced in map-file. The files will be reported by jsdelivrbot as binary. One can's speak here about some "randomly" effects. You can use any from the files as the input and to write exactly, which "Github api" jsdelivrbot calls with which parameters, which output parameters you test and why the results of working the api-method are wrong. If you mean that the method works in the wrong way then you could prepare the demo which demonstrates the bug.

megawac commented 8 years ago

https://developer.github.com/v3/pulls/#list-pull-requests-files

On Wed, Mar 23, 2016 at 12:50 PM, Oleg Kiriljuk notifications@github.com wrote:

Sorry, but it sounds still not serious if you write about "Github api" the name of the API method which you mean.

I included in my issue the exact reference https://github.com/jsdelivr/jsdelivr/pull/10383#issuecomment-195035143 to the report with a lot of files and included an example with one minimized file grid.locale-ja.min.js https://github.com/free-jqgrid/jqGrid/blob/v4.13.1/js/i18n/grid.locale-ja.min.js, the corresponding map-file grid.locale-ja.min.map https://github.com/free-jqgrid/jqGrid/blob/v4.13.1/js/i18n/grid.locale-ja.min.map and the source file grid.locale-ja.js https://github.com/free-jqgrid/jqGrid/blob/v4.13.1/js/i18n/grid.locale-ja.js referenced in map-file. The files will be reported by jsdelivrbot as binary. One can's speak here about some "randomly" effects. You can use any from the files as the input and to write exactly, which "Github api" jsdelivrbot calls with which parameters, which output parameters you test and why the results of working the api-method are wrong. If you mean that the method works in th e wrong way then you could prepare the demo which demonstrates the bug.

— You are receiving this because you modified the open/close state. Reply to this email directly or view it on GitHub https://github.com/jsdelivr/bot/issues/55#issuecomment-200436833

OlegKi commented 8 years ago

It looks like the reference to the documentation and not like some test case. How you use the API exactle?. Do you mean https://api.github.com/repos/jsdelivr/jsdelivr/pulls/10383/files, which displays the first page of the list of files? I never used "Github api" before, but after simple searching in Google I found Why am I not seeing all my results?, which contains the reference to the page and this one, which describe how to use pagination. The response from https://api.github.com/repos/jsdelivr/jsdelivr/pulls/10383/files for example contains

header

which shows that https://api.github.com/repositories/5022564/pulls/10383/files?page=2 ,,, https://api.github.com/repositories/5022564/pulls/10383/files?page=7 provides the next pages of results.

Should it solve the problem?

megawac commented 8 years ago

@OlegKi look at https://api.github.com/repositories/5022564/pulls/10383/files?page=7 notice that all the libraries that the bot mentions are binary have the following attributes:

    "status": "added",
    "additions": 0,
    "deletions": 0,
    "changes": 0,

As I mentioned previously, the github api documents this sort of file as binary.

OlegKi commented 8 years ago

Everything what I claimed till now were always with clear references. You wrote just now "the github api documents this sort of file as binary." If you claim the statement then you should post the corresponding reference.

For me the information just inform us that the files was unchanged. For example

 ...
"filename": "files/free-jqgrid/4.13.1/css/ui.jqgrid.css",
"status": "added",
"additions": 1445,
"deletions": 0,
"changes": 1445,
...
"sha": null,
"filename": "files/free-jqgrid/4.13.1/plugins/jquery.contextmenu-ui.js",
"status": "added",
"additions": 0,
"deletions": 0,
"changes": 0,
...

says that I made many changes in the file ui.jqgrid.css and no changes in the file jquery.contextmenu-ui.js. It's true.

megawac commented 8 years ago

Sorry mate, I'm in the middle of a workday and responding via mobile. I don't have time to look through documentation for the GH services.

says that I made many changes in the file ui.jqgrid.css and no changes in the file jquery.contextmenu-ui.js.

That would suggest you created (see the status) an empty file, sniffing the contents url yields more than 0 bytes. This indicates the file is binary if we can trust GH API has appropriately processed the file.

Though logically, if a static file (i.e. text based) file changes, there must be a) a line added b) line removed c) line changed. Otherwise the file must be empty (if static, or binary)

On Wed, Mar 23, 2016 at 2:14 PM, Oleg Kiriljuk notifications@github.com wrote:

Everything what I claimed till now were always with clear references. You wrote just now "the github api documents this sort of file as binary." If you claim the statement then you should post the corresponding reference.

For me the information just inform us that the files was unchanged. For example

... "filename": "files/free-jqgrid/4.13.1/css/ui.jqgrid.css", "status": "added", "additions": 1445, "deletions": 0, "changes": 1445, ... "sha": null, "filename": "files/free-jqgrid/4.13.1/plugins/jquery.contextmenu-ui.js", "status": "added", "additions": 0, "deletions": 0, "changes": 0, ...

says that I made many changes in the file ui.jqgrid.css and no changes in the file jquery.contextmenu-ui.js.

— You are receiving this because you modified the open/close state. Reply to this email directly or view it on GitHub https://github.com/jsdelivr/bot/issues/55#issuecomment-200473476

OlegKi commented 8 years ago

If you don't have time to post an answer now, you can do it later of cause. I don't need a quick answer. I need the solution of the problem. I had to wait many days after publishing of every version of my JavaScript library till the corresponding changes was applied on jsdelivr. Because of that I decided to post the issue and I want to solve the problem now to save my time and the time of other developers in the future.

Thus don't hurry and analyse all carefully, when you have time and you will be on you computer of cause. I suggest you not to jump to any common problem with an empty file for example. We have clear test case: the file grid.locale-ja.js or the file jquery.contextmenu-ui.js. We should test the jsdelivrbot with the files and to find out how the existing code of jsdelivrbot could be changed to correctly interpret the files. If you will claim that the reason of the problem is some bug in "Github api" then I expect the problem will be solved too, by the developer of the APIs. If you claim this than you should prove the claim: you should point to the description of the API, and prepare the test case with grid.locale-ja.js or jquery.contextmenu-ui.js file. You should post the issue to the developers of the API and the bug will be fixed in the Github API by the developers of the API.

megawac commented 8 years ago

I've just disabled this check for now. Feel free to report it to Github if you want but I haven't had very much success with their support team in the past.

OlegKi commented 8 years ago

Thank you very much!