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

Fix local building in git projects #1617

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

adrianschroeter
Copy link
Member

osc did not find it's store and was unable to run a local build in a project git

@pep8speaks
Copy link

pep8speaks commented Aug 29, 2024

Hello @adrianschroeter! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2024-10-14 13:19:40 UTC

@adrianschroeter
Copy link
Member Author

@wfeldt this should solve your issue

@adrianschroeter
Copy link
Member Author

note: this is not a good implementation and needs extensions if we want to support the experimental _subdirs way as well. However, it unblocks people for now again with local operations.

@adrianschroeter
Copy link
Member Author

@wfeldt can you test again if it works for you now? (it does for me)

Copy link
Contributor

@dmach dmach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should make clear how to properly detect project and package checkouts that are in git and decide how to work with their metadata.

@@ -32,7 +32,7 @@ def __init__(self, path, check=True):

# TODO: how to determine if the current git repo contains a project or a package?
self.is_project = False
self.is_package = os.path.exists(os.path.join(self.abspath, ".git"))
self.is_package = os.path.exists(os.path.join(self.abspath, ".git")) or os.path.exists(os.path.join(self.abspath, "../.git"))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you really suggesting that package is every directory that has .git in the parent dir?
We should rather find a better way of identifying project and package git checkouts.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

solved this in a better way now hopefully.

Problem here is that "is_package" is always false for the store on project git. But we still have packages ...

@@ -68,11 +68,16 @@ def apiurl(self):

@property
def project(self):
if self._project is None:
with open(os.path.join(os.path.join(self.abspath, '.osc/_project'))) as f:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extra os.path.join().

Is the .osc directory always present?
Couldn't we use it for detecting project and package names?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

at least always when used "osc co". However, using git cli now to find toplevel directory as dirk recommended.

osc/obs_scm/project.py Outdated Show resolved Hide resolved
osc/store.py Outdated Show resolved Hide resolved
@adrianschroeter adrianschroeter force-pushed the fix_local_build branch 2 times, most recently from 8cc57e0 to 06e8afb Compare September 5, 2024 09:48
Comment on lines 35 to 44
# NOTE: we have only one store in project-git for all packages
self._toplevel = self._run_git(["rev-parse", "--show-toplevel"])
if self._toplevel == self.abspath:
self.is_project = True
self.is_package = False
else:
self.is_project = False
self.is_package = True
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this is the change that broke unit tests.
See tests / unit - read only fixtures (pull_request)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am unable to find that check/failure, however I fixed a different one when executing it local.

There are still two test case failures, but IMHO the test cases are to blame here: tests/test_git_scm_store.py sets up the the work area with plain git (not osc co) and fails over missing .osc/_project later one.

IMHO this can not work, we either should set it up using "osc co" or also setup the .osc meta data.

osc/store.py Fixed Show fixed Hide fixed
osc/store.py Fixed Show fixed Hide fixed
osc/store.py Fixed Show fixed Hide fixed
osc/store.py Outdated Show resolved Hide resolved
adrianschroeter and others added 2 commits October 14, 2024 15:17
osc did not find it's store and was unable to run a local build
in a project git
Co-authored-by: Jose D. Gomez R. <[email protected]>
@dmach
Copy link
Contributor

dmach commented Oct 14, 2024

I have pushed a fixed version that passes the unit tests.
I still don't like how it guesses that a git repo contains a project or a package (some form of metadata would be nice), but it doesn't seem to be making things worse at least.

try:
with open(os.path.join(self._toplevel, '.osc/_project')) as f:
self._project = f.readline().strip()
except FileNotFoundError:

Check notice

Code scanning / CodeQL

Empty except Note

'except' clause does nothing but pass and there is no explanatory comment.
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

Successfully merging this pull request may close these issues.

4 participants