{"action":"created","issue":{"url":"https://api.github.com/repos/ocsigen/lwt/issues/523","repository_url":"https://api.github.com/repos/ocsigen/lwt","labels_url":"https://api.github.com/repos/ocsigen/lwt/issues/523/labels{/name}","comments_url":"https://api.github.com/repos/ocsigen/lwt/issues/523/comments","events_url":"https://api.github.com/repos/ocsigen/lwt/issues/523/events","html_url":"https://github.com/ocsigen/lwt/issues/523","id":284371189,"number":523,"title":"Remove the Unix stubs generator","user":{"login":"aantron","id":12073668,"avatar_url":"https://avatars2.githubusercontent.com/u/12073668?v=4","gravatar_id":"","url":"https://api.github.com/users/aantron","html_url":"https://github.com/aantron","followers_url":"https://api.github.com/users/aantron/followers","following_url":"https://api.github.com/users/aantron/following{/other_user}","gists_url":"https://api.github.com/users/aantron/gists{/gist_id}","starred_url":"https://api.github.com/users/aantron/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/aantron/subscriptions","organizations_url":"https://api.github.com/users/aantron/orgs","repos_url":"https://api.github.com/users/aantron/repos","events_url":"https://api.github.com/users/aantron/events{/privacy}","received_events_url":"https://api.github.com/users/aantron/received_events","type":"User","site_admin":false},"labels":[{"id":607583563,"url":"https://api.github.com/repos/ocsigen/lwt/labels/medium","name":"medium","color":"f9d0c4","default":false},{"id":583175428,"url":"https://api.github.com/repos/ocsigen/lwt/labels/technical%20debt","name":"technical debt","color":"000000","default":false}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":5,"created_at":"2017-12-24T16:18:25Z","updated_at":"2017-12-25T11:40:00Z","closed_at":null,"author_association":"COLLABORATOR","body":"The [stubs generator](https://github.com/ocsigen/lwt/blob/caa642e8ecf043dae522bb7bedc90b1ca471c9e3/src/unix/stubs/gen_stubs.ml) (in [`src/unix/stubs`](https://github.com/ocsigen/lwt/tree/caa642e8ecf043dae522bb7bedc90b1ca471c9e3/src/unix/stubs)) generates [several of the C files](https://github.com/ocsigen/lwt/blob/caa642e8ecf043dae522bb7bedc90b1ca471c9e3/src/unix/jbuild#L51-L78) that make up the Unix binding, and one `.ml` file:\r\n\r\n```\r\n    lwt_unix_job_access.c\r\n    lwt_unix_job_chdir.c\r\n    lwt_unix_job_chmod.c\r\n    lwt_unix_job_chown.c\r\n    lwt_unix_job_chroot.c\r\n    lwt_unix_job_close.c\r\n    lwt_unix_job_fchmod.c\r\n    lwt_unix_job_fchown.c\r\n    lwt_unix_job_fdatasync.c\r\n    lwt_unix_job_fsync.c\r\n    lwt_unix_job_ftruncate.c\r\n    lwt_unix_job_link.c\r\n    lwt_unix_job_lseek.c\r\n    lwt_unix_job_mkdir.c\r\n    lwt_unix_job_mkfifo.c\r\n    lwt_unix_job_rename.c\r\n    lwt_unix_job_rmdir.c\r\n    lwt_unix_job_symlink.c\r\n    lwt_unix_job_tcdrain.c\r\n    lwt_unix_job_tcflow.c\r\n    lwt_unix_job_tcflush.c\r\n    lwt_unix_job_tcsendbreak.c\r\n    lwt_unix_job_truncate.c\r\n    lwt_unix_job_unlink.c\r\n    lwt_unix_jobs_generated.ml\r\n```\r\n\r\nIt is meant to cut down on repetitive code. However, I personally have found it to be slightly opaque, and potentially doing more harm to legibility and maintainability than good.\r\n\r\nThis issue is about removing the stubs generator, and replacing it by ordinary code. Alternatively, if the stubs generator is really very useful, then this issue is about explaining why that is :)\r\n\r\nProbably the easiest way to remove the stubs generator is to dump the code it generates, and commit it. It should be found in `_build/default/src/unix/jobs-unix`, and we want to move it from there to `src/unix/(unix|windows)_c`. After that, the `jobs-unix` directory can be deleted from the source tree.\r\n\r\nI am not sure if the generator is making different code on Unix and Win32 platforms – I would assume so. In that case, to dump on Win32, a Windows machine may be needed, but the easiest way may be to echo the contents of the generated files into the build log in AppVeyor, and take them from there.\r\n\r\ncc @cedlemo due to recent work on the Unix binding :)"},"comment":{"url":"https://api.github.com/repos/ocsigen/lwt/issues/comments/353863420","html_url":"https://github.com/ocsigen/lwt/issues/523#issuecomment-353863420","issue_url":"https://api.github.com/repos/ocsigen/lwt/issues/523","id":353863420,"user":{"login":"cedlemo","id":1135841,"avatar_url":"https://avatars1.githubusercontent.com/u/1135841?v=4","gravatar_id":"","url":"https://api.github.com/users/cedlemo","html_url":"https://github.com/cedlemo","followers_url":"https://api.github.com/users/cedlemo/followers","following_url":"https://api.github.com/users/cedlemo/following{/other_user}","gists_url":"https://api.github.com/users/cedlemo/gists{/gist_id}","starred_url":"https://api.github.com/users/cedlemo/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/cedlemo/subscriptions","organizations_url":"https://api.github.com/users/cedlemo/orgs","repos_url":"https://api.github.com/users/cedlemo/repos","events_url":"https://api.github.com/users/cedlemo/events{/privacy}","received_events_url":"https://api.github.com/users/cedlemo/received_events","type":"User","site_admin":false},"created_at":"2017-12-25T11:40:00Z","updated_at":"2017-12-25T11:40:00Z","author_association":"CONTRIBUTOR","body":"I have been able to modify the gen_stubs.ml like this: \r\n\r\n```\r\ndiff --git a/src/unix/stubs/gen_stubs.ml b/src/unix/stubs/gen_stubs.ml\r\nindex a8ae736..c8b612a 100644\r\n--- a/src/unix/stubs/gen_stubs.ml\r\n+++ b/src/unix/stubs/gen_stubs.ml\r\n@@ -730,7 +730,7 @@ module GenFake = struct\r\n end\r\n \r\n let gen job =\r\n-  let fname = \"lwt_unix_job_\" ^ job.name ^ \".c\" in\r\n+  let fname = \"unix_\" ^ job.name ^ \"_job.c\" in\r\n   let oc = open_out fname in\r\n   let job64 = {\r\n     job with\r\n```\r\n\r\nand compile it with :\r\n\r\n `ocamlc -w +A-4-29 -o gen_stubs.exe gen_stubs.ml`\r\n\r\nso that it generates the files with the same pattern for their names as we did in the unix_c/ directory.\r\n\r\nI should be able to add a generic header with copyright if needed (could you give me a hint on where I could find one for those files ?). \r\n\r\nFurthermore, the C files use the check based on the `LWT_ON_WINDOWS` macro in order to compile or not the code.\r\n\r\nWould you like to keep the gen_stubs.ml generator or not (modified or not) ?"}}
{"actor":{"display_login":"cedlemo"}}