Remove extern mod foo (name="bar") syntax, closes #9543

[fhahn]

This patch for #9543 throws an obsolete syntax error for extern mod foo (name="bar") .
I was wondering if this is the correct place to do this?

I think the wording of the error message could probably be improved as well.

If this approach is OK, I'm going to run the whole test suite tomorrow and update the old syntax to the new one.

[fhahn]

I've updated to obsolete syntax in the tests.

One thing I was wondering about: should extern mod bar(name = "std", vers = "0.9-pre"); behave like extern mod bar = "std"; for the name resolution?

[metajack]

It's a package id, so the most specific one would be extern mod bar = "std#0.9-pre";

[fhahn]

Does this mean we could get rid of the whole extern mod bar(x=y, a=b); syntax? I'm not sure if #9543 was only limited to extern mod bar(name = foo);.

[metajack]

I thought this was the plan. My PR for stable crate hashes removes the tests for this functionality since link args is getting removed there.

[fhahn]

Ah I see!

So I guess I should wait until your PR (#10593 , if I'm not wrong) is merged and remove the extern mod bar() syntax then?

[metajack]

You can remove it now if you like. I can rebase my PR over yours when it lands. They are orthogonal changes that happen to overlap in removing a few of the same tests :)

[alexcrichton]

It seems a little odd to only remove extern mod(name = "bar") yet still allow extern mod(name = "bar", foo = "baz"). We should either completely support linkage attributes or completely deny them I would imagine.

[fhahn]

That seemed odd to me as well.

I've updated my patch and removed the parsing of linkage attributes altogether and display a obsolete syntax message instead.
In order to compile rust with this patch, we'll need @metajack 's changes, because a linkage version is used in some files.

[alexcrichton]

@metajack, @brson, this was the agreed-upon path forward for pkgid, right?

[metajack]

@alexcrichton Yes. I was planning to make the old syntax an error instead of a warning, but that is probably something that should be decided by Rust core team. In any case, this is better than the silent ignoring we have now.

[fhahn]

@metajack this version of the patch gives an error fhahn@c199905#diff-da9d34ca1d0f6beee2838cf02e07345cR4433

[alexcrichton]

This needs a rebase again, and I would be comfortable merging this if it also removes the array of attributes from view_item_extern_mod.

[fhahn]

I've rebase this PR and removed the arry of attributes.

[alexcrichton]

Ah now I remember something I remembered awhile ago, these need to change to link to the proper libstd. Right now this is just linking to an arbitrary libstd but we want it to target a specific version (in this case 0.9-pre). The extern mod statements below should not have None, but rather Some("std#0.9-pre") (or the equivalent thereof)

[fhahn]

I've pushed another commit, addressing this point.

[alexcrichton]

Just a few more minor comments. With this addressed and a squash into one commit, looks good to me!

[fhahn]

After injecting the std libs with version numbers, I noticed a problem with rustpkg. I've pushed a fix for rustpkg to work with versioned std libs, but there is an inconsistency with the version handling in rustpkg. rustpkg::version::split_version returns versions like 0.9-pre, but this version string isn't a valid version for try_parsing_version. In my opinion, both functions should return compatible versions.

I've squashed everything into two commits, one for the syntax removal and one for the injection with versions, because they seem like two distinct changes to me, but I could squash them into one big commit.

[fhahn]

It seems like I forgot to run make check before pushing the most recent version, one line was too long. I've pushed an update version.