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

Merge build systems in subdirectories of otherlibs. #788

Merged
merged 6 commits into from Sep 5, 2016

Conversation

Projects
None yet
4 participants
@shindere
Contributor

shindere commented Aug 30, 2016

Nothing needed to be done in the directories that are built only on Unix.

@alainfrisch

This comment has been minimized.

Show comment
Hide comment
@alainfrisch

alainfrisch Aug 30, 2016

Contributor

(For the reader: this is mostly about moving code around to ensure that everything is in Makefile and that Makefile.nt is always trivial; most of the sharing was already done before.)

Would it be possible to further factorize the post-processing of .depend (to produce .depend.nt) in the common in otherlibs/Makefile?

For the Windows-specific directories, I'd suggest ensuring that calling make from Unix fails explicitly.

Contributor

alainfrisch commented Aug 30, 2016

(For the reader: this is mostly about moving code around to ensure that everything is in Makefile and that Makefile.nt is always trivial; most of the sharing was already done before.)

Would it be possible to further factorize the post-processing of .depend (to produce .depend.nt) in the common in otherlibs/Makefile?

For the Windows-specific directories, I'd suggest ensuring that calling make from Unix fails explicitly.

@alainfrisch

This comment has been minimized.

Show comment
Hide comment
@alainfrisch

alainfrisch Aug 30, 2016

Contributor

A more ambitious approach would be to ensure that the Makefile in otherlibs/unix and otherlibs/graph could be used even under Windows and in that case automatically delegate to otherlibs/win32unix and otherlibs/win32graph to produce the required artifacts. These win32-specific directories could then be moved e.g. to otherlibs/unix/win32 and otherlibs/graph/win32 to make it clear that they are really alternative implementations of the same library. This might allow some more sharing between the build instructions for the Unix and Windows variants.

Contributor

alainfrisch commented Aug 30, 2016

A more ambitious approach would be to ensure that the Makefile in otherlibs/unix and otherlibs/graph could be used even under Windows and in that case automatically delegate to otherlibs/win32unix and otherlibs/win32graph to produce the required artifacts. These win32-specific directories could then be moved e.g. to otherlibs/unix/win32 and otherlibs/graph/win32 to make it clear that they are really alternative implementations of the same library. This might allow some more sharing between the build instructions for the Unix and Windows variants.

@shindere

This comment has been minimized.

Show comment
Hide comment
@shindere

shindere Aug 31, 2016

Contributor

Alain Frisch (2016/08/30 15:30 -0700):

Would it be possible to further factorize the post-processing of
.depend (to produce .depend.nt) in the common in otherlibs/Makefile?

Sure. It has been done. The depend target has also been factorized and
the whole branch has been rebased on the latest trunk.

For the Windows-specific directories, I'd suggest ensuring that calling make from Unix fails explicitly.
A more ambitious approach would be to ensure that the Makefile in
otherlibs/unix and otherlibs/graph could be used even under Windows
and in that case automatically delegate to otherlibs/win32unix and
otherlibs/win32graph to produce the required artifacts. These
win32-specific directories could then be moved e.g. to
otherlibs/unix/win32 and otherlibs/graph/win32 to make it clear that
they are really alternative implementations of the same library. This
might allow some more sharing between the build instructions for the
Unix and Windows variants.

Could these be implemented in separate commits?
Would it be okay to just add a TODO comment in the makefiles?

Contributor

shindere commented Aug 31, 2016

Alain Frisch (2016/08/30 15:30 -0700):

Would it be possible to further factorize the post-processing of
.depend (to produce .depend.nt) in the common in otherlibs/Makefile?

Sure. It has been done. The depend target has also been factorized and
the whole branch has been rebased on the latest trunk.

For the Windows-specific directories, I'd suggest ensuring that calling make from Unix fails explicitly.
A more ambitious approach would be to ensure that the Makefile in
otherlibs/unix and otherlibs/graph could be used even under Windows
and in that case automatically delegate to otherlibs/win32unix and
otherlibs/win32graph to produce the required artifacts. These
win32-specific directories could then be moved e.g. to
otherlibs/unix/win32 and otherlibs/graph/win32 to make it clear that
they are really alternative implementations of the same library. This
might allow some more sharing between the build instructions for the
Unix and Windows variants.

Could these be implemented in separate commits?
Would it be okay to just add a TODO comment in the makefiles?

@alainfrisch

This comment has been minimized.

Show comment
Hide comment
@alainfrisch

alainfrisch Aug 31, 2016

Contributor

A make -f Makefile.nt clean (before any build) now fails with

make[1]: Entering directory '/home/frisch/ocaml/trunk/otherlibs/win32graph'
../Makefile:146: .depend.nt: No such file or directory
Contributor

alainfrisch commented Aug 31, 2016

A make -f Makefile.nt clean (before any build) now fails with

make[1]: Entering directory '/home/frisch/ocaml/trunk/otherlibs/win32graph'
../Makefile:146: .depend.nt: No such file or directory
@alainfrisch

This comment has been minimized.

Show comment
Hide comment
@alainfrisch

alainfrisch Aug 31, 2016

Contributor

Would it be okay to just add a TODO comment in the makefiles?

Yes, of course.

Contributor

alainfrisch commented Aug 31, 2016

Would it be okay to just add a TODO comment in the makefiles?

Yes, of course.

@shindere

This comment has been minimized.

Show comment
Hide comment
@shindere

shindere Aug 31, 2016

Contributor

Alain Frisch (2016/08/31 00:45 -0700):

A make -f Makefile.nt clean (before any build) now fails with

make[1]: Entering directory '/home/frisch/ocaml/trunk/otherlibs/win32graph'
../Makefile:146: .depend.nt: No such file or directory

Ah indeed, sorry.

It would actually be possible to make sure .depend is not included when
cleaning, but that would not be enough because in its present state
win32graph does not use any .depend file.

So my suggestion would be to renounce to the factorizatio commit for
now, because this would lead further than was initially intended.

What do you think?

Contributor

shindere commented Aug 31, 2016

Alain Frisch (2016/08/31 00:45 -0700):

A make -f Makefile.nt clean (before any build) now fails with

make[1]: Entering directory '/home/frisch/ocaml/trunk/otherlibs/win32graph'
../Makefile:146: .depend.nt: No such file or directory

Ah indeed, sorry.

It would actually be possible to make sure .depend is not included when
cleaning, but that would not be enough because in its present state
win32graph does not use any .depend file.

So my suggestion would be to renounce to the factorizatio commit for
now, because this would lead further than was initially intended.

What do you think?

@xavierleroy

This comment has been minimized.

Show comment
Hide comment
@xavierleroy

xavierleroy Aug 31, 2016

Contributor

I don't understand the specific issue with win32graph, but would like to say:

  • This refactoring of makefiles is great and something we should have done a long time ago, so please don't give up easily, and let us all try to help @shindere complete it.
  • otherlibs/win32graph, like otherlibs/graph, should eventually be removed from the core distribution and live and be distributed as a separate project. Maybe it is time to do that.
Contributor

xavierleroy commented Aug 31, 2016

I don't understand the specific issue with win32graph, but would like to say:

  • This refactoring of makefiles is great and something we should have done a long time ago, so please don't give up easily, and let us all try to help @shindere complete it.
  • otherlibs/win32graph, like otherlibs/graph, should eventually be removed from the core distribution and live and be distributed as a separate project. Maybe it is time to do that.
@alainfrisch

This comment has been minimized.

Show comment
Hide comment
@alainfrisch

alainfrisch Aug 31, 2016

Contributor

What do you think?

Yes, ok.

Contributor

alainfrisch commented Aug 31, 2016

What do you think?

Yes, ok.

@shindere

This comment has been minimized.

Show comment
Hide comment
@shindere

shindere Aug 31, 2016

Contributor

Alain Frisch (2016/08/31 03:14 -0700):

What do you think?

Yes, ok.

So this factorization commit is now gone.

I added a note in otherlibs/win32unix suming up our discussion but I
left the Makefile in otherlibs/win32graph unchanged for now.

Contributor

shindere commented Aug 31, 2016

Alain Frisch (2016/08/31 03:14 -0700):

What do you think?

Yes, ok.

So this factorization commit is now gone.

I added a note in otherlibs/win32unix suming up our discussion but I
left the Makefile in otherlibs/win32graph unchanged for now.

@shindere

This comment has been minimized.

Show comment
Hide comment
@shindere

shindere Aug 31, 2016

Contributor

Xavier Leroy (2016/08/31 02:07 -0700):

  • otherlibs/win32graph, like otherlibs/graph, should eventually be
    removed from the core distribution and live and be distributed as a
    separate project. Maybe it is time to do that.

I can take care of that if you like. I'd just need to make sure what it
involves technically, in addition to removing the directories and all
the references to them in other files, or perhaps just leaving a
directory with one file saying it has been removed, as is done for
ocamlbuild I think.

Contributor

shindere commented Aug 31, 2016

Xavier Leroy (2016/08/31 02:07 -0700):

  • otherlibs/win32graph, like otherlibs/graph, should eventually be
    removed from the core distribution and live and be distributed as a
    separate project. Maybe it is time to do that.

I can take care of that if you like. I'd just need to make sure what it
involves technically, in addition to removing the directories and all
the references to them in other files, or perhaps just leaving a
directory with one file saying it has been removed, as is done for
ocamlbuild I think.

shindere added some commits Aug 23, 2016

In otherlibs/win32{graph,unix}: normalize the buid system.
Since these directories are built only on Windows, they contained
only a Makefile.nt. For each directory, this commit moves the
content of its Makefile.nt to Makefile and creates a Makefile.nt
that includes the corresponding Makefile.
@shindere

This comment has been minimized.

Show comment
Hide comment
@shindere

shindere Aug 31, 2016

Contributor

Alain Frisch (2016/08/31 00:48 -0700):

Would it be okay to just add a TODO comment in the makefiles?

Yes, of course.

Done in win32unix. I didn't touch win32graph yet, in case it is decided
to remove it from the distribution.

Contributor

shindere commented Aug 31, 2016

Alain Frisch (2016/08/31 00:48 -0700):

Would it be okay to just add a TODO comment in the makefiles?

Yes, of course.

Done in win32unix. I didn't touch win32graph yet, in case it is decided
to remove it from the distribution.

@damiendoligez damiendoligez merged commit 98549a1 into ocaml:trunk Sep 5, 2016

2 checks passed

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

@shindere shindere deleted the shindere:merge-otherlibs-makefiles branch Oct 6, 2016

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

Merge pull request #788 from shindere/merge-otherlibs-makefiles
Merge build systems in subdirectories of otherlibs.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment