Add '--dmd' and '--dc' to print the path of the installed compiler by wilzbach · Pull Request #297 · dlang/installer

wilzbach · 2018-02-15T00:57:02Z

This has come up frequently in the past.

Use case: one has a generic compiler variable like $DMD in a CI script and wants to use the install script without activating the environment.

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.

script/install.sh

changelog/where.dd

WebDrake · 2018-02-15T01:35:06Z

My questions seem to have got lost with the updated patch. I'll summarize: why make where install a compiler if not found, rather than just report (in some appropriate way) that it is not found?

The simplest solution would probably be that where produces no output if the compiler is not installed, or the install location if it is.

Then, if in some script it's wanted that a compiler be installed if not found, that could be implemented by querying where and issuing a separate install command if necessary.

wilzbach · 2018-02-15T01:39:12Z

My questions seem to have got lost with the updated patch.

No worries. They weren't lost. I was about to reply to them.

I'll summarize: why make where install a compiler if not found, rather than just report (in some appropriate way) that it is not found?

The updated patch removed where as a separate option. It now behaves analog to -a | --activate which is only an option for install (the default command) and will print the path to the sourceable activate script.

I hppe this answers all your questions as --where can't be called with anything else than the install command?

WebDrake · 2018-02-15T01:48:25Z

I'm not sure it really feels like the right answer. Could we take a pause on this and revisit tomorrow? I need some sleep ;-)

I don't think it really satisfies what I see as my use-case. I would say that there are 2 different options that would satisfy what I'm looking for:

provide an alternative flag for install that allows the installed compiler to be added to the PATH without overwriting all the other environment vars like DC, DMD, etc. This feels a bit finnicky, though, because it could get tricky to track everything that's been added to the path (or not).
provide an easy way to query the location of a named installed compiler, that would give the location if it's installed, or no output if not.

Bottom line, I like the idea of the where command. I just think it would satisfy my needs better if it just behaved as per the second bullet point above. Is there a reason why that's difficult or problematic?

wilzbach · 2018-02-15T02:03:00Z

I'm not sure it really feels like the right answer. Could we take a pause on this and revisit tomorrow? I need some sleep ;-)

No worries. There's quite a bottleneck for installer (it's only @MartinNowak who can effectively approve PRs), so the review backlog is quite long - https://github.com/dlang/installer/pulls

I don't think it really satisfies what I see as my use-case.

I'm not sure what's your use case, but it satisfies the need for dlang/dmd and dlang/tools.

DMD:

Previously:

source ~/dlang/install.sh "$DMD"
HOST_DMD=$DMD"
deactivate

Now

HOST_DMD=$(~/dlang/install.sh --where "$DMD")

tools

~/dlang/install.sh install ldc
LDMD2=$(find ~/dlang -type f -name "ldmd2")

now:

LDMD2=$(~/dlang/install.sh install --where ldc)

provide an alternative flag for install that allows the installed compiler to be added to the PATH without overwriting all the other environment vars like DC, DMD, etc. This feels a bit finnicky, though, because it could get tricky to track everything that's been added to the path (or not).

Not a huge fan of this. It's also more complicated than this PR.

provide an easy way to query the location of a named installed compiler, that would give the location if it's installed, or no output if not.

How would that work with ~/dlang/install install ldc?
I mean sure you could write a /compiler_path file in the root directory of the ldc archive and then do a dirname on the output of --activate, but then we would break code if we ever change the location of the activate script.

joseph-wakeling-sociomantic · 2018-02-15T11:04:58Z

On thinking it over: yes, as a flag to install this is probably the best way of doing things. It means that the answer is only possible at the time of install, and that's a useful simplification, because it means that there's no need to keep state hanging around, or rely on assumed rules about where compilers get placed.

A separate where command could work on the grounds that the install location for a given specified compiler is 100% deterministic (for the install script) based on the compiler name. But of course this is not quite true when one considers generic ldc, dmd, gdc as opposed to specified versions. So best avoided.

Something that's not clear from your above examples: would the result be the directory where the compiler lives, or the full path to the compiler executable? I ask because:

LDMD2=$(find ~/dlang -type f -name "ldmd2")

versus

LDMD2=$(~/dlang/install.sh install --where ldc)

... do not seem implicitly to be the same thing. After all, with LDC and GDC there are two compiler executables installed, with 2 different names (ldc2 and ldmd2, and gdc and gdmd).

script/install.sh

WebDrake · 2018-02-15T21:17:57Z

script/install.sh

+                    log "
+Run \`source     $ROOT/$2/activate${suffix:-}\` in your shell to use $2.
+This will se    tup PATH, LIBRARY_PATH, LD_LIBRARY_PATH, DMD, DC, and PS1.
+Run \`deacti    vate\` later on to restore your environment."


What's the reason for the weird whitespace on these lines?

changelog/where.dd

wilzbach · 2018-02-16T07:22:38Z

Something that's not clear from your above examples: would the result be the directory where the compiler lives, or the full path to the compiler executable? I ask because:
...
... do not seem implicitly to be the same thing. After all, with LDC and GDC there are two compiler executables installed, with 2 different names (ldc2 and ldmd2, and gdc and gdmd).

Contrary to the previous changelog entry, they were doing the same thing (--where was returning <path-to-ldc-bin>/ldmd2). Sorry for the confusion!

However, after thinking about this, I think I found an even better solution with --dmd and --dc.
This is analog to the existing $DMD and $DC variables that this install script exposes.
I like this more because it's explicit what interface the user wants and no dirname hacks need to be done to switch the interface of the compiler from ldmd to ldc.

changelog/install-dmd-dc.dd

joseph-wakeling-sociomantic · 2018-02-16T10:31:38Z

changelog/install-dmd-dc.dd

+
+$(H4 Examples)
+
+$(CONSOLE curl https://dlang.org/install.sh | bash -s --dmd


Not sure it's really helpful to put in this curl stuff. It distracts from the example of how to use the install.sh script itself.

It's how you get the installer script and would use it for a CI script.
It also provides a seamless experience for readers who want to try out the new flags as they can just copy/paste the command.

Sure, but it would be clearer if the example was annotated to that effect, e.g. download and run install script, printing out the installed compiler.

joseph-wakeling-sociomantic · 2018-02-16T10:32:26Z

changelog/install-dmd-dc.dd

+print information about the installed compiler binary:
+
+$(UL
+    $(LI `--dmd` returns the path of a specific compiler executable (DMD-like interface))


What will this be if the compiler does not expose a DMD-like interface?

I don't think SDC will grow into a full compiler any time soon and even if, for a seamless experience it most likely will implement the DMD interface (IIRC it already does so).
The same applies for other compilers and if there's ever a case in a far far far distant future where we would want to include a new compiler, but are too lazy to wrap the DMD interface around it, fatal error is the Unix way.
Also as mentioned this is the changelog entry - not the documentation, so imho there's no need to go into details here.

Also as mentioned this is the changelog entry - not the documentation, so imho there's no need to go into details here.

I didn't ask for the changelog to be altered. I asked what the result will be if the compiler does not expose a DMD-like interface.

This can't happen as install.sh only supports GDC, DMD and SDC. If there's ever a new compiler which would be added to install.sh, it would fail, I presume.

WebDrake · 2018-02-18T14:01:43Z

script/install.sh


  -a --activate     Only print the path to the activate script
+  --dmd             Only print the path to the compiler binary (e.g. dmd, ldmd2, gdmd)
+  --dc              Only print the path to the compiler binary (e.g. dmd, ldc, gdc)


What will happen if I use both --dmd and --dc together?

It looks like this documentation could be clearer about the distinction between the flags, i.e. that one is for the DMD-alike compiler binary

Yep, conceptually these are actions, not flags.

MartinNowak · 2018-02-18T21:52:22Z

HOST_DMD=$(~/dlang/install.sh --where "$DMD")

Sorry, I've only grokked part of the discussion.

Just using the compiler without the environment is not reliable, as the sc.ini/dmd.conf in your user home take precendence over the ones next to the compiler binary :/.
I'm still convinced that a lot of non-sense is rooted on this single problem. Why would a makefile need an absolute path to the compiler? Just so that it can for usage of -conf= or -conf=path/to/dmd.conf.
dlang/dmd#4256

wilzbach · 2018-02-19T06:05:46Z

Just using the compiler without the environment is not reliable, as the sc.ini/dmd.conf in your user home take precendence over the ones next to the compiler binary :/.
I'm still convinced that a lot of non-sense is rooted on this single problem.

Urgh I wasn't even aware of this. Let's fix this mess then -> dlang/dmd#7915

Sorry, I've only grokked part of the discussion.

The idea is to have a --dmd and --dc flag which returns the location of the compiler binary (they return the same output as $DMD and $DC), s.t

the environment doesn't need to be loaded which turns out is very error-prone (e.g. our recent CI fiasco at dmd)
multiple compiler can be installed and used in parallel without needing to switch environments (in @WebDrake's use case for dlang/tools the rdmd testsuite even accepts multiple compiler binaries)

Just using the compiler without the environment is not reliable, as the sc.ini/dmd.conf in your user home take precendence over the ones next to the compiler binary :

Well, that wouldn't be a huge problem for CIs (the main motivation for this feature), but of course one could always do sth. like this with dirname:

DMD=$(~/dlang/install.sh 2.078.3  --dmd)
make DMD="$DMD" DFLAGS="-conf="$(dirname "$DMD")/dmd.conf)"

In any case the motivation was that --dmd is cleaner and less prone to errors than find ~/dlang -type f -name "ldmd2" which is currently used.

CyberShadow

Good idea except for the flag/action confusion.

Instead of install.sh install --dc, install.sh get-dc-path --install seems more logical.

CyberShadow · 2018-11-25T15:15:45Z

changelog/install-dmd-dc.dd

@@ -0,0 +1,27 @@
+`~/dlang/install.sh` now supports `--dmd` and `--dc`


These are not options, they're actions. They should be named "get-dmd-path" and "get-dc-path" OSLT.

Am I missing something?

CyberShadow · 2018-11-25T15:17:10Z

script/install.sh


  -a --activate     Only print the path to the activate script
+  --dmd             Only print the path to the compiler binary (e.g. dmd, ldmd2, gdmd)
+  --dc              Only print the path to the compiler binary (e.g. dmd, ldc, gdc)


Yep, conceptually these are actions, not flags.

AndrewEdwards · 2020-09-06T21:30:40Z

@wilzbach @WebDrake @joseph-wakeling-sociomantic @CyberShadow @MartinNowak

What is the current stand on this? Is there actually something preventing this from being merged? With the exception of the requested change from "--dmd" and "--dc" to "get-dmd-path" and "get-dc-path" all other concerns appears to have been addressed. @wilzbach can you address @CyberShadow's concern and resolve the conflicts to prepare this for merger? @MartinNowak, what other concerns if any do you have about this pull request?

wilzbach · 2020-09-07T00:39:33Z

@wilzbach can you address @CyberShadow's concern and resolve the conflicts to prepare this for merger?

Ok. I reworked it. I combined them under a single get-path action and added support for getting the path to the requested dub binary as well.

CyberShadow · 2020-09-07T01:29:08Z

Is it possible to remove those annoying CodeCov annotations? They make the diff unreadable.

AndrewEdwards · 2020-09-07T01:31:42Z

I see what you mean. Sorry, not sure how to do that. I'll defer to @wilzbach.

CyberShadow · 2020-09-07T01:36:54Z

for (var n of document.querySelectorAll('.check-annotation')) n.parentElement.remove(n);

in the console does it.

AndrewEdwards · 2020-09-07T01:43:49Z

I'll need to set up my environment first. Currently just using the web interface. As a temporary fix, you can disable them per file by unchecking the show annotations option.

CyberShadow · 2020-09-07T01:44:49Z

Ah, thanks. I missed that. That's much simpler.

CyberShadow

Thank you. Good work on the thorough test script.

CyberShadow · 2020-09-07T01:42:21Z

test/common.sh

+    actual="$1"
+    expected="$2"
+    if [ "$actual" != "$expected" ] ; then
+        echo "Actual: $actual"
+        echo "Expected: $expected"
+        exit 1
+    fi


For this I would simply write diff -u <(cat <<< "$1") <(cat <<< "$2"), but this is OK too.

wilzbach · 2020-09-07T03:36:30Z

Wow. I didn't think this would ever get merged. Thanks!

I opened a documentation PR -> dlang/dlang.org#2849

WebDrake reviewed Feb 15, 2018

View reviewed changes

script/install.sh Outdated Show resolved Hide resolved

WebDrake reviewed Feb 15, 2018

View reviewed changes

changelog/where.dd Outdated Show resolved Hide resolved

wilzbach force-pushed the where branch from fa195ed to d711923 Compare February 15, 2018 01:32

wilzbach changed the title ~~Add 'where' - print the path of the installed compiler~~ Add '--where' - print the path of the installed compiler Feb 15, 2018

wilzbach force-pushed the where branch from d711923 to ffebd42 Compare February 15, 2018 01:43

wilzbach force-pushed the where branch from ffebd42 to 42ec04d Compare February 15, 2018 01:52

WebDrake reviewed Feb 15, 2018

View reviewed changes

script/install.sh Outdated Show resolved Hide resolved

WebDrake reviewed Feb 15, 2018

View reviewed changes

changelog/where.dd Outdated Show resolved Hide resolved

wilzbach force-pushed the where branch 6 times, most recently from c234904 to 7ad4054 Compare February 16, 2018 05:59

wilzbach changed the title ~~Add '--where' - print the path of the installed compiler~~ Add '--dmd' and '--dc' to print the path of the installed compiler Feb 16, 2018

joseph-wakeling-sociomantic reviewed Feb 16, 2018

View reviewed changes

changelog/install-dmd-dc.dd Outdated Show resolved Hide resolved

joseph-wakeling-sociomantic reviewed Feb 16, 2018

View reviewed changes

changelog/install-dmd-dc.dd Outdated Show resolved Hide resolved

joseph-wakeling-sociomantic reviewed Feb 16, 2018

View reviewed changes

WebDrake reviewed Feb 18, 2018

View reviewed changes

wilzbach mentioned this pull request Apr 5, 2018

Add support for rdmd in gdc + set $RDMD variable #317

Open

CyberShadow reviewed Nov 25, 2018

View reviewed changes

wilzbach mentioned this pull request Sep 3, 2020

CircleCi: use build.d and run.d directly dlang/dmd#11674

Merged

AndrewEdwards mentioned this pull request Sep 6, 2020

Improve the lookup sequence for finding the config file dlang/dmd#7915

Closed

wilzbach force-pushed the where branch 9 times, most recently from 9cc5e54 to b89c279 Compare September 7, 2020 00:38

wilzbach force-pushed the where branch from b89c279 to 8cdd931 Compare September 7, 2020 00:43

Add 'get-path` to print the path of the installed compiler

0974b88

wilzbach force-pushed the where branch from 8cdd931 to 0974b88 Compare September 7, 2020 00:50

AndrewEdwards requested a review from CyberShadow September 7, 2020 01:27

CyberShadow approved these changes Sep 7, 2020

View reviewed changes

AndrewEdwards added the auto-merge label Sep 7, 2020

dlang-bot merged commit 581c1b0 into dlang:master Sep 7, 2020

wilzbach mentioned this pull request Sep 7, 2020

install.sh: document get-path and ,dub dlang/dlang.org#2849

Merged

wilzbach deleted the where branch September 7, 2020 03:35


		$(H4 Examples)

		$(CONSOLE curl https://dlang.org/install.sh \| bash -s --dmd

		@@ -0,0 +1,27 @@
		`~/dlang/install.sh` now supports `--dmd` and `--dc`

Uh oh!

Conversation

wilzbach commented Feb 15, 2018

Uh oh!

dlang-bot commented Feb 15, 2018

Bugzilla references

Uh oh!

Uh oh!

Uh oh!

WebDrake commented Feb 15, 2018

Uh oh!

wilzbach commented Feb 15, 2018

Uh oh!

WebDrake commented Feb 15, 2018

Uh oh!

wilzbach commented Feb 15, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

DMD:

tools

Uh oh!

joseph-wakeling-sociomantic commented Feb 15, 2018

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

wilzbach commented Feb 16, 2018

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MartinNowak commented Feb 18, 2018

Uh oh!

wilzbach commented Feb 19, 2018

Uh oh!

CyberShadow left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AndrewEdwards commented Sep 6, 2020

Uh oh!

wilzbach commented Sep 7, 2020

Uh oh!

CyberShadow commented Sep 7, 2020

Uh oh!

AndrewEdwards commented Sep 7, 2020

Uh oh!

CyberShadow commented Sep 7, 2020

Uh oh!

AndrewEdwards commented Sep 7, 2020

Uh oh!

CyberShadow commented Sep 7, 2020

Uh oh!

CyberShadow left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wilzbach commented Sep 7, 2020

Uh oh!

wilzbach commented Feb 15, 2018 •

edited

Loading