This is an archive of the discontinued Mercurial Phabricator instance.

wix: add a hook for a prebuild script to inject extra libraries
ClosedPublic

Authored by durin42 on Mar 21 2019, 8:55 AM.

Details

Summary

I need this to build packages for Google so we can bundle some
extensions in the installed image. My assumption is that this is most
interesting for the .msi images so I only wired it up there. I'm not
thrilled with the interface this provides, but it was an easy way to
retain debug messages on Windows while also having enough structure to
know what lines are actually module names for py2exe.

Still pending on my end: I need to bundle a couple of config files,
and at least one data file. I'm open to advice on how to do those
things, and how to do this better.

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

durin42 created this revision.Mar 21 2019, 8:55 AM

I like the flexibility. But I'm not super keen about the interface here. Using a script to inject custom options seems like it could be useful. But as it is currently implemented, the script simply prints out \0 delimited package names. So one UI wart is --extra-prebuild-script being a somewhat generic name but that script only emits package names. --extra-packages-script would be a better name.

What do you think about defining an --extra-package CLI argument that takes nargs=* or an --extra-packages that takes a comma-delimited list and then thread that through to the existing extra_packages keyword argument? That seems a bit simpler and easier to extend than scripts.

I like the flexibility. But I'm not super keen about the interface here. Using a script to inject custom options seems like it could be useful. But as it is currently implemented, the script simply prints out \0 delimited package names. So one UI wart is --extra-prebuild-script being a somewhat generic name but that script only emits package names. --extra-packages-script would be a better name.

The script also has to install packages into the virtualenv so they're findable.

What do you think about defining an --extra-package CLI argument that takes nargs=* or an --extra-packages that takes a comma-delimited list and then thread that through to the existing extra_packages keyword argument? That seems a bit simpler and easier to extend than scripts.

I'm open to having both an --extra-package *and* a script argument, if that's what you're proposing. Does that sound workable?

How about we add an argument to define the path(s) to the pip requirements file(s) to use? By default, it can use the requirements.txt in the repo. Would that solve your use case?

How about we add an argument to define the path(s) to the pip requirements file(s) to use? By default, it can use the requirements.txt in the repo. Would that solve your use case?

No, because I have some stuff which (out of tragic necessity at the moment) isn't pip installable, and I have to install it into the virtualenv by banging some rocks together. :(

How about we add an argument to define the path(s) to the pip requirements file(s) to use? By default, it can use the requirements.txt in the repo. Would that solve your use case?

No, because I have some stuff which (out of tragic necessity at the moment) isn't pip installable, and I have to install it into the virtualenv by banging some rocks together. :(

Bleh. So you do need the power of a full script here.

I'll queue with the naming change to "virtualenv populate script" or something along those lines.

How about we add an argument to define the path(s) to the pip requirements file(s) to use? By default, it can use the requirements.txt in the repo. Would that solve your use case?

No, because I have some stuff which (out of tragic necessity at the moment) isn't pip installable, and I have to install it into the virtualenv by banging some rocks together. :(

Bleh. So you do need the power of a full script here.

Yep. :(

I'll queue with the naming change to "virtualenv populate script" or something along those lines.

Thanks.

indygreg requested changes to this revision.Apr 2 2019, 1:41 PM

Was a newer version of this patch with the requested changes going to be uploaded? It's weird to see additional patches in the series without this one updated...

This revision now requires changes to proceed.Apr 2 2019, 1:41 PM

Was a newer version of this patch with the requested changes going to be uploaded? It's weird to see additional patches in the series without this one updated...

You had indicated up-thread you were going to queue it, possibly with some in-flight tweaks. I had to rebase it as I was working on follow-ups, but I was merely waiting for you to land the patch as you'd indicated.

Are you now saying you want me to fix things for you? That's fine, it's just not what you said.

I would prefer you fix them :)

I would prefer you fix them :)

Okay, then what should we call it? --extra-packages-script since that's what it's for?

I would prefer you fix them :)

Okay, then what should we call it? --extra-packages-script since that's what it's for?

Sounds good!

And maybe change it to emit a newline delimited list of packages? I don't see a benefit to using \0 since we don't need binary safety. IMO \0 just makes it harder to implement said scripts.

durin42 updated this revision to Diff 14628.Apr 2 2019, 3:43 PM
This revision was automatically updated to reflect the committed changes.