Skip to content
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

rdmd: Add --objdir option #350

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

CyberShadow
Copy link
Member

Sometimes, when messing with various toolchains / debugging linking problems, it's useful to look at the object files generated by the compiler.

Currently, rdmd unconditionally places them in a temporary directory, and (also unconditionally) deletes them on exit. -od is never passed on to the compiler, instead it's usurped by rdmd to specify the location where to place the final executable.

This introduces a new switch which allows specifying a custom directory to place object files in.
A check was already present to ensure we don't clean up a directory outside the temporary work directory (which might contain other files), which previously apparently only had an effect with -lib (due to a workaround for how dmd parses -lib paths).

@dlang-bot
Copy link
Contributor

Thanks for your pull request, @CyberShadow!

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub fetch digger
dub run digger -- build "master + tools#350"

@wilzbach
Copy link
Member

wilzbach commented May 8, 2018

Looks like gdmd/gdc needs a patch for this:

[execute] ["/tmp/rdmd_app_", "--compiler=/home/travis/dlang/gdc-4.8.5/bin/gdmd", "-m64", "--build-only", "--force", "-lib", "-of=rdmdTestLib/altlibtest.a", "rdmdTestSrc/test.d"]
[execute] ["/tmp/rdmd_app_", "--compiler=/home/travis/dlang/gdc-4.8.5/bin/gdmd", "-m64", "--build-only", "--force", "--objdir=rdmdTestObj", "rdmdTestSrc/test.d"]
object.Exception@rdmd_test.d(565): Enforcement failed

@CyberShadow
Copy link
Member Author

No idea what's wrong, it works with both dmd and ldmd2 on my machine. gdmd fails with an unrelated error:

gdc: error: unrecognized command line option ‘-fd-verbose’; did you mean ‘--verbose’?

@WebDrake Any idea what's wrong?

@WebDrake
Copy link
Contributor

I suspect @ibuclaw could give you more insight ;-) Since gdmd is just a perl script mapping dmd-alike options to gdc options, I presume that something is going wrong with the -v option that rdmd uses to generate dependencies (i.e. it's mapping to an option that is not supported, or no longer supported, by the gdc version you're using).

@joseph-wakeling-sociomantic

BTW (... he said, merging into his suave work persona ...), why add a separate --objdir option, rather than reworking how -od is interpreted? It should be a matter of making sure that the directory supplied via -od is parsed into a separate variable (rather than exe) and then reworking the couple of bits of logic where rdmd behaves currently differently depending on whether exe contains a file path or a directory path.

@CyberShadow
Copy link
Member Author

Sorry, I don't follow. How would you then tell rdmd to put the object files in one directory, and the final executable in another?

@joseph-wakeling-sociomantic

Wouldn't -offull/path/to/exe -odobjdir work ... ?

@CyberShadow
Copy link
Member Author

I guess, but that will still break the existing meaning of -od.

-od also has the advantage that you don't need to calculate the full output filename, which is useful for scripting, or even short-hand invocations (rdmd -od. file.d to build and run in the current directory).

@joseph-wakeling-sociomantic

I guess, but that will still break the existing meaning of -od.

For rdmd or for dmd?

I would suggest it is a bug if rdmd does not handle the -od option exactly like dmd does. It's essentially an implementation detail that rdmd has to parse the -o... options rather than passing them straight on to dmd. They should all however be passed on to dmd, and it's a bug that -od... isn't if an -of... is also defined.

@CyberShadow
Copy link
Member Author

I guess, but I don't think it's worth breaking things to fix it at this point.

@joseph-wakeling-sociomantic

What do you fear will actually break? I know I'm usually the one pushing on no breaking change, but the practical upshot of the current behaviour is that one can't use -of... and -od... together with rdmd, so people won't.

So it should be innocuous to add the possibility of actually having -of... and -od... together, and having things behave Just Like DMD.

@CyberShadow
Copy link
Member Author

OK, but what about the case of -od without -of?

@joseph-wakeling-sociomantic

Doesn't that behave normally with rdmd anyway, just as if one invoked the corresponding dmd -od... call?

@joseph-wakeling-sociomantic

There's no reason why the kind of change I'm proposing should affect that.

@CyberShadow
Copy link
Member Author

Doesn't that behave normally with rdmd anyway, just as if one invoked the corresponding dmd -od... call?

Um, no? rdmd -od currently puts the executable file in the specified directory, and the object files still in the temporary directory. dmd -od puts both in the specified directory.

There's no reason why the kind of change I'm proposing should affect that.

Sorry, are you proposing that whether -of is present or not changes the meaning of -od?

@joseph-wakeling-sociomantic

Um, no? rdmd -od currently puts the executable file in the specified directory, and the object files still in the temporary directory. dmd -od puts both in the specified directory.

Well, I think that behaviour of rdmd -od... is arguably a bug. Do we have any reason to suppose that anyone is relying on that subtlety?

Bear in mind that -od is not an rdmd option (it's not covered in the application help). It's a dmd option. So to have it diverge in any way from normal dmd behaviour seems problematic. And dmd's help output is quite explicit that -od... is intended to specify the directory where object files will be placed.

@CyberShadow
Copy link
Member Author

Well, I think that behaviour of rdmd -od... is arguably a bug.

I don't think it's relevant at this point.

Do we have any reason to suppose that anyone is relying on that subtlety?

I wouldn't call that a subtlety. It's too obvious of a part of the interface to change at this point.

Bear in mind that -od is not an rdmd option (it's not covered in the application help). It's a dmd option. So to have it diverge in any way from normal dmd behaviour seems problematic. And dmd's help output is quite explicit that -od... is intended to specify the directory where object files will be placed.

I think the proper course of action would be to improve the documentation at this point.

If we wanted to fix -od, we would probably have to first introduce a new switch with the same mechanics as current -od (i.e. --exe-dir OSLT), wait a few releases, and then either deprecate -od with the intent to change its meaning after a few more releases, or just change its meaning then and hope nothing breaks too badly. None of this seems worthwhile to me.

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.

5 participants