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

Allow build scripts to output -C flags #1293

Closed
Manishearth opened this issue Feb 12, 2015 · 16 comments
Closed

Allow build scripts to output -C flags #1293

Manishearth opened this issue Feb 12, 2015 · 16 comments

Comments

@Manishearth
Copy link
Member

Currently, build scripts can only modify the -l and -L flags that are passed to rustc.

Would it be possible to include -C in those, at least -C link-args?

@alexcrichton
Copy link
Member

Usage of -C link-args is inherently not portable and -C has far too many options that build scripts should probably be deciding. What kind of flags do you have in mind that need to be passed?

@Manishearth
Copy link
Member Author

-C link-args="$LDFLAGS -lGLESv2 -L $GONKDIR/backup-flame/system/lib/"

where LDFLAGS="-mandroid -L$GONKDIR/out/target/product/$GONK_PRODUCT/obj/lib -Wl,-rpath-link=$GONKDIR/out/target/product/$GONK_PRODUCT/obj/lib --sysroot=$GONKDIR/out/target/product/$GONK_PRODUCT/obj/"

So, I'm using:

  • -mandroid
  • -L
  • -Wl,-rpath-link
  • --sysroot

within link-args.

Portability is the responsibility of the build script, isn't it? (figuring out what the system has and choosing flags on that basis)

@Manishearth
Copy link
Member Author

Right now our workflow is like this:

  • cargo build until we get link failures on the final binary
  • cargo build -v, copy the huge rustc invocation
  • call rustc manually and add the extra args

@alexcrichton
Copy link
Member

These do not necessarily seem like flags that a build script is responsible for:

-mandroid

Is this not necessary on all crates being compiled?

-L

Already taken care of :)

-Wl,-rpath-link

Is this actually necessary to run the resulting binary? Flags like this often assume some method of execution whereas other methods like LD_LIBRARY_PATH exist (or rpath in Cargo's config itself)

--sysroot

Like -mandroid, why is this only needed for one invocation, not all?

@larsbergstrom
Copy link

@michaelwu can you clarify the reason for some of those args in the Servo Gonk build?

@Manishearth
Copy link
Member Author

I ran the following:

LD_LIBRARY_PATH="$GONKDIR/out/target/product/$GONK_PRODUCT/obj/lib" ../../mach rustc $INFINITEARGS -lGLESv2 -L $GONKDIR/backup-flame/system/lib/ -L/home/manishearth/Mozilla/fxos/b2s/B2G/out/target/product/flame/obj/lib ;buildstat 

($INFINITEARGS is the original invocation)
and I get:

note: /home/manishearth/Mozilla/fxos/b2s/B2G/prebuilts/gcc/linux-x86/arm/arm-linux-androideabi-4.7/bin/../lib/gcc/arm-linux-androideabi/4.7/../../../../arm-linux-androideabi/bin/ld: error: cannot open crtbegin_dynamic.o: No such file or directory
/home/manishearth/Mozilla/fxos/b2s/B2G/prebuilts/gcc/linux-x86/arm/arm-linux-androideabi-4.7/bin/../lib/gcc/arm-linux-androideabi/4.7/../../../../arm-linux-androideabi/bin/ld: error: cannot open crtend_android.o: No such file or directory
collect2: error: ld returned 1 exit status

I guess one of them is necessary? I'll poke a bit further

@Manishearth
Copy link
Member Author

The sysroot was what we needed.

LD_LIBRARY_PATH="$GONKDIR/out/target/product/$GONK_PRODUCT/obj/lib" ../../mach rustc $INFINITEARGS -lGLESv2 -L $GONKDIR/backup-flame/system/lib/ -L/home/manishearth/Mozilla/fxos/b2s/B2G/out/target/product/flame/obj/lib  -C link-args="--sysroot=$GONKDIR/out/target/product/$GONK_PRODUCT/obj/"

@michaelwu
Copy link

I don't remember. Most if not all are derived from what the Android build system gives us, and it's very likely that every single thing is needed. -mandroid might be the only thing we can omit.

@michaelwu
Copy link

Hmm and I remember thinking the Android linker doesn't support rpath, but that was a long time ago when I was actually doing android linker hacks.

@Manishearth
Copy link
Member Author

What about sysroot? That's the only one we seem to really need; the rest can be moved into -L, -l, and LD_LIBRARY_PATH

@larsbergstrom
Copy link

@Manishearth also suggested that a more amenable solution might be to pass the array of -Ls to build.rs somehow. Not sure how. (env var seems like a bad idea since then it needs parsing again).

Right now, our alternatives appear to be:

  1. emit a static library from cargo, then use a variety of shell scripts to re-find all of the dependencies that Cargo already knew about to construct the -L paths and string that into a custom Makefile build step

  2. allow -C link-args so that cargo can directly build the binary

@sondrele
Copy link

Similar issue here. I'm building for an ARM core and the last step involves passing -C link-args to rustc.
This is automated with a Makefile, so it's not a major issue, but it would have been nice if it was possible to directly with cargo.

@larsbergstrom
Copy link

It occurs to me there is another option:

  1. Switch from Cargo deps to git submodules with path= overrides. That would allow us to know the paths without doing the shell script magic from option Say no to sexism, say no to TOML! #1 and we could feed that into a custom Makefile.

@Manishearth
Copy link
Member Author

We'd still need to collect all the -l flags. Makes more sense to directly hardcode it, in that case.

FWIW I tried to build it statically (needed these changes ). It's quite hackish and the final binary just crashes (probably due to the hacks). I'm probably doing something wrong here, but it shouldn't be this hard anyway.

There's nothing stopping the programmer from shelling out to a makefile and calling rustc from there with the necessary link args; so we're not really adding new, unstable, functionality if we add link-args to the rustc-flags options. We're just making it easier so that the programmer doesn't have to duplicate the efforts of Cargo in gathering the rest of the link args.

We've already got two examples of where current rustc-flags support is inadequate; I really think that adding link-args to it would be a good idea. Even if its just allowed for the final binary (avoiding crosscrate issues). Alternatively, as mentioned before, provide the rustc invocation to the build script, preferably in a structured form.

Admittedly, in Servo's case, there is a workaround: Use bash or python to parse the output, extract the invocation, and rerun it. But this seems worse and even less portable/stable.

@kennytm
Copy link
Member

kennytm commented Feb 15, 2015

cc #544.

@alexcrichton
Copy link
Member

Ok, I'm going to close this as a dupe of #595 and #544 in this case. This is basically a direct dupe of #544 (as it covers flags in general) and I suspect that #595 would alleviate compilation here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants