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

Remove unused variable from the build system #907

Merged
merged 1 commit into from Nov 11, 2016

Conversation

Projects
None yet
4 participants
@shindere
Contributor

shindere commented Nov 10, 2016

No description provided.

Remove unused variable from the build system
The DLLCCCOMPOPTS variable is defined in the makefiles but is not
actually used anywhere.
@shindere

This comment has been minimized.

Show comment
Hide comment
@shindere

shindere Nov 10, 2016

Contributor

Especially interested in your comments, @alainfrisch, @gasche,
@whitequark and @adrien-n

Contributor

shindere commented Nov 10, 2016

Especially interested in your comments, @alainfrisch, @gasche,
@whitequark and @adrien-n

@whitequark

This comment has been minimized.

Show comment
Hide comment
@whitequark

whitequark Nov 10, 2016

Contributor

LGTM--looks like the last major user was removed in 3958a92, and last user was labltk.

Contributor

whitequark commented Nov 10, 2016

LGTM--looks like the last major user was removed in 3958a92, and last user was labltk.

@gasche

This comment has been minimized.

Show comment
Hide comment
@gasche

gasche Nov 10, 2016

Member

So according to debian code search, there is no Debian package that use this variable (the OCaml compilers define it but don't use it). Google indicates that it was used in old %.c.$(DO) Makefile rules for Windows, which are still copy-pasted in a few projects that include the OCaml sources (for example ucsd-progsys, loop), but those occurrences were removed from the compiler distribution in commit 3958a92 , when @alainfrisch merged the natdynlink branch. Alain may know why these variables are not necessary anymore.

This variable DLLCCCOMPOPTS is exported in %{lib}/ocaml/Makefile.config, so it is in theory possible that third-party packages would rely on it as part of their build. But I can't find any on google. I would guess that if it's not necessary to build the compiler anymore, any package using the variable could also rewrite their build system to not use it.

Member

gasche commented Nov 10, 2016

So according to debian code search, there is no Debian package that use this variable (the OCaml compilers define it but don't use it). Google indicates that it was used in old %.c.$(DO) Makefile rules for Windows, which are still copy-pasted in a few projects that include the OCaml sources (for example ucsd-progsys, loop), but those occurrences were removed from the compiler distribution in commit 3958a92 , when @alainfrisch merged the natdynlink branch. Alain may know why these variables are not necessary anymore.

This variable DLLCCCOMPOPTS is exported in %{lib}/ocaml/Makefile.config, so it is in theory possible that third-party packages would rely on it as part of their build. But I can't find any on google. I would guess that if it's not necessary to build the compiler anymore, any package using the variable could also rewrite their build system to not use it.

@adrien-n

This comment has been minimized.

Show comment
Hide comment
@adrien-n

adrien-n Nov 11, 2016

Contributor

I don't have much to add to what has already been said: indeed, it is unused in the compiler and seems to have been lingering for several years now.

As for it being present in Makefile.config, I'm not surprised it's difficult to find something using it because that file has not been exported for very long as far as I remember and the entry is probably only there because now everything from the ocaml configure step is exported, regardless of any actual usefulness.

Contributor

adrien-n commented Nov 11, 2016

I don't have much to add to what has already been said: indeed, it is unused in the compiler and seems to have been lingering for several years now.

As for it being present in Makefile.config, I'm not surprised it's difficult to find something using it because that file has not been exported for very long as far as I remember and the entry is probably only there because now everything from the ocaml configure step is exported, regardless of any actual usefulness.

@gasche gasche merged commit d5833a6 into ocaml:trunk Nov 11, 2016

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@gasche

This comment has been minimized.

Show comment
Hide comment
@gasche

gasche Nov 11, 2016

Member

@whitequark, @adrien-n: thanks a lot for your feedback -- not only on this PR, but the others as well.

Member

gasche commented Nov 11, 2016

@whitequark, @adrien-n: thanks a lot for your feedback -- not only on this PR, but the others as well.

@shindere

This comment has been minimized.

Show comment
Hide comment
@shindere

shindere Nov 13, 2016

Contributor

@gasche, @whitequark, @adrien-n: many thanks for your useful feedback
on this PR. Will try to investigate more in the future so that you guys
do not have to do the checkings.

Contributor

shindere commented Nov 13, 2016

@gasche, @whitequark, @adrien-n: many thanks for your useful feedback
on this PR. Will try to investigate more in the future so that you guys
do not have to do the checkings.

@shindere shindere deleted the shindere:remove-unused-make-variable branch Nov 13, 2016

camlspotter pushed a commit to camlspotter/ocaml that referenced this pull request Oct 17, 2017

Merge pull request #907 from shindere/remove-unused-make-variable
Remove unused variable from the build system
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment