-
Notifications
You must be signed in to change notification settings - Fork 242
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
requirements: Add all mandatory requirements #1142
Conversation
OK, we have a problem, because avocado-36lts is called |
00d281d
to
e8bc2fb
Compare
@clebergnu would you please take a look at this one? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ldoktor after successfully installing requirements on requirements.txt
, this is what get when running this on a fresh environment:
Processing dependencies for avocado-plugins-vt==53.0
Searching for avocado-framework>=36.0
Reading https://pypi.python.org/simple/avocado-framework/
Downloading https://pypi.python.org/packages/ce/37/0b60e639fee3993fde67ae992e84d68b8266d322fd8807c7cbafe2abb696/avocado-framework-54.0.tar.gz#md5=28760d3dbde674bedf56e012f352c1c6
Best match: avocado-framework 54.0
Processing avocado-framework-54.0.tar.gz
Writing /tmp/easy_install-PscIBs/avocado-framework/setup.cfg
Running avocado-framework/setup.py -q bdist_egg --dist-dir /tmp/easy_install-PscIBs/avocado-framework/egg-dist-tmp-uS874q
warning: no files found matching 'examples/*.yaml'
Following this (put aside to aid the reading of the error itself):
error: Setup script exited with error: SandboxViolation: open('/etc/avocado/avocado.conf', 'wb') {}
The package setup script has attempted to modify files on your system
that are not within the EasyInstall build area, and has been aborted.
This package cannot be safely installed by EasyInstall, and may not
support alternate installation locations even if you run its setup
script by hand. Please inform the package's author and the EasyInstall
maintainers to find out if a fix or workaround is available.
Running pip install avocado-framework
works just fine, on the very same environment. So, it looks like the problem is having it chained inside another module (avocado-vt
in this case) setup.py.
Maybe the easy solution is to have both a requirements.txt
and a requirements-py26.txt
file?
setup.py
Outdated
# Avocado-36 was called "avocado" and newer ones are called | ||
# avocado-framework. Let's only include it in case it's not | ||
# available. | ||
from avocado import Test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the goal is to check for Avocado, the way to go is to use pkg_resources
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, I can change that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, I could detect avocado-framwork
and proceed, but in case it's not present, but avocado
is we'd have to still check whether it is our avocado, or whether it's just some other avocado module (like the django one). Using this is IMO simpler and clearer, what do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or let's just simply remove all of it. It's <36 code...
setup.py
Outdated
if sys.version_info[:2] >= (2, 7): | ||
requirements.append("avocado-framework>=36.0") | ||
else: | ||
# In avocado-36 there is a bug which requires the stevedore |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"requires stevedore" or "requires the stevedore dependency"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
Can you please provide the actual steps? I can't reproduce your issue by:
Both work fine. |
@ldoktor the biggest difference is that I tried a system wide install, running as root. That's relevant also because it's similar to what RPM package build will try to do. Let me know if you can replicate that. |
@clebergnu still unable to reproduce. Sharing the exact steps would help, this time I tried:
|
OK, I'm able to reproduce it with |
52c4aff
to
c42fbbd
Compare
@clebergnu This is not intended for merging right now, but it's WiP to tackle the packaging of Avocado-vt. I'm leaving on vacation so feel free to think about this and let me know. I still have to make it work with LTS (by making it optional) and move the rest of the Currently this version requires avocado-framework/avocado#2780 (but the real version should not require it to support 52 LTS) |
850ec0a
to
f466319
Compare
Changes
|
This pull request introduces 8 alerts and fixes 1 when merging f466319 into c7143ef - view on LGTM.com new alerts:
fixed alerts:
|
Don't run the setup on import, but only when the script is actually executed. Signed-off-by: Lukáš Doktor <[email protected]>
The requirements are quite outdated. Let's include the mandatory ones to `setup.py` and include only optional ones in `requirements.txt`. Note that the "avocado-framework" dependency excludes some older versions of avocado that were called "avocado" only, but given those versions are ancient it shouldn't be a problem. Signed-off-by: Lukáš Doktor <[email protected]>
Using the "setup.py" directly is not recommended as it invokes old and buggy easy_install. Let's use and promote "pip" installation method everywhere. Signed-off-by: Lukáš Doktor <[email protected]>
This pull request introduces 7 alerts and fixes 1 when merging 0c6d354 into c7143ef - view on LGTM.com new alerts:
fixed alerts:
|
Oups, forgot to include rpm changes and also I want to reorder to configs (insert them to 0 instead of extending). Will send it later today... |
Let's use the same naming standard that other avocado plugins are using (avocado-framework-plugin-[name]). Signed-off-by: Lucas Meneghel Rodrigues <[email protected]>
Let's build for the most recent (at the moment), Ubuntu LTS (18.04). Signed-off-by: Lucas Meneghel Rodrigues <[email protected]>
The SettingPlugin allows to extend setting paths. let's use this plugin to include our config files instead of injecting them to a magic location. Signed-off-by: Lukáš Doktor <[email protected]>
Well, we still need to deliver certain files outside the package, which is not pythonic, but hopefully fix for that can wait for next-time as if I understand it correctly it requires to deliver Anyway, new changes:
|
This pull request introduces 7 alerts and fixes 1 when merging 5ffef81 into 3001176 - view on LGTM.com new alerts:
fixed alerts:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've tested this extensively, and given that it affects aspects that would need to be dealt in the release process anyway, I'm merging these as a build/release fix. Thanks!
The requirements are quite outdated, this commit adds all of them to
both, requirements.txt and setup.py in order to just install avocado-vt
and get all required dependencies installed automatically.
Signed-off-by: Lukáš Doktor [email protected]