search

adrian-thurston / colm

The Colm Programming Language
MIT License
164 stars 31 forks source link

if buildDir is not set with -B, try to find a default #144

Closed adrian-thurston closed 2 years ago

adrian-thurston commented 2 years ago

When running from the build directory, if one forgets to give -B, and colm was previously installed at the current prefix, colm programs will build against the installed version, not the local source tree. This won't be immediately obvious and could lead to confusion during development.

To prevent this, try to find the buildDir if one is not given. The tests can continue to specify the buildDir with -B, which is the most safe way.

The buildDir is defaulted by testing the buildDir known at compile time against LD_LIBRARY_PATH. Libtool on ubuntu 20.04 will place the buildDir at the head of LD_LIBRARY_PATH. It seems likely that it would do this on all systems, making it a seemingly good test.

adrian-thurston commented 2 years ago

@viccie30 I wanted to add this because I was running from the build directory, I forgot -B (old habits) and I got linked against a older version that was previously installed at the prefix. I didn't realize it and it led to some confusion.

adrian-thurston commented 2 years ago

I guess the only risk is if the buildDir happens to get reused for something else and that something else needs to go into LD_LIBRARY_PATH. Then the installed colm would fail to run.

viccie30 commented 2 years ago

I'm at my day job, so I haven't tested it yet, but just looking at the code I think it should work for building with shared libcolm enabled. If I'm not mistaken, libtool doesn't employ a wrapper script when only linking statically, meaning that this would break when building with --disable-shared.

I'll try something tonight and see if there's a better way, but I'm not sure.

viccie30 commented 2 years ago

Your code does indeed work when colm is built with --enable-shared and does not work when built with --disable-shared.

I've coded another possible implementation in https://github.com/viccie30/colm/commit/ab722e3fd51dbdd6cf9c8041f9ef0b4d789996ca. It's based on stat-calls and checks both the regular executable location and the .libs/_libs subdirectory.

I've tested that it runs the test suite successfully with both statically and dynamically linked colm executables, even when I delete the -B options I added in my earlier pull request.

Both mine and your implementations are a bit hackish, but I don't think there's a "proper" way to do this.

adrian-thurston commented 2 years ago

Very nice, I like it!

I think automake will use install and I don't think that will ever hard link. It wouldn't make much sense because build directories can be on any device. I suppose it could try and if that fails then copy. But I feel like install never does that. Some quick searching didn't turn up anything. The manpage also says it copies.

I suppose if if we find it does on some system we can also stat the installed binary and if it also matches then bail out of setting buildDir.

viccie30 commented 2 years ago

At the very least GNU coreutils doesn't use hard links, if I read the source right: https://github.com/coreutils/coreutils/blob/8931d571d06b9d7b01b47bca265996cfbb285cdc/src/install.c#L267.

Thanks for merging!

ryandesign commented 1 year ago

This fails in two situations I can see: when /proc is not mounted or doesn't exist, or when colm is installed using a hard link.

There are plenty of operating systems that don't have /proc; macOS is one.