Skip to content

Get rid of Makefile.nt in the yacc directory.#762

Merged
alainfrisch merged 1 commit intoocaml:trunkfrom
shindere:merge-yacc-makefiles
Aug 26, 2016
Merged

Get rid of Makefile.nt in the yacc directory.#762
alainfrisch merged 1 commit intoocaml:trunkfrom
shindere:merge-yacc-makefiles

Conversation

@shindere
Copy link
Contributor

The Makefile has been changed so that it also works on Windows.

As a consequence, in OCaml's main Makefile.nt, building things in the
yacc directory can now be done by invoking $(MAKE) rather than
$(MAKEREC).

@dra27
Copy link
Member

dra27 commented Aug 24, 2016

Is this sensible to do piecemeal? I though other instances simply leave Makefile.nt as a simple include once the main Makefile is updated?

@shindere
Copy link
Contributor Author

shindere commented Aug 24, 2016 via email

@alainfrisch
Copy link
Contributor

I think it's fine to eliminate duplication directory by directory (this is how we have been proceeding for some time). The question from @dra27 (I think) is whether we want to leave a tiny Makefile.nt which only includes Makefile. And when all Makefile.nt have been made trivial this way, we get rid of all of them at once. The benefit of keeping Makefile.nt is that developers under Windows can simply do "make -f Makefile.nt" (as they are used to do) anywhere, instead of having to do either that or just "make" depending on the directory.

@shindere
Copy link
Contributor Author

shindere commented Aug 24, 2016 via email

@shindere
Copy link
Contributor Author

shindere commented Aug 24, 2016 via email

@alainfrisch
Copy link
Contributor

Any idea on this?

Let's ask our git expert. @gasche?

@alainfrisch
Copy link
Contributor

Is it okay to replace the calls to $(MAKEREC) by calls to $(MAKE) (in the main Makefile.nt) right now (as is done in the two PRs)

I would have a slight preference for not doing it, in order to ensure that the Makefile.nt won't be removed too early by someone else who finds out that it is not used anymore.

@shindere
Copy link
Contributor Author

Alain Frisch (2016/08/24 03:01 -0700):

Is it okay to replace the calls to $(MAKEREC) by calls to $(MAKE) (in the main Makefile.nt) right now (as is done in the two PRs)

I would have a slight preference for not doing it, in order to ensure
that the Makefile.nt won't be removed too early by someone else who
finds out that it is not used anymore.

I get your point. I'm still unsure because not doing the replacement now
also means that the fact that the build system works without the
Makefile.nt is not verified. Moreover, I hope to complete the transition
in not too long (typically a few weeks, less than a month probably), so
I'm not sure how high the risk is that somebody accidentally removes a
file or so.

@dra27
Copy link
Member

dra27 commented Aug 24, 2016

Are you presently working on merging the entire build system (total elimination of Makefile.nt)?

@shindere
Copy link
Contributor Author

David Allsopp (2016/08/24 04:51 -0700):

Are you presently working on merging the entire build system (total
elimination of Makefile.nt)?

Yes. Unless there is an objection agains it, it seems to me it would be
better to have the two systems merged before starting the work on
autoconf.

Of course, any comment is more than welcome.

@dra27
Copy link
Member

dra27 commented Aug 24, 2016

Definitely no objection - that's awesome! It was on my list for post-OPAM, but no way I'd be getting to it before the middle of next year! If you need any help/testing (especially with the FlexDLL bootstrapping stuff), do ping me.

Personally, I'd put all the changes through as one pull request (with lots of commits), but that's just a preference.

@shindere
Copy link
Contributor Author

shindere commented Aug 24, 2016 via email

@shindere
Copy link
Contributor Author

shindere commented Aug 24, 2016 via email

@dra27
Copy link
Member

dra27 commented Aug 24, 2016

If it'd helpful (it'd certainly be interesting for me!), can we have a chat about the directions for this? It's an area that I've been musing on a lot but, with other pressures, not done a huge amount with, and it'd be good to know where everyone else's thoughts are too!

@shindere
Copy link
Contributor Author

shindere commented Aug 24, 2016 via email

@alainfrisch
Copy link
Contributor

@dra27 Can you comment on whether you'd prefer to keep calling Makefile.nt (instead of just Makefile) from the root Makefile.nt for those subdirectories where the merger is done?

@dra27
Copy link
Member

dra27 commented Aug 25, 2016

Yes, cf. lex/Makefile.nt and otherlibs/{raw_spacetime_lib,dynlink}/Makefile.nt.

@alainfrisch
Copy link
Contributor

not doing the replacement now also means that the fact that the build system works without the Makefile.nt is not verified

Considering that Makefile.nt is just an include of Makefile, this can hardly break. Considering @dra27 preference, I would suggest keeping the call to sub Makefile.nt files from the root one (i.e. use $(MAKEREC)).

Can you apply the change, also to the other similar PR?

yacc/Makefile has been changed so that it also works on Windows.
@shindere shindere force-pushed the merge-yacc-makefiles branch from 72f2224 to bb18f35 Compare August 25, 2016 09:15
@shindere
Copy link
Contributor Author

Alain Frisch (2016/08/25 00:06 -0700):

Considering that Makefile.nt is just an include of Makefile, this can
hardly break. Considering @dra27 preference, I would suggest keeping
the call to sub Makefile.nt files from the root one (i.e. use
$(MAKEREC)).

Okay.

Can you apply the change, also to the other similar PR?

Sure. Both PRs have now been updated and rebased.

Thanks a lot for your comments @alainfrisch and @dra27!

@alainfrisch alainfrisch merged commit d917ff6 into ocaml:trunk Aug 26, 2016
alainfrisch added a commit that referenced this pull request Aug 26, 2016
@shindere shindere deleted the merge-yacc-makefiles branch August 26, 2016 07:19
camlspotter pushed a commit to camlspotter/ocaml that referenced this pull request Oct 17, 2017
yacc/Makefile has been changed so that it also works on Windows.
camlspotter pushed a commit to camlspotter/ocaml that referenced this pull request Oct 17, 2017
EduardoRFS pushed a commit to esy-ocaml/ocaml that referenced this pull request Dec 17, 2021
stedolan pushed a commit to stedolan/ocaml that referenced this pull request Sep 21, 2022
EmileTrotignon pushed a commit to EmileTrotignon/ocaml that referenced this pull request Jan 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants