Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disable zip feature in Bytecode mode #1248

Closed
wants to merge 2 commits into from

Conversation

Halbaroth
Copy link
Collaborator

As Basile noticed, the Javascript code in My_zip is dead too. I
removed it and clean a bit the module.

I also produces an appropriate run error if we try to use the feature
in Javascript because camlzip does not work properly in bytecode.

Currently, the error cannot be trigger because we do not use yet the
Dolmen version of Solving_loop in the javascript worker. This will
change in my next PR.

We should use the same test as we only want to check that Alt-Ergo can
unzip correctly the input file if its name has the form:
name.format.zip.
@@ -1,2 +1,2 @@

unknown
unsat
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is normal. I changed the test in order to use the same one for both native and SMT language.

As Basile noticed, the Javascript code in `My_zip` is dead too. I
removed it and clean a bit the module.

I also produces an appropriate `run error` if we try to use the feature
in Javascript because `camlzip` does not work properly in bytecode.

Currently, the error cannot be trigger because we do not use yet the
Dolmen version of `Solving_loop` in the javascript worker. This will
change in my next PR.
Copy link
Collaborator

@bclement-ocp bclement-ocp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason for the difference between the unzip.ae.zip and unzip.smt2.zip is this:

$ file tests/misc/unzip.ae.zip
tests/misc/unzip.ae.zip: Zip archive data, at least v1.0 to extract, compression method=store

$ file tests/misc/unzip.smt2.zip
tests/misc/unzip.smt2.zip: Zip archive data, at least v2.0 to extract, compression method=deflate

The unzip.ae.zip file is not compressed (method=store) while the unzip.smt2.zip file is compressed with zlib (method=deflate). Since zlib is not available with js_of_ocaml, only uncompressed zip files are supported in js_of_ocaml.

I am fine keeping the check (with the proper test, see inline comment) but I think we should remove the check for js_of_ocaml when reading zip files entirely -- if the zip file is uncompressed, it will just work; if it is compressed, we will fail due to a missing primitive anyway (it's not the nicest way to fail, but we don't expect people to actually use the jsoo version as a binary -- only through the worker).

Comment on lines +40 to +41
if Stdlib.(Sys.backend_type = Bytecode) then
Errors.unsupported_feature "Zip format in javascript mode";
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this is disabling zip files for non-jsoo bytecode but not jsoo (which I think has Other "js_of_ocaml" as a backend type). Although I think this test should be removed entirely -- see toplevel comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants