AcademySoftwareFoundation / rez

An integrated package configuration, build and deployment system for software
https://rez.readthedocs.io
Apache License 2.0
945 stars 336 forks source link

Request: Allow to define multiple local/release folders #426

Open bfloch opened 7 years ago

bfloch commented 7 years ago

This includes the details for the request. https://groups.google.com/forum/#!topic/rez-config/nazmdCCg1XM

In a nutshell, in the config:

The benefit is that now different paths might act as local/release and will be correctly discovered.

To release/build to a different local/release folder you just need to override the packages_index in the package.py:

config = {
    "packages_index":  1,
}

For the sake of clean implementation it does break config compatibility (to some little degree). For example:

local_packages_path = "~/packages"
release_packages_path = "~/.rez/packages/int"

Becomes:

local_packages_paths = ["~/packages"]
release_packages_paths = ["~/.rez/packages/int"]
packages_index = 0

I will make a pull request for this feature soon.

bfloch commented 7 years ago

Here an example of how this might be used. Given a rezconfig.py

local_packages_paths = ["~/packages/inhouse", "~/packages/external",]
release_packages_paths = ["~/.rez/packages/inhouse", "~/.rez/packages/external",]
packages_index = 0

All inhouse developent packages do not need to do anything special.

In a package.py for a external package you might just:

config = {
    "packages_index":  1,
}

Now rez-build / rez-release will release to /external for this particular package. Make sure to include all the paths to the package search paths.

nerdvegas commented 7 years ago

Hey Blazej,

A couple of things:

  1. As mentioned earlier I really need to keep backwards compatibility. I think the setting name should not change (ie, local_packages_path, not local_packages_paths), and it should support both string (old format) and list (new format). I also don't want to support both local_packages_path and local_packages_paths, that's too confusing. Even though strictly speaking your new names are more correct, backwards compatibility wins!

  2. 'packages_index': If I understand correctly, this controls both the local and release path that the package will use? If so, that's probably ok for most cases, but I think there should also be the option to rez-build/rez-release to different paths as well. The name is also not quite descriptive enough - how about 'packages_path_index'?

So taking these issues into account, may I suggest something like this:

in rezconfig

local_packages_path = { # can also be a string (backwards compat) "internal": "~/packages/int", "external": "~/packages/ext" }

release_packages_path = { # can also be a string (backwards compat) "internal": "/sw/packages/int", "external": "/sw/packages/ext" }

in package.py

config = { "packages_path_index": "external", }

OR: config = { "packages_path_index": { "local": "external", "release": "external" } }

There are multiple advantages to a dict-based approach like this:

Order doesn't matter for these settings either, since its only 'packages_path' that defines search order.

Thx A

On Wed, May 17, 2017 at 6:10 AM, Blazej Floch notifications@github.com wrote:

Here an example of how this might be used. Given a rezconfig.py

local_packages_paths = ["~/packages/inhouse", "~/packages/external",] release_packages_paths = ["~/.rez/packages/inhouse", "~/.rez/packages/external",] packages_index = 0

All inhouse developent packages do not need to do anything special.

In a package.py for a external package you might just:

config = { "packages_index": 1, }

Now rez-build / rez-release will release to /external for this particular package. Make sure to include all the paths to the package search paths.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/nerdvegas/rez/issues/426#issuecomment-301900673, or mute the thread https://github.com/notifications/unsubscribe-auth/ABjqSqLw4dWrzrglx9PV5pIj6cWlSOPsks5r6gKdgaJpZM4Nc-K7 .

KelSolaar commented 7 years ago

Subscribing to this one as I'm exactly in the same situation with separation of ThirdParty and InHouse packages.

I think there should also be the option to rez-build/rez-release to different paths as well.

Yes this is probably what I would like to see, a side effect benefit is that it would not break anything while would be backward compatible.

KelSolaar commented 7 years ago

Looking at the codebase a bit, rez-build has a prefix argument to install to a custom path but rez-release does not have the equivalent and it it a bit more complicated than just adding the command line argument. It has to be passed along multiple objects/modules.

KelSolaar commented 7 years ago

So while trying to find a non-intrusive, minimalist and interim solution yesterday I ended up doing that:

in rez_configuration.py:

release_thirdparty_packages_path = "K:\\Packages\\ThirdParty"

in package.py:

def release_thirdparty_packages_path():
    import imp
    import os

    config = imp.load_source('config', os.environ['REZ_CONFIG_FILE'])

    return config.release_thirdparty_packages_path

config = {
    'release_packages_path': release_thirdparty_packages_path()}
nerdvegas commented 7 years ago

There's a less intrusive way to do something like this, see: https://github.com/nerdvegas/rez/wiki/Configuring-Rez#package_definition_build_python_paths

So, you could go like this:

in your rez config

package_definition_build_python_paths = ['/foo/rezhelpers']

in /foo/rezhelpers/conf.py

release_thirdparty_packages_path = "K:\Packages\ThirdParty"

in package.py

import conf config = { 'release_packages_path': conf.release_thirdparty_packages_path}

So the 'import conf' imports your custom python conf file, which is on sys.path because of the package_definition_build_python_paths setting. The upside to this approach is that 'conf' is available in any of your package.pys, you don't need to write extra code in each to get custom settings like this. It also more clearly delineates between builtin rez settings, and your own studio-specific settings. Oh and also, don't count on REZ_CONFIG_FILE being a single path - it can (or can in a pending PR, I don't recall atm) actually be a searchpath, not a single file.

Hth A

On Thu, Jun 1, 2017 at 9:18 AM, Thomas Mansencal notifications@github.com wrote:

So while trying to find a non-intrusive, minimalist and interim solution yesterday I ended up doing that:

in rez_configuration.py:

release_thirdparty_packages_path = "K:\Packages\ThirdParty"

in package.py:

def release_thirdparty_packages_path(): import imp import os

config = imp.load_source('config', os.environ['REZ_CONFIG_FILE'])

return config.release_thirdparty_packages_path

config = { 'release_packages_path': release_thirdparty_packages_path()}

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/nerdvegas/rez/issues/426#issuecomment-305345256, or mute the thread https://github.com/notifications/unsubscribe-auth/ABjqSrQJQRtfPHS6nB99uYDCWkz5QmWNks5r_fVDgaJpZM4Nc-K7 .

KelSolaar commented 7 years ago

Sweet, shorter and better! :)

Thanks!

KelSolaar commented 7 years ago

@nerdvegas : So it looks like the path is not appended to sys.path. I'm printing its content from package.py after issuing rez-build:

['C:\\Program Files\\rez\\Scripts\\rez\\rez-build.exe',                               
 'c:\\progra~1\\rez\\lib\\site-packages\\rez-2.13.0-py2.7.egg',                       
 'c:\\progra~1\\rez\\scripts\\python27.zip',                                          
 'c:\\progra~1\\rez\\DLLs',                                                           
 'c:\\progra~1\\rez\\lib',                                                            
 'c:\\progra~1\\rez\\lib\\plat-win',                                                  
 'c:\\progra~1\\rez\\lib\\lib-tk',                                                    
 'c:\\progra~1\\rez\\scripts',                                                        
 'C:\\Program Files\\Anaconda2\\Lib',                                                 
 'C:\\Program Files\\Anaconda2\\DLLs',                                                
 'C:\\Program Files\\Anaconda2\\Lib\\lib-tk',                                         
 'c:\\progra~1\\rez',                                                                 
 'c:\\progra~1\\rez\\lib\\site-packages']                                             
nerdvegas commented 7 years ago

Ack, it may be that that extra path is only added to sys.path for @early functions. Should be easy to change that, I think it'd be useful to be there during entire eval of package.py.

A

KelSolaar commented 7 years ago

@nerdvegas : I can open a separate issue for that if you want?

nerdvegas commented 7 years ago

Yes pls.

On Thu, Jun 1, 2017 at 11:31 AM, Thomas Mansencal notifications@github.com wrote:

@nerdvegas https://github.com/nerdvegas : I can open a separate issue for that if you want?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/nerdvegas/rez/issues/426#issuecomment-305363724, or mute the thread https://github.com/notifications/unsubscribe-auth/ABjqSsvXcYOunidz5ZB1cgSbYqIqL-h6ks5r_hRwgaJpZM4Nc-K7 .

instinct-vfx commented 7 years ago

For reference because @nerdvegas mentioned it above. Support for multiple paths is implemented in this PR https://github.com/nerdvegas/rez/pull/402

KelSolaar commented 7 years ago

@instinct-vfx : Yes, I kind of decided to not pick anything that is not merged, trying to avoid maintenance/tracking hell.

instinct-vfx commented 7 years ago

Yeah i can very well understand that. I initially went that route too, but there are some windows issues that i need fixed, and there is a bit of a backlog in regards to PRs. I just wanted to mention it as @nerdvegas referenced it above but was not sure if it was in the releases or not. Just in case someone is wondering.

KelSolaar commented 7 years ago

From Google Group, I have the feeling that there is a significantly growing amount of people that are using Windows nowadays. I suspect that the VR/AR trend with UE4/Unity is a big driver for the recent Windows retro-adoption.

Anyway, I have the feeling that it could be interesting to try consolidating the effort by gathering all the people that are on Windows. It is maybe a case of merging the relevant PR into a temporary fork, in a location we agree on and make a big PR back to nerdvegas/origin. It could also be a disaster, but nonetheless I thought I would mention that :)

instinct-vfx commented 7 years ago

I'd love that and would also contribute. We should take this to the mailing list though i guess. Also as Alan mentioned before that he would like to leave windows support to people actually using windows it would make sense to think about how to organize this in an "orderly fashion" so fixes / changes don't get lost.

KelSolaar commented 7 years ago

Waiting for @nerdvegas thoughts and if he is happy, we can probably start a thread.

nerdvegas commented 7 years ago

Hey guys,

Yes I think that separately coordinating a windows fork and consolidating all those related PRs would be a great way to go. I don't develop for Windows so my ability to do anything useful with those existing PRs is quite limited. If the relevant people get involved and are able to consolidate to one PR that addresses the current issues, then I will be happy to merge it to master.

Thx A

On Fri, Jun 2, 2017 at 7:33 AM, Thomas Mansencal notifications@github.com wrote:

Waiting for @nerdvegas https://github.com/nerdvegas thoughts and if he is happy, we can probably start a thread.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/nerdvegas/rez/issues/426#issuecomment-305627161, or mute the thread https://github.com/notifications/unsubscribe-auth/ABjqSmMIlrl3HnUuVKT7FFm0WMVqfvW_ks5r_y4tgaJpZM4Nc-K7 .

conormuirhead commented 7 years ago

👋 Hiya, not sure if y'all realize this or not, but you keep @mentioning me 😄. Any chance you could make an effort to not @mention my username: early. Thanks!

nerdvegas commented 7 years ago

Haha your username is early? We have a python function decorator called @early, hence the mentions. Might be a bit hard to avoid but I'll keep it in mind.

Thx A

On Fri, Jun 2, 2017 at 8:58 AM, Conor Muirhead notifications@github.com wrote:

👋 Hiya, not sure if y'all realize this or not, but you keep @mentioning me 😄. Any chance you could make an effort to not @mention my username: early. Thanks!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/nerdvegas/rez/issues/426#issuecomment-305643523, or mute the thread https://github.com/notifications/unsubscribe-auth/ABjqSi2MDZLRdMhHyXNDveKB39lk1uHPks5r_0ILgaJpZM4Nc-K7 .

conormuirhead commented 7 years ago

Yeah, no biggie, I figured as much :). I actually just decided to just ditch my old early username in favor of what I use elsewhere, so go ahead and @early to your hearts content!

KelSolaar commented 7 years ago

I will check next week that upstream (work) is fine with the idea of having me officially helping on the project: it is only semi-acknowledged at the moment that I poke at rez from time to time. From there I guess I will be able to help a bit on the Windows stuff.

bfloch commented 7 years ago

Hi, Just a heads up. I'm implementing the changes as suggested by @nerdvegas here: https://github.com/nerdvegas/rez/issues/426#issuecomment-302254762

Here is how the defaults will look like:

# The package search path. Rez uses this to find packages. A package with the
# same name and version in an earlier path takes precedence.
packages_path = [
    "~/packages/int",       # locally installed internal pkgs, not yet deployed
    "~/packages/ext",       # locally installed external pkgs, not yet deployed
    "~/.rez/packages/int",  # internally developed pkgs, deployed
    "~/.rez/packages/ext",  # external (3rd party) pkgs, such as houdini, boost
]

# The path that Rez will locally install packages to when rez-build is used
# The packages_path_index controls which path to use.
# Note: Can be a string for backwards compatibility.
local_packages_path = {
    "internal": "~/packages/int",
    "external": "~/packages/ext",
}

# The path that Rez will deploy packages to when rez-release is used. For
# production use, you will probably want to change this to a site-wide location.
# The packages_path_index controls which path to use.
# Note: Can be a string for backwards compatibility.
release_packages_path = {
    "internal": "~/.rez/packages/int",
    "external": "~/.rez/packages/ext",
}

# Defines which local/release_packages_paths are enabled
# Feel free to override this within a package to build/release to different locations.
# Is ignored for *_packages_path that are strings for backwards compatibility.
packages_path_index = "external"
# May also be dict for specific control over local or release
# packages_path_index = {
#     "local":   "internal",
#     "release": "external",
# }
bfloch commented 7 years ago

Pull request updated https://github.com/nerdvegas/rez/pull/428

nerdvegas commented 7 years ago

Just a couple of things:

packages_path_index: I feel like target_packages_path would be a better name here.

I'd also just make sure that if the target packages_path is ambiguous (because there's no packages_path_index but there are dict-based local/release_packages_path) then an error is raised.

Cheers

A

On Sat, Jul 1, 2017 at 8:21 AM, Blazej Floch notifications@github.com wrote:

Pull request updated #428 https://github.com/nerdvegas/rez/pull/428

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/nerdvegas/rez/issues/426#issuecomment-312385428, or mute the thread https://github.com/notifications/unsubscribe-auth/ABjqSv5oJ3iXwOcfkOp5bl80YVMpDoLFks5sJXT4gaJpZM4Nc-K7 .

bfloch commented 7 years ago

I did the rename. Pushing in a sec. As to the error: I got this already, compare: https://github.com/nerdvegas/rez/pull/428/files#r126718189

bfloch commented 6 years ago

Would love to have a thirdparty tester on the PR. We use it since 6+ months.