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

multiPackage with MultiSources after remove of pickle module #4

Closed
rhubert opened this issue Feb 9, 2016 · 15 comments
Closed

multiPackage with MultiSources after remove of pickle module #4

rhubert opened this issue Feb 9, 2016 · 15 comments

Comments

@rhubert
Copy link
Contributor

rhubert commented Feb 9, 2016

Hi Jan,
I merged your commits this morning. I think db49f07 brought in a regression when using multiPackage recipes. Without this commit there is only one source directory and different build / dist directories.
Now there is a separate source directory for each multiPackage package. After reverting db49f07 it's building from the same src again.
BR
Ralf

@jkloetzke
Copy link
Member

There is a slight chance that specially crafted environment variables could go undetected. I pushed a fix for that but I don't expect that this is the case here. The commit in question would possibly reduce the detected variants, not increase them as reported.

Therefor I suspect that the old directories just got unused and new ones got created due to the changed digest algorithm. Could you please double check that all the directories are actually used (maybe "bob clean")?

@rhubert
Copy link
Contributor Author

rhubert commented Feb 10, 2016

With the latest fix it's still the same issue. I opened a pull request to the tutorials showing the issue. First time I want to build against debug version of lib:

bob dev root/
WARNING: developer mode might exhibit problems and is subject to change! Use with care.
>> root
>> root/lib-debug
   CHECKOUT  dev/src/lib-debug/1/workspace
   BUILD     dev/build/lib-debug/1/workspace
   PACKAGE   dev/dist/lib-debug/1/workspace
>> root
   CHECKOUT  dev/src/root/1/workspace
   BUILD     dev/build/root/1/workspace
   PACKAGE   dev/dist/root/1/workspace
Build result is in dev/dist/root/1/workspace

Source path was lib-debug. Now I change the root recipe to build against release lib.

bob dev root/
WARNING: developer mode might exhibit problems and is subject to change! Use with care.
>> root
   PRUNE     dev/dist/root/1/workspace (recipe changed)
>> root/lib-release
   CHECKOUT  dev/src/lib-release/1/workspace
   BUILD     dev/build/lib-release/1/workspace
   PACKAGE   dev/dist/lib-release/1/workspace
>> root
   CHECKOUT  skipped (fixed package dev/src/root/1/workspace)
   PRUNE     dev/build/root/1/workspace (recipe changed)
   BUILD     dev/build/root/1/workspace
   PACKAGE   dev/dist/root/1/workspace
Build result is in dev/dist/root/1/workspace

-> checkout the same sources into a new directory :(

But - and now I really confused - with this example there are always two source directories ( db49f07 don't care). In my project and without db49f07 I have debug and release building from the same sources?

@rhubert
Copy link
Contributor Author

rhubert commented Feb 10, 2016

I pushed another commit on my bob-tutorials branch, adding two more recipes.
root3 is depending on root and root2. root depends on lib-release. Now it doesn't matter what root2 depends on (lib-release or lib-debug), lib is always build from src/lib-release.

But if you change the dependency in root to lib-debug you'll get a new src folder?

Not sure what's the bug in this case.. :-|

@jkloetzke
Copy link
Member

Well, this is the dark side of the dev mode at work. ;-) The problem is how the dev mode calculates the used directories. The algorithm is actually quite simple:

  1. Parse all recipes into some internal representation and resolve inherited classes
  2. Starting from the root recipes traverse the whole dependency tree (depth first) and assign the directory for each step (pym/bob/cmds/build.py:724).

The key to the behavior is the developNameFormatter (build.py:291). If the digest of a step was already seen it will reuse the directory. Otherwise a new directory is assigned for the digest (derived from the recipe name). This is similar to the release mode but the dev mode does not store this assignment anywhere. It is recomputed every time bob runs.

So what directory will a common step in a multiPackage be assigned? This depends what is actually used, i.e. what is reachable from any root recipe and and in which order. Whatever is calculated first assigns the directory name of the common step. If you change the dependencies or their order it might have such an effect. The only guarantee for stable directory names across changes to the recipes is the release mode.

@rhubert
Copy link
Contributor Author

rhubert commented Feb 10, 2016

But the release mode has some other disadvantages.. What do you think about having a checkoutDir property in the recipe which is used in the develop name formatter?

@jkloetzke
Copy link
Member

We could possibly use the plain recipe name for checkout and build steps and only use the final package name in the packaging step. This would solve your particular problem. I don't think that having a dedicated checkoutDir property is a good idea.

The drawback is that the following recipe (test.yaml):

checkoutSCM:
   scm: git
   url: ...
   branch: "${BRANCH}"
multiPackage:
   foo:
      environment:
          BRANCH: alpha
   bar:
      environment:
          BRANCH: beta

would result in two checkout dirs that would be called "dev/src/test/1/..." and "dev/src/test/2/..." instead of having the real package name (test-foo, test-bar). Thoughts?

How about checkout steps of other recipes that happen to do exactly the same? Currently they are done in one directory whose name is determined by the dependency order...

@rhubert
Copy link
Contributor Author

rhubert commented Feb 11, 2016

thinking further you could have a recipe like this:

checkoutSCM:
   scm: git
   url: ...
   branch: "${BRANCH}"
multiPackage:
   foo:
      environment:
          BRANCH: alpha
   foo-debug:
      environment:
          BRANCH: alpha
          CFLAGS: "-g"
   bar:
      environment:
          BRANCH: beta
   bar-debug:
      environment:
          BRANCH: beta
          CFLAGS: "-g"

now I'd expect to get directories:

src/test-alpha/1
src/test-beta/1
build/test-bar/1 # build from src/test-beta
build/test-bar-debug/1 # build from src/test-beta
build/test-foo/1 # build from src/test-alpha
build/test-foo-debug/1 # build from src/test-alpha

so for checkoutStep directoryname should be plain recipe Name appended by x, and for build and package Step use packageName as already done.

Now x could be calculated by bob depending on scm. For git it is branch, tag or commit. SVN could use the revision. But there are still a lot of checkout possibilities not covered by this (f.e. having url checkout with url defined in multiPackage environment variable.)

That's why IMO it's more clear having a checkoutDir property:

checkoutSCM:
   scm: git
   url: ...
   branch: "${BRANCH}"

multiPackage:
   foo:
      checkoutDir: alpha
      environment:
          BRANCH: alpha
   foo-debug:
      checkoutDir: alpha
      environment:
          BRANCH: alpha
          CFLAGS: "-g"
   bar:
      checkoutDir: beta
      environment:
          BRANCH: beta
   bar-debug:
      checkoutDir: beta
      environment:
          BRANCH: beta
          CFLAGS: "-g"

Now directory is calculated as plain recipe name - checkoutDir if checkout dir is available. If it's not available use only plain-recipe name (you'll probably get: src/test/1 and src/test/2).

And of course if you do something like this:

   bar:
      checkoutDir: beta
      environment:
          BRANCH: alpha
   bar-debug:
      checkoutDir: beta
      environment:
          BRANCH: beta
          CFLAGS: "-g"

you'll get src/test-beta/1 and src/test-beta/2.

@jkloetzke
Copy link
Member

I've pushed a preliminary implementation of plugins in my fork: https://github.com/jkloetzke/bob/tree/plugins

In contrib/plugins there is a sample plugin that implements something like described above. Put it into a directory called plugins into the recipe tree and add the following to config.yaml:

plugins:
    - path-fmt

Then you can add a checkoutDir or a platform property to recipes to adjust the directory layout. Let me know if this the right approach.

@rhubert
Copy link
Contributor Author

rhubert commented Feb 29, 2016

Hi Jan,
I finally found some time to merge and test the plugin. It's working like a charm. 👍

@rhubert
Copy link
Contributor Author

rhubert commented Feb 29, 2016

... bob jenkins push:

Traceback (most recent call last):
  File "/c/projects/bob/pym/bob/scrips.py", line 76, in bob
    availableCommands[verb][0](argv, bobRoot)
  File "/c/projects/bob/pym/bob/scrips.py", line 38, in __jenkins
    doJenkins(*args, **kwargs)
  File "/c/projects/bob/pym/bob/cmds/jenkins.py", line 836, in doJenkins
    availableJenkinsCmds[args.subcommand][0](recipes, args.args)
  File "/c/projects/bob/pym/bob/cmds/jenkins.py", line 710, in doJenkinsPush
    jobXML = job.dumpXML(origXML, nodes, windows)
  File "/c/projects/bob/pym/bob/cmds/jenkins.py", line 222, in dumpXML
    whiteList.extend([ JenkinsJob.__tgzName(d) for d in self.__deps.values()])
  File "/c/projects/bob/pym/bob/cmds/jenkins.py", line 222, in <listcomp>
    whiteList.extend([ JenkinsJob.__tgzName(d) for d in self.__deps.values()])
  File "/c/projects/bob/pym/bob/cmds/jenkins.py", line 140, in __tgzName
    return d.getExecPath().replace('/', '_') + ".tgz"
  File "/c/projects/bob/pym/bob/input.py", line 685, in getExecPath
    return self.__pathFormatter(self, 'exec', self.__package._getStates())
TypeError: <lambda>() takes 2 positional arguments but 3 were given

There is a jenkinsNameFormatter in the plugin but not in normal bob code?

@jkloetzke
Copy link
Member

Nice. :-) I will have a look into the crash. The jenkins code was untested, obviously. I will have to rebase the branch on the latest master version and make some adjustments to clarify what APIs are available for plugins. This may take a couple of days...

@rhubert
Copy link
Contributor Author

rhubert commented Mar 1, 2016

I assume I made a mistake merging the branches... Let me double check this before you start... :-/

@jkloetzke
Copy link
Member

I could reproduce the crash on my branch yesterday. Some parts of the Jenkins logic were simply not adapted. I will probably push an update this evening. Attention: I will rebase my branch because there are some conflicts with the current master too. Take care when merging with your stuff then.

@jkloetzke
Copy link
Member

Hi @Rahu,

I've merged the plugins branch after some final cleanup. I think this should solve you problem. Note that the type of hooks changed a little bit (no useless factory, just the plain function). If in doubt have a look at the example in contrib/plugins.

Regards,
Jan

@rhubert
Copy link
Contributor Author

rhubert commented Mar 9, 2016

Great! Thanks for your effort. 👍

This issue was closed.
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

2 participants