Skip to content

make install: remove old files to account for new packed stdlib#1724

Merged
nojb merged 1 commit intoocaml:trunkfrom
nojb:fix_stdlib_install
Apr 16, 2018
Merged

make install: remove old files to account for new packed stdlib#1724
nojb merged 1 commit intoocaml:trunkfrom
nojb:fix_stdlib_install

Conversation

@nojb
Copy link
Copy Markdown
Contributor

@nojb nojb commented Apr 13, 2018

See MPR#7773. The module M from the standard library is now contained in modules called stdlib__m.cm*, so they do not override the "old" modules m.cm*, which confuses the compiler. Similarly, pervasives is not overwritten by the new stdlib.

So let's erase the old files when doing make install to avoid any conflict.

stdlib/Makefile Outdated
install::
# Transitional: when upgrading from 4.06 -> 4.07, module M is in stdlib__m.cm*, while previously
# it was in m.cm*, which confuses the compiler. Also, pervasives.* is now stdlib.*.
rm -f $(patsubst stdlib__%,"$(INSTALL_LIBDIR)/%", $(filter stdlib__%,$(wildcard *.cm*)))
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Why not $(OBJS) rather than $(wildcard *.cm*)? If you run make world.opt install for instance, I'm not entirely sure $(wildcard *.cm*) will be interpreted as expected

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Indeed, using $(OBJS) is much better. Fixed.

@ghost ghost self-assigned this Apr 16, 2018
@nojb nojb force-pushed the fix_stdlib_install branch from 507e903 to 7373f56 Compare April 16, 2018 09:32
@nojb nojb merged commit fe9a521 into ocaml:trunk Apr 16, 2018
@nojb
Copy link
Copy Markdown
Contributor Author

nojb commented Apr 16, 2018

This is d231743 in 4.07.

@nojb nojb deleted the fix_stdlib_install branch April 16, 2018 10:30
@nojb
Copy link
Copy Markdown
Contributor Author

nojb commented Apr 16, 2018

I think we should also delete $(INSTALL_LIBDIR)/bigarray.* to avoid problems if one is not installing the bigarray library.

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.

1 participant