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

better merging of test project file #1707

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

00vareladavid
Copy link
Contributor

The current test project merging has two flaws:

  1. Detecting whether two dependencies are actually the same. It uses dumb string comparison without any normalization.
  2. It errors out when it detects two conflicting deps. This is just unnecessary.

This PR fixes both of those:

  1. Determine the actual location we expect to load the source from and more intelligently determine if these two paths are the same (this is samefile in the code)
  2. In the case where to deps conflicting, we should just warn, choose the dep specified by the main project, and move on (basically what @fredrikekre recommended in "cannot merge projects" error #1585 )

Additional note:
samefile should ideally just compare the filesystem node id, but the dep might not exist at this point, so we have to fallback to string comparisons in the worst case.

fix #1585

@tkf
Copy link
Member

tkf commented Mar 23, 2020

2. choose the dep specified by the main project, and move on

This is not always the best choice, as discussed in #1585 and #1714.

@schlichtanders
Copy link

If there is no agreement on 2., couldn't we just have 1. for now?

It would already solve some setups, like e.g. dev ..

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.

"cannot merge projects" error
3 participants