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

Multiple emit targets don’t ignore `-o` option, contrary to the warning #20130

Closed
nagisa opened this Issue Dec 22, 2014 · 17 comments

Comments

Projects
None yet
9 participants
@nagisa
Copy link
Contributor

nagisa commented Dec 22, 2014

$ ls -alh
…
drwxr-xr-x 1 nagisa nagisa  26 Dec 22 11:32 src/
…
$ rustc src/lib.rs --emit=llvm-ir,llvm-bc,obj,asm,link --crate-type=rlib -o rdd
warning: ignoring specified output filename because multiple outputs were requested
$ ls -alh
total 136K
…
-rw-r--r-- 1 nagisa nagisa  86K Dec 22 16:41 liblib.rlib
-rw-r--r-- 1 nagisa nagisa 5.1K Dec 22 16:41 rdd.bc
-rw-r--r-- 1 nagisa nagisa  17K Dec 22 16:41 rdd.ll
-rw-r--r-- 1 nagisa nagisa 9.7K Dec 22 16:41 rdd.o
-rw-r--r-- 1 nagisa nagisa  16K Dec 22 16:41 rdd.s
drwxr-xr-x 1 nagisa nagisa  26 Dec 22 11:32 src/
…

Although compiler warns that output filename was ignored, only the link target ignores it. All other targets generate file with provided output filename.

@thorncp

This comment has been minimized.

Copy link
Contributor

thorncp commented Jan 17, 2015

6e7968b suggests that this behavior is intended. Specifically:

The -o option has also been redefined to be used for all flavors of outputs.
This means that we no longer ignore it for libraries.

I think the warning message is misleading, and is probably not even necessary. What do we think about removing this warning entirely, and updating rustc -h and the man page to reflect this behavior?

@nagisa

This comment has been minimized.

Copy link
Contributor Author

nagisa commented Jan 17, 2015

That commit message contradicts itself further down (and still doesn’t match the current behaviour):

If the -o flag is specified, and only one output type is specified, the
output will be emitted at this location. If more than one output type is
specified, then the filename of -o is ignored, and all output goes in the
directory that -o specifies.

The emphasis mine. What’s the expected behaviour here @alexcrichton?

@brson

This comment has been minimized.

Copy link
Contributor

brson commented Jan 17, 2015

I don't recall what was intended but it does seem that the current behavior is inconsistent and the warning message is wrong.

Based on the current behavior my preference is that if there are multiple output types that we name them all based on the provided file stem, meaning just change the library output to conform to the existing behavior.

@alexcrichton What do you think?

@nagisa

This comment has been minimized.

Copy link
Contributor Author

nagisa commented Jan 17, 2015

This is possibly a good time and place to brainstorm about this, since this is one of the cornerstones of compiler usage.

This is what sounds most sensible to me:

  • --out-dir never ignored;
  • -o is alias for --crate-name.
$ # This is what it would look like
$ rustc --crate-type=rlib --out-dir=target/ -o foobar test.rs --emit=[everything] && ls target/
foobar.o foobar.s foobar.ll foobar.bc libfoobar.rlib

It does not depend on any combination of flags and works predictably (though very incompatible to other compilers) well.

@alexcrichton

This comment has been minimized.

Copy link
Member

alexcrichton commented Jan 18, 2015

My thinking is the same as @brson's, the intended operation is something like:

// If foo.rs only generates one file, it will be called precisely `foo`
$ rustc foo.rs -o foo
// Use `foo` as a filestem for all outputs, ignore the precise filename "foo"
$ rustc --emit=[lots] foo.rs -o foo

The --out-dir options always takes precedence and will ignore -o entirely if specified, I believe. The major purpose of the -o flag is to compile executables such as rustc foo.rs -o bar.

@nagisa

This comment has been minimized.

Copy link
Contributor Author

nagisa commented Jan 18, 2015

I submitted a rust-lang/rfcs#595 regarding this.

@wthrowe

This comment has been minimized.

Copy link
Contributor

wthrowe commented Oct 8, 2015

Since we can't actually change this behavior anymore since it would be breaking, would a pull request that just gets rid of the incorrect warning be accepted?

@brson

This comment has been minimized.

Copy link
Contributor

brson commented Jun 27, 2016

Yes, let's change this so that, only if link is one of the requested, it says: "warning: ignoring output name requested with -o for "link" output because multiple outputs were requested".

In Rust 2 we should consider changing it so all output types are affected by the rename.

@Manishearth

This comment has been minimized.

Copy link
Member

Manishearth commented Jun 28, 2016

Willing to help anyone who wants to work on this

@hgallagher1993

This comment has been minimized.

Copy link
Contributor

hgallagher1993 commented Jul 6, 2016

Ya I'd take it

@brson

This comment has been minimized.

Copy link
Contributor

brson commented Jul 8, 2016

@hgallagher1993 You got it!

@brson

This comment has been minimized.

Copy link
Contributor

brson commented Jul 15, 2016

@hgallagher1993 How's it going?

@hgallagher1993

This comment has been minimized.

Copy link
Contributor

hgallagher1993 commented Jul 16, 2016

@brson I actually haven’t had a chance to even look at it, long hours in the office all last week. It's going to have my undivided attention for the next few days 😃 I'm probably going to need a bit of this as well @Manishearth

@brson

This comment has been minimized.

Copy link
Contributor

brson commented Jul 18, 2016

OK, thanks for the status update!

@hgallagher1993

This comment has been minimized.

Copy link
Contributor

hgallagher1993 commented Jul 23, 2016

@Manishearth I've been looking at this task and I was just wondering could I get some direction on how to start this...still completely new to the compiler 😄

@Manishearth

This comment has been minimized.

Copy link
Member

Manishearth commented Jul 25, 2016

So the warning is here:

sess.warn("ignoring specified output filename because multiple outputs were \

RIght now we warn if the number of unnamed output types is more than one. The actual condition is "if the number of output types is more than one AND if one of the unnamed emit options is link"

The emit options are stored in a hashmap in sess.opts.output_types. The current code just looks for hashmap entries that exist but have a None value. What we need to look for is an entry where the key is Exe and the value is None.

Something like && sess.opts.output_types.get(Exe).is_none in that if should fix it. You will need to update the text to that from #20130 (comment) too.

bors added a commit that referenced this issue Jan 9, 2017

Auto merge of #38840 - kjaleshire:multiple-targets-error-fix, r=nrc
Warn that the link target ignores the given name

Hi, new contributor here. This is my stab at #20130, any feedback welcome!
@kjaleshire

This comment has been minimized.

Copy link
Contributor

kjaleshire commented Jan 24, 2017

I believe this issue is resolved. See #38840 .

@nagisa nagisa closed this Jan 24, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.