* Reject a build if there is a cycle among the outputs. This is
necessary because existing code assumes that the references graph is acyclic.
This commit is contained in:
		
							parent
							
								
									254b3399ba
								
							
						
					
					
						commit
						b1004f40f7
					
				
					 4 changed files with 24 additions and 9 deletions
				
			
		|  | @ -278,10 +278,6 @@ public: | ||||||
| }; | }; | ||||||
| 
 | 
 | ||||||
| 
 | 
 | ||||||
| MakeError(SubstError, Error) |  | ||||||
| MakeError(BuildError, Error) /* denotes a permanent build failure */ |  | ||||||
| 
 |  | ||||||
| 
 |  | ||||||
| //////////////////////////////////////////////////////////////////////
 | //////////////////////////////////////////////////////////////////////
 | ||||||
| 
 | 
 | ||||||
| 
 | 
 | ||||||
|  | @ -1982,7 +1978,8 @@ void DerivationGoal::computeClosure() | ||||||
|     } |     } | ||||||
| 
 | 
 | ||||||
|     /* Register each output path as valid, and register the sets of
 |     /* Register each output path as valid, and register the sets of
 | ||||||
|        paths referenced by each of them. */ |        paths referenced by each of them.  If there are cycles in the | ||||||
|  |        outputs, this will fail. */ | ||||||
|     ValidPathInfos infos; |     ValidPathInfos infos; | ||||||
|     foreach (DerivationOutputs::iterator, i, drv.outputs) { |     foreach (DerivationOutputs::iterator, i, drv.outputs) { | ||||||
|         ValidPathInfo info; |         ValidPathInfo info; | ||||||
|  |  | ||||||
|  | @ -372,8 +372,13 @@ static void addAdditionalRoots(StoreAPI & store, PathSet & roots) | ||||||
| 
 | 
 | ||||||
| 
 | 
 | ||||||
| static void dfsVisit(StoreAPI & store, const PathSet & paths, | static void dfsVisit(StoreAPI & store, const PathSet & paths, | ||||||
|     const Path & path, PathSet & visited, Paths & sorted) |     const Path & path, PathSet & visited, Paths & sorted, | ||||||
|  |     PathSet & parents) | ||||||
| { | { | ||||||
|  |     if (parents.find(path) != parents.end()) | ||||||
|  |         throw BuildError(format("cycle detected in the references of `%1%'") % path); | ||||||
|  |     parents.insert(path); | ||||||
|  |      | ||||||
|     if (visited.find(path) != visited.end()) return; |     if (visited.find(path) != visited.end()) return; | ||||||
|     visited.insert(path); |     visited.insert(path); | ||||||
|      |      | ||||||
|  | @ -385,18 +390,19 @@ static void dfsVisit(StoreAPI & store, const PathSet & paths, | ||||||
|         /* Don't traverse into paths that don't exist.  That can
 |         /* Don't traverse into paths that don't exist.  That can
 | ||||||
|            happen due to substitutes for non-existent paths. */ |            happen due to substitutes for non-existent paths. */ | ||||||
|         if (*i != path && paths.find(*i) != paths.end()) |         if (*i != path && paths.find(*i) != paths.end()) | ||||||
|             dfsVisit(store, paths, *i, visited, sorted); |             dfsVisit(store, paths, *i, visited, sorted, parents); | ||||||
| 
 | 
 | ||||||
|     sorted.push_front(path); |     sorted.push_front(path); | ||||||
|  |     parents.erase(path); | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
| 
 | 
 | ||||||
| Paths topoSortPaths(StoreAPI & store, const PathSet & paths) | Paths topoSortPaths(StoreAPI & store, const PathSet & paths) | ||||||
| { | { | ||||||
|     Paths sorted; |     Paths sorted; | ||||||
|     PathSet visited; |     PathSet visited, parents; | ||||||
|     foreach (PathSet::const_iterator, i, paths) |     foreach (PathSet::const_iterator, i, paths) | ||||||
|         dfsVisit(store, paths, *i, visited, sorted); |         dfsVisit(store, paths, *i, visited, sorted, parents); | ||||||
|     return sorted; |     return sorted; | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
|  |  | ||||||
|  | @ -966,12 +966,14 @@ void LocalStore::registerValidPaths(const ValidPathInfos & infos) | ||||||
|     while (1) { |     while (1) { | ||||||
|         try { |         try { | ||||||
|             SQLiteTxn txn(db); |             SQLiteTxn txn(db); | ||||||
|  |             PathSet paths; | ||||||
|      |      | ||||||
|             foreach (ValidPathInfos::const_iterator, i, infos) { |             foreach (ValidPathInfos::const_iterator, i, infos) { | ||||||
|                 assert(i->hash.type == htSHA256); |                 assert(i->hash.type == htSHA256); | ||||||
|                 /* !!! Maybe the registration info should be updated if the
 |                 /* !!! Maybe the registration info should be updated if the
 | ||||||
|                    path is already valid. */ |                    path is already valid. */ | ||||||
|                 if (!isValidPath(i->path)) addValidPath(*i); |                 if (!isValidPath(i->path)) addValidPath(*i); | ||||||
|  |                 paths.insert(i->path); | ||||||
|             } |             } | ||||||
| 
 | 
 | ||||||
|             foreach (ValidPathInfos::const_iterator, i, infos) { |             foreach (ValidPathInfos::const_iterator, i, infos) { | ||||||
|  | @ -980,6 +982,12 @@ void LocalStore::registerValidPaths(const ValidPathInfos & infos) | ||||||
|                     addReference(referrer, queryValidPathId(*j)); |                     addReference(referrer, queryValidPathId(*j)); | ||||||
|             } |             } | ||||||
| 
 | 
 | ||||||
|  |             /* Do a topological sort of the paths.  This will throw an
 | ||||||
|  |                error if a cycle is detected and roll back the | ||||||
|  |                transaction.  Cycles can only occur when a derivation | ||||||
|  |                has multiple outputs. */ | ||||||
|  |             topoSortPaths(*this, paths); | ||||||
|  | 
 | ||||||
|             txn.commit(); |             txn.commit(); | ||||||
|             break; |             break; | ||||||
|         } catch (SQLiteBusy & e) { |         } catch (SQLiteBusy & e) { | ||||||
|  |  | ||||||
|  | @ -349,6 +349,10 @@ void exportPaths(StoreAPI & store, const Paths & paths, | ||||||
|     bool sign, Sink & sink); |     bool sign, Sink & sink); | ||||||
| 
 | 
 | ||||||
| 
 | 
 | ||||||
|  | MakeError(SubstError, Error) | ||||||
|  | MakeError(BuildError, Error) /* denotes a permanent build failure */ | ||||||
|  | 
 | ||||||
|  | 
 | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
| 
 | 
 | ||||||
|  |  | ||||||
		Loading…
	
	Add table
		Add a link
		
	
		Reference in a new issue